fix(backends): ensure that analytic functions do not receive a window…

… frame

ibis/backends/impala/tests/snapshots/test_analytic_functions/test_analytic_exprs/first/out.sql

@@ -1,3 +1,3 @@
 SELECT
-  FIRST_VALUE(`t0`.`double_col`) OVER (ORDER BY `t0`.`id` ASC) AS `First(double_col, ())`
+  FIRST_VALUE(`t0`.`double_col`) OVER (ORDER BY `t0`.`id` ASC ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS `First(double_col, ())`
 FROM `functional_alltypes` AS `t0`

ibis/backends/impala/tests/snapshots/test_analytic_functions/test_analytic_exprs/last/out.sql

@@ -1,3 +1,3 @@
 SELECT
-  LAST_VALUE(`t0`.`double_col`) OVER (ORDER BY `t0`.`id` ASC) AS `Last(double_col, ())`
+  LAST_VALUE(`t0`.`double_col`) OVER (ORDER BY `t0`.`id` ASC ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS `Last(double_col, ())`
 FROM `functional_alltypes` AS `t0`

ibis/backends/impala/tests/snapshots/test_exprs/test_filter_with_analytic/out.sql

@@ -1 +1 @@
-SELECT * FROM (SELECT `t1`.`col`, COUNT(*) OVER (ORDER BY NULL ASC) AS `analytic` FROM (SELECT `t0`.`col`, NULL AS `filter` FROM `x` AS `t0` WHERE NULL IS NULL) AS `t1`) AS `t2`
+SELECT * FROM (SELECT `t1`.`col`, COUNT(*) OVER (ORDER BY NULL ASC ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS `analytic` FROM (SELECT `t0`.`col`, NULL AS `filter` FROM `x` AS `t0` WHERE NULL IS NULL) AS `t1`) AS `t2`

ibis/backends/impala/tests/snapshots/test_sql/test_group_by_with_window_preserves_range/out.sql

@@ -1 +1 @@
-SELECT `t0`.`one`, `t0`.`two`, `t0`.`three`, SUM(`t0`.`two`) OVER (PARTITION BY `t0`.`three` ORDER BY `t0`.`one` ASC) AS `four` FROM `my_data` AS `t0`
+SELECT `t0`.`one`, `t0`.`two`, `t0`.`three`, SUM(`t0`.`two`) OVER (PARTITION BY `t0`.`three` ORDER BY `t0`.`one` ASC ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS `four` FROM `my_data` AS `t0`

ibis/backends/impala/tests/snapshots/test_window/test_add_default_order_by/out.sql

@@ -12,7 +12,7 @@ SELECT
   `t0`.`k`,
   LAG(`t0`.`f`) OVER (PARTITION BY `t0`.`g` ORDER BY NULL ASC) AS `lag`,
   LEAD(`t0`.`f`) OVER (PARTITION BY `t0`.`g` ORDER BY NULL ASC) - `t0`.`f` AS `fwd_diff`,
-  FIRST_VALUE(`t0`.`f`) OVER (PARTITION BY `t0`.`g` ORDER BY NULL ASC) AS `first`,
-  LAST_VALUE(`t0`.`f`) OVER (PARTITION BY `t0`.`g` ORDER BY NULL ASC) AS `last`,
+  FIRST_VALUE(`t0`.`f`) OVER (PARTITION BY `t0`.`g` ORDER BY NULL ASC ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS `first`,
+  LAST_VALUE(`t0`.`f`) OVER (PARTITION BY `t0`.`g` ORDER BY NULL ASC ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS `last`,
   LAG(`t0`.`f`) OVER (PARTITION BY `t0`.`g` ORDER BY `t0`.`d` ASC) AS `lag2`
 FROM `alltypes` AS `t0`

ibis/backends/impala/tests/snapshots/test_window/test_aggregate_in_projection/out.sql

@@ -10,5 +10,5 @@ SELECT
   `t0`.`i`,
   `t0`.`j`,
   `t0`.`k`,
-  `t0`.`f` / SUM(`t0`.`f`) OVER (ORDER BY NULL ASC) AS `normed_f`
+  `t0`.`f` / SUM(`t0`.`f`) OVER (ORDER BY NULL ASC ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS `normed_f`
 FROM `alltypes` AS `t0`

ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/max/out1.sql

@@ -1,3 +1,3 @@
 SELECT
-  MAX(`t0`.`f`) OVER (ORDER BY `t0`.`d` ASC) AS `foo`
+  MAX(`t0`.`f`) OVER (ORDER BY `t0`.`d` ASC ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS `foo`
 FROM `alltypes` AS `t0`

ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/max/out2.sql

@@ -1,3 +1,3 @@
 SELECT
-  MAX(`t0`.`f`) OVER (ORDER BY `t0`.`d` ASC) AS `foo`
+  MAX(`t0`.`f`) OVER (ORDER BY `t0`.`d` ASC ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS `foo`
 FROM `alltypes` AS `t0`

ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/mean/out1.sql

@@ -1,3 +1,3 @@
 SELECT
-  AVG(`t0`.`f`) OVER (ORDER BY `t0`.`d` ASC) AS `foo`
+  AVG(`t0`.`f`) OVER (ORDER BY `t0`.`d` ASC ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS `foo`
 FROM `alltypes` AS `t0`

ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/mean/out2.sql

@@ -1,3 +1,3 @@
 SELECT
-  AVG(`t0`.`f`) OVER (ORDER BY `t0`.`d` ASC) AS `foo`
+  AVG(`t0`.`f`) OVER (ORDER BY `t0`.`d` ASC ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS `foo`
 FROM `alltypes` AS `t0`

ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/min/out1.sql

@@ -1,3 +1,3 @@
 SELECT
-  MIN(`t0`.`f`) OVER (ORDER BY `t0`.`d` ASC) AS `foo`
+  MIN(`t0`.`f`) OVER (ORDER BY `t0`.`d` ASC ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS `foo`
 FROM `alltypes` AS `t0`

ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/min/out2.sql

@@ -1,3 +1,3 @@
 SELECT
-  MIN(`t0`.`f`) OVER (ORDER BY `t0`.`d` ASC) AS `foo`
+  MIN(`t0`.`f`) OVER (ORDER BY `t0`.`d` ASC ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS `foo`
 FROM `alltypes` AS `t0`

ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/sum/out1.sql

@@ -1,3 +1,3 @@
 SELECT
-  SUM(`t0`.`f`) OVER (ORDER BY `t0`.`d` ASC) AS `foo`
+  SUM(`t0`.`f`) OVER (ORDER BY `t0`.`d` ASC ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS `foo`
 FROM `alltypes` AS `t0`

ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/sum/out2.sql

@@ -1,3 +1,3 @@
 SELECT
-  SUM(`t0`.`f`) OVER (ORDER BY `t0`.`d` ASC) AS `foo`
+  SUM(`t0`.`f`) OVER (ORDER BY `t0`.`d` ASC ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS `foo`
 FROM `alltypes` AS `t0`

ibis/backends/impala/tests/snapshots/test_window/test_multiple_windows/out.sql

@@ -1,4 +1,4 @@
 SELECT
   `t0`.`g`,
-  SUM(`t0`.`f`) OVER (PARTITION BY `t0`.`g` ORDER BY NULL ASC) - SUM(`t0`.`f`) OVER (ORDER BY NULL ASC) AS `result`
+  SUM(`t0`.`f`) OVER (PARTITION BY `t0`.`g` ORDER BY NULL ASC ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) - SUM(`t0`.`f`) OVER (ORDER BY NULL ASC ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS `result`
 FROM `alltypes` AS `t0`

ibis/backends/impala/tests/snapshots/test_window/test_order_by_desc/out2.sql

@@ -1,4 +1,4 @@
 SELECT
   LAG(`t0`.`d`) OVER (PARTITION BY `t0`.`g` ORDER BY `t0`.`f` DESC NULLS LAST) AS `foo`,
-  MAX(`t0`.`a`) OVER (PARTITION BY `t0`.`g` ORDER BY `t0`.`f` DESC NULLS LAST) AS `Max(a)`
+  MAX(`t0`.`a`) OVER (PARTITION BY `t0`.`g` ORDER BY `t0`.`f` DESC NULLS LAST ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS `Max(a)`
 FROM `alltypes` AS `t0`

ibis/backends/impala/tests/snapshots/test_window/test_window_frame_specs/cumulative/out.sql

@@ -1,3 +1,3 @@
 SELECT
-  SUM(`t0`.`d`) OVER (ORDER BY `t0`.`f` ASC) AS `foo`
+  SUM(`t0`.`d`) OVER (ORDER BY `t0`.`f` ASC ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS `foo`
 FROM `alltypes` AS `t0`

ibis/backends/impala/tests/snapshots/test_window/test_window_frame_specs/foll_0/out.sql

@@ -1,3 +1,3 @@
 SELECT
-  SUM(`t0`.`d`) OVER (ORDER BY `t0`.`f` ASC) AS `foo`
+  SUM(`t0`.`d`) OVER (ORDER BY `t0`.`f` ASC ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS `foo`
 FROM `alltypes` AS `t0`

ibis/backends/sql/compilers/base.py

@@ -1185,12 +1185,12 @@ def visit_WindowFunction(self, op, *, how, func, start, end, group_by, order_by)
         )
         order = sge.Order(expressions=order_by) if order_by else None
 
-        spec = self._minimize_spec(op.start, op.end, spec)
+        spec = self._minimize_spec(op, spec)
 
         return sge.Window(this=func, partition_by=group_by, order=order, spec=spec)
 
     @staticmethod
-    def _minimize_spec(start, end, spec):
+    def _minimize_spec(op, spec):
         return spec
 
     def visit_LagLead(self, op, *, arg, offset, default):

ibis/backends/sql/compilers/bigquery/__init__.py

@@ -19,6 +19,8 @@
 from ibis.backends.sql.compilers.bigquery.udf.core import PythonToJavaScriptTranslator
 from ibis.backends.sql.datatypes import BigQueryType, BigQueryUDFType
 from ibis.backends.sql.rewrites import (
+    FirstValue,
+    LastValue,
     exclude_unsupported_window_frame_from_ops,
     exclude_unsupported_window_frame_from_rank,
     exclude_unsupported_window_frame_from_row_number,
@@ -323,12 +325,10 @@ def _compile_python_udf(self, udf_node: ops.ScalarUDF) -> sge.Create:
         return func
 
     @staticmethod
-    def _minimize_spec(start, end, spec):
-        if (
-            start is None
-            and isinstance(getattr(end, "value", None), ops.Literal)
-            and end.value.value == 0
-            and end.following
+    def _minimize_spec(op, spec):
+        # bigquery doesn't allow certain window functions to specify a window frame
+        if isinstance(func := op.func, ops.Analytic) and not isinstance(
+            func, (ops.First, ops.Last, FirstValue, LastValue, ops.NthValue)
         ):
             return None
         return spec

ibis/backends/sql/compilers/clickhouse.py

@@ -120,13 +120,8 @@ class ClickHouseCompiler(SQLGlotCompiler):
     }
 
     @staticmethod
-    def _minimize_spec(start, end, spec):
-        if (
-            start is None
-            and isinstance(getattr(end, "value", None), ops.Literal)
-            and end.value.value == 0
-            and end.following
-        ):
+    def _minimize_spec(op, spec):
+        if isinstance(op.func, ops.NTile):
             return None
         return spec
 

ibis/backends/sql/compilers/exasol.py

@@ -10,6 +10,8 @@
 from ibis.backends.sql.datatypes import ExasolType
 from ibis.backends.sql.dialects import Exasol
 from ibis.backends.sql.rewrites import (
+    FirstValue,
+    LastValue,
     exclude_unsupported_window_frame_from_ops,
     exclude_unsupported_window_frame_from_rank,
     exclude_unsupported_window_frame_from_row_number,
@@ -85,12 +87,9 @@ class ExasolCompiler(SQLGlotCompiler):
     }
 
     @staticmethod
-    def _minimize_spec(start, end, spec):
-        if (
-            start is None
-            and isinstance(getattr(end, "value", None), ops.Literal)
-            and end.value.value == 0
-            and end.following
+    def _minimize_spec(op, spec):
+        if isinstance(func := op.func, ops.Analytic) and not isinstance(
+            func, (ops.First, ops.Last, FirstValue, LastValue, ops.NthValue)
         ):
             return None
         return spec

ibis/backends/sql/compilers/flink.py

@@ -128,16 +128,16 @@ def _generate_groups(groups):
         return groups
 
     @staticmethod
-    def _minimize_spec(start, end, spec):
+    def _minimize_spec(op, spec):
         if (
-            start is None
-            and isinstance(getattr(end, "value", None), ops.Literal)
+            op.start is None
+            and isinstance(getattr(end := op.end, "value", None), ops.Literal)
             and end.value.value == 0
             and end.following
         ):
             return None
         elif (
-            isinstance(getattr(end, "value", None), ops.Cast)
+            isinstance(getattr(end := op.end, "value", None), ops.Cast)
             and end.value.arg.value == 0
             and end.following
         ):

ibis/backends/sql/compilers/impala.py

@@ -10,7 +10,12 @@
 from ibis.backends.sql.compilers.base import NULL, STAR, SQLGlotCompiler
 from ibis.backends.sql.datatypes import ImpalaType
 from ibis.backends.sql.dialects import Impala
-from ibis.backends.sql.rewrites import lower_sample, rewrite_empty_order_by_window
+from ibis.backends.sql.rewrites import (
+    FirstValue,
+    LastValue,
+    lower_sample,
+    rewrite_empty_order_by_window,
+)
 
 
 class ImpalaCompiler(SQLGlotCompiler):
@@ -73,28 +78,11 @@ class ImpalaCompiler(SQLGlotCompiler):
     }
 
     @staticmethod
-    def _minimize_spec(start, end, spec):
-        # start is None means unbounded preceding
-        if start is None:
-            # end is None: unbounded following
-            # end == 0 => current row
-            # these are treated the same because for the functions where these
-            # are not allowed they end up behaving the same
-            #
-            # I think we're not covering some cases here:
-            # These will be treated the same, even though they're not
-            # - window(order_by=x, rows=(None, None))  # should be equivalent to `over ()`
-            # - window(order_by=x, rows=(None, 0))     # equivalent to a cumulative aggregation
-            #
-            # TODO(cpcloud): we need to clean up the semantics of unbounded
-            # following vs current row at the API level.
-            #
-            if end is None or (
-                isinstance(getattr(end, "value", None), ops.Literal)
-                and end.value.value == 0
-                and end.following
-            ):
-                return None
+    def _minimize_spec(op, spec):
+        if isinstance(func := op.func, ops.Analytic) and not isinstance(
+            func, (ops.First, ops.Last, FirstValue, LastValue, ops.NthValue)
+        ):
+            return None
         return spec
 
     def visit_Log2(self, op, *, arg):

ibis/backends/sql/compilers/mssql.py

@@ -20,6 +20,8 @@
 from ibis.backends.sql.datatypes import MSSQLType
 from ibis.backends.sql.dialects import MSSQL
 from ibis.backends.sql.rewrites import (
+    FirstValue,
+    LastValue,
     exclude_unsupported_window_frame_from_ops,
     exclude_unsupported_window_frame_from_rank,
     exclude_unsupported_window_frame_from_row_number,
@@ -147,12 +149,9 @@ def _generate_groups(groups):
         return groups
 
     @staticmethod
-    def _minimize_spec(start, end, spec):
-        if (
-            start is None
-            and isinstance(getattr(end, "value", None), ops.Literal)
-            and end.value.value == 0
-            and end.following
+    def _minimize_spec(op, spec):
+        if isinstance(func := op.func, ops.Analytic) and not isinstance(
+            func, (ops.First, ops.Last, FirstValue, LastValue, ops.NthValue)
         ):
             return None
         return spec

ibis/backends/sql/compilers/mysql.py

@@ -101,12 +101,9 @@ def POS_INF(self):
     }
 
     @staticmethod
-    def _minimize_spec(start, end, spec):
-        if (
-            start is None
-            and isinstance(getattr(end, "value", None), ops.Literal)
-            and end.value.value == 0
-            and end.following
+    def _minimize_spec(op, spec):
+        if isinstance(
+            op.func, (ops.RankBase, ops.CumeDist, ops.NTile, ops.PercentRank)
         ):
             return None
         return spec

ibis/backends/sql/compilers/oracle.py

@@ -454,7 +454,7 @@ def visit_WindowFunction(self, op, *, how, func, start, end, group_by, order_by)
 
         order = sge.Order(expressions=order_by) if order_by else None
 
-        spec = self._minimize_spec(op.start, op.end, spec)
+        spec = self._minimize_spec(op, spec)
 
         return sge.Window(this=func, partition_by=group_by, order=order, spec=spec)
 

ibis/backends/sql/compilers/snowflake.py

@@ -25,6 +25,8 @@
 from ibis.backends.sql.datatypes import SnowflakeType
 from ibis.backends.sql.dialects import Snowflake
 from ibis.backends.sql.rewrites import (
+    FirstValue,
+    LastValue,
     exclude_unsupported_window_frame_from_ops,
     exclude_unsupported_window_frame_from_row_number,
     lower_log2,
@@ -237,17 +239,6 @@ def _compile_udf(self, udf_node: ops.ScalarUDF):
     _compile_pandas_udf = _compile_udf
     _compile_python_udf = _compile_udf
 
-    @staticmethod
-    def _minimize_spec(start, end, spec):
-        if (
-            start is None
-            and isinstance(getattr(end, "value", None), ops.Literal)
-            and end.value.value == 0
-            and end.following
-        ):
-            return None
-        return spec
-
     def visit_Literal(self, op, *, value, dtype):
         if value is None:
             return super().visit_Literal(op, value=value, dtype=dtype)
@@ -668,6 +659,14 @@ def visit_Xor(self, op, *, left, right):
         # boolxor accepts numerics ... and returns a boolean? wtf?
         return self.f.boolxor(self.cast(left, dt.int8), self.cast(right, dt.int8))
 
+    @staticmethod
+    def _minimize_spec(op, spec):
+        if isinstance(func := op.func, ops.Analytic) and not isinstance(
+            func, (ops.First, ops.Last, FirstValue, LastValue, ops.NthValue)
+        ):
+            return None
+        return spec
+
     def visit_WindowFunction(self, op, *, how, func, start, end, group_by, order_by):
         if start is None:
             start = {}
@@ -698,7 +697,7 @@ def visit_WindowFunction(self, op, *, how, func, start, end, group_by, order_by)
         order = sge.Order(expressions=order_by) if order_by else None
 
         orig_spec = spec
-        spec = self._minimize_spec(op.start, op.end, orig_spec)
+        spec = self._minimize_spec(op, orig_spec)
 
         # due to https://docs.snowflake.com/en/sql-reference/functions-analytic#window-frame-usage-notes
         # we need to make the default window rows (since range isn't supported)

ibis/backends/sql/compilers/trino.py

@@ -22,6 +22,8 @@
 from ibis.backends.sql.datatypes import TrinoType
 from ibis.backends.sql.dialects import Trino
 from ibis.backends.sql.rewrites import (
+    FirstValue,
+    LastValue,
     exclude_unsupported_window_frame_from_ops,
     exclude_unsupported_window_frame_from_rank,
     exclude_unsupported_window_frame_from_row_number,
@@ -104,12 +106,9 @@ class TrinoCompiler(SQLGlotCompiler):
     }
 
     @staticmethod
-    def _minimize_spec(start, end, spec):
-        if (
-            start is None
-            and isinstance(getattr(end, "value", None), ops.Literal)
-            and end.value.value == 0
-            and end.following
+    def _minimize_spec(op, spec):
+        if isinstance(func := op.func, ops.Analytic) and not isinstance(
+            func, (ops.First, ops.Last, FirstValue, LastValue, ops.NthValue)
         ):
             return None
         return spec