diff --git a/CHANGELOG.md b/CHANGELOG.md index 46826ba4a..1f6ccddb5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -137,7 +137,8 @@ - Chg #1037: Change result type of `QueryBuilderInterface::getExpressionBuilder()` and `DQLQueryBuilderInterface::getExpressionBuilder()` methods to `ExpressionBuilderInterface` (@vjik) - New #1029: Add functions as expressions (@Tigrov) -- New #1040: Add `DateTimeValue` class @vjik) +- Enh #1042: Refactor `AbstractDMLQueryBuilder` class to `upsert()` method (@Tigrov) +- New #1040: Add `DateTimeValue` class (@vjik) ## 1.3.0 March 21, 2024 diff --git a/src/QueryBuilder/AbstractDMLQueryBuilder.php b/src/QueryBuilder/AbstractDMLQueryBuilder.php index 65f292fbc..c3bf60e57 100644 --- a/src/QueryBuilder/AbstractDMLQueryBuilder.php +++ b/src/QueryBuilder/AbstractDMLQueryBuilder.php @@ -172,6 +172,20 @@ public function withTypecasting(bool $typecasting = true): static return $new; } + /** + * @psalm-param array $columns + */ + protected function buildSimpleSelect(array $columns): string + { + $quoter = $this->quoter; + + foreach ($columns as $name => &$column) { + $column .= ' AS ' . $quoter->quoteSimpleColumnName($name); + } + + return 'SELECT ' . implode(', ', $columns); + } + /** * Prepare traversable for batch insert. * @@ -293,12 +307,11 @@ protected function extractColumnNames(array|Iterator $rows, array $columns): arr * @throws InvalidConfigException * @throws NotSupportedException * - * @return array Array of column names, values, and params. + * @return string[] Array of column names, values, and params. * * @psalm-param ParamsType $params - * @psalm-return array{0: string[], 1: string, 2: array} */ - protected function prepareInsertSelectSubQuery(QueryInterface $columns, array $params = []): array + protected function getQueryColumnNames(QueryInterface $columns, array &$params = []): array { /** @psalm-var string[] $select */ $select = $columns->getSelect(); @@ -307,8 +320,6 @@ protected function prepareInsertSelectSubQuery(QueryInterface $columns, array $p throw new InvalidArgumentException('Expected select query object with enumerated (named) parameters'); } - [$values, $params] = $this->queryBuilder->build($columns, $params); - $names = []; foreach ($select as $title => $field) { @@ -327,7 +338,7 @@ protected function prepareInsertSelectSubQuery(QueryInterface $columns, array $p } } - return [$names, $values, $params]; + return $names; } /** @@ -356,7 +367,8 @@ protected function prepareInsertValues(string $table, array|QueryInterface $colu } if ($columns instanceof QueryInterface) { - [$names, $values, $params] = $this->prepareInsertSelectSubQuery($columns, $params); + $names = $this->getQueryColumnNames($columns, $params); + [$values, $params] = $this->queryBuilder->build($columns, $params); return [$names, [], $values, $params]; } @@ -382,11 +394,6 @@ protected function prepareInsertValues(string $table, array|QueryInterface $colu /** * Prepare column names and placeholders for `UPDATE` SQL statement. * - * @throws Exception - * @throws InvalidConfigException - * @throws InvalidArgumentException - * @throws NotSupportedException - * * @psalm-param ParamsType $params * * @return string[] @@ -396,24 +403,53 @@ protected function prepareUpdateSets(string $table, array $columns, array &$para $sets = []; $columns = $this->normalizeColumnNames($columns); $tableColumns = $this->typecasting ? $this->schema->getTableSchema($table)?->getColumns() ?? [] : []; + $queryBuilder = $this->queryBuilder; + $quoter = $this->quoter; foreach ($columns as $name => $value) { if (isset($tableColumns[$name])) { $value = $tableColumns[$name]->dbTypecast($value); } - if ($value instanceof ExpressionInterface) { - $placeholder = $this->queryBuilder->buildExpression($value, $params); - } else { - $placeholder = $this->queryBuilder->bindParam($value, $params); - } + $quotedName = $quoter->quoteSimpleColumnName($name); + $builtValue = $queryBuilder->buildValue($value, $params); - $sets[] = $this->quoter->quoteColumnName($name) . '=' . $placeholder; + $sets[] = "$quotedName=$builtValue"; } return $sets; } + /** + * Prepare column names and placeholders for upsert SQL statement. + * + * @psalm-param array|true $updateColumns + * @psalm-param ParamsType $params + * + * @return string[] + */ + protected function prepareUpsertSets( + string $table, + array|bool $updateColumns, + array|null $updateNames, + array &$params + ): array { + if ($updateColumns === true) { + $quoter = $this->quoter; + $sets = []; + + /** @psalm-var string[] $updateNames */ + foreach ($updateNames as $name) { + $quotedName = $quoter->quoteSimpleColumnName($name); + $sets[] = "$quotedName=EXCLUDED.$quotedName"; + } + + return $sets; + } + + return $this->prepareUpdateSets($table, $updateColumns, $params); + } + /** * Prepare column names and constraints for "upsert" operation. * @@ -431,11 +467,12 @@ protected function prepareUpsertColumns( array &$constraints = [] ): array { if ($insertColumns instanceof QueryInterface) { - [$insertNames] = $this->prepareInsertSelectSubQuery($insertColumns); + $insertNames = $this->getQueryColumnNames($insertColumns); } else { - $insertNames = $this->getNormalizeColumnNames(array_keys($insertColumns)); + $insertNames = array_keys($insertColumns); } + $insertNames = $this->getNormalizeColumnNames($insertNames); $uniqueNames = $this->getTableUniqueColumnNames($table, $insertNames, $constraints); if ($updateColumns === true) { diff --git a/tests/AbstractQueryBuilderTest.php b/tests/AbstractQueryBuilderTest.php index 3ba404c04..8a2487dac 100644 --- a/tests/AbstractQueryBuilderTest.php +++ b/tests/AbstractQueryBuilderTest.php @@ -2264,7 +2264,7 @@ public function testUpdate( array|string $condition, array $params, string $expectedSql, - array $expectedParams + array $expectedParams = [], ): void { $db = $this->getConnection(); $qb = $db->getQueryBuilder(); diff --git a/tests/Common/CommonCommandTest.php b/tests/Common/CommonCommandTest.php index 5d05cef73..e665d181b 100644 --- a/tests/Common/CommonCommandTest.php +++ b/tests/Common/CommonCommandTest.php @@ -1728,12 +1728,13 @@ public function testUpdateWithoutTypecasting(): void $command->update('{{type}}', $values); - $this->assertSame([ - ':qp0' => 1, - ':qp1' => 'test', - ':qp2' => 3.14, - ':qp3' => $db->getDriverName() === 'oci' ? '1' : true, - ], $command->getParams()); + $expectedParams = [':qp0' => 'test']; + + if ($db->getDriverName() === 'oci') { + $expectedParams[':qp1'] = '1'; + } + + $this->assertSame($expectedParams, $command->getParams()); $command = $command->withDbTypecasting(false); $command->update('{{type}}', $values); diff --git a/tests/Common/CommonQueryBuilderTest.php b/tests/Common/CommonQueryBuilderTest.php index af26243ea..65c159bd5 100644 --- a/tests/Common/CommonQueryBuilderTest.php +++ b/tests/Common/CommonQueryBuilderTest.php @@ -175,21 +175,22 @@ public function testUpdateWithoutTypecasting(): void $params = []; $qb->update('{{type}}', $values, [], $params); - $this->assertSame([ - ':qp0' => 1, - ':qp1' => 'test', - ':qp2' => 3.14, - ':qp3' => $db->getDriverName() === 'oci' ? '1' : true, - ], $params); + $expectedParams = [':qp0' => new Param('test', DataType::STRING)]; + + if ($db->getDriverName() === 'oci') { + $expectedParams[':qp1'] = new Param('1', DataType::STRING); + } + + Assert::arraysEquals($expectedParams, $params); $params = []; $qb->withTypecasting(false)->update('{{type}}', $values, [], $params); - $this->assertSame([ - ':qp0' => '1', - ':qp1' => 'test', - ':qp2' => '3.14', - ':qp3' => '1', + Assert::arraysEquals([ + ':qp0' => new Param('1', DataType::STRING), + ':qp1' => new Param('test', DataType::STRING), + ':qp2' => new Param('3.14', DataType::STRING), + ':qp3' => new Param('1', DataType::STRING), ], $params); } diff --git a/tests/Db/QueryBuilder/QueryBuilderTest.php b/tests/Db/QueryBuilder/QueryBuilderTest.php index db788bc94..2826f5c52 100644 --- a/tests/Db/QueryBuilder/QueryBuilderTest.php +++ b/tests/Db/QueryBuilder/QueryBuilderTest.php @@ -221,7 +221,7 @@ public function testUpdate( array|string $condition, array $params, string $expectedSql, - array $expectedParams + array $expectedParams = [], ): void { $db = $this->getConnection(); $qb = $db->getQueryBuilder(); diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index 36973875e..6757ca7ad 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -1215,7 +1215,7 @@ public static function update(): array SQL ), [ - ':qp0' => '{{test}}', + ':qp0' => new Param('{{test}}', DataType::STRING), ], ], [ @@ -1229,7 +1229,7 @@ public static function update(): array SQL ), [ - ':qp0' => '{{test}}', + ':qp0' => new Param('{{test}}', DataType::STRING), ], ], [ @@ -1244,7 +1244,7 @@ public static function update(): array ), [ 'id' => 'boolean', - ':qp1' => '{{test}}', + ':qp1' => new Param('{{test}}', DataType::STRING), ], ], [ @@ -1254,10 +1254,9 @@ public static function update(): array [], static::replaceQuotes( << 1], ], 'Expressions without params' => [ '{{product}}', @@ -1269,7 +1268,6 @@ public static function update(): array UPDATE [[product]] SET [[name]]=UPPER([[name]]) WHERE [[name]] = LOWER([[name]]) SQL ), - [], ], 'Expression with params and without params' => [ '{{product}}', @@ -1407,11 +1405,10 @@ public static function update(): array [':val' => new Expression("label=':val' AND name=:val", [':val' => 'Apple'])], static::replaceQuotes( << 10, ':val_0' => 'Apple', ], ], @@ -1422,11 +1419,10 @@ public static function update(): array [':val' => new Expression("label=':val'", [':val' => 'Apple'])], static::replaceQuotes( << 10, ':val_0' => 'Apple', ], ], @@ -1460,8 +1456,7 @@ public static function upsert(): array ':qp1' => 'bar {{city}}', ':qp2' => 1, ':qp3' => null, - ':qp4' => 'foo {{city}}', - ':qp5' => 2, + ':qp4' => new Param('foo {{city}}', DataType::STRING), ], ], 'regular values without update part' => [ @@ -1491,7 +1486,10 @@ public static function upsert(): array ->limit(1), ['address' => 'foo {{city}}', 'status' => 2, 'orders' => new Expression('T_upsert.orders + 1')], '', - [':qp0' => new Param('user1', DataType::STRING), ':qp1' => 'foo {{city}}', ':qp2' => 2], + [ + ':qp0' => new Param('user1', DataType::STRING), + ':qp1' => new Param('foo {{city}}', DataType::STRING), + ], ], 'query without update part' => [ 'T_upsert', @@ -1536,7 +1534,7 @@ public static function upsert(): array ), ['ts' => 0, '[[orders]]' => new Expression('T_upsert.orders + 1')], '', - [':phEmail' => 'dynamic@example.com', ':qp1' => 0], + [':phEmail' => 'dynamic@example.com'], ], 'query, values and expressions without update part' => [ 'T_upsert',