Refactor DMLQueryBuilder::upsert()

CHANGELOG.md

@@ -46,6 +46,7 @@
 - New #316: Realize `Schema::loadResultColumn()` method (@Tigrov)
 - New #323: Use `DateTimeColumn` class for datetime column types (@Tigrov)
 - Enh #324: Refactor `Command::insertWithReturningPks()` method (@Tigrov)
+- Enh #325: Refactor `DMLQueryBuilder::upsert()` method (@Tigrov)
 
 ## 1.3.0 March 21, 2024
 

src/Command.php

@@ -8,6 +8,7 @@
 use Yiisoft\Db\Constant\DataType;
 use Yiisoft\Db\Constant\PhpType;
 use Yiisoft\Db\Driver\Pdo\AbstractPdoCommand;
+use Yiisoft\Db\Exception\NotSupportedException;
 use Yiisoft\Db\Query\QueryInterface;
 use Yiisoft\Db\QueryBuilder\AbstractQueryBuilder;
 
@@ -36,6 +37,12 @@ public function insertWithReturningPks(string $table, array|QueryInterface $colu
             return [];
         }
 
+        if ($columns instanceof QueryInterface) {
+            throw new NotSupportedException(
+                __METHOD__ . '() is not supported by Oracle when inserting sub-query.'
+            );
+        }
+
         $params = [];
         $sql = $this->getQueryBuilder()->insert($table, $columns, $params);
 

src/DMLQueryBuilder.php

@@ -107,13 +107,15 @@ public function upsert(
         }
 
         $insertValues = [];
-        $mergeSql = 'MERGE INTO ' . $quotedTableName . ' USING (' . $values . ') "EXCLUDED" ON (' . $on . ')';
+        $quotedInsertNames = array_map($this->quoter->quoteColumnName(...), $insertNames);
 
-        foreach ($insertNames as $quotedName) {
+        foreach ($quotedInsertNames as $quotedName) {
             $insertValues[] = '"EXCLUDED".' . $quotedName;
         }
 
-        $insertSql = 'INSERT (' . implode(', ', $insertNames) . ')' . ' VALUES (' . implode(', ', $insertValues) . ')';
+        $mergeSql = 'MERGE INTO ' . $quotedTableName . ' USING (' . $values . ') "EXCLUDED" ON (' . $on . ')';
+        $insertSql = 'INSERT (' . implode(', ', $quotedInsertNames) . ')'
+            . ' VALUES (' . implode(', ', $insertValues) . ')';
 
         if ($updateColumns === false || $updateNames === []) {
             /** there are no columns to update */
@@ -123,24 +125,25 @@ public function upsert(
         if ($updateColumns === true) {
             $updateColumns = [];
             /** @psalm-var string[] $updateNames */
-            foreach ($updateNames as $quotedName) {
-                $updateColumns[$quotedName] = new Expression('"EXCLUDED".' . $quotedName);
+            foreach ($updateNames as $name) {
+                $updateColumns[$name] = new Expression('"EXCLUDED".' . $this->quoter->quoteColumnName($name));
             }
         }
 
-        [$updates, $params] = $this->prepareUpdateSets($table, $updateColumns, $params);
+        $updates = $this->prepareUpdateSets($table, $updateColumns, $params);
         $updateSql = 'UPDATE SET ' . implode(', ', $updates);
 
         return "$mergeSql WHEN MATCHED THEN $updateSql WHEN NOT MATCHED THEN $insertSql";
     }
 
-    public function upsertWithReturningPks(
+    public function upsertReturning(
         string $table,
         array|QueryInterface $insertColumns,
         array|bool $updateColumns = true,
+        array|null $returnColumns = null,
         array &$params = [],
     ): string {
-        throw new NotSupportedException(__METHOD__ . ' is not supported by Oracle.');
+        throw new NotSupportedException(__METHOD__ . '() is not supported by Oracle.');
     }
 
     protected function prepareInsertValues(string $table, array|QueryInterface $columns, array $params = []): array
@@ -152,10 +155,13 @@ protected function prepareInsertValues(string $table, array|QueryInterface $colu
 
             if ($tableSchema !== null) {
                 if (!empty($tableSchema->getPrimaryKey())) {
-                    $names = array_map($this->quoter->quoteColumnName(...), $tableSchema->getPrimaryKey());
+                    $names = $tableSchema->getPrimaryKey();
                 } else {
-                    /** @psalm-suppress PossiblyNullArgument */
-                    $names = [$this->quoter->quoteColumnName(array_key_first($tableSchema->getColumns()))];
+                    /**
+                     * @psalm-suppress PossiblyNullArgument
+                     * @var string[] $names
+                     */
+                    $names = [array_key_first($tableSchema->getColumns())];
                 }
 
                 $placeholders = array_fill(0, count($names), 'DEFAULT');

tests/CommandTest.php

@@ -14,6 +14,7 @@
 use Yiisoft\Db\Oracle\Tests\Provider\CommandProvider;
 use Yiisoft\Db\Oracle\Tests\Support\TestTrait;
 use Yiisoft\Db\Query\Query;
+use Yiisoft\Db\Query\QueryInterface;
 use Yiisoft\Db\Tests\Common\CommonCommandTest;
 use Yiisoft\Db\Tests\Support\Assert;
 use Yiisoft\Db\Transaction\TransactionInterface;
@@ -397,6 +398,14 @@ public function testInsertWithReturningPksWithPrimaryKeySignedDecimal(): void
         $db->close();
     }
 
+    public function testInsertWithReturningPksWithQuery(): void
+    {
+        $this->expectException(NotSupportedException::class);
+        $this->expectExceptionMessage('Yiisoft\Db\Oracle\Command::insertWithReturningPks() is not supported by Oracle when inserting sub-query.');
+
+        parent::testInsertWithReturningPksWithQuery();
+    }
+
     public function testInsertSelectAlias(): void
     {
         $db = $this->getConnection();
@@ -539,38 +548,64 @@ public function testUpsert(array $firstData, array $secondData): void
         parent::testUpsert($firstData, $secondData);
     }
 
-    public function testUpsertWithReturningPks(): void
+    #[DataProviderExternal(CommandProvider::class, 'upsertReturning')]
+    public function testUpsertReturning(
+        string $table,
+        array|QueryInterface $insertColumns,
+        array|bool $updateColumns,
+        array|null $returnColumns,
+        array $selectCondition,
+        array $expectedValues,
+    ): void {
+        $db = $this->getConnection();
+        $command = $db->createCommand();
+
+        $this->expectException(NotSupportedException::class);
+        $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertReturning() is not supported by Oracle.');
+
+        $command->upsertReturning($table, $insertColumns, $updateColumns, $returnColumns);
+    }
+
+    public function testUpsertReturningWithUnique(): void
+    {
+        $this->expectException(NotSupportedException::class);
+        $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertReturning() is not supported by Oracle.');
+
+        parent::testUpsertReturningWithUnique();
+    }
+
+    public function testUpsertReturningPks(): void
     {
         $db = $this->getConnection();
         $command = $db->createCommand();
 
         $this->expectException(NotSupportedException::class);
-        $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertWithReturningPks is not supported by Oracle.');
+        $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertReturning() is not supported by Oracle.');
 
-        $command->upsertWithReturningPks('{{customer}}', ['name' => 'test_1', 'email' => '[email protected]']);
+        $command->upsertReturningPks('{{customer}}', ['name' => 'test_1', 'email' => '[email protected]']);
     }
 
-    public function testUpsertWithReturningPksEmptyValues()
+    public function testUpsertReturningPksEmptyValues()
     {
         $db = $this->getConnection();
         $command = $db->createCommand();
 
         $this->expectException(NotSupportedException::class);
-        $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertWithReturningPks is not supported by Oracle.');
+        $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertReturning() is not supported by Oracle.');
 
-        $command->upsertWithReturningPks('null_values', []);
+        $command->upsertReturningPks('null_values', []);
     }
 
-    public function testUpsertWithReturningPksWithPhpTypecasting(): void
+    public function testUpsertReturningPksWithPhpTypecasting(): void
     {
         $db = $this->getConnection();
 
         $this->expectException(NotSupportedException::class);
-        $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertWithReturningPks is not supported by Oracle.');
+        $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertReturning() is not supported by Oracle.');
 
         $db->createCommand()
             ->withPhpTypecasting()
-            ->upsertWithReturningPks('notauto_pk', ['id_1' => 1, 'id_2' => 2.5, 'type' => 'test1']);
+            ->upsertReturningPks('notauto_pk', ['id_1' => 1, 'id_2' => 2.5, 'type' => 'test1']);
     }
 
     public function testQueryScalarWithBlob(): void

tests/Provider/CommandPDOProvider.php

@@ -24,7 +24,6 @@ public static function bindParam(): array
                     'name' => 'user1',
                     'address' => 'address1',
                     'status' => '1',
-                    'bool_status' => '1',
                     'profile_id' => '1',
                 ],
             ],

tests/Provider/CommandProvider.php

@@ -121,4 +121,9 @@ public static function createIndex(): array
             [['col1' => ColumnBuilder::integer()], ['col1'], IndexType::BITMAP, null],
         ];
     }
+
+    public static function upsertReturning(): array
+    {
+        return [['table', [], true, ['col1'], [], []]];
+    }
 }

tests/Provider/QueryBuilderProvider.php

@@ -189,7 +189,7 @@ public static function upsert(): array
             'values and expressions without update part' => [
                 1 => ['{{%T_upsert}}.[[email]]' => '[email protected]', '[[ts]]' => new Expression('ROUND((SYSDATE - DATE \'1970-01-01\')*24*60*60)')],
                 3 => <<<SQL
-                MERGE INTO {{%T_upsert}} USING (SELECT :qp0 AS "email", ROUND((SYSDATE - DATE '1970-01-01')*24*60*60) AS "ts" FROM "DUAL") "EXCLUDED" ON ({{%T_upsert}}."email"="EXCLUDED"."email") WHEN NOT MATCHED THEN INSERT ("email", "ts") VALUES ("EXCLUDED"."email", "EXCLUDED"."ts")
+                MERGE INTO "T_upsert" USING (SELECT :qp0 AS "email", ROUND((SYSDATE - DATE '1970-01-01')*24*60*60) AS "ts" FROM "DUAL") "EXCLUDED" ON ("T_upsert"."email"="EXCLUDED"."email") WHEN NOT MATCHED THEN INSERT ("email", "ts") VALUES ("EXCLUDED"."email", "EXCLUDED"."ts")
                 SQL,
             ],
             'query, values and expressions with update part' => [
@@ -214,7 +214,7 @@ public static function upsert(): array
                         ],
                     )->from('DUAL'),
                 3 => <<<SQL
-                MERGE INTO {{%T_upsert}} USING (SELECT :phEmail AS "email", ROUND((SYSDATE - DATE '1970-01-01')*24*60*60) AS [[ts]] FROM "DUAL") "EXCLUDED" ON ({{%T_upsert}}."email"="EXCLUDED"."email") WHEN NOT MATCHED THEN INSERT ("email", [[ts]]) VALUES ("EXCLUDED"."email", "EXCLUDED".[[ts]])
+                MERGE INTO "T_upsert" USING (SELECT :phEmail AS "email", ROUND((SYSDATE - DATE '1970-01-01')*24*60*60) AS [[ts]] FROM "DUAL") "EXCLUDED" ON ("T_upsert"."email"="EXCLUDED"."email") WHEN NOT MATCHED THEN INSERT ("email", [[ts]]) VALUES ("EXCLUDED"."email", "EXCLUDED".[[ts]])
                 SQL,
             ],
             'no columns to update' => [
@@ -224,7 +224,7 @@ public static function upsert(): array
             ],
             'no columns to update with unique' => [
                 3 => <<<SQL
-                MERGE INTO {{%T_upsert}} USING (SELECT :qp0 AS "email" FROM "DUAL") "EXCLUDED" ON ({{%T_upsert}}."email"="EXCLUDED"."email") WHEN NOT MATCHED THEN INSERT ("email") VALUES ("EXCLUDED"."email")
+                MERGE INTO "T_upsert" USING (SELECT :qp0 AS "email" FROM "DUAL") "EXCLUDED" ON ("T_upsert"."email"="EXCLUDED"."email") WHEN NOT MATCHED THEN INSERT ("email") VALUES ("EXCLUDED"."email")
                 SQL,
             ],
             'no unique columns in table - simple insert' => [

tests/QueryBuilderTest.php

@@ -480,21 +480,22 @@ public function testUpsert(
         parent::testUpsert($table, $insertColumns, $updateColumns, $expectedSql, $expectedParams);
     }
 
-    #[DataProviderExternal(QueryBuilderProvider::class, 'upsertWithReturningPks')]
-    public function testUpsertWithReturningPks(
+    #[DataProviderExternal(QueryBuilderProvider::class, 'upsertReturning')]
+    public function testUpsertReturning(
         string $table,
         array|QueryInterface $insertColumns,
         array|bool $updateColumns,
+        array|null $returnColumns,
         string $expectedSql,
         array $expectedParams
     ): void {
         $db = $this->getConnection();
         $qb = $db->getQueryBuilder();
 
         $this->expectException(NotSupportedException::class);
-        $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertWithReturningPks is not supported by Oracle.');
+        $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertReturning() is not supported by Oracle.');
 
-        $qb->upsertWithReturningPks($table, $insertColumns, $updateColumns);
+        $qb->upsertReturning($table, $insertColumns, $updateColumns);
     }
 
     public function testDefaultValues(): void

tests/Support/Fixture/oci.sql

@@ -77,7 +77,6 @@ CREATE TABLE "customer" (
   "name" varchar2(128),
   "address" varchar(4000),
   "status" integer DEFAULT 0,
-  "bool_status" char DEFAULT 0 check ("bool_status" in (0,1)),
   "profile_id" integer,
   CONSTRAINT "customer_PK" PRIMARY KEY ("id") ENABLE
 );
@@ -396,9 +395,9 @@ INSERT INTO "animal" ("type") VALUES ('yiiunit\data\ar\Dog');
 INSERT INTO "profile" ("description") VALUES ('profile customer 1');
 INSERT INTO "profile" ("description") VALUES ('profile customer 3');
 
-INSERT INTO "customer" ("email", "name", "address", "status", "bool_status", "profile_id") VALUES ('[email protected]', 'user1', 'address1', 1, 1, 1);
-INSERT INTO "customer" ("email", "name", "address", "status", "bool_status") VALUES ('[email protected]', 'user2', 'address2', 1, 1);
-INSERT INTO "customer" ("email", "name", "address", "status", "bool_status", "profile_id") VALUES ('[email protected]', 'user3', 'address3', 2, 0, 2);
+INSERT INTO "customer" ("email", "name", "address", "status", "profile_id") VALUES ('[email protected]', 'user1', 'address1', 1, 1);
+INSERT INTO "customer" ("email", "name", "address", "status") VALUES ('[email protected]', 'user2', 'address2', 1);
+INSERT INTO "customer" ("email", "name", "address", "status", "profile_id") VALUES ('[email protected]', 'user3', 'address3', 2, 2);
 
 INSERT INTO "category" ("name") VALUES ('Books');
 INSERT INTO "category" ("name") VALUES ('Movies');

tests/Support/TestTrait.php

@@ -4,8 +4,6 @@
 
 namespace Yiisoft\Db\Oracle\Tests\Support;
 
-use Yiisoft\Db\Exception\Exception;
-use Yiisoft\Db\Exception\InvalidConfigException;
 use Yiisoft\Db\Oracle\Connection;
 use Yiisoft\Db\Oracle\Dsn;
 use Yiisoft\Db\Oracle\Driver;
@@ -17,10 +15,15 @@ trait TestTrait
 
     private string $fixture = 'oci.sql';
 
-    /**
-     * @throws InvalidConfigException
-     * @throws Exception
-     */
+    public static function setUpBeforeClass(): void
+    {
+        $db = self::getDb();
+
+        DbHelper::loadFixture($db, __DIR__ . '/Fixture/oci.sql');
+
+        $db->close();
+    }
+
     protected function getConnection(bool $fixture = false): Connection
     {
         $db = new Connection($this->getDriver(), DbHelper::getSchemaCache());