From 2b611f5617bcf7a04daef02c9f5a3592915c5659 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 19:31:58 +0100 Subject: [PATCH 01/12] fix: fill each partial fixture column once across identifier case Required fixture columns were deduplicated by their authored spelling, so a column read as STATUS through a SELECT * import CTE and required again as the declared status was completed twice. Key required columns by their casefolded name, matching the metadata lookup, and complete each declared column at most once. Fixes CHI-630. --- .../planner_fixtures/main/align_columns.rs | 1 + .../planner_fixtures/main/required_columns.rs | 7 +- .../src/planner_fixtures/tests.rs | 2 + .../planner_fixtures/tests/test_alignment.rs | 9 ++ .../planner_fixtures/tests/test_required.rs | 116 ++++++++++++++++++ .../src/planner_fixtures/tests/test_types.rs | 9 ++ .../cli/commands/main/test/_test_types.py | 1 + .../cli/commands/main/test/helpers.py | 52 ++++++++ .../cli/commands/main/test/test_test.py | 11 ++ .../compiler/planner/_helpers/_test_types.py | 8 ++ .../planner/_helpers/test_fixture_planning.py | 83 +++++++++++++ 11 files changed, 296 insertions(+), 3 deletions(-) create mode 100644 crates/sqlbuild-analysis/src/planner_fixtures/tests/test_required.rs diff --git a/crates/sqlbuild-analysis/src/planner_fixtures/main/align_columns.rs b/crates/sqlbuild-analysis/src/planner_fixtures/main/align_columns.rs index 6d7a0ca9cd..3797aff2ed 100644 --- a/crates/sqlbuild-analysis/src/planner_fixtures/main/align_columns.rs +++ b/crates/sqlbuild-analysis/src/planner_fixtures/main/align_columns.rs @@ -52,6 +52,7 @@ pub fn align_columns(label: &str, input: AlignmentInput<'_>) -> Alignment { }; } completed.sort_unstable(); + completed.dedup(); Alignment { diagnostics, completed: Some(completed), diff --git a/crates/sqlbuild-analysis/src/planner_fixtures/main/required_columns.rs b/crates/sqlbuild-analysis/src/planner_fixtures/main/required_columns.rs index d8a2c70a58..d2a2c86b91 100644 --- a/crates/sqlbuild-analysis/src/planner_fixtures/main/required_columns.rs +++ b/crates/sqlbuild-analysis/src/planner_fixtures/main/required_columns.rs @@ -80,16 +80,17 @@ fn add_required( result.columns.len() - 1 }); let columns = &mut result.columns[position].1; - if let Some(index) = result.column_indices[position].get(name) { + let folded_name = python_casefold(name); + if let Some(index) = result.column_indices[position].get(&folded_name) { let column = &mut columns[*index]; if column.readers.last().is_none_or(|reader| reader != model) { column.readers.push(model.to_owned()); } } else { - result.column_indices[position].insert(name.to_owned(), columns.len()); + result.column_indices[position].insert(folded_name.clone(), columns.len()); columns.push(RequiredColumn { name: name.to_owned(), - folded_name: python_casefold(name), + folded_name, readers: vec![model.to_owned()], }); } diff --git a/crates/sqlbuild-analysis/src/planner_fixtures/tests.rs b/crates/sqlbuild-analysis/src/planner_fixtures/tests.rs index 3684bc64a0..38606aa81a 100644 --- a/crates/sqlbuild-analysis/src/planner_fixtures/tests.rs +++ b/crates/sqlbuild-analysis/src/planner_fixtures/tests.rs @@ -6,6 +6,8 @@ mod test_alignment; mod test_propagation; #[path = "tests/test_reads.rs"] mod test_reads; +#[path = "tests/test_required.rs"] +mod test_required; #[path = "tests/test_schema.rs"] mod test_schema; #[path = "tests/test_types.rs"] diff --git a/crates/sqlbuild-analysis/src/planner_fixtures/tests/test_alignment.rs b/crates/sqlbuild-analysis/src/planner_fixtures/tests/test_alignment.rs index 0274d14185..72f497c9fc 100644 --- a/crates/sqlbuild-analysis/src/planner_fixtures/tests/test_alignment.rs +++ b/crates/sqlbuild-analysis/src/planner_fixtures/tests/test_alignment.rs @@ -15,6 +15,15 @@ fn given_partial_fixtures_when_aligning_then_preserves_schema_and_diagnostic_ord expected_completed: Some(vec![1, 2]), expected_diagnostics: &[], }, + AlignmentTestCase { + description: "duplicate requirement completes once", + supplied: &["order_id"], + required: &["status", "status"], + star: false, + typed_nulls: false, + expected_completed: Some(vec![1]), + expected_diagnostics: &[], + }, AlignmentTestCase { description: "unknown and required diagnostics", supplied: &["details"], diff --git a/crates/sqlbuild-analysis/src/planner_fixtures/tests/test_required.rs b/crates/sqlbuild-analysis/src/planner_fixtures/tests/test_required.rs new file mode 100644 index 0000000000..d912689876 --- /dev/null +++ b/crates/sqlbuild-analysis/src/planner_fixtures/tests/test_required.rs @@ -0,0 +1,116 @@ +use std::collections::{HashMap, HashSet}; + +use crate::planner_fixtures::main::align_columns::align_columns; +use crate::planner_fixtures::main::required_columns::required_columns; +use crate::planner_fixtures::models::{ + AlignmentInput, FixtureColumn, FixtureReadCache, FixtureReadKey, FixtureReadModel, + FixtureReadSchemas, FixtureStarRelation, +}; +use crate::planner_fixtures::tests::helpers::column; +use crate::planner_fixtures::tests::test_types::RequiredColumnsTestCase; +use crate::required_columns::models::ColumnRead; + +#[test] +fn given_star_and_differently_cased_reads_when_requiring_then_each_column_is_required_once() { + let test_cases = [ + RequiredColumnsTestCase { + description: "upper-case reads through a star import", + reads: &["ORDER_ID", "STATUS"], + star: true, + expected_required: &["ORDER_ID", "STATUS", "customer_id"], + expected_completed: &[1, 2], + }, + RequiredColumnsTestCase { + description: "same-case reads through a star import", + reads: &["order_id", "status"], + star: true, + expected_required: &["order_id", "status", "customer_id"], + expected_completed: &[1, 2], + }, + RequiredColumnsTestCase { + description: "upper-case reads without a star", + reads: &["ORDER_ID", "STATUS", "Status"], + star: false, + expected_required: &["ORDER_ID", "STATUS"], + expected_completed: &[2], + }, + ]; + let key = ("source".to_owned(), "orders".to_owned()); + let metadata = vec![ + column("order_id", "VARCHAR", true), + column("customer_id", "VARCHAR", true), + column("status", "VARCHAR", true), + ]; + let keys = HashSet::from([key.clone()]); + let authoritative = HashMap::from([( + key.clone(), + metadata + .iter() + .map(|column| column.folded_name.clone()) + .collect::>(), + )]); + let relations = [FixtureStarRelation { + key: key.clone(), + columns: metadata.clone(), + authoritative: true, + }]; + for test_case in test_cases { + let models = [FixtureReadModel { + name: "orders_report".into(), + key: FixtureReadKey { + sql: String::new(), + references: Vec::new(), + dialect: Some("snowflake".into()), + }, + high_confidence: test_case + .reads + .iter() + .map(|name| ColumnRead { + resource_type: key.0.clone(), + resource_name: key.1.clone(), + column_name: (*name).into(), + }) + .collect(), + stars: test_case.star.then(|| key.clone()).into_iter().collect(), + }]; + let (_, result) = required_columns( + FixtureReadCache::default(), + &models, + FixtureReadSchemas { + keys: &keys, + authoritative: &authoritative, + relations: &relations, + }, + ); + let requirements = result.expect(test_case.description); + let required = &requirements.columns[0].1; + let names: Vec<&str> = required.iter().map(|column| column.name.as_str()).collect(); + assert_eq!( + names, test_case.expected_required, + "{}", + test_case.description + ); + let supplied = [FixtureColumn { + name: "order_id".into(), + folded_name: "order_id".into(), + sql_type: None, + nullable: None, + }]; + let alignment = align_columns( + "mock source 'orders'", + AlignmentInput { + supplied: &supplied, + metadata: &metadata, + required, + star: test_case.star, + typed_nulls: false, + }, + ); + assert_eq!( + alignment.completed.as_deref(), + Some(test_case.expected_completed), + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-analysis/src/planner_fixtures/tests/test_types.rs b/crates/sqlbuild-analysis/src/planner_fixtures/tests/test_types.rs index acbc523c4b..6dc5c8b85d 100644 --- a/crates/sqlbuild-analysis/src/planner_fixtures/tests/test_types.rs +++ b/crates/sqlbuild-analysis/src/planner_fixtures/tests/test_types.rs @@ -65,3 +65,12 @@ pub(crate) struct CollectionMismatchTestCase { pub(crate) inferred_type: Option<&'static str>, pub(crate) expected_mismatch: bool, } + +#[derive(Debug)] +pub(crate) struct RequiredColumnsTestCase { + pub(crate) description: &'static str, + pub(crate) reads: &'static [&'static str], + pub(crate) star: bool, + pub(crate) expected_required: &'static [&'static str], + pub(crate) expected_completed: &'static [usize], +} diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/_test_types.py index b395a42855..4a49c1b251 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/_test_types.py @@ -88,6 +88,7 @@ class PartialFixtureE2ETestCase: expected_artifact_fragments: tuple[str, ...] unexpected_artifact_fragments: tuple[str, ...] = field(default_factory=tuple) artifact_filename: str = "test_orders.sql" + single_artifact_fragments: tuple[str, ...] = field(default_factory=tuple) @dataclass(frozen=True) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/helpers.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/helpers.py index 9028627451..a1e0f63271 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/helpers.py @@ -821,6 +821,58 @@ def build_mixed_case_partial_fixture_project_files() -> dict[str, str]: return files +def build_upper_case_star_import_partial_fixture_project_files() -> dict[str, str]: + """Build upper-case references to lower-case source columns through a star import CTE.""" + + return { + "sqlbuild_project.toml": ( + 'name = "partial_source_fixture"\n' + 'adapter = "duckdb"\n\n' + "[connection]\n" + 'database = "partial_source_fixture.duckdb"\n' + ), + "sources/raw.yml": ( + "sources:\n" + " - name: raw__orders\n description: Raw order records\n" + " schema: raw\n" + " table: orders\n" + " columns:\n" + " - name: order_id\n type: VARCHAR\n" + " - name: customer_id\n type: VARCHAR\n" + " - name: status\n type: VARCHAR\n" + ), + "models/orders.sql": ( + 'MODEL (description "Orders with their status", materialized table);\n\n' + "WITH raw__orders AS (\n" + " SELECT *\n" + ' FROM __source("raw__orders")\n' + "),\n\n" + "orders AS (\n" + " SELECT\n" + " ORDER_ID,\n" + " STATUS\n" + " FROM raw__orders\n" + ")\n\n" + "SELECT\n" + " ORDER_ID AS order_id,\n" + " STATUS AS status\n" + "FROM orders\n" + ), + "tests/unit/test_orders.sql": ( + 'TEST (name "orders__keeps_status");\n\n' + "WITH __source__raw__orders AS (\n" + " SELECT CAST(column1 AS VARCHAR) AS order_id\n" + " FROM VALUES ('o1') AS t(column1)\n" + "),\n\n" + "__expected__orders AS (\n" + " SELECT CAST(column1 AS VARCHAR) AS order_id\n" + " FROM VALUES ('o1') AS t(column1)\n" + ")\n\n" + "SELECT 1\n" + ), + } + + def build_invalid_partial_fixture_project_files( *, status_column_attributes: str, supplied_column: str = "order_id" ) -> dict[str, str]: diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_test.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_test.py index e03de5d349..d589e6c021 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_test.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_test.py @@ -66,6 +66,7 @@ build_unsatisfied_leaf_test_project_files, build_unspecified_nullability_fixture_project_files, build_untyped_null_fixture_project_files, + build_upper_case_star_import_partial_fixture_project_files, ) from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import ( assert_fragments_in_order, @@ -246,6 +247,14 @@ ), unexpected_artifact_fragments=('"__sqlbuild_partial_fixture"."Status"',), ), + PartialFixtureE2ETestCase( + description="upper-case reads through a star import CTE fill each column once", + repo_files=build_upper_case_star_import_partial_fixture_project_files(), + expected_stdout_fragment="PASS=1", + expected_artifact_fragments=("__sqlbuild_partial_fixture",), + single_artifact_fragments=('CAST(NULL AS VARCHAR) AS "status"',), + artifact_filename="orders__keeps_status.sql", + ), ), ids=lambda case: case.description, ) @@ -275,6 +284,8 @@ def test_given_required_typed_column_omitted_when_testing_then_completes_fixture assert fragment in artifact_sql for fragment in test_case.unexpected_artifact_fragments: assert fragment not in artifact_sql + for fragment in test_case.single_artifact_fragments: + assert artifact_sql.count(fragment) == 1, artifact_sql @pytest.mark.parametrize( diff --git a/tests/integration/src/sqlbuild/compiler/planner/_helpers/_test_types.py b/tests/integration/src/sqlbuild/compiler/planner/_helpers/_test_types.py index 9f9f5861ca..365c03d796 100644 --- a/tests/integration/src/sqlbuild/compiler/planner/_helpers/_test_types.py +++ b/tests/integration/src/sqlbuild/compiler/planner/_helpers/_test_types.py @@ -18,6 +18,14 @@ class FixturePlanningIntegrationTestCase: query_sql: str = 'SELECT order_id, status FROM __source("raw_orders")' +@dataclass(frozen=True) +class CaseFoldedFixtureFillTestCase: + description: str + adapter_name: str + project_toml: str + expected_single_fragments: tuple[str, ...] + + @dataclass(frozen=True) class GatherWatermarkTypeTestCase: description: str diff --git a/tests/integration/src/sqlbuild/compiler/planner/_helpers/test_fixture_planning.py b/tests/integration/src/sqlbuild/compiler/planner/_helpers/test_fixture_planning.py index f60d8ca0e2..233cc82020 100644 --- a/tests/integration/src/sqlbuild/compiler/planner/_helpers/test_fixture_planning.py +++ b/tests/integration/src/sqlbuild/compiler/planner/_helpers/test_fixture_planning.py @@ -18,13 +18,21 @@ build_selected_test_entries, discover_project_inputs, ) +from sqlbuild.adapter.contract.classes.base_adapter import BaseAdapter from sqlbuild.adapters.duckdb.classes.duckdb_adapter import DuckDbAdapter +from sqlbuild.adapters.snowflake.classes.snowflake_adapter import SnowflakeAdapter from sqlbuild.compiler.compile.types import CompiledProjectView from tests.integration.src.sqlbuild.compiler.planner._helpers._test_types import ( + CaseFoldedFixtureFillTestCase, FixturePlanningIntegrationTestCase, FixtureSchemaIntegrationTestCase, ) +_ADAPTERS: dict[str, Callable[[], BaseAdapter]] = { + "duckdb": DuckDbAdapter, + "snowflake": SnowflakeAdapter, +} + @pytest.mark.parametrize( "test_case", @@ -107,6 +115,81 @@ def test_given_partial_fixture_when_compiling_and_planning_then_completes_requir assert test_case.expected_sql_fragment in entry.chain[0].resolved_sql +@pytest.mark.parametrize( + "test_case", + ( + CaseFoldedFixtureFillTestCase( + description="duckdb fills each omitted column once", + adapter_name="duckdb", + project_toml='adapter = "duckdb"\n\n[connection]\ndatabase = ":memory:"\n', + expected_single_fragments=('CAST(NULL AS VARCHAR) AS "status"',), + ), + CaseFoldedFixtureFillTestCase( + description="snowflake fills each omitted column once", + adapter_name="snowflake", + project_toml='adapter = "snowflake"\n', + expected_single_fragments=('CAST(NULL AS VARCHAR) AS "STATUS"',), + ), + ), + ids=lambda case: case.description, +) +def test_given_upper_case_reads_through_star_import_when_planning_then_fills_each_column_once( + test_case: CaseFoldedFixtureFillTestCase, + tmp_path: Path, + write_repo_files: Callable[[Path, dict[str, str]], None], +) -> None: + write_repo_files( + tmp_path, + { + "sqlbuild_project.toml": f'name = "fixture_fill_demo"\n{test_case.project_toml}', + "sources/raw.yml": ( + "sources:\n" + " - name: raw__orders\n description: Raw order records\n" + " database: example_db\n" + " schema: raw\n" + " table: orders\n" + " columns:\n" + " - name: order_id\n type: VARCHAR\n" + " - name: customer_id\n type: VARCHAR\n" + " - name: status\n type: VARCHAR\n" + ), + "models/orders.sql": ( + 'MODEL (description "Orders with their status.", database example_db, ' + "schema analytics);\n\n" + 'WITH raw__orders AS (SELECT * FROM __source("raw__orders")),\n' + "orders AS (SELECT ORDER_ID, STATUS FROM raw__orders)\n" + "SELECT ORDER_ID AS order_id, STATUS AS status FROM orders\n" + ), + "tests/unit/test_orders.sql": ( + "TEST();\n\n" + "WITH\n" + "__source__raw__orders AS (SELECT CAST('o1' AS VARCHAR) AS order_id),\n" + "__expected__orders AS (SELECT CAST('o1' AS VARCHAR) AS order_id)\n" + "SELECT 1\n" + ), + }, + ) + discovered_inputs: DiscoveredProjectInputs = discover_project_inputs(project_dir=tmp_path) + adapter: BaseAdapter = _ADAPTERS[test_case.adapter_name]() + + project: CompiledProjectView = build_compiled_project( + discovered_inputs=discovered_inputs, + adapter=adapter, + ) + entries, warnings = build_selected_test_entries( + sql_test_selection=SqlTestSelection(), + project=project, + adapter=adapter, + selected_keys=frozenset((project.models[0].key,)), + ) + + assert warnings == [] + assert len(entries) == 1 + resolved_sql: str = entries[0].chain[0].resolved_sql + for fragment in test_case.expected_single_fragments: + assert resolved_sql.count(fragment) == 1, resolved_sql + + @pytest.mark.parametrize( "test_case", ( From 41f59807854b591adc415a1749e18aa5a81550be Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 19:32:08 +0100 Subject: [PATCH 02/12] fix: reject IS [NOT] TRUE/FALSE on Snowflake and SQL Server at compile Both warehouses reject truth-value tests the SQL parser accepts, so models, audits, SQL tests and hooks compiled cleanly and failed at execution. Report a blocking B103 at the IS keyword of the authored resource with a null-safe rewrite. DuckDB, MotherDuck, PostgreSQL, BigQuery and Databricks accept them. Fixes CHI-631. --- .../_helpers/resource_sql/locations.rs | 29 ++ .../src/semantic_checks/constants.rs | 10 + .../_helpers/diagnostics.rs | 2 + .../_helpers/function_names.rs | 14 +- .../src/semantic_validation/_helpers/mod.rs | 1 + .../_helpers/truth_tests.rs | 136 +++++++++ .../src/semantic_validation/main.rs | 1 + .../main/truth_test_spans.rs | 9 + .../src/semantic_validation/tests.rs | 2 + .../tests/test_truth_tests.rs | 192 ++++++++++++ .../semantic_validation/tests/test_types.rs | 21 ++ .../_helpers/analysis_corpus/failure_cases.py | 13 + .../docs/concepts/semantic-compilation.md | 9 + .../cli/commands/main/compile/_test_types.py | 17 ++ .../main/compile/test_truth_value_tests.py | 188 ++++++++++++ .../analysis-unsupported-truth-test.json | 274 ++++++++++++++++++ .../_helpers/corpus/test_analysis_codes.py | 1 + .../docs/concepts/semantic-compilation.mdx | 9 + 18 files changed, 921 insertions(+), 7 deletions(-) create mode 100644 crates/sqlbuild-analysis/src/semantic_validation/_helpers/truth_tests.rs create mode 100644 crates/sqlbuild-analysis/src/semantic_validation/main/truth_test_spans.rs create mode 100644 crates/sqlbuild-analysis/src/semantic_validation/tests/test_truth_tests.rs create mode 100644 tests/e2e/src/sqlbuild/cli/commands/main/compile/test_truth_value_tests.py create mode 100644 tests/goldens/compiler/failure/analysis-unsupported-truth-test.json diff --git a/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/locations.rs b/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/locations.rs index 4174c247a8..0cc89868c2 100644 --- a/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/locations.rs +++ b/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/locations.rs @@ -5,6 +5,7 @@ use sqlbuild_core::text::main::ignorecase_key::ignorecase_key; use crate::semantic_checks::_helpers::resource_sql::resources::{ResourceSql, find_code_points}; use crate::semantic_checks::_helpers::resource_sql::scanning::word_at; use crate::semantic_checks::models::ResourceSqlFailure; +use crate::semantic_validation::main::truth_test_spans::truth_test_spans; use crate::semantic_validation::models::{BindingPositions, PositionInput}; /// A binding finding: code, message, line, column, start, end and severity. @@ -18,6 +19,8 @@ pub(crate) type Finding = ( String, ); +const UNSUPPORTED_TRUTH_TEST_CODE: &str = "B103"; + /// `(line, column, end line, end column)`. pub(crate) type Span = (i64, i64, Option, Option); @@ -115,6 +118,25 @@ fn mapped_span( ))) } +/// The authored span of the truth-value test a B103 finding starts at, matched by its order. +fn truth_test_span(resource: &ResourceSql, cleaned_sql: &str, start: usize) -> Option { + let cleaned: Vec<(usize, usize)> = truth_test_spans(cleaned_sql); + let order: usize = cleaned + .iter() + .position(|(test_start, _)| cleaned_sql[..*test_start].chars().count() == start)?; + let authored: Vec<(usize, usize)> = truth_test_spans(&resource.authored_body); + if authored.len() != cleaned.len() { + return None; + } + let (test_start, test_end) = *authored.get(order)?; + let body_offset: usize = find_code_points(&resource.contents, &resource.authored_body)?; + let offset: usize = body_offset + resource.authored_body[..test_start].chars().count(); + let length: usize = resource.authored_body[test_start..test_end].chars().count(); + let (line, column) = offset_location(&resource.contents, offset); + let (end_line, end_column) = offset_location(&resource.contents, offset + length); + Some((line, column, Some(end_line), Some(end_column))) +} + /// Python's `_location`: the offending authored text, falling back to the body start. pub(crate) fn location( resource: &ResourceSql, @@ -129,6 +151,13 @@ pub(crate) fn location( return Ok((line, column, None, None)); } } + if finding.0 == UNSUPPORTED_TRUTH_TEST_CODE + && let Some(span) = finding + .4 + .and_then(|start| truth_test_span(resource, cleaned_sql, start)) + { + return Ok(span); + } if let Some(span) = mapped_span(resource, cleaned_sql, finding)? { return Ok(span); } diff --git a/crates/sqlbuild-analysis/src/semantic_checks/constants.rs b/crates/sqlbuild-analysis/src/semantic_checks/constants.rs index fc721ba5db..0f5f5702c9 100644 --- a/crates/sqlbuild-analysis/src/semantic_checks/constants.rs +++ b/crates/sqlbuild-analysis/src/semantic_checks/constants.rs @@ -127,6 +127,11 @@ pub(crate) const SEMANTIC_HELP: &[(&str, &str)] = &[ "B102", "pass the number of arguments required by this function's signature", ), + ( + "B103", + "rewrite the truth-value test null-safely, for example COALESCE(x, FALSE) for x IS TRUE, \ + and NOT COALESCE(x, FALSE) or x IS DISTINCT FROM TRUE for x IS NOT TRUE", + ), ( "B210", "use an expression whose inferred type satisfies this SQL construct", @@ -202,6 +207,11 @@ pub(crate) const FINDING_KINDS: &[(&str, &str, &str)] = &[ ("^B00[45]$", "reference error", "reference errors"), ("^B101$", "unknown function", "unknown functions"), ("^B102$", "function arity error", "function arity errors"), + ( + "^B103$", + "unsupported truth-value test", + "unsupported truth-value tests", + ), ("^B216$", "column count mismatch", "column count mismatches"), ("^B21\\d$", "type mismatch", "type mismatches"), ( diff --git a/crates/sqlbuild-analysis/src/semantic_validation/_helpers/diagnostics.rs b/crates/sqlbuild-analysis/src/semantic_validation/_helpers/diagnostics.rs index 80bb11cdff..0983e58f70 100644 --- a/crates/sqlbuild-analysis/src/semantic_validation/_helpers/diagnostics.rs +++ b/crates/sqlbuild-analysis/src/semantic_validation/_helpers/diagnostics.rs @@ -2,6 +2,7 @@ use crate::semantic_validation::_helpers::catalog::diagnostic_row; use crate::semantic_validation::_helpers::function_names; +use crate::semantic_validation::_helpers::truth_tests; use crate::semantic_validation::_helpers::type_locations::locate_type_findings; use crate::semantic_validation::models::FunctionProbes; use crate::semantic_validation::types::DiagnosticRow; @@ -165,6 +166,7 @@ pub(crate) fn map_diagnostics( errors.extend(function_names::unsupported_function_errors( sql, dialect, probes, )); + errors.extend(truth_tests::unsupported_truth_test_errors(sql, dialect)); let valid = !errors .iter() .any(|error| error.severity == ValidationSeverity::Error); diff --git a/crates/sqlbuild-analysis/src/semantic_validation/_helpers/function_names.rs b/crates/sqlbuild-analysis/src/semantic_validation/_helpers/function_names.rs index c4c20a44d1..fba1e4be4c 100644 --- a/crates/sqlbuild-analysis/src/semantic_validation/_helpers/function_names.rs +++ b/crates/sqlbuild-analysis/src/semantic_validation/_helpers/function_names.rs @@ -111,11 +111,11 @@ struct CatalogNames { } /// Incremental byte-to-character position, so locating every call stays linear in the SQL. -struct Cursor { +pub(crate) struct Cursor { byte: usize, - characters: usize, - line: usize, - column: usize, + pub(crate) characters: usize, + pub(crate) line: usize, + pub(crate) column: usize, } impl CatalogSink for CatalogNames { @@ -142,7 +142,7 @@ impl CatalogSink for CatalogNames { } impl Cursor { - fn new() -> Self { + pub(crate) fn new() -> Self { Self { byte: 0, characters: 0, @@ -151,7 +151,7 @@ impl Cursor { } } - fn advance(&mut self, sql: &str, byte: usize) { + pub(crate) fn advance(&mut self, sql: &str, byte: usize) { for character in sql[self.byte..byte].chars() { self.characters += 1; if character == '\n' { @@ -412,7 +412,7 @@ fn keyword_end(sql: &[u8], index: usize, keyword: &str) -> Option { .then_some(end) } -fn is_word_byte(byte: u8) -> bool { +pub(crate) fn is_word_byte(byte: u8) -> bool { byte.is_ascii_alphanumeric() || byte == b'_' || byte == b'$' } diff --git a/crates/sqlbuild-analysis/src/semantic_validation/_helpers/mod.rs b/crates/sqlbuild-analysis/src/semantic_validation/_helpers/mod.rs index d9b898d3e4..b75be9835b 100644 --- a/crates/sqlbuild-analysis/src/semantic_validation/_helpers/mod.rs +++ b/crates/sqlbuild-analysis/src/semantic_validation/_helpers/mod.rs @@ -4,4 +4,5 @@ pub mod function_names; pub mod identifiers; pub mod normalization; pub mod positions; +pub mod truth_tests; pub mod type_locations; diff --git a/crates/sqlbuild-analysis/src/semantic_validation/_helpers/truth_tests.rs b/crates/sqlbuild-analysis/src/semantic_validation/_helpers/truth_tests.rs new file mode 100644 index 0000000000..896b12f20d --- /dev/null +++ b/crates/sqlbuild-analysis/src/semantic_validation/_helpers/truth_tests.rs @@ -0,0 +1,136 @@ +//! Truth-value tests (`IS [NOT] TRUE` / `IS [NOT] FALSE`) on dialects whose warehouse rejects them. + +use crate::semantic_validation::_helpers::function_names::{Cursor, is_word_byte}; +use polyglot_sql::{DialectType, ValidationError}; +use sqlbuild_sqltext::sql_scan::main::non_code_end::non_code_end; +use sqlbuild_sqltext::sql_scan::models::{QuotePolicy, Unclosed}; + +pub(crate) const UNSUPPORTED_TRUTH_TEST_CODE: &str = "B103"; +const IS_KEYWORD: &str = "IS"; +const NOT_KEYWORD: &str = "NOT"; +const TRUE_KEYWORD: &str = "TRUE"; +const FALSE_KEYWORD: &str = "FALSE"; + +/// One authored `IS [NOT] TRUE` or `IS [NOT] FALSE`, spanning `IS` through the truth value. +#[derive(Debug, PartialEq, Eq)] +pub(crate) struct TruthTest { + pub(crate) start: usize, + pub(crate) end: usize, + pub(crate) negated: bool, + pub(crate) value: bool, +} + +/// Return located B103 diagnostics for truth-value tests the dialect's warehouse rejects. +pub(crate) fn unsupported_truth_test_errors( + sql: &str, + dialect: DialectType, +) -> Vec { + if !rejects_truth_tests(dialect) { + return Vec::new(); + } + let mut cursor: Cursor = Cursor::new(); + truth_tests(sql) + .into_iter() + .map(|test| { + let message: String = format!( + "'{}' is not supported for dialect {dialect:?}; write {}", + spelling(&test), + rewrite(dialect, &test) + ); + cursor.advance(sql, test.start); + let start: usize = cursor.characters; + let end: usize = start + sql[test.start..test.end].chars().count(); + ValidationError::error(message, UNSUPPORTED_TRUTH_TEST_CODE) + .with_location(cursor.line, cursor.column) + .with_span(Some(start), Some(end)) + }) + .collect() +} + +/// Whether the dialect's warehouse rejects `IS [NOT] TRUE` and `IS [NOT] FALSE`. +pub(crate) fn rejects_truth_tests(dialect: DialectType) -> bool { + matches!(dialect, DialectType::Snowflake | DialectType::TSQL) +} + +/// Return every `IS [NOT] TRUE|FALSE` keyword sequence outside comments and quoted text. +pub(crate) fn truth_tests(sql: &str) -> Vec { + let bytes: &[u8] = sql.as_bytes(); + let mut tests: Vec = Vec::new(); + let mut pending: Option<(usize, bool)> = None; + let mut index: usize = 0; + while index < bytes.len() { + let byte: u8 = bytes[index]; + if byte.is_ascii_whitespace() { + index += 1; + continue; + } + match non_code_end(bytes, index, QuotePolicy::COMPILER) { + Ok(Some(end)) => { + if !matches!(byte, b'-' | b'/') { + pending = None; + } + index = end; + continue; + } + Ok(None) => {} + Err(Unclosed::BlockComment | Unclosed::Quote | Unclosed::Parenthesis) => break, + } + if !is_word_byte(byte) { + pending = None; + index += 1; + continue; + } + let start: usize = index; + while index < bytes.len() && is_word_byte(bytes[index]) { + index += 1; + } + let word: &str = &sql[start..index]; + pending = match pending { + Some((is_start, false)) if word.eq_ignore_ascii_case(NOT_KEYWORD) => { + Some((is_start, true)) + } + Some((is_start, negated)) + if word.eq_ignore_ascii_case(TRUE_KEYWORD) + || word.eq_ignore_ascii_case(FALSE_KEYWORD) => + { + tests.push(TruthTest { + start: is_start, + end: index, + negated, + value: word.eq_ignore_ascii_case(TRUE_KEYWORD), + }); + None + } + _ => word + .eq_ignore_ascii_case(IS_KEYWORD) + .then_some((start, false)), + }; + } + tests +} + +fn spelling(test: &TruthTest) -> String { + format!( + "{IS_KEYWORD} {}{}", + if test.negated { "NOT " } else { "" }, + if test.value { + TRUE_KEYWORD + } else { + FALSE_KEYWORD + } + ) +} + +/// The null-safe equivalent of one truth-value test in the dialect. +fn rewrite(dialect: DialectType, test: &TruthTest) -> &'static str { + match (dialect, test.negated, test.value) { + (DialectType::TSQL, false, true) => "COALESCE(, 0) = 1", + (DialectType::TSQL, true, true) => "COALESCE(, 0) = 0", + (DialectType::TSQL, false, false) => "COALESCE(, 1) = 0", + (DialectType::TSQL, true, false) => "COALESCE(, 1) = 1", + (_, false, true) => "COALESCE(, FALSE)", + (_, true, true) => "NOT COALESCE(, FALSE) or IS DISTINCT FROM TRUE", + (_, false, false) => "NOT COALESCE(, TRUE)", + (_, true, false) => "COALESCE(, TRUE) or IS DISTINCT FROM FALSE", + } +} diff --git a/crates/sqlbuild-analysis/src/semantic_validation/main.rs b/crates/sqlbuild-analysis/src/semantic_validation/main.rs index 9190654d75..a5d638eb14 100644 --- a/crates/sqlbuild-analysis/src/semantic_validation/main.rs +++ b/crates/sqlbuild-analysis/src/semantic_validation/main.rs @@ -7,6 +7,7 @@ pub mod normalize; pub mod normalize_batch; pub mod normalize_dialect; pub mod normalize_mapped; +pub mod truth_test_spans; use crate::semantic_validation::models::ProjectCatalog; use polyglot_sql::{ diff --git a/crates/sqlbuild-analysis/src/semantic_validation/main/truth_test_spans.rs b/crates/sqlbuild-analysis/src/semantic_validation/main/truth_test_spans.rs new file mode 100644 index 0000000000..955bae7796 --- /dev/null +++ b/crates/sqlbuild-analysis/src/semantic_validation/main/truth_test_spans.rs @@ -0,0 +1,9 @@ +use crate::semantic_validation::_helpers::truth_tests::truth_tests; + +/// Byte spans of every `IS [NOT] TRUE|FALSE` in `sql`, outside comments and quoted text. +pub(crate) fn truth_test_spans(sql: &str) -> Vec<(usize, usize)> { + truth_tests(sql) + .into_iter() + .map(|test| (test.start, test.end)) + .collect() +} diff --git a/crates/sqlbuild-analysis/src/semantic_validation/tests.rs b/crates/sqlbuild-analysis/src/semantic_validation/tests.rs index 418d1ace8a..21e41d1335 100644 --- a/crates/sqlbuild-analysis/src/semantic_validation/tests.rs +++ b/crates/sqlbuild-analysis/src/semantic_validation/tests.rs @@ -8,5 +8,7 @@ mod positions; mod semantic_validation; #[path = "tests/test_types.rs"] mod test_types; +#[path = "tests/test_truth_tests.rs"] +mod truth_tests; #[path = "tests/test_type_locations.rs"] mod type_locations; diff --git a/crates/sqlbuild-analysis/src/semantic_validation/tests/test_truth_tests.rs b/crates/sqlbuild-analysis/src/semantic_validation/tests/test_truth_tests.rs new file mode 100644 index 0000000000..f9ba73f68f --- /dev/null +++ b/crates/sqlbuild-analysis/src/semantic_validation/tests/test_truth_tests.rs @@ -0,0 +1,192 @@ +use crate::semantic_validation::_helpers::diagnostics::map_diagnostics; +use crate::semantic_validation::_helpers::truth_tests::{rejects_truth_tests, truth_tests}; +use crate::semantic_validation::models::FunctionProbes; +use crate::semantic_validation::tests::test_types::{ + LocatedTruthTestDiagnosticTestCase, TruthTestDialectTestCase, TruthTestScanTestCase, +}; +use polyglot_sql::{DialectType, ValidationResult}; + +#[test] +fn given_authored_sql_when_scanning_truth_tests_then_finds_only_keyword_sequences() { + let test_cases = [ + TruthTestScanTestCase { + description: "every truth value and negation is found", + sql: "SELECT a IS TRUE, b IS NOT TRUE, c is false, d Is Not False FROM t", + expected_tests: &[ + (9, 16, false, true), + (20, 31, true, true), + (35, 43, false, false), + (47, 59, true, false), + ], + }, + TruthTestScanTestCase { + description: "comments between keywords keep the sequence", + sql: "SELECT 1 FROM t WHERE flag IS /* note */ NOT -- why\n TRUE", + expected_tests: &[(27, 57, true, true)], + }, + TruthTestScanTestCase { + description: "NULL, DISTINCT FROM and other IS predicates are ignored", + sql: "SELECT 1 FROM t WHERE a IS NULL AND b IS NOT NULL AND c IS DISTINCT FROM TRUE", + expected_tests: &[], + }, + TruthTestScanTestCase { + description: "quoted text, quoted identifiers and comments are ignored", + sql: "SELECT 'x IS TRUE', \"IS\" AS y, z IS \"TRUE\" FROM t -- a IS TRUE\n/* b IS FALSE */", + expected_tests: &[], + }, + TruthTestScanTestCase { + description: "a word ending in IS is not the IS keyword", + sql: "SELECT axis TRUE, this NOT FALSE FROM t", + expected_tests: &[], + }, + ]; + for test_case in test_cases { + let found: Vec<(usize, usize, bool, bool)> = truth_tests(test_case.sql) + .iter() + .map(|test| (test.start, test.end, test.negated, test.value)) + .collect(); + assert_eq!(found, test_case.expected_tests, "{}", test_case.description); + } +} + +#[test] +fn given_dialect_when_checking_truth_tests_then_only_rejecting_warehouses_report() { + let test_cases = [ + TruthTestDialectTestCase { + description: "Snowflake rejects truth-value tests", + dialect: DialectType::Snowflake, + expected_rejected: true, + }, + TruthTestDialectTestCase { + description: "SQL Server rejects truth-value tests", + dialect: DialectType::TSQL, + expected_rejected: true, + }, + TruthTestDialectTestCase { + description: "DuckDB accepts truth-value tests", + dialect: DialectType::DuckDB, + expected_rejected: false, + }, + TruthTestDialectTestCase { + description: "PostgreSQL accepts truth-value tests", + dialect: DialectType::PostgreSQL, + expected_rejected: false, + }, + TruthTestDialectTestCase { + description: "BigQuery accepts truth-value tests", + dialect: DialectType::BigQuery, + expected_rejected: false, + }, + TruthTestDialectTestCase { + description: "Databricks accepts truth-value tests", + dialect: DialectType::Databricks, + expected_rejected: false, + }, + ]; + for test_case in test_cases { + let result = map_diagnostics( + "SELECT 1 FROM t WHERE flag IS NOT TRUE", + test_case.dialect, + ValidationResult { + valid: true, + errors: Vec::new(), + }, + &FunctionProbes::default(), + ) + .expect(test_case.description); + assert_eq!( + rejects_truth_tests(test_case.dialect), + test_case.expected_rejected, + "{}", + test_case.description + ); + assert_eq!( + result.valid, !test_case.expected_rejected, + "{}", + test_case.description + ); + assert_eq!( + result + .errors + .iter() + .map(|error| error.code.as_str()) + .collect::>(), + if test_case.expected_rejected { + vec!["B103"] + } else { + Vec::new() + }, + "{}", + test_case.description + ); + } +} + +#[test] +fn given_rejected_truth_test_when_mapping_diagnostics_then_returns_located_b103() +-> Result<(), String> { + let test_cases = [ + LocatedTruthTestDiagnosticTestCase { + description: "Snowflake points at IS after a multibyte literal", + dialect: DialectType::Snowflake, + sql: "SELECT 'é' AS label\nFROM orders\nWHERE is_test_order IS NOT TRUE", + expected_message: "'IS NOT TRUE' is not supported for dialect Snowflake; \ + write NOT COALESCE(, FALSE) or IS DISTINCT FROM TRUE", + expected_location: (Some(3), Some(21)), + expected_span: (Some(52), Some(63)), + }, + LocatedTruthTestDiagnosticTestCase { + description: "Snowflake IS FALSE suggests the TRUE default", + dialect: DialectType::Snowflake, + sql: "SELECT 1 FROM orders WHERE is_test_order is false", + expected_message: "'IS FALSE' is not supported for dialect Snowflake; \ + write NOT COALESCE(, TRUE)", + expected_location: (Some(1), Some(42)), + expected_span: (Some(41), Some(49)), + }, + LocatedTruthTestDiagnosticTestCase { + description: "SQL Server suggests a BIT comparison", + dialect: DialectType::TSQL, + sql: "SELECT 1 FROM orders WHERE is_test_order IS TRUE", + expected_message: "'IS TRUE' is not supported for dialect TSQL; \ + write COALESCE(, 0) = 1", + expected_location: (Some(1), Some(42)), + expected_span: (Some(41), Some(48)), + }, + ]; + for test_case in &test_cases { + let result = map_diagnostics( + test_case.sql, + test_case.dialect, + ValidationResult { + valid: true, + errors: Vec::new(), + }, + &FunctionProbes::default(), + )?; + let [error] = result.errors.as_slice() else { + return Err(format!("{}: {:?}", test_case.description, result.errors)); + }; + + assert!(!result.valid, "{}", test_case.description); + assert_eq!(error.code, "B103", "{}", test_case.description); + assert_eq!( + error.message, test_case.expected_message, + "{}", + test_case.description + ); + assert_eq!( + (error.line, error.column), + test_case.expected_location, + "{}", + test_case.description + ); + assert_eq!( + (error.start, error.end), + test_case.expected_span, + "{}", + test_case.description + ); + } + Ok(()) +} diff --git a/crates/sqlbuild-analysis/src/semantic_validation/tests/test_types.rs b/crates/sqlbuild-analysis/src/semantic_validation/tests/test_types.rs index 521de7be8e..9f58f8b22e 100644 --- a/crates/sqlbuild-analysis/src/semantic_validation/tests/test_types.rs +++ b/crates/sqlbuild-analysis/src/semantic_validation/tests/test_types.rs @@ -52,3 +52,24 @@ pub(crate) struct NormalizationBatchTestCase { pub(crate) requests: &'static [(&'static str, &'static [(&'static str, &'static str)])], pub(crate) expected_failures: &'static [usize], } + +pub(crate) struct TruthTestScanTestCase { + pub(crate) description: &'static str, + pub(crate) sql: &'static str, + pub(crate) expected_tests: &'static [(usize, usize, bool, bool)], +} + +pub(crate) struct TruthTestDialectTestCase { + pub(crate) description: &'static str, + pub(crate) dialect: polyglot_sql::DialectType, + pub(crate) expected_rejected: bool, +} + +pub(crate) struct LocatedTruthTestDiagnosticTestCase { + pub(crate) description: &'static str, + pub(crate) dialect: polyglot_sql::DialectType, + pub(crate) sql: &'static str, + pub(crate) expected_message: &'static str, + pub(crate) expected_location: (Option, Option), + pub(crate) expected_span: (Option, Option), +} diff --git a/scripts/compiler_differential/_helpers/analysis_corpus/failure_cases.py b/scripts/compiler_differential/_helpers/analysis_corpus/failure_cases.py index e95a90f5fd..63172895f0 100644 --- a/scripts/compiler_differential/_helpers/analysis_corpus/failure_cases.py +++ b/scripts/compiler_differential/_helpers/analysis_corpus/failure_cases.py @@ -144,6 +144,19 @@ def _binding_cases() -> tuple[FailureCase, ...]: ), }, ), + failure_case( + name="analysis-unsupported-truth-test", + expected_code="B103", + expected_message="'IS NOT TRUE' is not supported for dialect Snowflake", + files={ + FAILURE_CONFIG_PATH: _SNOWFLAKE_CONFIG + + '\n[defaults]\ndatabase = "ANALYTICS"\nschema = "ANALYTICS"\n', + **_mart( + f"SELECT customer_id, amount AS total_amount\nFROM {_ORDERS}\n" + "WHERE (amount > 1) IS NOT TRUE\n" + ), + }, + ), ) diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/semantic-compilation.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/semantic-compilation.md index 75ecd8dff0..9da9c2427e 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/semantic-compilation.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/semantic-compilation.md @@ -91,6 +91,14 @@ UDFs called through `__udf`, schema-qualified and method calls, quoted names, an functions are not affected. There is no per-model suppression: use the suggested spelling, or schema-qualify a call to an external UDF that shares an alias name. Other dialects are not checked. +Truth-value tests (`IS [NOT] TRUE` and `IS [NOT] FALSE`) are rejected for Snowflake and SQL +Server, whose warehouses do not accept them although the SQL parser does. SQLBuild reports a +blocking B103 error at the `IS` keyword with a null-safe rewrite, for example +`'IS NOT TRUE' is not supported for dialect Snowflake; write NOT COALESCE(, FALSE) or +IS DISTINCT FROM TRUE`. SQL Server suggestions compare a BIT value, such as +`COALESCE(, 0) = 1`. DuckDB (including MotherDuck), PostgreSQL, BigQuery and Databricks +accept truth-value tests. + Derived-table column aliases, UNPIVOT, and Snowflake GROUPING SETS are checked. DuckDB and Snowflake reject excess relation aliases; fewer aliases than output columns remain valid. Missing UNPIVOT input columns are errors. @@ -198,6 +206,7 @@ the notice never reduces them to a count alone. | B004–B005 | E222–E223 | Reference/scope errors | | B101 | E202 or SQLBuild | Unknown function, or a built-in spelling the dialect does not accept | | B102 | E203 or project declaration | Invalid function arity | +| B103 | SQLBuild | Truth-value test the dialect does not accept | | B210–B219 | E210–E219 | Proven native type/shape errors on enabled dialects | | B211–B218 | W210–W216 | Implicit conversions; blocking, fixed by an explicit `CAST` or comparison | | B216 | E216 | Set-operation, subquery, or row arity | diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py index dbbb6d7464..429b1573c5 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py @@ -420,6 +420,23 @@ class FunctionNameCompileTestCase: expected_diagnostics: tuple[tuple[str, str, int, int], ...] +@dataclass(frozen=True) +class TruthValueTestBuildTestCase: + description: str + repo_files: dict[str, str] + expected_exit_code: int + expected_stdout_fragment: str + + +@dataclass(frozen=True) +class TruthValueTestCompileTestCase: + description: str + repo_files: dict[str, str] + expected_exit_code: int + expected_diagnostics: tuple[tuple[str, str, str, int, int], ...] + expected_help: str | None = None + + @dataclass(frozen=True) class ModelHeaderKeyCompileCase: description: str diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_truth_value_tests.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_truth_value_tests.py new file mode 100644 index 0000000000..eff30c8b79 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_truth_value_tests.py @@ -0,0 +1,188 @@ +"""E2E tests for compile-time rejection of truth-value tests the target warehouse lacks.""" + +from __future__ import annotations + +import json +import subprocess +from pathlib import Path +from typing import Any + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.compile._test_types import ( + TruthValueTestBuildTestCase, + TruthValueTestCompileTestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project, run_sqb + +_SNOWFLAKE_PROJECT: str = 'name = "orders"\nadapter = "snowflake"\n' +_DUCKDB_PROJECT: str = ( + 'name = "orders"\nadapter = "duckdb"\n\n[connection]\ndatabase = "warehouse.duckdb"\n' +) +_SOURCES: str = ( + "sources:\n" + " - name: raw__orders\n" + " description: Raw order records\n" + " database: example_db\n" + " schema: raw\n" + " table: orders\n" + " contract: enforced\n" + " columns:\n" + " - name: order_id\n type: VARCHAR\n" + " - name: is_test_order\n type: BOOLEAN\n" +) +_DUCKDB_SOURCES: str = _SOURCES.replace( + " database: example_db\n schema: raw\n table: orders\n", + " expression: SELECT 'o1' AS order_id, NULL::BOOLEAN AS is_test_order\n", +) +_MODEL_PATH: str = "models/real_orders.sql" +_AUDIT_PATH: str = "audits/generic/test_orders_are_flagged.sql" +_TEST_PATH: str = "tests/unit/test_real_orders.sql" +_MODEL: str = ( + "MODEL (\n" + ' description "Orders that are not test orders",\n' + " materialized table,\n" + " database example_db,\n" + " schema analytics,\n" + " audits [test_orders_are_flagged],\n" + ");\n\n" + "WITH raw__orders AS (\n" + " SELECT *\n" + ' FROM __source("raw__orders")\n' + ")\n\n" + "SELECT\n" + " order_id,\n" + " is_test_order\n" + "FROM raw__orders\n" + "WHERE is_test_order IS NOT TRUE\n" +) +_AUDIT: str = "AUDIT ();\n\nSELECT order_id\nFROM @relation\nWHERE is_test_order IS TRUE\n" +_DUCKDB_MODEL: str = _MODEL.replace(" database example_db,\n", "") +_SQL_TEST: str = ( + "TEST ();\n\n" + "WITH __source__raw__orders AS (\n" + " SELECT 'o1' AS order_id, CAST(NULL AS BOOLEAN) AS is_test_order\n" + "),\n\n" + "__expected__real_orders AS (\n" + " SELECT 'o1' AS order_id, CAST(NULL AS BOOLEAN) AS is_test_order\n" + " WHERE (1 = 1) IS NOT FALSE\n" + ")\n\n" + "SELECT 1\n" +) +_SNOWFLAKE_HELP: str = ( + "rewrite the truth-value test null-safely, for example COALESCE(x, FALSE) for x IS TRUE, " + "and NOT COALESCE(x, FALSE) or x IS DISTINCT FROM TRUE for x IS NOT TRUE" +) + + +@pytest.mark.parametrize( + "test_case", + ( + TruthValueTestCompileTestCase( + description="snowflake rejects truth-value tests in models, audits and SQL tests", + repo_files={ + "sqlbuild_project.toml": _SNOWFLAKE_PROJECT, + "sources/raw.yml": _SOURCES, + _MODEL_PATH: _MODEL, + _AUDIT_PATH: _AUDIT, + _TEST_PATH: _SQL_TEST, + }, + expected_exit_code=1, + expected_diagnostics=( + ( + "B103", + "'IS NOT TRUE' is not supported for dialect Snowflake; " + "write NOT COALESCE(, FALSE) or IS DISTINCT FROM TRUE", + _MODEL_PATH, + 18, + 21, + ), + ( + "B103", + "audit 'test_orders_are_flagged' on model 'real_orders': 'IS TRUE' is not " + "supported for dialect Snowflake; write COALESCE(, FALSE)", + _AUDIT_PATH, + 5, + 21, + ), + ( + "B103", + "SQL test 'test_real_orders': 'IS NOT FALSE' is not supported for dialect " + "Snowflake; write COALESCE(, TRUE) or IS DISTINCT FROM FALSE", + _TEST_PATH, + 9, + 17, + ), + ), + expected_help=_SNOWFLAKE_HELP, + ), + TruthValueTestCompileTestCase( + description="duckdb accepts truth-value tests", + repo_files={ + "sqlbuild_project.toml": _DUCKDB_PROJECT, + "sources/raw.yml": _DUCKDB_SOURCES, + _MODEL_PATH: _DUCKDB_MODEL, + _AUDIT_PATH: _AUDIT, + _TEST_PATH: _SQL_TEST, + }, + expected_exit_code=0, + expected_diagnostics=(), + ), + ), + ids=lambda case: case.description, +) +def test_given_truth_value_test_when_compiling_then_dialect_support_is_enforced( + test_case: TruthValueTestCompileTestCase, tmp_path: Path +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, project_name="orders", repo_files=test_case.repo_files + ) + + result: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("--no-color", "compile", "--json", "--no-cache") + ) + + payload: dict[str, Any] = json.loads(result.stdout) + diagnostics: list[dict[str, Any]] = payload["diagnostics"] + assert result.returncode == test_case.expected_exit_code, result.stdout + result.stderr + assert ( + tuple( + (item["code"], item["message"], item["path"], item["line"], item["column"]) + for item in diagnostics + ) + == test_case.expected_diagnostics + ) + assert all(item["help"] == test_case.expected_help for item in diagnostics) + + +@pytest.mark.parametrize( + "test_case", + ( + TruthValueTestBuildTestCase( + description="duckdb builds, audits and tests truth-value tests", + repo_files={ + "sqlbuild_project.toml": _DUCKDB_PROJECT, + "sources/raw.yml": _DUCKDB_SOURCES, + _MODEL_PATH: _DUCKDB_MODEL, + _AUDIT_PATH: _AUDIT, + _TEST_PATH: _SQL_TEST, + }, + expected_exit_code=0, + expected_stdout_fragment="FAIL=0", + ), + ), + ids=lambda case: case.description, +) +def test_given_duckdb_truth_value_tests_when_building_then_warehouse_accepts_them( + test_case: TruthValueTestBuildTestCase, tmp_path: Path +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, project_name="orders", repo_files=test_case.repo_files + ) + + result: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("--no-color", "build") + ) + + assert result.returncode == test_case.expected_exit_code, result.stdout + result.stderr + assert test_case.expected_stdout_fragment in result.stdout, result.stdout diff --git a/tests/goldens/compiler/failure/analysis-unsupported-truth-test.json b/tests/goldens/compiler/failure/analysis-unsupported-truth-test.json new file mode 100644 index 0000000000..f9ac66d232 --- /dev/null +++ b/tests/goldens/compiler/failure/analysis-unsupported-truth-test.json @@ -0,0 +1,274 @@ +{ + "commands": [ + { + "label": "compile", + "exit_code": 1, + "diagnostics": [ + { + "phase": "compile", + "severity": "error", + "code": "B103", + "message": "'IS NOT TRUE' is not supported for dialect Snowflake; write NOT COALESCE(, FALSE) or IS DISTINCT FROM TRUE", + "resource_type": "model", + "resource_name": "customer_totals", + "path": "models/marts/customer_totals.sql", + "line": 7, + "column": 20, + "location": { + "path": "models/marts/customer_totals.sql", + "line": 7, + "column": 20, + "end_line": 7, + "end_column": 31 + }, + "help": "rewrite the truth-value test null-safely, for example COALESCE(x, FALSE) for x IS TRUE, and NOT COALESCE(x, FALSE) or x IS DISTINCT FROM TRUE for x IS NOT TRUE" + } + ] + } + ], + "compiled": { + "models/marts/customer_totals.sql": [ + "SELECT customer_id, amount AS total_amount", + "FROM __ref(\"stg_orders\")", + "WHERE (amount > 1) IS NOT TRUE", + "" + ], + "models/staging/stg_orders.sql": [ + "SELECT order_id, customer_id, amount, status", + "FROM __source(\"raw_orders\")", + "" + ] + }, + "manifest": { + "nodes": { + "model.failure_corpus.customer_totals": { + "database": "ANALYTICS", + "schema": "ANALYTICS", + "name": "customer_totals", + "resource_type": "model", + "package_name": "failure_corpus", + "path": "models/marts/customer_totals.sql", + "original_file_path": "models/marts/customer_totals.sql", + "unique_id": "model.failure_corpus.customer_totals", + "fqn": [ + "failure_corpus", + "models", + "marts", + "customer_totals" + ], + "alias": "customer_totals", + "config": { + "enabled": true, + "alias": null, + "schema": "ANALYTICS", + "database": "ANALYTICS", + "tags": [], + "meta": {}, + "group": null, + "materialized": "view", + "incremental_strategy": null, + "merge_exclude_columns": null, + "persist_docs": {}, + "post-hook": [], + "pre-hook": [], + "quoting": {}, + "column_types": {}, + "full_refresh": null, + "unique_key": null, + "on_schema_change": "ignore", + "on_configuration_change": "apply", + "grants": {}, + "packages": [], + "docs": { + "show": true, + "node_color": null + }, + "contract": { + "enforced": false, + "alias_types": true + } + }, + "tags": [], + "description": "Order totals per customer", + "columns": {}, + "meta": {}, + "group": null, + "docs": { + "show": true, + "node_color": null + }, + "patch_path": null, + "build_path": null, + "unrendered_config": {}, + "config_call_dict": {}, + "unrendered_config_call_dict": {}, + "relation_name": "ANALYTICS.ANALYTICS.customer_totals", + "depends_on": { + "macros": [], + "nodes": [ + "model.failure_corpus.stg_orders" + ] + } + }, + "model.failure_corpus.stg_orders": { + "database": "ANALYTICS", + "schema": "ANALYTICS", + "name": "stg_orders", + "resource_type": "model", + "package_name": "failure_corpus", + "path": "models/staging/stg_orders.sql", + "original_file_path": "models/staging/stg_orders.sql", + "unique_id": "model.failure_corpus.stg_orders", + "fqn": [ + "failure_corpus", + "models", + "staging", + "stg_orders" + ], + "alias": "stg_orders", + "config": { + "enabled": true, + "alias": null, + "schema": "ANALYTICS", + "database": "ANALYTICS", + "tags": [], + "meta": {}, + "group": null, + "materialized": "view", + "incremental_strategy": null, + "merge_exclude_columns": null, + "persist_docs": {}, + "post-hook": [], + "pre-hook": [], + "quoting": {}, + "column_types": {}, + "full_refresh": null, + "unique_key": null, + "on_schema_change": "ignore", + "on_configuration_change": "apply", + "grants": {}, + "packages": [], + "docs": { + "show": true, + "node_color": null + }, + "contract": { + "enforced": false, + "alias_types": true + } + }, + "tags": [], + "description": "Staged orders", + "columns": {}, + "meta": {}, + "group": null, + "docs": { + "show": true, + "node_color": null + }, + "patch_path": null, + "build_path": null, + "unrendered_config": {}, + "config_call_dict": {}, + "unrendered_config_call_dict": {}, + "relation_name": "ANALYTICS.ANALYTICS.stg_orders", + "depends_on": { + "macros": [], + "nodes": [ + "source.failure_corpus.raw_orders" + ] + } + } + }, + "sources": { + "source.failure_corpus.raw_orders": { + "database": null, + "schema": "", + "name": "raw_orders", + "resource_type": "source", + "package_name": "failure_corpus", + "path": "sources/raw.yml", + "original_file_path": "sources/raw.yml", + "unique_id": "source.failure_corpus.raw_orders", + "fqn": [ + "failure_corpus", + "raw_orders" + ], + "source_name": "raw_orders", + "source_description": "", + "loader": "", + "identifier": "raw_orders", + "quoting": {}, + "loaded_at_field": null, + "freshness": null, + "external": null, + "description": "Orders feed.", + "columns": { + "order_id": { + "name": "order_id", + "description": "", + "meta": {}, + "data_type": "INTEGER", + "constraints": [], + "quote": null, + "tags": [] + }, + "customer_id": { + "name": "customer_id", + "description": "", + "meta": {}, + "data_type": "INTEGER", + "constraints": [], + "quote": null, + "tags": [] + }, + "amount": { + "name": "amount", + "description": "", + "meta": {}, + "data_type": "DOUBLE", + "constraints": [], + "quote": null, + "tags": [] + }, + "status": { + "name": "status", + "description": "", + "meta": {}, + "data_type": "VARCHAR", + "constraints": [], + "quote": null, + "tags": [] + } + }, + "meta": {}, + "source_meta": {}, + "tags": [], + "config": {}, + "patch_path": null, + "unrendered_config": {}, + "relation_name": null + } + }, + "macros": {}, + "exposures": {}, + "metrics": {}, + "groups": {}, + "selectors": {}, + "disabled": {}, + "parent_map": { + "model.failure_corpus.customer_totals": [ + "model.failure_corpus.stg_orders" + ], + "model.failure_corpus.stg_orders": [ + "source.failure_corpus.raw_orders" + ], + "source.failure_corpus.raw_orders": [] + }, + "group_map": {}, + "docs": {}, + "saved_queries": {}, + "semantic_models": {}, + "unit_tests": {} + } +} diff --git a/tests/unit/scripts/compiler_differential/_helpers/corpus/test_analysis_codes.py b/tests/unit/scripts/compiler_differential/_helpers/corpus/test_analysis_codes.py index f774280585..8d062f104b 100644 --- a/tests/unit/scripts/compiler_differential/_helpers/corpus/test_analysis_codes.py +++ b/tests/unit/scripts/compiler_differential/_helpers/corpus/test_analysis_codes.py @@ -30,6 +30,7 @@ *(f"B00{number}" for number in (0, 2, 3, 4, 5)), "B101", "B102", + "B103", *(f"B2{number}" for number in range(10, 20)), *(f"B23{number}" for number in range(5)), "B300", diff --git a/website/src/content/docs/docs/concepts/semantic-compilation.mdx b/website/src/content/docs/docs/concepts/semantic-compilation.mdx index 8ead338596..0b6eefbe1b 100644 --- a/website/src/content/docs/docs/concepts/semantic-compilation.mdx +++ b/website/src/content/docs/docs/concepts/semantic-compilation.mdx @@ -81,6 +81,14 @@ UDFs called through `__udf`, schema-qualified and method calls, quoted names, an functions are not affected. There is no per-model suppression: use the suggested spelling, or schema-qualify a call to an external UDF that shares an alias name. Other dialects are not checked. +Truth-value tests (`IS [NOT] TRUE` and `IS [NOT] FALSE`) are rejected for Snowflake and SQL +Server, whose warehouses do not accept them although the SQL parser does. SQLBuild reports a +blocking B103 error at the `IS` keyword with a null-safe rewrite, for example +`'IS NOT TRUE' is not supported for dialect Snowflake; write NOT COALESCE(, FALSE) or +IS DISTINCT FROM TRUE`. SQL Server suggestions compare a BIT value, such as +`COALESCE(, 0) = 1`. DuckDB (including MotherDuck), PostgreSQL, BigQuery and Databricks +accept truth-value tests. + Derived-table column aliases, UNPIVOT, and Snowflake GROUPING SETS are checked. DuckDB and Snowflake reject excess relation aliases; fewer aliases than output columns remain valid. Missing UNPIVOT input columns are errors. @@ -188,6 +196,7 @@ the notice never reduces them to a count alone. | B004–B005 | E222–E223 | Reference/scope errors | | B101 | E202 or SQLBuild | Unknown function, or a built-in spelling the dialect does not accept | | B102 | E203 or project declaration | Invalid function arity | +| B103 | SQLBuild | Truth-value test the dialect does not accept | | B210–B219 | E210–E219 | Proven native type/shape errors on enabled dialects | | B211–B218 | W210–W216 | Implicit conversions; blocking, fixed by an explicit `CAST` or comparison | | B216 | E216 | Set-operation, subquery, or row arity | From f3a4019373a8a2d71ac35526d2105fdf270da354 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 19:32:08 +0100 Subject: [PATCH 03/12] fix: create the SQL Server source freshness table with an existence check SQL Server has no CREATE TABLE IF NOT EXISTS. Guard the freshness table like the other state tables and narrow its indexed key columns from NVARCHAR(MAX), which SQL Server cannot index, to NVARCHAR(450). Fixes CHI-518. --- .../sqlserver/classes/sqlserver_adapter.py | 21 ++++++++- .../adapters/sqlserver/test_client.py | 43 +++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/src/sqlbuild/adapters/sqlserver/classes/sqlserver_adapter.py b/src/sqlbuild/adapters/sqlserver/classes/sqlserver_adapter.py index 87482677b8..ba50791eba 100644 --- a/src/sqlbuild/adapters/sqlserver/classes/sqlserver_adapter.py +++ b/src/sqlbuild/adapters/sqlserver/classes/sqlserver_adapter.py @@ -450,13 +450,30 @@ def render_prune_fingerprint_history_sql( ) def render_create_source_freshness_table_sql(self, *, database: str | None, schema: str) -> str: - from sqlbuild._native import build_source_freshness_create_table_sql + from sqlbuild._native import ( + SOURCE_FRESHNESS_TABLE_NAME, + build_source_freshness_create_table_sql, + ) - return build_source_freshness_create_table_sql( + create_sql: str = build_source_freshness_create_table_sql( database=database, schema=schema, render_qualified_name=self.render_qualified_name, render_framework_type=self.render_framework_type, + ).replace("CREATE TABLE IF NOT EXISTS ", "CREATE TABLE ", 1) + for column in ( + "source_name", + "target_database", + "target_schema", + "target_name", + "run_id", + ): + create_sql = create_sql.replace(f"{column} NVARCHAR(MAX)", f"{column} NVARCHAR(450)") + return self._create_table_if_missing_sql( + create_sql=create_sql, + database=database, + schema=schema, + table_name=SOURCE_FRESHNESS_TABLE_NAME, ) def render_create_source_freshness_index_sqls( diff --git a/tests/unit/src/sqlbuild/adapters/sqlserver/test_client.py b/tests/unit/src/sqlbuild/adapters/sqlserver/test_client.py index 51f5cd77bf..a192084a60 100644 --- a/tests/unit/src/sqlbuild/adapters/sqlserver/test_client.py +++ b/tests/unit/src/sqlbuild/adapters/sqlserver/test_client.py @@ -567,6 +567,49 @@ def test_given_node_result_table_when_rendering_create_then_sqlserver_guards_mis assert expected_fragment in statement +@pytest.mark.parametrize( + "test_case", + [ + SqlServerLatestReadSqlTestCase( + description="renders guarded source freshness table with indexable keys", + database="shop", + schema="analytics", + expected_fragments=( + "IF NOT EXISTS (SELECT 1 FROM information_schema.tables ", + "WHERE table_schema = 'analytics' AND table_name = '_sqlbuild_source_freshness' " + "AND table_catalog = 'shop')", + "CREATE TABLE shop.analytics._sqlbuild_source_freshness (", + "source_name NVARCHAR(450)", + "target_database NVARCHAR(450)", + "target_schema NVARCHAR(450)", + "target_name NVARCHAR(450)", + "run_id NVARCHAR(450)", + ), + ) + ], + ids=lambda case: case.description, +) +def test_given_source_freshness_table_when_rendering_create_then_sqlserver_guards_missing_table( + test_case: SqlServerLatestReadSqlTestCase, +) -> None: + adapter: SqlServerAdapter = SqlServerAdapter() + + statement: str = adapter.render_create_source_freshness_table_sql( + database=test_case.database, + schema=test_case.schema, + ) + + for expected_fragment in test_case.expected_fragments: + assert expected_fragment in statement, statement + assert "CREATE TABLE IF NOT EXISTS" not in statement + index_sql: str = adapter.render_create_source_freshness_index_sqls( + database=test_case.database, schema=test_case.schema + )[0] + for key_column in ("source_name", "target_database", "target_schema", "target_name"): + assert key_column in index_sql + assert f"{key_column} NVARCHAR(MAX)" not in statement + + @pytest.mark.parametrize( "test_case", [ From 79c3e6826bb84bd7f748bc37c8b531a00d7ae757 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 19:32:08 +0100 Subject: [PATCH 04/12] fix: casefold names in explicit-reference relation matching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Relation and temporary-table names were lowercased rather than casefolded, so names such as Straße and STRASSE did not match as they do in Python and the runtime relation guard. Fixes CHI-595. --- .../src/explicit_references/_helpers/python_sql.rs | 4 +++- .../src/explicit_references/_helpers/relations.rs | 4 +++- .../src/explicit_references/tests/helpers.rs | 7 +++++++ .../explicit_references/tests/test_explicit_references.rs | 5 +++++ .../sqlbuild-analysis/src/relation_names/_helpers/walk.rs | 3 ++- .../compiler/references/main/test_relation_names.py | 7 +++++++ 6 files changed, 27 insertions(+), 3 deletions(-) diff --git a/crates/sqlbuild-analysis/src/explicit_references/_helpers/python_sql.rs b/crates/sqlbuild-analysis/src/explicit_references/_helpers/python_sql.rs index 97c479d155..efb87d6702 100644 --- a/crates/sqlbuild-analysis/src/explicit_references/_helpers/python_sql.rs +++ b/crates/sqlbuild-analysis/src/explicit_references/_helpers/python_sql.rs @@ -2,6 +2,8 @@ use std::collections::HashSet; +use sqlbuild_core::text::main::python_casefold::python_casefold; + use crate::explicit_references::_helpers::relations::{ ProjectRelation, matched, project_relations, }; @@ -123,7 +125,7 @@ impl OwnerScan<'_> { if joined(relation, "").contains(&self.request.placeholder) { return None; } - if relation.schema.is_none() && temporary.contains(&str::to_lowercase(&relation.name)) { + if relation.schema.is_none() && temporary.contains(&python_casefold(&relation.name)) { return None; } let Some(found) = matched(self.index, relation) else { diff --git a/crates/sqlbuild-analysis/src/explicit_references/_helpers/relations.rs b/crates/sqlbuild-analysis/src/explicit_references/_helpers/relations.rs index c4e22f8368..a30015b8a7 100644 --- a/crates/sqlbuild-analysis/src/explicit_references/_helpers/relations.rs +++ b/crates/sqlbuild-analysis/src/explicit_references/_helpers/relations.rs @@ -2,6 +2,8 @@ use std::collections::HashSet; +use sqlbuild_core::text::main::python_casefold::python_casefold; + use crate::explicit_references::constants::{ MIGRATE_FROM_QUOTES, MODEL_TYPE, SCHEMA_QUALIFIED_PARTS, SEED_TYPE, SOURCE_TYPE, }; @@ -59,7 +61,7 @@ pub(crate) fn matched<'a>( } fn folded(text: &str) -> String { - str::to_lowercase(text) + python_casefold(text) } /// `_parts_compatible`: either part missing, or equal casefolded. diff --git a/crates/sqlbuild-analysis/src/explicit_references/tests/helpers.rs b/crates/sqlbuild-analysis/src/explicit_references/tests/helpers.rs index 132b5bad09..7ee8a76c62 100644 --- a/crates/sqlbuild-analysis/src/explicit_references/tests/helpers.rs +++ b/crates/sqlbuild-analysis/src/explicit_references/tests/helpers.rs @@ -41,6 +41,13 @@ pub(crate) fn request() -> ReferenceRequest { database: None, expression: false, }, + SourceRelation { + name: "street_addresses".to_owned(), + table: Some("straße".to_owned()), + schema: Some("landing".to_owned()), + database: None, + expression: false, + }, SourceRelation { name: "computed".to_owned(), table: None, diff --git a/crates/sqlbuild-analysis/src/explicit_references/tests/test_explicit_references.rs b/crates/sqlbuild-analysis/src/explicit_references/tests/test_explicit_references.rs index 9fe8f52a87..acbddce0e6 100644 --- a/crates/sqlbuild-analysis/src/explicit_references/tests/test_explicit_references.rs +++ b/crates/sqlbuild-analysis/src/explicit_references/tests/test_explicit_references.rs @@ -20,6 +20,11 @@ fn given_written_relations_when_matching_project_relations_then_matches_python() written: ("raw_orders", Some("LANDING"), None), expected_match: Some(("source", "raw_orders", None)), }, + MatchTestCase { + description: "names match by casefold where lowercase differs", + written: ("STRASSE", Some("LANDING"), None), + expected_match: Some(("source", "street_addresses", None)), + }, MatchTestCase { description: "an expression source is no relation", written: ("computed", None, None), diff --git a/crates/sqlbuild-analysis/src/relation_names/_helpers/walk.rs b/crates/sqlbuild-analysis/src/relation_names/_helpers/walk.rs index 1d484407e8..28fe14df79 100644 --- a/crates/sqlbuild-analysis/src/relation_names/_helpers/walk.rs +++ b/crates/sqlbuild-analysis/src/relation_names/_helpers/walk.rs @@ -2,6 +2,7 @@ use polyglot_sql::{ComplexityGuardOptions, Dialect, Expression, ParseOptions}; use serde_json::{Map, Value, json}; +use sqlbuild_core::text::main::python_casefold::python_casefold; use crate::relation_names::constants::{ CATALOG_KEY, CREATE_TABLE_KEY, CTE_ALIAS_KEY, CTE_LIST_KEY, GENERIC_DIALECT, @@ -130,5 +131,5 @@ fn temporary_name(node: &Map) -> Option { return None; } let table: &Map = create.get(NAME_KEY)?.as_object()?; - identifier(table.get(NAME_KEY)).map(str::to_lowercase) + identifier(table.get(NAME_KEY)).map(python_casefold) } diff --git a/tests/unit/src/sqlbuild/compiler/references/main/test_relation_names.py b/tests/unit/src/sqlbuild/compiler/references/main/test_relation_names.py index dbb8f8b444..f49d2dba4e 100644 --- a/tests/unit/src/sqlbuild/compiler/references/main/test_relation_names.py +++ b/tests/unit/src/sqlbuild/compiler/references/main/test_relation_names.py @@ -52,6 +52,13 @@ ), expected_temporary=frozenset({"staged"}), ), + ExtractRelationNamesTestCase( + description="temporary table names are casefolded, not lowercased", + sql='CREATE TEMP TABLE "Straße" AS SELECT 1 AS id', + dialect="duckdb", + expected_relations=(RelationName(name="Straße"),), + expected_temporary=frozenset({"strasse"}), + ), ExtractRelationNamesTestCase( description="quoted dotted path splits into qualifier parts", sql="SELECT * FROM `warehouse.analytics.customers`", From a68952c1b51ca52298225381d5d5032d63471bdc Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 19:32:08 +0100 Subject: [PATCH 05/12] fix: report built-in audits on missing columns once at the model header A built-in audit attached to a column the model does not output was bound as resource SQL and reported once per column reference at a made-up audits/generic/.sql path. Report one B002 per audit at its declaration in MODEL() instead. Project audits that override a built-in keep their own file. Fixes CHI-476. --- .../_helpers/metadata_checks/positions.rs | 7 ++ .../_helpers/resource_sql/diagnostics.rs | 48 ++++++++- .../_helpers/resource_sql/resources.rs | 68 ++++++++++++- .../src/semantic_checks/tests/helpers.rs | 37 ++++++- .../tests/test_resource_sql.rs | 99 ++++++++++++++++++- .../src/semantic_checks/tests/test_types.rs | 8 ++ .../compile/test_resource_sql_validation.py | 41 ++++++-- 7 files changed, 287 insertions(+), 21 deletions(-) diff --git a/crates/sqlbuild-analysis/src/semantic_checks/_helpers/metadata_checks/positions.rs b/crates/sqlbuild-analysis/src/semantic_checks/_helpers/metadata_checks/positions.rs index 3043c52fe0..fe8e1367d3 100644 --- a/crates/sqlbuild-analysis/src/semantic_checks/_helpers/metadata_checks/positions.rs +++ b/crates/sqlbuild-analysis/src/semantic_checks/_helpers/metadata_checks/positions.rs @@ -50,6 +50,13 @@ fn match_start( Ok(offset) } +/// The code-point start of the first whole-word `name` at or after `offset`, else `offset`. +pub(crate) fn word_start(text: &str, name: &str, offset: usize) -> Result { + let characters: Vec = text.chars().collect(); + let name: Vec = name.chars().collect(); + match_start(&characters, &name, offset) +} + /// Python's `_text_position`: the 1-based line and column of `name` at or after `offset`. pub(crate) fn text_position( text: &str, diff --git a/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/diagnostics.rs b/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/diagnostics.rs index b4b251b058..5df90d2dcc 100644 --- a/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/diagnostics.rs +++ b/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/diagnostics.rs @@ -7,11 +7,14 @@ use sqlbuild_attachments::cursor_intrinsics::main::replace_intrinsics::replace_i use sqlbuild_core::panics::main::catch_compiler_panic::catch_compiler_panic; use crate::semantic_checks::_helpers::explanation::opt_outs::unneeded_opt_out; +use crate::semantic_checks::_helpers::metadata_checks::positions::{text_position, word_start}; use crate::semantic_checks::_helpers::resource_sql::help::{HelpContext, resource_sql_help}; use crate::semantic_checks::_helpers::resource_sql::locations::{ Finding, line_at, location, text_from, }; -use crate::semantic_checks::_helpers::resource_sql::resources::{ResourceSql, resources}; +use crate::semantic_checks::_helpers::resource_sql::resources::{ + MissingColumnAudit, ResourceSql, audit_label, missing_column_audits, resources, +}; use crate::semantic_checks::models::{ ResourceSqlDiagnostic, ResourceSqlFailure, ResourceSqlKind, ResourceSqlRequest, SemanticFailure, }; @@ -20,6 +23,7 @@ use crate::semantic_validation::models::{Columns, NormalizationInput, ProjectCat use crate::semantic_validation::types::{BindingRequest, Relations}; const UNKNOWN_TABLE_CODE: &str = "B000"; +const UNKNOWN_COLUMN_CODE: &str = "B002"; const UNKNOWN_TABLE_PREFIX: &str = "Unknown table "; const SYNTAX_ERROR_CODE: &str = "P001"; const OPT_OUT_CODE: &str = "P009"; @@ -201,15 +205,54 @@ fn opt_out_diagnostic( }) } +/// One B002 for a built-in audit on a missing model column, at its MODEL header declaration. +pub(crate) fn missing_column_diagnostic( + missing: &MissingColumnAudit<'_>, +) -> Result { + let MissingColumnAudit { + audit, + model, + column, + } = missing; + let definition: &str = &audit.definition_name; + let column_start: usize = word_start(&model.authored_sql, column, 0).map_err(semantic)?; + let (line, start) = + text_position(&model.authored_sql, definition, column_start).map_err(semantic)?; + let end: i64 = start + i64::try_from(definition.chars().count()).unwrap_or(0); + Ok(ResourceSqlDiagnostic { + kind: ResourceSqlKind::Audit, + resource_name: audit.name.clone(), + is_error: true, + code: UNKNOWN_COLUMN_CODE.to_owned(), + message: format!( + "{}: Unknown column '{column}' in table '{}'", + audit_label(audit), + model.name + ), + path: model.path.clone(), + span: (line, start, Some(line), Some(end)), + note: None, + help: Some(format!( + "add {column} to the SELECT list of model '{}' or attach the {definition} audit to \ + a column the model outputs", + model.name + )), + }) +} + /// Every audit, SQL test and SQL hook finding, in Python's first-seen order. pub(crate) fn resource_sql_findings( request: &ResourceSqlRequest, catalog: &ProjectCatalog, ) -> Result, ResourceSqlFailure> { + let mut diagnostics: Vec = missing_column_audits(request) + .iter() + .map(missing_column_diagnostic) + .collect::>()?; let pool = || catalog.analysis_pool(); let checked = resources(request, &pool).map_err(ResourceSqlFailure::Internal)?; if checked.is_empty() { - return Ok(Vec::new()); + return Ok(diagnostics); } let dialect = request.dialect.as_deref(); let mut seen: HashSet<&str> = HashSet::with_capacity(checked.len()); @@ -253,7 +296,6 @@ pub(crate) fn resource_sql_findings( .map_err(ResourceSqlFailure::Internal)?; let mut keys: HashSet = HashSet::new(); let mut opt_out_keys: HashSet<(String, i64, i64)> = HashSet::new(); - let mut diagnostics: Vec = Vec::new(); for resource in &checked { let cleaned = normalized[resource.sql.as_str()].as_str(); let findings = &results[request_index[&(cleaned, &resource.relations)]]; diff --git a/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/resources.rs b/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/resources.rs index 65c0767aec..4a590aee33 100644 --- a/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/resources.rs +++ b/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/resources.rs @@ -16,6 +16,7 @@ use crate::semantic_checks::models::{ }; const COLUMN_ONLY_BUILT_IN_AUDITS: [&str; 2] = ["not_null", "unique"]; +const MODEL_TARGET_KIND: &str = "model"; const SQL_ANALYSIS_KEY: &str = "sql_analysis"; /// One authored SQL body owned by a non-model resource, Python's `_ResourceSql`. @@ -60,7 +61,7 @@ fn text_byte_end(contents: &str, body: &str) -> usize { contents.find(body).unwrap_or(contents.len()) } -fn audit_label(audit: &AuditSqlInput) -> String { +pub(crate) fn audit_label(audit: &AuditSqlInput) -> String { let mut label = format!("audit '{}'", audit.definition_name); if let Some((kind, target)) = &audit.attached_target { label.push_str(&format!(" on {kind} '{target}'")); @@ -104,6 +105,66 @@ fn binds_cleanly( .is_some() } +/// A built-in audit attached to a model column its closed shape lacks: its model and column. +fn missing_model_column<'a>( + audit: &'a AuditSqlInput, + request: &ResourceSqlRequest, + shapes: &HashMap<&str, &[(String, String)]>, +) -> Option<(&'a str, &'a str)> { + let (Some((kind, target)), Some(column)) = (&audit.attached_target, &audit.attached_column) + else { + return None; + }; + if !audit.built_in || kind != MODEL_TARGET_KIND { + return None; + } + let columns: &[(String, String)] = shapes.get(target.as_str()).copied().unwrap_or(&[]); + (!columns.is_empty() + && resolve_binding_column( + column, + columns, + request.dialect.as_deref(), + request.quoted_ignore_case, + ) + .is_none()) + .then_some((target.as_str(), column.as_str())) +} + +/// One built-in audit on a missing model column, reported once at its MODEL header declaration. +pub(crate) struct MissingColumnAudit<'a> { + pub(crate) audit: &'a AuditSqlInput, + pub(crate) model: &'a HookModelInput, + pub(crate) column: &'a str, +} + +/// Built-in audits attached to model columns the model's closed shape lacks. +pub(crate) fn missing_column_audits(request: &ResourceSqlRequest) -> Vec> { + let shapes: HashMap<&str, &[(String, String)]> = shapes_by_name(request); + let models: HashMap<&str, &HookModelInput> = request + .models + .iter() + .map(|model| (model.name.as_str(), model)) + .collect(); + let mut found: Vec> = Vec::new(); + for audit in &request.audits { + let Some((target, column)) = missing_model_column(audit, request, &shapes) else { + continue; + }; + let Some(model) = models.get(target).copied() else { + continue; + }; + if model.analysis_disabled { + continue; + } + found.push(MissingColumnAudit { + audit, + model, + column, + }); + } + found +} + fn disabled_models(request: &ResourceSqlRequest) -> HashSet<&str> { request .models @@ -121,7 +182,10 @@ fn audit_resources(request: &ResourceSqlRequest, disabled: &HashSet<&str>) -> Ve .attached_target .as_ref() .is_some_and(|(_, target)| disabled.contains(&target.as_str())); - if target_disabled || binds_cleanly(audit, request, &shapes) { + if target_disabled + || binds_cleanly(audit, request, &shapes) + || missing_model_column(audit, request, &shapes).is_some() + { continue; } for (sql, authored) in &audit.bodies { diff --git a/crates/sqlbuild-analysis/src/semantic_checks/tests/helpers.rs b/crates/sqlbuild-analysis/src/semantic_checks/tests/helpers.rs index 8495d3de07..9e695d0328 100644 --- a/crates/sqlbuild-analysis/src/semantic_checks/tests/helpers.rs +++ b/crates/sqlbuild-analysis/src/semantic_checks/tests/helpers.rs @@ -1,4 +1,5 @@ use std::collections::{BTreeSet, HashMap}; +use std::sync::Arc; use polyglot_sql::{DialectType, ExpressionWalk, SchemaValidationOptions}; @@ -10,11 +11,11 @@ use crate::semantic_checks::main::complete_semantic_diagnostics::complete_semant use crate::semantic_checks::main::finish_type_recovery::finish_type_recovery; use crate::semantic_checks::main::plan_type_recovery::plan_type_recovery; use crate::semantic_checks::models::{ - CompletedDiagnostic, CompletionModel, CompletionRequest, DiagnosticOwner, FinalDiagnostic, - LineageOutput, MetadataFinding, MetadataFunction, MetadataModel, MetadataOutcome, - MetadataRequest, MetadataSource, MetadataSqlTest, ModelBinding, ModelMetadataFindings, - RawBinding, RecoveryModel, SemanticDiagnostic, SemanticFailure, SemanticLocation, - TypeRecoveryPlan, TypeRecoveryRequest, TypeRecoveryStep, + AuditSqlInput, CompletedDiagnostic, CompletionModel, CompletionRequest, DiagnosticOwner, + FinalDiagnostic, HookModelInput, LineageOutput, MetadataFinding, MetadataFunction, + MetadataModel, MetadataOutcome, MetadataRequest, MetadataSource, MetadataSqlTest, ModelBinding, + ModelMetadataFindings, RawBinding, RecoveryModel, ResourceSqlRequest, SemanticDiagnostic, + SemanticFailure, SemanticLocation, TypeRecoveryPlan, TypeRecoveryRequest, TypeRecoveryStep, }; use crate::semantic_checks::tests::test_types::{ CompletionTestCase, DescribedDiagnostic, MetadataTestCase, OperandTypeTestCase, @@ -702,3 +703,29 @@ pub(crate) fn expected_checked_outcome() -> MetadataOutcome { fallback_types: Vec::new(), } } + +/// A request whose `orders` model outputs only `order_id` and declares audits in its header. +pub(crate) fn missing_column_request(audit: AuditSqlInput) -> ResourceSqlRequest { + ResourceSqlRequest { + dialect: Some("duckdb".to_owned()), + quoted_ignore_case: false, + require_sql_analysis: false, + shapes: vec![( + "orders".to_owned(), + vec![("order_id".to_owned(), "INTEGER".to_owned())], + )], + audits: vec![audit], + sql_tests: Vec::new(), + models: vec![HookModelInput { + name: "orders".to_owned(), + path: "models/orders.sql".to_owned(), + authored_sql: Arc::from( + "MODEL (description \"Orders not_null\", columns (order_id (audits [unique]), \ + shipped_at (audits [not_null, unique])));\nSELECT 1 AS order_id\n", + ), + analysis_disabled: false, + hooks: Vec::new(), + }], + hook_files: Vec::new(), + } +} diff --git a/crates/sqlbuild-analysis/src/semantic_checks/tests/test_resource_sql.rs b/crates/sqlbuild-analysis/src/semantic_checks/tests/test_resource_sql.rs index e4ea278d64..53b07ab184 100644 --- a/crates/sqlbuild-analysis/src/semantic_checks/tests/test_resource_sql.rs +++ b/crates/sqlbuild-analysis/src/semantic_checks/tests/test_resource_sql.rs @@ -1,5 +1,13 @@ +use std::sync::Arc; + +use crate::semantic_checks::_helpers::resource_sql::diagnostics::missing_column_diagnostic; use crate::semantic_checks::_helpers::resource_sql::fixtures::prepared_test_sql; -use crate::semantic_checks::tests::test_types::PreparedTestSqlTestCase; +use crate::semantic_checks::_helpers::resource_sql::resources::missing_column_audits; +use crate::semantic_checks::models::AuditSqlInput; +use crate::semantic_checks::tests::helpers::missing_column_request; +use crate::semantic_checks::tests::test_types::{ + MissingColumnAuditTestCase, PreparedTestSqlTestCase, +}; #[test] fn given_sql_test_bodies_when_preparing_then_empty_fixtures_open_as_python_rewrites_them() { @@ -41,3 +49,92 @@ fn given_sql_test_bodies_when_preparing_then_empty_fixtures_open_as_python_rewri ); } } + +#[test] +fn given_built_in_audit_on_missing_column_when_checking_then_reports_once_at_its_declaration() { + let test_cases = [ + MissingColumnAuditTestCase { + description: "not_null on a missing column points at its MODEL declaration", + column: "shipped_at", + definition: "not_null", + built_in: true, + expected_finding: Some(( + "audit 'not_null' on model 'orders' column 'shipped_at': \ + Unknown column 'shipped_at' in table 'orders'", + 1, + 96, + 104, + )), + }, + MissingColumnAuditTestCase { + description: "unique after the column's first audit points at its own name", + column: "shipped_at", + definition: "unique", + built_in: true, + expected_finding: Some(( + "audit 'unique' on model 'orders' column 'shipped_at': \ + Unknown column 'shipped_at' in table 'orders'", + 1, + 106, + 112, + )), + }, + MissingColumnAuditTestCase { + description: "a column the model outputs is bound as resource SQL", + column: "order_id", + definition: "unique", + built_in: true, + expected_finding: None, + }, + MissingColumnAuditTestCase { + description: "a project audit overriding a built-in keeps its own file", + column: "shipped_at", + definition: "not_null", + built_in: false, + expected_finding: None, + }, + ]; + + for test_case in test_cases { + let request = missing_column_request(AuditSqlInput { + name: test_case.definition.to_owned(), + definition_name: test_case.definition.to_owned(), + attached_target: Some(("model".to_owned(), "orders".to_owned())), + attached_column: Some(test_case.column.to_owned()), + built_in: test_case.built_in, + path: format!("audits/generic/{}.sql", test_case.definition), + contents: Arc::from(""), + bodies: Vec::new(), + opts_out: false, + relations: vec!["orders".to_owned()], + }); + + let found: Option<(String, i64, i64, i64)> = + missing_column_audits(&request).first().map(|missing| { + let diagnostic = missing_column_diagnostic(missing).expect(test_case.description); + assert_eq!(diagnostic.code, "B002", "{}", test_case.description); + assert_eq!( + diagnostic.path, "models/orders.sql", + "{}", + test_case.description + ); + ( + diagnostic.message, + diagnostic.span.0, + diagnostic.span.1, + diagnostic.span.3.unwrap_or_default(), + ) + }); + assert_eq!( + found.as_ref().map(|(message, line, column, end)| ( + message.as_str(), + *line, + *column, + *end + )), + test_case.expected_finding, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-analysis/src/semantic_checks/tests/test_types.rs b/crates/sqlbuild-analysis/src/semantic_checks/tests/test_types.rs index e765fe752b..15b0781def 100644 --- a/crates/sqlbuild-analysis/src/semantic_checks/tests/test_types.rs +++ b/crates/sqlbuild-analysis/src/semantic_checks/tests/test_types.rs @@ -101,3 +101,11 @@ pub(crate) struct PreparedTestSqlTestCase { pub(crate) expected_sql: &'static str, pub(crate) expected_relations: &'static [&'static str], } + +pub(crate) struct MissingColumnAuditTestCase { + pub(crate) description: &'static str, + pub(crate) column: &'static str, + pub(crate) definition: &'static str, + pub(crate) built_in: bool, + pub(crate) expected_finding: Option<(&'static str, i64, i64, i64)>, +} diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_resource_sql_validation.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_resource_sql_validation.py index a4728990ce..e2f3a738ca 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_resource_sql_validation.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_resource_sql_validation.py @@ -190,27 +190,48 @@ ), ), ResourceSqlValidationCase( - description="built-in column audit on a column the model does not output", + description="built-in column audit on a missing column reports once at the header", files=(resource_sql_orders_model(", columns (shipped_at (audits [not_null]))"),), expected_diagnostics=( ( "B002", "audit 'not_null' on model 'orders' column 'shipped_at': " - "Unknown column 'shipped_at' in table 'orders' (context: SELECT)", - "audits/generic/not_null.sql", - 3, + "Unknown column 'shipped_at' in table 'orders'", + RESOURCE_SQL_ORDERS, + 1, ), + ( + "K001", + "declared column 'shipped_at' was not found in statically inferred output " + "for model 'orders'", + RESOURCE_SQL_ORDERS, + 1, + ), + ), + ), + ResourceSqlValidationCase( + description="explicit column contracts report each built-in audit once at the header", + files=( + ( + "sqlbuild_project.toml", + 'name = "orders"\nadapter = "duckdb"\n\n[connection]\n' + 'database = "warehouse.duckdb"\n\n' + '[settings]\ncolumn_contract_mode = "explicit"\n', + ), + resource_sql_orders_model(", columns (shipped_at (audits [not_null, unique]))"), + ), + expected_diagnostics=( ( "B002", "audit 'not_null' on model 'orders' column 'shipped_at': " - "Unknown column 'shipped_at' in table 'orders' (context: SELECT)", - "audits/generic/not_null.sql", - 5, + "Unknown column 'shipped_at' in table 'orders'", + RESOURCE_SQL_ORDERS, + 1, ), ( - "K001", - "declared column 'shipped_at' was not found in statically inferred output " - "for model 'orders'", + "B002", + "audit 'unique' on model 'orders' column 'shipped_at': " + "Unknown column 'shipped_at' in table 'orders'", RESOURCE_SQL_ORDERS, 1, ), From fdff82a0c0d1c169cd520b805fbd254a1d730c6a Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 19:43:26 +0100 Subject: [PATCH 06/12] fix: diagnose scenario expected columns before running and report unrun comparisons A scenario expected CTE listing fewer columns than its model failed in the warehouse with a raw set-operation error, and the result still reported 0 mismatched with actual=0 expected=0. Reject an expected CTE that omits model output columns or lists columns the model does not output with S512 at planning time, and report a comparison that did not run with an ERROR status and no counts; --json adds comparison_status and reports null counts. Refs CHI-633. --- Makefile | 3 +- .../src/compiler/main/mod.rs | 2 + .../main/scenario_expected_column_message.rs | 33 +++++ .../main/scenario_expected_columns.rs | 49 ++++++++ .../sqlbuild-analysis/src/compiler/models.rs | 11 ++ .../sqlbuild-analysis/src/compiler/tests.rs | 2 + .../tests/test_scenario_expected_columns.rs | 80 ++++++++++++ .../src/compiler/tests/test_types.rs | 9 ++ .../scenario_command/_helpers/expectations.rs | 44 ++++--- .../src/scenario_command/constants.rs | 1 + .../src/scenario_command/models.rs | 8 +- .../src/scenario_command/tests/helpers.rs | 10 +- .../tests/test_scenario_rows.rs | 17 +++ .../_helpers/planner_scenario/constants.rs | 2 + .../_helpers/planner_scenario/execution.rs | 47 +++++-- .../bindings/_helpers/result_output/checks.rs | 37 ++++-- .../references/docs/concepts/scenarios.md | 6 + src/sqlbuild/executor/scenario/models.py | 8 +- .../cli/commands/main/scenario/_test_types.py | 12 ++ .../cli/commands/main/scenario/helpers.py | 46 ++++++- .../main/scenario/test_expected_columns.py | 119 ++++++++++++++++++ .../content/docs/docs/concepts/scenarios.mdx | 6 + 22 files changed, 505 insertions(+), 47 deletions(-) create mode 100644 crates/sqlbuild-analysis/src/compiler/main/scenario_expected_column_message.rs create mode 100644 crates/sqlbuild-analysis/src/compiler/main/scenario_expected_columns.rs create mode 100644 crates/sqlbuild-analysis/src/compiler/tests/test_scenario_expected_columns.rs create mode 100644 tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_expected_columns.py diff --git a/Makefile b/Makefile index 5ee3df9120..b71adde5be 100644 --- a/Makefile +++ b/Makefile @@ -139,7 +139,8 @@ E2E_DUCKDB_CLI_DATA_PATHS := \ tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_promotion.py \ tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_concurrency.py \ tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_interrupts.py \ - tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_empty_fixture.py + tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_empty_fixture.py \ + tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_expected_columns.py E2E_DUCKDB_CLI_PATHS := \ tests/e2e/scripts/cli_preview \ diff --git a/crates/sqlbuild-analysis/src/compiler/main/mod.rs b/crates/sqlbuild-analysis/src/compiler/main/mod.rs index e33178c38d..c8aa850963 100644 --- a/crates/sqlbuild-analysis/src/compiler/main/mod.rs +++ b/crates/sqlbuild-analysis/src/compiler/main/mod.rs @@ -1,5 +1,7 @@ pub mod function_type_validation; pub mod missing_expected_columns; +pub mod scenario_expected_column_message; +pub mod scenario_expected_columns; pub mod scenario_expected_comparison_sql; pub mod sql_scenario_classification; pub mod sql_test_assembly; diff --git a/crates/sqlbuild-analysis/src/compiler/main/scenario_expected_column_message.rs b/crates/sqlbuild-analysis/src/compiler/main/scenario_expected_column_message.rs new file mode 100644 index 0000000000..0fccf9a3e8 --- /dev/null +++ b/crates/sqlbuild-analysis/src/compiler/main/scenario_expected_column_message.rs @@ -0,0 +1,33 @@ +//! The pre-execution error for a scenario expected CTE whose columns do not fit its model. + +use crate::compiler::models::ScenarioExpectedColumns; + +/// Why the expected CTE cannot be compared with its model, or `None` when it can. +#[must_use] +pub fn scenario_expected_column_message( + scenario_name: &str, + model_name: &str, + columns: &ScenarioExpectedColumns, + all_columns: bool, +) -> Option { + let mut problems: Vec = Vec::new(); + if all_columns && !columns.unlisted.is_empty() { + problems.push(format!( + "does not list columns that model '{model_name}' outputs: {}", + columns.unlisted.join(", ") + )); + } + if !columns.unknown.is_empty() { + problems.push(format!( + "lists columns that model '{model_name}' does not output: {}", + columns.unknown.join(", ") + )); + } + if problems.is_empty() { + return None; + } + Some(format!( + "scenario '{scenario_name}' expected output __expected__{model_name} {}", + problems.join("; and ") + )) +} diff --git a/crates/sqlbuild-analysis/src/compiler/main/scenario_expected_columns.rs b/crates/sqlbuild-analysis/src/compiler/main/scenario_expected_columns.rs new file mode 100644 index 0000000000..a1cb1379c3 --- /dev/null +++ b/crates/sqlbuild-analysis/src/compiler/main/scenario_expected_columns.rs @@ -0,0 +1,49 @@ +//! Compare a scenario `__expected__` CTE's listed columns with its model's output columns. + +use crate::compiler::_helpers::sql_tests::expected_columns::{ + expected_columns, normalized_identifier, +}; +use crate::compiler::_helpers::sql_tests::rendering::render_dialect; +use crate::compiler::models::ScenarioExpectedColumns; + +/// The columns an expected CTE lists and how they differ from the known model output. +#[must_use] +pub fn scenario_expected_columns( + expected_sql: &str, + dialect: Option<&str>, + model_columns: Option<&[String]>, +) -> ScenarioExpectedColumns { + let listed: Option> = expected_columns(expected_sql, &render_dialect(dialect)); + let (Some(listed_columns), Some(model_columns)) = (listed.as_deref(), model_columns) else { + return ScenarioExpectedColumns { + listed, + unlisted: Vec::new(), + unknown: Vec::new(), + }; + }; + let listed_names: Vec = listed_columns + .iter() + .map(|column| normalized_identifier(column)) + .collect(); + let model_names: Vec = model_columns + .iter() + .map(|column| normalized_identifier(column)) + .collect(); + let unlisted: Vec = model_columns + .iter() + .zip(&model_names) + .filter(|(_, name)| !listed_names.contains(name)) + .map(|(column, _)| column.clone()) + .collect(); + let unknown: Vec = listed_columns + .iter() + .zip(&listed_names) + .filter(|(_, name)| !model_names.contains(name)) + .map(|(column, _)| column.clone()) + .collect(); + ScenarioExpectedColumns { + listed, + unlisted, + unknown, + } +} diff --git a/crates/sqlbuild-analysis/src/compiler/models.rs b/crates/sqlbuild-analysis/src/compiler/models.rs index 04ab21109c..bee2c67c7f 100644 --- a/crates/sqlbuild-analysis/src/compiler/models.rs +++ b/crates/sqlbuild-analysis/src/compiler/models.rs @@ -395,3 +395,14 @@ pub struct SqlScenarioCtes { pub expected_models: Vec, pub assertion_names: Vec, } + +/// A scenario expected CTE's authored columns and how they differ from its model's output. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct ScenarioExpectedColumns { + /// The columns the CTE lists as authored, or `None` when they are not explicit. + pub listed: Option>, + /// Model output columns the CTE does not list. + pub unlisted: Vec, + /// Listed columns the model does not output. + pub unknown: Vec, +} diff --git a/crates/sqlbuild-analysis/src/compiler/tests.rs b/crates/sqlbuild-analysis/src/compiler/tests.rs index 2037327ce9..6f96fc48b5 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests.rs @@ -1,5 +1,7 @@ #[path = "tests/helpers.rs"] mod helpers; +#[path = "tests/test_scenario_expected_columns.rs"] +mod scenario_expected_columns; #[path = "tests/simple_template/helpers.rs"] mod simple_template_helpers; #[path = "tests/single_read/helpers.rs"] diff --git a/crates/sqlbuild-analysis/src/compiler/tests/test_scenario_expected_columns.rs b/crates/sqlbuild-analysis/src/compiler/tests/test_scenario_expected_columns.rs new file mode 100644 index 0000000000..80d5d2e9a6 --- /dev/null +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_scenario_expected_columns.rs @@ -0,0 +1,80 @@ +use crate::compiler::main::scenario_expected_column_message::scenario_expected_column_message; +use crate::compiler::main::scenario_expected_columns::scenario_expected_columns; +use crate::compiler::tests::test_types::ScenarioExpectedColumnsTestCase; + +#[test] +fn given_scenario_expected_cte_when_checking_columns_then_reports_unlisted_and_unknown() { + let test_cases = [ + ScenarioExpectedColumnsTestCase { + description: "every output column listed in another order and case", + expected_sql: "SELECT FALSE AS Is_Cancelled, 'placed' AS status, 'o1' AS order_id", + model_columns: Some(&["order_id", "status", "is_cancelled"]), + all_columns: true, + expected_message: None, + }, + ScenarioExpectedColumnsTestCase { + description: "an unlisted output column fails when every column is required", + expected_sql: "SELECT 'o1' AS order_id, 'placed' AS status", + model_columns: Some(&["order_id", "status", "is_cancelled"]), + all_columns: true, + expected_message: Some( + "scenario 'flags' expected output __expected__order_flags does not list columns \ + that model 'order_flags' outputs: is_cancelled", + ), + }, + ScenarioExpectedColumnsTestCase { + description: "an unlisted output column is ignored when listed columns are compared", + expected_sql: "SELECT 'o1' AS order_id, 'placed' AS status", + model_columns: Some(&["order_id", "status", "is_cancelled"]), + all_columns: false, + expected_message: None, + }, + ScenarioExpectedColumnsTestCase { + description: "a listed column the model lacks always fails", + expected_sql: "SELECT 'o1' AS order_id, 1 AS line_count", + model_columns: Some(&["order_id", "status"]), + all_columns: true, + expected_message: Some( + "scenario 'flags' expected output __expected__order_flags does not list columns \ + that model 'order_flags' outputs: status; and lists columns that model \ + 'order_flags' does not output: line_count", + ), + }, + ScenarioExpectedColumnsTestCase { + description: "an unknown model shape is not checked", + expected_sql: "SELECT 'o1' AS order_id", + model_columns: None, + all_columns: true, + expected_message: None, + }, + ScenarioExpectedColumnsTestCase { + description: "a star expected body is not checked", + expected_sql: "SELECT * FROM __ref(\"orders\")", + model_columns: Some(&["order_id", "status"]), + all_columns: true, + expected_message: None, + }, + ]; + for test_case in test_cases { + let model_columns: Option> = test_case + .model_columns + .map(|columns| columns.iter().map(|column| (*column).to_owned()).collect()); + let columns = scenario_expected_columns( + test_case.expected_sql, + Some("duckdb"), + model_columns.as_deref(), + ); + assert_eq!( + scenario_expected_column_message( + "flags", + "order_flags", + &columns, + test_case.all_columns + ) + .as_deref(), + test_case.expected_message, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs b/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs index d3884c1bb4..559f774889 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs @@ -204,3 +204,12 @@ pub(crate) struct SimpleTemplateCorpusOutcome { pub recognized: usize, pub mismatches: Vec, } + +#[derive(Debug)] +pub(crate) struct ScenarioExpectedColumnsTestCase { + pub(crate) description: &'static str, + pub(crate) expected_sql: &'static str, + pub(crate) model_columns: Option<&'static [&'static str]>, + pub(crate) all_columns: bool, + pub(crate) expected_message: Option<&'static str>, +} diff --git a/crates/sqlbuild-core/src/scenario_command/_helpers/expectations.rs b/crates/sqlbuild-core/src/scenario_command/_helpers/expectations.rs index b35248300c..4520ba327d 100644 --- a/crates/sqlbuild-core/src/scenario_command/_helpers/expectations.rs +++ b/crates/sqlbuild-core/src/scenario_command/_helpers/expectations.rs @@ -3,14 +3,14 @@ use crate::cli_text::models::CliStyle; use crate::scenario_command::_helpers::rows::{result_error, rows_label}; use crate::scenario_command::constants::{ - EXPECTATION_ERROR_INDENT, EXPECTATION_LABEL_WIDTH, EXPECTATION_NAME_WIDTH, FAIL_STATUS, - PASS_STATUS, + ERROR_STATUS, EXPECTATION_ERROR_INDENT, EXPECTATION_LABEL_WIDTH, EXPECTATION_NAME_WIDTH, + FAIL_STATUS, PASS_STATUS, }; use crate::scenario_command::models::{CodedText, ScenarioResultRow}; -/// One `expect` row. -fn expectation_line(item: &str, (failed, detail): (bool, String), style: CliStyle) -> String { - let status: String = style.status(if failed { FAIL_STATUS } else { PASS_STATUS }); +/// One `expect` row under `status`. +fn expectation_line(item: &str, (status, detail): (&str, String), style: CliStyle) -> String { + let status: String = style.status(status); format!( " {:, style: CliStyle) -> String { pub(crate) fn expectation_rows(row: &ScenarioResultRow, style: CliStyle) -> String { let mut text: String = String::new(); for expected in &row.expected { - let detail: String = if expected.failed { - format!(" {} mismatched", expected.mismatched_row_count) - } else { - String::new() - }; + let (status, detail): (&str, String) = + match (expected.failed, expected.mismatched_row_count) { + (false, _) => (PASS_STATUS, String::new()), + (true, Some(mismatched)) => (FAIL_STATUS, format!(" {mismatched} mismatched")), + (true, None) => (ERROR_STATUS, String::new()), + }; text.push_str(&expectation_line( &format!("expected {}", expected.model_name), - (expected.failed, detail), + (status, detail), style, )); text.push_str(&expectation_error(expected.error.as_ref(), style)); @@ -56,7 +57,14 @@ pub(crate) fn expectation_rows(row: &ScenarioResultRow, style: CliStyle) -> Stri }; text.push_str(&expectation_line( &format!("assertion {}", assertion.name), - (assertion.failed, detail), + ( + if assertion.failed { + FAIL_STATUS + } else { + PASS_STATUS + }, + detail, + ), style, )); text.push_str(&expectation_error(assertion.error.as_ref(), style)); @@ -68,12 +76,16 @@ pub(crate) fn expectation_rows(row: &ScenarioResultRow, style: CliStyle) -> Stri pub(crate) fn expectation_failures(row: &ScenarioResultRow) -> String { let mut text: String = String::new(); for expected in row.expected.iter().filter(|expected| expected.failed) { - text.push_str(&format!( - " expected {}: actual={} expected={} mismatched={}\n", - expected.model_name, + let (Some(actual), Some(expected_count), Some(mismatched)) = ( expected.actual_row_count, expected.expected_row_count, - expected.mismatched_row_count + expected.mismatched_row_count, + ) else { + continue; + }; + text.push_str(&format!( + " expected {}: actual={actual} expected={expected_count} mismatched={mismatched}\n", + expected.model_name, )); } for assertion in row.assertions.iter().filter(|assertion| assertion.failed) { diff --git a/crates/sqlbuild-core/src/scenario_command/constants.rs b/crates/sqlbuild-core/src/scenario_command/constants.rs index 5a5540fe8d..7fd8d20293 100644 --- a/crates/sqlbuild-core/src/scenario_command/constants.rs +++ b/crates/sqlbuild-core/src/scenario_command/constants.rs @@ -23,6 +23,7 @@ pub(crate) const BINARY_UNIT_SIZE: f64 = 1024.0; pub(crate) const PASS_STATUS: &str = "PASS"; pub(crate) const FAIL_STATUS: &str = "FAIL"; +pub(crate) const ERROR_STATUS: &str = "ERROR"; pub(crate) const DEFAULT_MAX_SNAPSHOT_ROWS_PER_RELATION: i64 = 10_000; pub(crate) const DEFAULT_MAX_SNAPSHOT_TOTAL_ROWS: i64 = 50_000; diff --git a/crates/sqlbuild-core/src/scenario_command/models.rs b/crates/sqlbuild-core/src/scenario_command/models.rs index f9255c724e..73205e6877 100644 --- a/crates/sqlbuild-core/src/scenario_command/models.rs +++ b/crates/sqlbuild-core/src/scenario_command/models.rs @@ -42,14 +42,14 @@ pub struct CodedText { pub help: Option, } -/// One expected-relation comparison of a scenario result. +/// One expected-relation comparison of a scenario result; counts are `None` if it did not run. #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct ExpectedRow { pub model_name: String, pub failed: bool, - pub actual_row_count: i64, - pub expected_row_count: i64, - pub mismatched_row_count: i64, + pub actual_row_count: Option, + pub expected_row_count: Option, + pub mismatched_row_count: Option, pub error: Option, } diff --git a/crates/sqlbuild-core/src/scenario_command/tests/helpers.rs b/crates/sqlbuild-core/src/scenario_command/tests/helpers.rs index 7571418f09..5bedba03a6 100644 --- a/crates/sqlbuild-core/src/scenario_command/tests/helpers.rs +++ b/crates/sqlbuild-core/src/scenario_command/tests/helpers.rs @@ -76,15 +76,15 @@ pub(super) fn failing_result_row() -> ScenarioResultRow { ExpectedRow { model_name: "fact_orders".to_owned(), failed: true, - actual_row_count: 3, - expected_row_count: 2, - mismatched_row_count: 2, + actual_row_count: Some(3), + expected_row_count: Some(2), + mismatched_row_count: Some(2), error: coded(Some("X506"), "mismatch", Some("look")), }, ExpectedRow { model_name: "dim_customers".to_owned(), - actual_row_count: 1, - expected_row_count: 1, + actual_row_count: Some(1), + expected_row_count: Some(1), ..ExpectedRow::default() }, ], diff --git a/crates/sqlbuild-core/src/scenario_command/tests/test_scenario_rows.rs b/crates/sqlbuild-core/src/scenario_command/tests/test_scenario_rows.rs index 60c710811b..9783e84d64 100644 --- a/crates/sqlbuild-core/src/scenario_command/tests/test_scenario_rows.rs +++ b/crates/sqlbuild-core/src/scenario_command/tests/test_scenario_rows.rs @@ -24,6 +24,7 @@ const RA1: &str = "orders_paid const RB0: &str = "orders_local ERROR\n snapshot missing\n Retained local DuckDB: /work/target/run/scenarios/orders_local/local.duckdb\n"; const RC0: &str = "orders_warehouse FAIL\n boom\n Rerun with --retain to inspect scenario-owned artifacts.\n"; const RD1: &str = "orders_ok \u{1b}[32mPASS\u{1b}[0m 0.01s\n expect expected m \u{1b}[32mPASS\u{1b}[0m\n"; +const RE0: &str = "orders_errored FAIL\n expect expected fact_orders ERROR\n error[X504]: comparison failed to run\n"; const CE0: &str = "orders_paid PASS 2 relations, 3 rows\n source raw_orders PASS 2 rows, 41 B\n seed countries PASS 1 row, 1.5 KB\n snapshot tests/_scenario_snapshots/orders_paid/scenario.json\n"; const SE0R: &str = "orders_paid REFRESHED 2 relations, 3 rows\n source raw_orders PASS 2 rows, 41 B\n seed countries PASS 1 row, 1.5 KB\n snapshot tests/_scenario_snapshots/orders_paid/scenario.json\n"; const CF1: &str = "orders_big \u{1b}[38;5;167mFAIL\u{1b}[0m 0 relations, 0 rows\n \u{1b}[38;5;167m\u{1b}[1merror[X512]:\u{1b}[0m too many rows\n \u{1b}[2m= help:\u{1b}[0m narrow it\n Rerun with --retain to inspect scenario-owned artifacts.\n source raw_orders \u{1b}[38;5;167mFAIL\u{1b}[0m 0 rows, 0 B\n snapshot /elsewhere/scenario.json\n"; @@ -75,6 +76,22 @@ fn given_scenario_result_when_rendering_then_matches_result_row_layout() { use_color: false, expected_text: RC0, }, + ResultRowTestCase { + description: "comparison that did not run shows its error without counts", + row: ScenarioResultRow { + name: "orders_errored".to_owned(), + status: "FAIL".to_owned(), + expected: vec![ExpectedRow { + model_name: "fact_orders".to_owned(), + failed: true, + error: coded(Some("X504"), "comparison failed to run", None), + ..ExpectedRow::default() + }], + ..ScenarioResultRow::default() + }, + use_color: false, + expected_text: RE0, + }, ResultRowTestCase { description: "passing run rounds its duration", row: ScenarioResultRow { diff --git a/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/constants.rs b/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/constants.rs index d88424ebf5..5bbce82b02 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/constants.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/constants.rs @@ -13,4 +13,6 @@ pub(super) const SCENARIO_PLAN_MISSING_FIXTURE_SQL: &str = "S508"; pub(super) const SCENARIO_PLAN_MISSING_RELATION_TARGET: &str = "S509"; pub(super) const SCENARIO_PLAN_INVALID_FIXTURE: &str = "S510"; pub(super) const SCENARIO_PLAN_UNRESOLVED_RELATION_MARKER: &str = "S511"; +pub(super) const SCENARIO_PLAN_EXPECTED_COLUMNS: &str = "S512"; +pub(super) const SCENARIO_EXPECTED_COLUMNS_HELP: &str = "list every column the model outputs in the expected CTE, and remove columns it does not output"; pub(super) const SCENARIO_PLAN_INTERNAL: &str = "S599"; diff --git a/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/execution.rs b/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/execution.rs index 333241eb0f..e6466e3649 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/execution.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/execution.rs @@ -11,7 +11,9 @@ use crate::bindings::_helpers::planner_identity::graph::build_execution_upstream use crate::bindings::_helpers::planner_identity::graph::topologically_order_keys; use crate::bindings::_helpers::planner_phases::objects::object; use crate::bindings::_helpers::planner_resolution::locations::build_function_locations; -use crate::bindings::_helpers::planner_scenario::constants::SCENARIO_PLAN_INVALID_FIXTURE; +use crate::bindings::_helpers::planner_scenario::constants::{ + SCENARIO_EXPECTED_COLUMNS_HELP, SCENARIO_PLAN_EXPECTED_COLUMNS, SCENARIO_PLAN_INVALID_FIXTURE, +}; use crate::bindings::_helpers::planner_scenario::{ entries::{function_entry, seed_entries}, fixtures::{extract_fixtures, fixture_plans}, @@ -27,6 +29,8 @@ use crate::bindings::_helpers::records::fields::replaced_keywords; use crate::bindings::_helpers::records::ids::RecordId; use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyResult, Python, pyfunction}; use pyo3::types::{PyBool, PyDict, PyDictMethods, PyFrozenSet, PyTuple}; +use sqlbuild_analysis::compiler::main::scenario_expected_column_message::scenario_expected_column_message; +use sqlbuild_analysis::compiler::main::scenario_expected_columns::scenario_expected_columns; #[pyfunction] #[pyo3(signature = (**kwargs))] @@ -166,7 +170,7 @@ pub(crate) fn build_scenario_execution_plan<'py>( warnings.push(warning?); } } - let (expected, assertions) = expectations(&scenario, &context)?; + let (expected, assertions) = expectations(&scenario, &context, &models)?; let plan = new_record_with( py, RecordId::ScenarioExecutionPlan, @@ -279,9 +283,40 @@ fn model_context<'py>( .into_any()) } +/// Reject an expected CTE whose listed columns cannot be compared with its model's output. +fn check_expected_columns( + scenario: &Bound<'_, PyAny>, + context: &SqlContext<'_>, + (model, model_name): (Option>, &str), + authored_sql: &str, +) -> PyResult<()> { + let py = scenario.py(); + let model_columns: Option> = match model { + Some(model) => model_reads::inferred_names(&model)?, + None => None, + }; + let dialect: Option = + native_field(&context.adapter, "sql_analysis_dialect_name")?.extract()?; + let columns = + scenario_expected_columns(authored_sql, dialect.as_deref(), model_columns.as_deref()); + let scenario_name: String = native_field(scenario, "name")?.extract()?; + let Some(message) = + scenario_expected_column_message(&scenario_name, model_name, &columns, true) + else { + return Ok(()); + }; + Err(error( + py, + &format!("{message} in {}", context.label), + SCENARIO_PLAN_EXPECTED_COLUMNS, + Some(SCENARIO_EXPECTED_COLUMNS_HELP), + )?) +} + fn expectations<'py>( scenario: &Bound<'py, PyAny>, context: &SqlContext<'py>, + models: &Bound<'py, PyDict>, ) -> PyResult<(Bound<'py, PyAny>, Bound<'py, PyAny>)> { let py = scenario.py(); let mut expected: Vec> = Vec::new(); @@ -290,11 +325,9 @@ fn expectations<'py>( let name: String = native_field(&cte, "name")?.extract()?; let name = name.strip_prefix("__expected__").unwrap_or(&name); let target = required_target(&context.relations, "model_locations", name, "model")?; - let sql = resolve_sql( - context, - &native_field(&cte, "sql_body")?.extract::()?, - false, - )?; + let authored: String = native_field(&cte, "sql_body")?.extract()?; + check_expected_columns(scenario, context, (models.get_item(name)?, name), &authored)?; + let sql = resolve_sql(context, &authored, false)?; expected.push(new_record_with( py, RecordId::ScenarioExpectedExpectationPlan, diff --git a/crates/sqlbuild-python/src/bindings/_helpers/result_output/checks.rs b/crates/sqlbuild-python/src/bindings/_helpers/result_output/checks.rs index 3d3b5d7582..a278ee3f0a 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/result_output/checks.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/result_output/checks.rs @@ -18,6 +18,13 @@ const DECIMAL_KIND: &str = "decimal"; const SCALAR_KINDS: [&str; 5] = ["string", "integer", "boolean", "float", "null"]; const COLLECTION_KINDS: [&str; 2] = ["list", "set"]; const ERROR_FIELDS: [&str; 3] = ["error_code", "error_help", "error_message"]; +const EXPECTED_COUNT_FIELDS: [&str; 3] = [ + "actual_row_count", + "expected_row_count", + "mismatched_row_count", +]; +const COMPARISON_COMPLETED: &str = "completed"; +const COMPARISON_NOT_RUN: &str = "not_run"; fn value<'py>(result: &Bound<'py, PyAny>, field: &str) -> PyResult> { native_field(&native_field(result, field)?, "value") @@ -393,19 +400,31 @@ pub(crate) fn scenario_check<'py>(result: &Bound<'py, PyAny>) -> PyResult = value(result, "status")?; let mut expected: Vec> = Vec::new(); for item in items(&native_field(result, "expected_results")?)? { + let counts: Vec<(&'static str, Bound<'py, PyAny>)> = fields(&item, &EXPECTED_COUNT_FIELDS)?; + let compared: bool = counts.iter().all(|(_, count)| !count.is_none()); let mut entries: Vec<(&'static str, Bound<'py, PyAny>)> = vec![ ("model_name", native_field(&item, "model_name")?), ("status", value(&item, "status")?), + ( + "comparison_status", + any( + py, + if compared { + COMPARISON_COMPLETED + } else { + COMPARISON_NOT_RUN + }, + )?, + ), ]; - entries.extend(fields( - &item, - &[ - "actual_row_count", - "expected_row_count", - "mismatched_row_count", - ], - )?); - expected.push(with_errors(&item, entries)?); + entries.extend(counts); + let result: Bound<'py, PyDict> = dict(py, entries)?; + for (name, error) in fields(&item, &ERROR_FIELDS)? { + if !error.is_none() { + result.set_item(name, error)?; + } + } + expected.push(result); } let mut assertions: Vec> = Vec::new(); for item in items(&native_field(result, "assertion_results")?)? { diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md index 6ef0c847bf..e49deb085a 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md @@ -109,6 +109,12 @@ __assert__all_orders_have_payments AS ( | `__expected__` | Full expected output. Compared order-insensitively against the scenario-built model. | | `__assert__` | Zero-row assertion. Passes if the query returns no rows; fails with the returned rows as diagnostics. | +An `__expected__` CTE must list every column the model outputs, and no others. When the +CTE's columns and the model's output columns are both known statically, a mismatch fails planning +with `S512`, naming the missing or extra columns, before the scenario runs. A comparison that fails +to run reports its error with an `ERROR` status and no row counts; in `--json` output its +`comparison_status` is `not_run` and its row counts are `null`. + For sources with two-part identity, use double underscores: `__source__raw__orders`. To mock a contracted source, ref, or seed with no rows, use `SELECT * FROM __empty_fixture()` as diff --git a/src/sqlbuild/executor/scenario/models.py b/src/sqlbuild/executor/scenario/models.py index 6764fe43ad..eb193696d4 100644 --- a/src/sqlbuild/executor/scenario/models.py +++ b/src/sqlbuild/executor/scenario/models.py @@ -310,14 +310,14 @@ class ScenarioCleanupExecutionResult: @dataclass(frozen=True) class ScenarioExpectedExpectationExecutionResult: - """Outcome of comparing one scenario model relation to expected SQL.""" + """Outcome of comparing one scenario model relation to expected SQL; no counts if not run.""" scenario_name: str model_name: str status: ExecutionStatus - actual_row_count: int = 0 - expected_row_count: int = 0 - mismatched_row_count: int = 0 + actual_row_count: int | None = None + expected_row_count: int | None = None + mismatched_row_count: int | None = None error_code: str | None = None error_help: str | None = None error_message: str | None = None diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py index 9811d4014c..25dc768087 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py @@ -249,3 +249,15 @@ class PromotionConflictReuseE2ETestCase: settings_steps: tuple[str, ...] expected_exit_codes: tuple[int, ...] expected_reused: tuple[bool, ...] + + +@dataclass(frozen=True) +class ScenarioExpectedColumnsE2ETestCase: + """Test case for comparing a scenario's expected CTE with its model's output columns.""" + + description: str + expected_sql: str + expected_exit_code: int + expected_stdout_fragments: tuple[str, ...] + unexpected_stdout_fragments: tuple[str, ...] = field(default_factory=tuple) + expected_json_results: tuple[dict[str, object], ...] = field(default_factory=tuple) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/helpers.py b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/helpers.py index 6440dbe38c..5f34add023 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/helpers.py @@ -12,7 +12,7 @@ from collections import defaultdict from itertools import takewhile from pathlib import Path -from typing import IO, NamedTuple, cast +from typing import IO, Any, NamedTuple, cast from sqlbuild._native import COMPILE_REUSE_DISABLE_ENV_VAR as REUSE_DISABLE_ENV_VAR from sqlbuild._native import ScenarioArtifactKind @@ -880,3 +880,47 @@ def run_reusable_compile(*, project_dir: Path) -> subprocess.CompletedProcess[st env={**os.environ, REUSE_DISABLE_ENV_VAR: "0"}, check=False, ) + + +_EXPECTED_COLUMNS_PROJECT_FILES: dict[str, str] = { + "sqlbuild_project.toml": ( + 'name = "scenario_expected_columns"\nadapter = "duckdb"\n\n' + '[connection]\ndatabase = "warehouse.duckdb"\n' + ), + "models/orders.sql": ( + 'MODEL (description "Orders", materialized table);\n\n' + "SELECT order_id, status\n" + "FROM (VALUES ('o1', 'placed')) AS t(order_id, status)\n" + ), + "models/order_flags.sql": ( + 'MODEL (description "Orders with a flag", materialized table);\n\n' + 'WITH orders AS (SELECT * FROM __ref("orders"))\n\n' + "SELECT order_id, status, status = 'cancelled' AS is_cancelled\n" + "FROM orders\n" + ), +} +_EXPECTED_COLUMNS_SCENARIO_PATH: str = "tests/scenarios/order_flags__expected.sql" +_EXPECTED_COLUMNS_SCENARIO_HEADER: str = ( + 'SCENARIO (description "Order flags keep their status");\n\n' +) +_EXPECTED_COLUMNS_FIXTURE: str = ( + "WITH __ref__orders AS (\n SELECT 'o1' AS order_id, 'placed' AS status\n),\n\n" +) + + +def build_expected_columns_scenario_project_files(*, expected_sql: str) -> dict[str, str]: + """Build an order-flags project whose scenario expects `expected_sql` for the model.""" + + return { + **_EXPECTED_COLUMNS_PROJECT_FILES, + _EXPECTED_COLUMNS_SCENARIO_PATH: ( + f"{_EXPECTED_COLUMNS_SCENARIO_HEADER}{_EXPECTED_COLUMNS_FIXTURE}" + f"__expected__order_flags AS (\n {expected_sql}\n)\n" + ), + } + + +def selected_fields(result: dict[str, Any], expected: dict[str, object]) -> dict[str, object]: + """Return the fields of `result` that `expected` names.""" + + return {key: result.get(key) for key in expected} diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_expected_columns.py b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_expected_columns.py new file mode 100644 index 0000000000..5380e2feb3 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_expected_columns.py @@ -0,0 +1,119 @@ +"""E2E tests for scenario expected CTEs whose columns differ from their model's output.""" + +from __future__ import annotations + +import json +import subprocess +from pathlib import Path +from typing import Any + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.scenario._test_types import ( + ScenarioExpectedColumnsE2ETestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.scenario.helpers import ( + build_expected_columns_scenario_project_files, + selected_fields, +) +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project, run_sqb + + +@pytest.mark.parametrize( + "test_case", + ( + ScenarioExpectedColumnsE2ETestCase( + description="complete expected output compares every column", + expected_sql="SELECT 'o1' AS order_id, 'placed' AS status, FALSE AS is_cancelled", + expected_exit_code=0, + expected_stdout_fragments=("PASS=1 FAIL=0 TOTAL=1",), + expected_json_results=( + { + "model_name": "order_flags", + "status": "success", + "comparison_status": "completed", + "actual_row_count": 1, + "expected_row_count": 1, + "mismatched_row_count": 0, + }, + ), + ), + ScenarioExpectedColumnsE2ETestCase( + description="expected output missing a model column is rejected before execution", + expected_sql="SELECT 'o1' AS order_id, 'placed' AS status", + expected_exit_code=1, + expected_stdout_fragments=( + "error[S512]", + "expected output __expected__order_flags does not list columns that model " + "'order_flags' outputs: is_cancelled", + ), + unexpected_stdout_fragments=("X504", "mismatched", "actual="), + ), + ScenarioExpectedColumnsE2ETestCase( + description="expected output listing a column the model lacks is rejected", + expected_sql=( + "SELECT 'o1' AS order_id, 'placed' AS status, FALSE AS is_cancelled, " + "1 AS line_count" + ), + expected_exit_code=1, + expected_stdout_fragments=( + "error[S512]", + "lists columns that model 'order_flags' does not output: line_count", + ), + unexpected_stdout_fragments=("X504", "mismatched"), + ), + ScenarioExpectedColumnsE2ETestCase( + description="comparison that fails to run reports its error without counts", + expected_sql=( + "SELECT 'o1' AS order_id, 'placed' AS status, " + "CAST('maybe' AS BOOLEAN) AS is_cancelled" + ), + expected_exit_code=1, + expected_stdout_fragments=("expected order_flags", "ERROR", "error[X504]"), + unexpected_stdout_fragments=("mismatched", "actual="), + expected_json_results=( + { + "model_name": "order_flags", + "status": "failed", + "comparison_status": "not_run", + "actual_row_count": None, + "expected_row_count": None, + "mismatched_row_count": None, + "error_code": "X504", + }, + ), + ), + ), + ids=lambda case: case.description, +) +def test_given_scenario_expected_columns_when_testing_then_comparison_is_reported_honestly( + test_case: ScenarioExpectedColumnsE2ETestCase, tmp_path: Path +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="scenario_expected_columns", + repo_files=build_expected_columns_scenario_project_files( + expected_sql=test_case.expected_sql + ), + ) + + text: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", "scenario", "test"), project_dir=project_dir + ) + structured: subprocess.CompletedProcess[str] = run_sqb( + command=("scenario", "test", "--json"), project_dir=project_dir + ) + + assert text.returncode == test_case.expected_exit_code, text.stdout + text.stderr + assert structured.returncode == test_case.expected_exit_code, structured.stderr + for fragment in test_case.expected_stdout_fragments: + assert fragment in text.stdout, text.stdout + for fragment in test_case.unexpected_stdout_fragments: + assert fragment not in text.stdout, text.stdout + payload: dict[str, Any] = json.loads(structured.stdout) + results: list[dict[str, Any]] = payload["checks"][0]["expected_results"] + assert len(results) == len(test_case.expected_json_results), results + assert ( + tuple(map(selected_fields, results, test_case.expected_json_results)) + == test_case.expected_json_results + ) diff --git a/website/src/content/docs/docs/concepts/scenarios.mdx b/website/src/content/docs/docs/concepts/scenarios.mdx index 56c0252873..78fb6c6517 100644 --- a/website/src/content/docs/docs/concepts/scenarios.mdx +++ b/website/src/content/docs/docs/concepts/scenarios.mdx @@ -97,6 +97,12 @@ __assert__all_orders_have_payments AS ( | `__expected__` | Full expected output. Compared order-insensitively against the scenario-built model. | | `__assert__` | Zero-row assertion. Passes if the query returns no rows; fails with the returned rows as diagnostics. | +An `__expected__` CTE must list every column the model outputs, and no others. When the +CTE's columns and the model's output columns are both known statically, a mismatch fails planning +with `S512`, naming the missing or extra columns, before the scenario runs. A comparison that fails +to run reports its error with an `ERROR` status and no row counts; in `--json` output its +`comparison_status` is `not_run` and its row counts are `null`. + For sources with two-part identity, use double underscores: `__source__raw__orders`. To mock a contracted source, ref, or seed with no rows, use `SELECT * FROM __empty_fixture()` as From 9b1f2d0250615ca75358e0052530d17395e95225 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 21:26:55 +0100 Subject: [PATCH 07/12] feat: compare scenario expected output on its listed columns A scenario __expected__ CTE had to list every column its model outputs, so adding a column to a model broke every scenario covering it. Compare the listed columns by name in both directions, as SQL unit tests do, and report missing rows alongside mismatched ones. SCENARIO (expected_columns "all") restores the requirement that every model column be listed, diagnosed with S512 before the scenario runs. Fixes CHI-633. --- .../compiler/_helpers/sql_tests/rendering.rs | 37 ++++-- .../main/scenario_expected_comparison_sql.rs | 18 ++- .../scenario_command/_helpers/expectations.rs | 12 +- .../src/scenario_command/models.rs | 1 + .../src/scenario_command/tests/helpers.rs | 1 + .../tests/test_scenario_rows.rs | 4 +- .../_helpers/analysis/sql_test_comparisons.rs | 12 +- .../bindings/_helpers/discovery/constants.rs | 7 +- .../_helpers/discovery/sql_test_records.rs | 10 ++ .../_helpers/planner_records/constants.rs | 1 + .../_helpers/planner_scenario/execution.rs | 33 ++++- .../bindings/_helpers/result_output/checks.rs | 3 +- .../_helpers/scenario_command/conversions.rs | 4 + .../references/docs/concepts/scenarios.md | 23 ++-- .../references/docs/concepts/testing.md | 1 + src/sqlbuild/_native.pyi | 8 +- .../scenario/main/_expected_expectations.py | 8 +- src/sqlbuild/executor/scenario/models.py | 1 + .../cli/commands/main/scenario/_test_types.py | 11 ++ .../cli/commands/main/scenario/helpers.py | 11 +- .../main/scenario/test_expected_columns.py | 115 +++++++++++++++++- .../commands/main/scenario/test_scenario.py | 4 +- .../executor/scenario/main/_test_types.py | 1 + .../main/test_expected_comparison_sql.py | 14 ++- .../content/docs/docs/concepts/scenarios.mdx | 23 ++-- .../content/docs/docs/concepts/testing.mdx | 1 + 26 files changed, 305 insertions(+), 59 deletions(-) diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/rendering.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/rendering.rs index 606e29a7c0..d1d2e2f72f 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/rendering.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/rendering.rs @@ -696,9 +696,26 @@ fn pruned_statement(statement: &str, dialect: SliceDialect) -> String { /// Columns compared for an expected-output step: the listed expected columns, else every column. fn compared_projection(step: &SqlTestChainStep) -> String { - step.expected_columns - .as_ref() - .map_or_else(|| "*".to_string(), |columns| columns.join(", ")) + compared_columns_projection(step.expected_columns.as_deref()) +} + +/// The projection comparing `columns` by name, or every column positionally without a list. +pub(crate) fn compared_columns_projection(columns: Option<&[String]>) -> String { + columns.map_or_else(|| "*".to_string(), |columns| columns.join(", ")) +} + +/// Row counts of `actual` and `expected` and of the rows each has that the other lacks. +pub(crate) fn comparison_counts_sql( + (actual, expected): (&str, &str), + projection: &str, + set_difference_operator: &str, +) -> String { + format!( + "(SELECT COUNT(*) FROM {actual}) AS actual_count, \ + (SELECT COUNT(*) FROM {expected}) AS expected_count, \ + (SELECT COUNT(*) FROM (SELECT {projection} FROM {actual} {set_difference_operator} SELECT {projection} FROM {expected}) AS __sqlbuild_mismatch) AS unexpected_count, \ + (SELECT COUNT(*) FROM (SELECT {projection} FROM {expected} {set_difference_operator} SELECT {projection} FROM {actual}) AS __sqlbuild_missing) AS missing_count" + ) } /// Render one planned test's comparison query, lifting authored CTE text verbatim. @@ -753,16 +770,14 @@ pub(crate) fn render_comparison_sql(request: &RenderRequest) -> Result, set_difference_operator: &str, ) -> String { format!( - "WITH __actual AS ({actual_sql}), __expected AS ({expected_sql}) SELECT \ - (SELECT COUNT(*) FROM __actual) AS actual_count, \ - (SELECT COUNT(*) FROM __expected) AS expected_count, \ - (SELECT COUNT(*) FROM (SELECT * FROM __actual {set_difference_operator} SELECT * FROM __expected\ - ) AS __sqlbuild_mismatch) AS mismatched_count" + "WITH __actual AS ({actual_sql}), __expected AS ({expected_sql}) SELECT {}", + comparison_counts_sql( + ("__actual", "__expected"), + &compared_columns_projection(compared_columns), + set_difference_operator, + ) ) } diff --git a/crates/sqlbuild-core/src/scenario_command/_helpers/expectations.rs b/crates/sqlbuild-core/src/scenario_command/_helpers/expectations.rs index 4520ba327d..5c7413aeec 100644 --- a/crates/sqlbuild-core/src/scenario_command/_helpers/expectations.rs +++ b/crates/sqlbuild-core/src/scenario_command/_helpers/expectations.rs @@ -35,7 +35,13 @@ pub(crate) fn expectation_rows(row: &ScenarioResultRow, style: CliStyle) -> Stri let (status, detail): (&str, String) = match (expected.failed, expected.mismatched_row_count) { (false, _) => (PASS_STATUS, String::new()), - (true, Some(mismatched)) => (FAIL_STATUS, format!(" {mismatched} mismatched")), + (true, Some(mismatched)) => ( + FAIL_STATUS, + format!( + " {mismatched} mismatched, {} missing", + expected.missing_row_count.unwrap_or_default() + ), + ), (true, None) => (ERROR_STATUS, String::new()), }; text.push_str(&expectation_line( @@ -84,8 +90,10 @@ pub(crate) fn expectation_failures(row: &ScenarioResultRow) -> String { continue; }; text.push_str(&format!( - " expected {}: actual={actual} expected={expected_count} mismatched={mismatched}\n", + " expected {}: actual={actual} expected={expected_count} mismatched={mismatched} \ + missing={}\n", expected.model_name, + expected.missing_row_count.unwrap_or_default() )); } for assertion in row.assertions.iter().filter(|assertion| assertion.failed) { diff --git a/crates/sqlbuild-core/src/scenario_command/models.rs b/crates/sqlbuild-core/src/scenario_command/models.rs index 73205e6877..6abc7ff468 100644 --- a/crates/sqlbuild-core/src/scenario_command/models.rs +++ b/crates/sqlbuild-core/src/scenario_command/models.rs @@ -50,6 +50,7 @@ pub struct ExpectedRow { pub actual_row_count: Option, pub expected_row_count: Option, pub mismatched_row_count: Option, + pub missing_row_count: Option, pub error: Option, } diff --git a/crates/sqlbuild-core/src/scenario_command/tests/helpers.rs b/crates/sqlbuild-core/src/scenario_command/tests/helpers.rs index 5bedba03a6..6f1702d9ba 100644 --- a/crates/sqlbuild-core/src/scenario_command/tests/helpers.rs +++ b/crates/sqlbuild-core/src/scenario_command/tests/helpers.rs @@ -79,6 +79,7 @@ pub(super) fn failing_result_row() -> ScenarioResultRow { actual_row_count: Some(3), expected_row_count: Some(2), mismatched_row_count: Some(2), + missing_row_count: Some(1), error: coded(Some("X506"), "mismatch", Some("look")), }, ExpectedRow { diff --git a/crates/sqlbuild-core/src/scenario_command/tests/test_scenario_rows.rs b/crates/sqlbuild-core/src/scenario_command/tests/test_scenario_rows.rs index 9783e84d64..0856ca550a 100644 --- a/crates/sqlbuild-core/src/scenario_command/tests/test_scenario_rows.rs +++ b/crates/sqlbuild-core/src/scenario_command/tests/test_scenario_rows.rs @@ -19,8 +19,8 @@ use crate::scenario_command::tests::test_types::{ CaptureRowTestCase, LocalArtifactsTestCase, ResultRowTestCase, WarehouseArtifactsTestCase, }; -const RA0: &str = "orders_paid FAIL 1.23s\n error[X506]: expected comparison failed\n second line\n = help: Compare the CTE.\n expect expected fact_orders FAIL 2 mismatched\n error[X506]: mismatch\n = help: look\n expect expected dim_customers PASS\n expect assertion no_negative FAIL 1 row\n expect assertion few FAIL 3 rows\n plain error\n expect assertion ok PASS\n Retained relations:\n source raw_orders -> main.__sqb_abc__source__raw_orders\n model fact_orders -> main.__sqb_abc__model__fact_orders\n expected fact_orders: actual=3 expected=2 mismatched=2\n assertion no_negative: failing_rows=1\n sample: (1, 'a', None)\n assertion few: failing_rows=3\n"; -const RA1: &str = "orders_paid \u{1b}[38;5;167mFAIL\u{1b}[0m 1.23s\n \u{1b}[38;5;167m\u{1b}[1merror[X506]:\u{1b}[0m expected comparison failed\n \u{1b}[2msecond line\u{1b}[0m\n \u{1b}[2m= help:\u{1b}[0m Compare the CTE.\n expect expected fact_orders \u{1b}[38;5;167mFAIL\u{1b}[0m 2 mismatched\n \u{1b}[38;5;167m\u{1b}[1merror[X506]:\u{1b}[0m mismatch\n \u{1b}[2m= help:\u{1b}[0m look\n expect expected dim_customers \u{1b}[32mPASS\u{1b}[0m\n expect assertion no_negative \u{1b}[38;5;167mFAIL\u{1b}[0m 1 row\n expect assertion few \u{1b}[38;5;167mFAIL\u{1b}[0m 3 rows\n plain error\n expect assertion ok \u{1b}[32mPASS\u{1b}[0m\n Retained relations:\n source raw_orders -> main.__sqb_abc__source__raw_orders\n model fact_orders -> main.__sqb_abc__model__fact_orders\n expected fact_orders: actual=3 expected=2 mismatched=2\n assertion no_negative: failing_rows=1\n sample: (1, 'a', None)\n assertion few: failing_rows=3\n"; +const RA0: &str = "orders_paid FAIL 1.23s\n error[X506]: expected comparison failed\n second line\n = help: Compare the CTE.\n expect expected fact_orders FAIL 2 mismatched, 1 missing\n error[X506]: mismatch\n = help: look\n expect expected dim_customers PASS\n expect assertion no_negative FAIL 1 row\n expect assertion few FAIL 3 rows\n plain error\n expect assertion ok PASS\n Retained relations:\n source raw_orders -> main.__sqb_abc__source__raw_orders\n model fact_orders -> main.__sqb_abc__model__fact_orders\n expected fact_orders: actual=3 expected=2 mismatched=2 missing=1\n assertion no_negative: failing_rows=1\n sample: (1, 'a', None)\n assertion few: failing_rows=3\n"; +const RA1: &str = "orders_paid \u{1b}[38;5;167mFAIL\u{1b}[0m 1.23s\n \u{1b}[38;5;167m\u{1b}[1merror[X506]:\u{1b}[0m expected comparison failed\n \u{1b}[2msecond line\u{1b}[0m\n \u{1b}[2m= help:\u{1b}[0m Compare the CTE.\n expect expected fact_orders \u{1b}[38;5;167mFAIL\u{1b}[0m 2 mismatched, 1 missing\n \u{1b}[38;5;167m\u{1b}[1merror[X506]:\u{1b}[0m mismatch\n \u{1b}[2m= help:\u{1b}[0m look\n expect expected dim_customers \u{1b}[32mPASS\u{1b}[0m\n expect assertion no_negative \u{1b}[38;5;167mFAIL\u{1b}[0m 1 row\n expect assertion few \u{1b}[38;5;167mFAIL\u{1b}[0m 3 rows\n plain error\n expect assertion ok \u{1b}[32mPASS\u{1b}[0m\n Retained relations:\n source raw_orders -> main.__sqb_abc__source__raw_orders\n model fact_orders -> main.__sqb_abc__model__fact_orders\n expected fact_orders: actual=3 expected=2 mismatched=2 missing=1\n assertion no_negative: failing_rows=1\n sample: (1, 'a', None)\n assertion few: failing_rows=3\n"; const RB0: &str = "orders_local ERROR\n snapshot missing\n Retained local DuckDB: /work/target/run/scenarios/orders_local/local.duckdb\n"; const RC0: &str = "orders_warehouse FAIL\n boom\n Rerun with --retain to inspect scenario-owned artifacts.\n"; const RD1: &str = "orders_ok \u{1b}[32mPASS\u{1b}[0m 0.01s\n expect expected m \u{1b}[32mPASS\u{1b}[0m\n"; diff --git a/crates/sqlbuild-python/src/bindings/_helpers/analysis/sql_test_comparisons.rs b/crates/sqlbuild-python/src/bindings/_helpers/analysis/sql_test_comparisons.rs index 3698831d1d..7dd4056ecd 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/analysis/sql_test_comparisons.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/analysis/sql_test_comparisons.rs @@ -235,15 +235,21 @@ pub(crate) fn describe_missing_expected_columns( )) } -/// SQL comparing a scenario's actual output with its expected SQL. +/// SQL comparing a scenario's actual output with its expected SQL on the compared columns. #[pyfunction] -#[pyo3(signature = (*, actual_sql, expected_sql, set_difference_operator))] +#[pyo3(signature = (*, actual_sql, expected_sql, set_difference_operator, compared_columns = None))] pub(crate) fn build_scenario_expected_comparison_sql( actual_sql: &str, expected_sql: &str, set_difference_operator: &str, + compared_columns: Option>, ) -> String { - scenario_expected_comparison_sql(actual_sql, expected_sql, set_difference_operator) + scenario_expected_comparison_sql( + actual_sql, + expected_sql, + compared_columns.as_deref(), + set_difference_operator, + ) } pub(crate) fn register(module: &Bound<'_, PyModule>) -> PyResult<()> { diff --git a/crates/sqlbuild-python/src/bindings/_helpers/discovery/constants.rs b/crates/sqlbuild-python/src/bindings/_helpers/discovery/constants.rs index 5503d6a77f..e7019c3865 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/discovery/constants.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/discovery/constants.rs @@ -103,7 +103,12 @@ pub(crate) const SQL_TEST_HEADER_KEYS: [&str; 8] = [ "sql_analysis", "timeout", ]; -pub(crate) const SQL_SCENARIO_HEADER_KEYS: [&str; 3] = ["description", "tags", "timeout"]; +pub(crate) const SQL_SCENARIO_HEADER_KEYS: [&str; 4] = + ["description", "expected_columns", "tags", "timeout"]; +/// `SCENARIO (expected_columns ...)` values: compare listed columns, or require every column. +pub(crate) const SCENARIO_EXPECTED_COLUMNS_KEY: &str = "expected_columns"; +pub(crate) const SCENARIO_EXPECTED_COLUMNS_VALUES: [&str; 2] = ["listed", "all"]; +pub(crate) const SCENARIO_EXPECTED_COLUMNS_ALL: &str = "all"; /// The fields a SQL hook entry reports, in output order. pub(crate) const SQL_HOOK_OUTPUT_FIELDS: [&str; 6] = [ "statement", diff --git a/crates/sqlbuild-python/src/bindings/_helpers/discovery/sql_test_records.rs b/crates/sqlbuild-python/src/bindings/_helpers/discovery/sql_test_records.rs index e464542475..c5abe9872c 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/discovery/sql_test_records.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/discovery/sql_test_records.rs @@ -191,6 +191,16 @@ fn scenario_file<'py>( )); } } + if let Some(columns) = header_values.get_item(constants::SCENARIO_EXPECTED_COLUMNS_KEY)? { + let valid: bool = columns.extract::().is_ok_and(|value| { + constants::SCENARIO_EXPECTED_COLUMNS_VALUES.contains(&value.as_str()) + }); + if !valid { + return failure(format!( + "SCENARIO() expected_columns in '{shown}' must be \"listed\" or \"all\"" + )); + } + } if file.sql_body.is_empty() { return failure(format!( "SQL scenario '{shown}' must define SQL after SCENARIO(...)" diff --git a/crates/sqlbuild-python/src/bindings/_helpers/planner_records/constants.rs b/crates/sqlbuild-python/src/bindings/_helpers/planner_records/constants.rs index 888d7e6765..5a059259dc 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/planner_records/constants.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/planner_records/constants.rs @@ -859,6 +859,7 @@ const R_SCENARIO_EXPECTED_EXPECTATION_PLAN: RecordSpec = RecordSpec { FieldSpec::required("model_name"), FieldSpec::required("actual_destination"), FieldSpec::required("expected_sql"), + FieldSpec::new("compared_columns", FieldDefault::None), ], }; const R_SCENARIO_ASSERTION_EXPECTATION_PLAN: RecordSpec = RecordSpec { diff --git a/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/execution.rs b/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/execution.rs index e6466e3649..3c1a004083 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/execution.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/execution.rs @@ -5,6 +5,9 @@ use crate::bindings::_helpers::compile_driver::python_calls::KeywordArguments; use crate::bindings::_helpers::compile_driver::python_calls::keywords; use crate::bindings::_helpers::compiled_project::loop_classes::by_attribute; use crate::bindings::_helpers::compiled_project::records::compiled_model_record; +use crate::bindings::_helpers::discovery::constants::{ + SCENARIO_EXPECTED_COLUMNS_ALL, SCENARIO_EXPECTED_COLUMNS_KEY, +}; use crate::bindings::_helpers::planner_entries::model_entries::plan_model; use crate::bindings::_helpers::planner_entries::models::ModelPlanContext; use crate::bindings::_helpers::planner_identity::graph::build_execution_upstream_deps; @@ -25,6 +28,7 @@ use crate::bindings::_helpers::planner_strategy::fixture_completion::complete_fi use crate::bindings::_helpers::planner_strategy::fixture_context::planning_context; use crate::bindings::_helpers::records::construct::{enum_member, new_record, new_record_with}; use crate::bindings::_helpers::records::fields::field as native_field; +use crate::bindings::_helpers::records::fields::mapping_get; use crate::bindings::_helpers::records::fields::replaced_keywords; use crate::bindings::_helpers::records::ids::RecordId; use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyResult, Python, pyfunction}; @@ -283,13 +287,21 @@ fn model_context<'py>( .into_any()) } -/// Reject an expected CTE whose listed columns cannot be compared with its model's output. -fn check_expected_columns( +/// Whether the scenario requires its expected CTEs to list every model output column. +fn requires_all_columns(scenario: &Bound<'_, PyAny>) -> PyResult { + let header: Bound<'_, PyAny> = + native_field(&native_field(scenario, "scenario_file")?, "header_values")?; + let value: Bound<'_, PyAny> = mapping_get(&header, SCENARIO_EXPECTED_COLUMNS_KEY)?; + Ok(!value.is_none() && value.extract::()? == SCENARIO_EXPECTED_COLUMNS_ALL) +} + +/// The columns compared by name, after rejecting an expected CTE that cannot be compared. +fn compared_columns( scenario: &Bound<'_, PyAny>, context: &SqlContext<'_>, (model, model_name): (Option>, &str), authored_sql: &str, -) -> PyResult<()> { +) -> PyResult>> { let py = scenario.py(); let model_columns: Option> = match model { Some(model) => model_reads::inferred_names(&model)?, @@ -300,10 +312,11 @@ fn check_expected_columns( let columns = scenario_expected_columns(authored_sql, dialect.as_deref(), model_columns.as_deref()); let scenario_name: String = native_field(scenario, "name")?.extract()?; + let all_columns: bool = requires_all_columns(scenario)?; let Some(message) = - scenario_expected_column_message(&scenario_name, model_name, &columns, true) + scenario_expected_column_message(&scenario_name, model_name, &columns, all_columns) else { - return Ok(()); + return Ok(columns.listed); }; Err(error( py, @@ -326,7 +339,8 @@ fn expectations<'py>( let name = name.strip_prefix("__expected__").unwrap_or(&name); let target = required_target(&context.relations, "model_locations", name, "model")?; let authored: String = native_field(&cte, "sql_body")?.extract()?; - check_expected_columns(scenario, context, (models.get_item(name)?, name), &authored)?; + let compared = + compared_columns(scenario, context, (models.get_item(name)?, name), &authored)?; let sql = resolve_sql(context, &authored, false)?; expected.push(new_record_with( py, @@ -335,6 +349,13 @@ fn expectations<'py>( ("model_name", string(py, name)), ("actual_destination", target), ("expected_sql", string(py, &sql)), + ( + "compared_columns", + match compared { + Some(columns) => PyTuple::new(py, columns)?.into_any(), + None => py.None().into_bound(py), + }, + ), ], )?); } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/result_output/checks.rs b/crates/sqlbuild-python/src/bindings/_helpers/result_output/checks.rs index a278ee3f0a..ffd2f0e442 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/result_output/checks.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/result_output/checks.rs @@ -18,10 +18,11 @@ const DECIMAL_KIND: &str = "decimal"; const SCALAR_KINDS: [&str; 5] = ["string", "integer", "boolean", "float", "null"]; const COLLECTION_KINDS: [&str; 2] = ["list", "set"]; const ERROR_FIELDS: [&str; 3] = ["error_code", "error_help", "error_message"]; -const EXPECTED_COUNT_FIELDS: [&str; 3] = [ +const EXPECTED_COUNT_FIELDS: [&str; 4] = [ "actual_row_count", "expected_row_count", "mismatched_row_count", + "missing_row_count", ]; const COMPARISON_COMPLETED: &str = "completed"; const COMPARISON_NOT_RUN: &str = "not_run"; diff --git a/crates/sqlbuild-python/src/bindings/_helpers/scenario_command/conversions.rs b/crates/sqlbuild-python/src/bindings/_helpers/scenario_command/conversions.rs index 005ee770c3..6041f97185 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/scenario_command/conversions.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/scenario_command/conversions.rs @@ -73,6 +73,7 @@ fn expected_row(expected: &Bound<'_, PyAny>) -> PyResult { actual_row_count: native_field(expected, "actual_row_count")?.extract()?, expected_row_count: native_field(expected, "expected_row_count")?.extract()?, mismatched_row_count: native_field(expected, "mismatched_row_count")?.extract()?, + missing_row_count: native_field(expected, "missing_row_count")?.extract()?, error: coded_error(expected)?, }) } @@ -263,9 +264,12 @@ fn expectation_sql( ], )? .extract()?; + let compared: Option> = + native_field(&expected, "compared_columns")?.extract()?; let sql: String = scenario_expected_comparison_sql( &format!("SELECT * FROM {actual_relation}"), &native_field(&expected, "expected_sql")?.extract::()?, + compared.as_deref(), &adapter .call_method0("render_set_difference_operator")? .extract::()?, diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md index e49deb085a..1dcdcb6a60 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md @@ -106,14 +106,22 @@ __assert__all_orders_have_payments AS ( | `__source__` | Fixture source input. Materializes as a scenario table. | | `__ref__` | Fixture model boundary. Prevents SQLBuild from building that model upstream. | | `__seed__` | Fixture seed data. | -| `__expected__` | Full expected output. Compared order-insensitively against the scenario-built model. | +| `__expected__` | Expected output. Compared order-insensitively against the scenario-built model on the listed columns. | | `__assert__` | Zero-row assertion. Passes if the query returns no rows; fails with the returned rows as diagnostics. | -An `__expected__` CTE must list every column the model outputs, and no others. When the -CTE's columns and the model's output columns are both known statically, a mismatch fails planning -with `S512`, naming the missing or extra columns, before the scenario runs. A comparison that fails -to run reports its error with an `ERROR` status and no row counts; in `--json` output its -`comparison_status` is `not_run` and its row counts are `null`. +As in unit tests, an `__expected__` CTE compares only the columns it lists, matched by +name, so column order does not matter and unlisted model columns are ignored. Row counts are always +compared, and rows are compared in both directions: `mismatched` counts model rows the expected +output lacks and `missing` counts expected rows the model did not produce. An expected CTE without +explicit column names, such as `SELECT *`, is compared on every column by position. + +When the CTE's columns and the model's output columns are both known statically, listing a column +the model does not output fails planning with `S512` before the scenario runs. Set +`expected_columns "all"` in the `SCENARIO()` header to also require every model output column to be +listed; a missing column then fails with `S512`, naming it. + +A comparison that fails to run reports its error with an `ERROR` status and no row counts; in +`--json` output its `comparison_status` is `not_run` and its row counts are `null`. For sources with two-part identity, use double underscores: `__source__raw__orders`. @@ -141,12 +149,13 @@ available only to the model are not included. See ### SCENARIO() header -The header holds metadata and an optional time limit: +The header holds metadata, an optional time limit and the expected-column mode: | Field | Description | |-------|-------------| | `description` | Required description of the behaviour the scenario proves ([Descriptions](models/configuration.md#descriptions)) | | `tags` | Optional list of string tags | +| `expected_columns` | Optional. `"listed"` (default) compares only the columns each `__expected__` CTE lists; `"all"` also requires every model output column to be listed | | `timeout` | Optional time limit for the scenario's warehouse run, such as `"10m"`, or `"disabled"`; overrides `[settings] scenario_timeout` and the target's `scenario_timeout` | At the limit SQLBuild cancels the scenario's running statement, still drops the relations the diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md index c78426471a..48dc842484 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md @@ -137,6 +137,7 @@ and `//` line comments on Snowflake. An `__expected__` CTE compares only the columns it lists, matched by name, so column order does not matter and unlisted model columns are ignored. Row counts are always compared. Listing a column the model does not output is a test error. +[Scenario](scenarios.md#scenario-format) expected output is compared the same way. SQLBuild reads test CTEs strictly and rejects other forms with a compile error that names the CTE: diff --git a/src/sqlbuild/_native.pyi b/src/sqlbuild/_native.pyi index 831b01efc3..a95aa3d592 100644 --- a/src/sqlbuild/_native.pyi +++ b/src/sqlbuild/_native.pyi @@ -5101,7 +5101,11 @@ def describe_missing_expected_columns( *, model_name: str, expected_columns: Sequence[str], available_columns: Iterable[str] ) -> str | None: ... def build_scenario_expected_comparison_sql( - *, actual_sql: str, expected_sql: str, set_difference_operator: str + *, + actual_sql: str, + expected_sql: str, + set_difference_operator: str, + compared_columns: tuple[str, ...] | None = None, ) -> str: ... def sql_test_resource_id( *, test_name: str, source_path: PurePath | None, block_index: int | None, case_name: str | None @@ -16113,11 +16117,13 @@ class ScenarioExpectedExpectationPlan(NativeRecord): model_name: str actual_destination: CompiledRelationLocation expected_sql: str + compared_columns: tuple[str, ...] | None def __init__( self, model_name: str, actual_destination: CompiledRelationLocation, expected_sql: str, + compared_columns: tuple[str, ...] | None = ..., ) -> None: ... class ScenarioAssertionExpectationPlan(NativeRecord): diff --git a/src/sqlbuild/executor/scenario/main/_expected_expectations.py b/src/sqlbuild/executor/scenario/main/_expected_expectations.py index 933d59cc38..873065cd4c 100644 --- a/src/sqlbuild/executor/scenario/main/_expected_expectations.py +++ b/src/sqlbuild/executor/scenario/main/_expected_expectations.py @@ -34,6 +34,7 @@ def execute_scenario_expected_expectation( actual_sql=f"SELECT * FROM {actual_relation}", expected_sql=expectation.expected_sql, set_difference_operator=adapter.render_set_difference_operator(), + compared_columns=expectation.compared_columns, ) try: cursor: Any = adapter.execute(connection=connection, sql=comparison_sql) @@ -70,9 +71,10 @@ def execute_scenario_expected_expectation( actual_count: int = int(row[0]) expected_count: int = int(row[1]) mismatched_count: int = int(row[2]) + missing_count: int = int(row[3]) status: ExecutionStatus = ( ExecutionStatus.SUCCESS - if mismatched_count == 0 and actual_count == expected_count + if mismatched_count == 0 and missing_count == 0 and actual_count == expected_count else ExecutionStatus.FAILED ) error_message = None @@ -80,7 +82,8 @@ def execute_scenario_expected_expectation( error_message = ( f"scenario '{scenario_name}' expected comparison for model " f"'{expectation.model_name}' failed: " - f"actual={actual_count} expected={expected_count} mismatched={mismatched_count}" + f"actual={actual_count} expected={expected_count} mismatched={mismatched_count} " + f"missing={missing_count}" ) return ScenarioExpectedExpectationExecutionResult( scenario_name=scenario_name, @@ -89,6 +92,7 @@ def execute_scenario_expected_expectation( actual_row_count=actual_count, expected_row_count=expected_count, mismatched_row_count=mismatched_count, + missing_row_count=missing_count, error_code=SCENARIO_EXEC_EXPECTED_FAILED if status == ExecutionStatus.FAILED else None, error_help=( "Compare the expected CTE with the retained scenario model relation. " diff --git a/src/sqlbuild/executor/scenario/models.py b/src/sqlbuild/executor/scenario/models.py index eb193696d4..75fdf52e24 100644 --- a/src/sqlbuild/executor/scenario/models.py +++ b/src/sqlbuild/executor/scenario/models.py @@ -318,6 +318,7 @@ class ScenarioExpectedExpectationExecutionResult: actual_row_count: int | None = None expected_row_count: int | None = None mismatched_row_count: int | None = None + missing_row_count: int | None = None error_code: str | None = None error_help: str | None = None error_message: str | None = None diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py index 25dc768087..2227ef521e 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py @@ -259,5 +259,16 @@ class ScenarioExpectedColumnsE2ETestCase: expected_sql: str expected_exit_code: int expected_stdout_fragments: tuple[str, ...] + header_options: str = "" unexpected_stdout_fragments: tuple[str, ...] = field(default_factory=tuple) expected_json_results: tuple[dict[str, object], ...] = field(default_factory=tuple) + + +@dataclass(frozen=True) +class ScenarioExpectedColumnsHeaderE2ETestCase: + """Test case for validating the SCENARIO() expected_columns option.""" + + description: str + header_options: str + expected_exit_code: int + expected_stderr_fragment: str diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/helpers.py b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/helpers.py index 5f34add023..7b789f8f04 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/helpers.py @@ -900,21 +900,22 @@ def run_reusable_compile(*, project_dir: Path) -> subprocess.CompletedProcess[st ), } _EXPECTED_COLUMNS_SCENARIO_PATH: str = "tests/scenarios/order_flags__expected.sql" -_EXPECTED_COLUMNS_SCENARIO_HEADER: str = ( - 'SCENARIO (description "Order flags keep their status");\n\n' -) +_EXPECTED_COLUMNS_DESCRIPTION: str = 'description "Order flags keep their status"' _EXPECTED_COLUMNS_FIXTURE: str = ( "WITH __ref__orders AS (\n SELECT 'o1' AS order_id, 'placed' AS status\n),\n\n" ) -def build_expected_columns_scenario_project_files(*, expected_sql: str) -> dict[str, str]: +def build_expected_columns_scenario_project_files( + *, expected_sql: str, header_options: str +) -> dict[str, str]: """Build an order-flags project whose scenario expects `expected_sql` for the model.""" return { **_EXPECTED_COLUMNS_PROJECT_FILES, _EXPECTED_COLUMNS_SCENARIO_PATH: ( - f"{_EXPECTED_COLUMNS_SCENARIO_HEADER}{_EXPECTED_COLUMNS_FIXTURE}" + f"SCENARIO ({_EXPECTED_COLUMNS_DESCRIPTION}{header_options});\n\n" + f"{_EXPECTED_COLUMNS_FIXTURE}" f"__expected__order_flags AS (\n {expected_sql}\n)\n" ), } diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_expected_columns.py b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_expected_columns.py index 5380e2feb3..7c00c50915 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_expected_columns.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_expected_columns.py @@ -11,6 +11,7 @@ from tests.e2e.src.sqlbuild.cli.commands.main.scenario._test_types import ( ScenarioExpectedColumnsE2ETestCase, + ScenarioExpectedColumnsHeaderE2ETestCase, ) from tests.e2e.src.sqlbuild.cli.commands.main.scenario.helpers import ( build_expected_columns_scenario_project_files, @@ -35,12 +36,74 @@ "actual_row_count": 1, "expected_row_count": 1, "mismatched_row_count": 0, + "missing_row_count": 0, }, ), ), ScenarioExpectedColumnsE2ETestCase( - description="expected output missing a model column is rejected before execution", + description="listed columns are compared by name and unlisted columns ignored", + expected_sql="SELECT 'placed' AS status, 'o1' AS order_id", + expected_exit_code=0, + expected_stdout_fragments=("PASS=1 FAIL=0 TOTAL=1",), + unexpected_stdout_fragments=("X504", "S512"), + expected_json_results=( + { + "status": "success", + "comparison_status": "completed", + "actual_row_count": 1, + "expected_row_count": 1, + "mismatched_row_count": 0, + "missing_row_count": 0, + }, + ), + ), + ScenarioExpectedColumnsE2ETestCase( + description="a wrong value in a listed column fails with both difference counts", + expected_sql="SELECT 'o1' AS order_id, 'shipped' AS status", + expected_exit_code=1, + expected_stdout_fragments=( + "expected order_flags FAIL 1 mismatched, 1 missing", + "error[X506]", + "expected order_flags: actual=1 expected=1 mismatched=1 missing=1", + ), + unexpected_stdout_fragments=("X504",), + expected_json_results=( + { + "status": "failed", + "comparison_status": "completed", + "actual_row_count": 1, + "expected_row_count": 1, + "mismatched_row_count": 1, + "missing_row_count": 1, + "error_code": "X506", + }, + ), + ), + ScenarioExpectedColumnsE2ETestCase( + description="an extra expected row fails on row counts", + expected_sql=( + "SELECT 'o1' AS order_id, 'placed' AS status " + "UNION ALL SELECT 'o2' AS order_id, 'placed' AS status" + ), + expected_exit_code=1, + expected_stdout_fragments=( + "expected order_flags: actual=1 expected=2 mismatched=0 missing=1", + ), + expected_json_results=( + { + "status": "failed", + "comparison_status": "completed", + "actual_row_count": 1, + "expected_row_count": 2, + "mismatched_row_count": 0, + "missing_row_count": 1, + }, + ), + ), + ScenarioExpectedColumnsE2ETestCase( + description="strict expected columns reject an unlisted column before execution", expected_sql="SELECT 'o1' AS order_id, 'placed' AS status", + header_options=', expected_columns "all"', expected_exit_code=1, expected_stdout_fragments=( "error[S512]", @@ -49,6 +112,21 @@ ), unexpected_stdout_fragments=("X504", "mismatched", "actual="), ), + ScenarioExpectedColumnsE2ETestCase( + description="strict expected columns accept every column in any order", + expected_sql="SELECT FALSE AS is_cancelled, 'placed' AS status, 'o1' AS order_id", + header_options=', expected_columns "all"', + expected_exit_code=0, + expected_stdout_fragments=("PASS=1 FAIL=0 TOTAL=1",), + expected_json_results=( + { + "status": "success", + "comparison_status": "completed", + "mismatched_row_count": 0, + "missing_row_count": 0, + }, + ), + ), ScenarioExpectedColumnsE2ETestCase( description="expected output listing a column the model lacks is rejected", expected_sql=( @@ -79,6 +157,7 @@ "actual_row_count": None, "expected_row_count": None, "mismatched_row_count": None, + "missing_row_count": None, "error_code": "X504", }, ), @@ -93,7 +172,7 @@ def test_given_scenario_expected_columns_when_testing_then_comparison_is_reporte tmp_path=tmp_path, project_name="scenario_expected_columns", repo_files=build_expected_columns_scenario_project_files( - expected_sql=test_case.expected_sql + expected_sql=test_case.expected_sql, header_options=test_case.header_options ), ) @@ -117,3 +196,35 @@ def test_given_scenario_expected_columns_when_testing_then_comparison_is_reporte tuple(map(selected_fields, results, test_case.expected_json_results)) == test_case.expected_json_results ) + + +@pytest.mark.parametrize( + "test_case", + ( + ScenarioExpectedColumnsHeaderE2ETestCase( + description="an unknown expected_columns value is rejected at discovery", + header_options=', expected_columns "some"', + expected_exit_code=1, + expected_stderr_fragment="expected_columns in", + ), + ), + ids=lambda case: case.description, +) +def test_given_invalid_expected_columns_option_when_testing_then_discovery_rejects_it( + test_case: ScenarioExpectedColumnsHeaderE2ETestCase, tmp_path: Path +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="scenario_expected_columns", + repo_files=build_expected_columns_scenario_project_files( + expected_sql="SELECT 'o1' AS order_id", header_options=test_case.header_options + ), + ) + + result: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", "scenario", "test"), project_dir=project_dir + ) + + assert result.returncode == test_case.expected_exit_code, result.stdout + result.stderr + assert test_case.expected_stderr_fragment in result.stderr, result.stderr + assert 'must be "listed" or "all"' in result.stderr, result.stderr diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_scenario.py b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_scenario.py index 5b87573170..fd7d13169f 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_scenario.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_scenario.py @@ -1954,7 +1954,7 @@ def test_given_waffle_shop_fixture_when_running_with_retain_then_keeps_scenario_ expected_artifact_fragments=( "WITH __actual AS", "__expected AS", - "mismatched_count", + "unexpected_count", ), ), ScenarioRuntimeArtifactTestCase( @@ -2034,7 +2034,7 @@ def test_given_scenario_project_when_running_scenario_test_then_writes_runtime_a expected_artifact_fragments=( "WITH __actual AS", "__sqb_local__model__order_totals", - "mismatched_count", + "unexpected_count", ), ), ScenarioLocalRuntimeArtifactTestCase( diff --git a/tests/unit/src/sqlbuild/executor/scenario/main/_test_types.py b/tests/unit/src/sqlbuild/executor/scenario/main/_test_types.py index cadbde17aa..5e2b17a923 100644 --- a/tests/unit/src/sqlbuild/executor/scenario/main/_test_types.py +++ b/tests/unit/src/sqlbuild/executor/scenario/main/_test_types.py @@ -68,6 +68,7 @@ class ScenarioExpectedComparisonSqlTestCase: description: str set_difference_operator: str expected_fragments: tuple[str, ...] + compared_columns: tuple[str, ...] | None = None @dataclass(frozen=True) diff --git a/tests/unit/src/sqlbuild/executor/scenario/main/test_expected_comparison_sql.py b/tests/unit/src/sqlbuild/executor/scenario/main/test_expected_comparison_sql.py index 62200c7067..ad7a7adc2e 100644 --- a/tests/unit/src/sqlbuild/executor/scenario/main/test_expected_comparison_sql.py +++ b/tests/unit/src/sqlbuild/executor/scenario/main/test_expected_comparison_sql.py @@ -18,7 +18,18 @@ "SELECT * FROM __actual EXCEPT SELECT * FROM __expected", ") AS __sqlbuild_mismatch", ), - ) + ), + ScenarioExpectedComparisonSqlTestCase( + description="compares listed columns by name in both directions", + set_difference_operator="EXCEPT", + compared_columns=("status", "order_id"), + expected_fragments=( + "SELECT status, order_id FROM __actual EXCEPT SELECT status, order_id FROM " + "__expected) AS __sqlbuild_mismatch) AS unexpected_count", + "SELECT status, order_id FROM __expected EXCEPT SELECT status, order_id FROM " + "__actual) AS __sqlbuild_missing) AS missing_count", + ), + ), ], ids=lambda case: case.description, ) @@ -29,6 +40,7 @@ def test_given_expected_output_when_building_comparison_then_aliases_set_differe actual_sql="SELECT 1 AS order_id", expected_sql="SELECT 1 AS order_id", set_difference_operator=test_case.set_difference_operator, + compared_columns=test_case.compared_columns, ) for expected_fragment in test_case.expected_fragments: diff --git a/website/src/content/docs/docs/concepts/scenarios.mdx b/website/src/content/docs/docs/concepts/scenarios.mdx index 78fb6c6517..0b1b18d46f 100644 --- a/website/src/content/docs/docs/concepts/scenarios.mdx +++ b/website/src/content/docs/docs/concepts/scenarios.mdx @@ -94,14 +94,22 @@ __assert__all_orders_have_payments AS ( | `__source__` | Fixture source input. Materializes as a scenario table. | | `__ref__` | Fixture model boundary. Prevents SQLBuild from building that model upstream. | | `__seed__` | Fixture seed data. | -| `__expected__` | Full expected output. Compared order-insensitively against the scenario-built model. | +| `__expected__` | Expected output. Compared order-insensitively against the scenario-built model on the listed columns. | | `__assert__` | Zero-row assertion. Passes if the query returns no rows; fails with the returned rows as diagnostics. | -An `__expected__` CTE must list every column the model outputs, and no others. When the -CTE's columns and the model's output columns are both known statically, a mismatch fails planning -with `S512`, naming the missing or extra columns, before the scenario runs. A comparison that fails -to run reports its error with an `ERROR` status and no row counts; in `--json` output its -`comparison_status` is `not_run` and its row counts are `null`. +As in unit tests, an `__expected__` CTE compares only the columns it lists, matched by +name, so column order does not matter and unlisted model columns are ignored. Row counts are always +compared, and rows are compared in both directions: `mismatched` counts model rows the expected +output lacks and `missing` counts expected rows the model did not produce. An expected CTE without +explicit column names, such as `SELECT *`, is compared on every column by position. + +When the CTE's columns and the model's output columns are both known statically, listing a column +the model does not output fails planning with `S512` before the scenario runs. Set +`expected_columns "all"` in the `SCENARIO()` header to also require every model output column to be +listed; a missing column then fails with `S512`, naming it. + +A comparison that fails to run reports its error with an `ERROR` status and no row counts; in +`--json` output its `comparison_status` is `not_run` and its row counts are `null`. For sources with two-part identity, use double underscores: `__source__raw__orders`. @@ -129,12 +137,13 @@ available only to the model are not included. See ### SCENARIO() header -The header holds metadata and an optional time limit: +The header holds metadata, an optional time limit and the expected-column mode: | Field | Description | |-------|-------------| | `description` | Required description of the behaviour the scenario proves ([Descriptions](/docs/concepts/models/configuration#descriptions)) | | `tags` | Optional list of string tags | +| `expected_columns` | Optional. `"listed"` (default) compares only the columns each `__expected__` CTE lists; `"all"` also requires every model output column to be listed | | `timeout` | Optional time limit for the scenario's warehouse run, such as `"10m"`, or `"disabled"`; overrides `[settings] scenario_timeout` and the target's `scenario_timeout` | At the limit SQLBuild cancels the scenario's running statement, still drops the relations the diff --git a/website/src/content/docs/docs/concepts/testing.mdx b/website/src/content/docs/docs/concepts/testing.mdx index 0040e51570..53bac26675 100644 --- a/website/src/content/docs/docs/concepts/testing.mdx +++ b/website/src/content/docs/docs/concepts/testing.mdx @@ -112,6 +112,7 @@ and `//` line comments on Snowflake. An `__expected__` CTE compares only the columns it lists, matched by name, so column order does not matter and unlisted model columns are ignored. Row counts are always compared. Listing a column the model does not output is a test error. +[Scenario](/docs/concepts/scenarios#scenario-format) expected output is compared the same way. SQLBuild reads test CTEs strictly and rejects other forms with a compile error that names the CTE: From c8e29a2b102aa0dad277972c4bb5deb2d1f55ec1 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 21:48:58 +0100 Subject: [PATCH 08/12] fix: let scenario checks read helper CTEs and report unrun assertions Scenario assertions and expected output ran without the file's helper CTEs, so reading one failed in the warehouse with a missing-table error, and the summary still reported 0 failing rows. Place the helpers a query reads, with the helpers and fixtures they read, ahead of it in dependency order through the SQL-test helper scope, build models an assertion reads through a helper, and give fixtures only the helpers they read. An assertion that did not run reports ERROR without a count; --json adds query_status and a null count. Fixes CHI-634. --- Makefile | 3 +- .../_helpers/sql_tests/helper_scope.rs | 18 +- .../src/compiler/_helpers/sql_tests/mod.rs | 1 + .../compiler/_helpers/sql_tests/planning.rs | 4 +- .../_helpers/sql_tests/scenario_scope.rs | 85 +++++++++ .../src/compiler/main/mod.rs | 3 + .../compiler/main/scenario_helper_scope.rs | 12 ++ .../src/compiler/main/scenario_reader_ctes.rs | 13 ++ .../src/compiler/main/scenario_reader_sql.rs | 11 ++ .../sqlbuild-analysis/src/compiler/models.rs | 17 ++ .../sqlbuild-analysis/src/compiler/tests.rs | 2 + .../tests/test_scenario_helper_ctes.rs | 61 ++++++ .../src/compiler/tests/test_types.rs | 7 + .../scenario_command/_helpers/expectations.rs | 29 ++- .../src/scenario_command/models.rs | 3 +- .../src/scenario_command/tests/helpers.rs | 4 +- .../tests/test_scenario_rows.rs | 10 +- .../compiled_project/scenario_loop.rs | 28 ++- .../_helpers/planner_scenario/execution.rs | 12 +- .../_helpers/planner_scenario/fixtures.rs | 31 +--- .../_helpers/planner_scenario/helper_scope.rs | 64 +++++++ .../bindings/_helpers/planner_scenario/mod.rs | 1 + .../bindings/_helpers/result_output/checks.rs | 32 +++- .../references/docs/concepts/scenarios.md | 11 +- src/sqlbuild/executor/scenario/models.py | 4 +- .../cli/commands/main/scenario/_test_types.py | 14 ++ .../cli/commands/main/scenario/helpers.py | 12 ++ .../main/scenario/test_helper_ctes.py | 175 ++++++++++++++++++ .../content/docs/docs/concepts/scenarios.mdx | 11 +- 29 files changed, 607 insertions(+), 71 deletions(-) create mode 100644 crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/scenario_scope.rs create mode 100644 crates/sqlbuild-analysis/src/compiler/main/scenario_helper_scope.rs create mode 100644 crates/sqlbuild-analysis/src/compiler/main/scenario_reader_ctes.rs create mode 100644 crates/sqlbuild-analysis/src/compiler/main/scenario_reader_sql.rs create mode 100644 crates/sqlbuild-analysis/src/compiler/tests/test_scenario_helper_ctes.rs create mode 100644 crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/helper_scope.rs create mode 100644 tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_helper_ctes.py diff --git a/Makefile b/Makefile index b71adde5be..61dc44ffe3 100644 --- a/Makefile +++ b/Makefile @@ -140,7 +140,8 @@ E2E_DUCKDB_CLI_DATA_PATHS := \ tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_concurrency.py \ tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_interrupts.py \ tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_empty_fixture.py \ - tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_expected_columns.py + tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_expected_columns.py \ + tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_helper_ctes.py E2E_DUCKDB_CLI_PATHS := \ tests/e2e/scripts/cli_preview \ diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/helper_scope.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/helper_scope.rs index eb628449de..6f042f9d3a 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/helper_scope.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/helper_scope.rs @@ -33,6 +33,7 @@ pub(crate) struct HelperScope { } /// One mock CTE other test CTEs can read by its generated name. +#[derive(Debug)] pub(crate) struct ScopeMock { pub(crate) generated_name: String, pub(crate) mock_name: String, @@ -41,6 +42,7 @@ pub(crate) struct ScopeMock { } /// One test-defined helper: its authored name and body, and its top-level CTE name and body. +#[derive(Debug)] struct ScopeHelper { name: String, original: String, @@ -51,6 +53,7 @@ struct ScopeHelper { } /// A helper whose relation references were resolved like an assertion's, after the model chain. +#[derive(Debug)] pub(crate) struct ResolvedHelper { /// The helper body with every reference replaced by the CTE that stands in for it. pub(crate) sql: String, @@ -67,7 +70,7 @@ pub(crate) struct ScopeDependency<'a> { } /// Where helper CTEs live in the rendered test query. -#[derive(Clone, Copy, Default, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] enum HelperPlacement { /// Each top-level test CTE carries the helpers it reads in its own WITH. #[default] @@ -95,7 +98,7 @@ enum ScopeNode { } /// Name references between one test's helper and mock CTEs, tokenized once per test. -#[derive(Default)] +#[derive(Debug, Default)] pub(crate) struct ScopeGraph { placement: HelperPlacement, helpers: Vec, @@ -386,6 +389,17 @@ impl ScopeGraph { self.collect_scope(self.closure(&identifier_tokens(sql, patterns), true)) } + /// Top-level CTEs `sql` reads, transitively and dependencies first; mocks only when asked. + pub(crate) fn reader_ctes( + &self, + sql: &str, + patterns: &SqlTestPatterns, + through_mocks: bool, + ) -> Vec<(String, String)> { + self.collect_scope(self.closure(&identifier_tokens(sql, patterns), through_mocks)) + .ctes + } + fn collect_scope(&self, nodes: Vec) -> HelperScope { let mut scope = HelperScope { ctes: Vec::new(), diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/mod.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/mod.rs index 0513121193..745b163bc2 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/mod.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/mod.rs @@ -21,4 +21,5 @@ pub mod reference_calls; pub mod relation_markers; pub mod rendering; pub mod scenario_extraction; +pub(crate) mod scenario_scope; pub(crate) mod simple_template; diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs index 0f2f4dfe4f..b0c1b964b8 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs @@ -102,7 +102,7 @@ struct ProjectContext { reserved_ctes: Arc>>>, } -#[derive(Clone)] +#[derive(Clone, Debug)] pub(crate) struct SqlTestPatterns { pub(crate) reference: Regex, pub(crate) source: Regex, @@ -116,7 +116,7 @@ pub(crate) struct SqlTestPatterns { } impl SqlTestPatterns { - fn new(lexical: LexicalSyntax) -> Result { + pub(crate) fn new(lexical: LexicalSyntax) -> Result { Ok(Self { reference: compile_pattern(r#"(?i)__ref\(\"([^\"]+)\"\)"#)?, source: compile_pattern(r#"(?i)__source\(\"([^\"]+)\"\)"#)?, diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/scenario_scope.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/scenario_scope.rs new file mode 100644 index 0000000000..76fe078c14 --- /dev/null +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/scenario_scope.rs @@ -0,0 +1,85 @@ +//! Scenario helper and fixture CTEs indexed through the SQL-test helper scope graph. + +use std::collections::BTreeMap; + +use crate::compiler::_helpers::sql_tests::cte_slices::SliceDialect; +use crate::compiler::_helpers::sql_tests::helper_scope::{ScopeGraph, ScopeRequest}; +use crate::compiler::_helpers::sql_tests::planning::{ + DBT_REF_PREFIX, REF_PREFIX, SEED_PREFIX, SOURCE_PREFIX, SqlTestPatterns, TestFixtures, +}; +use crate::compiler::models::{ScenarioHelperScope, ScenarioScopeInput, SqlTestCte}; + +const ERROR_PREFIX: &str = "compile_input:"; +const FIXTURE_PREFIXES: [&str; 4] = [REF_PREFIX, SOURCE_PREFIX, SEED_PREFIX, DBT_REF_PREFIX]; + +/// Index every helper, including ones that call references, and every fixture as a mock. +pub(crate) fn helper_scope(input: &ScenarioScopeInput<'_>) -> Result { + let patterns = SqlTestPatterns::new(input.lexical.clone()).map_err(plain)?; + let fixtures = scenario_fixtures(input.ctes); + let read_helper_names: Vec = fixtures + .helpers + .iter() + .map(|cte| cte.name.clone()) + .collect(); + let graph = ScopeGraph::new(&ScopeRequest { + fixtures: &fixtures, + patterns: &patterns, + file_label: input.file_label, + rename_dialect: None, + flat_dialect: true, + slice_dialect: SliceDialect::new(input.dialect), + read_helper_names: &read_helper_names, + }) + .map_err(plain)?; + Ok(ScenarioHelperScope { graph, patterns }) +} + +fn scenario_fixtures(ctes: &[(String, String)]) -> TestFixtures { + let mut fixtures = TestFixtures { + mock_refs: BTreeMap::new(), + mock_sources: BTreeMap::new(), + mock_seeds: BTreeMap::new(), + mock_dbt_refs: BTreeMap::new(), + mock_table_functions: BTreeMap::new(), + helpers: Vec::new(), + scope: ScopeGraph::default(), + expected: BTreeMap::new(), + assertions: Vec::new(), + }; + for (name, sql) in ctes { + let fixture: Option<(&str, &str)> = fixture_key(name); + let group = match fixture { + Some((REF_PREFIX, _)) => &mut fixtures.mock_refs, + Some((SOURCE_PREFIX, _)) => &mut fixtures.mock_sources, + Some((SEED_PREFIX, _)) => &mut fixtures.mock_seeds, + Some(_) => &mut fixtures.mock_dbt_refs, + None => { + fixtures.helpers.push(SqlTestCte { + name: name.clone(), + sql_body: sql.clone(), + }); + continue; + } + }; + if let Some((_, key)) = fixture { + group.insert(key.to_owned(), sql.clone()); + } + } + fixtures +} + +/// The fixture prefix and name of a fixture CTE, or `None` for a helper. +fn fixture_key(name: &str) -> Option<(&'static str, &str)> { + for prefix in FIXTURE_PREFIXES { + if let Some(key) = name.strip_prefix(prefix) { + return Some((prefix, key)); + } + } + None +} + +fn plain(message: String) -> String { + message + .strip_prefix(ERROR_PREFIX) + .map_or_else(|| message.clone(), str::to_owned) +} diff --git a/crates/sqlbuild-analysis/src/compiler/main/mod.rs b/crates/sqlbuild-analysis/src/compiler/main/mod.rs index c8aa850963..fc2ae2ccfb 100644 --- a/crates/sqlbuild-analysis/src/compiler/main/mod.rs +++ b/crates/sqlbuild-analysis/src/compiler/main/mod.rs @@ -3,6 +3,9 @@ pub mod missing_expected_columns; pub mod scenario_expected_column_message; pub mod scenario_expected_columns; pub mod scenario_expected_comparison_sql; +pub mod scenario_helper_scope; +pub mod scenario_reader_ctes; +pub mod scenario_reader_sql; pub mod sql_scenario_classification; pub mod sql_test_assembly; pub mod sql_test_chains; diff --git a/crates/sqlbuild-analysis/src/compiler/main/scenario_helper_scope.rs b/crates/sqlbuild-analysis/src/compiler/main/scenario_helper_scope.rs new file mode 100644 index 0000000000..1785da2b42 --- /dev/null +++ b/crates/sqlbuild-analysis/src/compiler/main/scenario_helper_scope.rs @@ -0,0 +1,12 @@ +//! Index a scenario file's helper and fixture CTEs for the queries that read them. + +use crate::compiler::_helpers::sql_tests::scenario_scope::helper_scope; +use crate::compiler::models::ScenarioHelperScope; +use crate::compiler::models::ScenarioScopeInput; + +/// The scenario's helper scope, or why a CTE cannot be placed, such as a shadowed helper. +pub fn scenario_helper_scope( + input: &ScenarioScopeInput<'_>, +) -> Result { + helper_scope(input) +} diff --git a/crates/sqlbuild-analysis/src/compiler/main/scenario_reader_ctes.rs b/crates/sqlbuild-analysis/src/compiler/main/scenario_reader_ctes.rs new file mode 100644 index 0000000000..75a2df9f8d --- /dev/null +++ b/crates/sqlbuild-analysis/src/compiler/main/scenario_reader_ctes.rs @@ -0,0 +1,13 @@ +//! The helper and fixture CTEs one scenario query reads. + +use crate::compiler::models::ScenarioHelperScope; + +/// The CTEs `sql` reads, transitively and dependencies first; fixtures only with `fixtures`. +#[must_use] +pub fn scenario_reader_ctes( + scope: &ScenarioHelperScope, + sql: &str, + fixtures: bool, +) -> Vec<(String, String)> { + scope.graph.reader_ctes(sql, &scope.patterns, fixtures) +} diff --git a/crates/sqlbuild-analysis/src/compiler/main/scenario_reader_sql.rs b/crates/sqlbuild-analysis/src/compiler/main/scenario_reader_sql.rs new file mode 100644 index 0000000000..adff7cb216 --- /dev/null +++ b/crates/sqlbuild-analysis/src/compiler/main/scenario_reader_sql.rs @@ -0,0 +1,11 @@ +//! One scenario query preceded by the helper and fixture CTEs it reads. + +use crate::compiler::_helpers::sql_tests::cte_sql::with_leading_ctes; +use crate::compiler::main::scenario_reader_ctes::scenario_reader_ctes; +use crate::compiler::models::ScenarioHelperScope; + +/// `sql` with the CTEs it reads ahead of it, merged into its own leading WITH. +#[must_use] +pub fn scenario_reader_sql(scope: &ScenarioHelperScope, sql: &str, fixtures: bool) -> String { + with_leading_ctes(&scenario_reader_ctes(scope, sql, fixtures), sql) +} diff --git a/crates/sqlbuild-analysis/src/compiler/models.rs b/crates/sqlbuild-analysis/src/compiler/models.rs index bee2c67c7f..87af4d5946 100644 --- a/crates/sqlbuild-analysis/src/compiler/models.rs +++ b/crates/sqlbuild-analysis/src/compiler/models.rs @@ -406,3 +406,20 @@ pub struct ScenarioExpectedColumns { /// Listed columns the model does not output. pub unknown: Vec, } + +/// The CTEs a scenario's checks may read: helpers as authored, fixtures by their stand-in bodies. +#[derive(Debug)] +pub struct ScenarioScopeInput<'a> { + /// Every non-check CTE by authored name; a fixture body is what a check reading it sees. + pub ctes: &'a [(String, String)], + pub lexical: LexicalSyntax, + pub dialect: Option<&'a str>, + pub file_label: &'a str, +} + +/// One scenario file's helper and fixture CTEs, indexed once for its fixtures and checks. +#[derive(Debug)] +pub struct ScenarioHelperScope { + pub(crate) graph: crate::compiler::_helpers::sql_tests::helper_scope::ScopeGraph, + pub(crate) patterns: crate::compiler::_helpers::sql_tests::planning::SqlTestPatterns, +} diff --git a/crates/sqlbuild-analysis/src/compiler/tests.rs b/crates/sqlbuild-analysis/src/compiler/tests.rs index 6f96fc48b5..2ba8066077 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests.rs @@ -2,6 +2,8 @@ mod helpers; #[path = "tests/test_scenario_expected_columns.rs"] mod scenario_expected_columns; +#[path = "tests/test_scenario_helper_ctes.rs"] +mod scenario_helper_ctes; #[path = "tests/simple_template/helpers.rs"] mod simple_template_helpers; #[path = "tests/single_read/helpers.rs"] diff --git a/crates/sqlbuild-analysis/src/compiler/tests/test_scenario_helper_ctes.rs b/crates/sqlbuild-analysis/src/compiler/tests/test_scenario_helper_ctes.rs new file mode 100644 index 0000000000..45becff149 --- /dev/null +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_scenario_helper_ctes.rs @@ -0,0 +1,61 @@ +use sqlbuild_sqltext::sql_scan::models::LexicalSyntax; + +use crate::compiler::main::scenario_helper_scope::scenario_helper_scope; +use crate::compiler::main::scenario_reader_sql::scenario_reader_sql; +use crate::compiler::models::ScenarioScopeInput; +use crate::compiler::tests::test_types::ScenarioHelperScopeTestCase; + +#[test] +fn given_scenario_helpers_when_scoping_a_reader_then_prepends_what_it_reads_in_order() { + let ctes = [ + ("__ref__orders", "SELECT * FROM main.fixture_orders"), + ("placed", "SELECT order_id FROM __ref__orders"), + ("unused", "SELECT * FROM __ref(\"order_flags\")"), + ("flagged", "SELECT order_id FROM placed"), + ] + .map(|(name, sql)| (name.to_owned(), sql.to_owned())); + let scope = scenario_helper_scope(&ScenarioScopeInput { + ctes: &ctes, + lexical: LexicalSyntax::default(), + dialect: Some("duckdb"), + file_label: "tests/scenarios/orders.sql", + }) + .expect("scope"); + let test_cases = [ + ScenarioHelperScopeTestCase { + description: "chained helpers and the fixture they read come first", + reader_sql: "SELECT * FROM flagged", + fixtures: true, + expected_sql: "WITH __ref__orders AS (SELECT * FROM main.fixture_orders), placed AS \ + (SELECT order_id FROM __ref__orders), flagged AS (SELECT order_id \ + FROM placed) SELECT * FROM flagged", + }, + ScenarioHelperScopeTestCase { + description: "a fixture reader gets helpers but not fixture stand-ins", + reader_sql: "SELECT * FROM placed", + fixtures: false, + expected_sql: "WITH placed AS (SELECT order_id FROM __ref__orders) SELECT * FROM placed", + }, + ScenarioHelperScopeTestCase { + description: "a reader with its own WITH keeps it after the helpers", + reader_sql: "WITH local AS (SELECT * FROM placed) SELECT * FROM local", + fixtures: false, + expected_sql: "WITH placed AS (SELECT order_id FROM __ref__orders), local AS \ + (SELECT * FROM placed) SELECT * FROM local", + }, + ScenarioHelperScopeTestCase { + description: "a reader without helper reads is unchanged", + reader_sql: "SELECT 1", + fixtures: true, + expected_sql: "SELECT 1", + }, + ]; + for test_case in test_cases { + assert_eq!( + scenario_reader_sql(&scope, test_case.reader_sql, test_case.fixtures), + test_case.expected_sql, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs b/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs index 559f774889..1cc31d2b0f 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs @@ -213,3 +213,10 @@ pub(crate) struct ScenarioExpectedColumnsTestCase { pub(crate) all_columns: bool, pub(crate) expected_message: Option<&'static str>, } + +pub(crate) struct ScenarioHelperScopeTestCase { + pub(crate) description: &'static str, + pub(crate) reader_sql: &'static str, + pub(crate) fixtures: bool, + pub(crate) expected_sql: &'static str, +} diff --git a/crates/sqlbuild-core/src/scenario_command/_helpers/expectations.rs b/crates/sqlbuild-core/src/scenario_command/_helpers/expectations.rs index 5c7413aeec..dca6bce4ef 100644 --- a/crates/sqlbuild-core/src/scenario_command/_helpers/expectations.rs +++ b/crates/sqlbuild-core/src/scenario_command/_helpers/expectations.rs @@ -52,25 +52,15 @@ pub(crate) fn expectation_rows(row: &ScenarioResultRow, style: CliStyle) -> Stri text.push_str(&expectation_error(expected.error.as_ref(), style)); } for assertion in &row.assertions { - let detail: String = if assertion.failed { - format!( - " {} {}", - assertion.failing_row_count, - rows_label(assertion.failing_row_count) - ) - } else { - String::new() + let (status, detail): (&str, String) = match (assertion.failed, assertion.failing_row_count) + { + (false, _) => (PASS_STATUS, String::new()), + (true, Some(count)) => (FAIL_STATUS, format!(" {count} {}", rows_label(count))), + (true, None) => (ERROR_STATUS, String::new()), }; text.push_str(&expectation_line( &format!("assertion {}", assertion.name), - ( - if assertion.failed { - FAIL_STATUS - } else { - PASS_STATUS - }, - detail, - ), + (status, detail), style, )); text.push_str(&expectation_error(assertion.error.as_ref(), style)); @@ -97,9 +87,12 @@ pub(crate) fn expectation_failures(row: &ScenarioResultRow) -> String { )); } for assertion in row.assertions.iter().filter(|assertion| assertion.failed) { + let Some(failing) = assertion.failing_row_count else { + continue; + }; text.push_str(&format!( - " assertion {}: failing_rows={}\n", - assertion.name, assertion.failing_row_count + " assertion {}: failing_rows={failing}\n", + assertion.name )); if let Some(sample) = &assertion.first_sample { text.push_str(&format!(" sample: {sample}\n")); diff --git a/crates/sqlbuild-core/src/scenario_command/models.rs b/crates/sqlbuild-core/src/scenario_command/models.rs index 6abc7ff468..c2a870d8b7 100644 --- a/crates/sqlbuild-core/src/scenario_command/models.rs +++ b/crates/sqlbuild-core/src/scenario_command/models.rs @@ -59,7 +59,8 @@ pub struct ExpectedRow { pub struct AssertionRow { pub name: String, pub failed: bool, - pub failing_row_count: i64, + /// `None` when the assertion query did not run. + pub failing_row_count: Option, /// `str()` of the first failing sample row, when there is one. pub first_sample: Option, pub error: Option, diff --git a/crates/sqlbuild-core/src/scenario_command/tests/helpers.rs b/crates/sqlbuild-core/src/scenario_command/tests/helpers.rs index 6f1702d9ba..29cae6471c 100644 --- a/crates/sqlbuild-core/src/scenario_command/tests/helpers.rs +++ b/crates/sqlbuild-core/src/scenario_command/tests/helpers.rs @@ -93,14 +93,14 @@ pub(super) fn failing_result_row() -> ScenarioResultRow { AssertionRow { name: "no_negative".to_owned(), failed: true, - failing_row_count: 1, + failing_row_count: Some(1), first_sample: Some("(1, 'a', None)".to_owned()), error: None, }, AssertionRow { name: "few".to_owned(), failed: true, - failing_row_count: 3, + failing_row_count: Some(3), first_sample: None, error: coded(None, "plain error", None), }, diff --git a/crates/sqlbuild-core/src/scenario_command/tests/test_scenario_rows.rs b/crates/sqlbuild-core/src/scenario_command/tests/test_scenario_rows.rs index 0856ca550a..6391b897d8 100644 --- a/crates/sqlbuild-core/src/scenario_command/tests/test_scenario_rows.rs +++ b/crates/sqlbuild-core/src/scenario_command/tests/test_scenario_rows.rs @@ -7,7 +7,7 @@ use crate::scenario_command::main::render_capture_result::render_capture_result; use crate::scenario_command::main::render_scenario_result::render_scenario_result; use crate::scenario_command::main::render_snapshot_sync_result::render_snapshot_sync_result; use crate::scenario_command::models::{ - CaptureRow, ExpectedRow, FixtureStatements, FunctionStatements, LoadedRelation, + AssertionRow, CaptureRow, ExpectedRow, FixtureStatements, FunctionStatements, LoadedRelation, LocalRuntimeInputs, ModelArtifactEntry, NamedStatements, RetainedInputs, ScenarioResultRow, WarehouseRuntimeInputs, }; @@ -24,7 +24,7 @@ const RA1: &str = "orders_paid const RB0: &str = "orders_local ERROR\n snapshot missing\n Retained local DuckDB: /work/target/run/scenarios/orders_local/local.duckdb\n"; const RC0: &str = "orders_warehouse FAIL\n boom\n Rerun with --retain to inspect scenario-owned artifacts.\n"; const RD1: &str = "orders_ok \u{1b}[32mPASS\u{1b}[0m 0.01s\n expect expected m \u{1b}[32mPASS\u{1b}[0m\n"; -const RE0: &str = "orders_errored FAIL\n expect expected fact_orders ERROR\n error[X504]: comparison failed to run\n"; +const RE0: &str = "orders_errored FAIL\n expect expected fact_orders ERROR\n error[X504]: comparison failed to run\n expect assertion no_negative ERROR\n error[X507]: assertion failed to run\n"; const CE0: &str = "orders_paid PASS 2 relations, 3 rows\n source raw_orders PASS 2 rows, 41 B\n seed countries PASS 1 row, 1.5 KB\n snapshot tests/_scenario_snapshots/orders_paid/scenario.json\n"; const SE0R: &str = "orders_paid REFRESHED 2 relations, 3 rows\n source raw_orders PASS 2 rows, 41 B\n seed countries PASS 1 row, 1.5 KB\n snapshot tests/_scenario_snapshots/orders_paid/scenario.json\n"; const CF1: &str = "orders_big \u{1b}[38;5;167mFAIL\u{1b}[0m 0 relations, 0 rows\n \u{1b}[38;5;167m\u{1b}[1merror[X512]:\u{1b}[0m too many rows\n \u{1b}[2m= help:\u{1b}[0m narrow it\n Rerun with --retain to inspect scenario-owned artifacts.\n source raw_orders \u{1b}[38;5;167mFAIL\u{1b}[0m 0 rows, 0 B\n snapshot /elsewhere/scenario.json\n"; @@ -87,6 +87,12 @@ fn given_scenario_result_when_rendering_then_matches_result_row_layout() { error: coded(Some("X504"), "comparison failed to run", None), ..ExpectedRow::default() }], + assertions: vec![AssertionRow { + name: "no_negative".to_owned(), + failed: true, + error: coded(Some("X507"), "assertion failed to run", None), + ..AssertionRow::default() + }], ..ScenarioResultRow::default() }, use_color: false, diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/scenario_loop.rs b/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/scenario_loop.rs index 6fec48c45a..ce1630be37 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/scenario_loop.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/scenario_loop.rs @@ -2,8 +2,12 @@ use pyo3::prelude::{Bound, Py, PyAny, PyAnyMethods, PyErr, PyResult, Python}; use pyo3::types::{PyDict, PyDictMethods, PyTuple}; +use sqlbuild_analysis::compiler::main::scenario_helper_scope::scenario_helper_scope; +use sqlbuild_analysis::compiler::main::scenario_reader_ctes::scenario_reader_ctes; use sqlbuild_analysis::compiler::main::sql_scenario_classification::extract_scenario; -use sqlbuild_analysis::compiler::models::{SqlScenarioCtes, SqlTestCteText}; +use sqlbuild_analysis::compiler::models::{ + ScenarioHelperScope, ScenarioScopeInput, SqlScenarioCtes, SqlTestCteText, +}; use sqlbuild_attachments::test_targets::main::scenario_source_violation::scenario_source_violation; use sqlbuild_attachments::test_targets::models::ScenarioCteSources; use sqlbuild_sqltext::sql_references::models::SqlReference; @@ -53,7 +57,7 @@ impl<'py> TestScope<'_, 'py> { .map_err(|message| compile_error(self.context.classes, &message, None))?; let origin: FileOrigin<'_> = (file_path, relative_path, contents); self.checked_scenario_sources(&ctes, origin)?; - let assertions: Vec = texts(&ctes.assertions); + let assertions: Vec = self.assertion_reads(&ctes, relative_path)?; let targets: Vec = assertion_targets(self.context, self.model_loop, &assertions, origin)?; let fields: Bound<'py, PyDict> = PyDict::new(py); @@ -94,6 +98,26 @@ impl<'py> TestScope<'_, 'py> { .unbind()) } + /// Each assertion with the helper CTEs it reads, whose model references are assertion targets. + fn assertion_reads( + &self, + ctes: &SqlScenarioCtes, + relative_path: &str, + ) -> PyResult> { + let scope: ScenarioHelperScope = scenario_helper_scope(&ScenarioScopeInput { + ctes: &ctes.authored, + lexical: self.model_loop.syntax.clone(), + dialect: None, + file_label: relative_path, + }) + .map_err(|message| compile_error(self.context.classes, &message, None))?; + let mut reads: Vec = texts(&ctes.assertions); + for (_, sql) in &ctes.assertions { + reads.extend(texts(&scenario_reader_ctes(&scope, sql, false))); + } + Ok(reads) + } + /// `_validate_scenario_source_references`: disallowed source reads, then a scan's error. fn checked_scenario_sources( &self, diff --git a/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/execution.rs b/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/execution.rs index 3c1a004083..6405d5f179 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/execution.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/execution.rs @@ -17,6 +17,7 @@ use crate::bindings::_helpers::planner_resolution::locations::build_function_loc use crate::bindings::_helpers::planner_scenario::constants::{ SCENARIO_EXPECTED_COLUMNS_HELP, SCENARIO_PLAN_EXPECTED_COLUMNS, SCENARIO_PLAN_INVALID_FIXTURE, }; +use crate::bindings::_helpers::planner_scenario::helper_scope::scenario_helper_scope; use crate::bindings::_helpers::planner_scenario::{ entries::{function_entry, seed_entries}, fixtures::{extract_fixtures, fixture_plans}, @@ -35,6 +36,7 @@ use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyResult, Python, pyfunction}; use pyo3::types::{PyBool, PyDict, PyDictMethods, PyFrozenSet, PyTuple}; use sqlbuild_analysis::compiler::main::scenario_expected_column_message::scenario_expected_column_message; use sqlbuild_analysis::compiler::main::scenario_expected_columns::scenario_expected_columns; +use sqlbuild_analysis::compiler::main::scenario_reader_sql::scenario_reader_sql; #[pyfunction] #[pyo3(signature = (**kwargs))] @@ -332,6 +334,7 @@ fn expectations<'py>( models: &Bound<'py, PyDict>, ) -> PyResult<(Bound<'py, PyAny>, Bound<'py, PyAny>)> { let py = scenario.py(); + let scope = scenario_helper_scope(scenario, context)?; let mut expected: Vec> = Vec::new(); for cte in native_field(scenario, "expected_ctes")?.try_iter()? { let cte = cte?; @@ -341,7 +344,11 @@ fn expectations<'py>( let authored: String = native_field(&cte, "sql_body")?.extract()?; let compared = compared_columns(scenario, context, (models.get_item(name)?, name), &authored)?; - let sql = resolve_sql(context, &authored, false)?; + let sql = resolve_sql( + context, + &scenario_reader_sql(&scope, &authored, true), + false, + )?; expected.push(new_record_with( py, RecordId::ScenarioExpectedExpectationPlan, @@ -363,9 +370,10 @@ fn expectations<'py>( for cte in native_field(scenario, "assertion_ctes")?.try_iter()? { let cte = cte?; let name: String = native_field(&cte, "name")?.extract()?; + let authored: String = native_field(&cte, "sql_body")?.extract()?; let sql = resolve_sql( context, - &native_field(&cte, "sql_body")?.extract::()?, + &scenario_reader_sql(&scope, &authored, true), false, )?; assertions.push(new_record_with( diff --git a/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/fixtures.rs b/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/fixtures.rs index 0507666d9d..6ab30be6f1 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/fixtures.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/fixtures.rs @@ -2,6 +2,7 @@ use crate::bindings::_helpers::compile_driver::python_calls::KeywordArguments; use crate::bindings::_helpers::planner_scenario::constants::SCENARIO_PLAN_MISSING_FIXTURE_SQL; +use crate::bindings::_helpers::planner_scenario::helper_scope::scenario_helper_scope; use crate::bindings::_helpers::planner_scenario::{ models::SqlContext, sql::{required_target, resolve_sql, source_label}, @@ -13,6 +14,7 @@ use crate::bindings::_helpers::records::fields::mapping_get_or; use crate::bindings::_helpers::records::ids::RecordId; use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyResult, Python, pyfunction}; use pyo3::types::{PyDict, PyDictMethods, PyTuple}; +use sqlbuild_analysis::compiler::main::scenario_reader_sql::scenario_reader_sql; pub(super) fn extract_fixtures<'py>( scenario: &Bound<'py, PyAny>, @@ -31,23 +33,7 @@ pub(super) fn fixture_plans<'py>( overrides: &Bound<'py, PyAny>, ) -> PyResult> { let py = scenario.py(); - let mut helpers: Vec = Vec::new(); - for cte in native_field(scenario, "authored_ctes")?.try_iter()? { - let cte = cte?; - let name: String = native_field(&cte, "name")?.extract()?; - if ["__source__", "__ref__", "__seed__", "__dbt_ref__"] - .iter() - .any(|prefix| name.starts_with(prefix)) - { - continue; - } - let sql = resolve_sql( - context, - &native_field(&cte, "sql_body")?.extract::()?, - true, - )?; - helpers.push(format!("{name} AS ({sql})")); - } + let scope = scenario_helper_scope(scenario, context)?; let mut result: Vec> = Vec::new(); for (kind, resource, names, locations) in [ ( @@ -85,12 +71,11 @@ pub(super) fn fixture_plans<'py>( if kind != super::constants::DBT_REF && !overrides.is_none() { sql = mapping_get_or(overrides, (resource, &name), sql)?; } - let sql = resolve_sql(context, &sql.extract::()?, true)?; - let sql = if helpers.is_empty() { - sql - } else { - format!("WITH {} {sql}", helpers.join(", ")) - }; + let sql = resolve_sql( + context, + &scenario_reader_sql(&scope, &sql.extract::()?, false), + true, + )?; result.push(new_record_with( py, RecordId::ScenarioFixturePlan, diff --git a/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/helper_scope.rs b/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/helper_scope.rs new file mode 100644 index 0000000000..fc24781998 --- /dev/null +++ b/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/helper_scope.rs @@ -0,0 +1,64 @@ +//! The helper and fixture CTEs a scenario's fixtures, expected output and assertions read. + +use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyResult}; +use sqlbuild_analysis::compiler::main::scenario_helper_scope::scenario_helper_scope as helper_scope; +use sqlbuild_analysis::compiler::models::{ScenarioHelperScope, ScenarioScopeInput}; + +use crate::bindings::_helpers::planner_scenario::constants::SCENARIO_PLAN_INTERNAL; +use crate::bindings::_helpers::planner_scenario::{models::SqlContext, values::error}; +use crate::bindings::_helpers::records::fields::field as native_field; +use crate::bindings::_helpers::records::fields::mapping_get; +use crate::bindings::_helpers::sqltext::lexical_syntax::LexicalSyntaxInput; + +const FIXTURE_LOCATIONS: [(&str, &str); 4] = [ + ("__source__", "source_fixture_locations"), + ("__ref__", "ref_fixture_locations"), + ("__seed__", "seed_fixture_locations"), + ("__dbt_ref__", "dbt_ref_fixture_locations"), +]; + +/// The scenario's helper scope, where a check reading a fixture by name reads its relation. +pub(super) fn scenario_helper_scope( + scenario: &Bound<'_, PyAny>, + context: &SqlContext<'_>, +) -> PyResult { + let py = scenario.py(); + let mut ctes: Vec<(String, String)> = Vec::new(); + for cte in native_field(scenario, "authored_ctes")?.try_iter()? { + let cte = cte?; + let name: String = native_field(&cte, "name")?.extract()?; + let body: String = fixture_relation_sql(context, &name)? + .map_or_else(|| native_field(&cte, "sql_body")?.extract(), Ok)?; + ctes.push((name, body)); + } + let syntax: LexicalSyntaxInput = native_field(&context.syntax, "native_mapping")?.extract()?; + let dialect: Option = + native_field(&context.adapter, "sql_analysis_dialect_name")?.extract()?; + helper_scope(&ScenarioScopeInput { + ctes: &ctes, + lexical: syntax.into(), + dialect: dialect.as_deref(), + file_label: &context.label, + }) + .map_err( + |message| match error(py, &message, SCENARIO_PLAN_INTERNAL, None) { + Ok(failure) | Err(failure) => failure, + }, + ) +} + +/// `SELECT *` from the relation a fixture CTE materialises, or `None` for a helper. +fn fixture_relation_sql(context: &SqlContext<'_>, name: &str) -> PyResult> { + for (prefix, field) in FIXTURE_LOCATIONS { + let Some(fixture) = name.strip_prefix(prefix) else { + continue; + }; + let location = mapping_get(&native_field(&context.relations, field)?, fixture)?; + if location.is_none() { + return Ok(None); + } + let relation: String = native_field(&location, "qualified_name")?.extract()?; + return Ok(Some(format!("SELECT * FROM {relation}"))); + } + Ok(None) +} diff --git a/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/mod.rs b/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/mod.rs index 66d16970e7..5296f7a525 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/mod.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/mod.rs @@ -7,6 +7,7 @@ mod entries; pub(crate) mod execution; mod fixtures; mod graph; +mod helper_scope; mod models; pub(crate) mod registration; mod relations; diff --git a/crates/sqlbuild-python/src/bindings/_helpers/result_output/checks.rs b/crates/sqlbuild-python/src/bindings/_helpers/result_output/checks.rs index ffd2f0e442..5019fbe113 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/result_output/checks.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/result_output/checks.rs @@ -429,16 +429,28 @@ pub(crate) fn scenario_check<'py>(result: &Bound<'py, PyAny>) -> PyResult> = Vec::new(); for item in items(&native_field(result, "assertion_results")?)? { - let entries: Vec<(&'static str, Bound<'py, PyAny>)> = vec![ - ("name", native_field(&item, "name")?), - ("status", value(&item, "status")?), - ( - "failing_row_count", - native_field(&item, "failing_row_count")?, - ), - ("sample_rows", native_field(&item, "sample_rows")?), - ]; - assertions.push(with_errors(&item, entries)?); + let failing: Bound<'py, PyAny> = native_field(&item, "failing_row_count")?; + let ran: &str = if failing.is_none() { + COMPARISON_NOT_RUN + } else { + COMPARISON_COMPLETED + }; + let result: Bound<'py, PyDict> = dict( + py, + vec![ + ("name", native_field(&item, "name")?), + ("status", value(&item, "status")?), + ("query_status", any(py, ran)?), + ("failing_row_count", failing), + ("sample_rows", native_field(&item, "sample_rows")?), + ], + )?; + for (name, error) in fields(&item, &ERROR_FIELDS)? { + if !error.is_none() { + result.set_item(name, error)?; + } + } + assertions.push(result); } with_errors( result, diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md index 1dcdcb6a60..6896a8c95e 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md @@ -120,8 +120,15 @@ the model does not output fails planning with `S512` before the scenario runs. S `expected_columns "all"` in the `SCENARIO()` header to also require every model output column to be listed; a missing column then fails with `S512`, naming it. -A comparison that fails to run reports its error with an `ERROR` status and no row counts; in -`--json` output its `comparison_status` is `not_run` and its row counts are `null`. +Any other CTE is a helper. As in unit tests, fixtures, expected CTEs and assertions can read +helpers, and helpers can read other helpers, `__ref(...)` and the other relation calls, and fixtures +by CTE name, such as `__ref__orders`. SQLBuild places each helper a query reads, directly or through +other helpers, ahead of it in dependency order; a fixture read by name reads the fixture's +materialised table. A model an assertion reads through a helper is built like one it reads directly. + +A comparison or assertion that fails to run reports its error with an `ERROR` status and no row +counts. In `--json` output an expected result's `comparison_status`, or an assertion result's +`query_status`, is `not_run`, and its row counts are `null`. For sources with two-part identity, use double underscores: `__source__raw__orders`. diff --git a/src/sqlbuild/executor/scenario/models.py b/src/sqlbuild/executor/scenario/models.py index 75fdf52e24..ec386b1e9d 100644 --- a/src/sqlbuild/executor/scenario/models.py +++ b/src/sqlbuild/executor/scenario/models.py @@ -326,12 +326,12 @@ class ScenarioExpectedExpectationExecutionResult: @dataclass(frozen=True) class ScenarioAssertionExpectationExecutionResult: - """Outcome of executing one zero-row scenario assertion.""" + """Outcome of executing one zero-row scenario assertion; no count if it did not run.""" scenario_name: str name: str status: ExecutionStatus - failing_row_count: int = 0 + failing_row_count: int | None = None sample_rows: tuple[tuple[object, ...], ...] = field(default_factory=tuple) error_code: str | None = None error_help: str | None = None diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py index 2227ef521e..fc82c52f17 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py @@ -272,3 +272,17 @@ class ScenarioExpectedColumnsHeaderE2ETestCase: header_options: str expected_exit_code: int expected_stderr_fragment: str + + +@dataclass(frozen=True) +class ScenarioHelperCteE2ETestCase: + """Test case for scenario checks that read helper CTEs declared in the same file.""" + + description: str + helper_ctes: str + check_ctes: str + expected_exit_code: int + expected_stdout_fragments: tuple[str, ...] + unexpected_stdout_fragments: tuple[str, ...] = field(default_factory=tuple) + expected_json_expected_results: tuple[dict[str, object], ...] = field(default_factory=tuple) + expected_json_assertion_results: tuple[dict[str, object], ...] = field(default_factory=tuple) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/helpers.py b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/helpers.py index 7b789f8f04..a491cd5d19 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/helpers.py @@ -925,3 +925,15 @@ def selected_fields(result: dict[str, Any], expected: dict[str, object]) -> dict """Return the fields of `result` that `expected` names.""" return {key: result.get(key) for key in expected} + + +def build_helper_scenario_project_files(*, helper_ctes: str, check_ctes: str) -> dict[str, str]: + """Build an order-flags project whose scenario declares helpers ahead of its checks.""" + + return { + **_EXPECTED_COLUMNS_PROJECT_FILES, + _EXPECTED_COLUMNS_SCENARIO_PATH: ( + f"SCENARIO ({_EXPECTED_COLUMNS_DESCRIPTION});\n\n" + f"{_EXPECTED_COLUMNS_FIXTURE}{helper_ctes},\n\n{check_ctes}\n\nSELECT 1\n" + ), + } diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_helper_ctes.py b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_helper_ctes.py new file mode 100644 index 0000000000..aa40ce24c8 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_helper_ctes.py @@ -0,0 +1,175 @@ +"""E2E tests for scenario expected output and assertions that read helper CTEs.""" + +from __future__ import annotations + +import json +import subprocess +from pathlib import Path +from typing import Any + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.scenario._test_types import ( + ScenarioHelperCteE2ETestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.scenario.helpers import ( + build_helper_scenario_project_files, + selected_fields, +) +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project, run_sqb + +_EXPECTED_ROWS: str = "expected_rows AS (\n SELECT 'o1' AS order_id, FALSE AS is_cancelled\n)" +_CHAINED_HELPERS: str = ( + 'flagged AS (\n SELECT order_id, is_cancelled FROM __ref("order_flags")\n),\n\n' + "placed AS (\n SELECT order_id FROM __ref__orders WHERE status = 'placed'\n),\n\n" + "placed_flags AS (\n" + " SELECT flagged.order_id, flagged.is_cancelled\n" + " FROM flagged JOIN placed ON flagged.order_id = placed.order_id\n" + ")" +) +_PASSED_ASSERTION: dict[str, object] = { + "name": "flags_match", + "status": "success", + "query_status": "completed", + "failing_row_count": 0, +} + + +@pytest.mark.parametrize( + "test_case", + ( + ScenarioHelperCteE2ETestCase( + description="an assertion reads a helper CTE", + helper_ctes=_EXPECTED_ROWS, + check_ctes=( + "__assert__flags_match AS (\n" + ' SELECT order_id FROM __ref("order_flags")\n' + " WHERE order_id NOT IN (SELECT order_id FROM expected_rows)\n" + ")" + ), + expected_exit_code=0, + expected_stdout_fragments=( + "expect assertion flags_match", + "PASS=1 FAIL=0 TOTAL=1", + ), + expected_json_assertion_results=(_PASSED_ASSERTION,), + ), + ScenarioHelperCteE2ETestCase( + description="expected output reads a helper CTE", + helper_ctes=_EXPECTED_ROWS, + check_ctes=( + "__expected__order_flags AS (\n" + " SELECT order_id, 'placed' AS status, is_cancelled FROM expected_rows\n" + ")" + ), + expected_exit_code=0, + expected_stdout_fragments=("PASS=1 FAIL=0 TOTAL=1",), + expected_json_expected_results=( + { + "model_name": "order_flags", + "status": "success", + "comparison_status": "completed", + "mismatched_row_count": 0, + "missing_row_count": 0, + }, + ), + ), + ScenarioHelperCteE2ETestCase( + description="chained helpers read a model and a fixture by name", + helper_ctes=_CHAINED_HELPERS, + check_ctes=( + "__assert__flags_match AS (\n" + " SELECT order_id FROM placed_flags WHERE is_cancelled\n" + ")" + ), + expected_exit_code=0, + expected_stdout_fragments=("PASS=1 FAIL=0 TOTAL=1",), + expected_json_assertion_results=(_PASSED_ASSERTION,), + ), + ScenarioHelperCteE2ETestCase( + description="an assertion through chained helpers reports its failing rows", + helper_ctes=_CHAINED_HELPERS, + check_ctes=( + "__assert__flags_match AS (\n" + " SELECT order_id FROM placed_flags WHERE NOT is_cancelled\n" + ")" + ), + expected_exit_code=1, + expected_stdout_fragments=( + "expect assertion flags_match FAIL 1 row", + "assertion flags_match: failing_rows=1", + ), + expected_json_assertion_results=( + { + "status": "failed", + "query_status": "completed", + "failing_row_count": 1, + "error_code": "X508", + }, + ), + ), + ScenarioHelperCteE2ETestCase( + description="an assertion that fails to run reports its error without counts", + helper_ctes=_EXPECTED_ROWS, + check_ctes=( + "__assert__flags_match AS (\n" + ' SELECT order_id FROM __ref("order_flags")\n' + " WHERE order_id NOT IN (SELECT order_id FROM missing_rows)\n" + ")" + ), + expected_exit_code=1, + expected_stdout_fragments=( + "expect assertion flags_match ERROR", + "error[X507]", + "missing_rows", + ), + unexpected_stdout_fragments=("0 rows", "failing_rows="), + expected_json_assertion_results=( + { + "status": "failed", + "query_status": "not_run", + "failing_row_count": None, + "error_code": "X507", + }, + ), + ), + ), + ids=lambda case: case.description, +) +def test_given_scenario_helper_ctes_when_testing_then_checks_read_them( + test_case: ScenarioHelperCteE2ETestCase, tmp_path: Path +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="scenario_helper_ctes", + repo_files=build_helper_scenario_project_files( + helper_ctes=test_case.helper_ctes, check_ctes=test_case.check_ctes + ), + ) + + text: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", "scenario", "test"), project_dir=project_dir + ) + structured: subprocess.CompletedProcess[str] = run_sqb( + command=("scenario", "test", "--json"), project_dir=project_dir + ) + + assert text.returncode == test_case.expected_exit_code, text.stdout + text.stderr + assert structured.returncode == test_case.expected_exit_code, structured.stderr + for fragment in test_case.expected_stdout_fragments: + assert fragment in text.stdout, text.stdout + for fragment in test_case.unexpected_stdout_fragments: + assert fragment not in text.stdout, text.stdout + check: dict[str, Any] = json.loads(structured.stdout)["checks"][0] + expected_results: list[dict[str, Any]] = check["expected_results"] + assertion_results: list[dict[str, Any]] = check["assertion_results"] + assert ( + tuple(map(selected_fields, expected_results, test_case.expected_json_expected_results)) + == test_case.expected_json_expected_results + ) + assert len(expected_results) == len(test_case.expected_json_expected_results) + assert ( + tuple(map(selected_fields, assertion_results, test_case.expected_json_assertion_results)) + == test_case.expected_json_assertion_results + ) + assert len(assertion_results) == len(test_case.expected_json_assertion_results) diff --git a/website/src/content/docs/docs/concepts/scenarios.mdx b/website/src/content/docs/docs/concepts/scenarios.mdx index 0b1b18d46f..0a70f20061 100644 --- a/website/src/content/docs/docs/concepts/scenarios.mdx +++ b/website/src/content/docs/docs/concepts/scenarios.mdx @@ -108,8 +108,15 @@ the model does not output fails planning with `S512` before the scenario runs. S `expected_columns "all"` in the `SCENARIO()` header to also require every model output column to be listed; a missing column then fails with `S512`, naming it. -A comparison that fails to run reports its error with an `ERROR` status and no row counts; in -`--json` output its `comparison_status` is `not_run` and its row counts are `null`. +Any other CTE is a helper. As in unit tests, fixtures, expected CTEs and assertions can read +helpers, and helpers can read other helpers, `__ref(...)` and the other relation calls, and fixtures +by CTE name, such as `__ref__orders`. SQLBuild places each helper a query reads, directly or through +other helpers, ahead of it in dependency order; a fixture read by name reads the fixture's +materialised table. A model an assertion reads through a helper is built like one it reads directly. + +A comparison or assertion that fails to run reports its error with an `ERROR` status and no row +counts. In `--json` output an expected result's `comparison_status`, or an assertion result's +`query_status`, is `not_run`, and its row counts are `null`. For sources with two-part identity, use double underscores: `__source__raw__orders`. From dbecbd99e168da95dac71dd5cf3a0999e4e6da8f Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 23:41:49 +0100 Subject: [PATCH 09/12] fix: grow the stack while keying deeply nested macro context values Keying a macro context value recurses once per nesting level up to the 900-level limit. In builds with large frames that recursion overflowed an 8 MB stack and crashed the process instead of reporting the value as unstorable. Each level now runs through stacker::maybe_grow, so the depth limit is reached on any stack size. --- Cargo.lock | 1 + crates/sqlbuild-python/Cargo.toml | 1 + .../src/bindings/_helpers/macro_bridge/store_keys.rs | 12 +++++++++++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index c8fe4287b7..3e70d24d4d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1262,6 +1262,7 @@ dependencies = [ "sqlbuild-scopes", "sqlbuild-sqltext", "sqlbuild-state", + "stacker", ] [[package]] diff --git a/crates/sqlbuild-python/Cargo.toml b/crates/sqlbuild-python/Cargo.toml index 5645b3946a..077bf8b416 100644 --- a/crates/sqlbuild-python/Cargo.toml +++ b/crates/sqlbuild-python/Cargo.toml @@ -38,6 +38,7 @@ sqlbuild-rules.workspace = true sqlbuild-scopes.workspace = true sqlbuild-sqltext.workspace = true sqlbuild-state.workspace = true +stacker.workspace = true [lints] workspace = true diff --git a/crates/sqlbuild-python/src/bindings/_helpers/macro_bridge/store_keys.rs b/crates/sqlbuild-python/src/bindings/_helpers/macro_bridge/store_keys.rs index af1a465080..f13306459b 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/macro_bridge/store_keys.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/macro_bridge/store_keys.rs @@ -23,6 +23,10 @@ use crate::bindings::_helpers::records::fields::mapping_items; const VALUE_RENDERER_FIELD: &str = "_value_renderer"; /// Nesting at which a value counts as too deep to key, as Python's recursion limit stops it. const MAX_DEPTH: usize = 900; +/// Stack left below which one more level of `canonical` moves onto a new stack segment. +const CANONICAL_RED_ZONE: usize = 128 * 1024; +/// Size of each stack segment `canonical` grows by. +const CANONICAL_STACK_GROWTH: usize = 2 * 1024 * 1024; create_exception!(_native, UncacheableValue, pyo3::exceptions::PyException); @@ -99,8 +103,14 @@ impl<'py> CanonicalTypes<'py> { }) } - /// `_canonical(value)`. + /// `_canonical(value)`, on a stack that grows so nesting up to `MAX_DEPTH` cannot overflow it. fn canonical(&self, value: &Bound<'py, PyAny>, depth: usize) -> PyResult { + stacker::maybe_grow(CANONICAL_RED_ZONE, CANONICAL_STACK_GROWTH, || { + self.canonical_level(value, depth) + }) + } + + fn canonical_level(&self, value: &Bound<'py, PyAny>, depth: usize) -> PyResult { if depth >= MAX_DEPTH { return Err(PyRecursionError::new_err( "maximum recursion depth exceeded", From ff2b406e627308de374ed8160ce063fc70b8ea2f Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 23:41:50 +0100 Subject: [PATCH 10/12] test: fail make test when a worker crashes instead of restarting it A crashed xdist worker was replaced and the run then never finished. With --max-worker-restart=0 the crash is reported as a failure and the run ends. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 61dc44ffe3..610731828c 100644 --- a/Makefile +++ b/Makefile @@ -51,7 +51,7 @@ test: @mkdir -p /tmp/opencode @log="/tmp/opencode/test-$$(date +%Y%m%d-%H%M%S).log"; \ echo "Logging to $$log"; \ - env PYTHONUNBUFFERED=1 SQLBUILD_CONCURRENCY=$(SQLBUILD_CONCURRENCY) uv run pytest tests/unit tests/integration -m "not real_warehouse and not dbt" -vv --color=yes -n auto --dist loadfile 2>&1 | tee "$$log"; \ + env PYTHONUNBUFFERED=1 SQLBUILD_CONCURRENCY=$(SQLBUILD_CONCURRENCY) uv run pytest tests/unit tests/integration -m "not real_warehouse and not dbt" -vv --color=yes -n auto --dist loadfile --max-worker-restart=0 2>&1 | tee "$$log"; \ status=$${PIPESTATUS[0]}; \ echo "TEST_EXIT=$$status (log: $$log)" | tee -a "$$log"; \ exit $$status From 75a2b93f75fb62bce8f54e8119779606b8c8d758 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 23:51:18 +0100 Subject: [PATCH 11/12] fix: reject scenario checks that redefine a helper CTE name at compile Scenario checks now receive the file's helper CTEs, so an expected or assertion CTE defining its own CTE with a helper's name failed in the warehouse with a duplicate CTE name. Pass the checks into the helper scope so the SQL-test shadowing check reports it at compile time. --- .../compiler/_helpers/sql_tests/planning.rs | 4 +- .../_helpers/sql_tests/scenario_scope.rs | 16 +++++- .../sqlbuild-analysis/src/compiler/models.rs | 2 + .../tests/test_scenario_helper_ctes.rs | 56 ++++++++++++++++++- .../src/compiler/tests/test_types.rs | 7 +++ .../compiled_project/scenario_loop.rs | 7 +++ .../_helpers/planner_scenario/helper_scope.rs | 1 + .../cli/commands/main/scenario/_test_types.py | 10 ++++ .../main/scenario/test_helper_ctes.py | 51 +++++++++++++++++ 9 files changed, 148 insertions(+), 6 deletions(-) diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs index b0c1b964b8..d5db202b72 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs @@ -63,8 +63,8 @@ pub(crate) const MOCK_CTE_PREFIXES: [&str; 5] = [ DBT_REF_PREFIX, TABLE_FUNCTION_PREFIX, ]; -const EXPECTED_PREFIX: &str = "__expected__"; -const ASSERT_PREFIX: &str = "__assert__"; +pub(crate) const EXPECTED_PREFIX: &str = "__expected__"; +pub(crate) const ASSERT_PREFIX: &str = "__assert__"; const REF_FUNCTION: &str = "__ref"; const SOURCE_FUNCTION: &str = "__source"; const SEED_FUNCTION: &str = "__seed"; diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/scenario_scope.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/scenario_scope.rs index 76fe078c14..5e78330479 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/scenario_scope.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/scenario_scope.rs @@ -5,7 +5,8 @@ use std::collections::BTreeMap; use crate::compiler::_helpers::sql_tests::cte_slices::SliceDialect; use crate::compiler::_helpers::sql_tests::helper_scope::{ScopeGraph, ScopeRequest}; use crate::compiler::_helpers::sql_tests::planning::{ - DBT_REF_PREFIX, REF_PREFIX, SEED_PREFIX, SOURCE_PREFIX, SqlTestPatterns, TestFixtures, + ASSERT_PREFIX, DBT_REF_PREFIX, EXPECTED_PREFIX, REF_PREFIX, SEED_PREFIX, SOURCE_PREFIX, + SqlTestPatterns, TestFixtures, }; use crate::compiler::models::{ScenarioHelperScope, ScenarioScopeInput, SqlTestCte}; @@ -15,7 +16,7 @@ const FIXTURE_PREFIXES: [&str; 4] = [REF_PREFIX, SOURCE_PREFIX, SEED_PREFIX, DBT /// Index every helper, including ones that call references, and every fixture as a mock. pub(crate) fn helper_scope(input: &ScenarioScopeInput<'_>) -> Result { let patterns = SqlTestPatterns::new(input.lexical.clone()).map_err(plain)?; - let fixtures = scenario_fixtures(input.ctes); + let fixtures = scenario_fixtures(input.ctes, input.checks); let read_helper_names: Vec = fixtures .helpers .iter() @@ -34,7 +35,7 @@ pub(crate) fn helper_scope(input: &ScenarioScopeInput<'_>) -> Result TestFixtures { +fn scenario_fixtures(ctes: &[(String, String)], checks: &[(String, String)]) -> TestFixtures { let mut fixtures = TestFixtures { mock_refs: BTreeMap::new(), mock_sources: BTreeMap::new(), @@ -65,6 +66,15 @@ fn scenario_fixtures(ctes: &[(String, String)]) -> TestFixtures { group.insert(key.to_owned(), sql.clone()); } } + for (name, sql) in checks { + if let Some(model) = name.strip_prefix(EXPECTED_PREFIX) { + fixtures.expected.insert(model.to_owned(), sql.clone()); + } else if let Some(assertion) = name.strip_prefix(ASSERT_PREFIX) { + fixtures + .assertions + .push((assertion.to_owned(), sql.clone())); + } + } fixtures } diff --git a/crates/sqlbuild-analysis/src/compiler/models.rs b/crates/sqlbuild-analysis/src/compiler/models.rs index 87af4d5946..37775178d6 100644 --- a/crates/sqlbuild-analysis/src/compiler/models.rs +++ b/crates/sqlbuild-analysis/src/compiler/models.rs @@ -412,6 +412,8 @@ pub struct ScenarioExpectedColumns { pub struct ScenarioScopeInput<'a> { /// Every non-check CTE by authored name; a fixture body is what a check reading it sees. pub ctes: &'a [(String, String)], + /// The scenario's `__expected__` and `__assert__` CTEs, checked against helper names. + pub checks: &'a [(String, String)], pub lexical: LexicalSyntax, pub dialect: Option<&'a str>, pub file_label: &'a str, diff --git a/crates/sqlbuild-analysis/src/compiler/tests/test_scenario_helper_ctes.rs b/crates/sqlbuild-analysis/src/compiler/tests/test_scenario_helper_ctes.rs index 45becff149..57bd319cc5 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/test_scenario_helper_ctes.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_scenario_helper_ctes.rs @@ -3,7 +3,9 @@ use sqlbuild_sqltext::sql_scan::models::LexicalSyntax; use crate::compiler::main::scenario_helper_scope::scenario_helper_scope; use crate::compiler::main::scenario_reader_sql::scenario_reader_sql; use crate::compiler::models::ScenarioScopeInput; -use crate::compiler::tests::test_types::ScenarioHelperScopeTestCase; +use crate::compiler::tests::test_types::{ + ScenarioHelperScopeTestCase, ScenarioHelperShadowTestCase, +}; #[test] fn given_scenario_helpers_when_scoping_a_reader_then_prepends_what_it_reads_in_order() { @@ -16,6 +18,7 @@ fn given_scenario_helpers_when_scoping_a_reader_then_prepends_what_it_reads_in_o .map(|(name, sql)| (name.to_owned(), sql.to_owned())); let scope = scenario_helper_scope(&ScenarioScopeInput { ctes: &ctes, + checks: &[], lexical: LexicalSyntax::default(), dialect: Some("duckdb"), file_label: "tests/scenarios/orders.sql", @@ -59,3 +62,54 @@ fn given_scenario_helpers_when_scoping_a_reader_then_prepends_what_it_reads_in_o ); } } + +#[test] +fn given_check_redefining_a_helper_name_when_scoping_then_rejects_like_sql_tests() { + let ctes = [ + ("__ref__orders", "SELECT * FROM main.fixture_orders"), + ("expected_rows", "SELECT 'o1' AS order_id"), + ] + .map(|(name, sql)| (name.to_owned(), sql.to_owned())); + let test_cases = [ + ScenarioHelperShadowTestCase { + description: "an assertion defining a helper's name inside its WITH", + check_name: "__assert__flags_match", + check_sql: "WITH expected_rows AS (SELECT 'o2' AS order_id) \ + SELECT order_id FROM expected_rows", + expected_rejected: true, + }, + ScenarioHelperShadowTestCase { + description: "an expected output defining a helper's name inside its WITH", + check_name: "__expected__order_flags", + check_sql: "WITH expected_rows AS (SELECT 'o2' AS order_id) \ + SELECT order_id FROM expected_rows", + expected_rejected: true, + }, + ScenarioHelperShadowTestCase { + description: "a check reading the helper without redefining it", + check_name: "__assert__reads_only", + check_sql: "SELECT order_id FROM expected_rows", + expected_rejected: false, + }, + ]; + for test_case in test_cases { + let checks = [( + test_case.check_name.to_owned(), + test_case.check_sql.to_owned(), + )]; + let rejected: bool = scenario_helper_scope(&ScenarioScopeInput { + ctes: &ctes, + checks: &checks, + lexical: LexicalSyntax::default(), + dialect: Some("duckdb"), + file_label: "tests/scenarios/orders.sql", + }) + .err() + .is_some_and(|message| message.contains("redefines helper CTE 'expected_rows'")); + assert_eq!( + rejected, test_case.expected_rejected, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs b/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs index 1cc31d2b0f..62cb4dd15f 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs @@ -220,3 +220,10 @@ pub(crate) struct ScenarioHelperScopeTestCase { pub(crate) fixtures: bool, pub(crate) expected_sql: &'static str, } + +pub(crate) struct ScenarioHelperShadowTestCase { + pub(crate) description: &'static str, + pub(crate) check_name: &'static str, + pub(crate) check_sql: &'static str, + pub(crate) expected_rejected: bool, +} diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/scenario_loop.rs b/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/scenario_loop.rs index ce1630be37..2771b1321c 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/scenario_loop.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/scenario_loop.rs @@ -104,8 +104,15 @@ impl<'py> TestScope<'_, 'py> { ctes: &SqlScenarioCtes, relative_path: &str, ) -> PyResult> { + let checks: Vec<(String, String)> = ctes + .expected + .iter() + .chain(&ctes.assertions) + .cloned() + .collect(); let scope: ScenarioHelperScope = scenario_helper_scope(&ScenarioScopeInput { ctes: &ctes.authored, + checks: &checks, lexical: self.model_loop.syntax.clone(), dialect: None, file_label: relative_path, diff --git a/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/helper_scope.rs b/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/helper_scope.rs index fc24781998..698b99c9cb 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/helper_scope.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/planner_scenario/helper_scope.rs @@ -36,6 +36,7 @@ pub(super) fn scenario_helper_scope( native_field(&context.adapter, "sql_analysis_dialect_name")?.extract()?; helper_scope(&ScenarioScopeInput { ctes: &ctes, + checks: &[], lexical: syntax.into(), dialect: dialect.as_deref(), file_label: &context.label, diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py index fc82c52f17..8906a34d34 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py @@ -286,3 +286,13 @@ class ScenarioHelperCteE2ETestCase: unexpected_stdout_fragments: tuple[str, ...] = field(default_factory=tuple) expected_json_expected_results: tuple[dict[str, object], ...] = field(default_factory=tuple) expected_json_assertion_results: tuple[dict[str, object], ...] = field(default_factory=tuple) + + +@dataclass(frozen=True) +class ScenarioHelperShadowingE2ETestCase: + """Test case for a scenario check that redefines a helper CTE name.""" + + description: str + check_ctes: str + expected_output_fragment: str + unexpected_output_fragment: str diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_helper_ctes.py b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_helper_ctes.py index aa40ce24c8..129318af3c 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_helper_ctes.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_helper_ctes.py @@ -11,6 +11,7 @@ from tests.e2e.src.sqlbuild.cli.commands.main.scenario._test_types import ( ScenarioHelperCteE2ETestCase, + ScenarioHelperShadowingE2ETestCase, ) from tests.e2e.src.sqlbuild.cli.commands.main.scenario.helpers import ( build_helper_scenario_project_files, @@ -173,3 +174,53 @@ def test_given_scenario_helper_ctes_when_testing_then_checks_read_them( == test_case.expected_json_assertion_results ) assert len(assertion_results) == len(test_case.expected_json_assertion_results) + + +@pytest.mark.parametrize( + "test_case", + ( + ScenarioHelperShadowingE2ETestCase( + description="an assertion redefines a helper CTE name", + check_ctes=( + "__assert__flags_match AS (\n" + " WITH expected_rows AS (SELECT 'o2' AS order_id)\n" + ' SELECT order_id FROM __ref("order_flags")\n' + " WHERE order_id NOT IN (SELECT order_id FROM expected_rows)\n" + ")" + ), + expected_output_fragment="redefines helper CTE 'expected_rows'", + unexpected_output_fragment="Duplicate CTE name", + ), + ScenarioHelperShadowingE2ETestCase( + description="expected output redefines a helper CTE name", + check_ctes=( + "__expected__order_flags AS (\n" + " WITH expected_rows AS (SELECT 'o1' AS order_id, FALSE AS is_cancelled)\n" + " SELECT order_id, 'placed' AS status, is_cancelled FROM expected_rows\n" + ")" + ), + expected_output_fragment="redefines helper CTE 'expected_rows'", + unexpected_output_fragment="Duplicate CTE name", + ), + ), + ids=lambda case: case.description, +) +def test_given_scenario_check_redefining_a_helper_when_compiling_then_reports_a_compile_error( + test_case: ScenarioHelperShadowingE2ETestCase, tmp_path: Path +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="scenario_helper_shadowing", + repo_files=build_helper_scenario_project_files( + helper_ctes=_EXPECTED_ROWS, check_ctes=test_case.check_ctes + ), + ) + + compiled: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", "compile"), project_dir=project_dir + ) + + output: str = compiled.stdout + compiled.stderr + assert compiled.returncode != 0, output + assert test_case.expected_output_fragment in output, output + assert test_case.unexpected_output_fragment not in output, output From fb16e7d32d36f2d2a9766b827f31adce3e25fd61 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Mon, 19 Oct 2026 00:45:11 +0100 Subject: [PATCH 12/12] test: expect scenario fixtures to carry only the helper CTEs they read CHI-634 gives each fixture only the helpers it reads, so the planner fixture test no longer expects every fixture to carry every helper. --- .../planner/_helpers/test_scenario_relations.py | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_relations.py b/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_relations.py index 8f824c76bf..39157307aa 100644 --- a/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_relations.py +++ b/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_relations.py @@ -161,7 +161,7 @@ def test_given_quoting_renderer_when_building_relation_plan_then_renders_through "test_case", [ ScenarioFixturePlanTestCase( - description="wraps fixture SQL with shared scenario helper CTEs", + description="wraps each fixture with only the scenario helper CTEs it reads", graph_plan=ScenarioGraphPlan( key=build_scenario_relation_test_project().models[0].key, name=SCENARIO_NAME, @@ -178,18 +178,9 @@ def test_given_quoting_renderer_when_building_relation_plan_then_renders_through "WITH helper_orders AS (SELECT 1 AS order_id, 10 AS customer_id) " "SELECT * FROM helper_orders" ), - "ref:stg_customers": ( - "WITH helper_orders AS (SELECT 1 AS order_id, 10 AS customer_id) " - "SELECT 10 AS customer_id" - ), - "dbt_ref:stripe__payments": ( - "WITH helper_orders AS (SELECT 1 AS order_id, 10 AS customer_id) " - "SELECT 1 AS payment_id, 10 AS customer_id" - ), - "seed:country_codes": ( - "WITH helper_orders AS (SELECT 1 AS order_id, 10 AS customer_id) " - "SELECT 'US' AS country_code" - ), + "ref:stg_customers": "SELECT 10 AS customer_id", + "dbt_ref:stripe__payments": "SELECT 1 AS payment_id, 10 AS customer_id", + "seed:country_codes": "SELECT 'US' AS country_code", }, expected_fixture_targets={ "source:raw__orders": "scenario_schema.__sqb_51b385aebe20__source__raw__orders",