From 789523f1d621a1a8537b7546158166fd93869595 Mon Sep 17 00:00:00 2001 From: Sergei Tigrov Date: Fri, 12 Apr 2024 18:44:28 +0700 Subject: [PATCH 1/2] Process expressions inside param values when build a query (#806) Co-authored-by: Sergei Predvoditelev Co-authored-by: Alexander Makarov --- CHANGELOG.md | 2 + UPGRADE.md | 11 + src/Expression/AbstractExpressionBuilder.php | 242 +++++++++++++++++++ src/Expression/ExpressionBuilder.php | 25 -- src/QueryBuilder/AbstractDQLQueryBuilder.php | 49 +--- src/Syntax/AbstractSqlParser.php | 153 ++++++++++++ tests/AbstractQueryBuilderTest.php | 16 +- tests/AbstractSqlParserTest.php | 42 ++++ tests/Common/CommonCommandTest.php | 19 +- tests/Db/QueryBuilder/QueryBuilderTest.php | 11 +- tests/Db/Syntax/SqlParserTest.php | 16 ++ tests/Provider/CommandProvider.php | 93 ++----- tests/Provider/QueryBuilderProvider.php | 232 ++++++++++++++++++ tests/Provider/SqlParserProvider.php | 134 ++++++++++ tests/Support/Stub/DQLQueryBuilder.php | 7 + tests/Support/Stub/ExpressionBuilder.php | 15 ++ tests/Support/Stub/SqlParser.php | 42 ++++ 17 files changed, 966 insertions(+), 143 deletions(-) create mode 100644 src/Expression/AbstractExpressionBuilder.php delete mode 100644 src/Expression/ExpressionBuilder.php create mode 100644 src/Syntax/AbstractSqlParser.php create mode 100644 tests/AbstractSqlParserTest.php create mode 100644 tests/Db/Syntax/SqlParserTest.php create mode 100644 tests/Provider/SqlParserProvider.php create mode 100644 tests/Support/Stub/ExpressionBuilder.php create mode 100644 tests/Support/Stub/SqlParser.php diff --git a/CHANGELOG.md b/CHANGELOG.md index e6765a0ef..d34d06330 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## 2.0.0 under development +- Enh #806: Non-unique placeholder names inside `Expression::$params` will be replaced with unique names (@Tigrov) +- Enh #806: Build `Expression` instances inside `Expression::$params` when build a query using `QueryBuilder` (@Tigrov) - Enh #766: Allow `ColumnInterface` as column type. (@Tigrov) ## 1.3.0 March 21, 2024 diff --git a/UPGRADE.md b/UPGRADE.md index c7c4accb1..dde268f37 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -5,6 +5,8 @@ there is version B between A and C, you need to following the instructions for b ## Upgrade from 1.x to 2.x +### `ColumnInterface` as column type + Add `ColumnInterface` support and change type of parameter `$type` from `string` to `ColumnInterface|string` in `addColumn()` method of your classes that implement the following interfaces: @@ -16,3 +18,12 @@ in `addColumn()` method of your classes that implement the following interfaces: - `Yiisoft\Db\Command\AbstractCommand`; - `Yiisoft\Db\QueryBuilder\AbstractDDLQueryBuilder`; - `Yiisoft\Db\QueryBuilder\AbstractQueryBuilder`. + +### Build `Expression` instances inside `Expression::$params` + +`ExpressionBuilder` is replaced by an abstract class `AbstractExpressionBuilder` with an instance of the +`QueryBuilderInterface` parameter in the constructor. Each DBMS driver should implement its own expression builder. + +`Expression::$params` can contain: +- non-unique placeholder names, they will be replaced with unique names. +- `Expression` instances, they will be built when building a query using `QueryBuilder`. diff --git a/src/Expression/AbstractExpressionBuilder.php b/src/Expression/AbstractExpressionBuilder.php new file mode 100644 index 000000000..b75775492 --- /dev/null +++ b/src/Expression/AbstractExpressionBuilder.php @@ -0,0 +1,242 @@ +__toString(); + $expressionParams = $expression->getParams(); + + if (empty($expressionParams)) { + return $sql; + } + + if (isset($expressionParams[0])) { + $params = array_merge($params, $expressionParams); + return $sql; + } + + $nonUniqueReplacements = $this->appendParams($expressionParams, $params); + $expressionReplacements = $this->buildParamExpressions($expressionParams, $params); + + $replacements = $this->mergeReplacements($nonUniqueReplacements, $expressionReplacements); + + if (empty($replacements)) { + return $sql; + } + + return $this->replacePlaceholders($sql, $replacements); + } + + /** + * Appends parameters to the list of query parameters replacing non-unique parameters with unique ones. + * + * @param array $expressionParams Parameters to be appended. + * @param array $params Parameters to be bound to the query. + * + * @psalm-param ParamsType $expressionParams + * @psalm-param ParamsType $params + * + * @return string[] Replacements for non-unique parameters. + */ + private function appendParams(array &$expressionParams, array &$params): array + { + $nonUniqueParams = []; + + /** @var non-empty-string $name */ + foreach ($expressionParams as $name => $value) { + $paramName = $name[0] === ':' ? substr($name, 1) : $name; + + if (!isset($params[$paramName]) && !isset($params[":$paramName"])) { + $params[$name] = $value; + continue; + } + + $nonUniqueParams[$name] = $value; + } + + $replacements = []; + + /** @var non-empty-string $name */ + foreach ($nonUniqueParams as $name => $value) { + $paramName = $name[0] === ':' ? substr($name, 1) : $name; + $uniqueName = $this->getUniqueName($paramName, $params); + + $replacements[":$paramName"] = ":$uniqueName"; + + if ($name[0] === ':') { + $uniqueName = ":$uniqueName"; + } + + $params[$uniqueName] = $value; + $expressionParams[$uniqueName] = $value; + unset($expressionParams[$name]); + } + + return $replacements; + } + + /** + * Build expression values of parameters. + * + * @param array $expressionParams Parameters from the expression. + * @param array $params Parameters to be bound to the query. + * + * @psalm-param ParamsType $expressionParams + * @psalm-param ParamsType $params + * + * @return string[] Replacements for parameters. + */ + private function buildParamExpressions(array $expressionParams, array &$params): array + { + $replacements = []; + + /** @var non-empty-string $name */ + foreach ($expressionParams as $name => $value) { + if (!$value instanceof ExpressionInterface || $value instanceof Param) { + continue; + } + + $placeholder = $name[0] !== ':' ? ":$name" : $name; + $replacements[$placeholder] = $this->queryBuilder->buildExpression($value, $params); + + /** @psalm-var ParamsType $params */ + unset($params[$name]); + } + + return $replacements; + } + + /** + * Merges replacements for non-unique parameters with replacements for expression parameters. + * + * @param string[] $replacements Replacements for non-unique parameters. + * @param string[] $expressionReplacements Replacements for expression parameters. + * + * @return string[] Merged replacements. + */ + private function mergeReplacements(array $replacements, array $expressionReplacements): array + { + if (empty($replacements)) { + return $expressionReplacements; + } + + if (empty($expressionReplacements)) { + return $replacements; + } + + /** @var non-empty-string $value */ + foreach ($replacements as $name => $value) { + if (isset($expressionReplacements[$value])) { + $replacements[$name] = $expressionReplacements[$value]; + unset($expressionReplacements[$value]); + } + } + + return $replacements + $expressionReplacements; + } + + /** + * Returns a unique name for the parameter without colon at the beginning. + * + * @param string $name Name of the parameter without colon at the beginning. + * @param array $params Parameters to be bound to the query. + * + * @psalm-param ParamsType $params + * + * @return string Unique name of the parameter with colon at the beginning. + * + * @psalm-return non-empty-string + */ + private function getUniqueName(string $name, array $params): string + { + $uniqueName = $name . '_0'; + + for ($i = 1; isset($params[$uniqueName]) || isset($params[":$uniqueName"]); ++$i) { + $uniqueName = $name . '_' . $i; + } + + return $uniqueName; + } + + /** + * Replaces placeholders with replacements in a SQL expression. + * + * @param string $sql SQL expression where the placeholder should be replaced. + * @param string[] $replacements Replacements for placeholders. + * + * @return string SQL expression with replaced placeholders. + */ + private function replacePlaceholders(string $sql, array $replacements): string + { + $parser = $this->createSqlParser($sql); + $offset = 0; + + while (null !== $placeholder = $parser->getNextPlaceholder($position)) { + if (isset($replacements[$placeholder])) { + /** @var int $position */ + $sql = substr_replace($sql, $replacements[$placeholder], $position + $offset, strlen($placeholder)); + + if (count($replacements) === 1) { + break; + } + + $offset += strlen($replacements[$placeholder]) - strlen($placeholder); + unset($replacements[$placeholder]); + } + } + + return $sql; + } + + /** + * Creates an instance of {@see AbstractSqlParser} for the given SQL expression. + * + * @param string $sql SQL expression to be parsed. + * + * @return AbstractSqlParser SQL parser instance. + */ + abstract protected function createSqlParser(string $sql): AbstractSqlParser; +} diff --git a/src/Expression/ExpressionBuilder.php b/src/Expression/ExpressionBuilder.php deleted file mode 100644 index fac480612..000000000 --- a/src/Expression/ExpressionBuilder.php +++ /dev/null @@ -1,25 +0,0 @@ -getParams()); - return $expression->__toString(); - } -} diff --git a/src/QueryBuilder/AbstractDQLQueryBuilder.php b/src/QueryBuilder/AbstractDQLQueryBuilder.php index 9f857328f..fd3951540 100644 --- a/src/QueryBuilder/AbstractDQLQueryBuilder.php +++ b/src/QueryBuilder/AbstractDQLQueryBuilder.php @@ -11,7 +11,6 @@ use Yiisoft\Db\Exception\InvalidConfigException; use Yiisoft\Db\Exception\NotSupportedException; use Yiisoft\Db\Expression\Expression; -use Yiisoft\Db\Expression\ExpressionBuilder; use Yiisoft\Db\Expression\ExpressionBuilderInterface; use Yiisoft\Db\Expression\ExpressionInterface; use Yiisoft\Db\QueryBuilder\Condition\HashCondition; @@ -105,25 +104,7 @@ public function build(QueryInterface $query, array $params = []): array $this->buildHaving($query->getHaving(), $params), ]; $sql = implode($this->separator, array_filter($clauses)); - $sql = $this->buildOrderByAndLimit($sql, $query->getOrderBy(), $query->getLimit(), $query->getOffset()); - - if (!empty($query->getOrderBy())) { - /** @psalm-var array */ - foreach ($query->getOrderBy() as $expression) { - if ($expression instanceof ExpressionInterface) { - $this->buildExpression($expression, $params); - } - } - } - - if (!empty($query->getGroupBy())) { - /** @psalm-var array */ - foreach ($query->getGroupBy() as $expression) { - if ($expression instanceof ExpressionInterface) { - $this->buildExpression($expression, $params); - } - } - } + $sql = $this->buildOrderByAndLimit($sql, $query->getOrderBy(), $query->getLimit(), $query->getOffset(), $params); $union = $this->buildUnion($query->getUnions(), $params); @@ -165,19 +146,22 @@ public function buildColumns(array|string $columns): string public function buildCondition(array|string|ExpressionInterface|null $condition, array &$params = []): string { - if (is_array($condition)) { - if (empty($condition)) { - return ''; + if (empty($condition)) { + if ($condition === '0') { + return '0'; } - $condition = $this->createConditionFromArray($condition); + return ''; } - if ($condition instanceof ExpressionInterface) { - return $this->buildExpression($condition, $params); + if (is_array($condition)) { + $condition = $this->createConditionFromArray($condition); + } elseif (is_string($condition)) { + $condition = new Expression($condition, $params); + $params = []; } - return $condition ?? ''; + return $this->buildExpression($condition, $params); } public function buildExpression(ExpressionInterface $expression, array &$params = []): string @@ -208,10 +192,7 @@ public function buildGroupBy(array $columns, array &$params = []): string /** @psalm-var array $columns */ foreach ($columns as $i => $column) { if ($column instanceof ExpressionInterface) { - $columns[$i] = $this->buildExpression($column); - if ($column instanceof Expression || $column instanceof QueryInterface) { - $params = array_merge($params, $column->getParams()); - } + $columns[$i] = $this->buildExpression($column, $params); } elseif (!str_contains($column, '(')) { $columns[$i] = $this->quoter->quoteColumnName($column); } @@ -299,10 +280,7 @@ public function buildOrderBy(array $columns, array &$params = []): string /** @psalm-var array $columns */ foreach ($columns as $name => $direction) { if ($direction instanceof ExpressionInterface) { - $orders[] = $this->buildExpression($direction); - if ($direction instanceof Expression || $direction instanceof QueryInterface) { - $params = array_merge($params, $direction->getParams()); - } + $orders[] = $this->buildExpression($direction, $params); } else { $orders[] = $this->quoter->quoteColumnName($name) . ($direction === SORT_DESC ? ' DESC' : ''); } @@ -524,7 +502,6 @@ protected function defaultExpressionBuilders(): array return [ Query::class => QueryExpressionBuilder::class, Param::class => ParamBuilder::class, - Expression::class => ExpressionBuilder::class, Condition\AbstractConjunctionCondition::class => Condition\Builder\ConjunctionConditionBuilder::class, Condition\NotCondition::class => Condition\Builder\NotConditionBuilder::class, Condition\AndCondition::class => Condition\Builder\ConjunctionConditionBuilder::class, diff --git a/src/Syntax/AbstractSqlParser.php b/src/Syntax/AbstractSqlParser.php new file mode 100644 index 000000000..ae6d6b250 --- /dev/null +++ b/src/Syntax/AbstractSqlParser.php @@ -0,0 +1,153 @@ +length = strlen($sql); + } + + /** + * Returns the next placeholder from the current position in SQL statement. + * + * @param int|null $position Position of the placeholder in SQL statement. + * + * @return string|null The next placeholder or null if it is not found. + */ + abstract public function getNextPlaceholder(int|null &$position = null): string|null; + + /** + * Parses and returns word symbols. Equals to `\w+` in regular expressions. + * + * @return string Parsed word symbols. + */ + final protected function parseWord(): string + { + $word = ''; + $continue = true; + + while ($continue && $this->position < $this->length) { + match ($this->sql[$this->position]) { + '_', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', + 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', + 'v', 'w', 'x', 'y', 'z', + 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', + 'V', 'W', 'X', 'Y', 'Z' => $word .= $this->sql[$this->position++], + default => $continue = false, + }; + } + + return $word; + } + + /** + * Parses and returns identifier. Equals to `[_a-zA-Z]\w+` in regular expressions. + * + * @return string Parsed identifier. + */ + protected function parseIdentifier(): string + { + return match ($this->sql[$this->position]) { + '_', + 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', + 'v', 'w', 'x', 'y', 'z', + 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', + 'V', 'W', 'X', 'Y', 'Z' => $this->sql[$this->position++] . $this->parseWord(), + default => '', + }; + } + + /** + * Skips quoted string without escape characters. + */ + final protected function skipQuotedWithoutEscape(string $endChar): void + { + do { + $this->skipToAfterChar($endChar); + } while (($this->sql[$this->position] ?? null) === $endChar && ++$this->position); + } + + /** + * Skips quoted string with escape characters. + */ + final protected function skipQuotedWithEscape(string $endChar): void + { + for (; $this->position < $this->length; ++$this->position) { + if ($this->sql[$this->position] === $endChar) { + ++$this->position; + return; + } + + if ($this->sql[$this->position] === '\\') { + ++$this->position; + } + } + } + + /** + * Skips all specified characters. + */ + final protected function skipChars(string $char): void + { + while ($this->position < $this->length && $this->sql[$this->position] === $char) { + ++$this->position; + } + } + + /** + * Skips to the character after the specified character. + */ + final protected function skipToAfterChar(string $char): void + { + for (; $this->position < $this->length; ++$this->position) { + if ($this->sql[$this->position] === $char) { + ++$this->position; + return; + } + } + } + + /** + * Skips to the character after the specified string. + */ + final protected function skipToAfterString(string $string): void + { + $firstChar = $string[0]; + $subString = substr($string, 1); + $length = strlen($subString); + + do { + $this->skipToAfterChar($firstChar); + + if (substr($this->sql, $this->position, $length) === $subString) { + $this->position += $length; + return; + } + } while ($this->position + $length < $this->length); + } +} diff --git a/tests/AbstractQueryBuilderTest.php b/tests/AbstractQueryBuilderTest.php index abcd543da..4ab4f287f 100644 --- a/tests/AbstractQueryBuilderTest.php +++ b/tests/AbstractQueryBuilderTest.php @@ -2196,16 +2196,18 @@ public function testUpdate( string $table, array $columns, array|string $condition, - string $expectedSQL, + array $params, + string $expectedSql, array $expectedParams ): void { $db = $this->getConnection(); - $qb = $db->getQueryBuilder(); - $actualParams = []; - $this->assertSame($expectedSQL, $qb->update($table, $columns, $condition, $actualParams)); - $this->assertSame($expectedParams, $actualParams); + $sql = $qb->update($table, $columns, $condition, $params); + $sql = $qb->quoter()->quoteSql($sql); + + $this->assertSame($expectedSql, $sql); + $this->assertEquals($expectedParams, $params); } /** @@ -2276,7 +2278,7 @@ public function testOverrideParameters1(): void { $db = $this->getConnection(); - $params = [':id' => 1, ':pv2' => new Expression('(select type from {{%animal}}) where id=1')]; + $params = [':id' => 1, ':pv2' => 'test']; $expression = new Expression('id = :id AND type = :pv2', $params); $query = new Query($db); @@ -2291,7 +2293,7 @@ public function testOverrideParameters1(): void $this->assertEquals([':id', ':pv2', ':pv2_0',], array_keys($command->getParams())); $this->assertEquals( DbHelper::replaceQuotes( - 'SELECT * FROM [[animal]] WHERE (id = 1 AND type = (select type from {{%animal}}) where id=1) AND ([[type]]=\'test1\')', + 'SELECT * FROM [[animal]] WHERE (id = 1 AND type = \'test\') AND ([[type]]=\'test1\')', $db->getDriverName() ), $command->getRawSql() diff --git a/tests/AbstractSqlParserTest.php b/tests/AbstractSqlParserTest.php new file mode 100644 index 000000000..af466d309 --- /dev/null +++ b/tests/AbstractSqlParserTest.php @@ -0,0 +1,42 @@ +createSqlParser($sql); + + $this->assertSame($expectedPlaceholder, $parser->getNextPlaceholder($position)); + $this->assertSame($expectedPosition, $position); + } + + /** @dataProvider \Yiisoft\Db\Tests\Provider\SqlParserProvider::getAllPlaceholders */ + public function testGetAllPlaceholders(string $sql, array $expectedPlaceholders, array $expectedPositions): void + { + $parser = $this->createSqlParser($sql); + + $placeholders = []; + $positions = []; + + while (null !== $placeholder = $parser->getNextPlaceholder($position)) { + $placeholders[] = $placeholder; + $positions[] = $position; + } + + $this->assertSame($expectedPlaceholders, $placeholders); + $this->assertSame($expectedPositions, $positions); + } +} diff --git a/tests/Common/CommonCommandTest.php b/tests/Common/CommonCommandTest.php index 305e74a64..924a94800 100644 --- a/tests/Common/CommonCommandTest.php +++ b/tests/Common/CommonCommandTest.php @@ -1896,14 +1896,25 @@ public function testUpdate( array $columns, array|string $conditions, array $params, - string $expected + array $expectedValues, + int $expectedCount, ): void { - $db = $this->getConnection(); + $db = $this->getConnection(true); $command = $db->createCommand(); - $sql = $command->update($table, $columns, $conditions, $params)->getSql(); + $count = $command->update($table, $columns, $conditions, $params)->execute(); + + $this->assertSame($expectedCount, $count); - $this->assertSame($expected, $sql); + $values = (new Query($db)) + ->from($table) + ->where($conditions, $params) + ->limit(1) + ->one(); + + foreach ($expectedValues as $name => $expectedValue) { + $this->assertEquals($expectedValue, $values[$name]); + } $db->close(); } diff --git a/tests/Db/QueryBuilder/QueryBuilderTest.php b/tests/Db/QueryBuilder/QueryBuilderTest.php index 7743142ab..bf646ad00 100644 --- a/tests/Db/QueryBuilder/QueryBuilderTest.php +++ b/tests/Db/QueryBuilder/QueryBuilderTest.php @@ -254,17 +254,20 @@ public function testUpdate( string $table, array $columns, array|string $condition, - string $expectedSQL, + array $params, + string $expectedSql, array $expectedParams ): void { $db = $this->getConnection(); $schemaMock = $this->createMock(Schema::class); $qb = new QueryBuilder($db->getQuoter(), $schemaMock); - $actualParams = []; - $this->assertSame($expectedSQL, $qb->update($table, $columns, $condition, $actualParams)); - $this->assertSame($expectedParams, $actualParams); + $sql = $qb->update($table, $columns, $condition, $params); + $sql = $qb->quoter()->quoteSql($sql); + + $this->assertSame($expectedSql, $sql); + $this->assertEquals($expectedParams, $params); } /** diff --git a/tests/Db/Syntax/SqlParserTest.php b/tests/Db/Syntax/SqlParserTest.php new file mode 100644 index 000000000..c44abdecf --- /dev/null +++ b/tests/Db/Syntax/SqlParserTest.php @@ -0,0 +1,16 @@ + '{{test}}'], [], [], - DbHelper::replaceQuotes( - << '{{test}}'], + 3, ], [ - '{{table}}', + '{{customer}}', ['name' => '{{test}}'], ['id' => 1], [], - DbHelper::replaceQuotes( - << '{{test}}'], - ['id' => 1], - ['id' => 'integer'], - DbHelper::replaceQuotes( - << '{{test}}'], + '{{customer}}', + ['{{customer}}.name' => '{{test}}'], ['id' => 1], - ['id' => 'string'], - DbHelper::replaceQuotes( - << '{{test}}'], - ['id' => 1], - ['id' => 'boolean'], - DbHelper::replaceQuotes( - << '{{test}}'], - ['id' => 1], - ['id' => 'boolean'], - DbHelper::replaceQuotes( - << '{{test}}'], - ['id' => 1], - ['id' => 'float'], - DbHelper::replaceQuotes( - << new Expression('1 + 2')], + ['id' => 2], + [], + ['status' => 3], + 1, + ], + [ + '{{customer}}', + ['status' => new Expression( + '1 + :val', + ['val' => new Expression('2 + :val', ['val' => 3])] + )], + '[[name]] != :val', + ['val' => new Expression('LOWER(:val)', ['val' => 'USER1'])], + ['name' => 'user2', 'status' => 6], + 2, ], ]; } diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index 79e7694c2..77122b9c9 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -4,6 +4,8 @@ namespace Yiisoft\Db\Tests\Provider; +use Yiisoft\Db\Command\DataType; +use Yiisoft\Db\Command\Param; use Yiisoft\Db\Expression\Expression; use Yiisoft\Db\Query\Query; use Yiisoft\Db\QueryBuilder\Condition\BetweenColumnsCondition; @@ -267,6 +269,7 @@ public static function buildCondition(): array /* not */ [['not', ''], '', []], + [['not', '0'], 'NOT (0)', []], [['not', 'name'], 'NOT (name)', []], [[ 'not', @@ -1112,10 +1115,59 @@ public static function selectExist(): array public static function update(): array { return [ + [ + '{{table}}', + ['name' => '{{test}}'], + [], + [], + DbHelper::replaceQuotes( + << '{{test}}', + ], + ], + [ + '{{table}}', + ['name' => '{{test}}'], + ['id' => 1], + [], + DbHelper::replaceQuotes( + << '{{test}}', + ':qp1' => 1, + ], + ], + [ + '{{table}}', + ['{{table}}.name' => '{{test}}'], + ['id' => 1], + ['id' => 'boolean'], + DbHelper::replaceQuotes( + << 'boolean', + ':qp1' => '{{test}}', + ':qp2' => 1, + ], + ], [ 'customer', ['status' => 1, 'updated_at' => new Expression('now()')], ['id' => 100], + [], DbHelper::replaceQuotes( << 1, ':qp1' => 100], ], + 'Expressions without params' => [ + '{{product}}', + ['name' => new Expression('UPPER([[name]])')], + '[[name]] = :name', + ['name' => new Expression('LOWER([[name]])')], + DbHelper::replaceQuotes( + << [ + '{{product}}', + ['price' => new Expression('[[price]] + :val', [':val' => 1])], + '[[start_at]] < :date', + ['date' => new Expression('NOW()')], + DbHelper::replaceQuotes( + << 1], + ], + 'Expression without params and with params' => [ + '{{product}}', + ['name' => new Expression('UPPER([[name]])')], + '[[name]] = :name', + ['name' => new Expression('LOWER(:val)', [':val' => 'Apple'])], + DbHelper::replaceQuotes( + << 'Apple'], + ], + 'Expressions with the same params' => [ + '{{product}}', + ['name' => new Expression('LOWER(:val)', ['val' => 'Apple'])], + '[[name]] != :name', + ['name' => new Expression('UPPER(:val)', ['val' => 'Banana'])], + DbHelper::replaceQuotes( + << 'Apple', + 'val_0' => 'Banana', + ], + ], + 'Expressions with the same params starting with and without colon' => [ + '{{product}}', + ['name' => new Expression('LOWER(:val)', [':val' => 'Apple'])], + '[[name]] != :name', + ['name' => new Expression('UPPER(:val)', ['val' => 'Banana'])], + DbHelper::replaceQuotes( + << 'Apple', + 'val_0' => 'Banana', + ], + ], + 'Expressions with the same and different params' => [ + '{{product}}', + ['price' => new Expression('[[price]] * :val + :val1', ['val' => 1.2, 'val1' => 2])], + '[[name]] IN :values', + ['values' => new Expression('(:val, :val2)', ['val' => 'Banana', 'val2' => 'Cherry'])], + DbHelper::replaceQuotes( + << 1.2, + 'val1' => 2, + 'val_0' => 'Banana', + 'val2' => 'Cherry', + ], + ], + 'Expressions with the different params' => [ + '{{product}}', + ['name' => new Expression('LOWER(:val)', ['val' => 'Apple'])], + '[[name]] != :name', + ['name' => new Expression('UPPER(:val1)', ['val1' => 'Banana'])], + DbHelper::replaceQuotes( + << 'Apple', + 'val1' => 'Banana', + ], + ], + 'Expressions with nested Expressions' => [ + '{{table}}', + ['name' => new Expression( + ':val || :val_0', + [ + 'val' => new Expression('LOWER(:val || :val_0)', ['val' => 'A', 'val_0' => 'B']), + 'val_0' => new Param('C', DataType::STRING), + ], + )], + '[[name]] != :val || :val_0', + [ + 'val_0' => new Param('F', DataType::STRING), + 'val' => new Expression('UPPER(:val || :val_0)', ['val' => 'D', 'val_0' => 'E']), + ], + DbHelper::replaceQuotes( + << 'A', + 'val_0_1' => 'B', + 'val_0_0' => new Param('C', DataType::STRING), + 'val_1' => 'D', + 'val_0_2' => 'E', + 'val_0' => new Param('F', DataType::STRING), + ], + ], + 'Expressions with indexed params' => [ + '{{product}}', + ['name' => new Expression('LOWER(?)', ['Apple'])], + '[[name]] != ?', + ['Banana'], + DbHelper::replaceQuotes( + << [ + '{{product}}', + ['price' => 10], + ':val', + [':val' => new Expression("label=':val' AND name=:val", [':val' => 'Apple'])], + DbHelper::replaceQuotes( + << 10, + ':val_0' => 'Apple', + ], + ], + 'Expressions without placeholders in SQL statement' => [ + '{{product}}', + ['price' => 10], + ':val', + [':val' => new Expression("label=':val'", [':val' => 'Apple'])], + DbHelper::replaceQuotes( + << 10, + ':val_0' => 'Apple', + ], + ], ]; } diff --git a/tests/Provider/SqlParserProvider.php b/tests/Provider/SqlParserProvider.php new file mode 100644 index 000000000..7a8e21eae --- /dev/null +++ b/tests/Provider/SqlParserProvider.php @@ -0,0 +1,134 @@ + ExpressionBuilder::class, + ]); + } } diff --git a/tests/Support/Stub/ExpressionBuilder.php b/tests/Support/Stub/ExpressionBuilder.php new file mode 100644 index 000000000..add541b6b --- /dev/null +++ b/tests/Support/Stub/ExpressionBuilder.php @@ -0,0 +1,15 @@ +length - 1; + + while ($this->position < $length) { + $pos = $this->position++; + + match ($this->sql[$pos]) { + ':' => ($word = $this->parseWord()) === '' + ? $this->skipChars(':') + : $result = ':' . $word, + '"', "'" => $this->skipQuotedWithoutEscape($this->sql[$pos]), + '-' => $this->sql[$this->position] === '-' + ? ++$this->position && $this->skipToAfterChar("\n") + : null, + '/' => $this->sql[$this->position] === '*' + ? ++$this->position && $this->skipToAfterString('*/') + : null, + default => null, + }; + + if ($result !== null) { + $position = $pos; + + return $result; + } + } + + return null; + } +} From 3f977a55a05a0f47a2008f59a91692aa490e136b Mon Sep 17 00:00:00 2001 From: Sergei Tigrov Date: Sat, 13 Apr 2024 15:02:47 +0700 Subject: [PATCH 2/2] Allow scalar values for `Query::select()` (#816) * Allow scalar values for `Query::select()` * Apply fixes from StyleCI * Apply Rector changes (CI) * Apply fixes from StyleCI * Update CHANGELOG.md and UPGRADE.md [skip ci] * Update CHANGELOG.md and UPGRADE.md [skip ci] * Update UPGRADE.md [skip ci] * Update UPGRADE.md [skip ci] * fix UPGRADE.md * improve `UPGRADE.md` * Improve psalm types for scalar values in select (#818) * Improve psalm types for scalar values in select * Remove unnecessary psalm annotations --------- Co-authored-by: Tigrov * Improve phpdoc [skip ci] * Update tests/Provider/QueryBuilderProvider.php Co-authored-by: Sergei Predvoditelev * Fix test * Add type string to test --------- Co-authored-by: StyleCI Bot Co-authored-by: Tigrov Co-authored-by: Sergei Predvoditelev --- CHANGELOG.md | 1 + UPGRADE.md | 8 ++++++ src/Query/Query.php | 27 +++++++++++-------- src/Query/QueryInterface.php | 2 ++ src/Query/QueryPartsInterface.php | 20 ++++++++++---- src/QueryBuilder/AbstractDQLQueryBuilder.php | 13 ++++++++- src/QueryBuilder/DQLQueryBuilderInterface.php | 3 +++ tests/AbstractQueryBuilderTest.php | 18 +++++++++++++ tests/AbstractQueryTest.php | 9 ++++++- tests/Provider/QueryBuilderProvider.php | 16 +++++++++++ tests/Provider/QueryProvider.php | 4 +++ 11 files changed, 103 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d34d06330..5eab3b8c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## 2.0.0 under development +- Enh #816: Allow scalar values for `$columns` parameter of `Query::select()` and `Query::addSelect()` methods (@Tigrov) - Enh #806: Non-unique placeholder names inside `Expression::$params` will be replaced with unique names (@Tigrov) - Enh #806: Build `Expression` instances inside `Expression::$params` when build a query using `QueryBuilder` (@Tigrov) - Enh #766: Allow `ColumnInterface` as column type. (@Tigrov) diff --git a/UPGRADE.md b/UPGRADE.md index dde268f37..6b040126c 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -19,6 +19,14 @@ in `addColumn()` method of your classes that implement the following interfaces: - `Yiisoft\Db\QueryBuilder\AbstractDDLQueryBuilder`; - `Yiisoft\Db\QueryBuilder\AbstractQueryBuilder`. +### Scalar values for columns in `Query` + +Change `$columns` parameter type from `array|string|ExpressionInterface` to `array|bool|float|int|string|ExpressionInterface` +in methods `select()` and `addSelect()` of your classes that implement `Yiisoft\Db\Query\QueryPartsInterface`. + +Add support any scalar values for `$columns` parameter of these methods in your classes that implement +`Yiisoft\Db\Query\QueryPartsInterface` or inherit `Yiisoft\Db\Query\Query`. + ### Build `Expression` instances inside `Expression::$params` `ExpressionBuilder` is replaced by an abstract class `AbstractExpressionBuilder` with an instance of the diff --git a/src/Query/Query.php b/src/Query/Query.php index 8321d7a96..d76b16b34 100644 --- a/src/Query/Query.php +++ b/src/Query/Query.php @@ -21,6 +21,7 @@ use function array_shift; use function array_unshift; use function count; +use function gettype; use function is_array; use function is_int; use function is_numeric; @@ -66,9 +67,12 @@ * ``` * * Query internally uses the {@see \Yiisoft\Db\QueryBuilder\AbstractQueryBuilder} class to generate the SQL statement. + * + * @psalm-import-type SelectValue from QueryPartsInterface */ class Query implements QueryInterface { + /** @psalm-var SelectValue $select */ protected array $select = []; protected string|null $selectOption = null; protected bool|null $distinct = null; @@ -178,7 +182,7 @@ public function andHaving(array|string|ExpressionInterface $condition, array $pa return $this; } - public function addSelect(array|string|ExpressionInterface $columns): static + public function addSelect(array|bool|float|int|string|ExpressionInterface $columns): static { if ($this->select === []) { return $this->select($columns); @@ -612,7 +616,7 @@ public function scalar(): bool|int|null|string|float }; } - public function select(array|string|ExpressionInterface $columns, string $option = null): static + public function select(array|bool|float|int|string|ExpressionInterface $columns, string $option = null): static { $this->select = $this->normalizeSelect($columns); $this->selectOption = $option; @@ -854,18 +858,20 @@ private function normalizeOrderBy(array|string|ExpressionInterface $columns): ar /** * Normalizes the `SELECT` columns passed to {@see select()} or {@see addSelect()}. + * + * @psalm-param SelectValue|scalar|ExpressionInterface $columns + * @psalm-return SelectValue */ - private function normalizeSelect(array|ExpressionInterface|string $columns): array + private function normalizeSelect(array|bool|float|int|string|ExpressionInterface $columns): array { - if ($columns instanceof ExpressionInterface) { - $columns = [$columns]; - } elseif (!is_array($columns)) { - $columns = preg_split('/\s*,\s*/', trim($columns), -1, PREG_SPLIT_NO_EMPTY); - } + $columns = match (gettype($columns)) { + 'array' => $columns, + 'string' => preg_split('/\s*,\s*/', trim($columns), -1, PREG_SPLIT_NO_EMPTY), + default => [$columns], + }; $select = []; - /** @psalm-var array $columns */ foreach ($columns as $columnAlias => $columnDefinition) { if (is_string($columnAlias)) { // Already in the normalized format, good for them. @@ -890,8 +896,7 @@ private function normalizeSelect(array|ExpressionInterface|string $columns): arr } } - // Either a string calling a function, DB expression, or sub-query - /** @psalm-var string */ + // Either a string calling a function, instance of ExpressionInterface or a scalar value. $select[] = $columnDefinition; } diff --git a/src/Query/QueryInterface.php b/src/Query/QueryInterface.php index 246d007bd..88b3afb8d 100644 --- a/src/Query/QueryInterface.php +++ b/src/Query/QueryInterface.php @@ -28,6 +28,7 @@ * Sorting is supported via {@see orderBy()} and items can be limited to match some conditions using {@see where()}. * * @psalm-import-type ParamsType from ConnectionInterface + * @psalm-import-type SelectValue from QueryPartsInterface */ interface QueryInterface extends ExpressionInterface, QueryPartsInterface, QueryFunctionsInterface { @@ -207,6 +208,7 @@ public function getParams(): array; /** * @return array The "select" value. + * @psalm-return SelectValue */ public function getSelect(): array; diff --git a/src/Query/QueryPartsInterface.php b/src/Query/QueryPartsInterface.php index 9d2b4dec2..9ab460430 100644 --- a/src/Query/QueryPartsInterface.php +++ b/src/Query/QueryPartsInterface.php @@ -15,6 +15,7 @@ * * {@see Query} uses these methods to build and manipulate SQL statements. * + * @psalm-type SelectValue = array * @psalm-import-type ParamsType from ConnectionInterface */ interface QueryPartsInterface @@ -64,11 +65,15 @@ public function addOrderBy(array|string|ExpressionInterface $columns): static; * $query->addSelect(["*", "CONCAT(first_name, ' ', last_name) AS full_name"])->one(); * ``` * - * @param array|ExpressionInterface|string $columns The columns to add to the select. + * @param array|ExpressionInterface|scalar $columns The columns to add to the select. * - * {@see select()} for more details about the format of this parameter. + * @see select() for more details about the format of this parameter. + * + * @since 2.0.0 `$columns` can be a scalar value or an array of scalar values. + * + * @psalm-param SelectValue|scalar|ExpressionInterface $columns */ - public function addSelect(array|string|ExpressionInterface $columns): static; + public function addSelect(array|bool|float|int|string|ExpressionInterface $columns): static; /** * Adds a filtering condition for a specific column and allow the user to choose a filter operator. @@ -514,7 +519,7 @@ public function rightJoin(array|string $table, array|string $on = '', array $par /** * Sets the `SELECT` part of the query. * - * @param array|ExpressionInterface|string $columns The columns to be selected. + * @param array|ExpressionInterface|scalar $columns The columns to be selected. * Columns can be specified in either a string (for example `id, name`) or an array (such as `['id', 'name']`). * Columns can be prefixed with table names (such as `user.id`) and/or contain column aliases * (for example `user.id AS user_id`). @@ -527,8 +532,13 @@ public function rightJoin(array|string $table, array|string $on = '', array $par * doesn't need alias, don't use a string key). * @param string|null $option More option that should be appended to the 'SELECT' keyword. For example, in MySQL, * the option 'SQL_CALC_FOUND_ROWS' can be used. + * + * @since 2.0.0 `$columns` can be a scalar value or an array of scalar values. + * For example, `$query->select(1)` will be converted to `SELECT 1`. + * + * @psalm-param SelectValue|scalar|ExpressionInterface $columns */ - public function select(array|string|ExpressionInterface $columns, string $option = null): static; + public function select(array|bool|float|int|string|ExpressionInterface $columns, string $option = null): static; /** * It allows you to specify more options for the `SELECT` clause of an SQL statement. diff --git a/src/QueryBuilder/AbstractDQLQueryBuilder.php b/src/QueryBuilder/AbstractDQLQueryBuilder.php index fd3951540..9119ffa7d 100644 --- a/src/QueryBuilder/AbstractDQLQueryBuilder.php +++ b/src/QueryBuilder/AbstractDQLQueryBuilder.php @@ -13,6 +13,7 @@ use Yiisoft\Db\Expression\Expression; use Yiisoft\Db\Expression\ExpressionBuilderInterface; use Yiisoft\Db\Expression\ExpressionInterface; +use Yiisoft\Db\Helper\DbStringHelper; use Yiisoft\Db\QueryBuilder\Condition\HashCondition; use Yiisoft\Db\QueryBuilder\Condition\Interface\ConditionInterface; use Yiisoft\Db\QueryBuilder\Condition\SimpleCondition; @@ -25,6 +26,7 @@ use function array_merge; use function array_shift; use function ctype_digit; +use function gettype; use function implode; use function is_array; use function is_int; @@ -324,7 +326,6 @@ public function buildSelect( return $select . ' *'; } - /** @psalm-var array $columns */ foreach ($columns as $i => $column) { if ($column instanceof ExpressionInterface) { if (is_int($i)) { @@ -333,6 +334,16 @@ public function buildSelect( $columns[$i] = $this->buildExpression($column, $params) . ' AS ' . $this->quoter->quoteColumnName($i); } + } elseif (!is_string($column)) { + $columns[$i] = match (gettype($column)) { + 'double' => DbStringHelper::normalizeFloat($column), + 'boolean' => $column ? 'TRUE' : 'FALSE', + default => (string) $column, + }; + + if (is_string($i)) { + $columns[$i] .= ' AS ' . $this->quoter->quoteColumnName($i); + } } elseif (is_string($i) && $i !== $column) { if (!str_contains($column, '(')) { $column = $this->quoter->quoteColumnName($column); diff --git a/src/QueryBuilder/DQLQueryBuilderInterface.php b/src/QueryBuilder/DQLQueryBuilderInterface.php index dd2076dbb..6a53585cb 100644 --- a/src/QueryBuilder/DQLQueryBuilderInterface.php +++ b/src/QueryBuilder/DQLQueryBuilderInterface.php @@ -11,6 +11,7 @@ use Yiisoft\Db\Exception\NotSupportedException; use Yiisoft\Db\Expression\ExpressionBuilderInterface; use Yiisoft\Db\Expression\ExpressionInterface; +use Yiisoft\Db\Query\QueryPartsInterface; use Yiisoft\Db\QueryBuilder\Condition\Interface\ConditionInterface; use Yiisoft\Db\Query\QueryInterface; @@ -20,6 +21,7 @@ * @link https://en.wikipedia.org/wiki/Data_query_language * * @psalm-import-type ParamsType from ConnectionInterface + * @psalm-import-type SelectValue from QueryPartsInterface */ interface DQLQueryBuilderInterface { @@ -224,6 +226,7 @@ public function buildOrderByAndLimit( * * @return string The `SELECT` clause built from {@see \Yiisoft\Db\Query\Query::select()}. * + * @psalm-param SelectValue $columns * @psalm-param ParamsType $params */ public function buildSelect( diff --git a/tests/AbstractQueryBuilderTest.php b/tests/AbstractQueryBuilderTest.php index 4ab4f287f..35cf22c43 100644 --- a/tests/AbstractQueryBuilderTest.php +++ b/tests/AbstractQueryBuilderTest.php @@ -2091,6 +2091,24 @@ public function testSelectSubquery(): void $this->assertEmpty($params); } + /** @dataProvider \Yiisoft\Db\Tests\Provider\QueryBuilderProvider::selectScalar */ + public function testSelectScalar(array|bool|float|int|string $columns, string $expected): void + { + $db = $this->getConnection(); + $qb = $db->getQueryBuilder(); + + $query = (new Query($db))->select($columns); + + [$sql, $params] = $qb->build($query); + + if ($db->getDriverName() === 'oci') { + $expected .= ' FROM DUAL'; + } + + $this->assertSame($expected, $sql); + $this->assertEmpty($params); + } + public function testSetConditionClasses(): void { $db = $this->getConnection(); diff --git a/tests/AbstractQueryTest.php b/tests/AbstractQueryTest.php index 754cab5a3..cae70b93f 100644 --- a/tests/AbstractQueryTest.php +++ b/tests/AbstractQueryTest.php @@ -661,6 +661,13 @@ public function testSelect(): void ['DISTINCT ON(tour_dates.date_from) tour_dates.date_from', 'tour_dates.id' => 'tour_dates.id'], $query->getSelect() ); + + $query = new Query($db); + $query->select(1); + $query->addSelect(true); + $query->addSelect(['float' => 12.34]); + + $this->assertSame([1, true, 'float' => 12.34], $query->getSelect()); } public function testSetJoin(): void @@ -779,7 +786,7 @@ public function testNormalizeOrderBy(array|string|Expression $columns, array|str /** * @dataProvider \Yiisoft\Db\Tests\Provider\QueryProvider::normalizeSelect */ - public function testNormalizeSelect(array|string|Expression $columns, array|string $expected): void + public function testNormalizeSelect(array|bool|float|int|string|ExpressionInterface $columns, array|string $expected): void { $query = (new Query($this->getConnection())); $this->assertEquals([], $query->getSelect()); diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index 77122b9c9..f8f0b5356 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -1112,6 +1112,22 @@ public static function selectExist(): array ]; } + public static function selectScalar(): array + { + return [ + [1, 'SELECT 1'], + ['custom_string', DbHelper::replaceQuotes('SELECT [[custom_string]]', static::$driverName)], + [true, 'SELECT TRUE'], + [false, 'SELECT FALSE'], + [12.34, 'SELECT 12.34'], + [[1, true, 12.34], 'SELECT 1, TRUE, 12.34'], + [ + ['a' => 1, 'b' => true, 12.34], + DbHelper::replaceQuotes('SELECT 1 AS [[a]], TRUE AS [[b]], 12.34', static::$driverName), + ], + ]; + } + public static function update(): array { return [ diff --git a/tests/Provider/QueryProvider.php b/tests/Provider/QueryProvider.php index 6833f7f4c..274cf2147 100644 --- a/tests/Provider/QueryProvider.php +++ b/tests/Provider/QueryProvider.php @@ -73,6 +73,10 @@ public static function normalizeSelect(): array ['email' => 'email', 'address' => 'address', 'status' => new Expression('1')], ], [new Expression('1 as Ab'), [new Expression('1 as Ab')]], + [1, [1]], + [true, [true]], + [12.34, [12.34]], + [['a' => 1, 'b' => true, 12.34], ['a' => 1, 'b' => true, 12.34]], ]; } }