Skip to content

Commit

Permalink
fix(bigquery): Do not generate null ordering on agg funcs (#4172)
Browse files Browse the repository at this point in the history
* fix(bigquery): Do not generate NULLS LAST on asc agg funcs

* PR Feedback 1
  • Loading branch information
VaggelisD authored Sep 30, 2024
1 parent dc2619a commit 0703152
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
15 changes: 14 additions & 1 deletion sqlglot/generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,8 @@ class Generator(metaclass=_Generator):
}

# Whether null ordering is supported in order by
# True: Full Support, None: No support, False: No support in window specifications
# True: Full Support, None: No support, False: No support for certain cases
# such as window specifications, aggregate functions etc
NULL_ORDERING_SUPPORTED: t.Optional[bool] = True

# Whether ignore nulls is inside the agg or outside.
Expand Down Expand Up @@ -2345,6 +2346,18 @@ def ordered_sql(self, expression: exp.Ordered) -> str:
f"'{nulls_sort_change.strip()}' translation not supported in window functions"
)
nulls_sort_change = ""
elif (
self.NULL_ORDERING_SUPPORTED is False
and (isinstance(expression.find_ancestor(exp.AggFunc, exp.Select), exp.AggFunc))
and (
(asc and nulls_sort_change == " NULLS LAST")
or (desc and nulls_sort_change == " NULLS FIRST")
)
):
self.unsupported(
f"'{nulls_sort_change.strip()}' translation not supported for aggregate functions with {sort_order} sort order"
)
nulls_sort_change = ""
elif self.NULL_ORDERING_SUPPORTED is None:
if expression.this.is_int:
self.unsupported(
Expand Down
14 changes: 14 additions & 0 deletions tests/dialects/test_bigquery.py
Original file line number Diff line number Diff line change
Expand Up @@ -1985,3 +1985,17 @@ def test_range_type(self):
self.validate_identity(
"SELECT RANGE(CAST('2022-10-01 14:53:27 America/Los_Angeles' AS TIMESTAMP), CAST('2022-10-01 16:00:00 America/Los_Angeles' AS TIMESTAMP))"
)

def test_null_ordering(self):
# Aggregate functions allow "NULLS FIRST" only with ascending order and
# "NULLS LAST" only with descending
for sort_order, null_order in (("ASC", "NULLS LAST"), ("DESC", "NULLS FIRST")):
self.validate_all(
f"SELECT color, ARRAY_AGG(id ORDER BY id {sort_order}) AS ids FROM colors GROUP BY 1",
read={
"": f"SELECT color, ARRAY_AGG(id ORDER BY id {sort_order} {null_order}) AS ids FROM colors GROUP BY 1"
},
write={
"bigquery": f"SELECT color, ARRAY_AGG(id ORDER BY id {sort_order}) AS ids FROM colors GROUP BY 1",
},
)

0 comments on commit 0703152

Please sign in to comment.