From 11f981bba034dd64a26f7b159f5c54e9d937ef13 Mon Sep 17 00:00:00 2001
From: Sergei Tigrov <rrr-r@ya.ru>
Date: Wed, 1 Nov 2023 10:37:14 +0700
Subject: [PATCH] Refactor DMLQueryBuilder (#271)

* Refactor DMLQueryBuilder

* Apply fixes from StyleCI

* Add line to CHANGELOG.md

* Remove `Generator` from tests

---------

Co-authored-by: StyleCI Bot <bot@styleci.io>
---
 CHANGELOG.md                            |  1 +
 src/DMLQueryBuilder.php                 | 41 ++++++++-----------------
 tests/Provider/QueryBuilderProvider.php |  5 +++
 tests/QueryBuilderTest.php              |  3 +-
 4 files changed, 19 insertions(+), 31 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f1545601..86f60ce1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 - Enh #263: Support json type (@Tigrov)
 - Bug #268: Fix foreign keys: support multiple foreign keys referencing to one table and possible null columns for reference (@Tigrov)
+- Bug #271: Refactor `DMLQueryBuilder`, related with yiisoft/db#746 (@Tigrov)
 
 ## 1.0.1 July 24, 2023
 
diff --git a/src/DMLQueryBuilder.php b/src/DMLQueryBuilder.php
index a3ef422e..798697a0 100644
--- a/src/DMLQueryBuilder.php
+++ b/src/DMLQueryBuilder.php
@@ -8,13 +8,10 @@
 use Yiisoft\Db\Exception\InvalidArgumentException;
 use Yiisoft\Db\Exception\NotSupportedException;
 use Yiisoft\Db\Expression\Expression;
-use Yiisoft\Db\Expression\ExpressionInterface;
 use Yiisoft\Db\Query\QueryInterface;
 use Yiisoft\Db\QueryBuilder\AbstractDMLQueryBuilder;
 
 use function implode;
-use function ltrim;
-use function reset;
 
 /**
  * Implements a DML (Data Manipulation Language) SQL statements for SQLite Server.
@@ -45,8 +42,8 @@ public function resetSequence(string $table, int|string $value = null): string
         if ($value !== null) {
             $value = "'" . ((int) $value - 1) . "'";
         } else {
-            $pk = $tableSchema->getPrimaryKey();
-            $key = $this->quoter->quoteColumnName(reset($pk));
+            $key = $tableSchema->getPrimaryKey()[0];
+            $key = $this->quoter->quoteColumnName($key);
             $value = '(SELECT MAX(' . $key . ') FROM ' . $tableName . ')';
         }
 
@@ -62,11 +59,6 @@ public function upsert(
         /** @psalm-var Constraint[] $constraints */
         $constraints = [];
 
-        /**
-         * @psalm-var string[] $insertNames
-         * @psalm-var string[] $updateNames
-         * @psalm-var array<string, ExpressionInterface|string>|bool $updateColumns
-         */
         [$uniqueNames, $insertNames, $updateNames] = $this->prepareUpsertColumns(
             $table,
             $insertColumns,
@@ -78,20 +70,19 @@ public function upsert(
             return $this->insert($table, $insertColumns, $params);
         }
 
-        /** @psalm-var string[] $placeholders */
         [, $placeholders, $values, $params] = $this->prepareInsertValues($table, $insertColumns, $params);
 
-        $insertSql = 'INSERT OR IGNORE INTO '
-            . $this->quoter->quoteTableName($table)
+        $quotedTableName = $this->quoter->quoteTableName($table);
+
+        $insertSql = 'INSERT OR IGNORE INTO ' . $quotedTableName
             . (!empty($insertNames) ? ' (' . implode(', ', $insertNames) . ')' : '')
-            . (!empty($placeholders) ? ' VALUES (' . implode(', ', $placeholders) . ')' : "$values");
+            . (!empty($placeholders) ? ' VALUES (' . implode(', ', $placeholders) . ')' : ' ' . $values);
 
         if ($updateColumns === false) {
             return $insertSql;
         }
 
         $updateCondition = ['or'];
-        $quotedTableName = $this->quoter->quoteTableName($table);
 
         foreach ($constraints as $constraint) {
             $constraintCondition = ['and'];
@@ -106,13 +97,9 @@ public function upsert(
 
         if ($updateColumns === true) {
             $updateColumns = [];
-            foreach ($updateNames as $name) {
-                $quotedName = $this->quoter->quoteColumnName($name);
-
-                if (strrpos($quotedName, '.') === false) {
-                    $quotedName = "(SELECT $quotedName FROM `EXCLUDED`)";
-                }
-                $updateColumns[$name] = new Expression($quotedName);
+            /** @psalm-var string[] $updateNames */
+            foreach ($updateNames as $quotedName) {
+                $updateColumns[$quotedName] = new Expression("(SELECT $quotedName FROM `EXCLUDED`)");
             }
         }
 
@@ -120,13 +107,9 @@ public function upsert(
             return $insertSql;
         }
 
-        /** @psalm-var array $params */
-        $updateSql = 'WITH "EXCLUDED" ('
-            . implode(', ', $insertNames)
-            . ') AS (' . (!empty($placeholders)
-                ? 'VALUES (' . implode(', ', $placeholders) . ')'
-                : ltrim("$values", ' ')) . ') ' .
-                $this->update($table, $updateColumns, $updateCondition, $params);
+        $updateSql = 'WITH "EXCLUDED" (' . implode(', ', $insertNames) . ') AS ('
+            . (!empty($placeholders) ? 'VALUES (' . implode(', ', $placeholders) . ')' : $values)
+            . ') ' . $this->update($table, $updateColumns, $updateCondition, $params);
 
         return "$updateSql; $insertSql;";
     }
diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php
index 6761ba42..f4d4e5d2 100644
--- a/tests/Provider/QueryBuilderProvider.php
+++ b/tests/Provider/QueryBuilderProvider.php
@@ -176,6 +176,11 @@ public static function upsert(): array
                 WITH "EXCLUDED" (`email`, `address`, `status`, `profile_id`) AS (VALUES (:qp0, :qp1, :qp2, :qp3)) UPDATE `T_upsert` SET `address`=(SELECT `address` FROM `EXCLUDED`), `status`=(SELECT `status` FROM `EXCLUDED`), `profile_id`=(SELECT `profile_id` FROM `EXCLUDED`) WHERE `T_upsert`.`email`=(SELECT `email` FROM `EXCLUDED`); INSERT OR IGNORE INTO `T_upsert` (`email`, `address`, `status`, `profile_id`) VALUES (:qp0, :qp1, :qp2, :qp3);
                 SQL,
             ],
+            'regular values with unique at not the first position' => [
+                3 => <<<SQL
+                WITH "EXCLUDED" (`address`, `email`, `status`, `profile_id`) AS (VALUES (:qp0, :qp1, :qp2, :qp3)) UPDATE `T_upsert` SET `address`=(SELECT `address` FROM `EXCLUDED`), `status`=(SELECT `status` FROM `EXCLUDED`), `profile_id`=(SELECT `profile_id` FROM `EXCLUDED`) WHERE `T_upsert`.`email`=(SELECT `email` FROM `EXCLUDED`); INSERT OR IGNORE INTO `T_upsert` (`address`, `email`, `status`, `profile_id`) VALUES (:qp0, :qp1, :qp2, :qp3);
+                SQL,
+            ],
             'regular values with update part' => [
                 3 => <<<SQL
                 WITH "EXCLUDED" (`email`, `address`, `status`, `profile_id`) AS (VALUES (:qp0, :qp1, :qp2, :qp3)) UPDATE `T_upsert` SET `address`=:qp4, `status`=:qp5, `orders`=T_upsert.orders + 1 WHERE `T_upsert`.`email`=(SELECT `email` FROM `EXCLUDED`); INSERT OR IGNORE INTO `T_upsert` (`email`, `address`, `status`, `profile_id`) VALUES (:qp0, :qp1, :qp2, :qp3);
diff --git a/tests/QueryBuilderTest.php b/tests/QueryBuilderTest.php
index 0730ee64..f5adf0b5 100644
--- a/tests/QueryBuilderTest.php
+++ b/tests/QueryBuilderTest.php
@@ -4,7 +4,6 @@
 
 namespace Yiisoft\Db\Sqlite\Tests;
 
-use Generator;
 use JsonException;
 use Yiisoft\Db\Exception\Exception;
 use Yiisoft\Db\Exception\InvalidArgumentException;
@@ -182,7 +181,7 @@ public function testAlterColumn(): void
      * @throws NotSupportedException
      * @throws Throwable
      */
-    public function testBatchInsert(string $table, array $columns, iterable|Generator $rows, string $expected): void
+    public function testBatchInsert(string $table, array $columns, iterable $rows, string $expected): void
     {
         parent::testBatchInsert($table, $columns, $rows, $expected);
     }