Refactor: simplify WITHIN GROUP ... FILTER support

Author: georgesittas

sqlglot/expressions.py

@@ -1577,7 +1577,7 @@ def recursive(self) -> bool:
 
 
 class WithinGroup(Expression):
-    arg_types = {"this": True, "expression": False, "filter": False}
+    arg_types = {"this": True, "expression": False}
 
 
 # clickhouse supports scalar ctes

sqlglot/generator.py

@@ -2724,11 +2724,7 @@ def windowspec_sql(self, expression: exp.WindowSpec) -> str:
     def withingroup_sql(self, expression: exp.WithinGroup) -> str:
         this = self.sql(expression, "this")
         expression_sql = self.sql(expression, "expression")[1:]  # order has a leading space
-        filter_sql = self.sql(expression, "filter")[1:]
-        result = f"{this} WITHIN GROUP ({expression_sql})"
-        if filter_sql:
-            result = f"{result} FILTER ({filter_sql})"
-        return result
+        return f"{this} WITHIN GROUP ({expression_sql})"
 
     def between_sql(self, expression: exp.Between) -> str:
         this = self.sql(expression, "this")

sqlglot/parser.py

@@ -6603,25 +6603,19 @@ def _parse_window(
         func = this
         comments = func.comments if isinstance(func, exp.Expression) else None
 
+        # T-SQL allows the OVER (...) syntax after WITHIN GROUP.
+        # https://learn.microsoft.com/en-us/sql/t-sql/functions/percentile-disc-transact-sql?view=sql-server-ver16
+        if self._match_text_seq("WITHIN", "GROUP"):
+            order = self._parse_wrapped(self._parse_order)
+            this = self.expression(exp.WithinGroup, this=this, expression=order)
+
         if self._match_pair(TokenType.FILTER, TokenType.L_PAREN):
             self._match(TokenType.WHERE)
             this = self.expression(
                 exp.Filter, this=this, expression=self._parse_where(skip_where_token=True)
             )
             self._match_r_paren()
 
-        # T-SQL allows the OVER (...) syntax after WITHIN GROUP.
-        # https://learn.microsoft.com/en-us/sql/t-sql/functions/percentile-disc-transact-sql?view=sql-server-ver16
-        if self._match_text_seq("WITHIN", "GROUP"):
-            order = self._parse_wrapped(self._parse_order)
-            if self._match(TokenType.FILTER):
-                where_clause = self._parse_wrapped(self._parse_where)
-                this = self.expression(
-                    exp.WithinGroup, this=this, expression=order, filter=where_clause
-                )
-            else:
-                this = self.expression(exp.WithinGroup, this=this, expression=order)
-
         # SQL spec defines an optional [ { IGNORE | RESPECT } NULLS ] OVER
         # Some dialects choose to implement and some do not.
         # https://dev.mysql.com/doc/refman/8.0/en/window-function-descriptions.html

tests/dialects/test_postgres.py

@@ -834,7 +834,7 @@ def test_postgres(self):
         )
 
         self.validate_identity(
-            "SELECT PERCENTILE_CONT(0.5) WITHIN GROUP (ORDER BY a) FILTER (WHERE CAST(b AS BOOLEAN)) AS mean_value FROM (VALUES (0, 't')) AS fake_data(a, b)"
+            "SELECT PERCENTILE_CONT(0.5) WITHIN GROUP (ORDER BY a) FILTER(WHERE CAST(b AS BOOLEAN)) AS mean_value FROM (VALUES (0, 't')) AS fake_data(a, b)"
         )
 
     def test_ddl(self):