fix(snowflake): recognise query sources with leading comments

converters/snowflake/pyproject.toml

@@ -39,6 +39,7 @@ keywords = [
 ]
 dependencies = [
     "PyYAML>=5.0",
+    "sqlglot>=30.12.0",
 ]
 
 [project.scripts]

converters/snowflake/requirements.txt

@@ -16,3 +16,4 @@
 # under the License.
 
 PyYAML>=5.0
+sqlglot>=30.12.0

converters/snowflake/src/ossie_snowflake/converter.py

@@ -25,10 +25,14 @@
 """
 
 import argparse
+import re
 import sys
 import warnings
 
 import yaml
+from sqlglot import tokenize
+from sqlglot.errors import TokenError
+from sqlglot.tokens import TokenType
 
 
 SUPPORTED_VERSION = "0.2.0.dev0"
@@ -421,6 +425,30 @@ def _normalize_identifier(identifier):
         return stripped
     return stripped.upper()
 
+_UNQUOTED_IDENTIFIER = re.compile(r"^[A-Za-z_][A-Za-z0-9_$]*$")
+_QUOTED_IDENTIFIER = re.compile(r'^"(?:[^"]|"")+"$')
+
+
+def _is_query_source(source_stripped):
+    """Recognize SELECT/WITH sources without requiring a full SQL parse."""
+    # Use Snowflake's comment and identifier rules, including `$` in names.
+    # Full parsing could reject newer Snowflake syntax that should pass through.
+    try:
+        tokens = tokenize(source_stripped, read="snowflake")
+    except TokenError:
+        return False
+
+    for token in tokens:
+        if token.token_type != TokenType.L_PAREN:
+            return token.token_type in (TokenType.SELECT, TokenType.WITH)
+    return False
+
+
+def _is_identifier(part):
+    """True if `part` is a valid quoted or unquoted Snowflake identifier."""
+    return bool(_UNQUOTED_IDENTIFIER.match(part) or _QUOTED_IDENTIFIER.match(part))
+
+
 def _split_identifiers(source_str):
     """Split a dot-separated identifier string while respecting double quotes."""
     parts = []
@@ -438,6 +466,20 @@ def _split_identifiers(source_str):
     parts.append("".join(current).strip())
     return parts
 
+
+def _try_parse_source_relation(source_stripped):
+    """Return a three-part relation, or None if its identifiers are invalid."""
+    parts = _split_identifiers(source_stripped)
+    if len(parts) == 3 and all(_is_identifier(part) for part in parts):
+        # Only uppercase unquoted identifiers; preserve quoted ones as-is.
+        return {
+            "database": _normalize_identifier(parts[0]),
+            "schema": _normalize_identifier(parts[1]),
+            "table": _normalize_identifier(parts[2]),
+        }
+    return None
+
+
 def _parse_source(source):
     """Parses an Ossie dataset source string into a Snowflake base_table dict.
 
@@ -451,24 +493,17 @@ def _parse_source(source):
     if not source_stripped:
         return None
 
-    # Detect subqueries — require whitespace after the keyword to avoid false
-    # positives on table names like WITH_TABLE or SELECT_RESULTS.
-    upper = source_stripped.upper()
-    if upper.startswith(("SELECT ", "SELECT\n", "SELECT\t",
-                          "WITH ", "WITH\n", "WITH\t")):
+    # Preserve query text, including comments, after trimming outer whitespace.
+    if _is_query_source(source_stripped):
         return {"definition": source_stripped}
 
-    parts = _split_identifiers(source_stripped)
-    if len(parts) == 3:
-        # Only uppercase unquoted identifiers; preserve quoted ones as-is.
-        return {
-            "database": _normalize_identifier(parts[0]),
-            "schema": _normalize_identifier(parts[1]),
-            "table": _normalize_identifier(parts[2]),
-        }
+    relation = _try_parse_source_relation(source_stripped)
+    if relation is not None:
+        return relation
 
     raise OssieConversionError(
-        f"Source '{source}' must be a fully qualified db.schema.table or a subquery"
+        f"Source '{source}' must be a fully qualified db.schema.table "
+        "(quoted or unquoted identifiers) or a SELECT/WITH query"
     )
 
 

converters/snowflake/tests/test_ossie_to_snowflake_yaml_converter.py

@@ -116,6 +116,21 @@ def test_three_part_name(self):
         result = _parse_source("db.schema.table")
         assert result == {"database": "DB", "schema": "SCHEMA", "table": "TABLE"}
 
+    @pytest.mark.parametrize("source, expected", [
+        ("  my_db . public . orders  ",
+         {"database": "MY_DB", "schema": "PUBLIC", "table": "ORDERS"}),
+        ('  "my.db" . public . "Order Details"  ',
+         {"database": '"my.db"', "schema": "PUBLIC", "table": '"Order Details"'}),
+        ('" padded " . public . orders',
+         {"database": '" padded "', "schema": "PUBLIC", "table": "ORDERS"}),
+        ("from.public.orders",
+         {"database": "FROM", "schema": "PUBLIC", "table": "ORDERS"}),
+        ("db.public.qualify",
+         {"database": "DB", "schema": "PUBLIC", "table": "QUALIFY"}),
+    ])
+    def test_existing_relation_normalization(self, source, expected):
+        assert _parse_source(source) == expected
+
     def test_quoted_identifiers_preserved(self):
         result = _parse_source('"myDb"."mySchema"."myTable"')
         assert result == {
@@ -164,6 +179,111 @@ def test_table_starting_with_select_not_subquery(self):
         with pytest.raises(OssieConversionError, match="fully qualified"):
             _parse_source("SELECT_RESULTS")
 
+    @pytest.mark.parametrize("source, database", [
+        ("select_results.public.t", "SELECT_RESULTS"),
+        ("select$archive.public.t", "SELECT$ARCHIVE"),
+        ("with$archive.public.t", "WITH$ARCHIVE"),
+        ("select1.public.t", "SELECT1"),
+        ("SELECT$.public.t", "SELECT$"),
+    ])
+    def test_table_named_like_keyword_prefix_is_a_relation(self, source, database):
+        # Snowflake allows `_`, digits and `$` after the first character of an
+        # unquoted identifier, so these are tables, not queries.
+        assert _parse_source(source) == {"database": database, "schema": "PUBLIC", "table": "T"}
+
+    @pytest.mark.parametrize("source", [
+        "-- revenue source\nSELECT amount FROM db.schema.orders",
+        "/* revenue source */ SELECT amount FROM db.schema.orders",
+        "// revenue source\nSELECT amount FROM db.schema.orders",
+        "-- first\n  -- second\n/* third */\nWITH c AS (SELECT 1) SELECT * FROM c",
+        "SELECT\r\namount FROM db.schema.orders",
+        "WITH\r\nc AS (SELECT 1 AS amount) SELECT amount FROM c",
+        "SELECT*FROM db.schema.orders",
+        "SELECT/*c*/ amount FROM db.schema.orders",
+        "(SELECT amount FROM db.schema.orders)",
+        "( -- inner\n  select amount from db.schema.orders )",
+        "select amount from db.schema.orders",
+    ])
+    def test_query_text_is_preserved_verbatim_as_definition(self, source):
+        # Leading comments, CRLF, missing whitespace after the keyword, and
+        # parentheses must not turn a query into a physical table reference.
+        assert _parse_source(source) == {"definition": source}
+
+    def test_query_with_leading_comment_and_fewer_dots_is_still_a_query(self):
+        source = "-- revenue\nSELECT 1 AS amount"
+        assert _parse_source(source) == {"definition": source}
+
+    @pytest.mark.parametrize("comment", ["-- revenue", "// revenue"])
+    @pytest.mark.parametrize("newline", ["\n", "\r\n", "\r"])
+    @pytest.mark.parametrize("query", [
+        "SELECT 1 AS amount",
+        "WITH c AS (SELECT 1 AS amount) SELECT amount FROM c",
+    ])
+    def test_line_comments_with_supported_line_endings(self, comment, newline, query):
+        source = comment + newline + query
+        assert _parse_source(source) == {"definition": source}
+
+    @pytest.mark.parametrize("source", [
+        "/* first */ ( // second\r\n ( -- third\n SELECT 1 ))",
+        "SELECT '-- not a comment' AS amount",
+        "SELECT $$// literal\n/* still literal */$$ AS amount",
+        "SELECT 1 AS amount; -- trailing comment",
+        "(SELECT 1 AS amount) UNION ALL (SELECT 2 AS amount)",
+        "SELECT * FROM weather RESAMPLE(USING observed_at INCREMENT BY INTERVAL '1 day')",
+    ])
+    def test_query_contents_do_not_require_parsing_or_rewriting(self, source):
+        assert _parse_source(source) == {"definition": source}
+
+    def test_query_trims_only_outer_whitespace(self):
+        source = "  \n// revenue\r\nSELECT  'Mixed Case' AS amount;\n\t"
+        assert _parse_source(source) == {
+            "definition": "// revenue\r\nSELECT  'Mixed Case' AS amount;"
+        }
+
+    @pytest.mark.parametrize("database", [
+        '"select"',
+        '"my"".db"',
+        '"/*db*/"',
+        '"@"',
+        '"café"',
+    ])
+    def test_quoted_database_stays_a_relation(self, database):
+        assert _parse_source(f"{database}.public.orders") == {
+            "database": database, "schema": "PUBLIC", "table": "ORDERS"
+        }
+
+    @pytest.mark.parametrize("source", [
+        "-- comment only",
+        "// comment only",
+        "/* comment only */",
+        "( /* comment only */ )",
+        "/* unclosed comment SELECT * FROM db.schema.orders",
+        "SELECT 'unterminated FROM db.schema.orders",
+        'SELECT "unterminated FROM db.schema.orders',
+        "SELECT $$unterminated FROM db.schema.orders",
+    ])
+    def test_unrecognizable_or_untokenizable_source_raises_conversion_error(self, source):
+        with pytest.raises(OssieConversionError, match="fully qualified"):
+            _parse_source(source)
+
+    @pytest.mark.parametrize("source", [
+        "-- c\nSELEC amount FROM db.schema.orders",   # typo: neither query nor relation
+        "foo bar.schema.table",                        # whitespace inside an unquoted part
+        "db.schema.table;",                            # trailing statement terminator
+        "1db.schema.table",                            # unquoted identifier cannot start with a digit
+        "db.public.orders AT (OFFSET => -60)",
+        "db.public.orders AS o",
+        "db.public.orders; db.public.other",
+        "/* comment */ db.public.orders",
+        "db.public.orders -- comment",
+    ])
+    def test_relation_shaped_garbage_is_rejected_not_uppercased(self, source):
+        with pytest.raises(OssieConversionError, match="fully qualified"):
+            _parse_source(source)
+
+    def test_dollar_sign_allowed_in_unquoted_identifier(self):
+        assert _parse_source("db$1.sch_2.t$") == {"database": "DB$1", "schema": "SCH_2", "table": "T$"}
+
 
 # ---------------------------------------------------------------------------
 # _extract_synonyms
@@ -732,6 +852,38 @@ def test_subquery_source(self):
         result = yaml.safe_load(convert_ossie_to_snowflake(_wrap_ossie(model)))
         assert "definition" in result["tables"][0]["base_table"]
 
+    @pytest.mark.parametrize("source", [
+        "-- revenue source\nSELECT * FROM db.s.t WHERE active = 1",
+        "// revenue source\rSELECT * FROM db.s.t WHERE active = 1",
+        "/* weather */ SELECT * FROM db.s.t RESAMPLE(USING observed_at INCREMENT BY INTERVAL '1 day')",
+    ])
+    def test_subquery_source_with_leading_comment_keeps_definition(self, source):
+        model = {
+            "name": "m",
+            "datasets": [
+                {
+                    "name": "t",
+                    "source": source,
+                    "fields": [
+                        {
+                            "name": "c",
+                            "expression": {
+                                "dialects": [
+                                    {"dialect": "ANSI_SQL", "expression": "c"}
+                                ]
+                            },
+                            "dimension": {"is_time": False},
+                        }
+                    ],
+                }
+            ],
+        }
+        with warnings.catch_warnings(record=True) as caught:
+            warnings.simplefilter("always")
+            result = yaml.safe_load(convert_ossie_to_snowflake(_wrap_ossie(model)))
+        assert result["tables"][0]["base_table"] == {"definition": source}
+        assert not [w for w in caught if "source" in str(w.message).lower()]
+
 
 # ---------------------------------------------------------------------------
 # _warn_dropped_fields (Ossie concepts with no Snowflake counterpart)