From 0703152a25afced183dc5efd5f62311a48545420 Mon Sep 17 00:00:00 2001 From: Vaggelis Danias Date: Mon, 30 Sep 2024 15:23:21 +0300 Subject: [PATCH] fix(bigquery): Do not generate null ordering on agg funcs (#4172) * fix(bigquery): Do not generate NULLS LAST on asc agg funcs * PR Feedback 1 --- sqlglot/generator.py | 15 ++++++++++++++- tests/dialects/test_bigquery.py | 14 ++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/sqlglot/generator.py b/sqlglot/generator.py index 1f85126b7f..bce7c0b514 100644 --- a/sqlglot/generator.py +++ b/sqlglot/generator.py @@ -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. @@ -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( diff --git a/tests/dialects/test_bigquery.py b/tests/dialects/test_bigquery.py index e2adfeabce..d854165f2c 100644 --- a/tests/dialects/test_bigquery.py +++ b/tests/dialects/test_bigquery.py @@ -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", + }, + )