Merge branch 'master' into refactor_dml_query_builder

yiisoft · Oct 27, 2023 · b4ffbb8 · b4ffbb8
2 parents b82cb14 + 4f1dbb9
commit b4ffbb8
Show file tree

Hide file tree

Showing 12 changed files with 186 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,13 @@
 
 - Bug #746: Refactor `DMLQueryBuilder` and fix: unique indexes in `upsert()`, column names with table name and brackets, `batchInsert()` with associative arrays, enhanced documentation of `batchInsert()` and `update()` (@Tigrov)
 - Bug #751: Fix collected debug actions (@xepozz)
+- Chg #755: Deprecate `TableSchemaInterface::compositeForeignKey()` (@Tigrov) 
+- Enh #756: Refactor `Quoter` (@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)
+- 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/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",

diff --git 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
@@ -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
@@ -411,7 +411,7 @@ public function buildWithQueries(array $withs, array &$params): string
         $recursive = false;
         $result = [];
 
-        /** @psalm-var array<array-key, array{query:string|Query, alias:string, recursive:bool}> $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/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 = <<<PATTERN
-        ~^\s*((?:['"`\[]|{{).*?(?:['"`\]]|}})|\(.*?\)|.*?)(?:(?:\s+(?:as\s+)?)((?:['"`\[]|{{).*?(?:['"`\]]|}})|.*?))?\s*$~iux
+        ~^\s*((?:['"`\[]|{{).*?(?:['"`\]]|}})|\(.*?\)|.*?)(?:\s+(?:as\s+)?((?:['"`\[]|{{).*?(?:['"`\]]|}})|.*?))?\s*$~iux
         PATTERN;
 
         /** @psalm-var array<array-key, ExpressionInterface|string> $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/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';
 

diff --git 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;
 }
diff --git a/tests/AbstractQueryBuilderTest.php b/tests/AbstractQueryBuilderTest.php
@@ -668,7 +668,7 @@ public function testBuildWithQueries(): void
         $this->assertSame(
             DbHelper::replaceQuotes(
                 <<<SQL
-                WITH cte AS (SELECT * FROM [[admin_profile]])
+                WITH [[cte]] AS (SELECT * FROM [[admin_profile]])
                 SQL,
                 $db->getDriverName(),
             ),
@@ -1131,7 +1131,7 @@ public function testBuildWithQuery(): void
         $this->assertSame(
             DbHelper::replaceQuotes(
                 <<<SQL
-                WITH a1 AS (SELECT [[id]] FROM [[t1]] WHERE expr = 1), a2 AS ((SELECT [[id]] FROM [[t2]] INNER JOIN [[a1]] ON t2.id = a1.id WHERE expr = 2) UNION ( SELECT [[id]] FROM [[t3]] WHERE expr = 3 )) SELECT * FROM [[a2]]
+                WITH [[a1]] AS (SELECT [[id]] FROM [[t1]] WHERE expr = 1), [[a2]] AS ((SELECT [[id]] FROM [[t2]] INNER JOIN [[a1]] ON t2.id = a1.id WHERE expr = 2) UNION ( SELECT [[id]] FROM [[t3]] WHERE expr = 3 )) SELECT * FROM [[a2]]
                 SQL,
                 $db->getDriverName(),
             ),
@@ -1157,15 +1157,40 @@ public function testBuildWithQueryRecursive(): void
 
         [$sql, $params] = $qb->build($query);
 
-        $this->assertSame(
-            DbHelper::replaceQuotes(
-                <<<SQL
-                WITH RECURSIVE a1 AS (SELECT [[id]] FROM [[t1]] WHERE expr = 1) SELECT * FROM [[a1]]
-                SQL,
-                $db->getDriverName(),
-            ),
-            $sql,
+        $expected = DbHelper::replaceQuotes(
+            <<<SQL
+            WITH RECURSIVE [[a1]] AS (SELECT [[id]] FROM [[t1]] WHERE expr = 1) SELECT * FROM [[a1]]
+            SQL,
+            $db->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(
+            <<<SQL
+            WITH $expected AS (SELECT * FROM [[t]]) SELECT * FROM [[t]]
+            SQL,
+            $db->getDriverName(),
+        );
+
+        $this->assertSame($expectedSql, $sql);
         $this->assertSame([], $params);
     }
 

diff --git 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/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));
+    }
 }
diff --git a/tests/Provider/QueryBuilderProvider.php b/tests/Provider/QueryBuilderProvider.php
@@ -1250,4 +1250,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)'],
+        ];
+    }
 }