From df6f569b3429c79c7a4f8163fd234e7f65915cc1 Mon Sep 17 00:00:00 2001 From: Paul Teehan Date: Tue, 28 Apr 2026 01:19:39 +0200 Subject: [PATCH 1/5] fix(core): qualify check_filter columns in valid-reference JOIN query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A check-level filter on a valid_reference_data check produced ambiguous SQL whenever the filter referenced a column that also exists in the reference dataset (e.g. WHERE (id > 1) when both source and reference tables have an id column). Most warehouses (DuckDB, Postgres, Snowflake, ...) reject this with an "ambiguous reference" binder error and the check ends up NOT_EVALUATED. The same filter string is reused by the optimized aggregate row-count query (RowCountMetricImpl + AggregationQuery), which runs against the source CTE alone with no JOIN and no alias on the FROM. So we cannot simply prefix the filter at the metric level: a literal "C".id > 1 filter would break the aggregate query. Fix: qualify unqualified column references inside the filter only when rendering the InvalidReferenceCountQuery JOIN, keeping the metric's check_filter string untouched. The new helper uses sqlglot (already a dependency) to walk exp.Column nodes and set the source alias on any column without one — already-qualified columns, identifiers inside string literals, and function names are left alone. Falls back to the unmodified filter on parse errors so the database error surfaces as it did before. Adds an integration test that exercises a reference table sharing an id column with the source, with a check filter on id. --- soda-core/src/soda_core/common/sql_dialect.py | 13 +++++- soda-core/src/soda_core/common/sql_utils.py | 30 +++++++++++++ .../impl/check_types/invalidity_check.py | 15 ++++++- .../test_invalid_reference_check.py | 44 +++++++++++++++++++ 4 files changed, 100 insertions(+), 2 deletions(-) diff --git a/soda-core/src/soda_core/common/sql_dialect.py b/soda-core/src/soda_core/common/sql_dialect.py index 6cfaf8a2d..2def1602f 100644 --- a/soda-core/src/soda_core/common/sql_dialect.py +++ b/soda-core/src/soda_core/common/sql_dialect.py @@ -100,7 +100,10 @@ SqlExpression, SqlExpressionStr, ) -from soda_core.common.sql_utils import apply_sampling_to_sql +from soda_core.common.sql_utils import ( + apply_sampling_to_sql, + qualify_unqualified_columns_with_alias, +) from soda_core.common.statements.table_types import FullyQualifiedObjectName, TableType from typing_extensions import deprecated @@ -1452,6 +1455,14 @@ def apply_sampling( write_dialect=self.SQLGLOT_DIALECT, ) + def qualify_unqualified_columns_with_alias(self, sql_expression: str, alias: str) -> str: + return qualify_unqualified_columns_with_alias( + sql_expression=sql_expression, + alias=alias, + read_dialect=self.SQLGLOT_DIALECT, + write_dialect=self.SQLGLOT_DIALECT, + ) + ######################################################## # Metadata columns query ######################################################## diff --git a/soda-core/src/soda_core/common/sql_utils.py b/soda-core/src/soda_core/common/sql_utils.py index e8ec47779..af7c82597 100644 --- a/soda-core/src/soda_core/common/sql_utils.py +++ b/soda-core/src/soda_core/common/sql_utils.py @@ -37,6 +37,36 @@ def attach_sample_to_relation(rel: exp.Expression, sampler_limit: Number, sample rel.set("sample", build_sample_clause(sampler_limit, sampler_type)) +def qualify_unqualified_columns_with_alias( + sql_expression: str, + alias: str, + read_dialect: str | None = None, + write_dialect: str | None = None, +) -> str: + """ + Qualify every unqualified column reference in a SQL expression with the given table alias. + + Already-qualified columns are left untouched. If the expression cannot be parsed as SQL, + it is returned unchanged so any database error surfaces unmodified to the user. + """ + if not sql_expression or not sql_expression.strip(): + return sql_expression + + try: + tree = ( + sqlglot.parse_one(sql_expression, read=read_dialect) if read_dialect else sqlglot.parse_one(sql_expression) + ) + except sqlglot.errors.ParseError: + return sql_expression + + for column in tree.find_all(exp.Column): + if column.table: + continue + column.set("table", exp.to_identifier(alias, quoted=True)) + + return tree.sql(dialect=write_dialect) if write_dialect else tree.sql() + + def apply_sampling_to_sql( sql: str, sampler_limit: Number, diff --git a/soda-core/src/soda_core/contracts/impl/check_types/invalidity_check.py b/soda-core/src/soda_core/contracts/impl/check_types/invalidity_check.py index 0c416dbfd..9bfa4ee9b 100644 --- a/soda-core/src/soda_core/contracts/impl/check_types/invalidity_check.py +++ b/soda-core/src/soda_core/contracts/impl/check_types/invalidity_check.py @@ -248,11 +248,24 @@ def __init__( self.sql = self.data_source_impl.sql_dialect.build_select_sql(sql_ast) def build_query(self, cte: CTE, select_clause: SqlExpression) -> list[SqlExpression]: + # Qualify unqualified columns in the user's filter with the source alias so they + # are unambiguous after the JOIN with the reference dataset. The original + # check_filter is left untouched on self so the un-aliased aggregate row-count + # query (RowCountMetricImpl, run via AggregationQuery against the source CTE + # alone) still resolves. + join_query_filter: Optional[str] = ( + self.data_source_impl.sql_dialect.qualify_unqualified_columns_with_alias( + self.check_filter, self.referencing_alias + ) + if self.check_filter + else None + ) + query = [ WITH([cte, self.referenced_cte()]), select_clause, FROM(cte.alias).AS(self.referencing_alias), - WHERE.optional(SqlExpressionStr.optional(self.check_filter)), + WHERE.optional(SqlExpressionStr.optional(join_query_filter)), ] query.extend(self.query_join()) diff --git a/soda-tests/tests/integration/test_invalid_reference_check.py b/soda-tests/tests/integration/test_invalid_reference_check.py index 7daa0916b..cd5cf0cf9 100644 --- a/soda-tests/tests/integration/test_invalid_reference_check.py +++ b/soda-tests/tests/integration/test_invalid_reference_check.py @@ -43,6 +43,25 @@ ) +# Reference table that intentionally shares an `id` column with the source +# (`referencing_table_specification`). Used to exercise the case where a +# check-level filter on `id` becomes ambiguous in the JOIN-style invalid +# reference query. +referenced_table_with_shared_id_specification = ( + TestTableSpecification.builder() + .table_purpose("invalid_referenced_shared_id") + .column_integer("id") + .column_varchar("country_code", 2) + .rows( + rows=[ + (100, "NL"), + (101, "BE"), + ] + ) + .build() +) + + def test_invalid_count(data_source_test_helper: DataSourceTestHelper): # https://dev.sodadata.io/o/f35cb402-ad17-4aca-9166-02c9eb75c979/datasets/2945ba9d-b1ff-4cfd-b277-d5e4edfa2bd5/checks @@ -114,6 +133,31 @@ def test_invalid_count_with_check_filter(data_source_test_helper: DataSourceTest assert get_diagnostic_value(check_result, "check_rows_tested") == 3 +def test_invalid_count_with_check_filter_on_shared_column(data_source_test_helper: DataSourceTestHelper): + referencing_table = data_source_test_helper.ensure_test_table(referencing_table_specification) + referenced_table = data_source_test_helper.ensure_test_table(referenced_table_with_shared_id_specification) + + id_quoted: str = data_source_test_helper.quote_column("id") + + contract_verification_result: ContractVerificationResult = data_source_test_helper.assert_contract_fail( + test_table=referencing_table, + contract_yaml_str=f""" + columns: + - name: country + valid_reference_data: + dataset: {data_source_test_helper.build_dqn(referenced_table)} + column: country_code + checks: + - invalid: + filter: | + {id_quoted} > 1 + """, + ) + check_result: CheckResult = contract_verification_result.check_results[0] + assert get_diagnostic_value(check_result, "invalid_count") == 3 + assert get_diagnostic_value(check_result, "check_rows_tested") == 7 + + def test_invalid_count_with_check_and_dataset_filter(data_source_test_helper: DataSourceTestHelper): referencing_table = data_source_test_helper.ensure_test_table(referencing_table_specification) referenced_table = data_source_test_helper.ensure_test_table(referenced_table_specification) From 703fa250a8182d5a3cbaf56f6f89e2ec9a5b1cee Mon Sep 17 00:00:00 2001 From: Paul Teehan Date: Tue, 28 Apr 2026 12:13:29 +0200 Subject: [PATCH 2/5] fix(core): skip filter qualification when sqlglot has no native dialect The reference-check filter rewrite relies on a round-trip through sqlglot (parse -> qualify columns -> render). For data sources whose SqlDialect sets sqlglot_dialect="" (DB2 LUW and z/OS do this because sqlglot has no native DB2 dialect) the round-trip would fall back to sqlglot's default parser/printer, which can mangle DB2-specific syntax in filter expressions (timestamp literals like '2024-01-01-12.30.00.000000', CURRENT TIMESTAMP - 1 DAY, etc.). Short-circuit when read_dialect is empty/None and return the filter unchanged, preserving the pre-fix behaviour for those data sources. The original ambiguous-column bug remains there for now; it can be fixed separately once sqlglot adds a DB2 dialect or a hand-rolled column resolver is added. --- soda-core/src/soda_core/common/sql_utils.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/soda-core/src/soda_core/common/sql_utils.py b/soda-core/src/soda_core/common/sql_utils.py index af7c82597..21dc2a2ec 100644 --- a/soda-core/src/soda_core/common/sql_utils.py +++ b/soda-core/src/soda_core/common/sql_utils.py @@ -48,14 +48,22 @@ def qualify_unqualified_columns_with_alias( Already-qualified columns are left untouched. If the expression cannot be parsed as SQL, it is returned unchanged so any database error surfaces unmodified to the user. + + Data sources without a native sqlglot dialect (DB2, for example, sets + `sqlglot_dialect=""`) are not rewritten, because the round-trip through sqlglot's default + parser/printer can mangle dialect-specific syntax (e.g. DB2 timestamp literals like + `'2024-01-01-12.30.00'`). On those data sources the filter is returned unchanged and the + pre-fix behaviour applies, so a filter that references a column shared with the reference + table will still error at the database — no new regression vs today. """ if not sql_expression or not sql_expression.strip(): return sql_expression + if not read_dialect: + return sql_expression + try: - tree = ( - sqlglot.parse_one(sql_expression, read=read_dialect) if read_dialect else sqlglot.parse_one(sql_expression) - ) + tree = sqlglot.parse_one(sql_expression, read=read_dialect) except sqlglot.errors.ParseError: return sql_expression From 5066cd0c30c6eadefdedd729b2b28aa958a6e244 Mon Sep 17 00:00:00 2001 From: Paul Teehan Date: Tue, 28 Apr 2026 18:33:25 +0200 Subject: [PATCH 3/5] fix(core): drop DB2 sqlglot short-circuit now that DB2 has a native dialect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 703fa250 added a defensive `if not read_dialect: return sql_expression` guard to qualify_unqualified_columns_with_alias because at that time DB2 LUW and z/OS set sqlglot_dialect="" (no native sqlglot dialect — round-tripping DB2 SQL through sqlglot's default parser/printer can mangle DB2-specific syntax). The guard kept DB2 on the pre-fix code path: the ambiguous-column bug from DTL-1517 still surfaced on DB2, but no new mangling regression was introduced. soda-extensions now ships a native DB2 sqlglot dialect (registered as "db2" in soda_db2.base.sqlglot_dialect; both LUW and z/OS adapters set SQLGLOT_DIALECT="db2"). With every Soda data source declaring a real sqlglot dialect, the guard has nothing left to protect — drop it. Refresh the docstring to spell out the contract (every adapter must pass a real sqlglot dialect name in `sqlglot_dialect=`) so future adapters get a deliberate rather than accidental fix. --- soda-core/src/soda_core/common/sql_utils.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/soda-core/src/soda_core/common/sql_utils.py b/soda-core/src/soda_core/common/sql_utils.py index 21dc2a2ec..eabdbcc71 100644 --- a/soda-core/src/soda_core/common/sql_utils.py +++ b/soda-core/src/soda_core/common/sql_utils.py @@ -49,21 +49,19 @@ def qualify_unqualified_columns_with_alias( Already-qualified columns are left untouched. If the expression cannot be parsed as SQL, it is returned unchanged so any database error surfaces unmodified to the user. - Data sources without a native sqlglot dialect (DB2, for example, sets - `sqlglot_dialect=""`) are not rewritten, because the round-trip through sqlglot's default - parser/printer can mangle dialect-specific syntax (e.g. DB2 timestamp literals like - `'2024-01-01-12.30.00'`). On those data sources the filter is returned unchanged and the - pre-fix behaviour applies, so a filter that references a column shared with the reference - table will still error at the database — no new regression vs today. + If ``read_dialect`` / ``write_dialect`` are not provided (or are empty), sqlglot's + default parser/printer is used. Every Soda data source ships a non-empty + ``SQLGLOT_DIALECT`` today — including DB2 LUW + z/OS, both registered as ``"db2"`` + via ``soda_db2.base.sqlglot_dialect``. If you add a new adapter, make sure its + ``SqlDialect`` subclass passes a real sqlglot dialect name in ``sqlglot_dialect=``. """ if not sql_expression or not sql_expression.strip(): return sql_expression - if not read_dialect: - return sql_expression - try: - tree = sqlglot.parse_one(sql_expression, read=read_dialect) + tree = ( + sqlglot.parse_one(sql_expression, read=read_dialect) if read_dialect else sqlglot.parse_one(sql_expression) + ) except sqlglot.errors.ParseError: return sql_expression From 6555e08a2c945ba5ca482b1a84bbebae28fda49e Mon Sep 17 00:00:00 2001 From: Paul Teehan Date: Wed, 29 Apr 2026 15:49:15 +0200 Subject: [PATCH 4/5] fix(core): retry invalid-reference query with qualified filter on error Switch from always-qualify to retry-on-error. The first execution emits the user's check_filter byte-for-byte; only if the JOIN query fails do we retry once with the filter columns qualified against the source alias. If that retry also fails, both errors are surfaced (initial query + initial error, retry query + retry error) so the user is not misled about what actually went wrong. This avoids two surprises from the previous always-qualify approach: - sqlglot round-trip canonicalisation of every filter expression - snapshot churn on Athena/Databricks for tests whose filter is unambiguous Engine error-message wording for ambiguous-column errors varies across the ~15 supported databases; rather than maintaining a per-engine substring list, the retry fires on any error from InvalidReferenceCountQuery.execute and lets the existing per-engine integration test prove the fix works on each adapter. Also fold in two helper improvements flagged in review: - qualify_unqualified_columns_with_alias now skips columns inside nested SELECT scopes (subqueries, EXISTS bodies, scalar subqueries) so they resolve in their own scope rather than being silently aliased to the outer one. - Broaden the exception fallback to ``except Exception`` so unknown sqlglot dialects (which raise ValueError, not ParseError) also fall back to the un-rewritten input. Adds 11 unit tests in test_sql_utils.py covering the helper directly, including dialect-specific quoting across postgres / snowflake / duckdb / trino / athena / bigquery / databricks / tsql. Co-Authored-By: Claude Opus 4.7 (1M context) --- soda-core/src/soda_core/common/sql_utils.py | 40 ++++--- .../impl/check_types/invalidity_check.py | 72 +++++++++--- soda-tests/tests/unit/test_sql_utils.py | 109 +++++++++++++++++- 3 files changed, 186 insertions(+), 35 deletions(-) diff --git a/soda-core/src/soda_core/common/sql_utils.py b/soda-core/src/soda_core/common/sql_utils.py index eabdbcc71..b8e2ed852 100644 --- a/soda-core/src/soda_core/common/sql_utils.py +++ b/soda-core/src/soda_core/common/sql_utils.py @@ -44,16 +44,21 @@ def qualify_unqualified_columns_with_alias( write_dialect: str | None = None, ) -> str: """ - Qualify every unqualified column reference in a SQL expression with the given table alias. + Qualify every unqualified top-level column reference in a SQL expression with the given + table alias. - Already-qualified columns are left untouched. If the expression cannot be parsed as SQL, - it is returned unchanged so any database error surfaces unmodified to the user. + Already-qualified columns are left untouched. Columns inside subqueries are also left + untouched — they belong to the subquery's own scope, not the outer alias. If sqlglot + cannot parse or print the expression for any reason (parse error, unknown dialect), + the input is returned unchanged so any database error surfaces unmodified to the user. + + Note: a successful parse round-trips through sqlglot's printer, which canonicalises some + syntax (e.g. ``col::int`` becomes ``CAST(col AS INT)``, function names get uppercased). + On the same read/write dialect the rewrite is semantically equivalent in standard cases, + but recognised functions may be rewritten into canonical equivalents. If ``read_dialect`` / ``write_dialect`` are not provided (or are empty), sqlglot's - default parser/printer is used. Every Soda data source ships a non-empty - ``SQLGLOT_DIALECT`` today — including DB2 LUW + z/OS, both registered as ``"db2"`` - via ``soda_db2.base.sqlglot_dialect``. If you add a new adapter, make sure its - ``SqlDialect`` subclass passes a real sqlglot dialect name in ``sqlglot_dialect=``. + default parser/printer is used. """ if not sql_expression or not sql_expression.strip(): return sql_expression @@ -62,15 +67,20 @@ def qualify_unqualified_columns_with_alias( tree = ( sqlglot.parse_one(sql_expression, read=read_dialect) if read_dialect else sqlglot.parse_one(sql_expression) ) - except sqlglot.errors.ParseError: - return sql_expression - for column in tree.find_all(exp.Column): - if column.table: - continue - column.set("table", exp.to_identifier(alias, quoted=True)) - - return tree.sql(dialect=write_dialect) if write_dialect else tree.sql() + for column in tree.find_all(exp.Column): + if column.table: + continue + # Skip columns inside any nested SELECT scope (subqueries via exp.Subquery, + # EXISTS bodies via exp.Exists, scalar subqueries, CTEs). These resolve in + # their own scope first and shouldn't be silently aliased to the outer one. + if column.find_ancestor(exp.Select): + continue + column.set("table", exp.to_identifier(alias, quoted=True)) + + return tree.sql(dialect=write_dialect) if write_dialect else tree.sql() + except Exception: + return sql_expression def apply_sampling_to_sql( diff --git a/soda-core/src/soda_core/contracts/impl/check_types/invalidity_check.py b/soda-core/src/soda_core/contracts/impl/check_types/invalidity_check.py index 9bfa4ee9b..072d4c3bc 100644 --- a/soda-core/src/soda_core/contracts/impl/check_types/invalidity_check.py +++ b/soda-core/src/soda_core/contracts/impl/check_types/invalidity_check.py @@ -244,34 +244,38 @@ def __init__( self._cte = cte - sql_ast = self.build_query(cte=self._cte, select_clause=SELECT(COUNT(STAR()))) - self.sql = self.data_source_impl.sql_dialect.build_select_sql(sql_ast) - - def build_query(self, cte: CTE, select_clause: SqlExpression) -> list[SqlExpression]: - # Qualify unqualified columns in the user's filter with the source alias so they - # are unambiguous after the JOIN with the reference dataset. The original - # check_filter is left untouched on self so the un-aliased aggregate row-count - # query (RowCountMetricImpl, run via AggregationQuery against the source CTE - # alone) still resolves. - join_query_filter: Optional[str] = ( - self.data_source_impl.sql_dialect.qualify_unqualified_columns_with_alias( - self.check_filter, self.referencing_alias - ) - if self.check_filter - else None + sql_ast = self.build_query( + cte=self._cte, select_clause=SELECT(COUNT(STAR())), check_filter=self.check_filter ) + self.sql = self.data_source_impl.sql_dialect.build_select_sql(sql_ast) + def build_query( + self, cte: CTE, select_clause: SqlExpression, check_filter: Optional[str] = None + ) -> list[SqlExpression]: query = [ WITH([cte, self.referenced_cte()]), select_clause, FROM(cte.alias).AS(self.referencing_alias), - WHERE.optional(SqlExpressionStr.optional(join_query_filter)), + WHERE.optional(SqlExpressionStr.optional(check_filter)), ] query.extend(self.query_join()) return query + def _build_qualified_filter_sql(self) -> str: + """Re-render the JOIN query with check_filter columns qualified against the + source alias. Used as a one-shot retry when the original query errors.""" + qualified_filter: str = self.data_source_impl.sql_dialect.qualify_unqualified_columns_with_alias( + self.check_filter, self.referencing_alias + ) + sql_ast = self.build_query( + cte=self._cte, + select_clause=SELECT(COUNT(STAR())), + check_filter=qualified_filter, + ) + return self.data_source_impl.sql_dialect.build_select_sql(sql_ast) + def referenced_cte(self) -> CTE: valid_reference_data: ValidReferenceData = self.metric_impl.missing_and_validity.valid_reference_data referenced_dataset_name: str = valid_reference_data.dataset_name @@ -345,9 +349,39 @@ def query_join(self) -> SqlExpression: def execute(self) -> list[Measurement]: try: query_result: QueryResult = self.data_source_impl.execute_query(self.sql) - except Exception as e: - logger.error(msg=f"Could not execute invalid reference query {self.sql}: {e}", exc_info=True) - return [] + except Exception as initial_error: + # The most common cause of failure here is an ambiguous-column error when the + # check_filter references a name that exists in both the source and reference + # datasets (the JOIN exposes both). Engine error wording varies, so instead of + # sniffing the message we retry once with the filter columns qualified against + # the source alias. If the retry also fails (or there is no filter to retry + # with), surface the original error and, where applicable, the retry error too. + if not self.check_filter: + logger.error( + msg=f"Could not execute invalid reference query {self.sql}: {initial_error}", + exc_info=True, + ) + return [] + + retry_sql: str = self._build_qualified_filter_sql() + try: + query_result = self.data_source_impl.execute_query(retry_sql) + except Exception as retry_error: + logger.error( + msg=( + f"Could not execute invalid reference query.\n" + f"Initial query failed: {self.sql}\n" + f" Initial error: {initial_error}\n" + f"Retried with the check_filter columns qualified against source alias " + f"{self.referencing_alias!r} (common fix for ambiguous-column errors when " + f"the filter references a column that exists in both the source and " + f"reference datasets); the retry also failed.\n" + f"Retry query: {retry_sql}\n" + f" Retry error: {retry_error}" + ), + exc_info=True, + ) + return [] metric_value = query_result.rows[0][0] metric_impl: MetricImpl = self.metrics[0] diff --git a/soda-tests/tests/unit/test_sql_utils.py b/soda-tests/tests/unit/test_sql_utils.py index c2fefd1fd..bc99a8c66 100644 --- a/soda-tests/tests/unit/test_sql_utils.py +++ b/soda-tests/tests/unit/test_sql_utils.py @@ -7,7 +7,10 @@ import pytest import sqlglot from soda_core.common.metadata_types import SamplerType -from soda_core.common.sql_utils import apply_sampling_to_sql +from soda_core.common.sql_utils import ( + apply_sampling_to_sql, + qualify_unqualified_columns_with_alias, +) from sqlglot import exp @@ -259,3 +262,107 @@ def test_apply_sampling_complex_query() -> None: subquery = _get_first_subquery_with_alias(tree, "subquery") subquery_sample = subquery.args.get("sample") assert subquery_sample is None + + +# ------------------------------ +# qualify_unqualified_columns_with_alias +# ------------------------------ + + +def test_qualify_basic_unqualified_column() -> None: + out = qualify_unqualified_columns_with_alias("id > 1", "C", read_dialect="postgres", write_dialect="postgres") + assert out == '"C".id > 1' + + +def test_qualify_leaves_already_qualified_columns_alone() -> None: + out = qualify_unqualified_columns_with_alias( + '"R".country_code = country', "C", read_dialect="postgres", write_dialect="postgres" + ) + assert out == '"R".country_code = "C".country' + + +def test_qualify_does_not_touch_string_literals() -> None: + # 'foo' is a string literal — only the column `name` should be qualified + out = qualify_unqualified_columns_with_alias( + "lower(name) = 'foo'", "C", read_dialect="postgres", write_dialect="postgres" + ) + assert "'foo'" in out + assert '"C".name' in out + + +@pytest.mark.parametrize("expr", ["", " ", "\n\t"]) +def test_qualify_empty_input_returned_unchanged(expr: str) -> None: + out = qualify_unqualified_columns_with_alias(expr, "C", read_dialect="postgres", write_dialect="postgres") + assert out == expr + + +def test_qualify_unparseable_input_returned_unchanged() -> None: + expr = "(((unbalanced" + out = qualify_unqualified_columns_with_alias(expr, "C", read_dialect="postgres", write_dialect="postgres") + assert out == expr + + +def test_qualify_unknown_dialect_returned_unchanged() -> None: + # Unknown dialects raise ValueError inside sqlglot; the helper must not propagate it. + expr = "id > 1" + out = qualify_unqualified_columns_with_alias( + expr, "C", read_dialect="totally_fake_dialect", write_dialect="totally_fake_dialect" + ) + assert out == expr + + +def test_qualify_skips_columns_inside_in_subquery() -> None: + # Outer `id` belongs to the source and should be qualified. + # Inner `user_id` belongs to `other` and must NOT be aliased to "C". + out = qualify_unqualified_columns_with_alias( + "id IN (SELECT user_id FROM other)", "C", read_dialect="postgres", write_dialect="postgres" + ) + assert '"C".id' in out + assert '"C".user_id' not in out + + +def test_qualify_skips_columns_inside_exists_subquery() -> None: + # EXISTS bodies are wrapped in exp.Exists > exp.Select (not exp.Subquery). + # Inner unqualified `id` must NOT be aliased — it resolves in the subquery's own scope. + out = qualify_unqualified_columns_with_alias( + "EXISTS (SELECT 1 FROM other WHERE id = 1)", "C", read_dialect="postgres", write_dialect="postgres" + ) + assert '"C".id' not in out + + +def test_qualify_skips_columns_inside_scalar_subquery() -> None: + # Outer `id` qualified; inner aggregate's `x` left alone. + out = qualify_unqualified_columns_with_alias( + "(SELECT MAX(x) FROM other) > id", "C", read_dialect="postgres", write_dialect="postgres" + ) + assert '"C".id' in out + assert '"C".x' not in out + + +@pytest.mark.parametrize( + "dialect,expected_substring", + [ + ("postgres", '"C".id'), + ("snowflake", '"C".id'), + ("duckdb", '"C".id'), + ("trino", '"C".id'), + ("athena", '"C".id'), + ("bigquery", "`C`.id"), + ("databricks", "`C`.id"), + ("tsql", "[C].id"), + ], +) +def test_qualify_uses_dialect_specific_quoting(dialect: str, expected_substring: str) -> None: + out = qualify_unqualified_columns_with_alias("id > 1", "C", read_dialect=dialect, write_dialect=dialect) + assert expected_substring in out + + +def test_qualify_handles_window_function() -> None: + out = qualify_unqualified_columns_with_alias( + "ROW_NUMBER() OVER (PARTITION BY col1 ORDER BY col2) > 5", + "C", + read_dialect="postgres", + write_dialect="postgres", + ) + assert '"C".col1' in out + assert '"C".col2' in out From b10edd1bb84496218546c52e6176cb4bddf7d630 Mon Sep 17 00:00:00 2001 From: Paul Teehan Date: Wed, 29 Apr 2026 15:51:33 +0200 Subject: [PATCH 5/5] style(core): apply black formatting to invalidity_check.py Co-Authored-By: Claude Opus 4.7 (1M context) --- .../soda_core/contracts/impl/check_types/invalidity_check.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/soda-core/src/soda_core/contracts/impl/check_types/invalidity_check.py b/soda-core/src/soda_core/contracts/impl/check_types/invalidity_check.py index 072d4c3bc..15eb44da5 100644 --- a/soda-core/src/soda_core/contracts/impl/check_types/invalidity_check.py +++ b/soda-core/src/soda_core/contracts/impl/check_types/invalidity_check.py @@ -244,9 +244,7 @@ def __init__( self._cte = cte - sql_ast = self.build_query( - cte=self._cte, select_clause=SELECT(COUNT(STAR())), check_filter=self.check_filter - ) + sql_ast = self.build_query(cte=self._cte, select_clause=SELECT(COUNT(STAR())), check_filter=self.check_filter) self.sql = self.data_source_impl.sql_dialect.build_select_sql(sql_ast) def build_query(