Skip to content

Commit 158e8ac

Browse files
TigrovTigrov
authored
Replace upsertWithReturningPks() with upsertWithReturning() in DMLQueryBuilder class (#410)
1 parent 1d753d6 commit 158e8ac

File tree

8 files changed

+112
-46
lines changed

8 files changed

+112
-46
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
- Enh #403, #404: Use `DbArrayHelper::arrange()` instead of `DbArrayHelper::index()` method (@Tigrov)
4141
- New #397: Realize `Schema::loadResultColumn()` method (@Tigrov)
4242
- New #407: Use `DateTimeColumn` class for datetime column types (@Tigrov)
43-
- New #408: Implement `DMLQueryBuilder::upsertWithReturningPks()` method (@Tigrov)
43+
- New #408, #410: Implement `DMLQueryBuilder::upsertReturning()` method (@Tigrov)
4444

4545
## 1.3.0 March 21, 2024
4646

‎phpunit.xml.dist

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
<?xml version="1.0" encoding="UTF-8"?>
2-
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" bootstrap="vendor/autoload.php" colors="true" failOnRisky="true" failOnWarning="true" stopOnFailure="false" executionOrder="default" resolveDependencies="true" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.1/phpunit.xsd">
2+
<phpunit
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
bootstrap="vendor/autoload.php"
5+
colors="true"
6+
failOnRisky="true"
7+
failOnWarning="true"
8+
xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/schema/10.4.xsd"
9+
>
310
<coverage/>
411
<php>
512
<ini name="error_reporting" value="-1"/>

‎src/DMLQueryBuilder.php

Lines changed: 51 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
use function array_map;
1313
use function implode;
14+
use function str_ends_with;
15+
use function substr;
1416

1517
/**
1618
* Implements a DML (Data Manipulation Language) SQL statements for PostgreSQL Server.
@@ -19,9 +21,17 @@ final class DMLQueryBuilder extends AbstractDMLQueryBuilder
1921
{
2022
public function insertWithReturningPks(string $table, array|QueryInterface $columns, array &$params = []): string
2123
{
22-
$sql = $this->insert($table, $columns, $params);
24+
$insertSql = $this->insert($table, $columns, $params);
25+
$tableSchema = $this->schema->getTableSchema($table);
26+
$primaryKeys = $tableSchema?->getPrimaryKey() ?? [];
27+
28+
if (empty($primaryKeys)) {
29+
return $insertSql;
30+
}
2331

24-
return $this->appendReturningPksClause($sql, $table);
32+
$primaryKeys = array_map($this->quoter->quoteColumnName(...), $primaryKeys);
33+
34+
return $insertSql . ' RETURNING ' . implode(', ', $primaryKeys);
2535
}
2636

2737
public function resetSequence(string $table, int|string|null $value = null): string
@@ -81,33 +91,61 @@ public function upsert(
8191
}
8292
}
8393

84-
[$updates, $params] = $this->prepareUpdateSets($table, $updateColumns, $params);
94+
$quotedUniqueNames = array_map($this->quoter->quoteColumnName(...), $uniqueNames);
95+
$updates = $this->prepareUpdateSets($table, $updateColumns, $params);
8596

8697
return $insertSql
87-
. ' ON CONFLICT (' . implode(', ', $uniqueNames) . ') DO UPDATE SET ' . implode(', ', $updates);
98+
. ' ON CONFLICT (' . implode(', ', $quotedUniqueNames) . ')'
99+
. ' DO UPDATE SET ' . implode(', ', $updates);
88100
}
89101

90-
public function upsertWithReturningPks(
102+
public function upsertReturning(
91103
string $table,
92104
array|QueryInterface $insertColumns,
93105
array|bool $updateColumns = true,
106+
array|null $returnColumns = null,
94107
array &$params = [],
95108
): string {
96-
$sql = $this->upsert($table, $insertColumns, $updateColumns, $params);
109+
$upsertSql = $this->upsert($table, $insertColumns, $updateColumns, $params);
110+
111+
$returnColumns ??= $this->schema->getTableSchema($table)?->getColumnNames();
112+
113+
if (empty($returnColumns)) {
114+
return $upsertSql;
115+
}
116+
117+
$returnColumns = array_map($this->quoter->quoteColumnName(...), $returnColumns);
118+
119+
if (str_ends_with($upsertSql, ' ON CONFLICT DO NOTHING')) {
120+
$tableName = $this->quoter->quoteTableName($table);
121+
$dummyColumn = $this->getDummyColumn($table);
122+
123+
$uniqueNames = $this->prepareUpsertColumns($table, $insertColumns, $updateColumns)[0];
124+
$quotedUniqueNames = array_map($this->quoter->quoteColumnName(...), $uniqueNames);
97125

98-
return $this->appendReturningPksClause($sql, $table);
126+
$upsertSql = substr($upsertSql, 0, -10)
127+
. '(' . implode(', ', $quotedUniqueNames) . ')'
128+
. " DO UPDATE SET $dummyColumn = $tableName.$dummyColumn";
129+
}
130+
131+
return $upsertSql . ' RETURNING ' . implode(', ', $returnColumns);
99132
}
100133

101-
private function appendReturningPksClause(string $sql, string $table): string
134+
private function getDummyColumn(string $table): string
102135
{
103-
$returnColumns = $this->schema->getTableSchema($table)?->getPrimaryKey();
136+
/** @psalm-suppress PossiblyNullReference */
137+
$columns = $this->schema->getTableSchema($table)->getColumns();
104138

105-
if (!empty($returnColumns)) {
106-
$returnColumns = array_map($this->quoter->quoteColumnName(...), $returnColumns);
139+
foreach ($columns as $column) {
140+
if ($column->isPrimaryKey() || $column->isUnique()) {
141+
continue;
142+
}
107143

108-
$sql .= ' RETURNING ' . implode(', ', $returnColumns);
144+
/** @psalm-suppress PossiblyNullArgument */
145+
return $this->quoter->quoteColumnName($column->getName());
109146
}
110147

111-
return $sql;
148+
/** @psalm-suppress PossiblyNullArgument, PossiblyFalseReference */
149+
return $this->quoter->quoteColumnName(end($columns)->getName());
112150
}
113151
}

‎tests/Provider/CommandPDOProvider.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ public static function bindParam(): array
1616
'name' => 'user1',
1717
'address' => 'address1',
1818
'status' => 1,
19-
'bool_status' => true,
2019
'profile_id' => 1,
2120
];
2221

‎tests/Provider/QueryBuilderProvider.php

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ public static function upsert(): array
265265
],
266266
'values and expressions without update part' => [
267267
1 => ['{{%T_upsert}}.[[email]]' => 'dynamic@example.com', '[[ts]]' => new Expression('extract(epoch from now()) * 1000')],
268-
3 => 'INSERT INTO {{%T_upsert}} ("email", "ts") VALUES (:qp0, extract(epoch from now()) * 1000) ON CONFLICT DO NOTHING',
268+
3 => 'INSERT INTO "T_upsert" ("email", "ts") VALUES (:qp0, extract(epoch from now()) * 1000) ON CONFLICT DO NOTHING',
269269
],
270270
'query, values and expressions with update part' => [
271271
1 => (new Query(self::getDb()))
@@ -287,13 +287,13 @@ public static function upsert(): array
287287
'[[ts]]' => new Expression('extract(epoch from now()) * 1000'),
288288
],
289289
),
290-
3 => 'INSERT INTO {{%T_upsert}} ("email", [[ts]]) SELECT :phEmail AS "email", extract(epoch from now()) * 1000 AS [[ts]] ON CONFLICT DO NOTHING',
290+
3 => 'INSERT INTO "T_upsert" ("email", [[ts]]) SELECT :phEmail AS "email", extract(epoch from now()) * 1000 AS [[ts]] ON CONFLICT DO NOTHING',
291291
],
292292
'no columns to update' => [
293293
3 => 'INSERT INTO "T_upsert_1" ("a") VALUES (:qp0) ON CONFLICT DO NOTHING',
294294
],
295295
'no columns to update with unique' => [
296-
3 => 'INSERT INTO {{%T_upsert}} ("email") VALUES (:qp0) ON CONFLICT DO NOTHING',
296+
3 => 'INSERT INTO "T_upsert" ("email") VALUES (:qp0) ON CONFLICT DO NOTHING',
297297
],
298298
'no unique columns in table - simple insert' => [
299299
3 => 'INSERT INTO {{%animal}} ("type") VALUES (:qp0)',
@@ -317,33 +317,61 @@ public static function upsert(): array
317317
return $upsert;
318318
}
319319

320-
public static function upsertWithReturningPks(): array
320+
public static function upsertReturning(): array
321321
{
322322
$upsert = self::upsert();
323323

324-
foreach ($upsert as &$data) {
325-
$data[3] .= ' RETURNING "id"';
324+
$withoutUpdate = [
325+
'regular values without update part',
326+
'query without update part',
327+
'values and expressions without update part',
328+
'query, values and expressions without update part',
329+
'no columns to update with unique',
330+
];
331+
332+
foreach ($upsert as $name => &$data) {
333+
array_splice($data, 3, 0, [['id']]);
334+
if (in_array($name, $withoutUpdate, true)) {
335+
$data[4] = substr($data[4], 0, -10) . '("email") DO UPDATE SET "ts" = "T_upsert"."ts"';
336+
}
337+
338+
$data[4] .= ' RETURNING "id"';
326339
}
327340

328-
$upsert['no columns to update'][3] = 'INSERT INTO "T_upsert_1" ("a") VALUES (:qp0) ON CONFLICT DO NOTHING RETURNING "a"';
341+
$upsert['no columns to update'][3] = ['a'];
342+
$upsert['no columns to update'][4] = 'INSERT INTO "T_upsert_1" ("a") VALUES (:qp0) ON CONFLICT ("a")'
343+
. ' DO UPDATE SET "a" = "T_upsert_1"."a" RETURNING "a"';
329344

330345
return [
331346
...$upsert,
332347
'composite primary key' => [
333348
'notauto_pk',
334349
['id_1' => 1, 'id_2' => 2.5, 'type' => 'Test'],
335350
true,
351+
['id_1', 'id_2'],
336352
'INSERT INTO "notauto_pk" ("id_1", "id_2", "type") VALUES (:qp0, :qp1, :qp2)'
337353
. ' ON CONFLICT ("id_1", "id_2") DO UPDATE SET "type"=EXCLUDED."type" RETURNING "id_1", "id_2"',
338354
[':qp0' => 1, ':qp1' => 2.5, ':qp2' => 'Test'],
339355
],
340-
'no primary key' => [
356+
'no return columns' => [
341357
'type',
342358
['int_col' => 3, 'char_col' => 'a', 'float_col' => 1.2, 'bool_col' => true],
343359
true,
360+
[],
344361
'INSERT INTO "type" ("int_col", "char_col", "float_col", "bool_col") VALUES (:qp0, :qp1, :qp2, :qp3)',
345362
[':qp0' => 3, ':qp1' => 'a', ':qp2' => 1.2, ':qp3' => true],
346363
],
364+
'return all columns' => [
365+
'T_upsert',
366+
['email' => 'test@example.com', 'address' => 'test address', 'status' => 1, 'profile_id' => 1],
367+
true,
368+
null,
369+
'INSERT INTO "T_upsert" ("email", "address", "status", "profile_id") VALUES (:qp0, :qp1, :qp2, :qp3)'
370+
. ' ON CONFLICT ("email") DO UPDATE SET'
371+
. ' "address"=EXCLUDED."address", "status"=EXCLUDED."status", "profile_id"=EXCLUDED."profile_id"'
372+
. ' RETURNING "id", "ts", "email", "recovery_email", "address", "status", "orders", "profile_id"',
373+
[':qp0' => 'test@example.com', ':qp1' => 'test address', ':qp2' => 1, ':qp3' => 1],
374+
],
347375
];
348376
}
349377

‎tests/QueryBuilderTest.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -437,15 +437,16 @@ public function testUpsert(
437437
parent::testUpsert($table, $insertColumns, $updateColumns, $expectedSql, $expectedParams);
438438
}
439439

440-
#[DataProviderExternal(QueryBuilderProvider::class, 'upsertWithReturningPks')]
441-
public function testUpsertWithReturningPks(
440+
#[DataProviderExternal(QueryBuilderProvider::class, 'upsertReturning')]
441+
public function testUpsertReturning(
442442
string $table,
443443
array|QueryInterface $insertColumns,
444444
array|bool $updateColumns,
445+
array|null $returnColumns,
445446
string $expectedSql,
446447
array $expectedParams
447448
): void {
448-
parent::testUpsertWithReturningPks($table, $insertColumns, $updateColumns, $expectedSql, $expectedParams);
449+
parent::testUpsertReturning($table, $insertColumns, $updateColumns, $returnColumns, $expectedSql, $expectedParams);
449450
}
450451

451452
#[DataProviderExternal(QueryBuilderProvider::class, 'selectScalar')]

‎tests/Support/Fixture/pgsql.sql

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ CREATE TABLE "customer" (
7676
name varchar(128),
7777
address text,
7878
status integer DEFAULT 0,
79-
bool_status boolean DEFAULT FALSE,
8079
profile_id integer
8180
);
8281

@@ -270,9 +269,9 @@ INSERT INTO "profile" (description) VALUES ('profile customer 3');
270269
INSERT INTO "schema1"."profile" (description) VALUES ('profile customer 1');
271270
INSERT INTO "schema1"."profile" (description) VALUES ('profile customer 3');
272271

273-
INSERT INTO "customer" (email, name, address, status, bool_status, profile_id) VALUES ('user1@example.com', 'user1', 'address1', 1, true, 1);
274-
INSERT INTO "customer" (email, name, address, status, bool_status) VALUES ('user2@example.com', 'user2', 'address2', 1, true);
275-
INSERT INTO "customer" (email, name, address, status, bool_status, profile_id) VALUES ('user3@example.com', 'user3', 'address3', 2, false, 2);
272+
INSERT INTO "customer" (email, name, address, status, profile_id) VALUES ('user1@example.com', 'user1', 'address1', 1, 1);
273+
INSERT INTO "customer" (email, name, address, status) VALUES ('user2@example.com', 'user2', 'address2', 1);
274+
INSERT INTO "customer" (email, name, address, status, profile_id) VALUES ('user3@example.com', 'user3', 'address3', 2, 2);
276275

277276
INSERT INTO "category" (name) VALUES ('Books');
278277
INSERT INTO "category" (name) VALUES ('Movies');

‎tests/Support/TestTrait.php

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44

55
namespace Yiisoft\Db\Pgsql\Tests\Support;
66

7-
use Yiisoft\Db\Exception\Exception;
8-
use Yiisoft\Db\Exception\InvalidConfigException;
97
use Yiisoft\Db\Pgsql\Connection;
108
use Yiisoft\Db\Pgsql\Driver;
119
use Yiisoft\Db\Pgsql\Dsn;
@@ -16,10 +14,15 @@ trait TestTrait
1614
private string $dsn = '';
1715
private string $fixture = 'pgsql.sql';
1816

19-
/**
20-
* @throws InvalidConfigException
21-
* @throws Exception
22-
*/
17+
public static function setUpBeforeClass(): void
18+
{
19+
$db = self::getDb();
20+
21+
DbHelper::loadFixture($db, __DIR__ . '/Fixture/pgsql.sql');
22+
23+
$db->close();
24+
}
25+
2326
protected function getConnection(bool $fixture = false): Connection
2427
{
2528
$db = new Connection($this->getDriver(), DbHelper::getSchemaCache());
@@ -72,15 +75,6 @@ protected function setFixture(string $fixture): void
7275
$this->fixture = $fixture;
7376
}
7477

75-
public static function setUpBeforeClass(): void
76-
{
77-
$db = self::getDb();
78-
79-
DbHelper::loadFixture($db, __DIR__ . '/Fixture/pgsql.sql');
80-
81-
$db->close();
82-
}
83-
8478
protected function getDriver(): Driver
8579
{
8680
$driver = new Driver($this->getDsn(), self::getUsername(), self::getPassword());

0 commit comments

Comments
 (0)