diff --git a/converters/snowflake/pyproject.toml b/converters/snowflake/pyproject.toml index 3fa87820..3e6b9f2e 100644 --- a/converters/snowflake/pyproject.toml +++ b/converters/snowflake/pyproject.toml @@ -39,6 +39,7 @@ keywords = [ ] dependencies = [ "PyYAML>=5.0", + "sqlglot>=30.12.0", ] [project.scripts] diff --git a/converters/snowflake/requirements.txt b/converters/snowflake/requirements.txt index c5dcb696..b8ad8439 100644 --- a/converters/snowflake/requirements.txt +++ b/converters/snowflake/requirements.txt @@ -16,3 +16,4 @@ # under the License. PyYAML>=5.0 +sqlglot>=30.12.0 diff --git a/converters/snowflake/src/ossie_snowflake/converter.py b/converters/snowflake/src/ossie_snowflake/converter.py index d8588826..6a3168d7 100644 --- a/converters/snowflake/src/ossie_snowflake/converter.py +++ b/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 = [] @@ -451,15 +479,15 @@ 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} + # Anything else must be a relation name whose three parts are real + # identifiers; otherwise SQL text that was not recognised as a query would + # silently become a bogus database/schema/table. parts = _split_identifiers(source_stripped) - if len(parts) == 3: + 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]), @@ -468,7 +496,8 @@ def _parse_source(source): } 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" ) diff --git a/converters/snowflake/tests/test_ossie_to_snowflake_yaml_converter.py b/converters/snowflake/tests/test_ossie_to_snowflake_yaml_converter.py index bcc59fd4..5f852fc6 100644 --- a/converters/snowflake/tests/test_ossie_to_snowflake_yaml_converter.py +++ b/converters/snowflake/tests/test_ossie_to_snowflake_yaml_converter.py @@ -164,6 +164,104 @@ 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*/"', + ]) + 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 + ]) + 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 +830,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) diff --git a/converters/snowflake/uv.lock b/converters/snowflake/uv.lock index eaa7da9f..5f5fd37f 100644 --- a/converters/snowflake/uv.lock +++ b/converters/snowflake/uv.lock @@ -8,6 +8,7 @@ version = "0.2.0.dev0" source = { editable = "." } dependencies = [ { name = "pyyaml" }, + { name = "sqlglot" }, ] [package.dev-dependencies] @@ -16,7 +17,10 @@ dev = [ ] [package.metadata] -requires-dist = [{ name = "pyyaml", specifier = ">=5.0" }] +requires-dist = [ + { name = "pyyaml", specifier = ">=5.0" }, + { name = "sqlglot", specifier = ">=30.12.0" }, +] [package.metadata.requires-dev] dev = [{ name = "pytest", specifier = ">=8.0" }] @@ -136,3 +140,12 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/f0/7a/1c7270340330e575b92f397352af856a8c06f230aa3e76f86b39d01b416a/pyyaml-6.0.3-cp314-cp314t-win_amd64.whl", hash = "sha256:4ad1906908f2f5ae4e5a8ddfce73c320c2a1429ec52eafd27138b7f1cbe341c9", size = 174062, upload-time = "2025-09-25T21:32:55.767Z" }, { url = "https://files.pythonhosted.org/packages/f1/12/de94a39c2ef588c7e6455cfbe7343d3b2dc9d6b6b2f40c4c6565744c873d/pyyaml-6.0.3-cp314-cp314t-win_arm64.whl", hash = "sha256:ebc55a14a21cb14062aa4162f906cd962b28e2e9ea38f9b4391244cd8de4ae0b", size = 149341, upload-time = "2025-09-25T21:32:56.828Z" }, ] + +[[package]] +name = "sqlglot" +version = "30.18.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/e4/73/5b5ce3e23b3ded3ea1986c2c2991217460d5fd5161b3e00a8425f5dcb8a3/sqlglot-30.18.0.tar.gz", hash = "sha256:e57e1b205e341979d1df5b1212c1435c598a0437e4619e3f428b15d5bc3a5cc6", size = 6018496, upload-time = "2026-09-03T13:12:58.206Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/08/9f/3dd2ec8dd84a33e0092f1c815267bc055073f9e833b93e389c59c10f62cc/sqlglot-30.18.0-py3-none-any.whl", hash = "sha256:ee0f9a9f3e2193e763c326e52dfb377b96fdb4292f6305c3ff2d9a220e71c601", size = 748788, upload-time = "2026-09-03T13:12:56.467Z" }, +]