fix: cast OCI8 batch subquery values so updateBatch() and deleteBatch() accept mixed PHP types

Author: paulbalandan

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<string>                 $keys
+     * @param list<list<float|int|string>> $values
+     *
+     * @return array<string, int>
+     */
+    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<array-key, RawSql|string> $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;
+    }
 }

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);

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);

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.