From 2a7ca18855ae803ba4c8739c86bf2bd4faa0d45d Mon Sep 17 00:00:00 2001 From: Poorva Barve Date: Wed, 9 Sep 2026 20:43:25 -0700 Subject: [PATCH 1/3] fix(snowflake): recognise query sources with leading comments A dataset source that is a SQL query but does not start with the bare SELECT/WITH keyword (leading -- or /* */ comment, CRLF after the keyword, surrounding parentheses, or no whitespace as in SELECT*FROM) fell through to the relation path, which split the text on dots and emitted an uppercased bogus database/schema/table with no warning. Detect queries after skipping leading whitespace, SQL comments and opening parentheses, matching SELECT/WITH as a whole word so names such as SELECT_RESULTS stay relations. Query text is emitted verbatim as base_table.definition. The relation path now requires three valid quoted or unquoted identifiers and raises otherwise. Fixes #376 --- .../src/ossie_snowflake/converter.py | 41 ++++++++++-- .../test_ossie_to_snowflake_yaml_converter.py | 66 +++++++++++++++++++ 2 files changed, 100 insertions(+), 7 deletions(-) diff --git a/converters/snowflake/src/ossie_snowflake/converter.py b/converters/snowflake/src/ossie_snowflake/converter.py index d8588826..eddacf34 100644 --- a/converters/snowflake/src/ossie_snowflake/converter.py +++ b/converters/snowflake/src/ossie_snowflake/converter.py @@ -25,6 +25,7 @@ """ import argparse +import re import sys import warnings @@ -421,6 +422,30 @@ def _normalize_identifier(identifier): return stripped return stripped.upper() +# A dataset source is a SQL query when, after leading whitespace and SQL +# comments (`-- ...` and `/* ... */`) and any opening parentheses, it starts +# with SELECT or WITH as a whole word. The word boundary keeps table names such +# as SELECT_RESULTS on the relation path, while `SELECT*FROM`, `SELECT/*c*/`, +# and CRLF line endings after the keyword are still recognised as queries. +_LEADING_SQL_TRIVIA = re.compile(r"^(?:\s+|--[^\n]*(?:\n|$)|/\*.*?\*/)+", re.DOTALL) +_QUERY_KEYWORD = re.compile(r"(?:SELECT|WITH)\b", re.IGNORECASE) +_UNQUOTED_IDENTIFIER = re.compile(r"^[A-Za-z_][A-Za-z0-9_$]*$") +_QUOTED_IDENTIFIER = re.compile(r'^"(?:[^"]|"")+"$') + + +def _is_query_source(source_stripped): + """True if the source text is a SQL query rather than a relation name.""" + body = _LEADING_SQL_TRIVIA.sub("", source_stripped, count=1) + while body.startswith("("): + body = _LEADING_SQL_TRIVIA.sub("", body[1:], count=1) + return _QUERY_KEYWORD.match(body) is not None + + +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 +476,16 @@ 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")): + # Queries keep their exact text (including leading comments) so the + # emitted definition is what the author wrote. + 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 +494,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..618c0a02 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,44 @@ def test_table_starting_with_select_not_subquery(self): with pytest.raises(OssieConversionError, match="fully qualified"): _parse_source("SELECT_RESULTS") + def test_table_named_like_keyword_prefix_is_a_relation(self): + result = _parse_source("select_results.public.t") + assert result == {"database": "SELECT_RESULTS", "schema": "PUBLIC", "table": "T"} + + @pytest.mark.parametrize("source", [ + "-- revenue source\nSELECT amount FROM db.schema.orders", + "/* revenue source */ SELECT 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("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 +770,34 @@ def test_subquery_source(self): result = yaml.safe_load(convert_ossie_to_snowflake(_wrap_ossie(model))) assert "definition" in result["tables"][0]["base_table"] + def test_subquery_source_with_leading_comment_keeps_definition(self): + source = "-- revenue source\nSELECT * FROM db.s.t WHERE active = 1" + 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) From 368f3659c8d370b23473544c55f0c6f7961915a2 Mon Sep 17 00:00:00 2001 From: Poorva Barve Date: Wed, 9 Sep 2026 21:21:53 -0700 Subject: [PATCH 2/3] fix(snowflake): do not treat SELECT$... identifiers as queries The keyword check used a \b word boundary, but Snowflake allows `$` in unquoted identifiers, so a valid table reference such as select$archive.public.orders was classified as a query. Match the keyword only when the next character cannot continue an unquoted identifier. Also skip Snowflake's `// ...` single-line comments before a query, and cover both cases with tests. --- .../snowflake/src/ossie_snowflake/converter.py | 18 +++++++++++------- .../test_ossie_to_snowflake_yaml_converter.py | 15 ++++++++++++--- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/converters/snowflake/src/ossie_snowflake/converter.py b/converters/snowflake/src/ossie_snowflake/converter.py index eddacf34..095d0546 100644 --- a/converters/snowflake/src/ossie_snowflake/converter.py +++ b/converters/snowflake/src/ossie_snowflake/converter.py @@ -422,13 +422,17 @@ def _normalize_identifier(identifier): return stripped return stripped.upper() -# A dataset source is a SQL query when, after leading whitespace and SQL -# comments (`-- ...` and `/* ... */`) and any opening parentheses, it starts -# with SELECT or WITH as a whole word. The word boundary keeps table names such -# as SELECT_RESULTS on the relation path, while `SELECT*FROM`, `SELECT/*c*/`, -# and CRLF line endings after the keyword are still recognised as queries. -_LEADING_SQL_TRIVIA = re.compile(r"^(?:\s+|--[^\n]*(?:\n|$)|/\*.*?\*/)+", re.DOTALL) -_QUERY_KEYWORD = re.compile(r"(?:SELECT|WITH)\b", re.IGNORECASE) +# A dataset source is a SQL query when, after leading whitespace, SQL comments +# (`-- ...`, `// ...` and `/* ... */`) and any opening parentheses, it starts +# with SELECT or WITH followed by something that cannot continue an unquoted +# identifier. That keeps names such as SELECT_RESULTS or SELECT$ARCHIVE on the +# relation path (Snowflake allows `$` in unquoted identifiers, so `\b` would be +# wrong), while `SELECT*FROM`, `SELECT/*c*/` and CRLF after the keyword are +# still recognised as queries. +_LEADING_SQL_TRIVIA = re.compile( + r"^(?:\s+|--[^\n]*(?:\n|$)|//[^\n]*(?:\n|$)|/\*.*?\*/)+", re.DOTALL +) +_QUERY_KEYWORD = re.compile(r"^(?:SELECT|WITH)(?![A-Za-z0-9_$])", re.IGNORECASE) _UNQUOTED_IDENTIFIER = re.compile(r"^[A-Za-z_][A-Za-z0-9_$]*$") _QUOTED_IDENTIFIER = re.compile(r'^"(?:[^"]|"")+"$') 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 618c0a02..165dbc2f 100644 --- a/converters/snowflake/tests/test_ossie_to_snowflake_yaml_converter.py +++ b/converters/snowflake/tests/test_ossie_to_snowflake_yaml_converter.py @@ -164,13 +164,22 @@ def test_table_starting_with_select_not_subquery(self): with pytest.raises(OssieConversionError, match="fully qualified"): _parse_source("SELECT_RESULTS") - def test_table_named_like_keyword_prefix_is_a_relation(self): - result = _parse_source("select_results.public.t") - assert result == {"database": "SELECT_RESULTS", "schema": "PUBLIC", "table": "T"} + @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", From f67f582317c9a914a9fddf5ce2ba07a43d079b58 Mon Sep 17 00:00:00 2001 From: Poorva Barve Date: Mon, 14 Sep 2026 23:56:03 -0700 Subject: [PATCH 3/3] fix(snowflake): classify query sources with SQLGlot Use Snowflake tokenization to recognize SELECT/WITH sources while preserving the original SQL. Retain relation identifier validation and handle tokenization failures as conversion errors. Declare SQLGlot in the Snowflake package dependencies and lockfile. Add regression coverage for comments, line endings, quoted identifiers, query preservation, and malformed sources. --- converters/snowflake/pyproject.toml | 1 + converters/snowflake/requirements.txt | 1 + .../src/ossie_snowflake/converter.py | 34 +++++------ .../test_ossie_to_snowflake_yaml_converter.py | 59 ++++++++++++++++++- converters/snowflake/uv.lock | 15 ++++- 5 files changed, 89 insertions(+), 21 deletions(-) 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 095d0546..6a3168d7 100644 --- a/converters/snowflake/src/ossie_snowflake/converter.py +++ b/converters/snowflake/src/ossie_snowflake/converter.py @@ -30,6 +30,9 @@ import warnings import yaml +from sqlglot import tokenize +from sqlglot.errors import TokenError +from sqlglot.tokens import TokenType SUPPORTED_VERSION = "0.2.0.dev0" @@ -422,27 +425,23 @@ def _normalize_identifier(identifier): return stripped return stripped.upper() -# A dataset source is a SQL query when, after leading whitespace, SQL comments -# (`-- ...`, `// ...` and `/* ... */`) and any opening parentheses, it starts -# with SELECT or WITH followed by something that cannot continue an unquoted -# identifier. That keeps names such as SELECT_RESULTS or SELECT$ARCHIVE on the -# relation path (Snowflake allows `$` in unquoted identifiers, so `\b` would be -# wrong), while `SELECT*FROM`, `SELECT/*c*/` and CRLF after the keyword are -# still recognised as queries. -_LEADING_SQL_TRIVIA = re.compile( - r"^(?:\s+|--[^\n]*(?:\n|$)|//[^\n]*(?:\n|$)|/\*.*?\*/)+", re.DOTALL -) -_QUERY_KEYWORD = re.compile(r"^(?:SELECT|WITH)(?![A-Za-z0-9_$])", re.IGNORECASE) _UNQUOTED_IDENTIFIER = re.compile(r"^[A-Za-z_][A-Za-z0-9_$]*$") _QUOTED_IDENTIFIER = re.compile(r'^"(?:[^"]|"")+"$') def _is_query_source(source_stripped): - """True if the source text is a SQL query rather than a relation name.""" - body = _LEADING_SQL_TRIVIA.sub("", source_stripped, count=1) - while body.startswith("("): - body = _LEADING_SQL_TRIVIA.sub("", body[1:], count=1) - return _QUERY_KEYWORD.match(body) is not None + """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): @@ -480,8 +479,7 @@ def _parse_source(source): if not source_stripped: return None - # Queries keep their exact text (including leading comments) so the - # emitted definition is what the author wrote. + # Preserve query text, including comments, after trimming outer whitespace. if _is_query_source(source_stripped): return {"definition": source_stripped} 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 165dbc2f..5f852fc6 100644 --- a/converters/snowflake/tests/test_ossie_to_snowflake_yaml_converter.py +++ b/converters/snowflake/tests/test_ossie_to_snowflake_yaml_converter.py @@ -198,6 +198,57 @@ 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 @@ -779,8 +830,12 @@ def test_subquery_source(self): result = yaml.safe_load(convert_ossie_to_snowflake(_wrap_ossie(model))) assert "definition" in result["tables"][0]["base_table"] - def test_subquery_source_with_leading_comment_keeps_definition(self): - source = "-- revenue source\nSELECT * FROM db.s.t WHERE active = 1" + @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": [ 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" }, +]