Fix #17504: Fix upsert when `$updateColumns = true` but there are no columns to update in the table

framework/CHANGELOG.md

@@ -4,6 +4,7 @@ Yii Framework 2 Change Log
 2.0.26 under development
 ------------------------
 
+- Bug #17504: Fix upsert when `$updateColumns = true` but there are no columns to update in the table (alexkart)
 - Bug #17511: Fixed IPv6 subnets matching in `IpHelper::inRange()` (kamarton)
 
 

framework/db/cubrid/QueryBuilder.php

@@ -70,6 +70,10 @@ class QueryBuilder extends \yii\db\QueryBuilder
         if (empty($uniqueNames)) {
             return $this->insert($table, $insertColumns, $params);
         }
+        if ($updateNames === []) {
+            // there are no columns to update
+            $updateColumns = false;
+        }
 
         $onCondition = ['or'];
         $quotedTableName = $this->db->quoteTableName($table);

framework/db/mssql/QueryBuilder.php

@@ -454,6 +454,10 @@ class QueryBuilder extends \yii\db\QueryBuilder
         if (empty($uniqueNames)) {
             return $this->insert($table, $insertColumns, $params);
         }
+        if ($updateNames === []) {
+            // there are no columns to update
+            $updateColumns = false;
+        }
 
         $onCondition = ['or'];
         $quotedTableName = $this->db->quoteTableName($table);

framework/db/mysql/QueryBuilder.php

@@ -278,6 +278,10 @@ class QueryBuilder extends \yii\db\QueryBuilder
         if (empty($uniqueNames)) {
             return $insertSql;
         }
+        if ($updateNames === []) {
+            // there are no columns to update
+            $updateColumns = false;
+        }
 
         if ($updateColumns === true) {
             $updateColumns = [];

framework/db/oci/QueryBuilder.php

@@ -218,6 +218,10 @@ EOD;
         if (empty($uniqueNames)) {
             return $this->insert($table, $insertColumns, $params);
         }
+        if ($updateNames === []) {
+            // there are no columns to update
+            $updateColumns = false;
+        }
 
         $onCondition = ['or'];
         $quotedTableName = $this->db->quoteTableName($table);

framework/db/pgsql/QueryBuilder.php

@@ -338,6 +338,10 @@ class QueryBuilder extends \yii\db\QueryBuilder
         if (empty($uniqueNames)) {
             return $insertSql;
         }
+        if ($updateNames === []) {
+            // there are no columns to update
+            $updateColumns = false;
+        }
 
         if ($updateColumns === false) {
             return "$insertSql ON CONFLICT DO NOTHING";
@@ -368,6 +372,10 @@ class QueryBuilder extends \yii\db\QueryBuilder
         if (empty($uniqueNames)) {
             return $this->insert($table, $insertColumns, $params);
         }
+        if ($updateNames === []) {
+            // there are no columns to update
+            $updateColumns = false;
+        }
 
         /** @var Schema $schema */
         $schema = $this->db->getSchema();

framework/db/sqlite/QueryBuilder.php

@@ -74,6 +74,10 @@ class QueryBuilder extends \yii\db\QueryBuilder
         if (empty($uniqueNames)) {
             return $this->insert($table, $insertColumns, $params);
         }
+        if ($updateNames === []) {
+            // there are no columns to update
+            $updateColumns = false;
+        }
 
         list(, $placeholders, $values, $params) = $this->prepareInsertValues($table, $insertColumns, $params);
         $insertSql = 'INSERT OR IGNORE INTO ' . $this->db->quoteTableName($table)

tests/data/mssql.sql

@@ -22,6 +22,7 @@ IF OBJECT_ID('[T_constraints_3]', 'U') IS NOT NULL DROP TABLE [T_constraints_3];
 IF OBJECT_ID('[T_constraints_2]', 'U') IS NOT NULL DROP TABLE [T_constraints_2];
 IF OBJECT_ID('[T_constraints_1]', 'U') IS NOT NULL DROP TABLE [T_constraints_1];
 IF OBJECT_ID('[T_upsert]', 'U') IS NOT NULL DROP TABLE [T_upsert];
+IF OBJECT_ID('[T_upsert_1]', 'U') IS NOT NULL DROP TABLE [T_upsert_1];
 IF OBJECT_ID('[table.with.special.characters]', 'U') IS NOT NULL DROP TABLE [table.with.special.characters];
 IF OBJECT_ID('[stranger ''table]', 'U') IS NOT NULL DROP TABLE [stranger 'table];
 
@@ -329,6 +330,12 @@ CREATE TABLE [T_upsert]
     UNIQUE ([email], [recovery_email])
 );
 
+CREATE TABLE [T_upsert_1]
+(
+    [a] INT NOT NULL,
+    UNIQUE ([a])
+);
+
 CREATE TABLE [dbo].[table.with.special.characters] (
     [id] [int]
 );

tests/data/mysql.sql

@@ -32,6 +32,7 @@ DROP TABLE IF EXISTS `T_constraints_3` CASCADE;
 DROP TABLE IF EXISTS `T_constraints_2` CASCADE;
 DROP TABLE IF EXISTS `T_constraints_1` CASCADE;
 DROP TABLE IF EXISTS `T_upsert` CASCADE;
+DROP TABLE IF EXISTS `T_upsert_1`;
 
 CREATE TABLE `constraints`
 (
@@ -396,3 +397,8 @@ CREATE TABLE `T_upsert`
     UNIQUE (`email`, `recovery_email`)
 )
 ENGINE = 'InnoDB' DEFAULT CHARSET = 'utf8';
+
+CREATE TABLE `T_upsert_1` (
+  `a` int(11) NOT NULL,
+  PRIMARY KEY (`a`)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;

tests/data/postgres.sql

@@ -33,6 +33,7 @@ DROP TABLE IF EXISTS "T_constraints_3";
 DROP TABLE IF EXISTS "T_constraints_2";
 DROP TABLE IF EXISTS "T_constraints_1";
 DROP TABLE IF EXISTS "T_upsert";
+DROP TABLE IF EXISTS "T_upsert_1";
 
 DROP SCHEMA IF EXISTS "schema1" CASCADE;
 DROP SCHEMA IF EXISTS "schema2" CASCADE;
@@ -416,3 +417,8 @@ CREATE TABLE "T_upsert"
     "profile_id" INT NULL,
     UNIQUE ("email", "recovery_email")
 );
+
+CREATE TABLE "T_upsert_1"
+(
+    "a" INT NOT NULL PRIMARY KEY
+);

tests/data/sqlite.sql

@@ -29,6 +29,7 @@ DROP TABLE IF EXISTS "T_constraints_3";
 DROP TABLE IF EXISTS "T_constraints_2";
 DROP TABLE IF EXISTS "T_constraints_1";
 DROP TABLE IF EXISTS "T_upsert";
+DROP TABLE IF EXISTS "T_upsert_1";
 
 CREATE TABLE "profile" (
   id INTEGER NOT NULL,
@@ -354,3 +355,8 @@ CREATE TABLE "T_upsert"
     "profile_id" INT NULL,
     UNIQUE ("email", "recovery_email")
 );
+
+CREATE TABLE "T_upsert_1"
+(
+    "a" INTEGER NOT NULL PRIMARY KEY
+);

tests/framework/db/QueryBuilderTest.php

@@ -2171,6 +2171,17 @@ abstract class QueryBuilderTest extends DatabaseTestCase
                     ':qp1' => 0,
                 ],
             ],
+            'no columns to update' => [
+                'T_upsert_1',
+                [
+                    'a' => 1,
+                ],
+                true,
+                null,
+                [
+                    ':qp0' => 1,
+                ],
+            ],
         ];
     }
 
@@ -2182,6 +2193,8 @@ abstract class QueryBuilderTest extends DatabaseTestCase
      * @param array|null $updateColumns
      * @param string|string[] $expectedSQL
      * @param array $expectedParams
+     * @throws \yii\base\NotSupportedException
+     * @throws \Exception
      */
     public function testUpsert($table, $insertColumns, $updateColumns, $expectedSQL, $expectedParams)
     {

tests/framework/db/cubrid/QueryBuilderTest.php

@@ -107,6 +107,10 @@ class QueryBuilderTest extends \yiiunit\framework\db\QueryBuilderTest
         foreach ($concreteData as $testName => $data) {
             $newData[$testName] = array_replace($newData[$testName], $data);
         }
+
+        // skip test
+        unset($newData['no columns to update']);
+
         return $newData;
     }
 }

tests/framework/db/mssql/QueryBuilderTest.php

@@ -311,6 +311,9 @@ class QueryBuilderTest extends \yiiunit\framework\db\QueryBuilderTest
             'query, values and expressions without update part' => [
                 3 => 'MERGE {{%T_upsert}} WITH (HOLDLOCK) USING (SELECT :phEmail AS [email], now() AS [[time]]) AS [EXCLUDED] ([email], [[time]]) ON ({{%T_upsert}}.[email]=[EXCLUDED].[email]) WHEN MATCHED THEN UPDATE SET [ts]=:qp1, [[orders]]=T_upsert.orders + 1 WHEN NOT MATCHED THEN INSERT ([email], [[time]]) VALUES ([EXCLUDED].[email], [EXCLUDED].[[time]]);',
             ],
+            'no columns to update' => [
+                3 => 'MERGE [T_upsert_1] WITH (HOLDLOCK) USING (VALUES (:qp0)) AS [EXCLUDED] ([a]) ON ([T_upsert_1].[a]=[EXCLUDED].[a]) WHEN NOT MATCHED THEN INSERT ([a]) VALUES ([EXCLUDED].[a]);',
+            ],
         ];
         $newData = parent::upsertProvider();
         foreach ($concreteData as $testName => $data) {

tests/framework/db/mysql/QueryBuilderTest.php

@@ -256,6 +256,9 @@ class QueryBuilderTest extends \yiiunit\framework\db\QueryBuilderTest
             'query, values and expressions without update part' => [
                 3 => 'INSERT INTO {{%T_upsert}} (`email`, [[time]]) SELECT :phEmail AS `email`, now() AS [[time]] ON DUPLICATE KEY UPDATE `ts`=:qp1, [[orders]]=T_upsert.orders + 1',
             ],
+            'no columns to update' => [
+                3 => 'INSERT INTO `T_upsert_1` (`a`) VALUES (:qp0) ON DUPLICATE KEY UPDATE `a`=`T_upsert_1`.`a`',
+            ],
         ];
         $newData = parent::upsertProvider();
         foreach ($concreteData as $testName => $data) {

tests/framework/db/oci/QueryBuilderTest.php

@@ -239,6 +239,10 @@ WHERE rownum <= 1) "EXCLUDED" ON ("T_upsert"."email"="EXCLUDED"."email") WHEN NO
         foreach ($concreteData as $testName => $data) {
             $newData[$testName] = array_replace($newData[$testName], $data);
         }
+
+        // skip test
+        unset($newData['no columns to update']);
+
         return $newData;
     }
 }

tests/framework/db/pgsql/QueryBuilderTest.php

@@ -347,6 +347,12 @@ class QueryBuilderTest extends \yiiunit\framework\db\QueryBuilderTest
                     'INSERT INTO {{%T_upsert}} ("email", [[time]]) SELECT :phEmail AS "email", now() AS [[time]] ON CONFLICT ("email") DO UPDATE SET "ts"=:qp1, [[orders]]=T_upsert.orders + 1',
                 ],
             ],
+            'no columns to update' => [
+                3 => [
+                    'WITH "EXCLUDED" ("a") AS (VALUES (CAST(:qp0 AS int2))) INSERT INTO "T_upsert_1" ("a") SELECT "a" FROM "EXCLUDED" WHERE NOT EXISTS (SELECT 1 FROM "T_upsert_1" WHERE (("T_upsert_1"."a"="EXCLUDED"."a")))',
+                    'INSERT INTO "T_upsert_1" ("a") VALUES (:qp0) ON CONFLICT DO NOTHING',
+                ],
+            ],
         ];
         $newData = parent::upsertProvider();
         foreach ($concreteData as $testName => $data) {

tests/framework/db/sqlite/QueryBuilderTest.php

@@ -192,6 +192,9 @@ class QueryBuilderTest extends \yiiunit\framework\db\QueryBuilderTest
             'query, values and expressions without update part' => [
                 3 => 'WITH "EXCLUDED" (`email`, [[time]]) AS (SELECT :phEmail AS `email`, now() AS [[time]]) UPDATE {{%T_upsert}} SET `ts`=:qp1, [[orders]]=T_upsert.orders + 1 WHERE {{%T_upsert}}.`email`=(SELECT `email` FROM `EXCLUDED`); INSERT OR IGNORE INTO {{%T_upsert}} (`email`, [[time]]) SELECT :phEmail AS `email`, now() AS [[time]];',
             ],
+            'no columns to update' => [
+                3 => 'INSERT OR IGNORE INTO `T_upsert_1` (`a`) VALUES (:qp0)',
+            ],
         ];
         $newData = parent::upsertProvider();
         foreach ($concreteData as $testName => $data) {