Repository: tobymao/sqlglot

Issue:
snowflake -> duckdb ordered listagg transpilation error
DuckDB doesn't use the `WITHIN GROUP` syntax to order a listagg output. Its instead added into the function call like `listagg(col, sep ORDER BY col2)`

**Fully reproducible code snippet**

```{python}
snowsql = """
    select
        col0,
        listagg(col1, ', ') within group (order by col2)
    from (values
        (1, 'a', 3),
        (2, 'b', 2),
        (3, 'c', 1)
    )
    group by col0
"""
ducksql = sqlglot.transpile(snowsql, read='snowflake', write='duckdb')[0]
print(ducksql)
```
yields
```{sql}
SELECT col0, GROUP_CONCAT(col1, ', ') WITHIN GROUP (ORDER BY col2) FROM (VALUES (1, 'a', 3), (2, 'b', 2), (3, 'c', 1)) GROUP BY col0
```

which throws `ParserException: Parser Error: Unknown ordered aggregate "GROUP_CONCAT".`

I would expect the transpiler to output this:
```{sql}
select
    col0,
    group_concat(col1, ', ' order by col2)
from (values
    (1, 'a', 3),
    (1, 'b', 2),
    (1, 'c', 1)
)
group by col0
```



**Official Documentation**

https://duckdb.org/docs/sql/aggregates
https://docs.snowflake.com/en/sql-reference/functions/listagg


Discussion:

Patch:
diff --git a/sqlglot/dialects/duckdb.py b/sqlglot/dialects/duckdb.py
index 00ce1a0c..9f548266 100644
--- a/sqlglot/dialects/duckdb.py
+++ b/sqlglot/dialects/duckdb.py
@@ -28,7 +28,7 @@ from sqlglot.dialects.dialect import (
     timestrtotime_sql,
     unit_to_var,
 )
-from sqlglot.helper import flatten, seq_get
+from sqlglot.helper import seq_get
 from sqlglot.tokens import TokenType
 
 
@@ -155,16 +155,6 @@ def _unix_to_time_sql(self: DuckDB.Generator, expression: exp.UnixToTime) -> str
     return self.func("TO_TIMESTAMP", exp.Div(this=timestamp, expression=exp.func("POW", 10, scale)))
 
 
-def _rename_unless_within_group(
-    a: str, b: str
-) -> t.Callable[[DuckDB.Generator, exp.Expression], str]:
-    return lambda self, expression: (
-        self.func(a, *flatten(expression.args.values()))
-        if isinstance(expression.find_ancestor(exp.Select, exp.WithinGroup), exp.WithinGroup)
-        else self.func(b, *flatten(expression.args.values()))
-    )
-
-
 class DuckDB(Dialect):
     NULL_ORDERING = "nulls_are_last"
     SUPPORTS_USER_DEFINED_TYPES = False
@@ -425,8 +415,8 @@ class DuckDB(Dialect):
                 exp.cast(e.this, exp.DataType.Type.TIMESTAMP, copy=True),
             ),
             exp.ParseJSON: rename_func("JSON"),
-            exp.PercentileCont: _rename_unless_within_group("PERCENTILE_CONT", "QUANTILE_CONT"),
-            exp.PercentileDisc: _rename_unless_within_group("PERCENTILE_DISC", "QUANTILE_DISC"),
+            exp.PercentileCont: rename_func("QUANTILE_CONT"),
+            exp.PercentileDisc: rename_func("QUANTILE_DISC"),
             # DuckDB doesn't allow qualified columns inside of PIVOT expressions.
             # See: https://github.com/duckdb/duckdb/blob/671faf92411182f81dce42ac43de8bfb05d9909e/src/planner/binder/tableref/bind_pivot.cpp#L61-L62
             exp.Pivot: transforms.preprocess([transforms.unqualify_columns]),
@@ -617,3 +607,19 @@ class DuckDB(Dialect):
                     bracket = f"({bracket})[1]"
 
             return bracket
+
+        def withingroup_sql(self, expression: exp.WithinGroup) -> str:
+            expression_sql = self.sql(expression, "expression")
+
+            func = expression.this
+            if isinstance(func, exp.PERCENTILES):
+                # Make the order key the first arg and slide the fraction to the right
+                # https://duckdb.org/docs/sql/aggregates#ordered-set-aggregate-functions
+                order_col = expression.find(exp.Ordered)
+                if order_col:
+                    func.set("expression", func.this)
+                    func.set("this", order_col.this)
+
+            this = self.sql(expression, "this").rstrip(")")
+
+            return f"{this}{expression_sql})"
diff --git a/sqlglot/expressions.py b/sqlglot/expressions.py
index ed010a09..4df2ca4f 100644
--- a/sqlglot/expressions.py
+++ b/sqlglot/expressions.py
@@ -5880,6 +5880,8 @@ FUNCTION_BY_NAME = {name: func for func in ALL_FUNCTIONS for name in func.sql_na
 
 JSON_PATH_PARTS = subclasses(__name__, JSONPathPart, (JSONPathPart,))
 
+PERCENTILES = (PercentileCont, PercentileDisc)
+
 
 # Helpers
 @t.overload
diff --git a/sqlglot/transforms.py b/sqlglot/transforms.py
index a802e878..ec2a0df0 100644
--- a/sqlglot/transforms.py
+++ b/sqlglot/transforms.py
@@ -349,13 +349,10 @@ def explode_to_unnest(index_offset: int = 0) -> t.Callable[[exp.Expression], exp
     return _explode_to_unnest
 
 
-PERCENTILES = (exp.PercentileCont, exp.PercentileDisc)
-
-
 def add_within_group_for_percentiles(expression: exp.Expression) -> exp.Expression:
     """Transforms percentiles by adding a WITHIN GROUP clause to them."""
     if (
-        isinstance(expression, PERCENTILES)
+        isinstance(expression, exp.PERCENTILES)
         and not isinstance(expression.parent, exp.WithinGroup)
         and expression.expression
     ):
@@ -371,7 +368,7 @@ def remove_within_group_for_percentiles(expression: exp.Expression) -> exp.Expre
     """Transforms percentiles by getting rid of their corresponding WITHIN GROUP clause."""
     if (
         isinstance(expression, exp.WithinGroup)
-        and isinstance(expression.this, PERCENTILES)
+        and isinstance(expression.this, exp.PERCENTILES)
         and isinstance(expression.expression, exp.Order)
     ):
         quantile = expression.this.this


Test patch:
diff --git a/tests/dialects/test_duckdb.py b/tests/dialects/test_duckdb.py
index 28b6518b..9105a49a 100644
--- a/tests/dialects/test_duckdb.py
+++ b/tests/dialects/test_duckdb.py
@@ -652,8 +652,14 @@ class TestDuckDB(Validator):
             "SELECT CAST('2020-05-06' AS DATE) + INTERVAL 5 DAY",
             read={"bigquery": "SELECT DATE_ADD(CAST('2020-05-06' AS DATE), INTERVAL 5 DAY)"},
         )
-        self.validate_identity("SELECT PERCENTILE_CONT(0.25) WITHIN GROUP (ORDER BY y DESC) FROM t")
-        self.validate_identity("SELECT PERCENTILE_DISC(0.25) WITHIN GROUP (ORDER BY y DESC) FROM t")
+        self.validate_identity(
+            "SELECT PERCENTILE_CONT(0.25) WITHIN GROUP (ORDER BY y DESC) FROM t",
+            "SELECT QUANTILE_CONT(y, 0.25 ORDER BY y DESC) FROM t",
+        )
+        self.validate_identity(
+            "SELECT PERCENTILE_DISC(0.25) WITHIN GROUP (ORDER BY y DESC) FROM t",
+            "SELECT QUANTILE_DISC(y, 0.25 ORDER BY y DESC) FROM t",
+        )
         self.validate_all(
             "SELECT QUANTILE_CONT(x, q) FROM t",
             write={
diff --git a/tests/dialects/test_snowflake.py b/tests/dialects/test_snowflake.py
index 4ddbed2a..1cbf68c0 100644
--- a/tests/dialects/test_snowflake.py
+++ b/tests/dialects/test_snowflake.py
@@ -456,7 +456,7 @@ WHERE
                 },
                 write={
                     "": f"SELECT PERCENTILE_CONT(0.5) WITHIN GROUP (ORDER BY x NULLS LAST){suffix}",
-                    "duckdb": f"SELECT PERCENTILE_CONT(0.5) WITHIN GROUP (ORDER BY x){suffix}",
+                    "duckdb": f"SELECT QUANTILE_CONT(x, 0.5 ORDER BY x){suffix}",
                     "postgres": f"SELECT PERCENTILE_CONT(0.5) WITHIN GROUP (ORDER BY x){suffix}",
                     "snowflake": f"SELECT PERCENTILE_CONT(0.5) WITHIN GROUP (ORDER BY x){suffix}",
                 },
@@ -821,6 +821,13 @@ WHERE
                 "snowflake": "CASE WHEN x = a OR (x IS NULL AND a IS NULL) THEN b WHEN x = c OR (x IS NULL AND c IS NULL) THEN d ELSE e END",
             },
         )
+        self.validate_all(
+            "SELECT LISTAGG(col1, ', ') WITHIN GROUP (ORDER BY col2) FROM t",
+            write={
+                "duckdb": "SELECT GROUP_CONCAT(col1, ', ' ORDER BY col2) FROM t",
+                "snowflake": "SELECT LISTAGG(col1, ', ') WITHIN GROUP (ORDER BY col2) FROM t",
+            },
+        )
 
     def test_null_treatment(self):
         self.validate_all(