Repository: tobymao/sqlglot

Issue:
Snowflake nested fields are incorrectly translated using `JSON_EXTRACT`
**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**
```
print(sqlglot.transpile("select payload:testField from snowflake_table;", read="snowflake", write="trino", pretty=True)[0])
```
*Output:*
```
SELECT
  JSON_EXTRACT(payload, '$.testField')
FROM snowflake_table
```
*Expected output:*
```
SELECT
  payload.testField
FROM snowflake_table
```
`payload:testField` is how nested fields of VARIANT type are accessed in Snowflake, they should be translated into `payload.testField` for Trino, since they would be or `row` type in Trino and not JSON.
**Official Documentation**
[Querying VARIANT nested fields in Snowflake](https://docs.snowflake.com/en/user-guide/querying-semistructured#traversing-semi-structured-data)
[Trino nested field query example](https://trino.io/blog/2020/08/14/dereference-pushdown.html#example)


Discussion:
VaggelisD: CI tests failed on a snowflake executor test case, taking a look now
georgesittas: PR looks good @VaggelisD, couple of comments only

Patch:
diff --git a/sqlglot/dialects/clickhouse.py b/sqlglot/dialects/clickhouse.py
index 75be9436..75217499 100644
--- a/sqlglot/dialects/clickhouse.py
+++ b/sqlglot/dialects/clickhouse.py
@@ -174,7 +174,6 @@ class ClickHouse(Dialect):
             "DATE_FORMAT": _build_date_format,
             "DATE_SUB": build_date_delta(exp.DateSub, default_unit=None),
             "DATESUB": build_date_delta(exp.DateSub, default_unit=None),
-            "EXTRACT": exp.RegexpExtract.from_arg_list,
             "FORMATDATETIME": _build_date_format,
             "JSONEXTRACTSTRING": build_json_extract_path(
                 exp.JSONExtractScalar, zero_based_indexing=False
@@ -347,7 +346,6 @@ class ClickHouse(Dialect):
             "QUANTILE": lambda self: self._parse_quantile(),
         }
 
-        FUNCTION_PARSERS.pop("EXTRACT")
         FUNCTION_PARSERS.pop("MATCH")
 
         NO_PAREN_FUNCTION_PARSERS = parser.Parser.NO_PAREN_FUNCTION_PARSERS.copy()
@@ -410,6 +408,23 @@ class ClickHouse(Dialect):
             "INDEX",
         }
 
+        def _parse_extract(self) -> exp.Extract | exp.Anonymous:
+            index = self._index
+            this = self._parse_bitwise()
+            if self._match(TokenType.FROM):
+                self._retreat(index)
+                return super()._parse_extract()
+
+            # We return Anonymous here because extract and regexpExtract have different semantics,
+            # so parsing extract(foo, bar) into RegexpExtract can potentially break queries. E.g.,
+            # `extract('foobar', 'b')` works, but CH crashes for `regexpExtract('foobar', 'b')`.
+            #
+            # TODO: can we somehow convert the former into an equivalent `regexpExtract` call?
+            self._match(TokenType.COMMA)
+            return self.expression(
+                exp.Anonymous, this="extract", expressions=[this, self._parse_bitwise()]
+            )
+
         def _parse_assignment(self) -> t.Optional[exp.Expression]:
             this = super()._parse_assignment()
 
diff --git a/sqlglot/dialects/databricks.py b/sqlglot/dialects/databricks.py
index e2628dc9..00f63532 100644
--- a/sqlglot/dialects/databricks.py
+++ b/sqlglot/dialects/databricks.py
@@ -43,7 +43,7 @@ class Databricks(Spark):
     class Parser(Spark.Parser):
         LOG_DEFAULTS_TO_LN = True
         STRICT_CAST = True
-        COLON_IS_JSON_EXTRACT = True
+        COLON_IS_VARIANT_EXTRACT = True
 
         FUNCTIONS = {
             **Spark.Parser.FUNCTIONS,
diff --git a/sqlglot/dialects/dialect.py b/sqlglot/dialects/dialect.py
index 4c394be7..da212869 100644
--- a/sqlglot/dialects/dialect.py
+++ b/sqlglot/dialects/dialect.py
@@ -510,8 +510,28 @@ class Dialect(metaclass=_Dialect):
             return dialect
         if isinstance(dialect, str):
             try:
-                dialect_name, *kv_pairs = dialect.split(",")
-                kwargs = {k.strip(): v.strip() for k, v in (kv.split("=") for kv in kv_pairs)}
+                dialect_name, *kv_strings = dialect.split(",")
+                kv_pairs = (kv.split("=") for kv in kv_strings)
+                kwargs = {}
+                for pair in kv_pairs:
+                    key = pair[0].strip()
+                    value: t.Union[bool | str | None] = None
+
+                    if len(pair) == 1:
+                        # Default initialize standalone settings to True
+                        value = True
+                    elif len(pair) == 2:
+                        value = pair[1].strip()
+
+                        # Coerce the value to boolean if it matches to the truthy/falsy values below
+                        value_lower = value.lower()
+                        if value_lower in ("true", "1"):
+                            value = True
+                        elif value_lower in ("false", "0"):
+                            value = False
+
+                    kwargs[key] = value
+
             except ValueError:
                 raise ValueError(
                     f"Invalid dialect format: '{dialect}'. "
diff --git a/sqlglot/dialects/presto.py b/sqlglot/dialects/presto.py
index 69c1fbec..925af862 100644
--- a/sqlglot/dialects/presto.py
+++ b/sqlglot/dialects/presto.py
@@ -173,6 +173,35 @@ def _unix_to_time_sql(self: Presto.Generator, expression: exp.UnixToTime) -> str
     return f"FROM_UNIXTIME(CAST({timestamp} AS DOUBLE) / POW(10, {scale}))"
 
 
+def _jsonextract_sql(self: Presto.Generator, expression: exp.JSONExtract) -> str:
+    is_json_extract = self.dialect.settings.get("variant_extract_is_json_extract", True)
+
+    # Generate JSON_EXTRACT unless the user has configured that a Snowflake / Databricks
+    # VARIANT extract (e.g. col:x.y) should map to dot notation (i.e ROW access) in Presto/Trino
+    if not expression.args.get("variant_extract") or is_json_extract:
+        return self.func(
+            "JSON_EXTRACT", expression.this, expression.expression, *expression.expressions
+        )
+
+    this = self.sql(expression, "this")
+
+    # Convert the JSONPath extraction `JSON_EXTRACT(col, '$.x.y) to a ROW access col.x.y
+    segments = []
+    for path_key in expression.expression.expressions[1:]:
+        if not isinstance(path_key, exp.JSONPathKey):
+            # Cannot transpile subscripts, wildcards etc to dot notation
+            self.unsupported(f"Cannot transpile JSONPath segment '{path_key}' to ROW access")
+            continue
+        key = path_key.this
+        if not exp.SAFE_IDENTIFIER_RE.match(key):
+            key = f'"{key}"'
+        segments.append(f".{key}")
+
+    expr = "".join(segments)
+
+    return f"{this}{expr}"
+
+
 def _to_int(expression: exp.Expression) -> exp.Expression:
     if not expression.type:
         from sqlglot.optimizer.annotate_types import annotate_types
@@ -390,6 +419,7 @@ class Presto(Dialect):
             exp.If: if_sql(),
             exp.ILike: no_ilike_sql,
             exp.Initcap: _initcap_sql,
+            exp.JSONExtract: _jsonextract_sql,
             exp.Last: _first_last_sql,
             exp.LastValue: _first_last_sql,
             exp.LastDay: lambda self, e: self.func("LAST_DAY_OF_MONTH", e.this),
diff --git a/sqlglot/dialects/snowflake.py b/sqlglot/dialects/snowflake.py
index c7733801..1eab756b 100644
--- a/sqlglot/dialects/snowflake.py
+++ b/sqlglot/dialects/snowflake.py
@@ -241,7 +241,7 @@ class Snowflake(Dialect):
     class Parser(parser.Parser):
         IDENTIFY_PIVOT_STRINGS = True
         DEFAULT_SAMPLING_METHOD = "BERNOULLI"
-        COLON_IS_JSON_EXTRACT = True
+        COLON_IS_VARIANT_EXTRACT = True
 
         ID_VAR_TOKENS = {
             *parser.Parser.ID_VAR_TOKENS,
diff --git a/sqlglot/executor/python.py b/sqlglot/executor/python.py
index 674ef789..5988c4d9 100644
--- a/sqlglot/executor/python.py
+++ b/sqlglot/executor/python.py
@@ -447,6 +447,7 @@ class Python(Dialect):
             exp.Is: lambda self, e: (
                 self.binary(e, "==") if isinstance(e.this, exp.Literal) else self.binary(e, "is")
             ),
+            exp.JSONExtract: lambda self, e: self.func(e.key, e.this, e.expression, *e.expressions),
             exp.JSONPath: lambda self, e: f"[{','.join(self.sql(p) for p in e.expressions[1:])}]",
             exp.JSONPathKey: lambda self, e: f"'{self.sql(e.this)}'",
             exp.JSONPathSubscript: lambda self, e: f"'{e.this}'",
diff --git a/sqlglot/expressions.py b/sqlglot/expressions.py
index 600f57cb..764810dc 100644
--- a/sqlglot/expressions.py
+++ b/sqlglot/expressions.py
@@ -5481,7 +5481,13 @@ class JSONBContains(Binary, Func):
 
 
 class JSONExtract(Binary, Func):
-    arg_types = {"this": True, "expression": True, "only_json_types": False, "expressions": False}
+    arg_types = {
+        "this": True,
+        "expression": True,
+        "only_json_types": False,
+        "expressions": False,
+        "variant_extract": False,
+    }
     _sql_names = ["JSON_EXTRACT"]
     is_var_len_args = True
 
diff --git a/sqlglot/parser.py b/sqlglot/parser.py
index a9118938..d5fe792b 100644
--- a/sqlglot/parser.py
+++ b/sqlglot/parser.py
@@ -1204,8 +1204,8 @@ class Parser(metaclass=_Parser):
     # Whether the -> and ->> operators expect documents of type JSON (e.g. Postgres)
     JSON_ARROWS_REQUIRE_JSON_TYPE = False
 
-    # Whether the `:` operator is used to extract a value from a JSON document
-    COLON_IS_JSON_EXTRACT = False
+    # Whether the `:` operator is used to extract a value from a VARIANT column
+    COLON_IS_VARIANT_EXTRACT = False
 
     # Whether or not a VALUES keyword needs to be followed by '(' to form a VALUES clause.
     # If this is True and '(' is not found, the keyword will be treated as an identifier
@@ -1477,9 +1477,9 @@ class Parser(metaclass=_Parser):
 
     def _try_parse(self, parse_method: t.Callable[[], T], retreat: bool = False) -> t.Optional[T]:
         """
-        Attemps to backtrack if a parse function that contains a try/catch internally raises an error. This behavior can
-        be different depending on the uset-set ErrorLevel, so _try_parse aims to solve this by setting & resetting
-        the parser state accordingly
+        Attemps to backtrack if a parse function that contains a try/catch internally raises an error.
+        This behavior can be different depending on the uset-set ErrorLevel, so _try_parse aims to
+        solve this by setting & resetting the parser state accordingly
         """
         index = self._index
         error_level = self.error_level
@@ -4557,7 +4557,7 @@ class Parser(metaclass=_Parser):
 
         return this
 
-    def _parse_colon_as_json_extract(
+    def _parse_colon_as_variant_extract(
         self, this: t.Optional[exp.Expression]
     ) -> t.Optional[exp.Expression]:
         casts = []
@@ -4590,11 +4590,14 @@ class Parser(metaclass=_Parser):
             if path:
                 json_path.append(self._find_sql(self._tokens[start_index], end_token))
 
+        # The VARIANT extract in Snowflake/Databricks is parsed as a JSONExtract; Snowflake uses the json_path in GET_PATH() while
+        # Databricks transforms it back to the colon/dot notation
         if json_path:
             this = self.expression(
                 exp.JSONExtract,
                 this=this,
                 expression=self.dialect.to_json_path(exp.Literal.string(".".join(json_path))),
+                variant_extract=True,
             )
 
             while casts:
@@ -4651,7 +4654,7 @@ class Parser(metaclass=_Parser):
 
             this = self._parse_bracket(this)
 
-        return self._parse_colon_as_json_extract(this) if self.COLON_IS_JSON_EXTRACT else this
+        return self._parse_colon_as_variant_extract(this) if self.COLON_IS_VARIANT_EXTRACT else this
 
     def _parse_primary(self) -> t.Optional[exp.Expression]:
         if self._match_set(self.PRIMARY_PARSERS):
@@ -5345,7 +5348,7 @@ class Parser(metaclass=_Parser):
             order=self._match(TokenType.OVER) and self._parse_wrapped(self._parse_order),
         )
 
-    def _parse_extract(self) -> exp.Extract:
+    def _parse_extract(self) -> exp.Extract | exp.Anonymous:
         this = self._parse_function() or self._parse_var_or_string(upper=True)
 
         if self._match(TokenType.FROM):


Test patch:
diff --git a/tests/dialects/test_clickhouse.py b/tests/dialects/test_clickhouse.py
index f0452254..ef84d488 100644
--- a/tests/dialects/test_clickhouse.py
+++ b/tests/dialects/test_clickhouse.py
@@ -25,6 +25,7 @@ class TestClickhouse(Validator):
         self.assertEqual(expr.sql(dialect="clickhouse"), "COUNT(x)")
         self.assertIsNone(expr._meta)
 
+        self.validate_identity("SELECT EXTRACT(YEAR FROM toDateTime('2023-02-01'))")
         self.validate_identity("extract(haystack, pattern)")
         self.validate_identity("SELECT * FROM x LIMIT 1 UNION ALL SELECT * FROM y")
         self.validate_identity("SELECT CAST(x AS Tuple(String, Array(Nullable(Float64))))")
diff --git a/tests/dialects/test_dialect.py b/tests/dialects/test_dialect.py
index be262d4a..c0afb2f4 100644
--- a/tests/dialects/test_dialect.py
+++ b/tests/dialects/test_dialect.py
@@ -102,14 +102,10 @@ class TestDialect(Validator):
         lowercase_mysql = Dialect.get_or_raise("mysql, normalization_strategy = lowercase")
         self.assertEqual(lowercase_mysql.normalization_strategy.value, "LOWERCASE")
 
-        with self.assertRaises(ValueError) as cm:
+        with self.assertRaises(AttributeError) as cm:
             Dialect.get_or_raise("mysql, normalization_strategy")
 
-        self.assertEqual(
-            str(cm.exception),
-            "Invalid dialect format: 'mysql, normalization_strategy'. "
-            "Please use the correct format: 'dialect [, k1 = v2 [, ...]]'.",
-        )
+        self.assertEqual(str(cm.exception), "'bool' object has no attribute 'upper'")
 
         with self.assertRaises(ValueError) as cm:
             Dialect.get_or_raise("myqsl")
@@ -127,6 +123,12 @@ class TestDialect(Validator):
         self.assertEqual(oracle_with_settings.normalization_strategy.value, "LOWERCASE")
         self.assertEqual(oracle_with_settings.settings, {"version": "19.5"})
 
+        bool_settings = Dialect.get_or_raise("oracle, s1=TruE, s2=1, s3=FaLse, s4=0, s5=nonbool")
+        self.assertEqual(
+            bool_settings.settings,
+            {"s1": True, "s2": True, "s3": False, "s4": False, "s5": "nonbool"},
+        )
+
     def test_compare_dialects(self):
         bigquery_class = Dialect["bigquery"]
         bigquery_object = BigQuery()
diff --git a/tests/dialects/test_presto.py b/tests/dialects/test_presto.py
index ebb270ab..82b9d626 100644
--- a/tests/dialects/test_presto.py
+++ b/tests/dialects/test_presto.py
@@ -1192,3 +1192,18 @@ MATCH_RECOGNIZE (
                 "starrocks": "SIGN(x)",
             },
         )
+
+    def test_json_vs_row_extract(self):
+        for dialect in ("trino", "presto"):
+            s = parse_one('SELECT col:x:y."special string"', read="snowflake")
+
+            dialect_json_extract_setting = f"{dialect}, variant_extract_is_json_extract=True"
+            dialect_row_access_setting = f"{dialect}, variant_extract_is_json_extract=False"
+
+            # By default, Snowflake VARIANT will generate JSON_EXTRACT() in Presto/Trino
+            json_extract_result = """SELECT JSON_EXTRACT(col, '$.x.y["special string"]')"""
+            self.assertEqual(s.sql(dialect), json_extract_result)
+            self.assertEqual(s.sql(dialect_json_extract_setting), json_extract_result)
+
+            # If the setting is overriden to False, then generate ROW access (dot notation)
+            self.assertEqual(s.sql(dialect_row_access_setting), 'SELECT col.x.y."special string"')