From 82292b3ee234f6e22ec9d628a888962798f41d7f Mon Sep 17 00:00:00 2001 From: Tigrov Date: Thu, 22 May 2025 16:41:28 +0700 Subject: [PATCH 1/6] Refactor `DMLQueryBuilder::upsert()` --- src/DMLQueryBuilder.php | 28 ++++++++++++-------- tests/CommandTest.php | 34 ++++++++++++++++++++++--- tests/Provider/CommandProvider.php | 5 ++++ tests/Provider/QueryBuilderProvider.php | 6 ++--- tests/QueryBuilderTest.php | 9 ++++--- tests/Support/TestTrait.php | 13 +++++++--- 6 files changed, 70 insertions(+), 25 deletions(-) diff --git a/src/DMLQueryBuilder.php b/src/DMLQueryBuilder.php index b5f8aad..b6a3734 100644 --- a/src/DMLQueryBuilder.php +++ b/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 upsertWithReturning( 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'); diff --git a/tests/CommandTest.php b/tests/CommandTest.php index 1241e1d..bab4c3d 100644 --- a/tests/CommandTest.php +++ b/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; @@ -539,13 +540,40 @@ public function testUpsert(array $firstData, array $secondData): void parent::testUpsert($firstData, $secondData); } + #[DataProviderExternal(CommandProvider::class, 'upsertWithReturning')] + public function testUpsertWithReturning( + 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::upsertWithReturning() is not supported by Oracle.'); + + $command->upsertWithReturning($table, $insertColumns, $updateColumns, $returnColumns); + + } + + public function testUpsertWithReturningWithUnique(): void + { + $this->expectException(NotSupportedException::class); + $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertWithReturning() is not supported by Oracle.'); + + parent::testUpsertWithReturningWithUnique(); + } + public function testUpsertWithReturningPks(): 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::upsertWithReturning() is not supported by Oracle.'); $command->upsertWithReturningPks('{{customer}}', ['name' => 'test_1', 'email' => 'test_1@example.com']); } @@ -556,7 +584,7 @@ public function testUpsertWithReturningPksEmptyValues() $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::upsertWithReturning() is not supported by Oracle.'); $command->upsertWithReturningPks('null_values', []); } @@ -566,7 +594,7 @@ public function testUpsertWithReturningPksWithPhpTypecasting(): 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::upsertWithReturning() is not supported by Oracle.'); $db->createCommand() ->withPhpTypecasting() diff --git a/tests/Provider/CommandProvider.php b/tests/Provider/CommandProvider.php index 85842e4..bbc0a6d 100644 --- a/tests/Provider/CommandProvider.php +++ b/tests/Provider/CommandProvider.php @@ -121,4 +121,9 @@ public static function createIndex(): array [['col1' => ColumnBuilder::integer()], ['col1'], IndexType::BITMAP, null], ]; } + + public static function upsertWithReturning(): array + { + return [['table', [], true, ['col1'], [], []]]; + } } diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index e4de560..a847994 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -189,7 +189,7 @@ public static function upsert(): array 'values and expressions without update part' => [ 1 => ['{{%T_upsert}}.[[email]]' => 'dynamic@example.com', '[[ts]]' => new Expression('ROUND((SYSDATE - DATE \'1970-01-01\')*24*60*60)')], 3 => << [ @@ -214,7 +214,7 @@ public static function upsert(): array ], )->from('DUAL'), 3 => << [ @@ -224,7 +224,7 @@ public static function upsert(): array ], 'no columns to update with unique' => [ 3 => << [ diff --git a/tests/QueryBuilderTest.php b/tests/QueryBuilderTest.php index f93148b..57480f1 100644 --- a/tests/QueryBuilderTest.php +++ b/tests/QueryBuilderTest.php @@ -480,11 +480,12 @@ public function testUpsert( parent::testUpsert($table, $insertColumns, $updateColumns, $expectedSql, $expectedParams); } - #[DataProviderExternal(QueryBuilderProvider::class, 'upsertWithReturningPks')] - public function testUpsertWithReturningPks( + #[DataProviderExternal(QueryBuilderProvider::class, 'upsertWithReturning')] + public function testUpsertWithReturning( string $table, array|QueryInterface $insertColumns, array|bool $updateColumns, + array|null $returnColumns, string $expectedSql, array $expectedParams ): void { @@ -492,9 +493,9 @@ public function testUpsertWithReturningPks( $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::upsertWithReturning() is not supported by Oracle.'); - $qb->upsertWithReturningPks($table, $insertColumns, $updateColumns); + $qb->upsertWithReturning($table, $insertColumns, $updateColumns); } public function testDefaultValues(): void diff --git a/tests/Support/TestTrait.php b/tests/Support/TestTrait.php index 08a2fe3..2c7c420 100644 --- a/tests/Support/TestTrait.php +++ b/tests/Support/TestTrait.php @@ -17,10 +17,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()); From bccbff34d635855c5e28dc059590670e7bc9212e Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Sat, 24 May 2025 05:00:36 +0000 Subject: [PATCH 2/6] Apply fixes from StyleCI --- tests/CommandTest.php | 1 - tests/Support/TestTrait.php | 2 -- 2 files changed, 3 deletions(-) diff --git a/tests/CommandTest.php b/tests/CommandTest.php index bab4c3d..6c2fec4 100644 --- a/tests/CommandTest.php +++ b/tests/CommandTest.php @@ -556,7 +556,6 @@ public function testUpsertWithReturning( $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertWithReturning() is not supported by Oracle.'); $command->upsertWithReturning($table, $insertColumns, $updateColumns, $returnColumns); - } public function testUpsertWithReturningWithUnique(): void diff --git a/tests/Support/TestTrait.php b/tests/Support/TestTrait.php index 2c7c420..4715ef3 100644 --- a/tests/Support/TestTrait.php +++ b/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; From e4fea8dab176bfa78039d323221f75e0c8c4c8fc Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 24 May 2025 12:20:50 +0700 Subject: [PATCH 3/6] Fix tests --- tests/Provider/CommandPDOProvider.php | 1 - tests/Support/Fixture/oci.sql | 7 +++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/Provider/CommandPDOProvider.php b/tests/Provider/CommandPDOProvider.php index 9166b49..83a2d25 100644 --- a/tests/Provider/CommandPDOProvider.php +++ b/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', ], ], diff --git a/tests/Support/Fixture/oci.sql b/tests/Support/Fixture/oci.sql index ee06eef..6f61a00 100644 --- a/tests/Support/Fixture/oci.sql +++ b/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 ('user1@example.com', 'user1', 'address1', 1, 1, 1); -INSERT INTO "customer" ("email", "name", "address", "status", "bool_status") VALUES ('user2@example.com', 'user2', 'address2', 1, 1); -INSERT INTO "customer" ("email", "name", "address", "status", "bool_status", "profile_id") VALUES ('user3@example.com', 'user3', 'address3', 2, 0, 2); +INSERT INTO "customer" ("email", "name", "address", "status", "profile_id") VALUES ('user1@example.com', 'user1', 'address1', 1, 1); +INSERT INTO "customer" ("email", "name", "address", "status") VALUES ('user2@example.com', 'user2', 'address2', 1); +INSERT INTO "customer" ("email", "name", "address", "status", "profile_id") VALUES ('user3@example.com', 'user3', 'address3', 2, 2); INSERT INTO "category" ("name") VALUES ('Books'); INSERT INTO "category" ("name") VALUES ('Movies'); From 90524e83e2543ac9230f137062ccc766446d344e Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 24 May 2025 14:46:06 +0700 Subject: [PATCH 4/6] Add line to CHANGELOG.md [skip ci] --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c618045..17d1f0d 100644 --- a/CHANGELOG.md +++ b/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 From b07c23d4b10717e3772d1df920ea1771043d4c96 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 24 May 2025 15:51:04 +0700 Subject: [PATCH 5/6] Improve --- src/Command.php | 7 +++++++ tests/CommandTest.php | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/src/Command.php b/src/Command.php index 9f3f351..837be55 100644 --- a/src/Command.php +++ b/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); diff --git a/tests/CommandTest.php b/tests/CommandTest.php index 6c2fec4..b8a1d65 100644 --- a/tests/CommandTest.php +++ b/tests/CommandTest.php @@ -398,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(); From 0238a5ed3dbf9f4fd22a76bc3e6ba1ebdbbd0ec5 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Tue, 27 May 2025 15:21:05 +0700 Subject: [PATCH 6/6] Rename `upsertWithReturning()` to `upsertReturning()` --- src/DMLQueryBuilder.php | 2 +- tests/CommandTest.php | 32 +++++++++++++++--------------- tests/Provider/CommandProvider.php | 2 +- tests/QueryBuilderTest.php | 8 ++++---- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/DMLQueryBuilder.php b/src/DMLQueryBuilder.php index b6a3734..ef1cd77 100644 --- a/src/DMLQueryBuilder.php +++ b/src/DMLQueryBuilder.php @@ -136,7 +136,7 @@ public function upsert( return "$mergeSql WHEN MATCHED THEN $updateSql WHEN NOT MATCHED THEN $insertSql"; } - public function upsertWithReturning( + public function upsertReturning( string $table, array|QueryInterface $insertColumns, array|bool $updateColumns = true, diff --git a/tests/CommandTest.php b/tests/CommandTest.php index b8a1d65..f841eb1 100644 --- a/tests/CommandTest.php +++ b/tests/CommandTest.php @@ -548,8 +548,8 @@ public function testUpsert(array $firstData, array $secondData): void parent::testUpsert($firstData, $secondData); } - #[DataProviderExternal(CommandProvider::class, 'upsertWithReturning')] - public function testUpsertWithReturning( + #[DataProviderExternal(CommandProvider::class, 'upsertReturning')] + public function testUpsertReturning( string $table, array|QueryInterface $insertColumns, array|bool $updateColumns, @@ -561,51 +561,51 @@ public function testUpsertWithReturning( $command = $db->createCommand(); $this->expectException(NotSupportedException::class); - $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertWithReturning() is not supported by Oracle.'); + $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertReturning() is not supported by Oracle.'); - $command->upsertWithReturning($table, $insertColumns, $updateColumns, $returnColumns); + $command->upsertReturning($table, $insertColumns, $updateColumns, $returnColumns); } - public function testUpsertWithReturningWithUnique(): void + public function testUpsertReturningWithUnique(): void { $this->expectException(NotSupportedException::class); - $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertWithReturning() is not supported by Oracle.'); + $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertReturning() is not supported by Oracle.'); - parent::testUpsertWithReturningWithUnique(); + parent::testUpsertReturningWithUnique(); } - public function testUpsertWithReturningPks(): void + public function testUpsertReturningPks(): void { $db = $this->getConnection(); $command = $db->createCommand(); $this->expectException(NotSupportedException::class); - $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertWithReturning() is not supported by Oracle.'); + $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertReturning() is not supported by Oracle.'); - $command->upsertWithReturningPks('{{customer}}', ['name' => 'test_1', 'email' => 'test_1@example.com']); + $command->upsertReturningPks('{{customer}}', ['name' => 'test_1', 'email' => 'test_1@example.com']); } - public function testUpsertWithReturningPksEmptyValues() + public function testUpsertReturningPksEmptyValues() { $db = $this->getConnection(); $command = $db->createCommand(); $this->expectException(NotSupportedException::class); - $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertWithReturning() 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::upsertWithReturning() 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 diff --git a/tests/Provider/CommandProvider.php b/tests/Provider/CommandProvider.php index bbc0a6d..4a92d30 100644 --- a/tests/Provider/CommandProvider.php +++ b/tests/Provider/CommandProvider.php @@ -122,7 +122,7 @@ public static function createIndex(): array ]; } - public static function upsertWithReturning(): array + public static function upsertReturning(): array { return [['table', [], true, ['col1'], [], []]]; } diff --git a/tests/QueryBuilderTest.php b/tests/QueryBuilderTest.php index 57480f1..8f537c6 100644 --- a/tests/QueryBuilderTest.php +++ b/tests/QueryBuilderTest.php @@ -480,8 +480,8 @@ public function testUpsert( parent::testUpsert($table, $insertColumns, $updateColumns, $expectedSql, $expectedParams); } - #[DataProviderExternal(QueryBuilderProvider::class, 'upsertWithReturning')] - public function testUpsertWithReturning( + #[DataProviderExternal(QueryBuilderProvider::class, 'upsertReturning')] + public function testUpsertReturning( string $table, array|QueryInterface $insertColumns, array|bool $updateColumns, @@ -493,9 +493,9 @@ public function testUpsertWithReturning( $qb = $db->getQueryBuilder(); $this->expectException(NotSupportedException::class); - $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertWithReturning() is not supported by Oracle.'); + $this->expectExceptionMessage('Yiisoft\Db\Oracle\DMLQueryBuilder::upsertReturning() is not supported by Oracle.'); - $qb->upsertWithReturning($table, $insertColumns, $updateColumns); + $qb->upsertReturning($table, $insertColumns, $updateColumns); } public function testDefaultValues(): void