Repository: tobymao/sqlglot

Issue:
Transpile from Snowflake to DuckDB LISTAGG with nested DISTINCT
**Before you file an issue**
- Make sure you specify the "read" dialect eg. `parse_one(sql, read="spark")`
- Make sure you specify the "write" dialect eg. `ast.sql(dialect="duckdb")`
- Check if the issue still exists on main
✅

**Fully reproducible code snippet**
Please include a fully reproducible code snippet or the input sql, dialect, and expected output.

```python
from sqlglot import parse_one

sf_query = """
SELECT
    LISTAGG(DISTINCT (RAW_ORDERS."STATUS"), '|SEPARATOR|') WITHIN GROUP (ORDER BY (RAW_ORDERS."STATUS")) AS status_options
FROM
    DEVELOPER.SCHEMA.RAW_ORDERS AS RAW_ORDERS
FETCH NEXT 1 ROWS ONLY
"""

sf_query = parse_one(query, dialect="snowflake")
duckdb_query = sf_query.sql(dialect="duckdb", pretty=True)
```

Expected output
```sql
SELECT
  GROUP_CONCAT( DISTINCT("raw_orders"."STATUS"), '|SEPARATOR|' ORDER BY ("raw_orders"."STATUS")) AS "status_options"
FROM "developer"."schema"."raw_orders" AS "raw_orders"
LIMIT 1
```

Actual output
```sql
SELECT
  GROUP_CONCAT(
    DISTINCT CASE
      WHEN "raw_orders"."STATUS" IS NULL
      THEN NULL
      WHEN '|RECORD|' IS NULL
      THEN NULL
      ELSE (("raw_orders"."STATUS"), '|SEPARATOR|')
    END
  
  ORDER BY
    "raw_orders"."STATUS") AS "status_options"
FROM "developer"."schema"."raw_orders" AS "raw_orders"
LIMIT 1
```

The status column is a varchar column with low cardinality

Happy to add more details as needed.

Hints from issue comments:
@asarama: Awesome! Looking forward to the release being available. Will try to build from source if I get time.

Solution patch:
diff --git a/sqlglot/dialects/bigquery.py b/sqlglot/dialects/bigquery.py
index 09af7db0d9..690530eeea 100644
--- a/sqlglot/dialects/bigquery.py
+++ b/sqlglot/dialects/bigquery.py
@@ -27,6 +27,7 @@
     ts_or_ds_add_cast,
     unit_to_var,
     strposition_sql,
+    groupconcat_sql,
 )
 from sqlglot.helper import seq_get, split_num_words
 from sqlglot.tokens import TokenType
@@ -902,7 +903,9 @@ class Generator(generator.Generator):
                 "DATETIME", self.func("TIMESTAMP", e.this, e.args.get("zone")), "'UTC'"
             ),
             exp.GenerateSeries: rename_func("GENERATE_ARRAY"),
-            exp.GroupConcat: rename_func("STRING_AGG"),
+            exp.GroupConcat: lambda self, e: groupconcat_sql(
+                self, e, func_name="STRING_AGG", within_group=False
+            ),
             exp.Hex: lambda self, e: self.func("UPPER", self.func("TO_HEX", self.sql(e, "this"))),
             exp.HexString: lambda self, e: self.hexstring_sql(e, binary_function_repr="FROM_HEX"),
             exp.If: if_sql(false_value="NULL"),
diff --git a/sqlglot/dialects/dialect.py b/sqlglot/dialects/dialect.py
index 433c23ec5e..72d31bb319 100644
--- a/sqlglot/dialects/dialect.py
+++ b/sqlglot/dialects/dialect.py
@@ -1825,3 +1825,34 @@ def no_make_interval_sql(self: Generator, expression: exp.MakeInterval, sep: str
 def length_or_char_length_sql(self: Generator, expression: exp.Length) -> str:
     length_func = "LENGTH" if expression.args.get("binary") else "CHAR_LENGTH"
     return self.func(length_func, expression.this)
+
+
+def groupconcat_sql(
+    self: Generator,
+    expression: exp.GroupConcat,
+    func_name="LISTAGG",
+    sep: str = ",",
+    within_group: bool = True,
+    on_overflow: bool = False,
+) -> str:
+    this = expression.this
+    separator = expression.args.get("separator") or exp.Literal.string(sep)
+
+    on_overflow_sql = self.sql(expression, "on_overflow")
+    on_overflow_sql = f" ON OVERFLOW {on_overflow_sql}" if (on_overflow and on_overflow_sql) else ""
+
+    order = this.find(exp.Order)
+
+    if order and order.this:
+        this = order.this.pop()
+
+    args = self.format_args(this, f"{separator}{on_overflow_sql}")
+    listagg: exp.Expression = exp.Anonymous(this=func_name, expressions=[args])
+
+    if order:
+        if within_group:
+            listagg = exp.WithinGroup(this=listagg, expression=order)
+        else:
+            listagg.set("expressions", [f"{args}{self.sql(expression=expression.this)}"])
+
+    return self.sql(listagg)
diff --git a/sqlglot/dialects/duckdb.py b/sqlglot/dialects/duckdb.py
index 9e7be7cbed..5a7c20f371 100644
--- a/sqlglot/dialects/duckdb.py
+++ b/sqlglot/dialects/duckdb.py
@@ -36,6 +36,7 @@
     build_regexp_extract,
     explode_to_unnest_sql,
     no_make_interval_sql,
+    groupconcat_sql,
 )
 from sqlglot.generator import unsupported_args
 from sqlglot.helper import seq_get
@@ -427,7 +428,12 @@ class Parser(parser.Parser):
         FUNCTIONS.pop("DATE_SUB")
         FUNCTIONS.pop("GLOB")
 
-        FUNCTION_PARSERS = parser.Parser.FUNCTION_PARSERS.copy()
+        FUNCTION_PARSERS = {
+            **parser.Parser.FUNCTION_PARSERS,
+            **dict.fromkeys(
+                ("GROUP_CONCAT", "LISTAGG", "STRINGAGG"), lambda self: self._parse_string_agg()
+            ),
+        }
         FUNCTION_PARSERS.pop("DECODE")
 
         NO_PAREN_FUNCTION_PARSERS = {
@@ -583,6 +589,7 @@ class Generator(generator.Generator):
             exp.Encode: lambda self, e: encode_decode_sql(self, e, "ENCODE", replace=False),
             exp.GenerateDateArray: _generate_datetime_array_sql,
             exp.GenerateTimestampArray: _generate_datetime_array_sql,
+            exp.GroupConcat: lambda self, e: groupconcat_sql(self, e, within_group=False),
             exp.HexString: lambda self, e: self.hexstring_sql(e, binary_function_repr="FROM_HEX"),
             exp.Explode: rename_func("UNNEST"),
             exp.IntDiv: lambda self, e: self.binary(e, "//"),
diff --git a/sqlglot/dialects/postgres.py b/sqlglot/dialects/postgres.py
index 9ea293bfc5..e19fee35d2 100644
--- a/sqlglot/dialects/postgres.py
+++ b/sqlglot/dialects/postgres.py
@@ -34,6 +34,7 @@
     ts_or_ds_add_cast,
     strposition_sql,
     count_if_to_sum,
+    groupconcat_sql,
 )
 from sqlglot.generator import unsupported_args
 from sqlglot.helper import is_int, seq_get
@@ -109,19 +110,6 @@ def _substring_sql(self: Postgres.Generator, expression: exp.Substring) -> str:
     return f"SUBSTRING({this}{from_part}{for_part})"
 
 
-def _string_agg_sql(self: Postgres.Generator, expression: exp.GroupConcat) -> str:
-    separator = expression.args.get("separator") or exp.Literal.string(",")
-
-    order = ""
-    this = expression.this
-    if isinstance(this, exp.Order):
-        if this.this:
-            this = this.this.pop()
-        order = self.sql(expression.this)  # Order has a leading space
-
-    return f"STRING_AGG({self.format_args(this, separator)}{order})"
-
-
 def _auto_increment_to_serial(expression: exp.Expression) -> exp.Expression:
     auto = expression.find(exp.AutoIncrementColumnConstraint)
 
@@ -557,7 +545,9 @@ class Generator(generator.Generator):
             exp.DateSub: _date_add_sql("-"),
             exp.Explode: rename_func("UNNEST"),
             exp.ExplodingGenerateSeries: rename_func("GENERATE_SERIES"),
-            exp.GroupConcat: _string_agg_sql,
+            exp.GroupConcat: lambda self, e: groupconcat_sql(
+                self, e, func_name="STRING_AGG", within_group=False
+            ),
             exp.IntDiv: rename_func("DIV"),
             exp.JSONExtract: _json_extract_sql("JSON_EXTRACT_PATH", "->"),
             exp.JSONExtractScalar: _json_extract_sql("JSON_EXTRACT_PATH_TEXT", "->>"),
diff --git a/sqlglot/dialects/snowflake.py b/sqlglot/dialects/snowflake.py
index 46275ca0d2..eaf1ace02e 100644
--- a/sqlglot/dialects/snowflake.py
+++ b/sqlglot/dialects/snowflake.py
@@ -26,6 +26,7 @@
     strposition_sql,
     timestampdiff_sql,
     no_make_interval_sql,
+    groupconcat_sql,
 )
 from sqlglot.generator import unsupported_args
 from sqlglot.helper import flatten, is_float, is_int, seq_get
@@ -439,7 +440,6 @@ class Parser(parser.Parser):
             ),
             "LEN": lambda args: exp.Length(this=seq_get(args, 0), binary=True),
             "LENGTH": lambda args: exp.Length(this=seq_get(args, 0), binary=True),
-            "LISTAGG": exp.GroupConcat.from_arg_list,
             "NULLIFZERO": _build_if_from_nullifzero,
             "OBJECT_CONSTRUCT": _build_object_construct,
             "REGEXP_EXTRACT_ALL": _build_regexp_extract(exp.RegexpExtractAll),
@@ -482,6 +482,7 @@ class Parser(parser.Parser):
             **parser.Parser.FUNCTION_PARSERS,
             "DATE_PART": lambda self: self._parse_date_part(),
             "OBJECT_CONSTRUCT_KEEP_NULL": lambda self: self._parse_json_object(),
+            "LISTAGG": lambda self: self._parse_string_agg(),
         }
         FUNCTION_PARSERS.pop("TRIM")
 
@@ -950,7 +951,7 @@ class Generator(generator.Generator):
             exp.GenerateSeries: lambda self, e: self.func(
                 "ARRAY_GENERATE_RANGE", e.args["start"], e.args["end"] + 1, e.args.get("step")
             ),
-            exp.GroupConcat: rename_func("LISTAGG"),
+            exp.GroupConcat: lambda self, e: groupconcat_sql(self, e, sep=""),
             exp.If: if_sql(name="IFF", false_value="NULL"),
             exp.JSONExtractArray: _json_extract_value_array_sql,
             exp.JSONExtractScalar: lambda self, e: self.func(
diff --git a/sqlglot/dialects/trino.py b/sqlglot/dialects/trino.py
index 9502e9cd1a..f73ed60d86 100644
--- a/sqlglot/dialects/trino.py
+++ b/sqlglot/dialects/trino.py
@@ -1,7 +1,12 @@
 from __future__ import annotations
 
 from sqlglot import exp, parser
-from sqlglot.dialects.dialect import merge_without_target_sql, trim_sql, timestrtotime_sql
+from sqlglot.dialects.dialect import (
+    merge_without_target_sql,
+    trim_sql,
+    timestrtotime_sql,
+    groupconcat_sql,
+)
 from sqlglot.dialects.presto import Presto
 from sqlglot.tokens import TokenType
 import typing as t
@@ -67,6 +72,7 @@ class Generator(Presto.Generator):
             exp.ArraySum: lambda self,
             e: f"REDUCE({self.sql(e, 'this')}, 0, (acc, x) -> acc + x, acc -> acc)",
             exp.ArrayUniqueAgg: lambda self, e: f"ARRAY_AGG(DISTINCT {self.sql(e, 'this')})",
+            exp.GroupConcat: lambda self, e: groupconcat_sql(self, e, on_overflow=True),
             exp.LocationProperty: lambda self, e: self.property_sql(e),
             exp.Merge: merge_without_target_sql,
             exp.TimeStrToTime: lambda self, e: timestrtotime_sql(self, e, include_precision=True),
@@ -98,17 +104,3 @@ def jsonextract_sql(self, expression: exp.JSONExtract) -> str:
                 expression.this,
                 json_path + option + quote + on_condition,
             )
-
-        def groupconcat_sql(self, expression: exp.GroupConcat) -> str:
-            this = expression.this
-            separator = expression.args.get("separator") or exp.Literal.string(",")
-
-            if isinstance(this, exp.Order):
-                if this.this:
-                    this = this.this.pop()
-
-                on_overflow = self.sql(expression, "on_overflow")
-                on_overflow = f" ON OVERFLOW {on_overflow}" if on_overflow else ""
-                return f"LISTAGG({self.format_args(this, separator)}{on_overflow}) WITHIN GROUP ({self.sql(expression.this).lstrip()})"
-
-            return super().groupconcat_sql(expression)
diff --git a/sqlglot/parser.py b/sqlglot/parser.py
index 2b3d16e78a..5f953c965c 100644
--- a/sqlglot/parser.py
+++ b/sqlglot/parser.py
@@ -6254,7 +6254,8 @@ def _parse_string_agg(self) -> exp.GroupConcat:
         if not self._match(TokenType.R_PAREN) and args:
             # postgres: STRING_AGG([DISTINCT] expression, separator [ORDER BY expression1 {ASC | DESC} [, ...]])
             # bigquery: STRING_AGG([DISTINCT] expression [, separator] [ORDER BY key [{ASC | DESC}] [, ... ]] [LIMIT n])
-            args[-1] = self._parse_limit(this=self._parse_order(this=args[-1]))
+            # The order is parsed through `this` as a canonicalization for WITHIN GROUPs
+            args[0] = self._parse_limit(this=self._parse_order(this=args[0]))
             return self.expression(exp.GroupConcat, this=args[0], separator=seq_get(args, 1))
 
         # Checks if we can parse an order clause: WITHIN GROUP (ORDER BY <order_by_expression_list> [ASC | DESC]).


Test patch:
diff --git a/tests/dialects/test_bigquery.py b/tests/dialects/test_bigquery.py
index 9915a5a573..ec0c5f2975 100644
--- a/tests/dialects/test_bigquery.py
+++ b/tests/dialects/test_bigquery.py
@@ -142,7 +142,6 @@ def test_bigquery(self):
         self.validate_identity("SELECT test.Unknown FROM test")
         self.validate_identity(r"SELECT '\n\r\a\v\f\t'")
         self.validate_identity("SELECT * FROM tbl FOR SYSTEM_TIME AS OF z")
-        self.validate_identity("STRING_AGG(DISTINCT a ORDER BY b DESC, c DESC LIMIT 10)")
         self.validate_identity("SELECT PARSE_TIMESTAMP('%c', 'Thu Dec 25 07:30:00 2008', 'UTC')")
         self.validate_identity("SELECT ANY_VALUE(fruit HAVING MAX sold) FROM fruits")
         self.validate_identity("SELECT ANY_VALUE(fruit HAVING MIN sold) FROM fruits")
@@ -150,10 +149,6 @@ def test_bigquery(self):
         self.validate_identity("SELECT CAST(CURRENT_DATE AS STRING FORMAT 'DAY') AS current_day")
         self.validate_identity("SAFE_CAST(encrypted_value AS STRING FORMAT 'BASE64')")
         self.validate_identity("CAST(encrypted_value AS STRING FORMAT 'BASE64')")
-        self.validate_identity("STRING_AGG(a)")
-        self.validate_identity("STRING_AGG(a, ' & ')")
-        self.validate_identity("STRING_AGG(DISTINCT a, ' & ')")
-        self.validate_identity("STRING_AGG(a, ' & ' ORDER BY LENGTH(a))")
         self.validate_identity("DATE(2016, 12, 25)")
         self.validate_identity("DATE(CAST('2016-12-25 23:59:59' AS DATETIME))")
         self.validate_identity("SELECT foo IN UNNEST(bar) AS bla")
@@ -1844,12 +1839,6 @@ def test_user_defined_functions(self):
             "CREATE TEMPORARY FUNCTION string_length_0(strings ARRAY<STRING>) RETURNS FLOAT64 LANGUAGE js OPTIONS (library=['gs://ibis-testing-libraries/lodash.min.js']) AS '\\'use strict\\'; function string_length(strings) { return _.sum(_.map(strings, ((x) => x.length))); } return string_length(strings);'",
         )
 
-    def test_group_concat(self):
-        self.validate_all(
-            "SELECT a, GROUP_CONCAT(b) FROM table GROUP BY a",
-            write={"bigquery": "SELECT a, STRING_AGG(b) FROM table GROUP BY a"},
-        )
-
     def test_remove_precision_parameterized_types(self):
         self.validate_identity("CREATE TABLE test (a NUMERIC(10, 2))")
         self.validate_identity(
@@ -2347,3 +2336,19 @@ def test_format_temporal(self):
                 "snowflake": "SELECT TO_CHAR(CAST(CAST('2050-12-25 15:30:55+00' AS TIMESTAMPTZ) AS TIMESTAMP), 'mon-DD-yyyy')",
             },
         )
+
+    def test_string_agg(self):
+        self.validate_identity(
+            "SELECT a, GROUP_CONCAT(b) FROM table GROUP BY a",
+            "SELECT a, STRING_AGG(b, ',') FROM table GROUP BY a",
+        )
+
+        self.validate_identity("STRING_AGG(a, ' & ')")
+        self.validate_identity("STRING_AGG(DISTINCT a, ' & ')")
+        self.validate_identity("STRING_AGG(a, ' & ' ORDER BY LENGTH(a))")
+
+        self.validate_identity("STRING_AGG(a)", "STRING_AGG(a, ',')")
+        self.validate_identity(
+            "STRING_AGG(DISTINCT a ORDER BY b DESC, c DESC LIMIT 10)",
+            "STRING_AGG(DISTINCT a, ',' ORDER BY b DESC, c DESC LIMIT 10)",
+        )
diff --git a/tests/dialects/test_redshift.py b/tests/dialects/test_redshift.py
index d84d69ba59..47be0f71ea 100644
--- a/tests/dialects/test_redshift.py
+++ b/tests/dialects/test_redshift.py
@@ -42,8 +42,8 @@ def test_redshift(self):
                 "duckdb": "STRING_AGG(sellerid, ', ')",
             },
             write={
-                # GROUP_CONCAT and STRING_AGG are aliases in DuckDB
-                "duckdb": "GROUP_CONCAT(sellerid, ', ')",
+                # GROUP_CONCAT, LISTAGG and STRING_AGG are aliases in DuckDB
+                "duckdb": "LISTAGG(sellerid, ', ')",
                 "redshift": "LISTAGG(sellerid, ', ')",
             },
         )
diff --git a/tests/dialects/test_snowflake.py b/tests/dialects/test_snowflake.py
index 4d92c75fa2..77872efec0 100644
--- a/tests/dialects/test_snowflake.py
+++ b/tests/dialects/test_snowflake.py
@@ -52,7 +52,6 @@ def test_snowflake(self):
         self.validate_identity("SELECT OBJECT_CONSTRUCT()")
         self.validate_identity("SELECT DAYOFMONTH(CURRENT_TIMESTAMP())")
         self.validate_identity("SELECT DAYOFYEAR(CURRENT_TIMESTAMP())")
-        self.validate_identity("LISTAGG(data['some_field'], ',')")
         self.validate_identity("WEEKOFYEAR(tstamp)")
         self.validate_identity("SELECT QUARTER(CURRENT_TIMESTAMP())")
         self.validate_identity("SELECT SUM(amount) FROM mytable GROUP BY ALL")
@@ -852,13 +851,6 @@ def test_snowflake(self):
                 "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",
-            },
-        )
         self.validate_all(
             "SELECT APPROX_PERCENTILE(a, 0.5) FROM t",
             read={
@@ -2450,3 +2442,20 @@ def test_offset_without_limit(self):
                 "trino": "SELECT 1 ORDER BY 1 OFFSET 0",
             },
         )
+
+    def test_listagg(self):
+        self.validate_identity("LISTAGG(data['some_field'], ',')")
+
+        for distinct in ("", "DISTINCT "):
+            self.validate_all(
+                f"SELECT LISTAGG({distinct}col, '|SEPARATOR|') WITHIN GROUP (ORDER BY col2) FROM t",
+                read={
+                    "trino": f"SELECT LISTAGG({distinct}col, '|SEPARATOR|') WITHIN GROUP (ORDER BY col2) FROM t",
+                    "duckdb": f"SELECT LISTAGG({distinct}col, '|SEPARATOR|' ORDER BY col2) FROM t",
+                },
+                write={
+                    "snowflake": f"SELECT LISTAGG({distinct}col, '|SEPARATOR|') WITHIN GROUP (ORDER BY col2) FROM t",
+                    "trino": f"SELECT LISTAGG({distinct}col, '|SEPARATOR|') WITHIN GROUP (ORDER BY col2) FROM t",
+                    "duckdb": f"SELECT LISTAGG({distinct}col, '|SEPARATOR|' ORDER BY col2) FROM t",
+                },
+            )