Fix for upsert behavior (#189)

* Fix for upsert behavior * disable rector
yiisoft · Dec 28, 2022 · a5b57d2 · a5b57d2
1 parent ac6373d
commit a5b57d2
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 43 deletions.
diff --git a/.github/workflows/rector.yml → .github/workflows/rector.yml_ b/.github/workflows/rector.yml → .github/workflows/rector.yml_
diff --git a/src/DMLQueryBuilder.php b/src/DMLQueryBuilder.php
@@ -133,10 +133,10 @@ public function upsert(
                     'VALUES(' . $this->quoter->quoteColumnName($name) . ')'
                 );
             }
-        } elseif ($updateColumns === false) {
-            $columnName = (string) reset($uniqueNames);
-            $name = $this->quoter->quoteColumnName($columnName);
-            $updateColumns = [$name => new Expression($this->quoter->quoteTableName($table) . '.' . $name)];
+        }
+
+        if (empty($updateColumns)) {
+            return str_replace('INSERT INTO', 'INSERT IGNORE INTO', $insertSql);
         }
 
         /**

diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php
@@ -108,68 +108,54 @@ public function upsert(): array
     {
         $concreteData = [
             'regular values' => [
-                3 => <<<SQL
-                INSERT INTO `T_upsert` (`email`, `address`, `status`, `profile_id`) VALUES (:qp0, :qp1, :qp2, :qp3) ON DUPLICATE KEY UPDATE `address`=VALUES(`address`), `status`=VALUES(`status`), `profile_id`=VALUES(`profile_id`)
-                SQL,
+                3 => 'INSERT INTO `T_upsert` (`email`, `address`, `status`, `profile_id`) VALUES (:qp0, :qp1, :qp2, :qp3) ' .
+                    'ON DUPLICATE KEY UPDATE `address`=VALUES(`address`), `status`=VALUES(`status`), `profile_id`=VALUES(`profile_id`)',
             ],
             'regular values with update part' => [
-                3 => <<<SQL
-                INSERT INTO `T_upsert` (`email`, `address`, `status`, `profile_id`) VALUES (:qp0, :qp1, :qp2, :qp3) ON DUPLICATE KEY UPDATE `address`=:qp4, `status`=:qp5, `orders`=T_upsert.orders + 1
-                SQL,
+                3 => 'INSERT INTO `T_upsert` (`email`, `address`, `status`, `profile_id`) VALUES (:qp0, :qp1, :qp2, :qp3) ' .
+                    'ON DUPLICATE KEY UPDATE `address`=:qp4, `status`=:qp5, `orders`=T_upsert.orders + 1',
             ],
             'regular values without update part' => [
-                3 => <<<SQL
-                INSERT INTO `T_upsert` (`email`, `address`, `status`, `profile_id`) VALUES (:qp0, :qp1, :qp2, :qp3) ON DUPLICATE KEY UPDATE `email`=`T_upsert`.`email`
-                SQL,
+                3 => 'INSERT IGNORE INTO `T_upsert` (`email`, `address`, `status`, `profile_id`) VALUES (:qp0, :qp1, :qp2, :qp3)',
             ],
             'query' => [
-                3 => <<<SQL
-                INSERT INTO `T_upsert` (`email`, `status`) SELECT `email`, 2 AS `status` FROM `customer` WHERE `name`=:qp0 LIMIT 1 ON DUPLICATE KEY UPDATE `status`=VALUES(`status`)
-                SQL,
+                3 => 'INSERT INTO `T_upsert` (`email`, `status`) SELECT `email`, 2 AS `status` FROM `customer` ' .
+                    'WHERE `name`=:qp0 LIMIT 1 ON DUPLICATE KEY UPDATE `status`=VALUES(`status`)',
             ],
             'query with update part' => [
-                3 => <<<SQL
-                INSERT INTO `T_upsert` (`email`, `status`) SELECT `email`, 2 AS `status` FROM `customer` WHERE `name`=:qp0 LIMIT 1 ON DUPLICATE KEY UPDATE `address`=:qp1, `status`=:qp2, `orders`=T_upsert.orders + 1
-                SQL,
+                3 => 'INSERT INTO `T_upsert` (`email`, `status`) SELECT `email`, 2 AS `status` FROM `customer` ' .
+                    'WHERE `name`=:qp0 LIMIT 1 ON DUPLICATE KEY UPDATE `address`=:qp1, `status`=:qp2, `orders`=T_upsert.orders + 1',
             ],
             'query without update part' => [
-                3 => <<<SQL
-                INSERT INTO `T_upsert` (`email`, `status`) SELECT `email`, 2 AS `status` FROM `customer` WHERE `name`=:qp0 LIMIT 1 ON DUPLICATE KEY UPDATE `email`=`T_upsert`.`email`
-                SQL,
+                3 => 'INSERT IGNORE INTO `T_upsert` (`email`, `status`) SELECT `email`, 2 AS `status` FROM `customer` ' .
+                    'WHERE `name`=:qp0 LIMIT 1',
             ],
             'values and expressions' => [
-                3 => <<<SQL
-                INSERT INTO {{%T_upsert}} ({{%T_upsert}}.[[email]], [[ts]]) VALUES (:qp0, now())
-                SQL,
+                3 => 'INSERT INTO {{%T_upsert}} ({{%T_upsert}}.[[email]], [[ts]]) VALUES (:qp0, CURRENT_TIMESTAMP) ' .
+                    'ON DUPLICATE KEY UPDATE [[ts]]=VALUES([[ts]])',
             ],
             'values and expressions with update part' => [
-                3 => <<<SQL
-                INSERT INTO {{%T_upsert}} ({{%T_upsert}}.[[email]], [[ts]]) VALUES (:qp0, now())
-                SQL,
+                3 => 'INSERT INTO {{%T_upsert}} ({{%T_upsert}}.[[email]], [[ts]]) VALUES (:qp0, CURRENT_TIMESTAMP) ' .
+                    'ON DUPLICATE KEY UPDATE [[orders]]=T_upsert.orders + 1',
             ],
             'values and expressions without update part' => [
-                3 => <<<SQL
-                INSERT INTO {{%T_upsert}} ({{%T_upsert}}.[[email]], [[ts]]) VALUES (:qp0, now())
-                SQL,
+                3 => 'INSERT IGNORE INTO {{%T_upsert}} ({{%T_upsert}}.[[email]], [[ts]]) VALUES (:qp0, CURRENT_TIMESTAMP)',
             ],
             'query, values and expressions with update part' => [
-                3 => <<<SQL
-                INSERT INTO {{%T_upsert}} (`email`, [[time]]) SELECT :phEmail AS `email`, now() AS [[time]] ON DUPLICATE KEY UPDATE `ts`=:qp1, [[orders]]=T_upsert.orders + 1
-                SQL,
+                3 => 'INSERT INTO {{%T_upsert}} (`email`, [[ts]]) SELECT :phEmail AS `email`, CURRENT_TIMESTAMP AS [[ts]] ' .
+                    'ON DUPLICATE KEY UPDATE `ts`=:qp1, [[orders]]=T_upsert.orders + 1',
             ],
             'query, values and expressions without update part' => [
-                3 => <<<SQL
-                INSERT INTO {{%T_upsert}} (`email`, [[time]]) SELECT :phEmail AS `email`, now() AS [[time]] ON DUPLICATE KEY UPDATE `ts`=:qp1, [[orders]]=T_upsert.orders + 1
-                SQL,
+                3 => 'INSERT IGNORE INTO {{%T_upsert}} (`email`, [[ts]]) SELECT :phEmail AS `email`, CURRENT_TIMESTAMP AS [[ts]]',
             ],
             'no columns to update' => [
-                3 => <<<SQL
-                INSERT INTO `T_upsert_1` (`a`) VALUES (:qp0) ON DUPLICATE KEY UPDATE `a`=`T_upsert_1`.`a`
-                SQL,
+                3 => 'INSERT IGNORE INTO `T_upsert_1` (`a`) VALUES (:qp0)',
             ],
-            // @todo - SQL code have a bug. Need fix in next PR
             'no columns to update with unique' => [
-                3 => 'INSERT INTO {{%T_upsert}} (`email`) VALUES (:qp0) ON DUPLICATE KEY UPDATE ',
+                3 => 'INSERT IGNORE INTO {{%T_upsert}} (`email`) VALUES (:qp0)',
+            ],
+            'no unique columns in table - simple insert' => [
+                3 => 'INSERT INTO {{%animal}} (`type`) VALUES (:qp0)',
             ],
         ];
 

diff --git a/tests/QueryBuilderTest.php b/tests/QueryBuilderTest.php
@@ -555,4 +555,15 @@ public function testUpsert(
     ): void {
         parent::testUpsert($table, $insertColumns, $updateColumns, $expectedSQL, $expectedParams);
     }
+
+    /**
+     * @dataProvider \Yiisoft\Db\Mysql\Tests\Provider\QueryBuilderProvider::upsert()
+     */
+    public function testUpsertExecute(
+        string $table,
+        array|QueryInterface $insertColumns,
+        array|bool $updateColumns
+    ): void {
+        parent::testUpsertExecute($table, $insertColumns, $updateColumns);
+    }
 }
diff --git a/tests/Support/Fixture/mysql.sql b/tests/Support/Fixture/mysql.sql
@@ -374,7 +374,7 @@ ENGINE = 'InnoDB' DEFAULT CHARSET = 'utf8';
 CREATE TABLE `T_upsert`
 (
     `id` INT NOT NULL PRIMARY KEY AUTO_INCREMENT,
-    `ts` INT NULL,
+    `ts` BIGINT NULL,
     `email` VARCHAR(128) NOT NULL UNIQUE,
     `recovery_email` VARCHAR(128) NULL,
     `address` TEXT NULL,