From bc86edfc76cc2936790bb287b23242bf19d4d6bb Mon Sep 17 00:00:00 2001 From: "John Paul E. Balandan, CPA" Date: Tue, 6 Oct 2026 18:07:54 +0800 Subject: [PATCH] fix: cast OCI8 batch subquery values so `updateBatch()` and `deleteBatch()` accept mixed PHP types --- system/Database/OCI8/Builder.php | 105 ++++++++++++++++++-- tests/system/Database/Live/DeleteTest.php | 8 -- tests/system/Database/Live/UpdateTest.php | 12 --- user_guide_src/source/changelogs/v4.7.5.rst | 2 +- 4 files changed, 100 insertions(+), 27 deletions(-) diff --git a/system/Database/OCI8/Builder.php b/system/Database/OCI8/Builder.php index 118f237f597e..abdaf10ae4a1 100644 --- a/system/Database/OCI8/Builder.php +++ b/system/Database/OCI8/Builder.php @@ -16,6 +16,7 @@ use CodeIgniter\Database\BaseBuilder; use CodeIgniter\Database\Exceptions\DatabaseException; use CodeIgniter\Database\RawSql; +use stdClass; /** * Builder for OCI8 @@ -282,14 +283,21 @@ protected function _updateBatch(string $table, array $keys, array $values): stri if (isset($this->QBOptions['setQueryAsData'])) { $data = $this->QBOptions['setQueryAsData']; } else { + $updateFields = $this->QBOptions['updateFields'] ?? []; + $lengths = $this->longestValues($keys, $values); + $data = implode( " UNION ALL\n", array_map( - static fn ($value): string => 'SELECT ' . implode(', ', array_map( - static fn ($key, $index): string => $index . ' ' . $key, + fn (array $value): string => sprintf('SELECT %s FROM DUAL', implode(', ', array_map( + fn (string $key, float|int|string $index): string => sprintf( + '%s %s', + $this->cast((string) $index, $this->getSourceField($table, $key, $updateFields), $lengths[$key]), + $key, + ), $keys, $value, - )) . ' FROM DUAL', + ))), $values, ), ) . "\n"; @@ -466,14 +474,21 @@ protected function _deleteBatch(string $table, array $keys, array $values): stri if (isset($this->QBOptions['setQueryAsData'])) { $data = $this->QBOptions['setQueryAsData']; } else { + $constraints = $this->QBOptions['constraints'] ?? []; + $lengths = $this->longestValues($keys, $values); + $data = implode( " FROM DUAL UNION ALL\n", array_map( - static fn ($value): string => 'SELECT ' . implode(', ', array_map( - static fn ($key, $index): string => $index . ' ' . $key, + fn (array $value): string => sprintf('SELECT %s', implode(', ', array_map( + fn (string $key, float|int|string $index): string => sprintf( + '%s %s', + $this->cast((string) $index, $this->getSourceField($table, $key, $constraints), $lengths[$key]), + $key, + ), $keys, $value, - )), + ))), $values, ), ) . " FROM DUAL\n"; @@ -489,4 +504,82 @@ protected function fieldsFromQuery(string $sql): array { return $this->db->query('SELECT * FROM (' . $sql . ') "_u_" WHERE ROWNUM = 1')->getFieldNames(); } + + /** + * Returns the longest escaped value per key across the batch. + * + * @param list $keys + * @param list> $values + * + * @return array + */ + private function longestValues(array $keys, array $values): array + { + $lengths = []; + + foreach ($keys as $i => $key) { + $lengths[$key] = max(array_map(static fn (array $row): int => mb_strlen((string) $row[$i]), $values)); + } + + return $lengths; + } + + /** + * Returns the field shared by every destination column fed from the source key, or null when their types differ. + * + * @param array $map Destination column to source key. + */ + private function getSourceField(string $table, string $key, array $map): ?stdClass + { + $columns = array_filter(array_keys($map, $key, true), is_string(...)); + + if ($columns === []) { + return $this->getField($table, $key); + } + + $fields = array_map(fn (string $column): ?stdClass => $this->getField($table, $column), $columns); + $types = array_unique(array_map(static fn (?stdClass $field): string => $field instanceof stdClass ? $field->type . '(' . $field->max_length . ')' : '', $fields)); + + return count($types) === 1 ? reset($fields) : null; + } + + /** + * Returns the literal converted to the field type, or unchanged when the field is unknown or a LOB. + * + * @param int $length Longest value for the key in the batch. + */ + private function cast(string $value, ?stdClass $field, int $length): string + { + if (! $field instanceof stdClass) { + return $value; + } + + return match ($field->type) { + // Every row gets the same fixed width, wide enough for the longest value, so the UNION ALL stays CHAR and nothing is truncated. + 'CHAR', 'NCHAR' => sprintf('CAST(%s AS %s(%d))', $value, $field->type, max((int) $field->max_length, $length)), + 'VARCHAR2' => sprintf('TO_CHAR(%s)', $value), + 'NVARCHAR2' => sprintf('TO_NCHAR(%s)', $value), + 'RAW', 'LONG RAW', 'CLOB', 'NCLOB', 'BLOB', 'BFILE', 'LONG' => $value, + default => sprintf('CAST(%s AS %s)', $value, $field->type), + }; + } + + /** + * @param string $table Protected table name. + * @param string $fieldName Field name. May be protected. + */ + private function getField(string $table, string $fieldName): ?stdClass + { + $fieldName = trim($fieldName, $this->db->escapeChar); + + if (! isset($this->QBOptions['fields'][$table])) { + $this->QBOptions['fields'][$table] = []; + + foreach ($this->db->getFieldData($table) as $field) { + $this->QBOptions['fields'][$table][$field->name] = $field; + } + } + + return $this->QBOptions['fields'][$table][$fieldName] ?? null; + } } diff --git a/tests/system/Database/Live/DeleteTest.php b/tests/system/Database/Live/DeleteTest.php index daef35068386..8ef937fb00f5 100644 --- a/tests/system/Database/Live/DeleteTest.php +++ b/tests/system/Database/Live/DeleteTest.php @@ -125,10 +125,6 @@ public function testDeleteBatchWithMixedConstraintValueTypesInTextColumn(): void $this->markTestSkipped('SQL Server cannot compare `text` columns with `=`.'); } - if ($this->db->DBDriver === 'OCI8') { - $this->markTestSkipped('TODO: the OCI8 builder does not cast mixed `UNION ALL` values yet. Remove this skip once it does.'); - } - $table = 'type_test'; $builder = $this->db->table($table); @@ -161,10 +157,6 @@ public function testDeleteBatchWithMixedConstraintValueTypesInTextColumn(): void public function testDeleteBatchDoesNotTruncateConstraintValueForCharColumn(): void { - if ($this->db->DBDriver === 'OCI8') { - $this->markTestSkipped('TODO: Oracle resolves a `UNION ALL` of `CHAR` literals with different lengths to `VARCHAR2`, so the OCI8 builder must cast them. Remove this skip once it does.'); - } - $table = 'type_test'; $builder = $this->db->table($table); diff --git a/tests/system/Database/Live/UpdateTest.php b/tests/system/Database/Live/UpdateTest.php index 4e3944a6c6c2..41efa9b86c17 100644 --- a/tests/system/Database/Live/UpdateTest.php +++ b/tests/system/Database/Live/UpdateTest.php @@ -284,10 +284,6 @@ public function testUpdateBatchWithMixedValueTypesInTextColumn(): void $this->markTestSkipped('SQL Server cannot compare `text` columns with `=`.'); } - if ($this->db->DBDriver === 'OCI8') { - $this->markTestSkipped('TODO: the OCI8 builder does not cast mixed `UNION ALL` values yet. Remove this skip once it does.'); - } - $table = 'type_test'; $builder = $this->db->table($table); @@ -319,10 +315,6 @@ public function testUpdateBatchWithMixedValueTypesInTextColumn(): void public function testUpdateBatchDoesNotTruncateConstraintValueForCharColumn(): void { - if ($this->db->DBDriver === 'OCI8') { - $this->markTestSkipped('TODO: Oracle resolves a `UNION ALL` of `CHAR` literals with different lengths to `VARCHAR2`, so the OCI8 builder must cast them. Remove this skip once it does.'); - } - $table = 'type_test'; $builder = $this->db->table($table); @@ -358,10 +350,6 @@ public function testUpdateBatchWithMappedUpdateFieldsAndMixedValueTypes(): void $this->markTestSkipped('SQL Server cannot compare `text` columns with `=`.'); } - if ($this->db->DBDriver === 'OCI8') { - $this->markTestSkipped('TODO: the OCI8 builder does not cast mixed `UNION ALL` values yet. Remove this skip once it does.'); - } - $table = 'type_test'; $builder = $this->db->table($table); diff --git a/user_guide_src/source/changelogs/v4.7.5.rst b/user_guide_src/source/changelogs/v4.7.5.rst index 45416eef17da..600a9a7892b4 100644 --- a/user_guide_src/source/changelogs/v4.7.5.rst +++ b/user_guide_src/source/changelogs/v4.7.5.rst @@ -55,7 +55,7 @@ Bugs Fixed - **Database:** Fixed a bug where rebuilding a SQLite3 table (e.g., ``Forge::dropColumn()``, ``Forge::modifyColumn()``, ``Forge::dropForeignKey()`` and ``Forge::dropPrimaryKey()``) corrupted the table names referenced by its foreign keys when ``DBPrefix`` was set. - **Database:** Fixed a bug where Postgre query failures were silently ignored when ``DBDebug`` was enabled and PHP warnings were disabled. A ``DatabaseException`` is now thrown. - **Database:** Fixed a bug where ``getFieldData()`` failed for a schema-qualified table name on Postgre, SQLSRV and SQLite3, and where ``protectIdentifiers()`` prefixed an already protected segment of a dotted identifier a second time. -- **Database:** Fixed a bug where ``updateBatch()`` and ``deleteBatch()`` failed on PostgreSQL when the rows supplied different PHP types (e.g., a string and an integer) for the same column, because the ``UNION ALL`` subquery values were not cast to the column type. +- **Database:** Fixed a bug where ``updateBatch()`` and ``deleteBatch()`` failed on PostgreSQL and Oracle when the rows supplied different PHP types (e.g., a string and an integer) for the same column, because the ``UNION ALL`` subquery values were not cast to the column type. - **Debug:** Fixed a bug where ``Timer::start()`` treated ``0.0`` as an empty value and substituted the current time. - **Files:** Fixed a bug where ``File::move()`` and ``UploadedFile::move()`` set executable and overly permissive file permissions (``0777 & ~umask()`` instead of ``0666 & ~umask()``), and ``UploadedFile::move()`` targeted the parent directory instead of the destination file for ``chmod()``. - **Helpers:** Fixed a bug where ``get_dir_file_info()`` returned incomplete entries for subdirectories and missing files instead of omitting them.