Browse Source

Fixed window name identifier quoting. Fixed cloning window and cte names.

Corey Taylor 5 years ago
parent
commit
7982e8bf35

+ 1 - 0
src/Database/Expression/CommonTableExpression.php

@@ -227,6 +227,7 @@ class CommonTableExpression implements ExpressionInterface
      */
     public function __clone()
     {
+        $this->name = clone $this->name;
         if ($this->query) {
             $this->query = clone $this->query;
         }

+ 10 - 8
src/Database/Expression/WindowExpression.php

@@ -27,7 +27,7 @@ use InvalidArgumentException;
 class WindowExpression implements ExpressionInterface, WindowInterface
 {
     /**
-     * @var string|null
+     * @var \Cake\Database\Expression\IdentifierExpression
      */
     protected $name;
 
@@ -52,11 +52,11 @@ class WindowExpression implements ExpressionInterface, WindowInterface
     protected $exclusion;
 
     /**
-     * @param string|null $name Window name
+     * @param string $name Window name
      */
-    public function __construct(?string $name = null)
+    public function __construct(string $name = '')
     {
-        $this->name = $name;
+        $this->name = new IdentifierExpression($name);
     }
 
     /**
@@ -69,7 +69,7 @@ class WindowExpression implements ExpressionInterface, WindowInterface
      */
     public function isNamedOnly(): bool
     {
-        return $this->name && (!$this->partitions && !$this->frame && !$this->order);
+        return $this->name->getIdentifier() && (!$this->partitions && !$this->frame && !$this->order);
     }
 
     /**
@@ -80,7 +80,7 @@ class WindowExpression implements ExpressionInterface, WindowInterface
      */
     public function name(string $name)
     {
-        $this->name = $name;
+        $this->name = new IdentifierExpression($name);
 
         return $this;
     }
@@ -239,8 +239,8 @@ class WindowExpression implements ExpressionInterface, WindowInterface
     public function sql(ValueBinder $generator): string
     {
         $clauses = [];
-        if ($this->name) {
-            $clauses[] = $this->name;
+        if ($this->name->getIdentifier()) {
+            $clauses[] = $this->name->sql($generator);
         }
 
         if ($this->partitions) {
@@ -295,6 +295,7 @@ class WindowExpression implements ExpressionInterface, WindowInterface
      */
     public function traverse(Closure $visitor)
     {
+        $visitor($this->name);
         foreach ($this->partitions as $partition) {
             $visitor($partition);
             $partition->traverse($visitor);
@@ -338,6 +339,7 @@ class WindowExpression implements ExpressionInterface, WindowInterface
      */
     public function __clone()
     {
+        $this->name = clone $this->name;
         foreach ($this->partitions as $i => $partition) {
             $this->partitions[$i] = clone $partition;
         }

+ 1 - 1
src/Database/Query.php

@@ -1428,7 +1428,7 @@ class Query implements ExpressionInterface, IteratorAggregate
             }
         }
 
-        $this->_parts['window'][] = ['name' => $name, 'window' => $window];
+        $this->_parts['window'][] = ['name' => new IdentifierExpression($name), 'window' => $window];
         $this->_dirty();
 
         return $this;

+ 1 - 1
src/Database/QueryCompiler.php

@@ -306,7 +306,7 @@ class QueryCompiler
     {
         $windows = [];
         foreach ($parts as $window) {
-            $windows[] = $window['name'] . ' AS (' . $window['window']->sql($generator) . ')';
+            $windows[] = $window['name']->sql($generator) . ' AS (' . $window['window']->sql($generator) . ')';
         }
 
         return ' WINDOW ' . implode(', ', $windows);

+ 3 - 0
tests/TestCase/Database/Expression/CommonTableExpressionTest.php

@@ -143,6 +143,9 @@ class CommonTableExpressionTest extends TestCase
         $cte = new CommonTableExpression('test', function () {
             return $this->connection->newQuery()->select('1');
         });
+        $cte2 = (clone $cte)->name('test2');
+        $this->assertNotSame($cte->sql(new ValueBinder()), $cte2->sql(new ValueBinder()));
+
         $cte2 = (clone $cte)->field('col1');
         $this->assertNotSame($cte->sql(new ValueBinder()), $cte2->sql(new ValueBinder()));
     }

+ 4 - 0
tests/TestCase/Database/Expression/WindowExpressionTest.php

@@ -427,6 +427,10 @@ class WindowExpressionTest extends TestCase
      */
     public function testCloning()
     {
+        $w1 = (new WindowExpression())->name('test');
+        $w2 = (clone $w1)->name('test2');
+        $this->assertNotSame($w1->sql(new ValueBinder()), $w2->sql(new ValueBinder()));
+
         $w1 = (new WindowExpression())->partition('test');
         $w2 = (clone $w1)->partition('new');
         $this->assertNotSame($w1->sql(new ValueBinder()), $w2->sql(new ValueBinder()));

+ 11 - 2
tests/TestCase/Database/QueryTests/WindowQueryTests.php

@@ -38,12 +38,21 @@ class WindowQueryTests extends TestCase
      */
     protected $connection = null;
 
+    /**
+     * @var bool
+     */
+    protected $autoQuote;
+
+    /**
+     * @var bool
+     */
     protected $skipTests = false;
 
     public function setUp(): void
     {
         parent::setUp();
         $this->connection = ConnectionManager::get('test');
+        $this->autoQuote = $this->connection->getDriver()->isAutoQuotingEnabled();
 
         $driver = $this->connection->getDriver();
         if (
@@ -73,12 +82,12 @@ class WindowQueryTests extends TestCase
             ->select('*')
             ->window('name', new WindowExpression())
             ->sql();
-        $this->assertEqualsSql('SELECT * WINDOW name AS ()', $sql);
+        $this->assertRegExpSql('SELECT \* WINDOW <name> AS \(\)', $sql, !$this->autoQuote);
 
         $sql = $query
             ->window('name2', new WindowExpression('name'))
             ->sql();
-        $this->assertEqualsSql('SELECT * WINDOW name AS (), name2 AS (name)', $sql);
+        $this->assertRegExpSql('SELECT \* WINDOW <name> AS \(\), <name2> AS \(<name>\)', $sql, !$this->autoQuote);
 
         $sql = $query
             ->window('name', function ($window, $query) {