Repository: tobymao/sqlglot

Issue:
WITHIN GROUP ( order_by_clause ) FILTER breaks for Postgres
Read / convert dialect is Postgres.

**Fully reproducible code snippet**
```
sqlglot.errors.ParseError: Invalid expression / Unexpected token. Line 1, Col: 62.
  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)
```

**Official Documentation**
https://www.postgresql.org/docs/current/sql-expressions.html#SYNTAX-AGGREGATES


Discussion:
georgesittas: Hey @gl3nnleblanc, you'll need to run `make style` and then commit styling changes / fix mypy issues for the CI to pass.

Patch:
diff --git a/sqlglot/dialects/athena.py b/sqlglot/dialects/athena.py
index 108c7643..7ae47def 100644
--- a/sqlglot/dialects/athena.py
+++ b/sqlglot/dialects/athena.py
@@ -32,6 +32,18 @@ def _generate_as_hive(expression: exp.Expression) -> bool:
     return False
 
 
+def _is_iceberg_table(properties: exp.Properties) -> bool:
+    table_type_property = next(
+        (
+            p
+            for p in properties.expressions
+            if isinstance(p, exp.Property) and p.name == "table_type"
+        ),
+        None,
+    )
+    return bool(table_type_property and table_type_property.text("value").lower() == "iceberg")
+
+
 def _location_property_sql(self: Athena.Generator, e: exp.LocationProperty):
     # If table_type='iceberg', the LocationProperty is called 'location'
     # Otherwise, it's called 'external_location'
@@ -40,20 +52,25 @@ def _location_property_sql(self: Athena.Generator, e: exp.LocationProperty):
     prop_name = "external_location"
 
     if isinstance(e.parent, exp.Properties):
-        table_type_property = next(
-            (
-                p
-                for p in e.parent.expressions
-                if isinstance(p, exp.Property) and p.name == "table_type"
-            ),
-            None,
-        )
-        if table_type_property and table_type_property.text("value").lower() == "iceberg":
+        if _is_iceberg_table(e.parent):
             prop_name = "location"
 
     return f"{prop_name}={self.sql(e, 'this')}"
 
 
+def _partitioned_by_property_sql(self: Athena.Generator, e: exp.PartitionedByProperty) -> str:
+    # If table_type='iceberg' then the table property for partitioning is called 'partitioning'
+    # If table_type='hive' it's called 'partitioned_by'
+    # ref: https://docs.aws.amazon.com/athena/latest/ug/create-table-as.html#ctas-table-properties
+
+    prop_name = "partitioned_by"
+    if isinstance(e.parent, exp.Properties):
+        if _is_iceberg_table(e.parent):
+            prop_name = "partitioning"
+
+    return f"{prop_name}={self.sql(e, 'this')}"
+
+
 class Athena(Trino):
     """
     Over the years, it looks like AWS has taken various execution engines, bolted on AWS-specific modifications and then
@@ -132,7 +149,7 @@ class Athena(Trino):
         TRANSFORMS = {
             **Trino.Generator.TRANSFORMS,
             exp.FileFormatProperty: lambda self, e: f"format={self.sql(e, 'this')}",
-            exp.PartitionedByProperty: lambda self, e: f"partitioned_by={self.sql(e, 'this')}",
+            exp.PartitionedByProperty: _partitioned_by_property_sql,
             exp.LocationProperty: _location_property_sql,
         }
 
diff --git a/sqlglot/expressions.py b/sqlglot/expressions.py
index e3cc94e1..f6202b54 100644
--- a/sqlglot/expressions.py
+++ b/sqlglot/expressions.py
@@ -1577,7 +1577,7 @@ class With(Expression):
 
 
 class WithinGroup(Expression):
-    arg_types = {"this": True, "expression": False}
+    arg_types = {"this": True, "expression": False, "filter": False}
 
 
 # clickhouse supports scalar ctes
diff --git a/sqlglot/generator.py b/sqlglot/generator.py
index c4275812..2ba7cb08 100644
--- a/sqlglot/generator.py
+++ b/sqlglot/generator.py
@@ -2724,7 +2724,11 @@ class Generator(metaclass=_Generator):
     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
-        return f"{this} WITHIN GROUP ({expression_sql})"
+        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
 
     def between_sql(self, expression: exp.Between) -> str:
         this = self.sql(expression, "this")
diff --git a/sqlglot/parser.py b/sqlglot/parser.py
index 54407ee3..726cc616 100644
--- a/sqlglot/parser.py
+++ b/sqlglot/parser.py
@@ -6614,7 +6614,13 @@ class Parser(metaclass=_Parser):
         # 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(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.


Test patch:
diff --git a/tests/dialects/test_athena.py b/tests/dialects/test_athena.py
index ef96938b..c5ba4ccf 100644
--- a/tests/dialects/test_athena.py
+++ b/tests/dialects/test_athena.py
@@ -198,7 +198,8 @@ class TestAthena(Validator):
 
     def test_ctas(self):
         # Hive tables use 'external_location' to specify the table location, Iceberg tables use 'location' to specify the table location
-        # The 'table_type' property is used to determine if it's a Hive or an Iceberg table
+        # In addition, Hive tables used 'partitioned_by' to specify the partition fields and Iceberg tables use 'partitioning' to specify the partition fields
+        # The 'table_type' property is used to determine if it's a Hive or an Iceberg table. If it's omitted, it defaults to Hive
         # ref: https://docs.aws.amazon.com/athena/latest/ug/create-table-as.html#ctas-table-properties
         ctas_hive = exp.Create(
             this=exp.to_table("foo.bar"),
@@ -207,13 +208,16 @@ class TestAthena(Validator):
                 expressions=[
                     exp.FileFormatProperty(this=exp.Literal.string("parquet")),
                     exp.LocationProperty(this=exp.Literal.string("s3://foo")),
+                    exp.PartitionedByProperty(
+                        this=exp.Schema(expressions=[exp.to_column("partition_col")])
+                    ),
                 ]
             ),
             expression=exp.select("1"),
         )
         self.assertEqual(
             ctas_hive.sql(dialect=self.dialect, identify=True),
-            "CREATE TABLE \"foo\".\"bar\" WITH (format='parquet', external_location='s3://foo') AS SELECT 1",
+            "CREATE TABLE \"foo\".\"bar\" WITH (format='parquet', external_location='s3://foo', partitioned_by=ARRAY['partition_col']) AS SELECT 1",
         )
 
         ctas_iceberg = exp.Create(
@@ -223,11 +227,14 @@ class TestAthena(Validator):
                 expressions=[
                     exp.Property(this=exp.var("table_type"), value=exp.Literal.string("iceberg")),
                     exp.LocationProperty(this=exp.Literal.string("s3://foo")),
+                    exp.PartitionedByProperty(
+                        this=exp.Schema(expressions=[exp.to_column("partition_col")])
+                    ),
                 ]
             ),
             expression=exp.select("1"),
         )
         self.assertEqual(
             ctas_iceberg.sql(dialect=self.dialect, identify=True),
-            "CREATE TABLE \"foo\".\"bar\" WITH (table_type='iceberg', location='s3://foo') AS SELECT 1",
+            "CREATE TABLE \"foo\".\"bar\" WITH (table_type='iceberg', location='s3://foo', partitioning=ARRAY['partition_col']) AS SELECT 1",
         )
diff --git a/tests/dialects/test_postgres.py b/tests/dialects/test_postgres.py
index 1b1efc52..f35f1437 100644
--- a/tests/dialects/test_postgres.py
+++ b/tests/dialects/test_postgres.py
@@ -833,6 +833,10 @@ class TestPostgres(Validator):
             "/* + some comment */ SELECT b.foo, b.bar FROM baz AS b",
         )
 
+        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)"
+        )
+
     def test_ddl(self):
         # Checks that user-defined types are parsed into DataType instead of Identifier
         self.parse_one("CREATE TABLE t (a udt)").this.expressions[0].args["kind"].assert_is(