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

Author: paulbalandan

system/Database/OCI8/Builder.php

@@ -282,14 +282,24 @@ protected function _updateBatch(string $table, array $keys, array $values): stri
         if (isset($this->QBOptions['setQueryAsData'])) {
             $data = $this->QBOptions['setQueryAsData'];
         } else {
+            $updateFields = $this->QBOptions['updateFields'] ?? [];
+
             $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(
+                        function (string $key, float|int|string $index) use ($table, $updateFields): string {
+                            $column = array_search($key, $updateFields, true);
+
+                            return sprintf(
+                                '%s %s',
+                                $this->cast((string) $index, $this->getFieldType($table, is_string($column) ? $column : $key)),
+                                $key,
+                            );
+                        },
                         $keys,
                         $value,
-                    )) . ' FROM DUAL',
+                    ))),
                     $values,
                 ),
             ) . "\n";
@@ -466,14 +476,24 @@ protected function _deleteBatch(string $table, array $keys, array $values): stri
         if (isset($this->QBOptions['setQueryAsData'])) {
             $data = $this->QBOptions['setQueryAsData'];
         } else {
+            $constraints = $this->QBOptions['constraints'] ?? [];
+
             $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(
+                        function (string $key, float|int|string $index) use ($table, $constraints): string {
+                            $column = array_search($key, $constraints, true);
+
+                            return sprintf(
+                                '%s %s',
+                                $this->cast((string) $index, $this->getFieldType($table, is_string($column) ? $column : $key)),
+                                $key,
+                            );
+                        },
                         $keys,
                         $value,
-                    )),
+                    ))),
                     $values,
                 ),
             ) . " FROM DUAL\n";
@@ -489,4 +509,39 @@ protected function fieldsFromQuery(string $sql): array
     {
         return $this->db->query('SELECT * FROM (' . $sql . ') "_u_" WHERE ROWNUM = 1')->getFieldNames();
     }
+
+    private function cast(string $expression, ?string $type): string
+    {
+        return $type === null ? $expression : sprintf('CAST(%s AS %s)', $expression, $type);
+    }
+
+    /**
+     * Returns the type to cast a literal to for the field, or null when the field is unknown or a LOB.
+     *
+     * @param string $table     Protected table name.
+     * @param string $fieldName Field name. May be protected.
+     */
+    private function getFieldType(string $table, string $fieldName): ?string
+    {
+        $fieldName = trim($fieldName, $this->db->escapeChar);
+
+        if (! isset($this->QBOptions['fieldTypes'][$table])) {
+            $this->QBOptions['fieldTypes'][$table] = [];
+
+            foreach ($this->db->getFieldData(str_replace($this->db->escapeChar, '', $table)) as $field) {
+                $this->QBOptions['fieldTypes'][$table][$field->name] = $this->castType($field->type, $field->max_length);
+            }
+        }
+
+        return $this->QBOptions['fieldTypes'][$table][$fieldName] ?? null;
+    }
+
+    private function castType(string $type, int|string|null $maxLength): ?string
+    {
+        return match ($type) {
+            'CHAR', 'NCHAR', 'VARCHAR2', 'NVARCHAR2', 'RAW'      => sprintf('%s(%d)', $type, $maxLength),
+            'CLOB', 'NCLOB', 'BLOB', 'BFILE', 'LONG', 'LONG RAW' => null,
+            default                                              => $type,
+        };
+    }
 }

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

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);
@@ -323,10 +319,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

@@ -54,7 +54,7 @@ Bugs Fixed
 - **Cookie:** Fixed a bug where ``Cookie`` instances allowed invalid characters in path, domain, and prefix attributes rejected by ``setcookie()`` and ``setrawcookie()``.
 - **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 ``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.