From 35c8f796a7c3065ca255dc151f1c1a99f37737a7 Mon Sep 17 00:00:00 2001 From: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Date: Tue, 28 Jan 2025 10:46:05 -0500 Subject: [PATCH] fix(backends): fix most remaining backends ordered sum --- .../test_analytic_exprs/first/out.sql | 2 +- .../test_analytic_exprs/last/out.sql | 2 +- .../test_group_by_with_window_preserves_range/out.sql | 2 +- .../test_window/test_add_default_order_by/out.sql | 4 ++-- .../test_window/test_aggregate_in_projection/out.sql | 2 +- .../test_cumulative_functions/max/out1.sql | 2 +- .../test_cumulative_functions/max/out2.sql | 2 +- .../test_cumulative_functions/mean/out1.sql | 2 +- .../test_cumulative_functions/mean/out2.sql | 2 +- .../test_cumulative_functions/min/out1.sql | 2 +- .../test_cumulative_functions/min/out2.sql | 2 +- .../test_cumulative_functions/sum/out1.sql | 2 +- .../test_cumulative_functions/sum/out2.sql | 2 +- .../test_window/test_multiple_windows/out.sql | 2 +- .../snapshots/test_window/test_order_by_desc/out2.sql | 2 +- .../test_window_frame_specs/cumulative/out.sql | 2 +- .../test_window_frame_specs/foll_0/out.sql | 2 +- ibis/backends/sql/compilers/clickhouse.py | 11 ----------- ibis/backends/sql/compilers/mysql.py | 7 ++----- .../impala/out.sql | 4 ++-- ibis/backends/tests/test_window.py | 7 +------ 21 files changed, 23 insertions(+), 42 deletions(-) diff --git a/ibis/backends/impala/tests/snapshots/test_analytic_functions/test_analytic_exprs/first/out.sql b/ibis/backends/impala/tests/snapshots/test_analytic_functions/test_analytic_exprs/first/out.sql index 1391a6c01c51..14c575abc2d5 100644 --- a/ibis/backends/impala/tests/snapshots/test_analytic_functions/test_analytic_exprs/first/out.sql +++ b/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` \ No newline at end of file diff --git a/ibis/backends/impala/tests/snapshots/test_analytic_functions/test_analytic_exprs/last/out.sql b/ibis/backends/impala/tests/snapshots/test_analytic_functions/test_analytic_exprs/last/out.sql index 904f923e17e7..cbfa788b8797 100644 --- a/ibis/backends/impala/tests/snapshots/test_analytic_functions/test_analytic_exprs/last/out.sql +++ b/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` \ No newline at end of file diff --git a/ibis/backends/impala/tests/snapshots/test_sql/test_group_by_with_window_preserves_range/out.sql b/ibis/backends/impala/tests/snapshots/test_sql/test_group_by_with_window_preserves_range/out.sql index 79ea467e969b..63aacc7c6eb9 100644 --- a/ibis/backends/impala/tests/snapshots/test_sql/test_group_by_with_window_preserves_range/out.sql +++ b/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` \ No newline at end of file +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` \ No newline at end of file diff --git a/ibis/backends/impala/tests/snapshots/test_window/test_add_default_order_by/out.sql b/ibis/backends/impala/tests/snapshots/test_window/test_add_default_order_by/out.sql index 9a50bddd2c69..dffd16692851 100644 --- a/ibis/backends/impala/tests/snapshots/test_window/test_add_default_order_by/out.sql +++ b/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` \ No newline at end of file diff --git a/ibis/backends/impala/tests/snapshots/test_window/test_aggregate_in_projection/out.sql b/ibis/backends/impala/tests/snapshots/test_window/test_aggregate_in_projection/out.sql index 3004bcd8535f..8a730a7c6f9e 100644 --- a/ibis/backends/impala/tests/snapshots/test_window/test_aggregate_in_projection/out.sql +++ b/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` \ No newline at end of file diff --git a/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/max/out1.sql b/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/max/out1.sql index 1c00fd99c792..211f7becce07 100644 --- a/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/max/out1.sql +++ b/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` \ No newline at end of file diff --git a/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/max/out2.sql b/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/max/out2.sql index 1c00fd99c792..211f7becce07 100644 --- a/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/max/out2.sql +++ b/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` \ No newline at end of file diff --git a/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/mean/out1.sql b/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/mean/out1.sql index 8bfdfda64125..a623a3cfe066 100644 --- a/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/mean/out1.sql +++ b/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` \ No newline at end of file diff --git a/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/mean/out2.sql b/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/mean/out2.sql index 8bfdfda64125..a623a3cfe066 100644 --- a/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/mean/out2.sql +++ b/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` \ No newline at end of file diff --git a/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/min/out1.sql b/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/min/out1.sql index fa763e794811..bc120a638006 100644 --- a/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/min/out1.sql +++ b/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` \ No newline at end of file diff --git a/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/min/out2.sql b/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/min/out2.sql index fa763e794811..bc120a638006 100644 --- a/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/min/out2.sql +++ b/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` \ No newline at end of file diff --git a/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/sum/out1.sql b/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/sum/out1.sql index 4a0222bfac7c..bcd6a2ba703e 100644 --- a/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/sum/out1.sql +++ b/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` \ No newline at end of file diff --git a/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/sum/out2.sql b/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/sum/out2.sql index 4a0222bfac7c..bcd6a2ba703e 100644 --- a/ibis/backends/impala/tests/snapshots/test_window/test_cumulative_functions/sum/out2.sql +++ b/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` \ No newline at end of file diff --git a/ibis/backends/impala/tests/snapshots/test_window/test_multiple_windows/out.sql b/ibis/backends/impala/tests/snapshots/test_window/test_multiple_windows/out.sql index 7f161f658529..69e25f294384 100644 --- a/ibis/backends/impala/tests/snapshots/test_window/test_multiple_windows/out.sql +++ b/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` \ No newline at end of file diff --git a/ibis/backends/impala/tests/snapshots/test_window/test_order_by_desc/out2.sql b/ibis/backends/impala/tests/snapshots/test_window/test_order_by_desc/out2.sql index 148babbf968d..a5c1c098080e 100644 --- a/ibis/backends/impala/tests/snapshots/test_window/test_order_by_desc/out2.sql +++ b/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` \ No newline at end of file diff --git a/ibis/backends/impala/tests/snapshots/test_window/test_window_frame_specs/cumulative/out.sql b/ibis/backends/impala/tests/snapshots/test_window/test_window_frame_specs/cumulative/out.sql index 59c90d433f04..560362ff0a45 100644 --- a/ibis/backends/impala/tests/snapshots/test_window/test_window_frame_specs/cumulative/out.sql +++ b/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` \ No newline at end of file diff --git a/ibis/backends/impala/tests/snapshots/test_window/test_window_frame_specs/foll_0/out.sql b/ibis/backends/impala/tests/snapshots/test_window/test_window_frame_specs/foll_0/out.sql index 59c90d433f04..560362ff0a45 100644 --- a/ibis/backends/impala/tests/snapshots/test_window/test_window_frame_specs/foll_0/out.sql +++ b/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` \ No newline at end of file diff --git a/ibis/backends/sql/compilers/clickhouse.py b/ibis/backends/sql/compilers/clickhouse.py index 9f84071e450f..1deef9d44cd3 100644 --- a/ibis/backends/sql/compilers/clickhouse.py +++ b/ibis/backends/sql/compilers/clickhouse.py @@ -119,17 +119,6 @@ class ClickHouseCompiler(SQLGlotCompiler): ops.RandomScalar: "randCanonical", } - @staticmethod - def _minimize_spec(op, spec): - if ( - op.start is None - and isinstance(getattr(end := op.end, "value", None), ops.Literal) - and end.value.value == 0 - and end.following - ): - return None - return spec - def visit_Cast(self, op, *, arg, to): _interval_cast_suffixes = { "s": "Second", diff --git a/ibis/backends/sql/compilers/mysql.py b/ibis/backends/sql/compilers/mysql.py index 5dae4a1f4523..d8f0c3262d13 100644 --- a/ibis/backends/sql/compilers/mysql.py +++ b/ibis/backends/sql/compilers/mysql.py @@ -102,11 +102,8 @@ def POS_INF(self): @staticmethod def _minimize_spec(op, spec): - if ( - op.start is None - and isinstance(getattr(end := op.end, "value", None), ops.Literal) - and end.value.value == 0 - and end.following + if isinstance( + op.func, (ops.RankBase, ops.CumeDist, ops.NTile, ops.PercentRank) ): return None return spec diff --git a/ibis/backends/tests/snapshots/test_sql/test_mixed_qualified_and_unqualified_predicates/impala/out.sql b/ibis/backends/tests/snapshots/test_sql/test_mixed_qualified_and_unqualified_predicates/impala/out.sql index 787083fbab6a..642b68f972ff 100644 --- a/ibis/backends/tests/snapshots/test_sql/test_mixed_qualified_and_unqualified_predicates/impala/out.sql +++ b/ibis/backends/tests/snapshots/test_sql/test_mixed_qualified_and_unqualified_predicates/impala/out.sql @@ -5,11 +5,11 @@ FROM ( SELECT `t1`.`x`, `t1`.`y`, - AVG(`t1`.`x`) OVER (ORDER BY NULL ASC) AS _w + AVG(`t1`.`x`) OVER (ORDER BY NULL ASC ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS _w FROM ( SELECT `t0`.`x`, - SUM(`t0`.`x`) OVER (ORDER BY NULL ASC) AS `y` + SUM(`t0`.`x`) OVER (ORDER BY NULL ASC ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS `y` FROM `t` AS `t0` ) AS `t1` WHERE diff --git a/ibis/backends/tests/test_window.py b/ibis/backends/tests/test_window.py index c91c57d04d4e..ac028b145b3d 100644 --- a/ibis/backends/tests/test_window.py +++ b/ibis/backends/tests/test_window.py @@ -664,11 +664,6 @@ def test_simple_ungrouped_window_with_scalar_order_by(alltypes): True, id="ordered-mean", marks=[ - pytest.mark.notimpl( - ["impala"], - reason="default window semantics are different", - raises=AssertionError, - ), pytest.mark.notimpl( ["risingwave"], raises=PsycoPg2InternalError, @@ -1250,7 +1245,7 @@ def test_windowed_order_by_sequence_is_preserved(con): reason="window functions aren't yet implemented for the polars backend", ) @pytest.mark.notimpl( - ["clickhouse", "flink", "mysql"], + ["flink"], raises=AssertionError, reason="default behavior is the same as bigquery and hasn't been addressed", )