From 264d58958e43f4f366e269208c71f177160323ea Mon Sep 17 00:00:00 2001 From: Sergei Tigrov Date: Tue, 26 Sep 2023 13:01:15 +0700 Subject: [PATCH 1/5] Refactor `Quoter` (#756) Co-authored-by: Sergei Predvoditelev --- CHANGELOG.md | 4 ++++ src/Schema/Quoter.php | 33 ++++++++++++++++++++++----------- tests/Db/Schema/QuoterTest.php | 23 +++++++++++++++++++++++ 3 files changed, 49 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3af0daf3b..137af005f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ ## 1.1.2 under development - Bug #751: Fix collected debug actions (@xepozz) +- Enh #756: Refactor `Quoter` (@Tigrov) +- Bug #756: Fix `quoteSql()` for SQL containing table with prefix (@Tigrov) +- Bug #756: Fix `getTableNameParts()` for cases when different quotes for tables and columns (@Tigrov) +- Bug #756: Fix `quoteTableName()` for sub-query with alias (@Tigrov) ## 1.1.1 August 16, 2023 diff --git a/src/Schema/Quoter.php b/src/Schema/Quoter.php index 10a559222..1f8aef970 100644 --- a/src/Schema/Quoter.php +++ b/src/Schema/Quoter.php @@ -8,9 +8,13 @@ use Yiisoft\Db\Expression\ExpressionInterface; use function addcslashes; +use function array_slice; +use function count; use function explode; use function implode; use function is_string; +use function preg_match; +use function preg_replace; use function preg_replace_callback; use function str_contains; use function str_replace; @@ -44,7 +48,7 @@ public function cleanUpTableNames(array $tableNames): array { $cleanedUpTableNames = []; $pattern = << $tableNames */ @@ -104,7 +108,7 @@ public function ensureColumnName(string $name): string $name = $parts[count($parts) - 1]; } - return preg_replace('|^\[\[([_\w\-. ]+)\]\]$|', '\1', $name); + return preg_replace('|^\[\[([\w\-. ]+)]]$|', '\1', $name); } public function quoteColumnName(string $name): string @@ -135,8 +139,9 @@ public function quoteSimpleColumnName(string $name): string [$startingCharacter, $endingCharacter] = $this->columnQuoteCharacter; } - return $name === '*' || str_contains($name, $startingCharacter) ? $name : $startingCharacter . $name - . $endingCharacter; + return $name === '*' || str_starts_with($name, $startingCharacter) + ? $name + : $startingCharacter . $name . $endingCharacter; } public function quoteSimpleTableName(string $name): string @@ -147,13 +152,15 @@ public function quoteSimpleTableName(string $name): string [$startingCharacter, $endingCharacter] = $this->tableQuoteCharacter; } - return str_contains($name, $startingCharacter) ? $name : $startingCharacter . $name . $endingCharacter; + return str_starts_with($name, $startingCharacter) + ? $name + : $startingCharacter . $name . $endingCharacter; } public function quoteSql(string $sql): string { return preg_replace_callback( - '/({{(%?[\w\-. ]+%?)}}|\\[\\[([\w\-. ]+)]])/', + '/({{(%?[\w\-. ]+)%?}}|\\[\\[([\w\-. ]+)]])/', function ($matches) { if (isset($matches[3])) { return $this->quoteColumnName($matches[3]); @@ -167,7 +174,7 @@ function ($matches) { public function quoteTableName(string $name): string { - if (str_starts_with($name, '(') && str_ends_with($name, ')')) { + if (str_starts_with($name, '(')) { return $name; } @@ -194,7 +201,7 @@ public function quoteValue(mixed $value): mixed return $value; } - return '\'' . str_replace('\'', '\'\'', addcslashes($value, "\000\032")) . '\''; + return "'" . str_replace("'", "''", addcslashes($value, "\000\032")) . "'"; } public function unquoteSimpleColumnName(string $name): string @@ -205,7 +212,9 @@ public function unquoteSimpleColumnName(string $name): string $startingCharacter = $this->columnQuoteCharacter[0]; } - return !str_contains($name, $startingCharacter) ? $name : substr($name, 1, -1); + return !str_starts_with($name, $startingCharacter) + ? $name + : substr($name, 1, -1); } public function unquoteSimpleTableName(string $name): string @@ -216,7 +225,9 @@ public function unquoteSimpleTableName(string $name): string $startingCharacter = $this->tableQuoteCharacter[0]; } - return !str_contains($name, $startingCharacter) ? $name : substr($name, 1, -1); + return !str_starts_with($name, $startingCharacter) + ? $name + : substr($name, 1, -1); } /** @@ -229,7 +240,7 @@ protected function unquoteParts(array $parts, bool $withColumn): array $lastKey = count($parts) - 1; foreach ($parts as $k => &$part) { - $part = ($withColumn || $lastKey === $k) ? + $part = ($withColumn && $lastKey === $k) ? $this->unquoteSimpleColumnName($part) : $this->unquoteSimpleTableName($part); } diff --git a/tests/Db/Schema/QuoterTest.php b/tests/Db/Schema/QuoterTest.php index 0841dca80..3d7bd8599 100644 --- a/tests/Db/Schema/QuoterTest.php +++ b/tests/Db/Schema/QuoterTest.php @@ -87,4 +87,27 @@ public function testCleanUpTableNamesWithCastException(): void ['tableAlias' => 123], ); } + + public function testGetTableNamePartsWithDifferentQuotes(): void + { + $quoter = new Quoter('`', '"'); + + $this->assertSame(['schema', 'table'], $quoter->getTableNameParts('"schema"."table"')); + } + + public function testQuoteSqlWithTablePrefix(): void + { + $quoter = new Quoter('`', '`', 'prefix_'); + $sql = 'SELECT * FROM {{%table%}}'; + + $this->assertSame('SELECT * FROM `prefix_table`', $quoter->quoteSql($sql)); + } + + public function testQuoteTableNameWithQueryAlias() + { + $quoter = new Quoter('`', '`'); + $name = '(SELECT * FROM table) alias'; + + $this->assertSame($name, $quoter->quoteTableName($name)); + } } From a60a912638a0f691275a290345c1b7ee251e445d Mon Sep 17 00:00:00 2001 From: Sergei Tigrov Date: Tue, 26 Sep 2023 13:18:37 +0700 Subject: [PATCH 2/5] Deprecate compositeForeignKey() (#755) Co-authored-by: Sergei Predvoditelev --- CHANGELOG.md | 7 ++++--- src/Schema/TableSchemaInterface.php | 2 ++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 137af005f..bb84eb0d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,10 +3,11 @@ ## 1.1.2 under development - Bug #751: Fix collected debug actions (@xepozz) +- Chg #755: Deprecate `TableSchemaInterface::compositeForeignKey()` (@Tigrov) - Enh #756: Refactor `Quoter` (@Tigrov) -- Bug #756: Fix `quoteSql()` for SQL containing table with prefix (@Tigrov) -- Bug #756: Fix `getTableNameParts()` for cases when different quotes for tables and columns (@Tigrov) -- Bug #756: Fix `quoteTableName()` for sub-query with alias (@Tigrov) +- Bug #756: Fix `Quoter::quoteSql()` for SQL containing table with prefix (@Tigrov) +- Bug #756: Fix `Quoter::getTableNameParts()` for cases when different quotes for tables and columns (@Tigrov) +- Bug #756: Fix `Quoter::quoteTableName()` for sub-query with alias (@Tigrov) ## 1.1.1 August 16, 2023 diff --git a/src/Schema/TableSchemaInterface.php b/src/Schema/TableSchemaInterface.php index 1dd5d72d3..47fe0c107 100644 --- a/src/Schema/TableSchemaInterface.php +++ b/src/Schema/TableSchemaInterface.php @@ -210,6 +210,8 @@ public function foreignKey(string|int $id, array $to): void; * @param string $to The column name in foreign table. * * @throws NotSupportedException + * + * @deprecated will be removed in version 2.0.0 */ public function compositeForeignKey(int $id, string $from, string $to): void; } From 9649be9ba2b3d53204a053fef44448d780a5deac Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 13 Oct 2023 15:23:28 +0300 Subject: [PATCH 3/5] Update yiisoft/cache-file requirement from ^2.0 to ^3.1 (#764) Updates the requirements on [yiisoft/cache-file](https://github.com/yiisoft/cache-file) to permit the latest version. - [Release notes](https://github.com/yiisoft/cache-file/releases) - [Changelog](https://github.com/yiisoft/cache-file/blob/master/CHANGELOG.md) - [Commits](https://github.com/yiisoft/cache-file/compare/2.0.0...3.1.0) --- updated-dependencies: - dependency-name: yiisoft/cache-file dependency-type: direct:development ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 7ce739282..37e3f53c1 100644 --- a/composer.json +++ b/composer.json @@ -36,7 +36,7 @@ "spatie/phpunit-watcher": "^1.23", "vimeo/psalm": "^4.30|^5.12", "yiisoft/aliases": "^3.0", - "yiisoft/cache-file": "^2.0", + "yiisoft/cache-file": "^3.1", "yiisoft/di": "^1.0", "yiisoft/event-dispatcher": "^1.0", "yiisoft/json": "^1.0", From 071f46ab65d8b81af621075e86669d1dd64c9d52 Mon Sep 17 00:00:00 2001 From: Sergei Tigrov Date: Sat, 14 Oct 2023 14:39:47 +0700 Subject: [PATCH 4/5] Deprecate abstract type `SchemaInterface::TYPE_JSONB` (#765) * Deprecate `SchemaInterface::TYPE_JSONB` * Add line to CHANGELOG.md --- CHANGELOG.md | 1 + src/Schema/SchemaInterface.php | 2 ++ 2 files changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bb84eb0d1..8cb849a49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Bug #756: Fix `Quoter::quoteSql()` for SQL containing table with prefix (@Tigrov) - Bug #756: Fix `Quoter::getTableNameParts()` for cases when different quotes for tables and columns (@Tigrov) - Bug #756: Fix `Quoter::quoteTableName()` for sub-query with alias (@Tigrov) +- Chg #765: Deprecate `SchemaInterface::TYPE_JSONB` (@Tigrov) ## 1.1.1 August 16, 2023 diff --git a/src/Schema/SchemaInterface.php b/src/Schema/SchemaInterface.php index 684a11f6e..5bfa909a0 100644 --- a/src/Schema/SchemaInterface.php +++ b/src/Schema/SchemaInterface.php @@ -220,6 +220,8 @@ interface SchemaInterface extends ConstraintSchemaInterface public const TYPE_JSON = 'json'; /** * Define the abstract column type as `jsonb`. + * + * @deprecated will be removed in version 2.0.0. Use `SchemaInterface::TYPE_JSON` instead. */ public const TYPE_JSONB = 'jsonb'; From 4f1dbb95c9551b4359cdb2b188d0094bd70aa138 Mon Sep 17 00:00:00 2001 From: Sergei Tigrov Date: Sat, 14 Oct 2023 14:53:59 +0700 Subject: [PATCH 5/5] Fix CTE query expressions (#761) * Fix cte * Fix tests * Add `ExpressionInterface` as alias type and update tests * Update psalm * Update tests * Add line to CHANGELOG.md --- CHANGELOG.md | 1 + src/Query/Query.php | 2 +- src/Query/QueryPartsInterface.php | 5 ++- src/QueryBuilder/AbstractDQLQueryBuilder.php | 34 +++++++++++++- tests/AbstractQueryBuilderTest.php | 45 ++++++++++++++----- tests/Common/CommonQueryTest.php | 47 ++++++++++++++++++++ tests/Provider/QueryBuilderProvider.php | 11 +++++ 7 files changed, 130 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cb849a49..63bf93448 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Bug #756: Fix `Quoter::quoteSql()` for SQL containing table with prefix (@Tigrov) - Bug #756: Fix `Quoter::getTableNameParts()` for cases when different quotes for tables and columns (@Tigrov) - Bug #756: Fix `Quoter::quoteTableName()` for sub-query with alias (@Tigrov) +- Bug #761: Quote aliases of CTE in `WITH` queries (@Tigrov) - Chg #765: Deprecate `SchemaInterface::TYPE_JSONB` (@Tigrov) ## 1.1.1 August 16, 2023 diff --git a/src/Query/Query.php b/src/Query/Query.php index 98e69b962..4ba681e7b 100644 --- a/src/Query/Query.php +++ b/src/Query/Query.php @@ -662,7 +662,7 @@ public function where(array|string|ExpressionInterface|null $condition, array $p return $this; } - public function withQuery(QueryInterface|string $query, string $alias, bool $recursive = false): static + public function withQuery(QueryInterface|string $query, ExpressionInterface|string $alias, bool $recursive = false): static { $this->withQueries[] = ['query' => $query, 'alias' => $alias, 'recursive' => $recursive]; return $this; diff --git a/src/Query/QueryPartsInterface.php b/src/Query/QueryPartsInterface.php index f6b9ca974..8c3ddfd84 100644 --- a/src/Query/QueryPartsInterface.php +++ b/src/Query/QueryPartsInterface.php @@ -645,10 +645,11 @@ public function where(array|string|ExpressionInterface|null $condition, array $p * Prepends an SQL statement using `WITH` syntax. * * @param QueryInterface|string $query The SQL statement to append using `UNION`. - * @param string $alias The query alias in `WITH` construction. + * @param ExpressionInterface|string $alias The query alias in `WITH` construction. + * To specify the alias in plain SQL, you may pass an instance of {@see ExpressionInterface}. * @param bool $recursive Its `true` if using `WITH RECURSIVE` and `false` if using `WITH`. */ - public function withQuery(QueryInterface|string $query, string $alias, bool $recursive = false): static; + public function withQuery(QueryInterface|string $query, ExpressionInterface|string $alias, bool $recursive = false): static; /** * Specifies the `WITH` query clause for the query. diff --git a/src/QueryBuilder/AbstractDQLQueryBuilder.php b/src/QueryBuilder/AbstractDQLQueryBuilder.php index 77418e5af..9f857328f 100644 --- a/src/QueryBuilder/AbstractDQLQueryBuilder.php +++ b/src/QueryBuilder/AbstractDQLQueryBuilder.php @@ -411,7 +411,7 @@ public function buildWithQueries(array $withs, array &$params): string $recursive = false; $result = []; - /** @psalm-var array $withs */ + /** @psalm-var array{query:string|Query, alias:ExpressionInterface|string, recursive:bool}[] $withs */ foreach ($withs as $with) { if ($with['recursive']) { $recursive = true; @@ -421,7 +421,9 @@ public function buildWithQueries(array $withs, array &$params): string [$with['query'], $params] = $this->build($with['query'], $params); } - $result[] = $with['alias'] . ' AS (' . $with['query'] . ')'; + $quotedAlias = $this->quoteCteAlias($with['alias']); + + $result[] = $quotedAlias . ' AS (' . $with['query'] . ')'; } return 'WITH ' . ($recursive ? 'RECURSIVE ' : '') . implode(', ', $result); @@ -610,4 +612,32 @@ private function quoteTableNames(array $tables, array &$params): array return $tables; } + + /** + * Quotes an alias of Common Table Expressions (CTE) + * + * @param ExpressionInterface|string $name The alias name with or without column names to quote. + * + * @return string The quoted alias. + */ + private function quoteCteAlias(ExpressionInterface|string $name): string + { + if ($name instanceof ExpressionInterface) { + return $this->buildExpression($name); + } + + if (!str_contains($name, '(')) { + return $this->quoter->quoteTableName($name); + } + + if (!str_ends_with($name, ')')) { + return $name; + } + + /** @psalm-suppress PossiblyUndefinedArrayOffset */ + [$name, $columns] = explode('(', substr($name, 0, -1), 2); + $name = trim($name); + + return $this->quoter->quoteTableName($name) . '(' . $this->buildColumns($columns) . ')'; + } } diff --git a/tests/AbstractQueryBuilderTest.php b/tests/AbstractQueryBuilderTest.php index 6a50636e9..31de64c71 100644 --- a/tests/AbstractQueryBuilderTest.php +++ b/tests/AbstractQueryBuilderTest.php @@ -668,7 +668,7 @@ public function testBuildWithQueries(): void $this->assertSame( DbHelper::replaceQuotes( <<getDriverName(), ), @@ -1131,7 +1131,7 @@ public function testBuildWithQuery(): void $this->assertSame( DbHelper::replaceQuotes( <<getDriverName(), ), @@ -1157,15 +1157,40 @@ public function testBuildWithQueryRecursive(): void [$sql, $params] = $qb->build($query); - $this->assertSame( - DbHelper::replaceQuotes( - <<getDriverName(), - ), - $sql, + $expected = DbHelper::replaceQuotes( + <<getDriverName(), ); + + if (in_array($db->getDriverName(), ['oci', 'sqlsrv'], true)) { + $expected = str_replace('WITH RECURSIVE ', 'WITH ', $expected); + } + + $this->assertSame($expected, $sql); + $this->assertSame([], $params); + } + + /** @dataProvider \Yiisoft\Db\Tests\Provider\QueryBuilderProvider::cteAliases */ + public function testBuildWithQueryAlias($alias, $expected) + { + $db = $this->getConnection(); + $qb = $db->getQueryBuilder(); + + $withQuery = (new Query($db))->from('t'); + $query = (new Query($db))->withQuery($withQuery, $alias)->from('t'); + + [$sql, $params] = $qb->build($query); + + $expectedSql = DbHelper::replaceQuotes( + <<getDriverName(), + ); + + $this->assertSame($expectedSql, $sql); $this->assertSame([], $params); } diff --git a/tests/Common/CommonQueryTest.php b/tests/Common/CommonQueryTest.php index 7b098207f..a1a6bb1d0 100644 --- a/tests/Common/CommonQueryTest.php +++ b/tests/Common/CommonQueryTest.php @@ -4,6 +4,7 @@ namespace Yiisoft\Db\Tests\Common; +use Yiisoft\Db\Expression\Expression; use Yiisoft\Db\Query\Query; use Yiisoft\Db\Tests\AbstractQueryTest; @@ -34,4 +35,50 @@ public function testColumnIndexByWithClosure() $db->close(); } + + public function testWithQuery() + { + $db = $this->getConnection(true); + + $with = (new Query($db)) + ->distinct() + ->select(['status']) + ->from('customer'); + + $query = (new Query($db)) + ->withQuery($with, 'statuses') + ->from('statuses'); + + $this->assertEquals(2, $query->count()); + + $db->close(); + } + + public function testWithQueryRecursive() + { + $db = $this->getConnection(); + $quoter = $db->getQuoter(); + $isOracle = $db->getDriverName() === 'oci'; + + /** Sum 1 to 10 equals 55 */ + $quotedName = $quoter->quoteColumnName('n'); + $union = (new Query($db)) + ->select(new Expression($quotedName . ' + 1')) + ->from('t') + ->where(['<', 'n', 10]); + + $with = (new Query($db)) + ->select(new Expression('1')) + ->from($isOracle ? new Expression('DUAL') : []) + ->union($union, true); + + $sum = (new Query($db)) + ->withQuery($with, 't(n)', true) + ->from('t') + ->sum($quotedName); + + $this->assertEquals(55, $sum); + + $db->close(); + } } diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php index f86d147d9..0871a113b 100644 --- a/tests/Provider/QueryBuilderProvider.php +++ b/tests/Provider/QueryBuilderProvider.php @@ -1231,4 +1231,15 @@ public static function upsert(): array ], ]; } + + public static function cteAliases(): array + { + return [ + 'simple' => ['a', '[[a]]'], + 'with one column' => ['a(b)', '[[a]]([[b]])'], + 'with columns' => ['a(b,c,d)', '[[a]]([[b]], [[c]], [[d]])'], + 'with extra space' => ['a(b,c,d) ', 'a(b,c,d) '], + 'expression' => [new Expression('a(b,c,d)'), 'a(b,c,d)'], + ]; + } }