Repository: tobymao/sqlglot

Issue:
Issue in generating SQL for AST having LISTAGG function for Trino SQL
If we have a Trino SQL such as - 

```sql
SELECT LISTAGG(DISTINCT "tbl1_1"."id", ',') WITHIN GROUP (ORDER BY "tbl1_1"."id" ASC)
            FROM table_a "tbl1_1"
```

when we parse it and convert it back to SQL, the SQL being returned is (instead of the original one) - 

```sql
SELECT LISTAGG(DISTINCT CASE WHEN "tbl1_1"."id" IS NULL THEN NULL WHEN ',' IS NULL THEN NULL ELSE ("tbl1_1"."id", ',') END) WITHIN GROUP (ORDER BY "tbl1_1"."id" ASC) FROM table_a AS "tbl1_1"
```

The documentation for `LISTAGG` is [here](https://trino.io/docs/current/functions/aggregate.html#listagg).

Snippet for reproducing the issue - 

```python
from sqlglot import parse

if __name__ == '__main__':
    sql_str = """
    SELECT LISTAGG(DISTINCT "tbl1_1"."id", ',') WITHIN GROUP (ORDER BY "tbl1_1"."id" ASC)
            FROM table_a "tbl1_1"
    """
    expression_tree = parse(sql_str, read="trino")
    print(repr(expression_tree))
    print(expression_tree[0].sql(dialect="trino"))
```

Discussion:

Solution patch:
diff --git a/sqlglot/dialects/oracle.py b/sqlglot/dialects/oracle.py
index 81c2a4a5c3..982907d077 100644
--- a/sqlglot/dialects/oracle.py
+++ b/sqlglot/dialects/oracle.py
@@ -15,6 +15,7 @@
 from sqlglot.helper import seq_get
 from sqlglot.parser import OPTIONS_TYPE, build_coalesce
 from sqlglot.tokens import TokenType
+from sqlglot.errors import ParseError
 
 if t.TYPE_CHECKING:
     from sqlglot._typing import E
@@ -205,6 +206,57 @@ def _parse_json_array(self, expr_type: t.Type[E], **kwargs) -> E:
             )
 
         def _parse_hint(self) -> t.Optional[exp.Hint]:
+            start_index = self._index
+            should_fallback_to_string = False
+
+            if not self._match(TokenType.HINT):
+                return None
+
+            hints = []
+
+            try:
+                for hint in iter(
+                    lambda: self._parse_csv(
+                        lambda: self._parse_hint_function_call() or self._parse_var(upper=True),
+                    ),
+                    [],
+                ):
+                    hints.extend(hint)
+            except ParseError:
+                should_fallback_to_string = True
+
+            if not self._match_pair(TokenType.STAR, TokenType.SLASH):
+                should_fallback_to_string = True
+
+            if should_fallback_to_string:
+                self._retreat(start_index)
+                return self._parse_hint_fallback_to_string()
+
+            return self.expression(exp.Hint, expressions=hints)
+
+        def _parse_hint_function_call(self) -> t.Optional[exp.Expression]:
+            if not self._curr or not self._next or self._next.token_type != TokenType.L_PAREN:
+                return None
+
+            this = self._curr.text
+
+            self._advance(2)
+            args = self._parse_hint_args()
+            this = self.expression(exp.Anonymous, this=this, expressions=args)
+            self._match_r_paren(this)
+            return this
+
+        def _parse_hint_args(self):
+            args = []
+            result = self._parse_var()
+
+            while result:
+                args.append(result)
+                result = self._parse_var()
+
+            return args
+
+        def _parse_hint_fallback_to_string(self) -> t.Optional[exp.Hint]:
             if self._match(TokenType.HINT):
                 start = self._curr
                 while self._curr and not self._match_pair(TokenType.STAR, TokenType.SLASH):
@@ -271,6 +323,7 @@ class Generator(generator.Generator):
         LAST_DAY_SUPPORTS_DATE_PART = False
         SUPPORTS_SELECT_INTO = True
         TZ_TO_WITH_TIME_ZONE = True
+        QUERY_HINT_SEP = " "
 
         TYPE_MAPPING = {
             **generator.Generator.TYPE_MAPPING,
@@ -370,3 +423,15 @@ def into_sql(self, expression: exp.Into) -> str:
                 return f"{self.seg(into)} {self.sql(expression, 'this')}"
 
             return f"{self.seg(into)} {self.expressions(expression)}"
+
+        def hint_sql(self, expression: exp.Hint) -> str:
+            expressions = []
+
+            for expression in expression.expressions:
+                if isinstance(expression, exp.Anonymous):
+                    formatted_args = self.format_args(*expression.expressions, sep=" ")
+                    expressions.append(f"{self.sql(expression, 'this')}({formatted_args})")
+                else:
+                    expressions.append(self.sql(expression))
+
+            return f" /*+ {self.expressions(sqls=expressions, sep=self.QUERY_HINT_SEP).strip()} */"
diff --git a/sqlglot/dialects/presto.py b/sqlglot/dialects/presto.py
index 09607ada14..f41e8b4926 100644
--- a/sqlglot/dialects/presto.py
+++ b/sqlglot/dialects/presto.py
@@ -386,9 +386,6 @@ class Generator(generator.Generator):
             exp.GenerateSeries: sequence_sql,
             exp.GenerateDateArray: sequence_sql,
             exp.Group: transforms.preprocess([transforms.unalias_group]),
-            exp.GroupConcat: lambda self, e: self.func(
-                "ARRAY_JOIN", self.func("ARRAY_AGG", e.this), e.args.get("separator")
-            ),
             exp.If: if_sql(),
             exp.ILike: no_ilike_sql,
             exp.Initcap: _initcap_sql,
@@ -680,3 +677,10 @@ def jsonextract_sql(self, expression: exp.JSONExtract) -> str:
             expr = "".join(segments)
 
             return f"{this}{expr}"
+
+        def groupconcat_sql(self, expression: exp.GroupConcat) -> str:
+            return self.func(
+                "ARRAY_JOIN",
+                self.func("ARRAY_AGG", expression.this),
+                expression.args.get("separator"),
+            )
diff --git a/sqlglot/dialects/snowflake.py b/sqlglot/dialects/snowflake.py
index bc1a3154de..867850c540 100644
--- a/sqlglot/dialects/snowflake.py
+++ b/sqlglot/dialects/snowflake.py
@@ -384,7 +384,6 @@ class Parser(parser.Parser):
                 expressions=self._parse_csv(self._parse_id_var),
                 unset=True,
             ),
-            "SWAP": lambda self: self._parse_alter_table_swap(),
         }
 
         STATEMENT_PARSERS = {
@@ -654,10 +653,6 @@ def _parse_show_snowflake(self, this: str) -> exp.Show:
                 },
             )
 
-        def _parse_alter_table_swap(self) -> exp.SwapTable:
-            self._match_text_seq("WITH")
-            return self.expression(exp.SwapTable, this=self._parse_table(schema=True))
-
         def _parse_location_property(self) -> exp.LocationProperty:
             self._match(TokenType.EQ)
             return self.expression(exp.LocationProperty, this=self._parse_location_path())
@@ -1036,10 +1031,6 @@ def generatedasidentitycolumnconstraint_sql(
             increment = f" INCREMENT {increment}" if increment else ""
             return f"AUTOINCREMENT{start}{increment}"
 
-        def swaptable_sql(self, expression: exp.SwapTable) -> str:
-            this = self.sql(expression, "this")
-            return f"SWAP WITH {this}"
-
         def cluster_sql(self, expression: exp.Cluster) -> str:
             return f"CLUSTER BY ({self.expressions(expression, flat=True)})"
 
diff --git a/sqlglot/dialects/trino.py b/sqlglot/dialects/trino.py
index 290fc51c6b..e57f107b64 100644
--- a/sqlglot/dialects/trino.py
+++ b/sqlglot/dialects/trino.py
@@ -10,11 +10,15 @@ class Trino(Presto):
     SUPPORTS_USER_DEFINED_TYPES = False
     LOG_BASE_FIRST = True
 
+    class Tokenizer(Presto.Tokenizer):
+        HEX_STRINGS = [("X'", "'")]
+
     class Parser(Presto.Parser):
         FUNCTION_PARSERS = {
             **Presto.Parser.FUNCTION_PARSERS,
             "TRIM": lambda self: self._parse_trim(),
             "JSON_QUERY": lambda self: self._parse_json_query(),
+            "LISTAGG": lambda self: self._parse_string_agg(),
         }
 
         JSON_QUERY_OPTIONS: parser.OPTIONS_TYPE = {
@@ -65,5 +69,14 @@ def jsonextract_sql(self, expression: exp.JSONExtract) -> str:
 
             return self.func("JSON_QUERY", expression.this, json_path + option)
 
-    class Tokenizer(Presto.Tokenizer):
-        HEX_STRINGS = [("X'", "'")]
+        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()
+
+                return f"LISTAGG({self.format_args(this, separator)}) WITHIN GROUP ({self.sql(expression.this).lstrip()})"
+
+            return super().groupconcat_sql(expression)
diff --git a/sqlglot/expressions.py b/sqlglot/expressions.py
index 7f44c417ef..b446353c2b 100644
--- a/sqlglot/expressions.py
+++ b/sqlglot/expressions.py
@@ -7422,15 +7422,9 @@ def to_interval(interval: str | Literal) -> Interval:
 
         interval = interval.this
 
-    interval_parts = INTERVAL_STRING_RE.match(interval)  # type: ignore
-
-    if not interval_parts:
-        raise ValueError("Invalid interval string.")
-
-    return Interval(
-        this=Literal.string(interval_parts.group(1)),
-        unit=Var(this=interval_parts.group(2).upper()),
-    )
+    interval = maybe_parse(f"INTERVAL {interval}")
+    assert isinstance(interval, Interval)
+    return interval
 
 
 def to_table(
diff --git a/sqlglot/generator.py b/sqlglot/generator.py
index 20f7b1b07e..7a1942b181 100644
--- a/sqlglot/generator.py
+++ b/sqlglot/generator.py
@@ -185,6 +185,7 @@ class Generator(metaclass=_Generator):
         exp.Stream: lambda self, e: f"STREAM {self.sql(e, 'this')}",
         exp.StreamingTableProperty: lambda *_: "STREAMING",
         exp.StrictProperty: lambda *_: "STRICT",
+        exp.SwapTable: lambda self, e: f"SWAP WITH {self.sql(e, 'this')}",
         exp.TemporaryProperty: lambda *_: "TEMPORARY",
         exp.TagColumnConstraint: lambda self, e: f"TAG ({self.expressions(e, flat=True)})",
         exp.TitleColumnConstraint: lambda self, e: f"TITLE {self.sql(e, 'this')}",
@@ -3515,13 +3516,15 @@ def func(
         name = self.normalize_func(name) if normalize else name
         return f"{name}{prefix}{self.format_args(*args)}{suffix}"
 
-    def format_args(self, *args: t.Optional[str | exp.Expression]) -> str:
+    def format_args(self, *args: t.Optional[str | exp.Expression], sep: str = ", ") -> str:
         arg_sqls = tuple(
             self.sql(arg) for arg in args if arg is not None and not isinstance(arg, bool)
         )
         if self.pretty and self.too_wide(arg_sqls):
-            return self.indent("\n" + ",\n".join(arg_sqls) + "\n", skip_first=True, skip_last=True)
-        return ", ".join(arg_sqls)
+            return self.indent(
+                "\n" + f"{sep.strip()}\n".join(arg_sqls) + "\n", skip_first=True, skip_last=True
+            )
+        return sep.join(arg_sqls)
 
     def too_wide(self, args: t.Iterable) -> bool:
         return sum(len(arg) for arg in args) > self.max_text_width
diff --git a/sqlglot/parser.py b/sqlglot/parser.py
index b84623f226..bbd6d1cce8 100644
--- a/sqlglot/parser.py
+++ b/sqlglot/parser.py
@@ -1053,13 +1053,16 @@ class Parser(metaclass=_Parser):
 
     ALTER_PARSERS = {
         "ADD": lambda self: self._parse_alter_table_add(),
+        "AS": lambda self: self._parse_select(),
         "ALTER": lambda self: self._parse_alter_table_alter(),
         "CLUSTER BY": lambda self: self._parse_cluster(wrapped=True),
         "DELETE": lambda self: self.expression(exp.Delete, where=self._parse_where()),
         "DROP": lambda self: self._parse_alter_table_drop(),
         "RENAME": lambda self: self._parse_alter_table_rename(),
         "SET": lambda self: self._parse_alter_table_set(),
-        "AS": lambda self: self._parse_select(),
+        "SWAP": lambda self: self.expression(
+            exp.SwapTable, this=self._match(TokenType.WITH) and self._parse_table(schema=True)
+        ),
     }
 
     ALTER_ALTER_PARSERS = {


Test patch:
diff --git a/tests/dialects/test_oracle.py b/tests/dialects/test_oracle.py
index d2bbedcde5..36ce5d02e6 100644
--- a/tests/dialects/test_oracle.py
+++ b/tests/dialects/test_oracle.py
@@ -329,6 +329,57 @@ def test_hints(self):
         )
         self.validate_identity("INSERT /*+ APPEND */ INTO IAP_TBL (id, col1) VALUES (2, 'test2')")
         self.validate_identity("INSERT /*+ APPEND_VALUES */ INTO dest_table VALUES (i, 'Value')")
+        self.validate_identity(
+            "SELECT /*+ LEADING(departments employees) USE_NL(employees) */ * FROM employees JOIN departments ON employees.department_id = departments.department_id",
+            """SELECT /*+ LEADING(departments employees)
+  USE_NL(employees) */
+  *
+FROM employees
+JOIN departments
+  ON employees.department_id = departments.department_id""",
+            pretty=True,
+        )
+        self.validate_identity(
+            "SELECT /*+ USE_NL(bbbbbbbbbbbbbbbbbbbbbbbb) LEADING(aaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbb cccccccccccccccccccccccc dddddddddddddddddddddddd) INDEX(cccccccccccccccccccccccc) */ * FROM aaaaaaaaaaaaaaaaaaaaaaaa JOIN bbbbbbbbbbbbbbbbbbbbbbbb ON aaaaaaaaaaaaaaaaaaaaaaaa.id = bbbbbbbbbbbbbbbbbbbbbbbb.a_id JOIN cccccccccccccccccccccccc ON bbbbbbbbbbbbbbbbbbbbbbbb.id = cccccccccccccccccccccccc.b_id JOIN dddddddddddddddddddddddd ON cccccccccccccccccccccccc.id = dddddddddddddddddddddddd.c_id",
+        )
+        self.validate_identity(
+            "SELECT /*+ USE_NL(bbbbbbbbbbbbbbbbbbbbbbbb) LEADING(aaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbb cccccccccccccccccccccccc dddddddddddddddddddddddd) INDEX(cccccccccccccccccccccccc) */ * FROM aaaaaaaaaaaaaaaaaaaaaaaa JOIN bbbbbbbbbbbbbbbbbbbbbbbb ON aaaaaaaaaaaaaaaaaaaaaaaa.id = bbbbbbbbbbbbbbbbbbbbbbbb.a_id JOIN cccccccccccccccccccccccc ON bbbbbbbbbbbbbbbbbbbbbbbb.id = cccccccccccccccccccccccc.b_id JOIN dddddddddddddddddddddddd ON cccccccccccccccccccccccc.id = dddddddddddddddddddddddd.c_id",
+            """SELECT /*+ USE_NL(bbbbbbbbbbbbbbbbbbbbbbbb)
+  LEADING(
+    aaaaaaaaaaaaaaaaaaaaaaaa
+    bbbbbbbbbbbbbbbbbbbbbbbb
+    cccccccccccccccccccccccc
+    dddddddddddddddddddddddd
+  )
+  INDEX(cccccccccccccccccccccccc) */
+  *
+FROM aaaaaaaaaaaaaaaaaaaaaaaa
+JOIN bbbbbbbbbbbbbbbbbbbbbbbb
+  ON aaaaaaaaaaaaaaaaaaaaaaaa.id = bbbbbbbbbbbbbbbbbbbbbbbb.a_id
+JOIN cccccccccccccccccccccccc
+  ON bbbbbbbbbbbbbbbbbbbbbbbb.id = cccccccccccccccccccccccc.b_id
+JOIN dddddddddddddddddddddddd
+  ON cccccccccccccccccccccccc.id = dddddddddddddddddddddddd.c_id""",
+            pretty=True,
+        )
+        # Test that parsing error with keywords like select where etc falls back
+        self.validate_identity(
+            "SELECT /*+ LEADING(departments employees) USE_NL(employees) select where group by is order by */ * FROM employees JOIN departments ON employees.department_id = departments.department_id",
+            """SELECT /*+ LEADING(departments employees) USE_NL(employees) select where group by is order by */
+  *
+FROM employees
+JOIN departments
+  ON employees.department_id = departments.department_id""",
+            pretty=True,
+        )
+        # Test that parsing error with , inside hint function falls back
+        self.validate_identity(
+            "SELECT /*+ LEADING(departments, employees) */ * FROM employees JOIN departments ON employees.department_id = departments.department_id"
+        )
+        # Test that parsing error with keyword inside hint function falls back
+        self.validate_identity(
+            "SELECT /*+ LEADING(departments select) */ * FROM employees JOIN departments ON employees.department_id = departments.department_id"
+        )
 
     def test_xml_table(self):
         self.validate_identity("XMLTABLE('x')")
diff --git a/tests/dialects/test_starrocks.py b/tests/dialects/test_starrocks.py
index 44c54a6ee9..1edd7c6fd3 100644
--- a/tests/dialects/test_starrocks.py
+++ b/tests/dialects/test_starrocks.py
@@ -5,6 +5,9 @@
 class TestStarrocks(Validator):
     dialect = "starrocks"
 
+    def test_starrocks(self):
+        self.validate_identity("ALTER TABLE a SWAP WITH b")
+
     def test_ddl(self):
         ddl_sqls = [
             "DISTRIBUTED BY HASH (col1) BUCKETS 1",
diff --git a/tests/dialects/test_trino.py b/tests/dialects/test_trino.py
index 8c73ec1c84..33a02290ac 100644
--- a/tests/dialects/test_trino.py
+++ b/tests/dialects/test_trino.py
@@ -9,6 +9,9 @@ def test_trino(self):
         self.validate_identity("JSON_QUERY(content, 'lax $.HY.*')")
         self.validate_identity("JSON_QUERY(content, 'strict $.HY.*' WITH UNCONDITIONAL WRAPPER)")
         self.validate_identity("JSON_QUERY(content, 'strict $.HY.*' WITHOUT CONDITIONAL WRAPPER)")
+        self.validate_identity(
+            "SELECT LISTAGG(DISTINCT col, ',') WITHIN GROUP (ORDER BY col ASC) FROM tbl"
+        )
 
     def test_trim(self):
         self.validate_identity("SELECT TRIM('!' FROM '!foo!')")
diff --git a/tests/test_expressions.py b/tests/test_expressions.py
index 1c88952b94..8ff117a78c 100644
--- a/tests/test_expressions.py
+++ b/tests/test_expressions.py
@@ -935,15 +935,13 @@ def test_comment_alias(self):
     def test_to_interval(self):
         self.assertEqual(exp.to_interval("1day").sql(), "INTERVAL '1' DAY")
         self.assertEqual(exp.to_interval("  5     months").sql(), "INTERVAL '5' MONTHS")
-        with self.assertRaises(ValueError):
-            exp.to_interval("bla")
+        self.assertEqual(exp.to_interval("-2 day").sql(), "INTERVAL '-2' DAY")
 
         self.assertEqual(exp.to_interval(exp.Literal.string("1day")).sql(), "INTERVAL '1' DAY")
+        self.assertEqual(exp.to_interval(exp.Literal.string("-2 day")).sql(), "INTERVAL '-2' DAY")
         self.assertEqual(
             exp.to_interval(exp.Literal.string("  5   months")).sql(), "INTERVAL '5' MONTHS"
         )
-        with self.assertRaises(ValueError):
-            exp.to_interval(exp.Literal.string("bla"))
 
     def test_to_table(self):
         table_only = exp.to_table("table_name")