Skip to content

Commit 82e8b30

Browse files
committed
add remove_null_ordering_for_range_windows to fix all windows errors
1 parent b32c125 commit 82e8b30

File tree

3 files changed

+29
-4
lines changed
  • bigframes/core/compile/sqlglot/aggregations
  • tests/unit/core/compile/sqlglot

3 files changed

+29
-4
lines changed

bigframes/core/compile/sqlglot/aggregations/windows.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ def apply_window_if_present(
4444
order_by = None
4545
elif window.is_range_bounded:
4646
order_by = get_window_order_by((window.ordering[0],))
47+
order_by = remove_null_ordering_for_range_windows(order_by)
4748
else:
4849
order_by = get_window_order_by(window.ordering)
4950

@@ -150,6 +151,30 @@ def get_window_order_by(
150151
return tuple(order_by)
151152

152153

154+
def remove_null_ordering_for_range_windows(
155+
order_by: typing.Optional[tuple[sge.Ordered, ...]],
156+
) -> typing.Optional[tuple[sge.Ordered, ...]]:
157+
"""Removes NULL FIRST/LAST from ORDER BY expressions in RANGE windows.
158+
Here's the support matrix:
159+
✅ sum(x) over (order by y desc nulls last)
160+
🚫 sum(x) over (order by y asc nulls last)
161+
✅ sum(x) over (order by y asc nulls first)
162+
🚫 sum(x) over (order by y desc nulls first)
163+
"""
164+
if order_by is None:
165+
return None
166+
167+
new_order_by = []
168+
for key in order_by:
169+
kargs = key.args
170+
if kargs.get("desc") is True and kargs.get("nulls_first", False):
171+
kargs["nulls_first"] = False
172+
elif kargs.get("desc") is False and not kargs.setdefault("nulls_first", True):
173+
kargs["nulls_first"] = True
174+
new_order_by.append(sge.Ordered(**kargs))
175+
return tuple(new_order_by)
176+
177+
153178
def _get_window_bounds(
154179
value, is_preceding: bool
155180
) -> tuple[typing.Union[str, sge.Expression], typing.Optional[str]]:

tests/unit/core/compile/sqlglot/aggregations/test_windows.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ def test_apply_window_if_present_range_bounded(self):
127127
)
128128
self.assertEqual(
129129
result.sql(dialect="bigquery"),
130-
"value OVER (ORDER BY `col1` ASC NULLS LAST RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW)",
130+
"value OVER (ORDER BY `col1` ASC RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW)",
131131
)
132132

133133
def test_apply_window_if_present_range_bounded_timedelta(self):
@@ -142,7 +142,7 @@ def test_apply_window_if_present_range_bounded_timedelta(self):
142142
)
143143
self.assertEqual(
144144
result.sql(dialect="bigquery"),
145-
"value OVER (ORDER BY `col1` ASC NULLS LAST RANGE BETWEEN 86400000000 PRECEDING AND 43200000000 FOLLOWING)",
145+
"value OVER (ORDER BY `col1` ASC RANGE BETWEEN 86400000000 PRECEDING AND 43200000000 FOLLOWING)",
146146
)
147147

148148
def test_apply_window_if_present_all_params(self):

tests/unit/core/compile/sqlglot/snapshots/test_compile_window/test_compile_window_w_range_rolling/out.sql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,15 @@ WITH `bfcte_0` AS (
88
CASE
99
WHEN COALESCE(
1010
SUM(CAST(NOT `bfcol_1` IS NULL AS INT64)) OVER (
11-
ORDER BY UNIX_MICROS(`bfcol_0`) ASC NULLS LAST
11+
ORDER BY UNIX_MICROS(`bfcol_0`) ASC
1212
RANGE BETWEEN 2999999 PRECEDING AND CURRENT ROW
1313
),
1414
0
1515
) < 1
1616
THEN NULL
1717
ELSE COALESCE(
1818
SUM(`bfcol_1`) OVER (
19-
ORDER BY UNIX_MICROS(`bfcol_0`) ASC NULLS LAST
19+
ORDER BY UNIX_MICROS(`bfcol_0`) ASC
2020
RANGE BETWEEN 2999999 PRECEDING AND CURRENT ROW
2121
),
2222
0

0 commit comments

Comments
 (0)