Allow for multiple FOLLOWING/PRECEDING in a window range

Altered the range specification for window functions to allow
for two of the same PRECEDING or FOLLOWING keywords in a range
by allowing for the left side of the range to be positive
and for the right to be negative, e.g. (1, 3) is
"1 FOLLOWING AND 3 FOLLOWING".

Change-Id: I7d3a6c641151bb49219104968d18dac2266f3db8
Fixes: #4053

diff --git a/doc/build/changelog/unreleased_11/4053.rst b/doc/build/changelog/unreleased_11/4053.rst
new file mode 100644 (file)
index 0000000..718b58f
--- /dev/null
+++ b/doc/build/changelog/unreleased_11/4053.rst
@@ -0,0 +1,9 @@
+.. change::
+    :tags: bug, sql
+    :tickets: 4053
+
+    Altered the range specification for window functions to allow
+    for two of the same PRECEDING or FOLLOWING keywords in a range
+    by allowing for the left side of the range to be positive
+    and for the right to be negative, e.g. (1, 3) is
+    "1 FOLLOWING AND 3 FOLLOWING".

diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py
index 53009e2df18cd3d81543d9430dfda7748882d589..2dec3a5c39be8f6c2641624b62fb6cf4ea7ec70f 100644 (file)
--- a/lib/sqlalchemy/sql/compiler.py
+++ b/lib/sqlalchemy/sql/compiler.py
@@ -848,16 +848,25 @@ class SQLCompiler(Compiled):
              cast.typeclause._compiler_dispatch(self, **kwargs))
 
     def _format_frame_clause(self, range_, **kw):
+
         return '%s AND %s' % (
             "UNBOUNDED PRECEDING"
             if range_[0] is elements.RANGE_UNBOUNDED
             else "CURRENT ROW" if range_[0] is elements.RANGE_CURRENT
-            else "%s PRECEDING" % (self.process(range_[0], **kw), ),
+            else "%s PRECEDING" % (
+                self.process(elements.literal(abs(range_[0])), **kw), )
+            if range_[0] < 0
+            else "%s FOLLOWING" % (
+                self.process(elements.literal(range_[0]), **kw), ),
 
             "UNBOUNDED FOLLOWING"
             if range_[1] is elements.RANGE_UNBOUNDED
             else "CURRENT ROW" if range_[1] is elements.RANGE_CURRENT
-            else "%s FOLLOWING" % (self.process(range_[1], **kw), )
+            else "%s PRECEDING" % (
+                self.process(elements.literal(abs(range_[1])), **kw), )
+            if range_[1] < 0
+            else "%s FOLLOWING" % (
+                self.process(elements.literal(range_[1]), **kw), ),
         )
 
     def visit_over(self, over, **kwargs):

diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py
index 46a8d2c35906189a32c15826eefe9163df7af01d..36a6a655714ed5d1cc381ae1f5f35597cce49a1a 100644 (file)
--- a/lib/sqlalchemy/sql/elements.py
+++ b/lib/sqlalchemy/sql/elements.py
@@ -3162,6 +3162,10 @@ class Over(ColumnElement):
 
             func.row_number().over(order_by='x', range_=(-2, None))
 
+        * RANGE BETWEEN 1 FOLLOWING AND 3 FOLLOWING::
+
+            func.row_number().over(order_by='x', range_=(1, 3))
+
         .. versionadded:: 1.1 support for RANGE / ROWS within a window
 
 
@@ -3223,42 +3227,30 @@ class Over(ColumnElement):
             raise exc.ArgumentError("2-tuple expected for range/rows")
 
         if range_[0] is None:
-            preceding = RANGE_UNBOUNDED
+            lower = RANGE_UNBOUNDED
         else:
             try:
-                preceding = int(range_[0])
+                lower = int(range_[0])
             except ValueError:
                 raise exc.ArgumentError(
-                    "Integer or None expected for preceding value")
+                    "Integer or None expected for range value")
             else:
-                if preceding > 0:
-                    raise exc.ArgumentError(
-                        "Preceding value must be a "
-                        "negative integer, zero, or None")
-                elif preceding < 0:
-                    preceding = literal(abs(preceding))
-                else:
-                    preceding = RANGE_CURRENT
+                if lower == 0:
+                    lower = RANGE_CURRENT
 
         if range_[1] is None:
-            following = RANGE_UNBOUNDED
+            upper = RANGE_UNBOUNDED
         else:
             try:
-                following = int(range_[1])
+                upper = int(range_[1])
             except ValueError:
                 raise exc.ArgumentError(
-                    "Integer or None expected for following value")
+                    "Integer or None expected for range value")
             else:
-                if following < 0:
-                    raise exc.ArgumentError(
-                        "Following value must be a positive "
-                        "integer, zero, or None")
-                elif following > 0:
-                    following = literal(following)
-                else:
-                    following = RANGE_CURRENT
+                if upper == 0:
+                    upper = RANGE_CURRENT
 
-        return preceding, following
+        return lower, upper
 
     @property
     def func(self):

diff --git a/test/sql/test_compiler.py b/test/sql/test_compiler.py
index 05893d748ca53ec00bc846324d5fb601b1116ece..053619f46f0631ffd9616f9db78bca9b2700751a 100644 (file)
--- a/test/sql/test_compiler.py
+++ b/test/sql/test_compiler.py
@@ -2426,31 +2426,37 @@ class SelectTest(fixtures.TestBase, AssertsCompiledSQL):
             "(ORDER BY mytable.myid RANGE BETWEEN "
             ":param_1 PRECEDING AND :param_2 FOLLOWING)"
             " AS anon_1 FROM mytable",
-            {'param_1': 5, 'param_2': 10}
+            checkparams={'param_1': 5, 'param_2': 10}
         )
 
-    def test_over_invalid_framespecs(self):
-        assert_raises_message(
-            exc.ArgumentError,
-            "Preceding value must be a negative integer, zero, or None",
-            func.row_number().over, range_=(5, 10)
+        self.assert_compile(
+            select([func.row_number().over(order_by=expr, range_=(1, 10))]),
+            "SELECT row_number() OVER "
+            "(ORDER BY mytable.myid RANGE BETWEEN "
+            ":param_1 FOLLOWING AND :param_2 FOLLOWING)"
+            " AS anon_1 FROM mytable",
+            checkparams={'param_1': 1, 'param_2': 10}
         )
 
-        assert_raises_message(
-            exc.ArgumentError,
-            "Following value must be a positive integer, zero, or None",
-            func.row_number().over, range_=(-5, -8)
+        self.assert_compile(
+            select([func.row_number().over(order_by=expr, range_=(-10, -1))]),
+            "SELECT row_number() OVER "
+            "(ORDER BY mytable.myid RANGE BETWEEN "
+            ":param_1 PRECEDING AND :param_2 PRECEDING)"
+            " AS anon_1 FROM mytable",
+            checkparams={'param_1': 10, 'param_2': 1}
         )
 
+    def test_over_invalid_framespecs(self):
         assert_raises_message(
             exc.ArgumentError,
-            "Integer or None expected for preceding value",
+            "Integer or None expected for range value",
             func.row_number().over, range_=("foo", 8)
         )
 
         assert_raises_message(
             exc.ArgumentError,
-            "Integer or None expected for following value",
+            "Integer or None expected for range value",
             func.row_number().over, range_=(-5, "foo")
         )