Merge pull request #14742 from cakephp/window-frames

Simplified window frame configuration
cakephp · Jun 26, 2020 · 65c2f9f · 65c2f9f
2 parents 9e5d654 + c4069ec
commit 65c2f9f
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 92 deletions.
diff --git a/src/Database/Expression/AggregateExpression.php b/src/Database/Expression/AggregateExpression.php
@@ -79,11 +79,7 @@ public function order($fields)
     public function range($start, $end = 0)
     {
         $this->over();
-        if (func_num_args() === 1) {
-            $this->window->frame(self::RANGE, $start, self::PRECEDING);
-        } else {
-            $this->window->frame(self::RANGE, $start, self::PRECEDING, $end, self::FOLLOWING);
-        }
+        $this->window->range($start, $end);
 
         return $this;
     }
@@ -94,11 +90,7 @@ public function range($start, $end = 0)
     public function rows(?int $start, ?int $end = 0)
     {
         $this->over();
-        if (func_num_args() === 1) {
-            $this->window->frame(self::ROWS, $start, self::PRECEDING);
-        } else {
-            $this->window->frame(self::ROWS, $start, self::PRECEDING, $end, self::FOLLOWING);
-        }
+        $this->window->rows($start, $end);
 
         return $this;
     }
@@ -109,11 +101,7 @@ public function rows(?int $start, ?int $end = 0)
     public function groups(?int $start, ?int $end = 0)
     {
         $this->over();
-        if (func_num_args() === 1) {
-            $this->window->frame(self::GROUPS, $start, self::PRECEDING);
-        } else {
-            $this->window->frame(self::GROUPS, $start, self::PRECEDING, $end, self::FOLLOWING);
-        }
+        $this->window->groups($start, $end);
 
         return $this;
     }
@@ -125,15 +113,11 @@ public function frame(
         string $type,
         $startOffset,
         string $startDirection,
-        $endOffset = null,
-        string $endDirection = self::FOLLOWING
+        $endOffset,
+        string $endDirection
     ) {
         $this->over();
-        if (func_num_args() === 3) {
-            $this->window->frame($type, $startOffset, $startDirection);
-        } else {
-            $this->window->frame($type, $startOffset, $startDirection, $endOffset, $endDirection);
-        }
+        $this->window->frame($type, $startOffset, $startDirection, $endOffset, $endDirection);
 
         return $this;
     }

diff --git a/src/Database/Expression/WindowExpression.php b/src/Database/Expression/WindowExpression.php
@@ -139,10 +139,6 @@ public function order($fields)
      */
     public function range($start, $end = 0)
     {
-        if (func_num_args() === 1) {
-            return $this->frame(self::RANGE, $start, self::PRECEDING);
-        }
-
         return $this->frame(self::RANGE, $start, self::PRECEDING, $end, self::FOLLOWING);
     }
 
@@ -151,10 +147,6 @@ public function range($start, $end = 0)
      */
     public function rows(?int $start, ?int $end = 0)
     {
-        if (func_num_args() === 1) {
-            return $this->frame(self::ROWS, $start, self::PRECEDING);
-        }
-
         return $this->frame(self::ROWS, $start, self::PRECEDING, $end, self::FOLLOWING);
     }
 
@@ -163,10 +155,6 @@ public function rows(?int $start, ?int $end = 0)
      */
     public function groups(?int $start, ?int $end = 0)
     {
-        if (func_num_args() === 1) {
-            return $this->frame(self::GROUPS, $start, self::PRECEDING);
-        }
-
         return $this->frame(self::GROUPS, $start, self::PRECEDING, $end, self::FOLLOWING);
     }
 
@@ -177,23 +165,20 @@ public function frame(
         string $type,
         $startOffset,
         string $startDirection,
-        $endOffset = null,
-        string $endDirection = self::FOLLOWING
+        $endOffset,
+        string $endDirection
     ) {
         $this->frame = [
             'type' => $type,
             'start' => [
                 'offset' => $startOffset,
                 'direction' => $startDirection,
             ],
-        ];
-
-        if (func_num_args() > 3) {
-            $this->frame['end'] = [
+            'end' => [
                 'offset' => $endOffset,
                 'direction' => $endDirection,
-            ];
-        }
+            ],
+        ];
 
         return $this;
     }
@@ -252,28 +237,18 @@ public function sql(ValueBinder $generator): string
         }
 
         if ($this->frame) {
-            $offset = $this->buildOffsetSql(
+            $start = $this->buildOffsetSql(
                 $generator,
                 $this->frame['start']['offset'],
                 $this->frame['start']['direction']
             );
-
-            $frameSql = sprintf(
-                '%s %s%s',
-                $this->frame['type'],
-                isset($this->frame['end']) ? 'BETWEEN ' : '',
-                $offset
+            $end = $this->buildOffsetSql(
+                $generator,
+                $this->frame['end']['offset'],
+                $this->frame['end']['direction']
             );
 
-            if (isset($this->frame['end'])) {
-                $offset = $this->buildOffsetSql(
-                    $generator,
-                    $this->frame['end']['offset'],
-                    $this->frame['end']['direction']
-                );
-
-                $frameSql .= ' AND ' . $offset;
-            }
+            $frameSql = sprintf('%s BETWEEN %s AND %s', $this->frame['type'], $start, $end);
 
             if ($this->exclusion !== null) {
                 $frameSql .= ' EXCLUDE ' . $this->exclusion;

diff --git a/src/Database/Expression/WindowInterface.php b/src/Database/Expression/WindowInterface.php
@@ -136,8 +136,8 @@ public function frame(
         string $type,
         $startOffset,
         string $startDirection,
-        $endOffset = null,
-        string $endDirection = self::FOLLOWING
+        $endOffset,
+        string $endDirection
     );
 
     /**

diff --git a/tests/TestCase/Database/Expression/AggregateExpressionTest.php b/tests/TestCase/Database/Expression/AggregateExpressionTest.php
@@ -66,7 +66,7 @@ public function testWindowInterface()
 
         $f = (new AggregateExpression('MyFunction'))->range(null);
         $this->assertEqualsSql(
-            'MyFunction() OVER (RANGE UNBOUNDED PRECEDING)',
+            'MyFunction() OVER (RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW)',
             $f->sql(new ValueBinder())
         );
 
@@ -78,7 +78,7 @@ public function testWindowInterface()
 
         $f = (new AggregateExpression('MyFunction'))->rows(null);
         $this->assertEqualsSql(
-            'MyFunction() OVER (ROWS UNBOUNDED PRECEDING)',
+            'MyFunction() OVER (ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW)',
             $f->sql(new ValueBinder())
         );
 
@@ -90,7 +90,7 @@ public function testWindowInterface()
 
         $f = (new AggregateExpression('MyFunction'))->groups(null);
         $this->assertEqualsSql(
-            'MyFunction() OVER (GROUPS UNBOUNDED PRECEDING)',
+            'MyFunction() OVER (GROUPS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW)',
             $f->sql(new ValueBinder())
         );
 
@@ -100,16 +100,6 @@ public function testWindowInterface()
             $f->sql(new ValueBinder())
         );
 
-        $f = (new AggregateExpression('MyFunction'))->frame(
-            AggregateExpression::RANGE,
-            2,
-            AggregateExpression::PRECEDING
-        );
-        $this->assertEqualsSql(
-            'MyFunction() OVER (RANGE 2 PRECEDING)',
-            $f->sql(new ValueBinder())
-        );
-
         $f = (new AggregateExpression('MyFunction'))->frame(
             AggregateExpression::RANGE,
             2,
@@ -130,19 +120,19 @@ public function testWindowInterface()
 
         $f = (new AggregateExpression('MyFunction'))->range(null)->excludeCurrent();
         $this->assertEqualsSql(
-            'MyFunction() OVER (RANGE UNBOUNDED PRECEDING EXCLUDE CURRENT ROW)',
+            'MyFunction() OVER (RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW EXCLUDE CURRENT ROW)',
             $f->sql(new ValueBinder())
         );
 
         $f = (new AggregateExpression('MyFunction'))->range(null)->excludeGroup();
         $this->assertEqualsSql(
-            'MyFunction() OVER (RANGE UNBOUNDED PRECEDING EXCLUDE GROUP)',
+            'MyFunction() OVER (RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW EXCLUDE GROUP)',
             $f->sql(new ValueBinder())
         );
 
         $f = (new AggregateExpression('MyFunction'))->range(null)->excludeTies();
         $this->assertEqualsSql(
-            'MyFunction() OVER (RANGE UNBOUNDED PRECEDING EXCLUDE TIES)',
+            'MyFunction() OVER (RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW EXCLUDE TIES)',
             $f->sql(new ValueBinder())
         );
     }

diff --git a/tests/TestCase/Database/Expression/WindowExpressionTest.php b/tests/TestCase/Database/Expression/WindowExpressionTest.php
@@ -124,19 +124,19 @@ public function testRange()
     {
         $w = (new WindowExpression())->range(null);
         $this->assertEqualsSql(
-            'RANGE UNBOUNDED PRECEDING',
+            'RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW',
             $w->sql(new ValueBinder())
         );
 
         $w = (new WindowExpression())->range(0);
         $this->assertEqualsSql(
-            'RANGE CURRENT ROW',
+            'RANGE BETWEEN CURRENT ROW AND CURRENT ROW',
             $w->sql(new ValueBinder())
         );
 
         $w = (new WindowExpression())->range(2);
         $this->assertEqualsSql(
-            'RANGE 2 PRECEDING',
+            'RANGE BETWEEN 2 PRECEDING AND CURRENT ROW',
             $w->sql(new ValueBinder())
         );
 
@@ -198,19 +198,19 @@ public function testRows()
     {
         $w = (new WindowExpression())->rows(null);
         $this->assertEqualsSql(
-            'ROWS UNBOUNDED PRECEDING',
+            'ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW',
             $w->sql(new ValueBinder())
         );
 
         $w = (new WindowExpression())->rows(0);
         $this->assertEqualsSql(
-            'ROWS CURRENT ROW',
+            'ROWS BETWEEN CURRENT ROW AND CURRENT ROW',
             $w->sql(new ValueBinder())
         );
 
         $w = (new WindowExpression())->rows(2);
         $this->assertEqualsSql(
-            'ROWS 2 PRECEDING',
+            'ROWS BETWEEN 2 PRECEDING AND CURRENT ROW',
             $w->sql(new ValueBinder())
         );
 
@@ -261,19 +261,19 @@ public function testGroups()
     {
         $w = (new WindowExpression())->groups(null);
         $this->assertEqualsSql(
-            'GROUPS UNBOUNDED PRECEDING',
+            'GROUPS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW',
             $w->sql(new ValueBinder())
         );
 
         $w = (new WindowExpression())->groups(0);
         $this->assertEqualsSql(
-            'GROUPS CURRENT ROW',
+            'GROUPS BETWEEN CURRENT ROW AND CURRENT ROW',
             $w->sql(new ValueBinder())
         );
 
         $w = (new WindowExpression())->groups(2);
         $this->assertEqualsSql(
-            'GROUPS 2 PRECEDING',
+            'GROUPS BETWEEN 2 PRECEDING AND CURRENT ROW',
             $w->sql(new ValueBinder())
         );
 
@@ -330,19 +330,19 @@ public function testExclusion()
 
         $w = (new WindowExpression())->range(null)->excludeCurrent();
         $this->assertEqualsSql(
-            'RANGE UNBOUNDED PRECEDING EXCLUDE CURRENT ROW',
+            'RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW EXCLUDE CURRENT ROW',
             $w->sql(new ValueBinder())
         );
 
         $w = (new WindowExpression())->range(null)->excludeGroup();
         $this->assertEqualsSql(
-            'RANGE UNBOUNDED PRECEDING EXCLUDE GROUP',
+            'RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW EXCLUDE GROUP',
             $w->sql(new ValueBinder())
         );
 
         $w = (new WindowExpression())->range(null)->excludeTies();
         $this->assertEqualsSql(
-            'RANGE UNBOUNDED PRECEDING EXCLUDE TIES',
+            'RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW EXCLUDE TIES',
             $w->sql(new ValueBinder())
         );
     }
@@ -356,25 +356,25 @@ public function testCombined()
     {
         $w = (new WindowExpression())->partition('test')->range(null);
         $this->assertEqualsSql(
-            'PARTITION BY test RANGE UNBOUNDED PRECEDING',
+            'PARTITION BY test RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW',
             $w->sql(new ValueBinder())
         );
 
         $w = (new WindowExpression())->order('test')->range(null);
         $this->assertEqualsSql(
-            'ORDER BY test RANGE UNBOUNDED PRECEDING',
+            'ORDER BY test RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW',
             $w->sql(new ValueBinder())
         );
 
         $w = (new WindowExpression())->partition('test')->order('test')->range(null);
         $this->assertEqualsSql(
-            'PARTITION BY test ORDER BY test RANGE UNBOUNDED PRECEDING',
+            'PARTITION BY test ORDER BY test RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW',
             $w->sql(new ValueBinder())
         );
 
         $w = (new WindowExpression())->partition('test')->order('test')->range(null)->excludeCurrent();
         $this->assertEqualsSql(
-            'PARTITION BY test ORDER BY test RANGE UNBOUNDED PRECEDING EXCLUDE CURRENT ROW',
+            'PARTITION BY test ORDER BY test RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW EXCLUDE CURRENT ROW',
             $w->sql(new ValueBinder())
         );
     }