fix(bigquery): Do not generate NULL ordering on Windows (#4480)

* fix(bigquery): Do not generate NULL ordering on Windows * PR Feedback 1
tobymao · Dec 5, 2024 · 438ae4c · 438ae4c
1 parent 41c6d24
commit 438ae4c
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 12 deletions.
diff --git a/sqlglot/generator.py b/sqlglot/generator.py
@@ -2399,18 +2399,21 @@ 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")
-                )
+            elif self.NULL_ORDERING_SUPPORTED is False 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 = ""
+                # BigQuery does not allow these ordering/nulls combinations when used under
+                # an aggregation func or under a window containing one
+                ancestor = expression.find_ancestor(exp.AggFunc, exp.Window, exp.Select)
+
+                if isinstance(ancestor, exp.Window):
+                    ancestor = ancestor.this
+                if isinstance(ancestor, exp.AggFunc):
+                    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
@@ -2131,6 +2131,16 @@ def test_null_ordering(self):
                 },
             )
 
+            self.validate_all(
+                f"SELECT SUM(f1) OVER (ORDER BY f2 {sort_order}) FROM t",
+                read={
+                    "": f"SELECT SUM(f1) OVER (ORDER BY f2 {sort_order} {null_order}) FROM t",
+                },
+                write={
+                    "bigquery": f"SELECT SUM(f1) OVER (ORDER BY f2 {sort_order}) FROM t",
+                },
+            )
+
     def test_json_extract(self):
         self.validate_all(
             """SELECT JSON_QUERY('{"class": {"students": []}}', '$.class')""",

diff --git a/tests/dialects/test_duckdb.py b/tests/dialects/test_duckdb.py
@@ -79,7 +79,7 @@ def test_duckdb(self):
         self.validate_all(
             "SELECT SUM(X) OVER (ORDER BY x)",
             write={
-                "bigquery": "SELECT SUM(X) OVER (ORDER BY x NULLS LAST)",
+                "bigquery": "SELECT SUM(X) OVER (ORDER BY x)",
                 "duckdb": "SELECT SUM(X) OVER (ORDER BY x)",
                 "mysql": "SELECT SUM(X) OVER (ORDER BY CASE WHEN x IS NULL THEN 1 ELSE 0 END, x)",
             },