From 959414624163e855216fa2f791e5a027ed9e496c Mon Sep 17 00:00:00 2001 From: Tigrov Date: Fri, 18 Aug 2023 17:37:31 +0700 Subject: [PATCH 1/3] Refactor DMLQueryBuilder --- src/DMLQueryBuilder.php | 84 +++++++------------------ tests/Provider/CommandProvider.php | 2 +- tests/Provider/QueryBuilderProvider.php | 8 +++ 3 files changed, 32 insertions(+), 62 deletions(-) diff --git a/src/DMLQueryBuilder.php b/src/DMLQueryBuilder.php index 76af105e7..0c2d2eefd 100644 --- a/src/DMLQueryBuilder.php +++ b/src/DMLQueryBuilder.php @@ -11,16 +11,11 @@ use Yiisoft\Db\Exception\InvalidConfigException; 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 Yiisoft\Db\Schema\SchemaInterface; use function implode; use function in_array; -use function ltrim; -use function strrpos; -use function is_array; /** * Implements a DML (Data Manipulation Language) SQL statements for MSSQL Server. @@ -35,16 +30,12 @@ final class DMLQueryBuilder extends AbstractDMLQueryBuilder */ public function insertWithReturningPks(string $table, QueryInterface|array $columns, array &$params = []): string { - /** - * @psalm-var string[] $names - * @psalm-var string[] $placeholders - */ [$names, $placeholders, $values, $params] = $this->prepareInsertValues($table, $columns, $params); + $createdCols = []; + $insertedCols = []; + $returnColumns = $this->schema->getTableSchema($table)?->getColumns() ?? []; - $createdCols = $insertedCols = []; - $tableSchema = $this->schema->getTableSchema($table); - $returnColumns = $tableSchema?->getColumns() ?? []; foreach ($returnColumns as $returnColumn) { if ($returnColumn->isComputed()) { continue; @@ -54,9 +45,7 @@ public function insertWithReturningPks(string $table, QueryInterface|array $colu if (in_array($dbType, ['char', 'varchar', 'nchar', 'nvarchar', 'binary', 'varbinary'], true)) { $dbType .= '(MAX)'; - } - - if ($returnColumn->getDbType() === SchemaInterface::TYPE_TIMESTAMP) { + } elseif ($dbType === 'timestamp') { $dbType = $returnColumn->isAllowNull() ? 'varbinary(8)' : 'binary(8)'; } @@ -65,12 +54,10 @@ public function insertWithReturningPks(string $table, QueryInterface|array $colu $insertedCols[] = 'INSERTED.' . $quotedName; } - $sql = 'INSERT INTO ' - . $this->quoter->quoteTableName($table) + $sql = 'INSERT INTO ' . $this->quoter->quoteTableName($table) . (!empty($names) ? ' (' . implode(', ', $names) . ')' : '') . ' OUTPUT ' . implode(',', $insertedCols) . ' INTO @temporary_inserted' - . (!empty($placeholders) ? ' VALUES (' . implode(', ', $placeholders) . ')' : (string) $values); - + . (!empty($placeholders) ? ' VALUES (' . implode(', ', $placeholders) . ')' : ' ' . $values); return 'SET NOCOUNT ON;DECLARE @temporary_inserted TABLE (' . implode(', ', $createdCols) . ');' . $sql . ';SELECT * FROM @temporary_inserted;'; @@ -118,7 +105,6 @@ public function upsert( /** @psalm-var Constraint[] $constraints */ $constraints = []; - /** @psalm-var string[] $insertNames */ [$uniqueNames, $insertNames, $updateNames] = $this->prepareUpsertColumns( $table, $insertColumns, @@ -135,15 +121,12 @@ public function upsert( foreach ($constraints as $constraint) { $constraintCondition = ['and']; + $columnNames = (array) $constraint->getColumnNames(); - $columnNames = $constraint->getColumnNames() ?? []; - - if (is_array($columnNames)) { - /** @psalm-var string[] $columnNames */ - foreach ($columnNames as $name) { - $quotedName = $this->quoter->quoteColumnName($name); - $constraintCondition[] = "$quotedTableName.$quotedName=[EXCLUDED].$quotedName"; - } + /** @psalm-var string[] $columnNames */ + foreach ($columnNames as $name) { + $quotedName = $this->quoter->quoteColumnName($name); + $constraintCondition[] = "$quotedTableName.$quotedName=[EXCLUDED].$quotedName"; } $onCondition[] = $constraintCondition; @@ -151,32 +134,22 @@ public function upsert( $on = $this->queryBuilder->buildCondition($onCondition, $params); - /** @psalm-var string[] $placeholders */ [, $placeholders, $values, $params] = $this->prepareInsertValues($table, $insertColumns, $params); - $mergeSql = 'MERGE ' . $this->quoter->quoteTableName($table) . ' WITH (HOLDLOCK) ' - . 'USING (' . (!empty($placeholders) - ? 'VALUES (' . implode(', ', $placeholders) . ')' - : ltrim((string) $values, ' ')) . ') AS [EXCLUDED] (' . implode(', ', $insertNames) . ') ' . "ON ($on)"; - $insertValues = []; - foreach ($insertNames as $name) { - $quotedName = $this->quoter->quoteColumnName($name); + $mergeSql = 'MERGE ' . $quotedTableName . ' WITH (HOLDLOCK) USING (' + . (!empty($placeholders) ? 'VALUES (' . implode(', ', $placeholders) . ')' : $values) + . ') AS [EXCLUDED] (' . implode(', ', $insertNames) . ') ' . "ON ($on)"; - if (strrpos($quotedName, '.') === false) { - $quotedName = '[EXCLUDED].' . $quotedName; - } + $insertValues = []; - $insertValues[] = $quotedName; + foreach ($insertNames as $quotedName) { + $insertValues[] = '[EXCLUDED].' . $quotedName; } - $insertSql = 'INSERT (' . implode(', ', $insertNames) . ')' . ' VALUES (' . implode(', ', $insertValues) . ')'; + $insertSql = 'INSERT (' . implode(', ', $insertNames) . ') VALUES (' . implode(', ', $insertValues) . ')'; - if ($updateNames === []) { + if ($updateColumns === false || $updateNames === []) { /** there are no columns to update */ - $updateColumns = false; - } - - if ($updateColumns === false) { return "$mergeSql WHEN NOT MATCHED THEN $insertSql;"; } @@ -184,25 +157,14 @@ public function upsert( $updateColumns = []; /** @psalm-var string[] $updateNames */ - foreach ($updateNames as $name) { - $quotedName = $this->quoter->quoteColumnName($name); - if (strrpos($quotedName, '.') === false) { - $quotedName = '[EXCLUDED].' . $quotedName; - } - - $updateColumns[$name] = new Expression($quotedName); + foreach ($updateNames as $quotedName) { + $updateColumns[$quotedName] = new Expression('[EXCLUDED].' . $quotedName); } } - /** - * @var array $params - * - * @psalm-var string[] $updates - * @psalm-var array $updateColumns - */ [$updates, $params] = $this->prepareUpdateSets($table, $updateColumns, $params); - $updateSql = 'UPDATE SET ' . implode(', ', $updates); - return "$mergeSql WHEN MATCHED THEN $updateSql WHEN NOT MATCHED THEN $insertSql;"; + return "$mergeSql WHEN MATCHED THEN UPDATE SET " . implode(', ', $updates) + . " WHEN NOT MATCHED THEN $insertSql;"; } } diff --git a/tests/Provider/CommandProvider.php b/tests/Provider/CommandProvider.php index 70326b2bc..9f8336758 100644 --- a/tests/Provider/CommandProvider.php +++ b/tests/Provider/CommandProvider.php @@ -27,7 +27,7 @@ public static function batchInsert(): array $batchInsert['issue11242']['expectedParams'][':qp3'] = 1; - $batchInsert['wrongBehavior']['expectedParams'][':qp3'] = 0; + $batchInsert['table name with column name with brackets']['expectedParams'][':qp3'] = 0; $batchInsert['batchInsert binds params from expression']['expectedParams'][':qp3'] = 0; diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index aea94267a..d8883eaee 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -218,6 +218,14 @@ public static function upsert(): array '[EXCLUDED].[address], [EXCLUDED].[status], [EXCLUDED].[profile_id]);', ], + 'regular values with unique at not the first position' => [ + 3 => 'MERGE [T_upsert] WITH (HOLDLOCK) USING (VALUES (:qp0, :qp1, :qp2, :qp3)) AS [EXCLUDED] ' . + '([address], [email], [status], [profile_id]) ON ([T_upsert].[email]=[EXCLUDED].[email]) ' . + 'WHEN MATCHED THEN UPDATE SET [address]=[EXCLUDED].[address], [status]=[EXCLUDED].[status], [profile_id]=[EXCLUDED].[profile_id] ' . + 'WHEN NOT MATCHED THEN INSERT ([address], [email], [status], [profile_id]) VALUES (' . + '[EXCLUDED].[address], [EXCLUDED].[email], [EXCLUDED].[status], [EXCLUDED].[profile_id]);', + ], + 'regular values with update part' => [ 3 => 'MERGE [T_upsert] WITH (HOLDLOCK) USING (VALUES (:qp0, :qp1, :qp2, :qp3)) AS [EXCLUDED] ' . '([email], [address], [status], [profile_id]) ON ([T_upsert].[email]=[EXCLUDED].[email]) ' . From 103ae251e9bf3bff333936565477dd586ff0a7d5 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Mon, 21 Aug 2023 21:51:29 +0700 Subject: [PATCH 2/3] Improve test --- tests/Provider/CommandProvider.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Provider/CommandProvider.php b/tests/Provider/CommandProvider.php index 9f8336758..e6a2e658f 100644 --- a/tests/Provider/CommandProvider.php +++ b/tests/Provider/CommandProvider.php @@ -30,6 +30,7 @@ public static function batchInsert(): array $batchInsert['table name with column name with brackets']['expectedParams'][':qp3'] = 0; $batchInsert['batchInsert binds params from expression']['expectedParams'][':qp3'] = 0; + $batchInsert['with associative values']['expectedParams'][':qp3'] = 1; return $batchInsert; } From 7274c8a7af12cecbc2a271a83bdeecce69a8de94 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 26 Aug 2023 20:40:23 +0700 Subject: [PATCH 3/3] Add line to CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 283f3a3ce..1f257d531 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## 1.0.2 under development -- no changes in this release. +- Bug #275: Refactor `DMLQueryBuilder`, related with yiisoft/db#746 (@Tigrov) ## 1.0.1 July 24, 2023