Fix DMLQueryBuilder::insertBatch() method (#285)

yiisoft · Oct 18, 2024 · 51becbe · 51becbe
1 parent b6d0334
commit 51becbe
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,7 @@
 - New #280: Realize `ColumnBuilder` class (@Tigrov)
 - Enh #281: Update according changes in `ColumnSchemaInterface` (@Tigrov)
 - New #282: Add `ColumnDefinitionBuilder` class (@Tigrov)
+- Bug #285: Fix `DMLQueryBuilder::insertBatch()` method (@Tigrov)
 - Enh #283: Refactor `Dsn` class (@Tigrov)
 
 ## 1.3.0 March 21, 2024

diff --git a/src/DMLQueryBuilder.php b/src/DMLQueryBuilder.php
@@ -37,17 +37,15 @@ public function insertBatch(string $table, iterable $rows, array $columns = [],
             return '';
         }
 
-        $tableAndColumns = ' INTO ' . $this->quoter->quoteTableName($table);
+        $query = 'INSERT INTO ' . $this->quoter->quoteTableName($table);
 
         if (count($columns) > 0) {
             $quotedColumnNames = array_map($this->quoter->quoteColumnName(...), $columns);
 
-            $tableAndColumns .= ' (' . implode(', ', $quotedColumnNames) . ')';
+            $query .= ' (' . implode(', ', $quotedColumnNames) . ')';
         }
 
-        $tableAndColumns .= ' VALUES ';
-
-        return 'INSERT ALL' . $tableAndColumns . implode($tableAndColumns, $values) . ' SELECT 1 FROM SYS.DUAL';
+        return $query . "\nSELECT " . implode(" FROM DUAL UNION ALL\nSELECT ", $values) . ' FROM DUAL';
     }
 
     public function insertWithReturningPks(string $table, QueryInterface|array $columns, array &$params = []): string

diff --git a/tests/CommandTest.php b/tests/CommandTest.php
@@ -74,6 +74,35 @@ public function testBatchInsert(
         parent::testBatchInsert($table, $values, $columns, $expected, $expectedParams, $insertedRow);
     }
 
+    /** @link https://github.com/yiisoft/db-oracle/issues/284 */
+    public function testBatchInsertWithAutoincrement(): void
+    {
+        $db = $this->getConnection();
+        $command = $db->createCommand();
+
+        try {
+            $command->dropTable('test_batch_autoincrement')->execute();
+        } catch (Exception) {
+        }
+
+        $command->createTable('test_batch_autoincrement', [
+            'id' => PseudoType::PK,
+            'name' => ColumnType::STRING,
+        ])->execute();
+
+        $command->insertBatch('test_batch_autoincrement', [['name' => 'John'], ['name' => 'Emma']])->execute();
+
+        $this->assertSame(
+            [
+                ['id' => '1', 'name' => 'John'],
+                ['id' => '2', 'name' => 'Emma'],
+            ],
+            (new Query($db))->from('test_batch_autoincrement')->all()
+        );
+
+        $db->close();
+    }
+
     /**
      * @throws Exception
      * @throws InvalidConfigException

diff --git a/tests/Provider/CommandProvider.php b/tests/Provider/CommandProvider.php
@@ -24,13 +24,11 @@ public static function batchInsert(): array
     {
         $batchInsert = parent::batchInsert();
 
-        $batchInsert['multirow']['expected'] = <<<SQL
-        INSERT ALL INTO "type" ("int_col", "float_col", "char_col", "bool_col") VALUES (:qp0, :qp1, :qp2, :qp3) INTO "type" ("int_col", "float_col", "char_col", "bool_col") VALUES (:qp4, :qp5, :qp6, :qp7) SELECT 1 FROM SYS.DUAL
-        SQL;
-        $batchInsert['multirow']['expectedParams'][':qp3'] = '1';
-        $batchInsert['multirow']['expectedParams'][':qp7'] = '0';
-
         $replaceParams = [
+            'multirow' => [
+                ':qp3' => '1',
+                ':qp7' => '0',
+            ],
             'issue11242' => [
                 ':qp3' => '1',
             ],

diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php
@@ -48,18 +48,9 @@ public static function batchInsert(): array
     {
         $batchInsert = parent::batchInsert();
 
-        DbHelper::changeSqlForOracleBatchInsert($batchInsert['simple']['expected']);
-        DbHelper::changeSqlForOracleBatchInsert($batchInsert['escape-danger-chars']['expected']);
-        DbHelper::changeSqlForOracleBatchInsert($batchInsert['customer3']['expected']);
-        DbHelper::changeSqlForOracleBatchInsert($batchInsert['bool-false, bool2-null']['expected']);
-
-        $batchInsert['wrong']['expected'] = <<<SQL
-        INSERT ALL INTO {{%type}} ("float_col", "time") VALUES (:qp0, now()) INTO {{%type}} ("float_col", "time") VALUES (:qp1, now()) SELECT 1 FROM SYS.DUAL
-        SQL;
-
-        DbHelper::changeSqlForOracleBatchInsert($batchInsert['bool-false, time-now()']['expected']);
-        DbHelper::changeSqlForOracleBatchInsert($batchInsert['column table names are not checked']['expected']);
-        DbHelper::changeSqlForOracleBatchInsert($batchInsert['empty columns and non-exists table']['expected']);
+        foreach ($batchInsert as $key => $value) {
+            DbHelper::changeSqlForOracleBatchInsert($batchInsert[$key]['expected']);
+        }
 
         $batchInsert['bool-false, bool2-null']['expectedParams'][':qp0'] = '0';
         $batchInsert['bool-false, time-now()']['expectedParams'][':qp0'] = '0';