Skip to content

Commit 488296a

Browse files
TigrovTigrov
authored
Replace upsertWithReturningPks() with upsertWithReturning() in DMLQueryBuilder class (#357)
1 parent ad058cd commit 488296a

File tree

7 files changed

+130
-36
lines changed

7 files changed

+130
-36
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@
3838
- New #348: Realize `Schema::loadResultColumn()` method (@Tigrov)
3939
- New #354: Add `FOR` clause to query (@vjik)
4040
- New #355: Use `DateTimeColumn` class for datetime column types (@Tigrov)
41-
- New #356: Implement `DMLQueryBuilder::upsertWithReturningPks()` method (@Tigrov)
42-
- Enh #356: Refactor `Command::insertWithReturningPks()` and `DMLQueryBuilder::upsert()` methods (@Tigrov)
41+
- New #356, #357: Implement `DMLQueryBuilder::upsertReturning()` method (@Tigrov)
42+
- Enh #356, #357: Refactor `Command::insertWithReturningPks()` and `DMLQueryBuilder::upsert()` methods (@Tigrov)
4343

4444
## 1.2.0 March 21, 2024
4545

‎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" backupGlobals="false" colors="true" bootstrap="vendor/autoload.php" failOnRisky="true" failOnWarning="true" 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+
colors="true"
5+
bootstrap="vendor/autoload.php"
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/Command.php

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,35 +27,49 @@ final class Command extends AbstractPdoCommand
2727
{
2828
public function insertWithReturningPks(string $table, array|QueryInterface $columns): array|false
2929
{
30+
$tableSchema = $this->db->getSchema()->getTableSchema($table);
31+
$primaryKeys = $tableSchema?->getPrimaryKey() ?? [];
32+
$tableColumns = $tableSchema?->getColumns() ?? [];
33+
34+
foreach ($primaryKeys as $name) {
35+
/** @var ColumnInterface $column */
36+
$column = $tableColumns[$name];
37+
38+
if ($column->isAutoIncrement()) {
39+
continue;
40+
}
41+
42+
if ($columns instanceof QueryInterface) {
43+
throw new NotSupportedException(
44+
__METHOD__ . '() is not supported by SQLite for tables without auto increment when inserting sub-query.'
45+
);
46+
}
47+
48+
break;
49+
}
50+
3051
$params = [];
31-
$sql = $this->db->getQueryBuilder()->insert($table, $columns, $params);
32-
$this->setSql($sql)->bindValues($params);
52+
$insertSql = $this->db->getQueryBuilder()->insert($table, $columns, $params);
53+
$this->setSql($insertSql)->bindValues($params);
3354

3455
if ($this->execute() === 0) {
3556
return false;
3657
}
3758

38-
$tableSchema = $this->db->getSchema()->getTableSchema($table);
39-
$tablePrimaryKeys = $tableSchema?->getPrimaryKey() ?? [];
40-
41-
if (empty($tablePrimaryKeys)) {
59+
if (empty($primaryKeys)) {
4260
return [];
4361
}
4462

45-
if ($columns instanceof QueryInterface) {
46-
throw new NotSupportedException(__METHOD__ . '() not supported for QueryInterface by SQLite.');
47-
}
48-
4963
$result = [];
5064

51-
/** @var TableSchema $tableSchema */
52-
foreach ($tablePrimaryKeys as $name) {
65+
foreach ($primaryKeys as $name) {
5366
/** @var ColumnInterface $column */
54-
$column = $tableSchema->getColumn($name);
67+
$column = $tableColumns[$name];
5568

5669
if ($column->isAutoIncrement()) {
5770
$value = $this->db->getLastInsertId();
5871
} else {
72+
/** @var array $columns */
5973
$value = $columns[$name] ?? $column->getDefaultValue();
6074
}
6175

‎src/DMLQueryBuilder.php

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -80,27 +80,55 @@ public function upsert(
8080
}
8181
}
8282

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

8586
return $insertSql
86-
. ' ON CONFLICT (' . implode(', ', $uniqueNames) . ') DO UPDATE SET ' . implode(', ', $updates);
87+
. ' ON CONFLICT (' . implode(', ', $quotedUniqueNames) . ')'
88+
. ' DO UPDATE SET ' . implode(', ', $updates);
8789
}
8890

89-
public function upsertWithReturningPks(
91+
public function upsertReturning(
9092
string $table,
9193
array|QueryInterface $insertColumns,
9294
array|bool $updateColumns = true,
95+
array|null $returnColumns = null,
9396
array &$params = [],
9497
): string {
95-
$sql = $this->upsert($table, $insertColumns, $updateColumns, $params);
96-
$returnColumns = $this->schema->getTableSchema($table)?->getPrimaryKey();
98+
$upsertSql = $this->upsert($table, $insertColumns, $updateColumns, $params);
9799

98-
if (!empty($returnColumns)) {
99-
$returnColumns = array_map($this->quoter->quoteColumnName(...), $returnColumns);
100+
$returnColumns ??= $this->schema->getTableSchema($table)?->getColumnNames();
100101

101-
$sql .= ' RETURNING ' . implode(', ', $returnColumns);
102+
if (empty($returnColumns)) {
103+
return $upsertSql;
102104
}
103105

104-
return $sql;
106+
$returnColumns = array_map($this->quoter->quoteColumnName(...), $returnColumns);
107+
108+
if (str_ends_with($upsertSql, ' ON CONFLICT DO NOTHING')) {
109+
$dummyColumn = $this->getDummyColumn($table);
110+
111+
$upsertSql = substr($upsertSql, 0, -10) . "DO UPDATE SET $dummyColumn = $dummyColumn";
112+
}
113+
114+
return $upsertSql . ' RETURNING ' . implode(', ', $returnColumns);
115+
}
116+
117+
private function getDummyColumn(string $table): string
118+
{
119+
/** @psalm-suppress PossiblyNullReference */
120+
$columns = $this->schema->getTableSchema($table)->getColumns();
121+
122+
foreach ($columns as $column) {
123+
if ($column->isPrimaryKey() || $column->isUnique()) {
124+
continue;
125+
}
126+
127+
/** @psalm-suppress PossiblyNullArgument */
128+
return $this->quoter->quoteColumnName($column->getName());
129+
}
130+
131+
/** @psalm-suppress PossiblyNullArgument, PossiblyFalseReference */
132+
return $this->quoter->quoteColumnName(end($columns)->getName());
105133
}
106134
}

‎tests/CommandTest.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use Yiisoft\Db\Exception\Exception;
1111
use Yiisoft\Db\Exception\InvalidConfigException;
1212
use Yiisoft\Db\Exception\NotSupportedException;
13+
use Yiisoft\Db\Query\Query;
1314
use Yiisoft\Db\Sqlite\Tests\Support\TestTrait;
1415
use Yiisoft\Db\Tests\Common\CommonCommandTest;
1516

@@ -326,6 +327,21 @@ public function testGetRawSql(string $sql, array $params, string $expectedRawSql
326327
parent::testGetRawSql($sql, $params, $expectedRawSql);
327328
}
328329

330+
public function testInsertWithReturningPksWithSubqueryAndNoAutoincrement(): void
331+
{
332+
$db = $this->getConnection(true);
333+
$command = $db->createCommand();
334+
335+
$query = (new Query($db))->select(['order_id' => 1, 'item_id' => 2, 'quantity' => 3, 'subtotal' => 4]);
336+
337+
$this->expectException(NotSupportedException::class);
338+
$this->expectExceptionMessage(
339+
'Yiisoft\Db\Sqlite\Command::insertWithReturningPks() is not supported by SQLite for tables without auto increment when inserting sub-query.'
340+
);
341+
342+
$command->insertWithReturningPks('order_item', $query);
343+
}
344+
329345
/**
330346
* @throws Throwable
331347
* @throws InvalidConfigException

‎tests/Provider/QueryBuilderProvider.php

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use Yiisoft\Db\Tests\Support\TraversableObject;
1414

1515
use function array_replace;
16+
use function array_splice;
1617

1718
final class QueryBuilderProvider extends \Yiisoft\Db\Tests\Provider\QueryBuilderProvider
1819
{
@@ -160,7 +161,7 @@ public static function upsert(): array
160161
],
161162
'values and expressions without update part' => [
162163
3 => <<<SQL
163-
INSERT INTO {{%T_upsert}} (`email`, `ts`) VALUES (:qp0, CURRENT_TIMESTAMP) ON CONFLICT DO NOTHING
164+
INSERT INTO `T_upsert` (`email`, `ts`) VALUES (:qp0, CURRENT_TIMESTAMP) ON CONFLICT DO NOTHING
164165
SQL,
165166
],
166167
'query, values and expressions with update part' => [
@@ -170,7 +171,7 @@ public static function upsert(): array
170171
],
171172
'query, values and expressions without update part' => [
172173
3 => <<<SQL
173-
INSERT INTO {{%T_upsert}} (`email`, [[ts]]) SELECT :phEmail AS `email`, CURRENT_TIMESTAMP AS [[ts]] ON CONFLICT DO NOTHING
174+
INSERT INTO `T_upsert` (`email`, [[ts]]) SELECT :phEmail AS `email`, CURRENT_TIMESTAMP AS [[ts]] ON CONFLICT DO NOTHING
174175
SQL,
175176
],
176177
'no columns to update' => [
@@ -180,7 +181,7 @@ public static function upsert(): array
180181
],
181182
'no columns to update with unique' => [
182183
3 => <<<SQL
183-
INSERT INTO {{%T_upsert}} (`email`) VALUES (:qp0) ON CONFLICT DO NOTHING
184+
INSERT INTO `T_upsert` (`email`) VALUES (:qp0) ON CONFLICT DO NOTHING
184185
SQL,
185186
],
186187
'no unique columns in table - simple insert' => [
@@ -199,33 +200,60 @@ public static function upsert(): array
199200
return $upsert;
200201
}
201202

202-
public static function upsertWithReturningPks(): array
203+
public static function upsertReturning(): array
203204
{
204205
$upsert = self::upsert();
205206

206-
foreach ($upsert as &$data) {
207-
$data[3] .= ' RETURNING `id`';
207+
$withoutUpdate = [
208+
'regular values without update part',
209+
'query without update part',
210+
'values and expressions without update part',
211+
'query, values and expressions without update part',
212+
'no columns to update with unique',
213+
];
214+
215+
foreach ($upsert as $name => &$data) {
216+
array_splice($data, 3, 0, [['id']]);
217+
if (in_array($name, $withoutUpdate, true)) {
218+
$data[4] = substr($data[4], 0, -10) . 'DO UPDATE SET `ts` = `ts`';
219+
}
220+
221+
$data[4] .= ' RETURNING `id`';
208222
}
209223

210-
$upsert['no columns to update'][3] = 'INSERT INTO `T_upsert_1` (`a`) VALUES (:qp0) ON CONFLICT DO NOTHING RETURNING `a`';
224+
$upsert['no columns to update'][3] = ['a'];
225+
$upsert['no columns to update'][4] = 'INSERT INTO `T_upsert_1` (`a`) VALUES (:qp0) ON CONFLICT DO UPDATE SET `a` = `a` RETURNING `a`';
211226

212227
return [
213228
...$upsert,
214229
'composite primary key' => [
215230
'notauto_pk',
216231
['id_1' => 1, 'id_2' => 2.5, 'type' => 'Test'],
217232
true,
233+
['id_1', 'id_2'],
218234
'INSERT INTO `notauto_pk` (`id_1`, `id_2`, `type`) VALUES (:qp0, :qp1, :qp2)'
219235
. ' ON CONFLICT (`id_1`, `id_2`) DO UPDATE SET `type`=EXCLUDED.`type` RETURNING `id_1`, `id_2`',
220236
[':qp0' => 1, ':qp1' => 2.5, ':qp2' => 'Test'],
221237
],
222-
'no primary key' => [
238+
'no return columns' => [
223239
'type',
224240
['int_col' => 3, 'char_col' => 'a', 'float_col' => 1.2, 'bool_col' => true],
225241
true,
242+
[],
226243
'INSERT INTO `type` (`int_col`, `char_col`, `float_col`, `bool_col`) VALUES (:qp0, :qp1, :qp2, :qp3)',
227244
[':qp0' => 3, ':qp1' => 'a', ':qp2' => 1.2, ':qp3' => true],
228245
],
246+
'return all columns' => [
247+
'T_upsert',
248+
['email' => '[email protected]', 'address' => 'test address', 'status' => 1, 'profile_id' => 1],
249+
true,
250+
null,
251+
'INSERT INTO `T_upsert` (`email`, `address`, `status`, `profile_id`) VALUES (:qp0, :qp1, :qp2, :qp3)'
252+
. ' ON CONFLICT (`email`) DO UPDATE SET'
253+
. ' `address`=EXCLUDED.`address`, `status`=EXCLUDED.`status`, `profile_id`=EXCLUDED.`profile_id`'
254+
. ' RETURNING `id`, `ts`, `email`, `recovery_email`, `address`, `status`, `orders`, `profile_id`',
255+
[':qp0' => '[email protected]', ':qp1' => 'test address', ':qp2' => 1, ':qp3' => 1],
256+
],
229257
];
230258
}
231259

‎tests/QueryBuilderTest.php

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

626-
#[DataProviderExternal(QueryBuilderProvider::class, 'upsertWithReturningPks')]
627-
public function testUpsertWithReturningPks(
626+
#[DataProviderExternal(QueryBuilderProvider::class, 'upsertReturning')]
627+
public function testUpsertReturning(
628628
string $table,
629629
array|QueryInterface $insertColumns,
630630
array|bool $updateColumns,
631+
array|null $returnColumns,
631632
string $expectedSql,
632633
array $expectedParams
633634
): void {
634-
parent::testUpsertWithReturningPks($table, $insertColumns, $updateColumns, $expectedSql, $expectedParams);
635+
parent::testUpsertReturning($table, $insertColumns, $updateColumns, $returnColumns, $expectedSql, $expectedParams);
635636
}
636637

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

0 commit comments

Comments
 (0)