From e1f142fe5b21c256fd2caf848d82792713783b47 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Mon, 5 Feb 2024 11:27:01 +0700 Subject: [PATCH 01/30] Build expressions if they inside `Expression::$params` --- src/Expression/ExpressionBuilder.php | 93 ++++++++++++++++++- src/QueryBuilder/AbstractDQLQueryBuilder.php | 49 +++------- src/QueryBuilder/AbstractQueryBuilder.php | 2 +- src/QueryBuilder/DQLQueryBuilderInterface.php | 2 +- tests/AbstractQueryBuilderTest.php | 4 +- tests/Common/CommonCommandTest.php | 4 +- tests/Provider/CommandProvider.php | 63 +++++-------- tests/Provider/QueryBuilderProvider.php | 1 + 8 files changed, 136 insertions(+), 82 deletions(-) diff --git a/src/Expression/ExpressionBuilder.php b/src/Expression/ExpressionBuilder.php index fac480612..a78d192c8 100644 --- a/src/Expression/ExpressionBuilder.php +++ b/src/Expression/ExpressionBuilder.php @@ -4,7 +4,13 @@ namespace Yiisoft\Db\Expression; +use Yiisoft\Db\QueryBuilder\QueryBuilderInterface; + use function array_merge; +use function preg_quote; +use function preg_replace; +use function preg_replace_callback; +use function str_starts_with; /** * It's used to build expressions for use in database queries. @@ -17,9 +23,92 @@ */ class ExpressionBuilder implements ExpressionBuilderInterface { + public function __construct(private QueryBuilderInterface $queryBuilder) + { + } + public function build(Expression $expression, array &$params = []): string { - $params = array_merge($params, $expression->getParams()); - return $expression->__toString(); + $sql = $expression->__toString(); + $expressionParams = $expression->getParams(); + + if (empty($expressionParams)) { + return $sql; + } + + if (isset($params[0]) || isset($expressionParams[0])) { + $params = array_merge($params, $expressionParams); + return $sql; + } + + $sql = $this->replaceParamExpressions($sql, $expressionParams, $params); + + return $this->appendParams($sql, $expressionParams, $params); + } + + private function replaceParamExpressions(string $sql, array &$replaceableParams, array &$params): string + { + /** @var string $name */ + foreach ($replaceableParams as $name => $value) { + if (!$value instanceof ExpressionInterface) { + continue; + } + + $sql = preg_replace_callback( + $this->getPattern($name), + function () use ($value, &$params): string { + return $this->queryBuilder->buildExpression($value, $params); + }, + $sql, + 1, + $count, + ); + + if ($count > 0) { + unset($replaceableParams[$name]); + } + } + + return $sql; + } + + private function appendParams(string $sql, array $appendableParams, array &$params): string + { + /** @var string $name */ + foreach ($appendableParams as $name => $value) { + if (isset($params[$name])) { + $pattern = $this->getPattern($name); + $name = $this->getUniqueName($name, $params + $appendableParams); + + $placeholder = !str_starts_with($name, ':') ? ":$name" : $name; + + $sql = preg_replace($pattern, $placeholder, $sql, 1); + } + + $params[$name] = $value; + } + + return $sql; + } + + /** @psalm-return non-empty-string */ + private function getPattern(string $name): string + { + if (!str_starts_with($name, ':')) { + $name = ":$name"; + } + + return '/' . preg_quote($name, '/') . '\b/'; + } + + private function getUniqueName(string $name, array $params): string + { + $uniqueName = $name . '_0'; + + for ($i = 1; isset($params[$uniqueName]); ++$i) { + $uniqueName = $name . '_' . $i; + } + + return $uniqueName; } } diff --git a/src/QueryBuilder/AbstractDQLQueryBuilder.php b/src/QueryBuilder/AbstractDQLQueryBuilder.php index 9f857328f..1a9811635 100644 --- a/src/QueryBuilder/AbstractDQLQueryBuilder.php +++ b/src/QueryBuilder/AbstractDQLQueryBuilder.php @@ -92,7 +92,7 @@ public function __construct( $this->conditionClasses = $this->defaultConditionClasses(); } - public function build(QueryInterface $query, array $params = []): array + public function build(QueryInterface $query, array &$params = []): array { $query = $query->prepare($this->queryBuilder); $params = empty($params) ? $query->getParams() : array_merge($params, $query->getParams()); @@ -105,25 +105,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 +147,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 +193,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 +281,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' : ''); } diff --git a/src/QueryBuilder/AbstractQueryBuilder.php b/src/QueryBuilder/AbstractQueryBuilder.php index 53971f27b..a73c9ba53 100644 --- a/src/QueryBuilder/AbstractQueryBuilder.php +++ b/src/QueryBuilder/AbstractQueryBuilder.php @@ -129,7 +129,7 @@ public function bindParam(mixed $value, array &$params = []): string return $phName; } - public function build(QueryInterface $query, array $params = []): array + public function build(QueryInterface $query, array &$params = []): array { return $this->dqlBuilder->build($query, $params); } diff --git a/src/QueryBuilder/DQLQueryBuilderInterface.php b/src/QueryBuilder/DQLQueryBuilderInterface.php index 759c91150..4a0ca0ec4 100644 --- a/src/QueryBuilder/DQLQueryBuilderInterface.php +++ b/src/QueryBuilder/DQLQueryBuilderInterface.php @@ -39,7 +39,7 @@ interface DQLQueryBuilderInterface * * @psalm-return array{0: string, 1: array} */ - public function build(QueryInterface $query, array $params = []): array; + public function build(QueryInterface $query, array &$params = []): array; /** * Processes columns and quotes them if necessary. diff --git a/tests/AbstractQueryBuilderTest.php b/tests/AbstractQueryBuilderTest.php index a5a0169a7..94d580a59 100644 --- a/tests/AbstractQueryBuilderTest.php +++ b/tests/AbstractQueryBuilderTest.php @@ -2274,7 +2274,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); @@ -2289,7 +2289,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/Common/CommonCommandTest.php b/tests/Common/CommonCommandTest.php index 89710879f..236f2ada4 100644 --- a/tests/Common/CommonCommandTest.php +++ b/tests/Common/CommonCommandTest.php @@ -1888,7 +1888,8 @@ public function testUpdate( array $columns, array|string $conditions, array $params, - string $expected + string $expected, + array $expectedParams = [], ): void { $db = $this->getConnection(); @@ -1896,6 +1897,7 @@ public function testUpdate( $sql = $command->update($table, $columns, $conditions, $params)->getSql(); $this->assertSame($expected, $sql); + $this->assertSame($expectedParams, $command->getParams()); $db->close(); } diff --git a/tests/Provider/CommandProvider.php b/tests/Provider/CommandProvider.php index 689426f80..3cd5d8248 100644 --- a/tests/Provider/CommandProvider.php +++ b/tests/Provider/CommandProvider.php @@ -733,6 +733,9 @@ public static function update(): array SQL, static::$driverName, ), + [ + ':qp0' => '{{test}}' + ], ], [ '{{table}}', @@ -745,42 +748,10 @@ public static function update(): array SQL, static::$driverName, ), - ], - [ - '{{table}}', - ['name' => '{{test}}'], - ['id' => 1], - ['id' => 'integer'], - DbHelper::replaceQuotes( - << '{{test}}'], - ['id' => 1], - ['id' => 'string'], - DbHelper::replaceQuotes( - << '{{test}}'], - ['id' => 1], - ['id' => 'boolean'], - DbHelper::replaceQuotes( - << '{{test}}', + ':qp1' => 1, + ], ], [ '{{table}}', @@ -793,18 +764,30 @@ public static function update(): array SQL, static::$driverName, ), + [ + 'id' => 'boolean', + ':qp1' => '{{test}}', + ':qp2' => 1, + ], ], [ '{{table}}', - ['name' => '{{test}}'], - ['id' => 1], - ['id' => 'float'], + ['name' => new Expression( + '[[name]] || :name', + ['name' => new Expression('LOWER(:val)', ['val' => 'FOO'])] + )], + '[[name]] != :name', + ['name' => new Expression('LOWER(:val)', ['val' => 'BAR'])], DbHelper::replaceQuotes( << 'BAR', + 'val_0' => 'FOO', + ], ], ]; } diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index f37f97ebf..c823a822e 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -266,6 +266,7 @@ public static function buildCondition(): array /* not */ [['not', ''], '', []], + [['not', '0'], 'NOT (0)', []], [['not', 'name'], 'NOT (name)', []], [[ 'not', From c60f3a1ac48249b24db1c89ce0358d7df53eabc7 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Mon, 5 Feb 2024 14:08:45 +0700 Subject: [PATCH 02/30] Improve --- src/Expression/ExpressionBuilder.php | 38 ++++++++----------- src/QueryBuilder/AbstractDQLQueryBuilder.php | 2 +- src/QueryBuilder/AbstractQueryBuilder.php | 2 +- src/QueryBuilder/DQLQueryBuilderInterface.php | 2 +- tests/AbstractQueryBuilderTest.php | 12 +++--- tests/Db/QueryBuilder/QueryBuilderTest.php | 9 +++-- tests/Provider/QueryBuilderProvider.php | 17 +++++++++ 7 files changed, 49 insertions(+), 33 deletions(-) diff --git a/src/Expression/ExpressionBuilder.php b/src/Expression/ExpressionBuilder.php index a78d192c8..23d0ad1d5 100644 --- a/src/Expression/ExpressionBuilder.php +++ b/src/Expression/ExpressionBuilder.php @@ -6,10 +6,10 @@ use Yiisoft\Db\QueryBuilder\QueryBuilderInterface; +use function array_intersect_key; use function array_merge; use function preg_quote; use function preg_replace; -use function preg_replace_callback; use function str_starts_with; /** @@ -54,19 +54,12 @@ private function replaceParamExpressions(string $sql, array &$replaceableParams, continue; } - $sql = preg_replace_callback( - $this->getPattern($name), - function () use ($value, &$params): string { - return $this->queryBuilder->buildExpression($value, $params); - }, - $sql, - 1, - $count, - ); - - if ($count > 0) { - unset($replaceableParams[$name]); - } + $pattern = $this->getPattern($name); + $expression = $this->queryBuilder->buildExpression($value, $params); + + $sql = preg_replace($pattern, $expression, $sql, 1); + + unset($replaceableParams[$name]); } return $sql; @@ -74,18 +67,19 @@ function () use ($value, &$params): string { private function appendParams(string $sql, array $appendableParams, array &$params): string { + $nonUniqueParams = array_intersect_key($appendableParams, $params); + $params += $appendableParams; + /** @var string $name */ - foreach ($appendableParams as $name => $value) { - if (isset($params[$name])) { - $pattern = $this->getPattern($name); - $name = $this->getUniqueName($name, $params + $appendableParams); + foreach ($nonUniqueParams as $name => $value) { + $pattern = $this->getPattern($name); + $uniqueName = $this->getUniqueName($name, $params); - $placeholder = !str_starts_with($name, ':') ? ":$name" : $name; + $placeholder = !str_starts_with($uniqueName, ':') ? ":$uniqueName" : $uniqueName; - $sql = preg_replace($pattern, $placeholder, $sql, 1); - } + $sql = preg_replace($pattern, $placeholder, $sql, 1); - $params[$name] = $value; + $params[$uniqueName] = $value; } return $sql; diff --git a/src/QueryBuilder/AbstractDQLQueryBuilder.php b/src/QueryBuilder/AbstractDQLQueryBuilder.php index 1a9811635..15aec88d7 100644 --- a/src/QueryBuilder/AbstractDQLQueryBuilder.php +++ b/src/QueryBuilder/AbstractDQLQueryBuilder.php @@ -92,7 +92,7 @@ public function __construct( $this->conditionClasses = $this->defaultConditionClasses(); } - public function build(QueryInterface $query, array &$params = []): array + public function build(QueryInterface $query, array $params = []): array { $query = $query->prepare($this->queryBuilder); $params = empty($params) ? $query->getParams() : array_merge($params, $query->getParams()); diff --git a/src/QueryBuilder/AbstractQueryBuilder.php b/src/QueryBuilder/AbstractQueryBuilder.php index a73c9ba53..53971f27b 100644 --- a/src/QueryBuilder/AbstractQueryBuilder.php +++ b/src/QueryBuilder/AbstractQueryBuilder.php @@ -129,7 +129,7 @@ public function bindParam(mixed $value, array &$params = []): string return $phName; } - public function build(QueryInterface $query, array &$params = []): array + public function build(QueryInterface $query, array $params = []): array { return $this->dqlBuilder->build($query, $params); } diff --git a/src/QueryBuilder/DQLQueryBuilderInterface.php b/src/QueryBuilder/DQLQueryBuilderInterface.php index 4a0ca0ec4..759c91150 100644 --- a/src/QueryBuilder/DQLQueryBuilderInterface.php +++ b/src/QueryBuilder/DQLQueryBuilderInterface.php @@ -39,7 +39,7 @@ interface DQLQueryBuilderInterface * * @psalm-return array{0: string, 1: array} */ - public function build(QueryInterface $query, array &$params = []): array; + public function build(QueryInterface $query, array $params = []): array; /** * Processes columns and quotes them if necessary. diff --git a/tests/AbstractQueryBuilderTest.php b/tests/AbstractQueryBuilderTest.php index 94d580a59..7f74a2cf0 100644 --- a/tests/AbstractQueryBuilderTest.php +++ b/tests/AbstractQueryBuilderTest.php @@ -2194,16 +2194,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->assertSame($expectedParams, $params); } /** diff --git a/tests/Db/QueryBuilder/QueryBuilderTest.php b/tests/Db/QueryBuilder/QueryBuilderTest.php index c549aec3d..ae44d6c0d 100644 --- a/tests/Db/QueryBuilder/QueryBuilderTest.php +++ b/tests/Db/QueryBuilder/QueryBuilderTest.php @@ -253,6 +253,7 @@ public function testUpdate( string $table, array $columns, array|string $condition, + array $params, string $expectedSQL, array $expectedParams ): void { @@ -260,10 +261,12 @@ public function testUpdate( $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->assertSame($expectedParams, $params); } /** diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index c823a822e..b2558b729 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -1116,6 +1116,7 @@ public static function update(): array 'customer', ['status' => 1, 'updated_at' => new Expression('now()')], ['id' => 100], + [], DbHelper::replaceQuotes( << 1, ':qp1' => 100], ], + [ + '{{table}}', + ['name' => new Expression( + '[[name]] || :name', + ['name' => new Expression('LOWER(:val)', ['val' => 'FOO'])] + )], + '[[name]] != :name', + ['name' => new Expression('LOWER(:val)', ['val' => 'BAR'])], + DbHelper::replaceQuotes( + << 'BAR', 'val_0' => 'FOO'], + ], ]; } From f15c515dc9a4b8aa63ca9db7387b0207654a070c Mon Sep 17 00:00:00 2001 From: Tigrov Date: Mon, 5 Feb 2024 17:25:06 +0700 Subject: [PATCH 03/30] Fix errors --- src/Expression/ExpressionBuilder.php | 34 ++++++++++++++++++++++-- tests/Provider/CommandProvider.php | 8 +++--- tests/Provider/QueryBuilderProvider.php | 35 ++++++++++++++++++++++--- 3 files changed, 68 insertions(+), 9 deletions(-) diff --git a/src/Expression/ExpressionBuilder.php b/src/Expression/ExpressionBuilder.php index 23d0ad1d5..c8dca5c3f 100644 --- a/src/Expression/ExpressionBuilder.php +++ b/src/Expression/ExpressionBuilder.php @@ -41,11 +41,41 @@ public function build(Expression $expression, array &$params = []): string return $sql; } - $sql = $this->replaceParamExpressions($sql, $expressionParams, $params); + $nonUniqueParams = array_intersect_key($expressionParams, $params); + $params += $expressionParams; - return $this->appendParams($sql, $expressionParams, $params); + /** @var string $name */ + foreach ($nonUniqueParams as $name => $value) { + $pattern = $this->getPattern($name); + $uniqueName = $this->getUniqueName($name, $params); + + $replacement = !str_starts_with($uniqueName, ':') ? ":$uniqueName" : $uniqueName; + + $sql = preg_replace($pattern, $replacement, $sql, 1); + + $params[$uniqueName] = $value; + $expressionParams[$uniqueName] = $value; + unset($expressionParams[$name]); + } + + /** @var string $name */ + foreach ($expressionParams as $name => $value) { + if (!$value instanceof ExpressionInterface) { + continue; + } + + $pattern = $this->getPattern($name); + $replacement = $this->queryBuilder->buildExpression($value, $params); + + $sql = preg_replace($pattern, $replacement, $sql, 1); + + unset($params[$name]); + } + + return $sql; } + private function replaceParamExpressions(string $sql, array &$replaceableParams, array &$params): string { /** @var string $name */ diff --git a/tests/Provider/CommandProvider.php b/tests/Provider/CommandProvider.php index 3cd5d8248..22b4dd983 100644 --- a/tests/Provider/CommandProvider.php +++ b/tests/Provider/CommandProvider.php @@ -774,10 +774,10 @@ public static function update(): array '{{table}}', ['name' => new Expression( '[[name]] || :name', - ['name' => new Expression('LOWER(:val)', ['val' => 'FOO'])] + ['name' => new Expression('LOWER(:val)', ['val' => 'A'])] )], '[[name]] != :name', - ['name' => new Expression('LOWER(:val)', ['val' => 'BAR'])], + ['name' => new Expression('LOWER(:val)', ['val' => 'B'])], DbHelper::replaceQuotes( << 'BAR', - 'val_0' => 'FOO', + 'val' => 'A', + 'val_0' => 'B', ], ], ]; diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index b2558b729..d4aa09950 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -1129,17 +1129,46 @@ public static function update(): array '{{table}}', ['name' => new Expression( '[[name]] || :name', - ['name' => new Expression('LOWER(:val)', ['val' => 'FOO'])] + ['name' => new Expression('LOWER(:val)', ['val' => 'A'])] )], '[[name]] != :name', - ['name' => new Expression('LOWER(:val)', ['val' => 'BAR'])], + ['name' => new Expression('LOWER(:val)', ['val' => 'B'])], DbHelper::replaceQuotes( << 'BAR', 'val_0' => 'FOO'], + ['val' => 'B', 'val_0' => 'A'], + ], + [ + '{{table}}', + ['name' => new Expression( + ':val || :val_0', + [ + 'val' => new Expression('LOWER(:val || :val_0)', ['val' => 'A', 'val_0' => 'B']), + 'val_0' => 'C', + ], + )], + '[[name]] != :val || :val_0', + [ + 'val_0' => 'F', + 'val' => new Expression('LOWER(:val || :val_0)', ['val' => 'D', 'val_0' => 'E']), + ], + DbHelper::replaceQuotes( + << 'F', + 'val_0_0' => 'C', + 'val_2' => 'A', + 'val_0_1' => 'B', + 'val_1' => 'D', + 'val_0_2' => 'E', + ], ], ]; } From 17eb93758298c3f13ca274534cc00bd35b4cb911 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Tue, 6 Feb 2024 14:05:37 +0700 Subject: [PATCH 04/30] Fix errors --- src/Expression/ExpressionBuilder.php | 52 ++++++------------------- tests/Provider/QueryBuilderProvider.php | 14 +++---- 2 files changed, 19 insertions(+), 47 deletions(-) diff --git a/src/Expression/ExpressionBuilder.php b/src/Expression/ExpressionBuilder.php index c8dca5c3f..4c9b020d9 100644 --- a/src/Expression/ExpressionBuilder.php +++ b/src/Expression/ExpressionBuilder.php @@ -41,6 +41,13 @@ public function build(Expression $expression, array &$params = []): string return $sql; } + $sql = $this->appendParams($sql, $expressionParams, $params); + + return $this->replaceParamExpressions($sql, $expressionParams, $params); + } + + private function appendParams(string $sql, array &$expressionParams, array &$params): string + { $nonUniqueParams = array_intersect_key($expressionParams, $params); $params += $expressionParams; @@ -58,58 +65,23 @@ public function build(Expression $expression, array &$params = []): string unset($expressionParams[$name]); } - /** @var string $name */ - foreach ($expressionParams as $name => $value) { - if (!$value instanceof ExpressionInterface) { - continue; - } - - $pattern = $this->getPattern($name); - $replacement = $this->queryBuilder->buildExpression($value, $params); - - $sql = preg_replace($pattern, $replacement, $sql, 1); - - unset($params[$name]); - } - return $sql; } - - private function replaceParamExpressions(string $sql, array &$replaceableParams, array &$params): string + private function replaceParamExpressions(string $sql, array $expressionParams, array &$params): string { /** @var string $name */ - foreach ($replaceableParams as $name => $value) { + foreach ($expressionParams as $name => $value) { if (!$value instanceof ExpressionInterface) { continue; } $pattern = $this->getPattern($name); - $expression = $this->queryBuilder->buildExpression($value, $params); - - $sql = preg_replace($pattern, $expression, $sql, 1); - - unset($replaceableParams[$name]); - } - - return $sql; - } - - private function appendParams(string $sql, array $appendableParams, array &$params): string - { - $nonUniqueParams = array_intersect_key($appendableParams, $params); - $params += $appendableParams; - - /** @var string $name */ - foreach ($nonUniqueParams as $name => $value) { - $pattern = $this->getPattern($name); - $uniqueName = $this->getUniqueName($name, $params); - - $placeholder = !str_starts_with($uniqueName, ':') ? ":$uniqueName" : $uniqueName; + $replacement = $this->queryBuilder->buildExpression($value, $params); - $sql = preg_replace($pattern, $placeholder, $sql, 1); + $sql = preg_replace($pattern, $replacement, $sql, 1); - $params[$uniqueName] = $value; + unset($params[$name]); } return $sql; diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index d4aa09950..4bf1c2342 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -1132,14 +1132,14 @@ public static function update(): array ['name' => new Expression('LOWER(:val)', ['val' => 'A'])] )], '[[name]] != :name', - ['name' => new Expression('LOWER(:val)', ['val' => 'B'])], + ['name' => new Expression('UPPER(:val)', ['val' => 'B'])], DbHelper::replaceQuotes( << 'B', 'val_0' => 'A'], + ['val' => 'A', 'val_0' => 'B'], ], [ '{{table}}', @@ -1153,21 +1153,21 @@ public static function update(): array '[[name]] != :val || :val_0', [ 'val_0' => 'F', - 'val' => new Expression('LOWER(:val || :val_0)', ['val' => 'D', 'val_0' => 'E']), + 'val' => new Expression('UPPER(:val || :val_0)', ['val' => 'D', 'val_0' => 'E']), ], DbHelper::replaceQuotes( << 'F', - 'val_0_0' => 'C', 'val_2' => 'A', 'val_0_1' => 'B', + 'val_0_0' => 'C', 'val_1' => 'D', 'val_0_2' => 'E', + 'val_0' => 'F', ], ], ]; From b325bab24241f63b948949675db32b4a70c00a20 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Tue, 6 Feb 2024 17:23:37 +0700 Subject: [PATCH 05/30] Update tests --- tests/AbstractQueryBuilderTest.php | 2 +- tests/Common/CommonCommandTest.php | 21 ++++-- tests/Db/QueryBuilder/QueryBuilderTest.php | 6 +- tests/Provider/CommandProvider.php | 78 ++++++++-------------- tests/Provider/QueryBuilderProvider.php | 48 +++++++++++++ 5 files changed, 94 insertions(+), 61 deletions(-) diff --git a/tests/AbstractQueryBuilderTest.php b/tests/AbstractQueryBuilderTest.php index 7f74a2cf0..86b879e0e 100644 --- a/tests/AbstractQueryBuilderTest.php +++ b/tests/AbstractQueryBuilderTest.php @@ -2205,7 +2205,7 @@ public function testUpdate( $sql = $qb->quoter()->quoteSql($sql); $this->assertSame($expectedSql, $sql); - $this->assertSame($expectedParams, $params); + $this->assertEquals($expectedParams, $params); } /** diff --git a/tests/Common/CommonCommandTest.php b/tests/Common/CommonCommandTest.php index 236f2ada4..09dc00524 100644 --- a/tests/Common/CommonCommandTest.php +++ b/tests/Common/CommonCommandTest.php @@ -1888,16 +1888,25 @@ public function testUpdate( array $columns, array|string $conditions, array $params, - string $expected, - array $expectedParams = [], + 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($expected, $sql); - $this->assertSame($expectedParams, $command->getParams()); + $this->assertSame($expectedCount, $count); + + $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 ae44d6c0d..d38973c45 100644 --- a/tests/Db/QueryBuilder/QueryBuilderTest.php +++ b/tests/Db/QueryBuilder/QueryBuilderTest.php @@ -254,7 +254,7 @@ public function testUpdate( array $columns, array|string $condition, array $params, - string $expectedSQL, + string $expectedSql, array $expectedParams ): void { $db = $this->getConnection(); @@ -265,8 +265,8 @@ public function testUpdate( $sql = $qb->update($table, $columns, $condition, $params); $sql = $qb->quoter()->quoteSql($sql); - $this->assertSame($expectedSQL, $sql); - $this->assertSame($expectedParams, $params); + $this->assertSame($expectedSql, $sql); + $this->assertEquals($expectedParams, $params); } /** diff --git a/tests/Provider/CommandProvider.php b/tests/Provider/CommandProvider.php index 22b4dd983..156c50910 100644 --- a/tests/Provider/CommandProvider.php +++ b/tests/Provider/CommandProvider.php @@ -723,71 +723,47 @@ public static function update(): array { return [ [ - '{{table}}', + '{{customer}}', ['name' => '{{test}}'], [], [], - DbHelper::replaceQuotes( - << '{{test}}' - ], + ['name' => '{{test}}'], + 3, ], [ - '{{table}}', + '{{customer}}', ['name' => '{{test}}'], ['id' => 1], [], - DbHelper::replaceQuotes( - << '{{test}}', - ':qp1' => 1, - ], + ['name' => '{{test}}'], + 1, ], [ - '{{table}}', - ['{{table}}.name' => '{{test}}'], + '{{customer}}', + ['{{customer}}.name' => '{{test}}'], ['id' => 1], - ['id' => 'boolean'], - DbHelper::replaceQuotes( - << 'boolean', - ':qp1' => '{{test}}', - ':qp2' => 1, - ], + [], + ['name' => '{{test}}'], + 1, ], [ - '{{table}}', - ['name' => new Expression( - '[[name]] || :name', - ['name' => new Expression('LOWER(:val)', ['val' => 'A'])] + 'customer', + ['status' => new Expression('1 + 2')], + ['id' => 2], + [], + ['status' => 3], + 1, + ], + [ + '{{customer}}', + ['status' => new Expression( + '1 + :val', + ['val' => new Expression('2 + :val', ['val' => 3])] )], - '[[name]] != :name', - ['name' => new Expression('LOWER(:val)', ['val' => 'B'])], - DbHelper::replaceQuotes( - << 'A', - 'val_0' => 'B', - ], + '[[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 4bf1c2342..adbdba624 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -1112,6 +1112,54 @@ 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()')], From 54dedfb2d91a7c31a0c2869f48dc5004a0c706cc Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Tue, 6 Feb 2024 10:27:30 +0000 Subject: [PATCH 06/30] Apply fixes from StyleCI --- tests/Provider/QueryBuilderProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index adbdba624..6e1aacd19 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -1124,7 +1124,7 @@ public static function update(): array static::$driverName, ), [ - ':qp0' => '{{test}}' + ':qp0' => '{{test}}', ], ], [ From 02708bb4e7c34365c51fea115191916e0ae49823 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sun, 11 Feb 2024 14:02:59 +0700 Subject: [PATCH 07/30] Add tests --- tests/Provider/QueryBuilderProvider.php | 91 ++++++++++++++++++++++--- 1 file changed, 82 insertions(+), 9 deletions(-) diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index 6e1aacd19..5079085cb 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -1173,21 +1173,94 @@ public static function update(): array ), [':qp0' => 1, ':qp1' => 100], ], - [ - '{{table}}', - ['name' => new Expression( - '[[name]] || :name', - ['name' => new Expression('LOWER(:val)', ['val' => 'A'])] - )], + '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' => 'B'])], + ['name' => new Expression('UPPER(:val)', ['val' => 'Banana'])], DbHelper::replaceQuotes( << 'A', 'val_0' => 'B'], + [ + 'val' => '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', + ], ], [ '{{table}}', From 5984d3405c4c60e3aff1b52961a09cfbded35ee8 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sun, 11 Feb 2024 14:04:02 +0700 Subject: [PATCH 08/30] Resolve BC, make `$queryBuilder` optional in the constructor. --- src/Expression/ExpressionBuilder.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Expression/ExpressionBuilder.php b/src/Expression/ExpressionBuilder.php index 4c9b020d9..862c839b0 100644 --- a/src/Expression/ExpressionBuilder.php +++ b/src/Expression/ExpressionBuilder.php @@ -23,7 +23,7 @@ */ class ExpressionBuilder implements ExpressionBuilderInterface { - public function __construct(private QueryBuilderInterface $queryBuilder) + public function __construct(private QueryBuilderInterface|null $queryBuilder = null) { } @@ -36,7 +36,7 @@ public function build(Expression $expression, array &$params = []): string return $sql; } - if (isset($params[0]) || isset($expressionParams[0])) { + if ($this->queryBuilder === null || isset($params[0]) || isset($expressionParams[0])) { $params = array_merge($params, $expressionParams); return $sql; } From 7803afbc571ce5a8dadad8108d1082e025159afd Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Sun, 11 Feb 2024 07:04:23 +0000 Subject: [PATCH 09/30] Apply fixes from StyleCI --- tests/Provider/QueryBuilderProvider.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index 5079085cb..dd821984b 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -1175,9 +1175,9 @@ public static function update(): array ], 'Expressions without params' => [ '{{product}}', - ['name' => new Expression("UPPER([[name]])")], + ['name' => new Expression('UPPER([[name]])')], '[[name]] = :name', - ['name' => new Expression("LOWER([[name]])")], + ['name' => new Expression('LOWER([[name]])')], DbHelper::replaceQuotes( << [ '{{product}}', - ['name' => new Expression("UPPER([[name]])")], + ['name' => new Expression('UPPER([[name]])')], '[[name]] = :name', - ['name' => new Expression("LOWER(:val)", [':val' => 'Apple'])], + ['name' => new Expression('LOWER(:val)', [':val' => 'Apple'])], DbHelper::replaceQuotes( << Date: Sun, 11 Feb 2024 14:22:17 +0700 Subject: [PATCH 10/30] Fix psalm issue --- src/Expression/ExpressionBuilder.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Expression/ExpressionBuilder.php b/src/Expression/ExpressionBuilder.php index 862c839b0..271c74526 100644 --- a/src/Expression/ExpressionBuilder.php +++ b/src/Expression/ExpressionBuilder.php @@ -77,6 +77,7 @@ private function replaceParamExpressions(string $sql, array $expressionParams, a } $pattern = $this->getPattern($name); + /** @psalm-suppress PossiblyNullReference */ $replacement = $this->queryBuilder->buildExpression($value, $params); $sql = preg_replace($pattern, $replacement, $sql, 1); From b922159539854bee97142e6694885d5b292e0838 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Tue, 13 Feb 2024 11:00:09 +0700 Subject: [PATCH 11/30] Improve --- src/Expression/ExpressionBuilder.php | 35 +++++++++++++++++----------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/src/Expression/ExpressionBuilder.php b/src/Expression/ExpressionBuilder.php index 271c74526..50dcd4cb3 100644 --- a/src/Expression/ExpressionBuilder.php +++ b/src/Expression/ExpressionBuilder.php @@ -10,7 +10,6 @@ use function array_merge; use function preg_quote; use function preg_replace; -use function str_starts_with; /** * It's used to build expressions for use in database queries. @@ -36,7 +35,7 @@ public function build(Expression $expression, array &$params = []): string return $sql; } - if ($this->queryBuilder === null || isset($params[0]) || isset($expressionParams[0])) { + if ($this->queryBuilder === null || isset($expressionParams[0])) { $params = array_merge($params, $expressionParams); return $sql; } @@ -51,47 +50,57 @@ private function appendParams(string $sql, array &$expressionParams, array &$par $nonUniqueParams = array_intersect_key($expressionParams, $params); $params += $expressionParams; + if (empty($nonUniqueParams)) { + return $sql; + } + + $patterns = []; + $replacements = []; + /** @var string $name */ foreach ($nonUniqueParams as $name => $value) { - $pattern = $this->getPattern($name); + $patterns[] = $this->getPattern($name); $uniqueName = $this->getUniqueName($name, $params); - $replacement = !str_starts_with($uniqueName, ':') ? ":$uniqueName" : $uniqueName; - - $sql = preg_replace($pattern, $replacement, $sql, 1); + $replacements[] = $uniqueName[0] !== ':' ? ":$uniqueName" : $uniqueName; $params[$uniqueName] = $value; $expressionParams[$uniqueName] = $value; unset($expressionParams[$name]); } - return $sql; + return preg_replace($patterns, $replacements, $sql, 1); } private function replaceParamExpressions(string $sql, array $expressionParams, array &$params): string { + $patterns = []; + $replacements = []; + /** @var string $name */ foreach ($expressionParams as $name => $value) { if (!$value instanceof ExpressionInterface) { continue; } - $pattern = $this->getPattern($name); + $patterns[] = $this->getPattern($name); /** @psalm-suppress PossiblyNullReference */ - $replacement = $this->queryBuilder->buildExpression($value, $params); - - $sql = preg_replace($pattern, $replacement, $sql, 1); + $replacements[] = $this->queryBuilder->buildExpression($value, $params); unset($params[$name]); } - return $sql; + if (empty($patterns)) { + return $sql; + } + + return preg_replace($patterns, $replacements, $sql, 1); } /** @psalm-return non-empty-string */ private function getPattern(string $name): string { - if (!str_starts_with($name, ':')) { + if ($name[0] !== ':') { $name = ":$name"; } From a5bff4a04f49989a0a9f87523293f2761537c429 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Tue, 13 Feb 2024 11:00:29 +0700 Subject: [PATCH 12/30] Add test for indexed params --- tests/Provider/QueryBuilderProvider.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index dd821984b..c868e74e2 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -1291,6 +1291,20 @@ public static function update(): array 'val_0' => 'F', ], ], + 'Expressions with indexed params' => [ + '{{product}}', + ['name' => new Expression('LOWER(?)', ['Apple'])], + '[[name]] != ?', + ['Banana'], + DbHelper::replaceQuotes( + << Date: Thu, 28 Mar 2024 15:21:30 +0700 Subject: [PATCH 13/30] Fix using regex when string value containing a placeholder name --- src/Expression/ExpressionBuilder.php | 6 ++++-- tests/Provider/QueryBuilderProvider.php | 18 +++++++++++++++++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/Expression/ExpressionBuilder.php b/src/Expression/ExpressionBuilder.php index 50dcd4cb3..82a5d8809 100644 --- a/src/Expression/ExpressionBuilder.php +++ b/src/Expression/ExpressionBuilder.php @@ -62,7 +62,7 @@ private function appendParams(string $sql, array &$expressionParams, array &$par $patterns[] = $this->getPattern($name); $uniqueName = $this->getUniqueName($name, $params); - $replacements[] = $uniqueName[0] !== ':' ? ":$uniqueName" : $uniqueName; + $replacements[] = '$1' . ($uniqueName[0] !== ':' ? ":$uniqueName" : $uniqueName); $params[$uniqueName] = $value; $expressionParams[$uniqueName] = $value; @@ -104,7 +104,9 @@ private function getPattern(string $name): string $name = ":$name"; } - return '/' . preg_quote($name, '/') . '\b/'; + $skipQuotedStrings = '((?:([\'"`])(?:.*?(?:\\\\)*(?:\\\2)?)*\2.*?)*)'; + + return '/' . $skipQuotedStrings . preg_quote($name, '/') . '\b/'; } private function getUniqueName(string $name, array $params): string diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index c868e74e2..33fe4900f 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -1262,7 +1262,7 @@ public static function update(): array 'val1' => 'Banana', ], ], - [ + 'Expressions with nested Expressions' => [ '{{table}}', ['name' => new Expression( ':val || :val_0', @@ -1305,6 +1305,22 @@ public static function update(): array // Wrong order of params ['Banana', 'Apple'], ], + 'Expressions with a string value containing a placeholder name' => [ + '{{product}}', + ['price' => 10], + ':val', + [':val' => new Expression("label=':val\\\'a\\\'' label1=':val\\\'a\\\'' AND name=:val", [':val' => 'Apple'])], + DbHelper::replaceQuotes( + << 10, + ':val_0' => 'Apple', + ], + ], ]; } From 5daa211f5bf5d555be846396ca4ece2a2f3b0366 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 30 Mar 2024 08:57:26 +0700 Subject: [PATCH 14/30] Add SqlParser to fix case when string value containing a placeholder name. Fix unique param name w/ and w/o colon --- src/Expression/ExpressionBuilder.php | 152 +++++++++++++++----- src/Syntax/SqlParser.php | 183 ++++++++++++++++++++++++ tests/AbstractSqlParserTest.php | 45 ++++++ tests/Db/Syntax/SqlParserTest.php | 14 ++ tests/Provider/QueryBuilderProvider.php | 20 ++- tests/Provider/SqlParserProvider.php | 110 ++++++++++++++ 6 files changed, 484 insertions(+), 40 deletions(-) create mode 100644 src/Syntax/SqlParser.php create mode 100644 tests/AbstractSqlParserTest.php create mode 100644 tests/Db/Syntax/SqlParserTest.php create mode 100644 tests/Provider/SqlParserProvider.php diff --git a/src/Expression/ExpressionBuilder.php b/src/Expression/ExpressionBuilder.php index 82a5d8809..af4998357 100644 --- a/src/Expression/ExpressionBuilder.php +++ b/src/Expression/ExpressionBuilder.php @@ -4,12 +4,14 @@ namespace Yiisoft\Db\Expression; +use Yiisoft\Db\Connection\ConnectionInterface; use Yiisoft\Db\QueryBuilder\QueryBuilderInterface; +use Yiisoft\Db\Syntax\SqlParser; -use function array_intersect_key; use function array_merge; -use function preg_quote; -use function preg_replace; +use function strlen; +use function substr; +use function substr_replace; /** * It's used to build expressions for use in database queries. @@ -19,6 +21,8 @@ * * These expressions can be used with the query builder to build complex and customizable database queries * {@see Expression} class. + * + * @psalm-import-type ParamsType from ConnectionInterface */ class ExpressionBuilder implements ExpressionBuilderInterface { @@ -26,6 +30,16 @@ public function __construct(private QueryBuilderInterface|null $queryBuilder = n { } + /** + * Builds SQL statement from the given expression. + * + * @param Expression $expression The expression to be built. + * @param array $params The parameters to be bound to the query. + * + * @psalm-param ParamsType $params + * + * @return string SQL statement. + */ public function build(Expression $expression, array &$params = []): string { $sql = $expression->__toString(); @@ -45,78 +59,140 @@ public function build(Expression $expression, array &$params = []): string return $this->replaceParamExpressions($sql, $expressionParams, $params); } + /** + * Appends parameters to the list of query parameters replacing non-unique parameters with unique ones. + * + * @param string $sql SQL statement of the expression. + * @param array $expressionParams Parameters to be appended. + * @param array $params Parameters to be bound to the query. + * + * @psalm-param ParamsType $params + * + * @return string SQL statement with unique parameters. + */ private function appendParams(string $sql, array &$expressionParams, array &$params): string { - $nonUniqueParams = array_intersect_key($expressionParams, $params); - $params += $expressionParams; + $nonUniqueParams = []; - if (empty($nonUniqueParams)) { - return $sql; - } + /** @var non-empty-string $name */ + foreach ($expressionParams as $name => $value) { + $paramName = $name[0] === ':' ? substr($name, 1) : $name; - $patterns = []; - $replacements = []; + if (!isset($params[$paramName]) && !isset($params[":$paramName"])) { + $params[$name] = $value; + continue; + } + + $nonUniqueParams[$name] = $value; + } - /** @var string $name */ + /** @var non-empty-string $name */ foreach ($nonUniqueParams as $name => $value) { - $patterns[] = $this->getPattern($name); - $uniqueName = $this->getUniqueName($name, $params); - $replacements[] = '$1' . ($uniqueName[0] !== ':' ? ":$uniqueName" : $uniqueName); + $paramName = $name[0] === ':' ? substr($name, 1) : $name; + $uniqueName = $this->getUniqueName($paramName, $params); + + $sql = $this->replacePlaceholder($sql, ":$paramName", ":$uniqueName"); + + if ($name[0] === ':') { + $uniqueName = ":$uniqueName"; + } $params[$uniqueName] = $value; $expressionParams[$uniqueName] = $value; unset($expressionParams[$name]); } - return preg_replace($patterns, $replacements, $sql, 1); + return $sql; } + /** + * Replaces parameters with expression values in SQL statement. + * + * @param string $sql SQL statement where parameters should be replaced. + * @param array $expressionParams Parameters to be replaced. + * @param array $params Parameters to be bound to the query. + * + * @psalm-param ParamsType $expressionParams + * @psalm-param ParamsType $params + * + * @return string SQL statement with replaced parameters. + */ private function replaceParamExpressions(string $sql, array $expressionParams, array &$params): string { - $patterns = []; - $replacements = []; - - /** @var string $name */ + /** @var non-empty-string $name */ foreach ($expressionParams as $name => $value) { if (!$value instanceof ExpressionInterface) { continue; } - $patterns[] = $this->getPattern($name); + $placeholder = $name[0] !== ':' ? ":$name" : $name; /** @psalm-suppress PossiblyNullReference */ - $replacements[] = $this->queryBuilder->buildExpression($value, $params); + $replacement = $this->queryBuilder->buildExpression($value, $params); - unset($params[$name]); - } + $sql = $this->replacePlaceholder($sql, $placeholder, $replacement); - if (empty($patterns)) { - return $sql; + /** @psalm-var ParamsType $params */ + unset($params[$name]); } - return preg_replace($patterns, $replacements, $sql, 1); + return $sql; } - /** @psalm-return non-empty-string */ - private function getPattern(string $name): string + /** + * 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 { - if ($name[0] !== ':') { - $name = ":$name"; - } + $uniqueName = $name . '_0'; - $skipQuotedStrings = '((?:([\'"`])(?:.*?(?:\\\\)*(?:\\\2)?)*\2.*?)*)'; + for ($i = 1; isset($params[$uniqueName]) || isset($params[":$uniqueName"]); ++$i) { + $uniqueName = $name . '_' . $i; + } - return '/' . $skipQuotedStrings . preg_quote($name, '/') . '\b/'; + return $uniqueName; } - private function getUniqueName(string $name, array $params): string + /** + * Replaces the placeholder with the replacement in SQL statement. + * + * @param string $sql SQL statement where the placeholder should be replaced. + * @param string $placeholder Placeholder to be replaced. + * @param string $replacement Replacement for the placeholder. + * + * @return string SQL with the replaced placeholder. + */ + private function replacePlaceholder(string $sql, string $placeholder, string $replacement): string { - $uniqueName = $name . '_0'; + $parser = $this->createSqlParser($sql); - for ($i = 1; isset($params[$uniqueName]); ++$i) { - $uniqueName = $name . '_' . $i; + while (null !== $parsedPlaceholder = $parser->getNextPlaceholder($position)) { + if ($parsedPlaceholder === $placeholder) { + return substr_replace($sql, $replacement, $position, strlen($placeholder)); + } } - return $uniqueName; + return $sql; + } + + /** + * Creates an instance of {@see SqlParser} for the given SQL statement. + * + * @param string $sql SQL statement to be parsed. + * + * @return SqlParser SQL parser instance. + */ + protected function createSqlParser(string $sql): SqlParser + { + return new SqlParser($sql); } } diff --git a/src/Syntax/SqlParser.php b/src/Syntax/SqlParser.php new file mode 100644 index 000000000..c28e8fdb0 --- /dev/null +++ b/src/Syntax/SqlParser.php @@ -0,0 +1,183 @@ +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. + */ + public function getNextPlaceholder(int|null &$position = null): string|null + { + $result = null; + $length = $this->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; + } + + /** + * 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] === $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/AbstractSqlParserTest.php b/tests/AbstractSqlParserTest.php new file mode 100644 index 000000000..f1c77101d --- /dev/null +++ b/tests/AbstractSqlParserTest.php @@ -0,0 +1,45 @@ +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 = new SqlParser($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/Db/Syntax/SqlParserTest.php b/tests/Db/Syntax/SqlParserTest.php new file mode 100644 index 000000000..d6b7284bf --- /dev/null +++ b/tests/Db/Syntax/SqlParserTest.php @@ -0,0 +1,14 @@ + '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])], @@ -1309,10 +1325,10 @@ public static function update(): array '{{product}}', ['price' => 10], ':val', - [':val' => new Expression("label=':val\\\'a\\\'' label1=':val\\\'a\\\'' AND name=:val", [':val' => 'Apple'])], + [':val' => new Expression("label=':val' AND name=:val", [':val' => 'Apple'])], DbHelper::replaceQuotes( << Date: Sat, 30 Mar 2024 02:11:36 +0000 Subject: [PATCH 15/30] Apply fixes from StyleCI --- src/Expression/ExpressionBuilder.php | 1 - tests/Provider/SqlParserProvider.php | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Expression/ExpressionBuilder.php b/src/Expression/ExpressionBuilder.php index af4998357..51b82787f 100644 --- a/src/Expression/ExpressionBuilder.php +++ b/src/Expression/ExpressionBuilder.php @@ -88,7 +88,6 @@ private function appendParams(string $sql, array &$expressionParams, array &$par /** @var non-empty-string $name */ foreach ($nonUniqueParams as $name => $value) { - $paramName = $name[0] === ':' ? substr($name, 1) : $name; $uniqueName = $this->getUniqueName($paramName, $params); diff --git a/tests/Provider/SqlParserProvider.php b/tests/Provider/SqlParserProvider.php index c49bbcd4b..2dcd65d1b 100644 --- a/tests/Provider/SqlParserProvider.php +++ b/tests/Provider/SqlParserProvider.php @@ -15,7 +15,7 @@ public static function getNextPlaceholder(): array null, ], [ - "SELECT * FROM {{customer}} WHERE name = ::name", + 'SELECT * FROM {{customer}} WHERE name = ::name', null, null, ], From 79395aafbb31806f45144d87b1b57dd3ea9275ad Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 30 Mar 2024 10:09:37 +0700 Subject: [PATCH 16/30] Fix psalm issue --- src/Expression/ExpressionBuilder.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Expression/ExpressionBuilder.php b/src/Expression/ExpressionBuilder.php index 51b82787f..88074bbef 100644 --- a/src/Expression/ExpressionBuilder.php +++ b/src/Expression/ExpressionBuilder.php @@ -176,6 +176,7 @@ private function replacePlaceholder(string $sql, string $placeholder, string $re while (null !== $parsedPlaceholder = $parser->getNextPlaceholder($position)) { if ($parsedPlaceholder === $placeholder) { + /** @var int $position */ return substr_replace($sql, $replacement, $position, strlen($placeholder)); } } From 6f2cbb51a2855a73de9b9a14da5b2f489760dc98 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 30 Mar 2024 16:28:44 +0700 Subject: [PATCH 17/30] Cover tests --- src/Syntax/SqlParser.php | 2 +- tests/Provider/QueryBuilderProvider.php | 16 ++++++++++++++++ tests/Provider/SqlParserProvider.php | 19 +++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/Syntax/SqlParser.php b/src/Syntax/SqlParser.php index c28e8fdb0..509be0f4d 100644 --- a/src/Syntax/SqlParser.php +++ b/src/Syntax/SqlParser.php @@ -119,7 +119,7 @@ final protected function skipQuotedWithoutEscape(string $endChar): void { do { $this->skipToAfterChar($endChar); - } while ($this->sql[$this->position] === $endChar && ++$this->position); + } while (($this->sql[$this->position] ?? null) === $endChar && ++$this->position); } /** diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index 3104f562f..ed7deaaa4 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -1337,6 +1337,22 @@ public static function update(): array ':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 index 2dcd65d1b..e72563d52 100644 --- a/tests/Provider/SqlParserProvider.php +++ b/tests/Provider/SqlParserProvider.php @@ -74,6 +74,25 @@ public static function getNextPlaceholder(): array ':age', 52, ], + [ + << Date: Sat, 30 Mar 2024 16:31:12 +0700 Subject: [PATCH 18/30] Cover tests --- tests/Provider/SqlParserProvider.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Provider/SqlParserProvider.php b/tests/Provider/SqlParserProvider.php index e72563d52..d964217b8 100644 --- a/tests/Provider/SqlParserProvider.php +++ b/tests/Provider/SqlParserProvider.php @@ -77,7 +77,7 @@ public static function getNextPlaceholder(): array [ << Date: Sat, 30 Mar 2024 16:31:44 +0700 Subject: [PATCH 19/30] Cover tests --- tests/Provider/SqlParserProvider.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Provider/SqlParserProvider.php b/tests/Provider/SqlParserProvider.php index d964217b8..e7d42f828 100644 --- a/tests/Provider/SqlParserProvider.php +++ b/tests/Provider/SqlParserProvider.php @@ -80,7 +80,7 @@ public static function getNextPlaceholder(): array WHERE 2-1 AND name = :name SQL, ':name', - 52, + 60, ], [ << Date: Mon, 1 Apr 2024 11:28:58 +0700 Subject: [PATCH 20/30] Optimization --- src/Expression/ExpressionBuilder.php | 93 +++++++++++++++++++++------- 1 file changed, 69 insertions(+), 24 deletions(-) diff --git a/src/Expression/ExpressionBuilder.php b/src/Expression/ExpressionBuilder.php index 88074bbef..9adc2f5b2 100644 --- a/src/Expression/ExpressionBuilder.php +++ b/src/Expression/ExpressionBuilder.php @@ -9,6 +9,7 @@ use Yiisoft\Db\Syntax\SqlParser; use function array_merge; +use function count; use function strlen; use function substr; use function substr_replace; @@ -54,23 +55,30 @@ public function build(Expression $expression, array &$params = []): string return $sql; } - $sql = $this->appendParams($sql, $expressionParams, $params); + $nonUniqueReplacements = $this->appendParams($expressionParams, $params); + $expressionReplacements = $this->buildParamExpressions($expressionParams, $params); - return $this->replaceParamExpressions($sql, $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 string $sql SQL statement of the expression. * @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 SQL statement with unique parameters. + * @return string[] Replacements for non-unique parameters. */ - private function appendParams(string $sql, array &$expressionParams, array &$params): string + private function appendParams(array &$expressionParams, array &$params): array { $nonUniqueParams = []; @@ -86,12 +94,14 @@ private function appendParams(string $sql, array &$expressionParams, array &$par $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); - $sql = $this->replacePlaceholder($sql, ":$paramName", ":$uniqueName"); + $replacements[":$paramName"] = ":$uniqueName"; if ($name[0] === ':') { $uniqueName = ":$uniqueName"; @@ -102,23 +112,24 @@ private function appendParams(string $sql, array &$expressionParams, array &$par unset($expressionParams[$name]); } - return $sql; + return $replacements; } /** - * Replaces parameters with expression values in SQL statement. + * Build expression values of parameters. * - * @param string $sql SQL statement where parameters should be replaced. - * @param array $expressionParams Parameters to be replaced. + * @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 SQL statement with replaced parameters. + * @return string[] Replacements for parameters. */ - private function replaceParamExpressions(string $sql, array $expressionParams, array &$params): string + private function buildParamExpressions(array $expressionParams, array &$params): array { + $replacements = []; + /** @var non-empty-string $name */ foreach ($expressionParams as $name => $value) { if (!$value instanceof ExpressionInterface) { @@ -127,15 +138,42 @@ private function replaceParamExpressions(string $sql, array $expressionParams, a $placeholder = $name[0] !== ':' ? ":$name" : $name; /** @psalm-suppress PossiblyNullReference */ - $replacement = $this->queryBuilder->buildExpression($value, $params); - - $sql = $this->replacePlaceholder($sql, $placeholder, $replacement); + $replacements[$placeholder] = $this->queryBuilder->buildExpression($value, $params); /** @psalm-var ParamsType $params */ unset($params[$name]); } - return $sql; + 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; } /** @@ -162,22 +200,29 @@ private function getUniqueName(string $name, array $params): string } /** - * Replaces the placeholder with the replacement in SQL statement. + * Replaces placeholders with replacements in SQL statement. * * @param string $sql SQL statement where the placeholder should be replaced. - * @param string $placeholder Placeholder to be replaced. - * @param string $replacement Replacement for the placeholder. + * @param string[] $replacements Replacements for placeholders. * - * @return string SQL with the replaced placeholder. + * @return string SQL statement with the replaced placeholders. */ - private function replacePlaceholder(string $sql, string $placeholder, string $replacement): string + private function replacePlaceholders(string $sql, array $replacements): string { $parser = $this->createSqlParser($sql); + $offset = 0; - while (null !== $parsedPlaceholder = $parser->getNextPlaceholder($position)) { - if ($parsedPlaceholder === $placeholder) { + while (null !== $placeholder = $parser->getNextPlaceholder($position)) { + if (isset($replacements[$placeholder])) { /** @var int $position */ - return substr_replace($sql, $replacement, $position, strlen($placeholder)); + $sql = substr_replace($sql, $replacements[$placeholder], $position + $offset, strlen($placeholder)); + + if (count($replacements) === 1) { + break; + } + + $offset += strlen($replacements[$placeholder]) - strlen($placeholder); + unset($replacements[$placeholder]); } } From 6d1de52e77557ed43368f1e12e3c539daddfb9c9 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Mon, 1 Apr 2024 19:20:44 +0700 Subject: [PATCH 21/30] Make `SqlParser` abstract. Add test for `Param` and fix --- ...lder.php => AbstractExpressionBuilder.php} | 16 ++++--- src/QueryBuilder/AbstractDQLQueryBuilder.php | 2 - .../{SqlParser.php => AbstractSqlParser.php} | 34 +-------------- tests/AbstractSqlParserTest.php | 9 ++-- tests/Db/Syntax/SqlParserTest.php | 14 ------- tests/Provider/QueryBuilderProvider.php | 10 +++-- tests/Support/Stub/DQLQueryBuilder.php | 8 ++++ tests/Support/Stub/ExpressionBuilder.php | 15 +++++++ tests/Support/Stub/SqlParser.php | 42 +++++++++++++++++++ 9 files changed, 83 insertions(+), 67 deletions(-) rename src/Expression/{ExpressionBuilder.php => AbstractExpressionBuilder.php} (94%) rename src/Syntax/{SqlParser.php => AbstractSqlParser.php} (80%) delete mode 100644 tests/Db/Syntax/SqlParserTest.php create mode 100644 tests/Support/Stub/ExpressionBuilder.php create mode 100644 tests/Support/Stub/SqlParser.php diff --git a/src/Expression/ExpressionBuilder.php b/src/Expression/AbstractExpressionBuilder.php similarity index 94% rename from src/Expression/ExpressionBuilder.php rename to src/Expression/AbstractExpressionBuilder.php index 9adc2f5b2..1436d0984 100644 --- a/src/Expression/ExpressionBuilder.php +++ b/src/Expression/AbstractExpressionBuilder.php @@ -4,9 +4,10 @@ namespace Yiisoft\Db\Expression; +use Yiisoft\Db\Command\Param; use Yiisoft\Db\Connection\ConnectionInterface; use Yiisoft\Db\QueryBuilder\QueryBuilderInterface; -use Yiisoft\Db\Syntax\SqlParser; +use Yiisoft\Db\Syntax\AbstractSqlParser; use function array_merge; use function count; @@ -25,7 +26,7 @@ * * @psalm-import-type ParamsType from ConnectionInterface */ -class ExpressionBuilder implements ExpressionBuilderInterface +abstract class AbstractExpressionBuilder implements ExpressionBuilderInterface { public function __construct(private QueryBuilderInterface|null $queryBuilder = null) { @@ -132,7 +133,7 @@ private function buildParamExpressions(array $expressionParams, array &$params): /** @var non-empty-string $name */ foreach ($expressionParams as $name => $value) { - if (!$value instanceof ExpressionInterface) { + if (!$value instanceof ExpressionInterface || $value instanceof Param) { continue; } @@ -230,14 +231,11 @@ private function replacePlaceholders(string $sql, array $replacements): string } /** - * Creates an instance of {@see SqlParser} for the given SQL statement. + * Creates an instance of {@see AbstractSqlParser} for the given SQL statement. * * @param string $sql SQL statement to be parsed. * - * @return SqlParser SQL parser instance. + * @return AbstractSqlParser SQL parser instance. */ - protected function createSqlParser(string $sql): SqlParser - { - return new SqlParser($sql); - } + abstract protected function createSqlParser(string $sql): AbstractSqlParser; } diff --git a/src/QueryBuilder/AbstractDQLQueryBuilder.php b/src/QueryBuilder/AbstractDQLQueryBuilder.php index 15aec88d7..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; @@ -503,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/SqlParser.php b/src/Syntax/AbstractSqlParser.php similarity index 80% rename from src/Syntax/SqlParser.php rename to src/Syntax/AbstractSqlParser.php index 509be0f4d..ae6d6b250 100644 --- a/src/Syntax/SqlParser.php +++ b/src/Syntax/AbstractSqlParser.php @@ -12,7 +12,7 @@ * * This class provides methods to parse SQL statements and extract placeholders from them. */ -class SqlParser +abstract class AbstractSqlParser { /** * @var int Length of SQL statement. @@ -39,37 +39,7 @@ public function __construct(protected string $sql) * * @return string|null The next placeholder or null if it is not found. */ - public function getNextPlaceholder(int|null &$position = null): string|null - { - $result = null; - $length = $this->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; - } + abstract public function getNextPlaceholder(int|null &$position = null): string|null; /** * Parses and returns word symbols. Equals to `\w+` in regular expressions. diff --git a/tests/AbstractSqlParserTest.php b/tests/AbstractSqlParserTest.php index f1c77101d..af466d309 100644 --- a/tests/AbstractSqlParserTest.php +++ b/tests/AbstractSqlParserTest.php @@ -5,17 +5,14 @@ namespace Yiisoft\Db\Tests; use PHPUnit\Framework\TestCase; -use Yiisoft\Db\Syntax\SqlParser; +use Yiisoft\Db\Syntax\AbstractSqlParser; use Yiisoft\Db\Tests\Support\TestTrait; abstract class AbstractSqlParserTest extends TestCase { use TestTrait; - protected function createSqlParser(string $sql): SqlParser - { - return new SqlParser($sql); - } + abstract protected function createSqlParser(string $sql): AbstractSqlParser; /** @dataProvider \Yiisoft\Db\Tests\Provider\SqlParserProvider::getNextPlaceholder */ public function testGetNextPlaceholder(string $sql, string|null $expectedPlaceholder, int|null $expectedPosition): void @@ -29,7 +26,7 @@ public function testGetNextPlaceholder(string $sql, string|null $expectedPlaceho /** @dataProvider \Yiisoft\Db\Tests\Provider\SqlParserProvider::getAllPlaceholders */ public function testGetAllPlaceholders(string $sql, array $expectedPlaceholders, array $expectedPositions): void { - $parser = new SqlParser($sql); + $parser = $this->createSqlParser($sql); $placeholders = []; $positions = []; diff --git a/tests/Db/Syntax/SqlParserTest.php b/tests/Db/Syntax/SqlParserTest.php deleted file mode 100644 index d6b7284bf..000000000 --- a/tests/Db/Syntax/SqlParserTest.php +++ /dev/null @@ -1,14 +0,0 @@ - new Expression('LOWER(:val || :val_0)', ['val' => 'A', 'val_0' => 'B']), - 'val_0' => 'C', + 'val_0' => new Param('C', DataType::STRING), ], )], '[[name]] != :val || :val_0', [ - 'val_0' => 'F', + 'val_0' => new Param('F', DataType::STRING), 'val' => new Expression('UPPER(:val || :val_0)', ['val' => 'D', 'val_0' => 'E']), ], DbHelper::replaceQuotes( @@ -1301,10 +1303,10 @@ public static function update(): array [ 'val_2' => 'A', 'val_0_1' => 'B', - 'val_0_0' => 'C', + 'val_0_0' => new Param('C', DataType::STRING), 'val_1' => 'D', 'val_0_2' => 'E', - 'val_0' => 'F', + 'val_0' => new Param('F', DataType::STRING), ], ], 'Expressions with indexed params' => [ diff --git a/tests/Support/Stub/DQLQueryBuilder.php b/tests/Support/Stub/DQLQueryBuilder.php index cb3d793fa..00e8350a5 100644 --- a/tests/Support/Stub/DQLQueryBuilder.php +++ b/tests/Support/Stub/DQLQueryBuilder.php @@ -4,8 +4,16 @@ namespace Yiisoft\Db\Tests\Support\Stub; +use Yiisoft\Db\Expression\Expression; use Yiisoft\Db\QueryBuilder\AbstractDQLQueryBuilder; final class DQLQueryBuilder extends AbstractDQLQueryBuilder { + protected function defaultExpressionBuilders(): array + { + return [ + ...parent::defaultExpressionBuilders(), + Expression::class => 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 f8f226afde06b46cb56fee2e59c4383aa6e7e753 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Mon, 1 Apr 2024 20:18:40 +0700 Subject: [PATCH 22/30] Fix array merge for php8.0 --- tests/Support/Stub/DQLQueryBuilder.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/Support/Stub/DQLQueryBuilder.php b/tests/Support/Stub/DQLQueryBuilder.php index 00e8350a5..72924bbbf 100644 --- a/tests/Support/Stub/DQLQueryBuilder.php +++ b/tests/Support/Stub/DQLQueryBuilder.php @@ -11,9 +11,8 @@ final class DQLQueryBuilder extends AbstractDQLQueryBuilder { protected function defaultExpressionBuilders(): array { - return [ - ...parent::defaultExpressionBuilders(), + return array_merge(parent::defaultExpressionBuilders(), [ Expression::class => ExpressionBuilder::class, - ]; + ]); } } From 42c263e87b1c1d9cfc5f73e054587c8623efbb75 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Tue, 2 Apr 2024 17:43:16 +0700 Subject: [PATCH 23/30] Update comments and remove `null` for `$queryBuilder` --- src/Expression/AbstractExpressionBuilder.php | 25 ++++++++++---------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/Expression/AbstractExpressionBuilder.php b/src/Expression/AbstractExpressionBuilder.php index 1436d0984..678216aee 100644 --- a/src/Expression/AbstractExpressionBuilder.php +++ b/src/Expression/AbstractExpressionBuilder.php @@ -28,21 +28,23 @@ */ abstract class AbstractExpressionBuilder implements ExpressionBuilderInterface { - public function __construct(private QueryBuilderInterface|null $queryBuilder = null) + public function __construct(private QueryBuilderInterface $queryBuilder) { } /** - * Builds SQL statement from the given expression. + * Builds an SQL expression from the given expression object. * - * @param Expression $expression The expression to be built. + * This method is called by the query builder to build SQL expressions from {@see ExpressionInterface} objects. + * + * @param Expression $expression The expression to build. * @param array $params The parameters to be bound to the query. * * @psalm-param ParamsType $params * - * @return string SQL statement. + * @return string SQL expression. */ - public function build(Expression $expression, array &$params = []): string + public function build(ExpressionInterface $expression, array &$params = []): string { $sql = $expression->__toString(); $expressionParams = $expression->getParams(); @@ -51,7 +53,7 @@ public function build(Expression $expression, array &$params = []): string return $sql; } - if ($this->queryBuilder === null || isset($expressionParams[0])) { + if (isset($expressionParams[0])) { $params = array_merge($params, $expressionParams); return $sql; } @@ -138,7 +140,6 @@ private function buildParamExpressions(array $expressionParams, array &$params): } $placeholder = $name[0] !== ':' ? ":$name" : $name; - /** @psalm-suppress PossiblyNullReference */ $replacements[$placeholder] = $this->queryBuilder->buildExpression($value, $params); /** @psalm-var ParamsType $params */ @@ -201,12 +202,12 @@ private function getUniqueName(string $name, array $params): string } /** - * Replaces placeholders with replacements in SQL statement. + * Replaces placeholders with replacements in a SQL expression. * - * @param string $sql SQL statement where the placeholder should be replaced. + * @param string $sql SQL expression where the placeholder should be replaced. * @param string[] $replacements Replacements for placeholders. * - * @return string SQL statement with the replaced placeholders. + * @return string SQL expression with replaced placeholders. */ private function replacePlaceholders(string $sql, array $replacements): string { @@ -231,9 +232,9 @@ private function replacePlaceholders(string $sql, array $replacements): string } /** - * Creates an instance of {@see AbstractSqlParser} for the given SQL statement. + * Creates an instance of {@see AbstractSqlParser} for the given SQL expression. * - * @param string $sql SQL statement to be parsed. + * @param string $sql SQL expression to be parsed. * * @return AbstractSqlParser SQL parser instance. */ From 4c22f5827cff2aa984afab558277a1752ce5b271 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 6 Apr 2024 14:04:55 +0700 Subject: [PATCH 24/30] Add lines to CHANGELOG.md and UPGRADE.md [skip ci] --- CHANGELOG.md | 5 +++-- UPGRADE.md | 5 +++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bc579c83..57dcf250c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,9 @@ # Yii Database Change Log -## 1.3.1 under development +## 2.0.0 under development -- no changes in this release. +- Enh #806: Allows non-unique param names inside `Expression::$params`, they will be replaced with unique names (@Tigrov) +- Enh #806: Build `Expression` instances inside `Expression::$params` when build a query using `QueryBuilder` (@Tigrov) ## 1.3.0 March 21, 2024 diff --git a/UPGRADE.md b/UPGRADE.md index 45f0cd279..8bc1465c0 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -10,5 +10,10 @@ All the "Yes, it is" cool stuff, and Yii soul is still in place. Changes summary: +## 1.x to 2.0 +* `Expression::$params` can contain non-unique placeholder names, they will be replaced with unique names. +* `Expression::$params` can contain `Expression` instances, they will be built when building a query using `QueryBuilder`. + +## Yii2 to 1.0 * `Yiisoft\Db\Connection::$charset` has been removed. All supported PDO classes allow you to specify the connection charset in the DSN. From 76bac2c00b7e82b93ec45da167397110d624fb5b Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 6 Apr 2024 14:15:26 +0700 Subject: [PATCH 25/30] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 57dcf250c..99d31c3ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## 2.0.0 under development -- Enh #806: Allows non-unique param names inside `Expression::$params`, they will be replaced with unique names (@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) ## 1.3.0 March 21, 2024 From 2731b3af09a1d01c73eec42c076e9d2f49c187be Mon Sep 17 00:00:00 2001 From: Tigrov Date: Sat, 6 Apr 2024 07:20:58 +0000 Subject: [PATCH 26/30] Apply Rector changes (CI) --- src/Query/Query.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Query/Query.php b/src/Query/Query.php index 8e1e49f06..71e7d01f6 100644 --- a/src/Query/Query.php +++ b/src/Query/Query.php @@ -680,7 +680,6 @@ public function withQueries(array $withQueries): static * * Restores the value of select to make this query reusable. * - * @param ExpressionInterface|string $selectExpression * * @throws Exception * @throws InvalidArgumentException From 3c3f944be14aece766babc2d31e4100e75e430ed Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Sat, 6 Apr 2024 07:21:06 +0000 Subject: [PATCH 27/30] Apply fixes from StyleCI --- src/Query/Query.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Query/Query.php b/src/Query/Query.php index 71e7d01f6..8321d7a96 100644 --- a/src/Query/Query.php +++ b/src/Query/Query.php @@ -680,7 +680,6 @@ public function withQueries(array $withQueries): static * * Restores the value of select to make this query reusable. * - * * @throws Exception * @throws InvalidArgumentException * @throws InvalidConfigException From 6dd47919de32a8bfe5f128e8c17e357490428608 Mon Sep 17 00:00:00 2001 From: Sergei Tigrov Date: Tue, 9 Apr 2024 10:51:29 +0700 Subject: [PATCH 28/30] Apply suggestions from code review [skip ci] Co-authored-by: Alexander Makarov --- UPGRADE.md | 2 ++ src/Expression/AbstractExpressionBuilder.php | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/UPGRADE.md b/UPGRADE.md index 8bc1465c0..464e4d7ac 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -11,9 +11,11 @@ All the "Yes, it is" cool stuff, and Yii soul is still in place. Changes summary: ## 1.x to 2.0 + * `Expression::$params` can contain non-unique placeholder names, they will be replaced with unique names. * `Expression::$params` can contain `Expression` instances, they will be built when building a query using `QueryBuilder`. ## Yii2 to 1.0 + * `Yiisoft\Db\Connection::$charset` has been removed. All supported PDO classes allow you to specify the connection charset in the DSN. diff --git a/src/Expression/AbstractExpressionBuilder.php b/src/Expression/AbstractExpressionBuilder.php index 678216aee..b75775492 100644 --- a/src/Expression/AbstractExpressionBuilder.php +++ b/src/Expression/AbstractExpressionBuilder.php @@ -18,7 +18,7 @@ /** * It's used to build expressions for use in database queries. * - * It provides a methods {@see build()} for creating various types of expressions, such as conditions, joins, and + * It provides a {@see build()} method for creating various types of expressions, such as conditions, joins, and * ordering clauses. * * These expressions can be used with the query builder to build complex and customizable database queries From 5885a6f5e5dff79922c2c8de4d1c622de81bc649 Mon Sep 17 00:00:00 2001 From: Tigrov Date: Tue, 9 Apr 2024 11:39:23 +0700 Subject: [PATCH 29/30] Improve UPGRADE.md --- UPGRADE.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 77d6c08d4..dde268f37 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -21,5 +21,9 @@ in `addColumn()` method of your classes that implement the following interfaces: ### Build `Expression` instances inside `Expression::$params` -- `Expression::$params` can contain non-unique placeholder names, they will be replaced with unique names. -- `Expression::$params` can contain `Expression` instances, they will be built when building a query using `QueryBuilder`. +`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`. From fe50046a4e02f4ac6e3dc7e5cd5ba148fc26b0ba Mon Sep 17 00:00:00 2001 From: Tigrov Date: Tue, 9 Apr 2024 12:43:13 +0700 Subject: [PATCH 30/30] Add test with UTF-8 multibyte symbols --- tests/Db/Syntax/SqlParserTest.php | 16 ++++++++++++++++ tests/Provider/SqlParserProvider.php | 5 +++++ 2 files changed, 21 insertions(+) create mode 100644 tests/Db/Syntax/SqlParserTest.php 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 @@ +