From 300bf688f0e9893cca246aa756332d86dfcd06f8 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 6 Oct 2026 15:34:47 +0100 Subject: [PATCH 01/10] fix(test): inline each model once in SQL unit-test queries SQL unit tests hoisted the CTEs of the model under test into the test's top-level WITH for `__actual__` and also nested the model again as its `__ref__` CTE. Every model was emitted twice, and a hoisted name such as `final` could resolve inside a later step's nested WITH on warehouses that look up outer CTE names first. Each planned model step is now defined once as `__ref__ AS ()`, its own CTEs stay nested, and `__actual__` reads `SELECT * FROM __ref__`. Only T-SQL, which rejects a nested WITH, still flattens model CTEs, renaming collisions as before. --- .../compiler/_helpers/sql_tests/cte_sql.rs | 7 +- .../compiler/_helpers/sql_tests/rendering.rs | 66 ++++++++++---- .../src/compiler/tests/helpers.rs | 31 +++++-- .../compiler/tests/test_sql_test_rendering.rs | 12 +-- .../cli/commands/main/test/_test_types.py | 11 +++ .../cli/commands/main/test/helpers.py | 56 ++++++++++++ .../main/test/test_authored_sql_parity.py | 24 +++-- .../commands/main/test/test_model_inlining.py | 91 +++++++++++++++++++ .../sqlbuild/compiler/pipeline/test_main.py | 11 ++- .../testing/test_sql_test_build_parity.py | 29 +++--- 10 files changed, 278 insertions(+), 60 deletions(-) create mode 100644 tests/e2e/src/sqlbuild/cli/commands/main/test/test_model_inlining.py diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_sql.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_sql.rs index 9ab3edb795..b18143f487 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_sql.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_sql.rs @@ -5,10 +5,15 @@ use sqlbuild_sqltext::sql_scan::main::skip_whitespace::skip_whitespace; /// Render one `name AS (body)` definition, closing a trailing line comment first. pub(crate) fn cte_definition_sql(name: &str, sql: &str) -> String { + format!("{name} AS {}", parenthesized_sql(sql)) +} + +/// Wrap a query in parentheses, closing a trailing line comment first. +pub(crate) fn parenthesized_sql(sql: &str) -> String { let body = sql.trim_end(); let final_line = body.rsplit_once('\n').map_or(body, |(_, line)| line); let terminator = if final_line.contains("--") { "\n" } else { "" }; - format!("{name} AS ({body}{terminator})") + format!("({body}{terminator})") } /// Append CTEs whose names are not already present, keeping first definitions and order. 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 543a5d4ba6..2ba74f9f0c 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/rendering.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/rendering.rs @@ -10,8 +10,9 @@ use crate::compiler::_helpers::sql_tests::cte_slices::{ used_ctes, }; use crate::compiler::_helpers::sql_tests::cte_sql::{ - cte_definition_sql, leading_with_prefix_end, with_leading_ctes, + cte_definition_sql, leading_with_prefix_end, parenthesized_sql, with_leading_ctes, }; +use crate::compiler::_helpers::sql_tests::planning::REF_PREFIX; use rayon::iter::{IntoParallelIterator, ParallelIterator}; use serde::{Deserialize, Serialize}; @@ -195,22 +196,30 @@ impl RenderCteState { )) } - /// Place a chain step's generated CTEs at top level and return its comparison body. - fn actual_step_sql(&mut self, step: &ChainStep, enabled: bool) -> Result { + /// Define a step's inputs and its model once at top level, the model as a nested `__ref__` CTE. + fn actual_step_sql( + &mut self, + step: &ChainStep, + suffix: &str, + enabled: bool, + ) -> Result { let origin = format!("model '{}'", step.model_name); - if step.lifted_ctes.is_empty() { + let Some(body) = step.comparison_body_sql.as_deref() else { return self.lift(&step.resolved_sql, enabled, &origin); + }; + let body = strip_statement_terminators(body, self.dialect); + let model_cte = model_cte_name(&step.model_name, suffix); + let mut ctes = step.lifted_ctes.clone(); + ctes.push((model_cte.clone(), body.to_string())); + if let Some(collisions) = self.merge(&ctes, enabled, &origin)? { + return self.nested( + &nested_model_sql(&step.lifted_ctes, body), + enabled, + &origin, + &[collisions], + ); } - if let Some(collisions) = self.merge(&step.lifted_ctes, enabled, &origin)? { - return self.nested(&step.resolved_sql, enabled, &origin, &[collisions]); - } - self.lift( - step.comparison_body_sql - .as_deref() - .unwrap_or(&step.resolved_sql), - enabled, - &origin, - ) + Ok(format!("SELECT * FROM {model_cte}")) } /// Place an expected step's helper CTEs at top level and return its comparison body. @@ -427,6 +436,30 @@ fn collision_message(origin: &str, collisions: &[(&str, &LiftedCte)]) -> String .join("; ") } +/// The `__ref__` CTE of a step's model, under the name downstream steps reference it by. +fn model_cte_name(model_name: &str, suffix: &str) -> String { + let mut characters = model_name.chars(); + let plain = characters + .next() + .is_some_and(|first| first.is_ascii_alphabetic() || first == '_') + && characters.all(|character| character.is_ascii_alphanumeric() || character == '_'); + format!("{REF_PREFIX}{}", if plain { model_name } else { suffix }) +} + +/// A model body nested below its inputs, for a step whose inputs collide at top level. +fn nested_model_sql(inputs: &[(String, String)], body: &str) -> String { + if inputs.is_empty() { + return body.to_string(); + } + with_leading_ctes( + inputs, + &format!( + "SELECT * FROM {} AS __sqlbuild_model", + parenthesized_sql(body) + ), + ) +} + /// Describe a generated CTE by what it stands in for: an upstream model, source or seed. fn generated_cte_origin(name: &str) -> String { for (prefix, kind) in [ @@ -535,7 +568,8 @@ fn render_difference_sample_sql(request: &DifferenceSampleRequest) -> Result Result bool { limited["sql"], json!( "WITH __ref__stg_orders AS (SELECT 1 AS order_id),\n\ - __actual AS (SELECT * FROM __ref__stg_orders),\n\ + __ref__orders AS (SELECT * FROM __ref__stg_orders),\n\ + __actual AS (SELECT * FROM __ref__orders),\n\ __expected AS (SELECT 2 AS order_id)\n\ SELECT * FROM (SELECT * FROM __expected EXCEPT SELECT * FROM __actual) \ AS __sqlbuild_difference LIMIT 3" @@ -1283,7 +1284,7 @@ pub(super) fn repeated_model_sql_renders_like_separate_batches() -> bool { }) } -pub(super) fn colliding_model_ctes_nest_on_nested_with_dialects() -> bool { +pub(super) fn shared_model_cte_names_stay_nested_on_nested_with_dialects() -> bool { [ ("duckdb", "final"), ("snowflake", "final"), @@ -1300,7 +1301,12 @@ pub(super) fn colliding_model_ctes_nest_on_nested_with_dialects() -> bool { let sql = render_one(json!({ "sqlAnalysisDialect": dialect, "chain": [ - {"modelName": "stg_orders", "resolvedSql": upstream, "expectedCteSql": "SELECT 1 AS order_id"}, + { + "modelName": "stg_orders", + "resolvedSql": upstream, + "comparisonBodySql": upstream, + "expectedCteSql": "SELECT 1 AS order_id" + }, { "modelName": "orders", "resolvedSql": format!("WITH __ref__stg_orders AS ({upstream}), {}", &downstream[5..]), @@ -1310,11 +1316,14 @@ pub(super) fn colliding_model_ctes_nest_on_nested_with_dialects() -> bool { } ] })) - .expect("nested fallback"); + .expect("nested model scopes"); sql.starts_with(&format!( - "WITH {name} AS (SELECT 1 AS order_id),\n__ref__stg_orders AS ({upstream}),\n" - )) && sql.contains(&format!("__actual__stg_orders AS (SELECT * FROM {name})")) - && sql.contains(&format!("__actual__orders AS ({downstream})")) + "WITH __ref__stg_orders AS ({upstream}),\n\ + __ref__orders AS ({downstream}),\n\ + __actual__stg_orders AS (SELECT * FROM __ref__stg_orders),\n" + )) && sql.contains("__actual__orders AS (SELECT * FROM __ref__orders)") + && sql.matches(&upstream).count() == 1 + && sql.matches(&downstream).count() == 1 }) } @@ -1322,8 +1331,9 @@ pub(super) fn generated_with_bodies_stay_nested_verbatim() -> bool { let sql = render_one(colliding_chain_request("duckdb", "final")).expect("render"); sql.starts_with( "WITH __ref__stg_orders AS (WITH final AS (SELECT 1 AS order_id) SELECT * FROM final),\n\ - final AS (SELECT order_id + 1 AS order_id FROM __ref__stg_orders),\n\ - __actual__orders AS (SELECT final.order_id FROM final),\n", + __ref__orders AS (WITH final AS (SELECT order_id + 1 AS order_id FROM __ref__stg_orders) \ + SELECT final.order_id FROM final),\n\ + __actual__orders AS (SELECT * FROM __ref__orders),\n", ) } @@ -1333,7 +1343,8 @@ pub(super) fn tsql_model_cte_collisions_are_renamed_by_token_span() -> bool { "WITH final AS (SELECT 1 AS order_id),\n\ __ref__stg_orders AS (SELECT * FROM final),\n\ __sqb_cte_0 AS (SELECT order_id + 1 AS order_id FROM __ref__stg_orders),\n\ - __actual__orders AS (SELECT __sqb_cte_0.order_id FROM __sqb_cte_0),\n", + __ref__orders AS (SELECT __sqb_cte_0.order_id FROM __sqb_cte_0),\n\ + __actual__orders AS (SELECT * FROM __ref__orders),\n", ) } diff --git a/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_rendering.rs b/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_rendering.rs index 44053015dc..9dfbc3b5d8 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_rendering.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_rendering.rs @@ -6,10 +6,10 @@ use crate::compiler::tests::helpers::{ sqlserver_difference_sample_projects_bracketed_columns, }; use crate::compiler::tests::helpers::{ - colliding_model_ctes_nest_on_nested_with_dialects, generated_with_bodies_stay_nested_verbatim, - repeated_model_sql_renders_like_separate_batches, snowflake_function_synonyms_stay_as_authored, - trailing_statement_terminators_are_dropped, tsql_distinct_ctes_lift_verbatim, - tsql_fixture_and_model_cte_collisions_are_refused, + generated_with_bodies_stay_nested_verbatim, repeated_model_sql_renders_like_separate_batches, + shared_model_cte_names_stay_nested_on_nested_with_dialects, + snowflake_function_synonyms_stay_as_authored, trailing_statement_terminators_are_dropped, + tsql_distinct_ctes_lift_verbatim, tsql_fixture_and_model_cte_collisions_are_refused, tsql_identical_helper_ending_in_line_comment_is_shared, tsql_model_cte_collisions_are_renamed_by_token_span, tsql_unprovable_cte_renames_are_refused_with_named_ctes, @@ -20,8 +20,8 @@ use crate::compiler::tests::test_types::SqlTestRenderingTestCase; fn given_sql_rendering_cases_when_rendering_native_batches_then_expected_behavior_holds() { let test_cases = [ SqlTestRenderingTestCase { - description: "colliding model CTEs use the nested fallback on nested-WITH dialects", - run: colliding_model_ctes_nest_on_nested_with_dialects, + description: "model CTEs sharing a name stay nested in each model's own CTE", + run: shared_model_cte_names_stay_nested_on_nested_with_dialects, expected_success: true, }, SqlTestRenderingTestCase { 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 a8bb92c0fb..04be200740 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 @@ -240,3 +240,14 @@ class TerminalStepRowsE2ETestCase: expected_absent_fragments: tuple[str, ...] expected_absent_line_prefixes: tuple[str, ...] expected_screen_fragments: tuple[str, ...] + + +@dataclass(frozen=True) +class ModelInliningE2ETestCase: + """A two-model chain whose models share CTE names, tested with and without SQL analysis.""" + + description: str + sql_analysis_enabled: bool + expected_order_lines_sql: str + expected_exit_code: int + expected_output_fragments: tuple[str, ...] 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 e980efa666..bc298113a2 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/helpers.py @@ -1554,3 +1554,59 @@ def build_cte_scope_project_files(*, queries: tuple[str, ...], expected: str) -> for index, query in enumerate(queries): files[f"models/orders_{index}.sql"] = f"MODEL (description 'Test model.');\n{query}" return files + + +ORDER_LINES_MODEL_SQL: str = ( + 'WITH base_rows AS (SELECT order_id, amount FROM __source("raw_orders")),\n' + "final AS (SELECT order_id, amount * 2 AS line_total FROM base_rows)\n" + "SELECT order_id, line_total FROM final" +) +ORDER_TOTALS_MODEL_SQL: str = ( + "WITH final AS (\n" + " SELECT order_id, line_total + 1 AS order_total, 'paid' AS status\n" + ' FROM __ref("order_lines")\n' + ")\n" + "SELECT order_id, order_total, status FROM final" +) +ORDER_TOTALS_EXPECTED_ROWS_SQL: str = "SELECT 1 AS order_id, 21 AS order_total" + + +def build_shared_cte_name_chain_project_files( + *, sql_analysis_enabled: bool, expected_order_lines_sql: str +) -> dict[str, str]: + """Build two chained models that both end in `final`, plus helpers shared by fixtures.""" + + sql_analysis_value: str = {False: "false", True: "true"}[sql_analysis_enabled] + return { + "sqlbuild_project.toml": ( + 'name = "model_inlining"\nadapter = "duckdb"\n\n[connection]\n' + 'database = "model_inlining.duckdb"\n\n[settings]\n' + f"sql_analysis = {sql_analysis_value}\n" + ), + "sources/raw.yml": ( + "sources:\n - name: raw_orders\n description: Test source raw_orders.\n" + " schema: main\n table: raw_orders\n" + ), + "models/order_lines.sql": ( + f"MODEL (description 'Order lines.');\n\n{ORDER_LINES_MODEL_SQL}\n" + ), + "models/order_totals.sql": ( + f"MODEL (description 'Order totals.');\n\n{ORDER_TOTALS_MODEL_SQL}\n" + ), + "tests/unit/order_chain.sql": ( + 'TEST (name "order_chain");\n\n' + "WITH\n" + "base_rows AS (SELECT 1 AS order_id, 10 AS amount),\n" + "__source__raw_orders AS (SELECT order_id, amount FROM base_rows),\n" + f"expected_rows AS ({ORDER_TOTALS_EXPECTED_ROWS_SQL}),\n" + f"__expected__order_lines AS ({expected_order_lines_sql}),\n" + "__expected__order_totals AS (\n" + " SELECT order_id, order_total, 'paid' AS status FROM expected_rows\n" + "),\n" + "__assert__totals_match AS (\n" + ' SELECT order_id, order_total FROM __ref("order_totals")\n' + " EXCEPT SELECT order_id, order_total FROM expected_rows\n" + ")\n" + "SELECT 1\n" + ), + } diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_authored_sql_parity.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_authored_sql_parity.py index 8f0df214a6..7c0444d37a 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_authored_sql_parity.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_authored_sql_parity.py @@ -20,24 +20,25 @@ description="comments and dollar quotes in CTE bodies", compiled_test_name="test_stg_orders.sql", expected_fragments=( - "base AS (\n -- a comment with an unmatched ) parenthesis\n" + "__ref__stg_orders AS (WITH base AS (\n" + " -- a comment with an unmatched ) parenthesis\n" " SELECT id AS order_id, amount, 'it''s (fine)' AS note /* ( */\n" - " FROM __source__raw_orders)", + " FROM __source__raw_orders\n),", "tagged AS (SELECT order_id, amount, note, $$a ) b$$ AS tag FROM base)", - "__actual__stg_orders AS (SELECT order_id, amount, note, tag FROM tagged)", + "__actual__stg_orders AS (SELECT * FROM __ref__stg_orders)", ), ), AuthoredSqlParityTestCase( - description="nested WITH and a fixture CTE collision use the nested fallback", + description="nested WITH and a same-named fixture CTE keep separate scopes", compiled_test_name="test_orders.sql", expected_fragments=( - "totals AS (\n" + "__ref__orders AS (WITH totals AS (\n" " WITH ranked AS (SELECT order_id, amount, tag FROM __ref__stg_orders)\n" - " SELECT order_id, amount * 2 AS doubled, tag FROM ranked)", + " SELECT order_id, amount * 2 AS doubled, tag FROM ranked\n),\n" "helper_rows AS (SELECT order_id, doubled, tag FROM totals)", - "__expected__orders AS (WITH helper_rows AS " - "(SELECT 1 AS order_id, 20 AS doubled, 'x' AS tag) " - "SELECT order_id, doubled, tag FROM helper_rows)", + "\nhelper_rows AS (SELECT 1 AS order_id, 20 AS doubled, 'x' AS tag),\n", + "__actual__orders AS (SELECT * FROM __ref__orders)", + "__expected__orders AS (SELECT order_id, doubled, tag FROM helper_rows)", ), ), ), @@ -73,7 +74,10 @@ def test_given_authored_ctes_when_testing_then_passes_and_runs_the_authored_sql( AuthoredSqlParityTestCase( description="a WITH model ending in a statement terminator", compiled_test_name="test_m.sql", - expected_fragments=("__actual__m AS (SELECT id FROM a)",), + expected_fragments=( + "__ref__m AS (WITH a AS (SELECT id FROM __source__raw_orders)\nSELECT id FROM a)", + "__actual__m AS (SELECT * FROM __ref__m)", + ), ), ), ids=lambda case: case.description, diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_model_inlining.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_model_inlining.py new file mode 100644 index 0000000000..81c6dcfd37 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_model_inlining.py @@ -0,0 +1,91 @@ +"""Each model in a SQL test is inlined once, in its own scope, through the real DuckDB CLI.""" + +from pathlib import Path +from subprocess import CompletedProcess + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.test._test_types import ModelInliningE2ETestCase +from tests.e2e.src.sqlbuild.cli.commands.main.test.helpers import ( + build_shared_cte_name_chain_project_files, +) +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project, run_sqb + +_SINGLE_OCCURRENCE_FRAGMENTS: tuple[str, ...] = ( + "__ref__order_lines AS (", + "__ref__order_totals AS (", + "amount * 2 AS line_total", + "line_total + 1 AS order_total", +) +_EXPECTED_FRAGMENTS: tuple[str, ...] = ( + "__actual__order_lines AS (SELECT * FROM __ref__order_lines)", + "__actual__order_totals AS (SELECT * FROM __ref__order_totals)", +) +_PASSING_ORDER_LINES_SQL: str = "SELECT 1 AS order_id, 20 AS line_total" + + +@pytest.mark.parametrize( + "test_case", + ( + ModelInliningE2ETestCase( + description="analysed chain with shared final CTEs passes", + sql_analysis_enabled=True, + expected_order_lines_sql=_PASSING_ORDER_LINES_SQL, + expected_exit_code=0, + expected_output_fragments=("PASS=1 FAIL=0",), + ), + ModelInliningE2ETestCase( + description="textual chain with shared final CTEs passes", + sql_analysis_enabled=False, + expected_order_lines_sql=_PASSING_ORDER_LINES_SQL, + expected_exit_code=0, + expected_output_fragments=("PASS=1 FAIL=0",), + ), + ModelInliningE2ETestCase( + description="wrong upstream expectation fails on the upstream model only", + sql_analysis_enabled=True, + expected_order_lines_sql="SELECT 1 AS order_id, 99 AS line_total", + expected_exit_code=1, + expected_output_fragments=( + "PASS=0 FAIL=1", + "unexpected sample 1: order_id=1, line_total=20", + "missing sample 1: order_id=1, line_total=99", + ), + ), + ), + ids=lambda case: case.description, +) +def test_given_chained_models_sharing_cte_names_when_testing_then_each_model_is_inlined_once( + tmp_path: Path, test_case: ModelInliningE2ETestCase +) -> None: + project: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="model_inlining", + repo_files=build_shared_cte_name_chain_project_files( + sql_analysis_enabled=test_case.sql_analysis_enabled, + expected_order_lines_sql=test_case.expected_order_lines_sql, + ), + ) + + tested: CompletedProcess[str] = run_sqb(command=("--no-color", "test"), project_dir=project) + compiled: CompletedProcess[str] = run_sqb( + command=("--no-color", "compile"), project_dir=project + ) + + output: str = tested.stdout + tested.stderr + assert tested.returncode == test_case.expected_exit_code, output + for fragment in test_case.expected_output_fragments: + assert fragment in output, output + assert compiled.returncode == 0, compiled.stdout + compiled.stderr + for artifact_root in ("run", "compiled"): + sql: str = next( + (project / "target" / artifact_root / "tests").rglob("order_chain.sql") + ).read_text() + for fragment in _SINGLE_OCCURRENCE_FRAGMENTS: + assert sql.count(fragment) == 1, f"{fragment!r} in {artifact_root}:\n{sql}" + for fragment in _EXPECTED_FRAGMENTS: + assert fragment in sql, sql + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/integration/src/sqlbuild/compiler/pipeline/test_main.py b/tests/integration/src/sqlbuild/compiler/pipeline/test_main.py index f7dee2a733..505837d6d5 100644 --- a/tests/integration/src/sqlbuild/compiler/pipeline/test_main.py +++ b/tests/integration/src/sqlbuild/compiler/pipeline/test_main.py @@ -692,7 +692,7 @@ def test_given_project_with_defer_to_when_compiling_then_resolves_refs_to_deferr "test_case", [ SqlAnalysisChainCompileTargetIntegrationTestCase( - description="chain test compile target uses flat generated ctes", + description="chain test compile target defines each model once as a generated cte", project_files={ "sqlbuild_project.toml": ( 'name = "demo"\n' @@ -729,14 +729,17 @@ def test_given_project_with_defer_to_when_compiling_then_resolves_refs_to_deferr "WITH __source__raw AS (", "__ref__stg_orders AS (", "FROM __source__raw", - "local_helper AS (SELECT 1 AS one),\n", - "__actual__fact_orders AS (SELECT id, amount + one AS adjusted " - "FROM __ref__stg_orders CROSS JOIN local_helper)", + "__ref__fact_orders AS (WITH local_helper AS (SELECT 1 AS one) " + "SELECT id, amount + one AS adjusted FROM __ref__stg_orders " + "CROSS JOIN local_helper),\n", + "__actual__stg_orders AS (SELECT * FROM __ref__stg_orders)", + "__actual__fact_orders AS (SELECT * FROM __ref__fact_orders)", "FROM __ref__stg_orders", "'stg_orders' AS model_name", ), unexpected_fragments=( "__ref__stg_orders AS (WITH", + "\nlocal_helper AS (", "__REF(", "__SOURCE(", "\n\nWITH", diff --git a/tests/integration/src/sqlbuild/executor/testing/test_sql_test_build_parity.py b/tests/integration/src/sqlbuild/executor/testing/test_sql_test_build_parity.py index be1fb73399..f00fdeaf43 100644 --- a/tests/integration/src/sqlbuild/executor/testing/test_sql_test_build_parity.py +++ b/tests/integration/src/sqlbuild/executor/testing/test_sql_test_build_parity.py @@ -64,29 +64,29 @@ def test_given_fixture_project_test_when_rendering_then_whole_build_sql_appears_ "test_case", [ SqlTestBuildParityTestCase( - description="CTE bodies with comments, quotes and dollar quotes are lifted verbatim", + description="CTE bodies with comments, quotes and dollar quotes stay verbatim", test_name="test_stg_orders", model_name="stg_orders", - expected_whole_body_rendered=False, + expected_whole_body_rendered=True, expected_verbatim_fragments=( - "base AS (\n -- a comment with an unmatched ) parenthesis\n" + "__ref__stg_orders AS (WITH base AS (\n" + " -- a comment with an unmatched ) parenthesis\n" " SELECT id AS order_id, amount, 'it''s (fine)' AS note /* ( */\n" - " FROM __source__raw_orders)", + " FROM __source__raw_orders\n),", "tagged AS (SELECT order_id, amount, note, $$a ) b$$ AS tag FROM base)", - "__actual__stg_orders AS (SELECT order_id, amount, note, tag FROM tagged)", + "__actual__stg_orders AS (SELECT * FROM __ref__stg_orders)", ), ), SqlTestBuildParityTestCase( - description="nested WITH stays inside its lifted CTE beside a colliding fixture CTE", + description="model CTEs stay in the model's scope beside a same-named fixture CTE", test_name="test_orders", model_name="orders", - expected_whole_body_rendered=False, + expected_whole_body_rendered=True, expected_verbatim_fragments=( - "totals AS (\n" - " WITH ranked AS (SELECT order_id, amount, tag FROM __ref__stg_orders)\n" - " SELECT order_id, amount * 2 AS doubled, tag FROM ranked)", - "helper_rows AS (SELECT order_id, doubled, tag FROM totals)", - "__actual__orders AS (SELECT order_id, doubled, tag FROM helper_rows)", + "__ref__orders AS (WITH totals AS (\n" + " WITH ranked AS (SELECT order_id, amount, tag FROM __ref__stg_orders)\n", + "helper_rows AS (SELECT 1 AS order_id, 20 AS doubled, 'x' AS tag)", + "__actual__orders AS (SELECT * FROM __ref__orders)", ), ), ], @@ -107,6 +107,8 @@ def test_given_model_ctes_when_rendering_then_build_sql_slices_appear_verbatim( ) assert build_sql_matches_test_body(build_sql=build_sql, test_body=test_body), test_body + assert (test_body in rendered_sql) is test_case.expected_whole_body_rendered, rendered_sql + assert rendered_sql.count(test_body) == 1, rendered_sql assert all(fragment in rendered_sql for fragment in test_case.expected_verbatim_fragments), ( rendered_sql ) @@ -126,7 +128,8 @@ def test_given_shared_cte_name_in_chain_when_rendering_tsql_then_rename_runs_lik nested_sql: str = build_sql_test_comparison_sql(test_entry=entry, sql_analysis_dialect="duckdb") assert "__sqb_cte_0 AS (SELECT id + 1 AS id FROM __ref__stg)" in renamed_sql - assert "__actual__mart AS (SELECT __sqb_cte_0.id FROM __sqb_cte_0)" in renamed_sql + assert "__ref__mart AS (SELECT __sqb_cte_0.id FROM __sqb_cte_0)" in renamed_sql + assert "__actual__mart AS (SELECT * FROM __ref__mart)" in renamed_sql assert "AS (WITH" not in renamed_sql assert ( comparison_rows(adapter=adapter, connection=connection, sql=renamed_sql) From 5024fe5e3b81ddd26e911086f061da4857fdc503 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 6 Oct 2026 15:34:52 +0100 Subject: [PATCH 02/10] perf(test): emit SQL unit-test helper CTEs once at top level Every mock CTE carried a copy of every test-defined helper CTE, so a helper such as `expected_rows` used only by an assertion was repeated in each fixture. Mocks now contain only their own SQL, and the helpers a mock reads are defined once in the test's top-level WITH. A mock inlined into model SQL, where a model CTE could shadow a top-level helper, still carries only the helpers it reads. --- .../_helpers/sql_tests/helper_scope.rs | 88 ++++++++++++++++--- .../compiler/_helpers/sql_tests/planning.rs | 63 +++++++++---- .../commands/main/test/test_model_inlining.py | 4 + 3 files changed, 129 insertions(+), 26 deletions(-) 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 f26728038a..c7fe78f112 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 @@ -2,10 +2,11 @@ use std::collections::{BTreeMap, HashMap, HashSet}; +use crate::compiler::_helpers::sql_tests::cte_sql::{parenthesized_sql, with_leading_ctes}; use crate::compiler::_helpers::sql_tests::markers::{in_protected_range, protected_ranges}; use crate::compiler::_helpers::sql_tests::planning::{ DBT_REF_PREFIX, REF_PREFIX, SEED_PREFIX, SOURCE_PREFIX, SqlTestPatterns, TestFixtures, - compile_error, with_helper_ctes, + compile_error, }; use crate::constants::{ QUOTED_IDENTIFIER_DELIMITER_BYTES, SQL_TEST_ACTUAL_CTE, SQL_TEST_ACTUAL_CTE_PREFIX, @@ -26,6 +27,13 @@ pub(crate) struct ScopeMock { tokens: Vec, } +/// One test CTE another test CTE reads: a mock, or a helper when `mock_name` is `None`. +pub(crate) struct ScopeDependency<'a> { + pub(crate) generated_name: &'a str, + pub(crate) mock_name: Option<&'a str>, + pub(crate) sql: &'a str, +} + #[derive(Clone, Copy, PartialEq, Eq, Hash)] enum ScopeNode { Helper(usize), @@ -35,6 +43,7 @@ enum ScopeNode { /// Name references between one test's helper and mock CTEs, tokenized once per test. #[derive(Default)] pub(crate) struct ScopeGraph { + helpers: Vec<(String, String)>, helper_tokens: Vec>, helper_indexes: HashMap, mocks: Vec, @@ -83,12 +92,17 @@ impl ScopeGraph { mocks.push(ScopeMock { generated_name: format!("{prefix}{name}"), mock_name: name.clone(), - sql: with_helper_ctes(body, &fixtures.helpers), + sql: body.clone(), tokens: identifier_tokens(body, patterns), }); } } Self { + helpers: fixtures + .helpers + .iter() + .map(|cte| (cte.name.clone(), cte.sql_body.clone())) + .collect(), helper_tokens: fixtures .helpers .iter() @@ -142,28 +156,82 @@ impl ScopeGraph { order.ordered } - /// The generated CTE body of one mock, with every helper CTE in scope. + /// The authored body of one mock, placed as its own top-level CTE. pub(crate) fn mock_sql(&self, generated_name: &str) -> Option<&str> { self.mocks_by_generated_name .get(generated_name) .map(|index| self.mocks[*index].sql.as_str()) } - /// The parenthesized body of one mock inlined in place of its relation marker. + /// One mock inlined in model scope, carrying the helpers it reads so no model CTE shadows them. pub(crate) fn inlined_mock_sql(&self, generated_name: &str) -> Option { - self.mock_sql(generated_name).map(|sql| format!("({sql})")) + let mock = &self.mocks[*self.mocks_by_generated_name.get(generated_name)?]; + Some(parenthesized_sql( + &self.with_read_helpers(&mock.sql, &mock.tokens), + )) + } + + /// A table-function fixture inlined into model SQL, with the helpers it reads. + pub(crate) fn inlined_fixture_sql(&self, sql: &str, patterns: &SqlTestPatterns) -> String { + parenthesized_sql(&self.with_read_helpers(sql, &identifier_tokens(sql, patterns))) + } + + fn with_read_helpers(&self, sql: &str, tokens: &[String]) -> String { + let helpers: Vec<(String, String)> = self + .closure(tokens) + .into_iter() + .filter_map(|node| match node { + ScopeNode::Helper(index) => Some(self.helpers[index].clone()), + ScopeNode::Mock(_) => None, + }) + .collect(); + with_leading_ctes(&helpers, sql) + } + + /// Helpers and mocks one top-level mock reads, transitively and dependencies first. + pub(crate) fn mock_dependencies(&self, generated_name: &str) -> Vec> { + let Some(root) = self.mocks_by_generated_name.get(generated_name).copied() else { + return Vec::new(); + }; + self.dependencies(self.closure(&self.mocks[root].tokens), root) } - /// Mocks that one mock's body reads through helpers or by name, dependencies first. - pub(crate) fn mock_dependencies(&self, generated_name: &str) -> Vec<&ScopeMock> { + /// Top-level CTEs an inlined mock needs: the mocks it reads and everything they read. + pub(crate) fn inlined_mock_dependencies( + &self, + generated_name: &str, + ) -> Vec> { let Some(root) = self.mocks_by_generated_name.get(generated_name).copied() else { return Vec::new(); }; - self.closure(&self.mocks[root].tokens) + let mut order = ScopeOrder { + graph: self, + visited: HashSet::new(), + ordered: Vec::new(), + }; + for node in self.closure(&self.mocks[root].tokens) { + if matches!(node, ScopeNode::Mock(index) if index != root) { + order.visit(node); + } + } + self.dependencies(order.ordered, root) + } + + fn dependencies(&self, nodes: Vec, root: usize) -> Vec> { + nodes .into_iter() .filter_map(|node| match node { - ScopeNode::Mock(index) if index != root => Some(&self.mocks[index]), - _ => None, + ScopeNode::Mock(index) if index == root => None, + ScopeNode::Mock(index) => Some(ScopeDependency { + generated_name: &self.mocks[index].generated_name, + mock_name: Some(&self.mocks[index].mock_name), + sql: &self.mocks[index].sql, + }), + ScopeNode::Helper(index) => Some(ScopeDependency { + generated_name: &self.helpers[index].0, + mock_name: None, + sql: &self.helpers[index].1, + }), }) .collect() } 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 0dfeae02ca..6018ca46f8 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs @@ -279,6 +279,12 @@ pub(crate) struct TestFixtures { assertions: Vec<(String, String)>, } +impl TestFixtures { + fn is_helper(&self, name: &str) -> bool { + self.helpers.iter().any(|helper| helper.name == name) + } +} + struct DirectTestPlan { name: String, mode: String, @@ -492,6 +498,13 @@ impl GeneratedCteState { Ok(()) } + /// Add a test helper read by a mock; model CTEs of the same name stay nested apart from it. + fn insert_helper(&mut self, name: &str, sql: &str) { + if !self.generated.iter().any(|(existing, _)| existing == name) { + self.generated.push((name.to_string(), sql.to_string())); + } + } + fn mock_target(&mut self, request: MockTargetRequest<'_>) -> Result, String> { if !request.mocks.contains_key(request.referenced_name) { return Ok(None); @@ -511,10 +524,14 @@ impl GeneratedCteState { ))); } for dependency in request.fixtures.scope.mock_dependencies(&generated_name) { - self.reachable.insert(dependency.mock_name.clone()); + let Some(mock_name) = dependency.mock_name else { + self.insert_helper(dependency.generated_name, dependency.sql); + continue; + }; + self.reachable.insert(mock_name.to_string()); self.insert( - &dependency.generated_name, - &dependency.sql, + dependency.generated_name, + dependency.sql, request.file_label, )?; } @@ -805,7 +822,7 @@ fn plan_model_test( let (fixture_resolved_sql, reached_table_functions) = resolve_table_function_fixtures( query_sql, &fixtures.mock_table_functions, - &fixtures.helpers, + &fixtures.scope, &context.patterns, )?; reachable_mocks.extend(reached_table_functions); @@ -878,7 +895,7 @@ fn plan_model_test( let (fixture_resolved_sql, reached_table_functions) = resolve_table_function_fixtures( assertion_sql, &fixtures.mock_table_functions, - &fixtures.helpers, + &fixtures.scope, &context.patterns, )?; reachable_mocks.extend(reached_table_functions); @@ -1225,7 +1242,11 @@ fn analyze_and_resolve_sql( let target = if function_name == REF_FUNCTION { if let Some(chain_sql) = request.resolved_chain.get(&referenced_name) { for (name, sql) in &chain_sql.generated_ctes { - generated_state.insert(name, sql, request.file_label)?; + if request.fixtures.is_helper(name) { + generated_state.insert_helper(name, sql); + } else { + generated_state.insert(name, sql, request.file_label)?; + } } let generated_name = format!("{REF_PREFIX}{referenced_name}"); generated_state.insert( @@ -1405,7 +1426,7 @@ fn resolve_textual_sql(request: TextualResolutionRequest<'_>) -> Result) -> Result = Vec::new(); for generated_name in inlined { - let dependencies = request.fixtures.scope.mock_dependencies(&generated_name); - reached.extend(dependencies.iter().map(|mock| mock.mock_name.clone())); + let dependencies = request + .fixtures + .scope + .inlined_mock_dependencies(&generated_name); + reached.extend( + dependencies + .iter() + .filter_map(|dependency| dependency.mock_name.map(str::to_string)), + ); mock_ctes = with_unique_ctes( mock_ctes, - dependencies - .into_iter() - .map(|mock| (mock.generated_name.clone(), mock.sql.clone())), + dependencies.into_iter().map(|dependency| { + ( + dependency.generated_name.to_string(), + dependency.sql.to_string(), + ) + }), ); } Ok(TextualResolution { @@ -1432,7 +1463,7 @@ fn resolve_textual_sql(request: TextualResolutionRequest<'_>) -> Result, - helpers: &[CteInput], + scope: &ScopeGraph, patterns: &SqlTestPatterns, ) -> Result<(String, HashSet), String> { if fixtures.is_empty() { @@ -1445,7 +1476,7 @@ fn resolve_table_function_fixtures( |name, _call_suffix| { fixtures .get(name) - .map(|body| (format!("({})", with_helper_ctes(body, helpers)), true)) + .map(|body| (scope.inlined_fixture_sql(body, patterns), true)) }, ) } @@ -1658,8 +1689,8 @@ fn helper_with_clause(helpers: &[CteInput]) -> String { ) } -/// Prefix every helper CTE to a query; mock and direct-test bodies all see helpers this way. -pub(crate) fn with_helper_ctes(sql: &str, helpers: &[CteInput]) -> String { +/// Prefix every helper CTE to a direct test's actual and expected queries. +fn with_helper_ctes(sql: &str, helpers: &[CteInput]) -> String { if helpers.is_empty() { sql.to_string() } else { diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_model_inlining.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_model_inlining.py index 81c6dcfd37..a491ed1118 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_model_inlining.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_model_inlining.py @@ -7,6 +7,7 @@ from tests.e2e.src.sqlbuild.cli.commands.main.test._test_types import ModelInliningE2ETestCase from tests.e2e.src.sqlbuild.cli.commands.main.test.helpers import ( + ORDER_TOTALS_EXPECTED_ROWS_SQL, build_shared_cte_name_chain_project_files, ) from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project, run_sqb @@ -16,6 +17,8 @@ "__ref__order_totals AS (", "amount * 2 AS line_total", "line_total + 1 AS order_total", + "base_rows AS (SELECT 1 AS order_id, 10 AS amount)", + f"expected_rows AS ({ORDER_TOTALS_EXPECTED_ROWS_SQL})", ) _EXPECTED_FRAGMENTS: tuple[str, ...] = ( "__actual__order_lines AS (SELECT * FROM __ref__order_lines)", @@ -85,6 +88,7 @@ def test_given_chained_models_sharing_cte_names_when_testing_then_each_model_is_ assert sql.count(fragment) == 1, f"{fragment!r} in {artifact_root}:\n{sql}" for fragment in _EXPECTED_FRAGMENTS: assert fragment in sql, sql + assert "WITH expected_rows" not in sql, sql if __name__ == "__main__": From 7f3cd8c31b6cf157d024933af83997ac02402709 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 6 Oct 2026 15:34:56 +0100 Subject: [PATCH 03/10] feat(test): report each SQL test's duration_ms in JSON output `sqb test --json-output` checks had no timing, although the text output shows one per test. Each SQL test check now reports `duration_ms` from the same lifecycle record, matching the unit used by JSON assets. --- .../sqlbuild/references/docs/cli/test.md | 3 +- .../_helpers/execution_result_document.py | 1 + .../cli/commands/main/test/_test_types.py | 10 +++ .../main/test/test_json_output_duration.py | 61 +++++++++++++++++++ website/src/content/docs/docs/cli/test.mdx | 3 +- 5 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 tests/e2e/src/sqlbuild/cli/commands/main/test/test_json_output_duration.py diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/test.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/test.md index 909ceb1c74..4c3984075a 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/test.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/test.md @@ -33,7 +33,8 @@ each matching `TEST` template; case names are not model selectors. Text output identifies each case as ` []` and includes its source path and safe typed parameters. Structured JSON emits one check per case with stable source/block/case identity, declared parameter types and nullability, typed values, and a content fingerprint. Exact decimals -are strings in JSON so their scale is preserved. +are strings in JSON so their scale is preserved. Each test check in JSON output reports its +`duration_ms`. ## Examples diff --git a/src/sqlbuild/cli/output/_helpers/execution_result_document.py b/src/sqlbuild/cli/output/_helpers/execution_result_document.py index 3252087a36..525c6d98d2 100644 --- a/src/sqlbuild/cli/output/_helpers/execution_result_document.py +++ b/src/sqlbuild/cli/output/_helpers/execution_result_document.py @@ -825,6 +825,7 @@ def _format_sql_test_checks( **_sql_test_case_metadata(result), "passed": result.outcome == SqlTestOutcome.PASS, "status": result.outcome.value, + "duration_ms": _result_duration(result=result, fallback=None), "asset_name": _sql_test_asset_name(result), "error_code": result.error_code, "error_help": result.error_help, 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 04be200740..665e6a98d7 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 @@ -251,3 +251,13 @@ class ModelInliningE2ETestCase: expected_order_lines_sql: str expected_exit_code: int expected_output_fragments: tuple[str, ...] + + +@dataclass(frozen=True) +class JsonDurationE2ETestCase: + """A SQL test whose structured JSON output must report its duration.""" + + description: str + expected_order_lines_sql: str + expected_exit_code: int + expected_status: str diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_json_output_duration.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_json_output_duration.py new file mode 100644 index 0000000000..c5d1667b05 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_json_output_duration.py @@ -0,0 +1,61 @@ +"""`sqb test --json-output` reports each SQL test's duration through the real DuckDB CLI.""" + +import json +from pathlib import Path +from subprocess import CompletedProcess + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.test._test_types import JsonDurationE2ETestCase +from tests.e2e.src.sqlbuild.cli.commands.main.test.helpers import ( + build_shared_cte_name_chain_project_files, +) +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project, run_sqb + + +@pytest.mark.parametrize( + "test_case", + ( + JsonDurationE2ETestCase( + description="passing chain test reports its duration", + expected_order_lines_sql="SELECT 1 AS order_id, 20 AS line_total", + expected_exit_code=0, + expected_status="pass", + ), + JsonDurationE2ETestCase( + description="failing chain test reports its duration", + expected_order_lines_sql="SELECT 1 AS order_id, 99 AS line_total", + expected_exit_code=1, + expected_status="fail", + ), + ), + ids=lambda case: case.description, +) +def test_given_sql_test_when_writing_json_output_then_check_reports_duration_ms( + tmp_path: Path, test_case: JsonDurationE2ETestCase +) -> None: + project: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="json_duration", + repo_files=build_shared_cte_name_chain_project_files( + sql_analysis_enabled=True, + expected_order_lines_sql=test_case.expected_order_lines_sql, + ), + ) + json_path: Path = tmp_path / "result.json" + + result: CompletedProcess[str] = run_sqb( + command=("--no-color", "test", "--json-output", str(json_path)), project_dir=project + ) + + assert result.returncode == test_case.expected_exit_code, result.stdout + result.stderr + checks: list[dict[str, object]] = json.loads(json_path.read_text())["checks"] + assert [(check["name"], check["status"]) for check in checks] == [ + ("order_chain", test_case.expected_status) + ], checks + duration: object = checks[0].get("duration_ms") + assert isinstance(duration, int) and duration >= 0, checks + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/website/src/content/docs/docs/cli/test.mdx b/website/src/content/docs/docs/cli/test.mdx index faefa6a0e0..30741e865c 100644 --- a/website/src/content/docs/docs/cli/test.mdx +++ b/website/src/content/docs/docs/cli/test.mdx @@ -32,7 +32,8 @@ each matching `TEST` template; case names are not model selectors. Text output identifies each case as ` []` and includes its source path and safe typed parameters. Structured JSON emits one check per case with stable source/block/case identity, declared parameter types and nullability, typed values, and a content fingerprint. Exact decimals -are strings in JSON so their scale is preserved. +are strings in JSON so their scale is preserved. Each test check in JSON output reports its +`duration_ms`. ## Examples From 526fbee7618844beeceb2e6ec3f1f3be10b13c3e Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 6 Oct 2026 17:30:07 +0100 Subject: [PATCH 04/10] fix(test): report every parameterized SQL test case in JSON output JSON output looked up SQL test results by test name only, so parameterized cases, whose events carry the path, block and case identity, were dropped from checks. Use the shared sql_test_resource_id for both. --- .../_helpers/execution_result_document.py | 12 +++-- .../main/test/test_json_output_duration.py | 45 ++++++++++++++++++- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/sqlbuild/cli/output/_helpers/execution_result_document.py b/src/sqlbuild/cli/output/_helpers/execution_result_document.py index 525c6d98d2..dfea846d8c 100644 --- a/src/sqlbuild/cli/output/_helpers/execution_result_document.py +++ b/src/sqlbuild/cli/output/_helpers/execution_result_document.py @@ -50,6 +50,7 @@ ScenarioSnapshotCaptureRunResult, ) from sqlbuild.executor.scenario.types import ScenarioLocalRunStatus +from sqlbuild.executor.testing.main.resource_id import sql_test_resource_id from sqlbuild.executor.testing.models import ( SqlTestDifferenceSample, SqlTestExecutionResult, @@ -838,9 +839,12 @@ def _format_sql_test_checks( def _sql_test_check_id(result: SqlTestExecutionResult) -> str: - if result.case_name is None or result.source_path is None or result.block_index is None: - return f"sql_test:{result.test_name}" - return f"sql_test:{result.source_path.as_posix()}:{result.block_index}:{result.case_name}" + return sql_test_resource_id( + test_name=result.test_name, + source_path=result.source_path, + block_index=result.block_index, + case_name=result.case_name, + ) def _sql_test_case_metadata(result: SqlTestExecutionResult) -> dict[str, object]: @@ -1181,7 +1185,7 @@ def _result_resource_identity(*, result: object) -> tuple[str | None, str | None resource_id = f"check:{result.node_name}" elif isinstance(result, SqlTestExecutionResult): resource_name = result.test_name - resource_id = f"sql_test:{result.test_name}" + resource_id = _sql_test_check_id(result) elif isinstance(result, AuditExecutionResult): resource_name = result.audit_name resource_id = _audit_check_id(result) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_json_output_duration.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_json_output_duration.py index c5d1667b05..91a6979a4a 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_json_output_duration.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_json_output_duration.py @@ -6,8 +6,12 @@ import pytest -from tests.e2e.src.sqlbuild.cli.commands.main.test._test_types import JsonDurationE2ETestCase +from tests.e2e.src.sqlbuild.cli.commands.main.test._test_types import ( + JsonDurationE2ETestCase, + ParameterizedJsonE2ETestCase, +) from tests.e2e.src.sqlbuild.cli.commands.main.test.helpers import ( + build_parameterized_test_project_files, build_shared_cte_name_chain_project_files, ) from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project, run_sqb @@ -57,5 +61,44 @@ def test_given_sql_test_when_writing_json_output_then_check_reports_duration_ms( assert isinstance(duration, int) and duration >= 0, checks +@pytest.mark.parametrize( + "test_case", + ( + ParameterizedJsonE2ETestCase( + description="every parameterized case is a check with its duration", + expected_checks=( + ("customer_status", "customer_only"), + ("order_status", "closed_case"), + ("order_status", "open_case"), + ), + ), + ), + ids=lambda case: case.description, +) +def test_given_parameterized_cases_when_writing_json_output_then_each_case_reports_duration_ms( + tmp_path: Path, test_case: ParameterizedJsonE2ETestCase +) -> None: + project: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="json_cases", + repo_files=build_parameterized_test_project_files(), + ) + json_path: Path = tmp_path / "result.json" + + result: CompletedProcess[str] = run_sqb( + command=("--no-color", "test", "--json-output", str(json_path)), project_dir=project + ) + + assert result.returncode == 0, result.stdout + result.stderr + checks: list[dict[str, object]] = json.loads(json_path.read_text())["checks"] + assert sorted((str(check["parent_name"]), str(check["case_name"])) for check in checks) == list( + test_case.expected_checks + ), checks + for check in checks: + duration: object = check.get("duration_ms") + assert isinstance(duration, int) and duration >= 0, check + assert check["status"] == "pass", check + + if __name__ == "__main__": pytest.main([__file__, "-vv"]) From e060543e31bb2ef7ef7c9c4dbc28fcf02f3455cd Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 6 Oct 2026 17:30:07 +0100 Subject: [PATCH 05/10] fix(test): isolate SQL unit-test helper names from model CTEs Helpers placed at the top level of the test query could be read by a nested model CTE or bare table of the same name on warehouses where outer CTEs win or are ambiguous. With analysis, helpers now get reserved __helper__ CTE names and test-authored readers are rewritten through the AST; without analysis they stay inline in the fixtures that read them. Rendering refuses any nested CTE that reuses a top-level name, a test CTE that redefines a helper is a compile error, and model bodies drop a trailing terminator in planning so downstream steps share one definition. --- .../compiler/_helpers/sql_tests/cte_rename.rs | 57 +++ .../_helpers/sql_tests/helper_names.rs | 76 ++++ .../_helpers/sql_tests/helper_scope.rs | 397 +++++++++++++----- .../src/compiler/_helpers/sql_tests/mod.rs | 1 + .../compiler/_helpers/sql_tests/planning.rs | 244 +++++++---- .../compiler/_helpers/sql_tests/rendering.rs | 206 ++++++--- .../sqlbuild-analysis/src/compiler/tests.rs | 2 + .../src/compiler/tests/helpers.rs | 262 ++++++++++-- .../tests/test_sql_test_name_isolation.rs | 142 +++++++ .../compiler/tests/test_sql_test_rendering.rs | 6 +- .../src/compiler/tests/test_types.rs | 38 ++ .../references/docs/concepts/testing.md | 7 +- .../cli/commands/main/test/_test_types.py | 39 ++ .../cli/commands/main/test/helpers.py | 70 +++ .../main/test/test_authored_sql_parity.py | 9 +- .../main/test/test_helper_name_isolation.py | 158 +++++++ .../commands/main/test/test_helper_scope.py | 6 +- .../commands/main/test/test_model_inlining.py | 13 +- .../cli/commands/main/test/test_test.py | 6 +- .../src/sqlbuild/executor/testing/helpers.py | 10 +- .../sql_test_assembly/test_plan_test.py | 38 +- .../content/docs/docs/concepts/testing.mdx | 7 +- 22 files changed, 1461 insertions(+), 333 deletions(-) create mode 100644 crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/helper_names.rs create mode 100644 crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_name_isolation.rs create mode 100644 tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_name_isolation.py diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_rename.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_rename.rs index 92833a274b..7e8aa4f839 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_rename.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_rename.rs @@ -82,6 +82,63 @@ pub(crate) fn rename_ctes( preserves_tokens((sql, &tokens), (&renamed, &renamed_tokens), &replaced).then_some(renamed) } +/// Case-folded names of every CTE a statement defines at any depth; `None` when unscannable. +pub(crate) fn defined_cte_keys(sql: &str, dialect: SliceDialect) -> Option> { + let tokens = match tokens(sql, dialect) { + Ok(tokens) => tokens, + Err(_) => return None, + }; + let code: Vec<&Token> = tokens.iter().filter(|token| !token.comment).collect(); + let text = |index: usize| code.get(index).map(|token| &sql[token.start..token.end]); + let keyword = |index: usize, word: &str| { + code.get(index) + .and_then(|token| token.key.as_deref()) + .is_some_and(|key| key == word) + }; + let mut keys: Vec = Vec::new(); + for (index, token) in code.iter().enumerate() { + let Some(key) = token.key.as_deref() else { + continue; + }; + let opens_list = index > 0 + && (keyword(index - 1, "with") + || keyword(index - 1, "recursive") + || text(index - 1) == Some(",")); + if !opens_list { + continue; + } + let mut next = index + 1; + if text(next) == Some("(") { + let mut depth = 0usize; + while let Some(part) = text(next) { + depth = match part { + "(" => depth + 1, + ")" => depth.saturating_sub(1), + _ => depth, + }; + next += 1; + if depth == 0 { + break; + } + } + } + if !keyword(next, "as") { + continue; + } + next += 1; + if keyword(next, "not") { + next += 1; + } + if keyword(next, "materialized") { + next += 1; + } + if text(next) == Some("(") { + keys.push(key.to_string()); + } + } + Some(keys) +} + /// How one occurrence of a renamed CTE's name is used. #[derive(Clone, Copy, Debug, PartialEq, Eq)] enum Usage { diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/helper_names.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/helper_names.rs new file mode 100644 index 0000000000..77ed5bcf9c --- /dev/null +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/helper_names.rs @@ -0,0 +1,76 @@ +//! Reserved top-level names for test-defined helper CTEs and the rewrite of their references. + +use std::collections::{HashMap, HashSet}; + +use polyglot_sql::{Dialect, Expression, ExpressionWalk}; + +pub(crate) const HELPER_PREFIX: &str = "__helper__"; + +/// The reserved top-level CTE name of one test-defined helper. +pub(crate) fn helper_cte_name(name: &str) -> String { + format!("{HELPER_PREFIX}{name}") +} + +/// Point unqualified relation references at reserved helper names by AST span, keeping their alias. +pub(crate) fn rename_helper_references( + sql: &str, + helpers: &HashMap, + dialect: &Dialect, +) -> Option { + let statements = match dialect.parse(sql) { + Ok(statements) => statements, + Err(_) => return None, + }; + let [statement] = statements.as_slice() else { + return None; + }; + let char_offsets: Vec = sql + .char_indices() + .map(|(offset, _)| offset) + .chain(std::iter::once(sql.len())) + .collect(); + let mut aliased: HashSet = HashSet::new(); + for node in statement.dfs() { + if let Expression::Alias(alias) = node + && let Expression::Table(table) = &alias.this + && let Some(span) = table.name.span + { + aliased.insert(span.start); + } + } + let mut edits: Vec<(usize, usize, String)> = Vec::new(); + for node in statement.dfs() { + let Expression::Table(table) = node else { + continue; + }; + if table.schema.is_some() || table.catalog.is_some() { + continue; + } + let Some(reserved) = helpers.get(&table.name.name.to_lowercase()) else { + continue; + }; + let span = table.name.span?; + let start = *char_offsets.get(span.start)?; + let end = *char_offsets.get(span.end)?; + let authored = sql.get(start..end)?; + let replacement = if table.alias.is_some() || aliased.contains(&span.start) { + reserved.clone() + } else { + format!("{reserved} AS {authored}") + }; + edits.push((start, end, replacement)); + } + edits.sort_by_key(|(start, _, _)| *start); + let mut rewritten = String::with_capacity(sql.len() + edits.len() * HELPER_PREFIX.len()); + let mut cursor = 0; + for (start, end, replacement) in edits { + if start < cursor { + return None; + } + rewritten.push_str(&sql[cursor..start]); + rewritten.push_str(&replacement); + cursor = end; + } + rewritten.push_str(&sql[cursor..]); + Some(rewritten) +} 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 c7fe78f112..b5501d357a 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 @@ -2,11 +2,18 @@ use std::collections::{BTreeMap, HashMap, HashSet}; -use crate::compiler::_helpers::sql_tests::cte_sql::{parenthesized_sql, with_leading_ctes}; +use polyglot_sql::Dialect; + +use crate::compiler::_helpers::sql_tests::cte_rename::defined_cte_keys; +use crate::compiler::_helpers::sql_tests::cte_slices::SliceDialect; +use crate::compiler::_helpers::sql_tests::cte_sql::with_leading_ctes; +use crate::compiler::_helpers::sql_tests::helper_names::{ + helper_cte_name, rename_helper_references, +}; use crate::compiler::_helpers::sql_tests::markers::{in_protected_range, protected_ranges}; use crate::compiler::_helpers::sql_tests::planning::{ - DBT_REF_PREFIX, REF_PREFIX, SEED_PREFIX, SOURCE_PREFIX, SqlTestPatterns, TestFixtures, - compile_error, + DBT_REF_PREFIX, REF_PREFIX, SEED_PREFIX, SOURCE_PREFIX, SqlTestPatterns, TABLE_FUNCTION_PREFIX, + TestFixtures, compile_error, }; use crate::constants::{ QUOTED_IDENTIFIER_DELIMITER_BYTES, SQL_TEST_ACTUAL_CTE, SQL_TEST_ACTUAL_CTE_PREFIX, @@ -27,6 +34,15 @@ pub(crate) struct ScopeMock { tokens: Vec, } +/// One test-defined helper: its authored name and body, and its top-level CTE name and body. +struct ScopeHelper { + name: String, + original: String, + cte_name: String, + sql: String, + tokens: Vec, +} + /// One test CTE another test CTE reads: a mock, or a helper when `mock_name` is `None`. pub(crate) struct ScopeDependency<'a> { pub(crate) generated_name: &'a str, @@ -34,6 +50,26 @@ pub(crate) struct ScopeDependency<'a> { pub(crate) sql: &'a str, } +/// Where helper CTEs live in the rendered test query. +#[derive(Clone, Copy, Default, PartialEq, Eq)] +enum HelperPlacement { + /// Each top-level test CTE carries the helpers it reads in its own WITH. + #[default] + Inline, + /// Helpers are top-level CTEs under the names their readers were rewritten to. + TopLevel, +} + +/// Inputs that decide how one test's helpers are named and placed. +pub(crate) struct ScopeRequest<'a> { + pub(crate) fixtures: &'a TestFixtures, + pub(crate) patterns: &'a SqlTestPatterns, + pub(crate) file_label: &'a str, + pub(crate) rename_dialect: Option<&'a Dialect>, + pub(crate) flat_dialect: bool, + pub(crate) slice_dialect: SliceDialect, +} + #[derive(Clone, Copy, PartialEq, Eq, Hash)] enum ScopeNode { Helper(usize), @@ -43,12 +79,13 @@ enum ScopeNode { /// Name references between one test's helper and mock CTEs, tokenized once per test. #[derive(Default)] pub(crate) struct ScopeGraph { - helpers: Vec<(String, String)>, - helper_tokens: Vec>, + placement: HelperPlacement, + helpers: Vec, helper_indexes: HashMap, mocks: Vec, mock_indexes: HashMap, mocks_by_generated_name: HashMap, + rewritten: HashMap, } /// Graph nodes in dependency-first order, built by depth-first traversal. @@ -56,12 +93,16 @@ struct ScopeOrder<'a> { graph: &'a ScopeGraph, visited: HashSet, ordered: Vec, + through_mocks: bool, } impl ScopeOrder<'_> { fn visit_tokens(&mut self, tokens: &[String]) { for token in tokens { if let Some(node) = self.graph.node(token) { + if !self.through_mocks && matches!(node, ScopeNode::Mock(_)) { + continue; + } self.visit(node); } } @@ -78,18 +119,20 @@ impl ScopeOrder<'_> { } impl ScopeGraph { - /// Tokenize every helper body and authored mock body of one test. - pub(crate) fn new(fixtures: &TestFixtures, patterns: &SqlTestPatterns) -> Self { + /// Tokenize one test's helpers and mocks and decide how its helpers are named and placed. + pub(crate) fn new(request: &ScopeRequest<'_>) -> Result { + let fixtures = request.fixtures; + let patterns = request.patterns; let mock_groups: [(&str, &BTreeMap); 4] = [ (REF_PREFIX, &fixtures.mock_refs), (SOURCE_PREFIX, &fixtures.mock_sources), (SEED_PREFIX, &fixtures.mock_seeds), (DBT_REF_PREFIX, &fixtures.mock_dbt_refs), ]; - let mut mocks: Vec = Vec::new(); + let mut graph = Self::default(); for (prefix, group) in mock_groups { for (name, body) in group { - mocks.push(ScopeMock { + graph.mocks.push(ScopeMock { generated_name: format!("{prefix}{name}"), mock_name: name.clone(), sql: body.clone(), @@ -97,35 +140,116 @@ impl ScopeGraph { }); } } - Self { - helpers: fixtures - .helpers - .iter() - .map(|cte| (cte.name.clone(), cte.sql_body.clone())) - .collect(), - helper_tokens: fixtures - .helpers - .iter() - .map(|cte| identifier_tokens(&cte.sql_body, patterns)) - .collect(), - helper_indexes: fixtures - .helpers - .iter() - .enumerate() - .map(|(index, cte)| (cte.name.to_ascii_lowercase(), index)) - .collect(), - mock_indexes: mocks - .iter() - .enumerate() - .map(|(index, mock)| (mock.generated_name.to_ascii_lowercase(), index)) - .collect(), - mocks_by_generated_name: mocks - .iter() - .enumerate() - .map(|(index, mock)| (mock.generated_name.clone(), index)) - .collect(), - mocks, + for (index, mock) in graph.mocks.iter().enumerate() { + graph + .mock_indexes + .insert(mock.generated_name.to_ascii_lowercase(), index); + graph + .mocks_by_generated_name + .insert(mock.generated_name.clone(), index); + } + if fixtures.helpers.is_empty() { + return Ok(graph); + } + for (index, cte) in fixtures.helpers.iter().enumerate() { + graph + .helper_indexes + .insert(cte.name.to_ascii_lowercase(), index); + graph.helpers.push(ScopeHelper { + name: cte.name.clone(), + original: cte.sql_body.clone(), + cte_name: cte.name.clone(), + sql: cte.sql_body.clone(), + tokens: identifier_tokens(&cte.sql_body, patterns), + }); + } + let readers = test_readers(fixtures); + graph.reject_shadowed_helpers(&readers, request)?; + if let Some(dialect) = request.rename_dialect + && let Some(rewritten) = graph.rewrite_readers(&readers, dialect, patterns) + { + graph.placement = HelperPlacement::TopLevel; + for helper in &mut graph.helpers { + helper.cte_name = helper_cte_name(&helper.name); + if let Some(sql) = rewritten.get(&helper.original) { + helper.sql = sql.clone(); + } + } + for mock in &mut graph.mocks { + if let Some(sql) = rewritten.get(&mock.sql) { + mock.sql = sql.clone(); + } + } + graph.rewritten = rewritten; + return Ok(graph); } + if request.flat_dialect { + graph.placement = HelperPlacement::TopLevel; + return Ok(graph); + } + let inline: Vec = graph + .mocks + .iter() + .map(|mock| graph.with_read_helpers(&mock.sql, &mock.tokens)) + .collect(); + for (mock, sql) in graph.mocks.iter_mut().zip(inline) { + mock.sql = sql; + } + Ok(graph) + } + + /// Reject a test CTE that redefines a helper's name in its own WITH, which no dialect scopes alike. + fn reject_shadowed_helpers( + &self, + readers: &[(String, &str)], + request: &ScopeRequest<'_>, + ) -> Result<(), String> { + for (label, sql) in readers { + for key in defined_cte_keys(sql, request.slice_dialect).unwrap_or_default() { + if let Some(index) = self.helper_indexes.get(&key) { + return Err(compile_error(&format!( + "SQL test '{}' defines CTE '{}' inside CTE '{label}', which redefines \ + helper CTE '{}'; give one of them a different name", + request.file_label, key, self.helpers[*index].name + ))); + } + } + } + Ok(()) + } + + /// Rewrite helper references in every test CTE, or `None` when any one cannot be rewritten. + fn rewrite_readers( + &self, + readers: &[(String, &str)], + dialect: &Dialect, + patterns: &SqlTestPatterns, + ) -> Option> { + let names: HashMap = self + .helpers + .iter() + .map(|helper| { + ( + helper.name.to_ascii_lowercase(), + helper_cte_name(&helper.name), + ) + }) + .collect(); + let mut rewritten: HashMap = HashMap::new(); + for (_, sql) in readers { + if rewritten.contains_key(*sql) + || !identifier_tokens(sql, patterns) + .iter() + .any(|token| self.helper_indexes.contains_key(token)) + { + continue; + } + rewritten.insert( + (*sql).to_string(), + rename_helper_references(sql, &names, dialect)?, + ); + } + Some(rewritten) } fn node(&self, token: &str) -> Option { @@ -141,102 +265,157 @@ impl ScopeGraph { fn tokens(&self, node: ScopeNode) -> &[String] { match node { - ScopeNode::Helper(index) => &self.helper_tokens[index], + ScopeNode::Helper(index) => &self.helpers[index].tokens, ScopeNode::Mock(index) => &self.mocks[index].tokens, } } - fn closure(&self, tokens: &[String]) -> Vec { + fn closure(&self, tokens: &[String], through_mocks: bool) -> Vec { let mut order = ScopeOrder { graph: self, visited: HashSet::new(), ordered: Vec::new(), + through_mocks, }; order.visit_tokens(tokens); order.ordered } - /// The authored body of one mock, placed as its own top-level CTE. + /// Whether a CTE name is a test helper placed at the top level of the test query. + pub(crate) fn is_top_level_helper(&self, name: &str) -> bool { + self.placement == HelperPlacement::TopLevel + && self.helpers.iter().any(|helper| helper.cte_name == name) + } + + /// The body of one mock as its own top-level CTE. pub(crate) fn mock_sql(&self, generated_name: &str) -> Option<&str> { self.mocks_by_generated_name .get(generated_name) .map(|index| self.mocks[*index].sql.as_str()) } - /// One mock inlined in model scope, carrying the helpers it reads so no model CTE shadows them. - pub(crate) fn inlined_mock_sql(&self, generated_name: &str) -> Option { - let mock = &self.mocks[*self.mocks_by_generated_name.get(generated_name)?]; - Some(parenthesized_sql( - &self.with_read_helpers(&mock.sql, &mock.tokens), - )) - } - - /// A table-function fixture inlined into model SQL, with the helpers it reads. - pub(crate) fn inlined_fixture_sql(&self, sql: &str, patterns: &SqlTestPatterns) -> String { - parenthesized_sql(&self.with_read_helpers(sql, &identifier_tokens(sql, patterns))) + /// One test-authored query as placed in the test query: rewritten, or carrying its helpers. + pub(crate) fn reader_sql(&self, sql: &str, patterns: &SqlTestPatterns) -> String { + match self.placement { + HelperPlacement::TopLevel => self + .rewritten + .get(sql) + .cloned() + .unwrap_or_else(|| sql.to_string()), + HelperPlacement::Inline => { + self.with_read_helpers(sql, &identifier_tokens(sql, patterns)) + } + } } fn with_read_helpers(&self, sql: &str, tokens: &[String]) -> String { let helpers: Vec<(String, String)> = self - .closure(tokens) + .closure(tokens, false) .into_iter() .filter_map(|node| match node { - ScopeNode::Helper(index) => Some(self.helpers[index].clone()), + ScopeNode::Helper(index) => Some(( + self.helpers[index].name.clone(), + self.helpers[index].original.clone(), + )), ScopeNode::Mock(_) => None, }) .collect(); with_leading_ctes(&helpers, sql) } - /// Helpers and mocks one top-level mock reads, transitively and dependencies first. + /// Top-level CTEs one top-level mock reads, transitively and dependencies first. pub(crate) fn mock_dependencies(&self, generated_name: &str) -> Vec> { let Some(root) = self.mocks_by_generated_name.get(generated_name).copied() else { return Vec::new(); }; - self.dependencies(self.closure(&self.mocks[root].tokens), root) + self.closure(&self.mocks[root].tokens, true) + .into_iter() + .filter_map(|node| self.dependency(node, Some(root))) + .collect() } - /// Top-level CTEs an inlined mock needs: the mocks it reads and everything they read. - pub(crate) fn inlined_mock_dependencies( - &self, - generated_name: &str, - ) -> Vec> { - let Some(root) = self.mocks_by_generated_name.get(generated_name).copied() else { - return Vec::new(); - }; - let mut order = ScopeOrder { - graph: self, - visited: HashSet::new(), - ordered: Vec::new(), + /// Top-level CTEs a test-authored query placed inline in model SQL reads. + pub(crate) fn reader_scope(&self, sql: &str, patterns: &SqlTestPatterns) -> HelperScope { + self.collect_scope(self.closure(&identifier_tokens(sql, patterns), true)) + } + + fn collect_scope(&self, nodes: Vec) -> HelperScope { + let mut scope = HelperScope { + ctes: Vec::new(), + reached_mocks: HashSet::new(), }; - for node in self.closure(&self.mocks[root].tokens) { - if matches!(node, ScopeNode::Mock(index) if index != root) { - order.visit(node); + for node in nodes { + if let ScopeNode::Mock(index) = node { + scope + .reached_mocks + .insert(self.mocks[index].mock_name.clone()); + } + if let Some(dependency) = self.dependency(node, None) { + scope.ctes.push(( + dependency.generated_name.to_string(), + dependency.sql.to_string(), + )); } } - self.dependencies(order.ordered, root) + scope } - fn dependencies(&self, nodes: Vec, root: usize) -> Vec> { - nodes - .into_iter() - .filter_map(|node| match node { - ScopeNode::Mock(index) if index == root => None, - ScopeNode::Mock(index) => Some(ScopeDependency { - generated_name: &self.mocks[index].generated_name, - mock_name: Some(&self.mocks[index].mock_name), - sql: &self.mocks[index].sql, - }), - ScopeNode::Helper(index) => Some(ScopeDependency { - generated_name: &self.helpers[index].0, + fn dependency(&self, node: ScopeNode, root: Option) -> Option> { + match node { + ScopeNode::Mock(index) if Some(index) == root => None, + ScopeNode::Mock(index) => Some(ScopeDependency { + generated_name: &self.mocks[index].generated_name, + mock_name: Some(&self.mocks[index].mock_name), + sql: &self.mocks[index].sql, + }), + ScopeNode::Helper(index) => { + (self.placement == HelperPlacement::TopLevel).then(|| ScopeDependency { + generated_name: &self.helpers[index].cte_name, mock_name: None, - sql: &self.helpers[index].1, - }), - }) - .collect() + sql: &self.helpers[index].sql, + }) + } + } } } +/// Every test-authored query that may read a helper, labelled by its CTE name. +fn test_readers(fixtures: &TestFixtures) -> Vec<(String, &str)> { + let mut readers: Vec<(String, &str)> = Vec::new(); + readers.extend( + fixtures + .helpers + .iter() + .map(|cte| (cte.name.clone(), cte.sql_body.as_str())), + ); + for (prefix, group) in [ + (REF_PREFIX, &fixtures.mock_refs), + (SOURCE_PREFIX, &fixtures.mock_sources), + (SEED_PREFIX, &fixtures.mock_seeds), + (DBT_REF_PREFIX, &fixtures.mock_dbt_refs), + (TABLE_FUNCTION_PREFIX, &fixtures.mock_table_functions), + ] { + readers.extend( + group + .iter() + .map(|(name, sql)| (format!("{prefix}{name}"), sql.as_str())), + ); + } + readers.extend( + fixtures + .expected + .iter() + .map(|(name, sql)| (format!("__expected__{name}"), sql.as_str())), + ); + readers.extend( + fixtures + .assertions + .iter() + .map(|(name, sql)| (format!("__assert__{name}"), sql.as_str())), + ); + readers +} + /// Return the helper and mock CTEs `sql` reads, transitively and dependencies first. pub(crate) fn helper_scope_ctes( sql: &str, @@ -244,37 +423,25 @@ pub(crate) fn helper_scope_ctes( patterns: &SqlTestPatterns, file_label: &str, ) -> Result { - let mut scope = HelperScope { - ctes: Vec::new(), - reached_mocks: HashSet::new(), - }; - if fixtures.helpers.is_empty() && fixtures.scope.mocks.is_empty() { - return Ok(scope); + let graph = &fixtures.scope; + if graph.helpers.is_empty() && graph.mocks.is_empty() { + return Ok(HelperScope { + ctes: Vec::new(), + reached_mocks: HashSet::new(), + }); } - for node in fixtures.scope.closure(&identifier_tokens(sql, patterns)) { - match node { - ScopeNode::Mock(index) => { - let mock = &fixtures.scope.mocks[index]; - scope.reached_mocks.insert(mock.mock_name.clone()); - scope - .ctes - .push((mock.generated_name.clone(), mock.sql.clone())); - } - ScopeNode::Helper(index) => { - let helper = &fixtures.helpers[index]; - if is_generated_cte_name(&helper.name) { - return Err(compile_error(&format!( - "SQL test '{file_label}' defines CTE '{}', which conflicts with the generated CTE", - helper.name - ))); - } - scope - .ctes - .push((helper.name.clone(), helper.sql_body.clone())); - } + let nodes = graph.closure(&identifier_tokens(sql, patterns), true); + for node in &nodes { + if let ScopeNode::Helper(index) = node + && is_generated_cte_name(&graph.helpers[*index].name) + { + return Err(compile_error(&format!( + "SQL test '{file_label}' defines CTE '{}', which conflicts with the generated CTE", + graph.helpers[*index].name + ))); } } - Ok(scope) + Ok(graph.collect_scope(nodes)) } /// Append scoped CTEs after a step's generated CTEs, keeping one definition per name. 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 90b14e37ba..b89dd43537 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/mod.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/mod.rs @@ -3,6 +3,7 @@ pub mod cte_slices; pub mod cte_sql; pub mod expected_columns; pub mod extraction; +pub mod helper_names; pub mod helper_scope; pub mod markers; pub mod planning; 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 6018ca46f8..98bdee42a2 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs @@ -9,13 +9,15 @@ use rayon::iter::{IntoParallelIterator, ParallelIterator}; use regex::Regex; use serde::{Deserialize, Serialize}; -use crate::compiler::_helpers::sql_tests::cte_slices::SliceDialect; +use crate::compiler::_helpers::sql_tests::cte_rename::defined_cte_keys; +use crate::compiler::_helpers::sql_tests::cte_slices::{SliceDialect, strip_statement_terminators}; use crate::compiler::_helpers::sql_tests::cte_sql::{ cte_definition_sql, leading_with_prefix_end, with_leading_ctes, with_unique_ctes, }; use crate::compiler::_helpers::sql_tests::expected_columns::expected_columns; +use crate::compiler::_helpers::sql_tests::helper_names::HELPER_PREFIX; use crate::compiler::_helpers::sql_tests::helper_scope::{ - ScopeGraph, helper_scope_ctes, merged_scoped_ctes, + ScopeGraph, ScopeRequest, helper_scope_ctes, merged_scoped_ctes, }; use crate::compiler::_helpers::sql_tests::markers::{ ProtectedRanges, marker_names, marker_names_in, replace_callable_markers, @@ -26,7 +28,10 @@ use crate::compiler::_helpers::sql_tests::rendering::{ AssertionStep, ChainStep, RenderRequest, render_comparison_sql, render_dialect, rendered_chain_steps, }; -use crate::constants::{TABLE_FUNCTION_TEST_MODE, UDF_TEST_MODE}; +use crate::constants::{ + SQL_TEST_ACTUAL_CTE, SQL_TEST_ACTUAL_CTE_PREFIX, SQL_TEST_EXPECTED_CTE, + TABLE_FUNCTION_TEST_MODE, UDF_TEST_MODE, +}; use sqlbuild_sqltext::sql_scan::models::LexicalSyntax; const DEFAULT_WORKERS: usize = 4; @@ -36,7 +41,7 @@ pub(crate) const REF_PREFIX: &str = "__ref__"; pub(crate) const SOURCE_PREFIX: &str = "__source__"; pub(crate) const SEED_PREFIX: &str = "__seed__"; pub(crate) const DBT_REF_PREFIX: &str = "__dbt_ref__"; -const TABLE_FUNCTION_PREFIX: &str = "__table_fn__"; +pub(crate) const TABLE_FUNCTION_PREFIX: &str = "__table_fn__"; pub(crate) const MOCK_CTE_PREFIXES: [&str; 5] = [ REF_PREFIX, SOURCE_PREFIX, @@ -272,17 +277,11 @@ pub(crate) struct TestFixtures { pub(crate) mock_sources: BTreeMap, pub(crate) mock_seeds: BTreeMap, pub(crate) mock_dbt_refs: BTreeMap, - mock_table_functions: BTreeMap, + pub(crate) mock_table_functions: BTreeMap, pub(crate) helpers: Vec, pub(crate) scope: ScopeGraph, - expected: BTreeMap, - assertions: Vec<(String, String)>, -} - -impl TestFixtures { - fn is_helper(&self, name: &str) -> bool { - self.helpers.iter().any(|helper| helper.name == name) - } + pub(crate) expected: BTreeMap, + pub(crate) assertions: Vec<(String, String)>, } struct DirectTestPlan { @@ -357,7 +356,12 @@ impl TextualChain { reachable.extend(resolution.reached); self.mock_ctes .insert(model_name.to_string(), resolution.mock_ctes); - self.insert(model_name, resolution.sql, chain_references); + let body = strip_statement_terminators( + &resolution.sql, + SliceDialect::new(Some(&inputs.context.dialect)), + ) + .to_string(); + self.insert(model_name, body, chain_references); } Ok(reachable) } @@ -435,6 +439,7 @@ struct AssertionResolutionRequest<'a> { struct AnalysisResolutionRequest<'a> { query_sql: &'a str, + fixture_ctes: &'a [(String, String)], fixtures: &'a TestFixtures, resolved_chain: &'a HashMap, functions: &'a HashMap, @@ -781,13 +786,40 @@ fn plan_direct_test( }) } +/// The first CTE a model defines under a name reserved for generated test-query CTEs. +fn reserved_cte_name(query_sql: &str, dialect: SliceDialect) -> Option { + let reserved = MOCK_CTE_PREFIXES.iter().copied().chain([ + HELPER_PREFIX, + EXPECTED_PREFIX, + ASSERT_PREFIX, + SQL_TEST_ACTUAL_CTE_PREFIX, + ]); + let prefixes: Vec<&str> = reserved.collect(); + defined_cte_keys(query_sql, dialect) + .unwrap_or_default() + .into_iter() + .find(|key| { + key == SQL_TEST_ACTUAL_CTE + || key == SQL_TEST_EXPECTED_CTE + || prefixes.iter().any(|prefix| key.starts_with(prefix)) + }) +} + fn plan_model_test( plan: ModelTestPlan, context: &ProjectContext, ) -> Result { let mut fixtures = classify_fixtures(plan.authored_ctes, plan.expected_ctes, plan.assertion_ctes); - fixtures.scope = ScopeGraph::new(&fixtures, &context.patterns); + fixtures.scope = ScopeGraph::new(&ScopeRequest { + fixtures: &fixtures, + patterns: &context.patterns, + file_label: &plan.file_label, + rename_dialect: (context.sql_analysis_enabled || context.rejects_nested_with) + .then_some(context.render_dialect.as_ref()), + flat_dialect: context.rejects_nested_with, + slice_dialect: SliceDialect::new(Some(&context.dialect)), + })?; let expected_names = chain_root_names(plan.expected_model_names, &fixtures, &context.patterns); let ordered_names = topo_sort_model_chain(TopoSortRequest { expected_names: &expected_names, @@ -819,16 +851,22 @@ fn plan_model_test( .model_query_overrides .get(model_name) .unwrap_or(&model.query_sql); - let (fixture_resolved_sql, reached_table_functions) = resolve_table_function_fixtures( - query_sql, - &fixtures.mock_table_functions, - &fixtures.scope, - &context.patterns, - )?; - reachable_mocks.extend(reached_table_functions); + if let Some(name) = reserved_cte_name(query_sql, SliceDialect::new(Some(&context.dialect))) + { + return Err(compile_error(&format!( + "SQL test '{}' reads model '{model_name}', which defines CTE '{name}'; names \ + starting with a SQLBuild test prefix such as __helper__ or __ref__ are reserved \ + for the test query, so rename the CTE in the model", + plan.file_label + ))); + } + let table_functions = + resolve_table_function_fixtures(query_sql, &fixtures, &context.patterns)?; + reachable_mocks.extend(table_functions.reached.iter().cloned()); let analyzed = if context.sql_analysis_enabled && analysis_resolved.len() == model_index { analyze_and_resolve_sql(AnalysisResolutionRequest { - query_sql: &fixture_resolved_sql, + query_sql: &table_functions.sql, + fixture_ctes: &table_functions.ctes, fixtures: &fixtures, resolved_chain: &analysis_resolved, functions: &context.functions, @@ -867,8 +905,8 @@ fn plan_model_test( patterns: &context.patterns, reported: &mut reported_missing_mocks, })); - let expected_cte_sql = fixtures.expected.get(model_name).cloned(); - let expected_lifted_ctes = match expected_cte_sql.as_deref() { + let authored_expected = fixtures.expected.get(model_name); + let expected_lifted_ctes = match authored_expected { Some(sql) => { let scope = helper_scope_ctes(sql, &fixtures, &context.patterns, &plan.file_label)?; reachable_mocks.extend(scope.reached_mocks); @@ -879,10 +917,10 @@ fn plan_model_test( chain.push(ChainStep { model_name: model_name.clone(), resolved_sql, - expected_columns: expected_cte_sql - .as_deref() + expected_columns: authored_expected .and_then(|sql| expected_columns(sql, &context.render_dialect)), - expected_cte_sql, + expected_cte_sql: authored_expected + .map(|sql| fixtures.scope.reader_sql(sql, &context.patterns)), expected_lifted_ctes, lifted_ctes, comparison_body_sql, @@ -892,17 +930,16 @@ fn plan_model_test( let mut assertions: Vec = Vec::new(); let mut textual_assertion_chain: Option = None; for (assertion_name, assertion_sql) in &fixtures.assertions { - let (fixture_resolved_sql, reached_table_functions) = resolve_table_function_fixtures( - assertion_sql, - &fixtures.mock_table_functions, - &fixtures.scope, - &context.patterns, - )?; - reachable_mocks.extend(reached_table_functions); + let placed_sql = fixtures.scope.reader_sql(assertion_sql, &context.patterns); + let table_functions = + resolve_table_function_fixtures(&placed_sql, &fixtures, &context.patterns)?; + reachable_mocks.extend(table_functions.reached.iter().cloned()); + let fixture_resolved_sql = &table_functions.sql; let analyzed = if context.sql_analysis_enabled && analysis_resolved.len() == ordered_names.len() { analyze_and_resolve_sql(AnalysisResolutionRequest { - query_sql: &fixture_resolved_sql, + query_sql: fixture_resolved_sql, + fixture_ctes: &table_functions.ctes, fixtures: &fixtures, resolved_chain: &analysis_resolved, functions: &context.functions, @@ -941,7 +978,7 @@ fn plan_model_test( .ok_or_else(|| planner_error("textual assertion chain is unavailable"))?; let (resolved, reached) = resolve_assertion_textual_sql(AssertionResolutionRequest { - assertion_sql: &fixture_resolved_sql, + assertion_sql: fixture_resolved_sql, fixtures: &fixtures, chain, functions: &context.functions, @@ -949,15 +986,16 @@ fn plan_model_test( patterns: &context.patterns, })?; reachable_mocks.extend(reached); + let lifted = with_unique_ctes(table_functions.ctes.clone(), resolved.lifted_ctes); ( - resolved.resolved_sql, - resolved.lifted_ctes, + with_leading_ctes(&lifted, &resolved.body_sql), + lifted, Some(resolved.body_sql), ) } }; let scope = helper_scope_ctes( - &fixture_resolved_sql, + assertion_sql, &fixtures, &context.patterns, &plan.file_label, @@ -1237,12 +1275,17 @@ fn analyze_and_resolve_sql( names: template.existing_cte_names, ..GeneratedCteState::default() }; + for (name, sql) in request.fixture_ctes { + generated_state.insert_helper(name, sql); + } let mut replacements: HashMap<(String, String), String> = HashMap::new(); for (function_name, referenced_name) in template.marker_calls { let target = if function_name == REF_FUNCTION { if let Some(chain_sql) = request.resolved_chain.get(&referenced_name) { for (name, sql) in &chain_sql.generated_ctes { - if request.fixtures.is_helper(name) { + if request.fixtures.scope.is_top_level_helper(name) + || name.starts_with(TABLE_FUNCTION_PREFIX) + { generated_state.insert_helper(name, sql); } else { generated_state.insert(name, sql, request.file_label)?; @@ -1300,7 +1343,11 @@ fn analyze_and_resolve_sql( replacements.insert((function_name, referenced_name), target); } } - let cte_body_sql = replace_relation_markers(request.query_sql, &replacements, request.patterns); + let cte_body_sql = strip_statement_terminators( + &replace_relation_markers(request.query_sql, &replacements, request.patterns), + SliceDialect::new(Some(request.dialect_name)), + ) + .to_string(); let resolved_sql = with_leading_ctes(&generated_state.generated, &cte_body_sql); Ok(Some(AnalysisResolvedSql { resolved_sql, @@ -1352,7 +1399,7 @@ where Ok(cached.get_or_init(initialize).clone()) } -/// Textually resolved SQL plus the mocks it reached and the mock CTEs its inlined mocks read. +/// Textually resolved SQL plus the mocks it reached and the top-level mock CTEs it reads. struct TextualResolution { sql: String, reached: HashSet, @@ -1361,7 +1408,7 @@ struct TextualResolution { fn resolve_textual_sql(request: TextualResolutionRequest<'_>) -> Result { let mut reached: HashSet = HashSet::new(); - let mut inlined: Vec = Vec::new(); + let mut referenced_mocks: Vec = Vec::new(); let mut chain_references = request.chain_references; let mut result = replace_named_markers( request.query_sql, @@ -1378,10 +1425,9 @@ fn resolve_textual_sql(request: TextualResolutionRequest<'_>) -> Result) -> Result) -> Result) -> Result = Vec::new(); - for generated_name in inlined { - let dependencies = request - .fixtures - .scope - .inlined_mock_dependencies(&generated_name); + let mut mock_ctes: Vec<(String, String)> = table_functions.ctes; + for generated_name in referenced_mocks { + let scope = &request.fixtures.scope; + let dependencies = scope.mock_dependencies(&generated_name); reached.extend( dependencies .iter() .filter_map(|dependency| dependency.mock_name.map(str::to_string)), ); + let mock_sql = scope + .mock_sql(&generated_name) + .unwrap_or_default() + .to_string(); mock_ctes = with_unique_ctes( mock_ctes, - dependencies.into_iter().map(|dependency| { - ( - dependency.generated_name.to_string(), - dependency.sql.to_string(), - ) - }), + dependencies + .into_iter() + .map(|dependency| { + ( + dependency.generated_name.to_string(), + dependency.sql.to_string(), + ) + }) + .chain(std::iter::once((generated_name, mock_sql))), ); } Ok(TextualResolution { @@ -1460,25 +1509,54 @@ fn resolve_textual_sql(request: TextualResolutionRequest<'_>) -> Result, + ctes: Vec<(String, String)>, +} + fn resolve_table_function_fixtures( sql: &str, - fixtures: &BTreeMap, - scope: &ScopeGraph, + fixtures: &TestFixtures, patterns: &SqlTestPatterns, -) -> Result<(String, HashSet), String> { - if fixtures.is_empty() { - return Ok((sql.to_string(), HashSet::new())); +) -> Result { + let mut resolved = TableFunctionFixtures { + sql: sql.to_string(), + reached: HashSet::new(), + ctes: Vec::new(), + }; + if fixtures.mock_table_functions.is_empty() { + return Ok(resolved); } - replace_callable_markers( + let (replaced, reached) = replace_callable_markers( sql, &patterns.table_function, &patterns.lexical, |name, _call_suffix| { fixtures - .get(name) - .map(|body| (scope.inlined_fixture_sql(body, patterns), true)) + .mock_table_functions + .contains_key(name) + .then(|| (format!("{TABLE_FUNCTION_PREFIX}{name}"), true)) }, - ) + )?; + resolved.sql = replaced; + for (name, body) in &fixtures.mock_table_functions { + if !reached.contains(name) { + continue; + } + let scope = fixtures.scope.reader_scope(body, patterns); + resolved.reached.extend(scope.reached_mocks); + resolved.ctes = with_unique_ctes( + std::mem::take(&mut resolved.ctes), + scope.ctes.into_iter().chain(std::iter::once(( + format!("{TABLE_FUNCTION_PREFIX}{name}"), + fixtures.scope.reader_sql(body, patterns), + ))), + ); + } + resolved.reached.extend(reached); + Ok(resolved) } fn resolve_function_calls( 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 2ba74f9f0c..72a24956ab 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/rendering.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/rendering.rs @@ -4,19 +4,20 @@ use std::collections::{HashMap, HashSet}; use polyglot_sql::{Dialect, DialectType}; +use crate::compiler::_helpers::sql_tests::cte_rename::defined_cte_keys; use crate::compiler::_helpers::sql_tests::cte_rename::{CteRename, rename_ctes}; use crate::compiler::_helpers::sql_tests::cte_slices::{ SliceDialect, WithSlices, identifier_keys, split_top_level_with, strip_statement_terminators, used_ctes, }; -use crate::compiler::_helpers::sql_tests::cte_sql::{ - cte_definition_sql, leading_with_prefix_end, parenthesized_sql, with_leading_ctes, -}; +use crate::compiler::_helpers::sql_tests::cte_sql::{cte_definition_sql, leading_with_prefix_end}; +use crate::compiler::_helpers::sql_tests::helper_names::HELPER_PREFIX; use crate::compiler::_helpers::sql_tests::planning::REF_PREFIX; use rayon::iter::{IntoParallelIterator, ParallelIterator}; use serde::{Deserialize, Serialize}; const DEFAULT_WORKERS: usize = 4; +const STEP_BODY_CTE: &str = "__sqb_step_body"; const MAX_WORKERS: usize = 4; const WORKER_STACK_BYTES: usize = 16 * 1024 * 1024; @@ -125,15 +126,17 @@ fn same_header(left: &str, right: &str) -> bool { struct RenderCteState { dialect: SliceDialect, + nested_test_sql: bool, lifted: Vec, name_counts: CteSuffixCounter, renamed_ctes: usize, } impl RenderCteState { - fn new(dialect: SliceDialect) -> Self { + fn new(dialect: SliceDialect, model_test: bool) -> Self { Self { dialect, + nested_test_sql: model_test && !dialect.rejects_nested_with(), lifted: Vec::new(), name_counts: CteSuffixCounter::default(), renamed_ctes: 0, @@ -143,7 +146,7 @@ impl RenderCteState { /// Lift a query's CTEs verbatim and return its body, nesting it unchanged on a name clash. fn lift(&mut self, sql: &str, enabled: bool, origin: &str) -> Result { let sql = strip_statement_terminators(sql, self.dialect); - if !enabled || leading_with_prefix_end(sql).is_none() { + if !enabled || self.nested_test_sql || leading_with_prefix_end(sql).is_none() { return Ok(sql.to_string()); } let split = match split_top_level_with(sql, self.dialect) { @@ -211,15 +214,12 @@ impl RenderCteState { let model_cte = model_cte_name(&step.model_name, suffix); let mut ctes = step.lifted_ctes.clone(); ctes.push((model_cte.clone(), body.to_string())); - if let Some(collisions) = self.merge(&ctes, enabled, &origin)? { - return self.nested( - &nested_model_sql(&step.lifted_ctes, body), - enabled, - &origin, - &[collisions], - ); - } - Ok(format!("SELECT * FROM {model_cte}")) + self.merge_inputs( + &ctes, + &format!("SELECT * FROM {model_cte}"), + enabled, + &origin, + ) } /// Place an expected step's helper CTEs at top level and return its comparison body. @@ -230,18 +230,8 @@ impl RenderCteState { enabled: bool, ) -> Result { let origin = format!("the expected rows of model '{}'", step.model_name); - if step.expected_lifted_ctes.is_empty() { - return self.lift(expected_sql, enabled, &origin); - } - if let Some(collisions) = self.merge(&step.expected_lifted_ctes, enabled, &origin)? { - return self.nested( - &with_leading_ctes(&step.expected_lifted_ctes, expected_sql), - enabled, - &origin, - &[collisions], - ); - } - self.lift(expected_sql, enabled, &origin) + let body = self.merge_inputs(&step.expected_lifted_ctes, expected_sql, enabled, &origin)?; + self.lift(&body, enabled, &origin) } /// Place an assertion's helper CTEs at top level and return its comparison body. @@ -251,19 +241,76 @@ impl RenderCteState { enabled: bool, ) -> Result { let origin = format!("assertion '{}'", assertion.name); - if !assertion.lifted_ctes.is_empty() - && let Some(collisions) = self.merge(&assertion.lifted_ctes, enabled, &origin)? - { - return self.nested(&assertion.resolved_sql, enabled, &origin, &[collisions]); + let body = assertion + .comparison_body_sql + .as_deref() + .unwrap_or(&assertion.resolved_sql); + let body = self.merge_inputs(&assertion.lifted_ctes, body, enabled, &origin)?; + self.lift(&body, enabled, &origin) + } + + /// Merge inputs at top level, renaming any whose name is taken by other text, and return the body. + fn merge_inputs( + &mut self, + inputs: &[(String, String)], + body: &str, + enabled: bool, + origin: &str, + ) -> Result { + let Some(collisions) = self.merge(inputs, enabled, origin)? else { + return Ok(body.to_string()); + }; + let refused = || { + format!( + "{collisions}; rename the CTE in the model or the fixture so the names are unique" + ) + }; + let (renamed_inputs, renamed_body) = + self.renamed_inputs(inputs, body).ok_or_else(refused)?; + if self.merge(&renamed_inputs, enabled, origin)?.is_some() { + return Err(refused()); } - self.lift( - assertion - .comparison_body_sql - .as_deref() - .unwrap_or(&assertion.resolved_sql), - enabled, - &origin, - ) + Ok(renamed_body) + } + + /// Inputs whose names collide with lifted CTEs renamed by token span, with the body's references. + fn renamed_inputs( + &mut self, + inputs: &[(String, String)], + body: &str, + ) -> Option<(Vec<(String, String)>, String)> { + let definitions: Vec = inputs + .iter() + .map(|(name, sql)| cte_definition_sql(name, sql)) + .chain(std::iter::once(cte_definition_sql(STEP_BODY_CTE, body))) + .collect(); + let statement = format!("WITH {}\nSELECT 1", definitions.join(",\n")); + let Ok(Some(split)) = split_top_level_with(&statement, self.dialect) else { + return None; + }; + let colliding = self.colliding_indices(&split, &vec![true; split.ctes.len()]); + let taken = identifier_keys(&statement, self.dialect); + let names: Vec = colliding.iter().map(|_| self.fresh_name(&taken)).collect(); + let renames: Vec> = colliding + .iter() + .zip(&names) + .map(|(index, name)| CteRename { + index: *index, + name, + }) + .collect(); + let renamed = rename_ctes(&statement, &split, &renames, self.dialect)?; + let Ok(Some(split)) = split_top_level_with(&renamed, self.dialect) else { + return None; + }; + let (body_cte, input_ctes) = split.ctes.split_last()?; + Some(( + input_ctes + .iter() + .map(|cte| (cte.header.to_string(), cte.body.to_string())) + .collect(), + body_cte.body.to_string(), + )) } /// Add generated CTEs verbatim, describing any name already lifted with different text. @@ -415,6 +462,31 @@ impl RenderCteState { self.name_counts.next(model_name) } + /// Refuse a query where a nested CTE reuses a top-level name, which dialects resolve differently. + fn verify_isolated_names(&self, comparison_ctes: &[(String, String)]) -> Result<(), String> { + let top_level: Vec<(&str, &str, String)> = self + .lifted + .iter() + .map(|cte| (cte.header.as_str(), cte.body.as_str(), cte.origin.clone())) + .chain( + comparison_ctes + .iter() + .map(|(name, sql)| (name.as_str(), sql.as_str(), format!("CTE '{name}'"))), + ) + .collect(); + let keys: HashSet = top_level.iter().map(|(name, _, _)| cte_key(name)).collect(); + for (_, body, origin) in &top_level { + let nested = defined_cte_keys(body, self.dialect).unwrap_or_default(); + if let Some(name) = nested.iter().find(|name| keys.contains(*name)) { + return Err(format!( + "{origin} defines CTE '{name}', which is also a CTE SQLBuild generates at the \ + top level of the test query; rename the CTE" + )); + } + } + Ok(()) + } + fn definitions(&mut self) -> Vec { std::mem::take(&mut self.lifted) .iter() @@ -428,14 +500,21 @@ fn collision_message(origin: &str, collisions: &[(&str, &LiftedCte)]) -> String .iter() .map(|(header, existing)| { format!( - "CTE '{header}' of {origin} collides with CTE '{}' of {}", - existing.header, existing.origin + "CTE '{}' of {origin} collides with CTE '{}' of {}", + display_cte_name(header), + display_cte_name(&existing.header), + existing.origin ) }) .collect::>() .join("; ") } +/// A CTE name as its author wrote it, without SQLBuild's reserved helper prefix. +fn display_cte_name(name: &str) -> &str { + name.strip_prefix(HELPER_PREFIX).unwrap_or(name) +} + /// The `__ref__` CTE of a step's model, under the name downstream steps reference it by. fn model_cte_name(model_name: &str, suffix: &str) -> String { let mut characters = model_name.chars(); @@ -446,23 +525,10 @@ fn model_cte_name(model_name: &str, suffix: &str) -> String { format!("{REF_PREFIX}{}", if plain { model_name } else { suffix }) } -/// A model body nested below its inputs, for a step whose inputs collide at top level. -fn nested_model_sql(inputs: &[(String, String)], body: &str) -> String { - if inputs.is_empty() { - return body.to_string(); - } - with_leading_ctes( - inputs, - &format!( - "SELECT * FROM {} AS __sqlbuild_model", - parenthesized_sql(body) - ), - ) -} - /// Describe a generated CTE by what it stands in for: an upstream model, source or seed. fn generated_cte_origin(name: &str) -> String { for (prefix, kind) in [ + (HELPER_PREFIX, "helper"), ("__ref__", "model"), ("__source__", "source"), ("__seed__", "seed"), @@ -566,8 +632,10 @@ fn render_difference_sample_sql(request: &DifferenceSampleRequest) -> Result Result = Vec::new(); + let model_test = !request.assertions.is_empty() + || request + .chain + .iter() + .any(|step| step.comparison_body_sql.is_some()); + let mut cte_state = RenderCteState::new( + SliceDialect::new(request.sql_analysis_dialect.as_deref()), + model_test, + ); + let mut comparison_ctes: Vec<(String, String)> = Vec::new(); let mut select_parts: Vec = Vec::new(); let rendered_steps = rendered_chain_steps(&request.chain, &request.assertions); let mut probe_actual_cte: Option = None; @@ -625,7 +700,7 @@ pub(crate) fn render_comparison_sql(request: &RenderRequest) -> Result Result Result Result bool { let response = extract_batch_json(r#"{"tests":[{"sql":"WITH helper AS (SELECT 1 AS id), __source__raw_orders AS (SELECT id FROM helper), __expected__orders AS (SELECT id FROM helper), __assert__positive AS (SELECT id FROM helper WHERE id < 0) SELECT 1","fileLabel":"tests/orders.sql","mode":"model"},{"sql":"WITH input AS (SELECT 1 AS value), __udf_actual__ AS (SELECT __udf(\"increment\")(value) AS value FROM input), __udf_expected__ AS (SELECT 2 AS value) SELECT 1","fileLabel":"tests/increment.sql","mode":"udf"}]}"#).expect("batch succeeds"); @@ -567,7 +570,10 @@ pub(crate) fn plan_without_rendering_returns_executable_steps() -> bool { assert_eq!(artifact["chain"][0]["expectedCteSql"], Value::Null); assert_eq!( artifact["chain"][1]["liftedCtes"], - json!([["__ref__stg_orders", "SELECT * FROM (SELECT 1 AS order_id)"]]) + json!([ + ["__source__raw_orders", "SELECT 1 AS order_id"], + ["__ref__stg_orders", "SELECT * FROM __source__raw_orders"] + ]) ); assert_eq!( artifact["chain"][1]["comparisonBodySql"], @@ -583,7 +589,7 @@ pub(crate) fn plan_without_rendering_returns_executable_steps() -> bool { json!("SELECT * FROM __ref__orders WHERE order_id < 0") ); assert_eq!( - artifact["assertions"][0]["liftedCtes"][1], + artifact["assertions"][0]["liftedCtes"][2], json!(["__ref__orders", "SELECT * FROM __ref__stg_orders"]) ); assert!( @@ -644,7 +650,8 @@ pub(crate) fn textual_assertion_with_clause_merges_lifted_ctes() -> bool { assert_eq!( resolved_sql, concat!( - "WITH __ref__stg_orders AS (SELECT * FROM (SELECT 1 AS order_id)), ", + "WITH __source__raw_orders AS (SELECT 1 AS order_id), ", + "__ref__stg_orders AS (SELECT * FROM __source__raw_orders), ", "__ref__orders AS (SELECT * FROM __ref__stg_orders), ", "negative_orders AS (SELECT * FROM __ref__orders WHERE order_id < 0) ", "SELECT * FROM negative_orders" @@ -1100,26 +1107,92 @@ fn top_level_cte_position(sql: &str, name: &str) -> usize { positions[0] } +/// Case-folded names of the CTEs a rendered test query defines at its top level. +pub(crate) fn top_level_cte_keys(sql: &str, dialect: &str) -> Vec { + let dialect = + crate::compiler::_helpers::sql_tests::cte_slices::SliceDialect::new(Some(dialect)); + crate::compiler::_helpers::sql_tests::cte_slices::split_top_level_with(sql, dialect) + .expect("rendered SQL scans") + .expect("rendered SQL has a WITH") + .ctes + .iter() + .map(|cte| cte.key.clone()) + .collect() +} + +fn assert_rendered_shape(sql: &str, shape: &RenderedShapeTestCase, helpers: &[&str]) { + let top_level = top_level_cte_keys(sql, "duckdb"); + for helper in helpers { + assert!(!top_level.contains(&(*helper).to_string()), "{sql}"); + } + for (first, second) in shape.expected_order { + assert!( + top_level_cte_position(sql, first) < top_level_cte_position(sql, second), + "{}: {first} before {second}: {sql}", + shape.description + ); + } + for fragment in shape.expected_fragments { + assert!( + sql.contains(fragment), + "{}: {fragment}: {sql}", + shape.description + ); + } + for fragment in shape.expected_absent_fragments { + assert!( + !sql.contains(fragment), + "{}: {fragment}: {sql}", + shape.description + ); + } +} + pub(crate) fn helper_ctes_are_in_scope_for_assertions_and_expected_rows() -> bool { - for sql_analysis_enabled in [true, false] { + let shapes = [ + RenderedShapeTestCase { + description: "sql analysis on", + sql_analysis_enabled: true, + expected_order: &[ + ("__ref__stg_orders", "__expected__orders"), + ("__expected__orders", "__assert__ids_match"), + ("__ref__stg_orders", "__helper__matched_ids"), + ("__helper__expected_rows", "__helper__matched_ids"), + ("__helper__expected_rows", "__expected__orders"), + ("__helper__matched_ids", "__assert__ids_match"), + ], + expected_fragments: &[ + "__expected__orders AS (SELECT order_id, amount FROM __helper__expected_rows AS expected_rows)", + ], + expected_absent_fragments: &["__helper__unused_rows"], + }, + RenderedShapeTestCase { + description: "sql analysis off", + sql_analysis_enabled: false, + expected_order: &[ + ("__ref__stg_orders", "__expected__orders"), + ("__expected__orders", "__assert__ids_match"), + ], + expected_fragments: &[ + "__expected__orders AS (WITH expected_rows AS (SELECT 1 AS order_id, 10 AS amount) \ + SELECT order_id, amount FROM expected_rows)", + ], + expected_absent_fragments: &["unused_rows"], + }, + ]; + for shape in shapes { let response: Value = serde_json::from_str( - &plan_helper_scope_request(sql_analysis_enabled, "matched_ids") + &plan_helper_scope_request(shape.sql_analysis_enabled, "matched_ids") .expect("test assumption must hold"), ) .expect("test assumption must hold"); let sql = response["artifacts"][0]["sql"] .as_str() .expect("test assumption must hold"); - let mock = top_level_cte_position(sql, "__ref__stg_orders"); - let expected_rows = top_level_cte_position(sql, "expected_rows"); - let matched_ids = top_level_cte_position(sql, "matched_ids"); - let expected = top_level_cte_position(sql, "__expected__orders"); - let assertion = top_level_cte_position(sql, "__assert__ids_match"); - assert!(mock < matched_ids && expected_rows < matched_ids, "{sql}"); - assert!(expected_rows < expected && matched_ids < assertion, "{sql}"); - assert!( - !sql.contains(",\nunused_rows AS ("), - "unreferenced helpers stay out of top-level scope: {sql}" + assert_rendered_shape( + sql, + &shape, + &["expected_rows", "matched_ids", "unused_rows"], ); assert_eq!(response["artifacts"][0]["warnings"], json!([]), "{sql}"); } @@ -1172,7 +1245,36 @@ pub(crate) fn difference_sample_lifts_expected_helper_ctes() -> bool { } pub(crate) fn mock_read_through_helper_brings_its_mock_dependencies_into_scope() -> bool { - for sql_analysis_enabled in [true, false] { + let shapes = [ + RenderedShapeTestCase { + description: "sql analysis on", + sql_analysis_enabled: true, + expected_order: &[ + ("__ref__raw_orders", "__ref__stg_orders"), + ("__ref__raw_orders", "__actual__orders"), + ("__ref__raw_orders", "__helper__base_rows"), + ("__helper__base_rows", "__ref__stg_orders"), + ("__ref__stg_orders", "__helper__expected_rows"), + ("__helper__expected_rows", "__assert__rows_match"), + ], + expected_fragments: &[], + expected_absent_fragments: &[], + }, + RenderedShapeTestCase { + description: "sql analysis off", + sql_analysis_enabled: false, + expected_order: &[ + ("__ref__raw_orders", "__ref__stg_orders"), + ("__ref__raw_orders", "__actual__orders"), + ], + expected_fragments: &[ + "__ref__stg_orders AS (WITH base_rows AS (SELECT order_id FROM __ref__raw_orders) \ + SELECT order_id FROM base_rows)", + ], + expected_absent_fragments: &[], + }, + ]; + for shape in shapes { let response: Value = serde_json::from_str( &crate::compiler::main::sql_test_planning::plan_and_render_json( &json!({ @@ -1204,7 +1306,7 @@ pub(crate) fn mock_read_through_helper_brings_its_mock_dependencies_into_scope() }] } }], - "sqlAnalysisEnabled": sql_analysis_enabled, + "sqlAnalysisEnabled": shape.sql_analysis_enabled, "sqlAnalysisDialect": "duckdb", "setDifferenceOperator": "EXCEPT" }) @@ -1216,17 +1318,12 @@ pub(crate) fn mock_read_through_helper_brings_its_mock_dependencies_into_scope() let sql = response["artifacts"][0]["sql"] .as_str() .expect("test assumption must hold"); - let raw_mock = top_level_cte_position(sql, "__ref__raw_orders"); - let stg_mock = top_level_cte_position(sql, "__ref__stg_orders"); - let expected_rows = top_level_cte_position(sql, "expected_rows"); - let actual = top_level_cte_position(sql, "__actual__orders"); - let assertion = top_level_cte_position(sql, "__assert__rows_match"); - assert!(raw_mock < stg_mock && stg_mock < expected_rows, "{sql}"); - assert!(raw_mock < actual && expected_rows < assertion, "{sql}"); + assert_rendered_shape(sql, &shape, &["base_rows", "expected_rows"]); assert_eq!(response["artifacts"][0]["warnings"], json!([]), "{sql}"); } true } + fn render_one(request: Value) -> Result { let response = crate::compiler::_helpers::sql_tests::rendering::render_json( &json!({"requests": [request]}).to_string(), @@ -1363,8 +1460,8 @@ pub(super) fn tsql_unprovable_cte_renames_are_refused_with_named_ctes() -> bool true } -pub(super) fn tsql_fixture_and_model_cte_collisions_are_refused() -> bool { - let error = render_one(json!({ +pub(super) fn tsql_fixture_and_model_cte_collisions_are_renamed() -> bool { + let sql = render_one(json!({ "sqlAnalysisDialect": "tsql", "chain": [{ "modelName": "orders", @@ -1373,14 +1470,13 @@ pub(super) fn tsql_fixture_and_model_cte_collisions_are_refused() -> bool { "expectedLiftedCtes": [["helper_rows", "SELECT 2 AS order_id"]] }] })) - .expect_err("T-SQL refusal"); - assert_eq!( - error, - "CTE 'helper_rows' of the expected rows of model 'orders' collides with CTE \ - 'helper_rows' of model 'orders'; T-SQL does not allow a nested WITH, so rename the \ - CTE in the model or the fixture so the names are unique" - ); - true + .expect("T-SQL rename"); + sql.starts_with( + "WITH helper_rows AS (SELECT 1 AS order_id),\n\ + __sqb_cte_0 AS (SELECT 2 AS order_id),\n\ + __actual__orders AS (SELECT order_id FROM helper_rows),\n\ + __expected__orders AS (SELECT order_id FROM __sqb_cte_0)\n", + ) } pub(super) fn tsql_identical_helper_ending_in_line_comment_is_shared() -> bool { @@ -1599,3 +1695,99 @@ pub(crate) fn relation_marker_oracle_mismatches(dialect: &str) -> Vec { }) .collect() } + +/// Dialects and their analysis modes; analysis reads BigQuery and Databricks markers as strings. +const DIALECT_MODES: [(&str, &[bool]); 6] = [ + ("duckdb", &[true, false]), + ("snowflake", &[true, false]), + ("postgres", &[true, false]), + ("bigquery", &[false]), + ("databricks", &[false]), + ("tsql", &[true, false]), +]; + +/// Plan a fixture-read helper beside a model on every dialect and check names stay isolated. +pub(crate) fn helper_names_are_isolated(shared: &SharedNameTestCase) -> bool { + for (dialect, modes) in DIALECT_MODES { + for &sql_analysis_enabled in modes { + let label = format!( + "{} on {dialect}, analysis {sql_analysis_enabled}", + shared.description + ); + let artifact = plan_shape(&PlanShape { + dialect, + sql_analysis_enabled, + model_sql: shared.model_sql, + helper_name: shared.helper_name, + expected_sql: "SELECT 1 AS order_id", + }) + .expect("plan"); + assert_eq!(artifact["warnings"], json!([]), "{label}"); + let sql = artifact["sql"].as_str().expect("rendered SQL"); + assert_names_isolated(sql, dialect, shared.helper_name, &label); + assert_eq!( + sql.matches(shared.expected_model_reads).count(), + 1, + "{label}: {sql}" + ); + } + } + true +} + +/// The compile error, or else the first warning, of planning one shape. +pub(crate) fn plan_shape_refusal(shape: &PlanShape<'_>) -> String { + plan_shape(shape) + .map(|artifact| { + artifact["warnings"][0]["message"] + .as_str() + .unwrap_or_default() + .to_string() + }) + .unwrap_or_else(|error| error) +} + +fn plan_shape(shape: &PlanShape<'_>) -> Result { + let response = crate::compiler::main::sql_test_planning::plan_and_render_json( + &json!({ + "lexicalSyntax": generic_lexical_syntax(), + "models": [{"name": "orders", "querySql": shape.model_sql, "modelDependencies": []}], + "tests": [{"name": "orders_case", "fileLabel": "tests/orders.sql", "payload": { + "kind": "model", + "authoredCtes": [ + {"name": shape.helper_name, "sqlBody": "SELECT 1 AS order_id"}, + {"name": "__source__raw_orders", "sqlBody": format!("SELECT order_id FROM {}", shape.helper_name)} + ], + "expectedCtes": [{"name": "__expected__orders", "sqlBody": shape.expected_sql}], + "expectedModelNames": ["orders"], + "assertionCtes": [] + }}], + "sqlAnalysisEnabled": shape.sql_analysis_enabled, + "sqlAnalysisDialect": shape.dialect + }) + .to_string(), + )?; + Ok(serde_json::from_str::(&response).expect("plan JSON")["artifacts"][0].clone()) +} + +/// Top-level CTE names and every nested CTE name are disjoint, and no helper keeps its name. +fn assert_names_isolated(sql: &str, dialect: &str, helper_name: &str, label: &str) { + let slice_dialect = SliceDialect::new(Some(dialect)); + let split = split_top_level_with(sql, slice_dialect) + .expect("rendered SQL scans") + .expect("rendered SQL has a WITH"); + let top_level: HashSet = split.ctes.iter().map(|cte| cte.key.clone()).collect(); + let helper_at_top_level = split.ctes.iter().any(|cte| { + cte.key == helper_name + && (!slice_dialect.rejects_nested_with() || cte.body.contains("SELECT 1 AS order_id")) + }); + assert!(!helper_at_top_level, "{label}: {sql}"); + for cte in &split.ctes { + let nested = defined_cte_keys(cte.body, slice_dialect).expect("body scans"); + assert!( + nested.iter().all(|name| !top_level.contains(name)), + "{label}: {} nests {nested:?}: {sql}", + cte.header + ); + } +} diff --git a/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_name_isolation.rs b/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_name_isolation.rs new file mode 100644 index 0000000000..8c555fa0de --- /dev/null +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_name_isolation.rs @@ -0,0 +1,142 @@ +use std::collections::HashMap; + +use polyglot_sql::Dialect; + +use crate::compiler::_helpers::sql_tests::helper_names::rename_helper_references; +use crate::compiler::tests::helpers::{helper_names_are_isolated, plan_shape_refusal}; +use crate::compiler::tests::test_types::{ + HelperRenameTestCase, NameRefusalTestCase, PlanShape, SharedNameTestCase, +}; + +const SHARED_CTE_NAME_MODEL: &str = "WITH base_rows AS (SELECT order_id FROM __source(\"raw_orders\")) \ + SELECT base_rows.order_id FROM base_rows"; +const PHYSICAL_TABLE_MODEL: &str = "WITH final AS (SELECT o.order_id FROM __source(\"raw_orders\") AS o \ + INNER JOIN lookup_rows AS l ON l.order_id = o.order_id) SELECT order_id FROM final"; + +#[test] +fn given_helper_references_when_renaming_then_only_relation_references_change() { + let test_cases = [ + HelperRenameTestCase { + description: "unaliased reference keeps its spelling as the alias", + sql: "SELECT expected_rows.order_id FROM expected_rows", + expected_sql: Some( + "SELECT expected_rows.order_id FROM __helper__expected_rows AS expected_rows", + ), + }, + HelperRenameTestCase { + description: "aliased references keep their alias", + sql: "SELECT e.order_id FROM expected_rows AS e JOIN Expected_Rows f ON e.order_id = f.order_id", + expected_sql: Some( + "SELECT e.order_id FROM __helper__expected_rows AS e JOIN __helper__expected_rows f ON e.order_id = f.order_id", + ), + }, + HelperRenameTestCase { + description: "columns, aliases, strings and qualified tables named like a helper stay", + sql: "SELECT expected_rows AS expected_rows, 'expected_rows' AS label \ + FROM main.expected_rows -- expected_rows", + expected_sql: Some( + "SELECT expected_rows AS expected_rows, 'expected_rows' AS label \ + FROM main.expected_rows -- expected_rows", + ), + }, + HelperRenameTestCase { + description: "references in subqueries, join conditions and set operations change", + sql: "SELECT '订单' AS label FROM __ref(\"orders\") AS o \ + JOIN t ON o.order_id IN (SELECT order_id FROM expected_rows) \ + EXCEPT SELECT '订单', order_id FROM \"expected_rows\"", + expected_sql: Some( + "SELECT '订单' AS label FROM __ref(\"orders\") AS o \ + JOIN t ON o.order_id IN (SELECT order_id FROM __helper__expected_rows AS expected_rows) \ + EXCEPT SELECT '订单', order_id FROM __helper__expected_rows AS \"expected_rows\"", + ), + }, + HelperRenameTestCase { + description: "SQL that does not parse is not rewritten", + sql: "SELECT FROM WHERE expected_rows", + expected_sql: None, + }, + ]; + let helpers: HashMap = HashMap::from([( + "expected_rows".to_string(), + "__helper__expected_rows".to_string(), + )]); + let dialect = Dialect::get_by_name("duckdb").expect("DuckDB dialect"); + for test_case in test_cases { + assert_eq!( + rename_helper_references(test_case.sql, &helpers, &dialect).as_deref(), + test_case.expected_sql, + "{}", + test_case.description + ); + } +} + +#[test] +fn given_helpers_sharing_model_names_when_rendering_then_names_are_isolated_on_every_dialect() { + let test_cases = [ + SharedNameTestCase { + description: "helper read by a fixture shares a model's CTE name", + model_sql: SHARED_CTE_NAME_MODEL, + helper_name: "base_rows", + expected_model_reads: "SELECT base_rows.order_id FROM base_rows", + expected_isolated: true, + }, + SharedNameTestCase { + description: "helper read by a fixture shares a physical table the model reads", + model_sql: PHYSICAL_TABLE_MODEL, + helper_name: "lookup_rows", + expected_model_reads: "INNER JOIN lookup_rows AS l", + expected_isolated: true, + }, + ]; + for test_case in test_cases { + assert_eq!( + helper_names_are_isolated(&test_case), + test_case.expected_isolated, + "{}", + test_case.description + ); + } +} + +#[test] +fn given_names_that_cannot_be_isolated_when_planning_then_test_is_refused() { + let test_cases = [ + NameRefusalTestCase { + description: "model CTE uses a generated helper name", + shape: PlanShape { + dialect: "snowflake", + sql_analysis_enabled: true, + model_sql: "WITH staged AS (WITH __helper__base_rows AS (SELECT 2 AS order_id) \ + SELECT order_id FROM __source(\"raw_orders\")) SELECT order_id FROM staged", + helper_name: "base_rows", + expected_sql: "SELECT 1 AS order_id", + }, + expected_message: "compile_input:SQL test 'tests/orders.sql' reads model 'orders', \ + which defines CTE '__helper__base_rows'; names starting with a SQLBuild test \ + prefix such as __helper__ or __ref__ are reserved for the test query, so rename \ + the CTE in the model", + }, + NameRefusalTestCase { + description: "test CTE redefines a helper in its own WITH", + shape: PlanShape { + dialect: "duckdb", + sql_analysis_enabled: true, + model_sql: SHARED_CTE_NAME_MODEL, + helper_name: "base_rows", + expected_sql: "WITH base_rows AS (SELECT 1 AS order_id) SELECT order_id FROM base_rows", + }, + expected_message: "compile_input:SQL test 'tests/orders.sql' defines CTE 'base_rows' \ + inside CTE '__expected__orders', which redefines helper CTE 'base_rows'; give one \ + of them a different name", + }, + ]; + for test_case in test_cases { + assert_eq!( + plan_shape_refusal(&test_case.shape), + test_case.expected_message, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_rendering.rs b/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_rendering.rs index 9dfbc3b5d8..d7baa3a076 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_rendering.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_rendering.rs @@ -9,7 +9,7 @@ use crate::compiler::tests::helpers::{ generated_with_bodies_stay_nested_verbatim, repeated_model_sql_renders_like_separate_batches, shared_model_cte_names_stay_nested_on_nested_with_dialects, snowflake_function_synonyms_stay_as_authored, trailing_statement_terminators_are_dropped, - tsql_distinct_ctes_lift_verbatim, tsql_fixture_and_model_cte_collisions_are_refused, + tsql_distinct_ctes_lift_verbatim, tsql_fixture_and_model_cte_collisions_are_renamed, tsql_identical_helper_ending_in_line_comment_is_shared, tsql_model_cte_collisions_are_renamed_by_token_span, tsql_unprovable_cte_renames_are_refused_with_named_ctes, @@ -40,8 +40,8 @@ fn given_sql_rendering_cases_when_rendering_native_batches_then_expected_behavio expected_success: true, }, SqlTestRenderingTestCase { - description: "T-SQL fixture and model CTE collisions are refused", - run: tsql_fixture_and_model_cte_collisions_are_refused, + description: "T-SQL fixture and model CTE collisions are renamed by token span", + run: tsql_fixture_and_model_cte_collisions_are_renamed, expected_success: true, }, SqlTestRenderingTestCase { diff --git a/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs b/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs index b42cb7e7ed..0005b34f4e 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs @@ -74,3 +74,41 @@ pub(crate) struct RelationMarkerOracleTestCase { pub(crate) dialect: &'static str, pub(crate) expected_mismatches: &'static [&'static str], } + +/// The rendered shape one analysis mode must produce: CTE order pairs and SQL fragments. +pub(crate) struct RenderedShapeTestCase { + pub(crate) description: &'static str, + pub(crate) sql_analysis_enabled: bool, + pub(crate) expected_order: &'static [(&'static str, &'static str)], + pub(crate) expected_fragments: &'static [&'static str], + pub(crate) expected_absent_fragments: &'static [&'static str], +} + +pub(crate) struct HelperRenameTestCase { + pub(crate) description: &'static str, + pub(crate) sql: &'static str, + pub(crate) expected_sql: Option<&'static str>, +} + +/// One model and the helper named after something it reads. +pub(crate) struct SharedNameTestCase { + pub(crate) description: &'static str, + pub(crate) model_sql: &'static str, + pub(crate) helper_name: &'static str, + pub(crate) expected_model_reads: &'static str, + pub(crate) expected_isolated: bool, +} + +pub(crate) struct NameRefusalTestCase { + pub(crate) description: &'static str, + pub(crate) shape: PlanShape<'static>, + pub(crate) expected_message: &'static str, +} + +pub(crate) struct PlanShape<'a> { + pub(crate) dialect: &'a str, + pub(crate) sql_analysis_enabled: bool, + pub(crate) model_sql: &'a str, + pub(crate) helper_name: &'a str, + pub(crate) expected_sql: &'a str, +} 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 13ee48ccc8..9a4cb99578 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md @@ -98,8 +98,11 @@ The trailing `SELECT 1` is required as a ceremonial closing statement. | `__assert__` | Zero-row assertion. Passes if the query returns no rows; fails with the returned rows as diagnostics. | | `__macro__` | Mock a macro. Replaces every `@(...)` call with the mock value. | -Any CTE without one of these prefixes is treated as a helper CTE, available to mock, model, -`__expected__`, and `__assert__` SQL in the test. +Any CTE without one of these prefixes is treated as a helper CTE, available to mock, helper, +`__expected__`, and `__assert__` SQL in the test. Models never see helpers: a model's own CTEs and +the tables it reads keep their meaning even when a helper shares their name. A CTE inside another +test CTE must not redefine a helper's name, and a model CTE must not use a name SQLBuild generates +for the test query, such as `__ref__orders` or `__helper__expected_rows`. SQLBuild splits test files into CTEs and finds `__ref`, `__source`, `__seed`, `__dbt_ref`, `__udf` and `__table_fn` calls outside comments and quoted text, in tests, scenarios, models, 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 665e6a98d7..1306b8d223 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 @@ -2,7 +2,9 @@ from __future__ import annotations +from collections.abc import Mapping from dataclasses import dataclass, field +from pathlib import Path @dataclass(frozen=True) @@ -261,3 +263,40 @@ class JsonDurationE2ETestCase: expected_order_lines_sql: str expected_exit_code: int expected_status: str + + +@dataclass(frozen=True) +class ParameterizedJsonE2ETestCase: + """Parameterized SQL tests whose cases must each appear in structured JSON output.""" + + description: str + expected_checks: tuple[tuple[str, str], ...] + + +@dataclass(frozen=True) +class HelperIsolationE2ETestCase: + """A helper named like a physical table or a model CTE, read only by a fixture.""" + + description: str + sql_analysis_enabled: bool + expected_output_fragments: tuple[str, ...] + + +@dataclass(frozen=True) +class IsolatedCompileE2ETestCase: + """A project whose compiled SQL tests must keep helper and model CTE names apart.""" + + description: str + project_files: Mapping[str, str] + compile_arguments: tuple[str, ...] + helper_names: tuple[str, ...] + expected_minimum_tests: int + + +@dataclass(frozen=True) +class FixtureProjectIsolationE2ETestCase: + """A fixture project whose compiled SQL tests must keep CTE names apart.""" + + description: str + project_dir: Path + expected_minimum_tests: int 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 bc298113a2..e96ac85bae 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/helpers.py @@ -2,6 +2,12 @@ from __future__ import annotations +import json +import re +from typing import Any + +import duckdb + def build_dynamic_pivot_test_project_files() -> dict[str, str]: """Build a Snowflake dynamic-pivot project for offline test-plan inspection.""" @@ -1610,3 +1616,67 @@ def build_shared_cte_name_chain_project_files( "SELECT 1\n" ), } + + +_CTE_ENTRY_KEY: re.Pattern[str] = re.compile(r'\{"key": "((?:[^"\\]|\\.)*)", "value": \{"aliases"') + + +def rendered_cte_names(sql: str) -> tuple[set[str], set[str]]: + """Return a rendered test query's top-level CTE names and every CTE name nested below them.""" + + row: tuple[Any, ...] | None = duckdb.sql( + "SELECT json_serialize_sql($sql)", params={"sql": sql} + ).fetchone() + assert row is not None + entries: list[dict[str, Any]] = json.loads(row[0])["statements"][0]["node"]["cte_map"]["map"] + top_level: set[str] = {entry["key"].lower() for entry in entries} + nested_json: str = json.dumps([entry["value"] for entry in entries]) + nested: set[str] = {name.lower() for name in _CTE_ENTRY_KEY.findall(nested_json)} + return top_level, nested + + +def assert_test_query_names_isolated(sql: str, helper_names: tuple[str, ...]) -> None: + """Assert no nested CTE reuses a top-level name and no helper keeps its name at top level.""" + + top_level, nested = rendered_cte_names(sql) + assert not top_level & nested, (top_level & nested, sql) + assert not top_level & {name.lower() for name in helper_names}, sql + + +def build_lookup_project_files(*, sql_analysis_enabled: bool) -> dict[str, str]: + """Build models reading a physical table and a model CTE named like a fixture-only helper.""" + + sql_analysis_value: str = {False: "false", True: "true"}[sql_analysis_enabled] + return { + "sqlbuild_project.toml": ( + 'name = "lookup"\nadapter = "duckdb"\n\n[connection]\n' + 'database = "lookup.duckdb"\n\n[settings]\n' + f"sql_analysis = {sql_analysis_value}\n" + ), + "sources/raw.yml": ( + "sources:\n - name: raw_orders\n description: Test source raw_orders.\n" + " schema: main\n table: raw_orders\n" + ), + "models/matched_orders.sql": ( + "MODEL (description 'Orders found in the lookup table.');\n\n" + "WITH lookup_rows AS (SELECT order_id FROM main.lookup_rows)\n" + 'SELECT o.order_id FROM __source("raw_orders") AS o\n' + "INNER JOIN lookup_rows AS l ON l.order_id = o.order_id\n" + ), + "models/listed_orders.sql": ( + "MODEL (description 'Orders listed in the physical lookup table.');\n\n" + 'SELECT o.order_id FROM __source("raw_orders") AS o\n' + "INNER JOIN lookup_rows AS l ON l.order_id = o.order_id\n" + ), + "tests/unit/lookup.sql": ( + 'TEST (name "lookup");\n\n' + "WITH\n" + "lookup_rows AS (SELECT 99 AS order_id),\n" + "__source__raw_orders AS (\n" + " SELECT 1 AS order_id UNION ALL SELECT order_id FROM lookup_rows\n" + "),\n" + "__expected__matched_orders AS (SELECT 1 AS order_id),\n" + "__expected__listed_orders AS (SELECT 1 AS order_id)\n" + "SELECT 1\n" + ), + } diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_authored_sql_parity.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_authored_sql_parity.py index 7c0444d37a..22ebdcb535 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_authored_sql_parity.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_authored_sql_parity.py @@ -36,9 +36,10 @@ " WITH ranked AS (SELECT order_id, amount, tag FROM __ref__stg_orders)\n" " SELECT order_id, amount * 2 AS doubled, tag FROM ranked\n),\n" "helper_rows AS (SELECT order_id, doubled, tag FROM totals)", - "\nhelper_rows AS (SELECT 1 AS order_id, 20 AS doubled, 'x' AS tag),\n", + "\n__helper__helper_rows AS (SELECT 1 AS order_id, 20 AS doubled, 'x' AS tag),\n", "__actual__orders AS (SELECT * FROM __ref__orders)", - "__expected__orders AS (SELECT order_id, doubled, tag FROM helper_rows)", + "__expected__orders AS (SELECT order_id, doubled, tag FROM __helper__helper_rows " + "AS helper_rows)", ), ), ), @@ -72,7 +73,7 @@ def test_given_authored_ctes_when_testing_then_passes_and_runs_the_authored_sql( "test_case", ( AuthoredSqlParityTestCase( - description="a WITH model ending in a statement terminator", + description="a WITH model ending in a terminator read by a later step and assert", compiled_test_name="test_m.sql", expected_fragments=( "__ref__m AS (WITH a AS (SELECT id FROM __source__raw_orders)\nSELECT id FROM a)", @@ -104,6 +105,8 @@ def test_given_model_ending_in_semicolon_when_testing_then_terminator_is_dropped ).read_text() for fragment in test_case.expected_fragments: assert fragment in sql, sql + assert sql.count("__ref__m AS (") == 1, sql + assert ";" not in sql and "__sqb_cte_" not in sql, sql if __name__ == "__main__": diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_name_isolation.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_name_isolation.py new file mode 100644 index 0000000000..377791ca75 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_name_isolation.py @@ -0,0 +1,158 @@ +"""Test helpers never share a name with model CTEs or relations through the real DuckDB CLI.""" + +import shutil +from pathlib import Path +from subprocess import CompletedProcess + +import duckdb +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.test._test_types import ( + FixtureProjectIsolationE2ETestCase, + HelperIsolationE2ETestCase, + IsolatedCompileE2ETestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.test.helpers import ( + assert_test_query_names_isolated, + build_helper_scope_project_files, + build_lookup_project_files, + build_shared_cte_name_chain_project_files, +) +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import ( + REPO_ROOT, + WAFFLE_SHOP_DIR, + prepare_inline_project, + run_sqb, +) + +_PASSING_ORDER_LINES_SQL: str = "SELECT 1 AS order_id, 20 AS line_total" + + +@pytest.mark.parametrize( + "test_case", + ( + HelperIsolationE2ETestCase( + description="analysed models read the physical table, not the fixture helper", + sql_analysis_enabled=True, + expected_output_fragments=("PASS=1 FAIL=0",), + ), + HelperIsolationE2ETestCase( + description="textual models read the physical table, not the fixture helper", + sql_analysis_enabled=False, + expected_output_fragments=("PASS=1 FAIL=0",), + ), + ), + ids=lambda case: case.description, +) +def test_given_helper_named_like_relations_models_read_when_testing_then_models_read_their_own( + tmp_path: Path, test_case: HelperIsolationE2ETestCase +) -> None: + project: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="lookup", + repo_files=build_lookup_project_files(sql_analysis_enabled=test_case.sql_analysis_enabled), + ) + with duckdb.connect(str(project / "lookup.duckdb")) as connection: + connection.execute("CREATE TABLE main.lookup_rows AS SELECT 1 AS order_id") + + tested: CompletedProcess[str] = run_sqb(command=("--no-color", "test"), project_dir=project) + + output: str = tested.stdout + tested.stderr + assert tested.returncode == 0, output + for fragment in test_case.expected_output_fragments: + assert fragment in output, output + sql: str = next((project / "target" / "run" / "tests").rglob("lookup.sql")).read_text() + assert_test_query_names_isolated(sql, ("lookup_rows",)) + + +@pytest.mark.parametrize( + "test_case", + ( + FixtureProjectIsolationE2ETestCase( + description="e2e waffle shop fixture", + project_dir=WAFFLE_SHOP_DIR, + expected_minimum_tests=1, + ), + FixtureProjectIsolationE2ETestCase( + description="website waffle shop example", + project_dir=REPO_ROOT / "website" / "examples" / "waffle-shop", + expected_minimum_tests=1, + ), + ), + ids=lambda case: case.description, +) +def test_given_fixture_project_when_compiling_tests_then_cte_names_are_isolated( + tmp_path: Path, test_case: FixtureProjectIsolationE2ETestCase +) -> None: + project: Path = tmp_path / test_case.project_dir.name + shutil.copytree(test_case.project_dir, project) + + compiled: CompletedProcess[str] = run_sqb( + command=("--no-color", "compile"), project_dir=project + ) + + assert compiled.returncode == 0, compiled.stdout + compiled.stderr + paths: list[Path] = sorted((project / "target" / "compiled" / "tests").rglob("*.sql")) + assert len(paths) >= test_case.expected_minimum_tests + for path in paths: + assert_test_query_names_isolated(path.read_text(), ()) + + +@pytest.mark.parametrize( + "test_case", + ( + IsolatedCompileE2ETestCase( + description="analysed chain with shared model and helper CTE names", + project_files=build_shared_cte_name_chain_project_files( + sql_analysis_enabled=True, expected_order_lines_sql=_PASSING_ORDER_LINES_SQL + ), + compile_arguments=(), + helper_names=("base_rows", "expected_rows"), + expected_minimum_tests=1, + ), + IsolatedCompileE2ETestCase( + description="textual chain with shared model and helper CTE names", + project_files=build_shared_cte_name_chain_project_files( + sql_analysis_enabled=False, expected_order_lines_sql=_PASSING_ORDER_LINES_SQL + ), + compile_arguments=("--no-sql-analysis",), + helper_names=("base_rows", "expected_rows"), + expected_minimum_tests=1, + ), + IsolatedCompileE2ETestCase( + description="analysed helpers read by expected rows and assertions", + project_files=build_helper_scope_project_files(), + compile_arguments=(), + helper_names=("doubled", "expected_rows"), + expected_minimum_tests=1, + ), + IsolatedCompileE2ETestCase( + description="textual helpers read by expected rows and assertions", + project_files=build_helper_scope_project_files(), + compile_arguments=("--no-sql-analysis",), + helper_names=("doubled", "expected_rows"), + expected_minimum_tests=1, + ), + ), + ids=lambda case: case.description, +) +def test_given_helper_projects_when_compiling_tests_then_helpers_keep_no_top_level_name( + tmp_path: Path, test_case: IsolatedCompileE2ETestCase +) -> None: + project: Path = prepare_inline_project( + tmp_path=tmp_path, project_name="isolation", repo_files=test_case.project_files + ) + + compiled: CompletedProcess[str] = run_sqb( + command=("--no-color", "compile", *test_case.compile_arguments), project_dir=project + ) + + assert compiled.returncode == 0, compiled.stdout + compiled.stderr + paths: list[Path] = sorted((project / "target" / "compiled" / "tests").rglob("*.sql")) + assert len(paths) >= test_case.expected_minimum_tests + for path in paths: + assert_test_query_names_isolated(path.read_text(), test_case.helper_names) + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_scope.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_scope.py index e05e1081f5..d7cdd2880b 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_scope.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_helper_scope.py @@ -61,5 +61,7 @@ def test_given_helper_ctes_when_expected_and_assertions_use_them_then_tests_comp compiled_sql: str = next( (project_dir / "target" / "compiled" / "tests").rglob("helper_reads_mock_and_helper.sql") ).read_text(encoding="utf-8") - assert compiled_sql.count("\ndoubled AS (") == 1, compiled_sql - assert compiled_sql.count("\nexpected_rows AS (") == 1, compiled_sql + assert compiled_sql.count("\n__helper__doubled AS (") == 1, compiled_sql + assert compiled_sql.count("\n__helper__expected_rows AS (") == 1, compiled_sql + assert "\ndoubled AS (" not in compiled_sql, compiled_sql + assert "\nexpected_rows AS (" not in compiled_sql, compiled_sql diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_model_inlining.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_model_inlining.py index a491ed1118..b00457367e 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_model_inlining.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_model_inlining.py @@ -8,6 +8,7 @@ from tests.e2e.src.sqlbuild.cli.commands.main.test._test_types import ModelInliningE2ETestCase from tests.e2e.src.sqlbuild.cli.commands.main.test.helpers import ( ORDER_TOTALS_EXPECTED_ROWS_SQL, + assert_test_query_names_isolated, build_shared_cte_name_chain_project_files, ) from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project, run_sqb @@ -17,8 +18,10 @@ "__ref__order_totals AS (", "amount * 2 AS line_total", "line_total + 1 AS order_total", - "base_rows AS (SELECT 1 AS order_id, 10 AS amount)", - f"expected_rows AS ({ORDER_TOTALS_EXPECTED_ROWS_SQL})", +) +_HELPER_DEFINITIONS: tuple[str, ...] = ( + "__helper__base_rows AS (SELECT 1 AS order_id, 10 AS amount)", + f"__helper__expected_rows AS ({ORDER_TOTALS_EXPECTED_ROWS_SQL})", ) _EXPECTED_FRAGMENTS: tuple[str, ...] = ( "__actual__order_lines AS (SELECT * FROM __ref__order_lines)", @@ -80,6 +83,7 @@ def test_given_chained_models_sharing_cte_names_when_testing_then_each_model_is_ for fragment in test_case.expected_output_fragments: assert fragment in output, output assert compiled.returncode == 0, compiled.stdout + compiled.stderr + analysed: bool = test_case.sql_analysis_enabled for artifact_root in ("run", "compiled"): sql: str = next( (project / "target" / artifact_root / "tests").rglob("order_chain.sql") @@ -88,7 +92,10 @@ def test_given_chained_models_sharing_cte_names_when_testing_then_each_model_is_ assert sql.count(fragment) == 1, f"{fragment!r} in {artifact_root}:\n{sql}" for fragment in _EXPECTED_FRAGMENTS: assert fragment in sql, sql - assert "WITH expected_rows" not in sql, sql + assert_test_query_names_isolated(sql, ("base_rows", "expected_rows")) + helper_counts: list[int] = [sql.count(fragment) for fragment in _HELPER_DEFINITIONS] + expected_counts: list[int] = [int(analysed) for _ in _HELPER_DEFINITIONS] + assert helper_counts == expected_counts, f"{artifact_root}:\n{sql}" if __name__ == "__main__": 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 8704acfdc0..4b394c821e 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 @@ -922,8 +922,10 @@ def test_given_assertion_only_sql_test_when_assertion_returns_rows_then_it_fails description="model test replaces complete table function invocation", expected_stdout_fragment="PASS=1", expected_artifact_fragments=( - "WITH fixture_rows AS", - "FROM fixture_rows", + "WITH __helper__fixture_rows AS (SELECT 7 AS order_id)", + "__table_fn__customer_orders AS (SELECT order_id FROM __helper__fixture_rows " + "AS fixture_rows)", + "FROM __table_fn__customer_orders", "__actual__orders", ), unexpected_artifact_fragments=("customer_orders(7)", "customer_id * 100"), diff --git a/tests/integration/src/sqlbuild/executor/testing/helpers.py b/tests/integration/src/sqlbuild/executor/testing/helpers.py index b7d7d40518..77801bf3be 100644 --- a/tests/integration/src/sqlbuild/executor/testing/helpers.py +++ b/tests/integration/src/sqlbuild/executor/testing/helpers.py @@ -246,7 +246,7 @@ def comparison_rows(*, adapter: DuckDbAdapter, connection: Any, sql: str) -> lis def build_terminated_model_project_files() -> dict[str, str]: - """Build a DuckDB project whose WITH model ends in a statement terminator.""" + """Build a DuckDB project whose WITH model ends in a terminator and is read downstream.""" return { "sqlbuild_project.toml": ( @@ -262,11 +262,17 @@ def build_terminated_model_project_files() -> dict[str, str]: 'WITH a AS (SELECT id FROM __source("raw_orders"))\n' "SELECT id FROM a;\n" ), + "models/m_next.sql": ( + "MODEL (description 'Test model m_next.', materialized table);\n\n" + 'SELECT id + 1 AS id FROM __ref("m")\n' + ), "tests/unit/test_m.sql": ( "TEST();\n\n" "WITH\n" "__source__raw_orders AS (SELECT 1 AS id),\n" - "__expected__m AS (SELECT 1 AS id)\n" + "__expected__m AS (SELECT 1 AS id),\n" + "__expected__m_next AS (SELECT 2 AS id),\n" + '__assert__m_has_one_row AS (SELECT id FROM __ref("m") WHERE id <> 1)\n' "SELECT 1\n" ), } diff --git a/tests/unit/src/sqlbuild/compiler/planner/_helpers/sql_test_assembly/test_plan_test.py b/tests/unit/src/sqlbuild/compiler/planner/_helpers/sql_test_assembly/test_plan_test.py index 08740712e6..943ae3ae90 100644 --- a/tests/unit/src/sqlbuild/compiler/planner/_helpers/sql_test_assembly/test_plan_test.py +++ b/tests/unit/src/sqlbuild/compiler/planner/_helpers/sql_test_assembly/test_plan_test.py @@ -1488,7 +1488,7 @@ def test_given_adapter_function_when_resolving_with_analysis_then_keeps_authored "test_case", [ PlanTestChainTestCase( - description="model and fixture CTEs share a name on T-SQL", + description="model and helper CTEs share a name on T-SQL", model_queries={ "orders": ( 'WITH helper_rows AS (SELECT order_id FROM __ref("raw_orders")) ' @@ -1500,36 +1500,38 @@ def test_given_adapter_function_when_resolving_with_analysis_then_keeps_authored helper_ctes={"helper_rows": "SELECT 2 AS order_id"}, expected_model_names=("orders",), expected_chain_length=1, - expected_error_fragments=( - "CTE 'helper_rows' of the expected rows of model 'orders' collides with " - "CTE 'helper_rows' of model 'orders'", - "T-SQL does not allow a nested WITH, so rename the CTE in the model or the fixture", - ), + expected_sql_fragments={ + "orders": "__helper__helper_rows AS (SELECT 2 AS order_id)", + "__actual__orders": "helper_rows AS (SELECT order_id FROM __ref__raw_orders)", + "__expected__orders": ( + "__expected__orders AS (SELECT order_id FROM __helper__helper_rows AS " + "helper_rows)" + ), + }, expected_cte_bodies={"orders": "SELECT order_id FROM helper_rows"}, ) ], ids=lambda case: case.description, ) -def test_given_tsql_cte_name_collision_when_planning_then_test_is_refused_naming_the_ctes( +def test_given_tsql_shared_cte_name_when_planning_then_helper_and_model_cte_stay_distinct( test_case: PlanTestChainTestCase, ) -> None: compiled_test: CompiledSqlTest project: CompiledProject compiled_test, project = build_test_and_project(test_case) + adapter: BaseAdapter = build_comparison_test_adapter("sqlserver") - result: SqlTestPlanResult - result, _ = plan_single_test_allowing_errors( - test=compiled_test, - project=project, - adapter=build_comparison_test_adapter("sqlserver"), - sql_analysis_enabled=True, + entry, _ = plan_single_test( + test=compiled_test, project=project, adapter=adapter, sql_analysis_enabled=True + ) + comparison_sql: str = build_sql_test_comparison_sql( + test_entry=entry, + set_difference_operator=adapter.render_set_difference_operator(), + sql_analysis_dialect=adapter.sql_analysis_dialect(), ) - assert result.entry is None - for expected_fragment in test_case.expected_error_fragments: - assert any(expected_fragment in message for message in result.fixture_diagnostics), ( - result.fixture_diagnostics - ) + for expected_fragment in test_case.expected_sql_fragments.values(): + assert expected_fragment in comparison_sql, comparison_sql @pytest.mark.parametrize( diff --git a/website/src/content/docs/docs/concepts/testing.mdx b/website/src/content/docs/docs/concepts/testing.mdx index 34ebfda398..8bf92b634a 100644 --- a/website/src/content/docs/docs/concepts/testing.mdx +++ b/website/src/content/docs/docs/concepts/testing.mdx @@ -74,8 +74,11 @@ The trailing `SELECT 1` is required as a ceremonial closing statement. | `__assert__` | Zero-row assertion. Passes if the query returns no rows; fails with the returned rows as diagnostics. | | `__macro__` | Mock a macro. Replaces every `@(...)` call with the mock value. | -Any CTE without one of these prefixes is treated as a helper CTE, available to mock, model, -`__expected__`, and `__assert__` SQL in the test. +Any CTE without one of these prefixes is treated as a helper CTE, available to mock, helper, +`__expected__`, and `__assert__` SQL in the test. Models never see helpers: a model's own CTEs and +the tables it reads keep their meaning even when a helper shares their name. A CTE inside another +test CTE must not redefine a helper's name, and a model CTE must not use a name SQLBuild generates +for the test query, such as `__ref__orders` or `__helper__expected_rows`. SQLBuild splits test files into CTEs and finds `__ref`, `__source`, `__seed`, `__dbt_ref`, `__udf` and `__table_fn` calls outside comments and quoted text, in tests, scenarios, models, From 6a7a0c72722744692030b0e617da737a8c9951b6 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 6 Oct 2026 19:12:54 +0100 Subject: [PATCH 06/10] fix(test): ignore named windows when detecting CTE definitions A WINDOW clause entry such as `base AS (ORDER BY id)` was read as a CTE, so an assertion with a named window sharing a helper's name was refused. Only commas directly inside a WITH list now separate CTE definitions. --- .../compiler/_helpers/sql_tests/cte_rename.rs | 52 ++++++++++++++++++- .../tests/test_sql_test_cte_rename.rs | 34 +++++++++++- .../src/compiler/tests/test_types.rs | 6 +++ 3 files changed, 89 insertions(+), 3 deletions(-) diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_rename.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_rename.rs index 7e8aa4f839..9dc9adc61a 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_rename.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_rename.rs @@ -95,6 +95,7 @@ pub(crate) fn defined_cte_keys(sql: &str, dialect: SliceDialect) -> Option = Vec::new(); for (index, token) in code.iter().enumerate() { let Some(key) = token.key.as_deref() else { @@ -103,7 +104,7 @@ pub(crate) fn defined_cte_keys(sql: &str, dialect: SliceDialect) -> Option 0 && (keyword(index - 1, "with") || keyword(index - 1, "recursive") - || text(index - 1) == Some(",")); + || (text(index - 1) == Some(",") && in_with_list[index - 1])); if !opens_list { continue; } @@ -139,6 +140,55 @@ pub(crate) fn defined_cte_keys(sql: &str, dialect: SliceDialect) -> Option Vec { + let mut lists: Vec = vec![false]; + let mut in_list: Vec = Vec::with_capacity(code.len()); + for token in code { + match &sql[token.start..token.end] { + "(" => lists.push(false), + ")" => { + if lists.len() > 1 { + let _ = lists.pop(); + } + } + _ => { + if let (Some(key), Some(current)) = (token.key.as_deref(), lists.last_mut()) { + if key == "with" { + *current = true; + } else if LIST_ENDING_CLAUSES.contains(&key) { + *current = false; + } + } + } + } + in_list.push(lists.last().copied().unwrap_or(false)); + } + in_list +} + /// How one occurrence of a renamed CTE's name is used. #[derive(Clone, Copy, Debug, PartialEq, Eq)] enum Usage { diff --git a/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_cte_rename.rs b/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_cte_rename.rs index 9e68da2843..66413678b6 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_cte_rename.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_cte_rename.rs @@ -1,6 +1,6 @@ -use crate::compiler::_helpers::sql_tests::cte_rename::{CteRename, rename_ctes}; +use crate::compiler::_helpers::sql_tests::cte_rename::{CteRename, defined_cte_keys, rename_ctes}; use crate::compiler::_helpers::sql_tests::cte_slices::{SliceDialect, split_top_level_with}; -use crate::compiler::tests::test_types::CteRenameTestCase; +use crate::compiler::tests::test_types::{CteRenameTestCase, DefinedCteKeysTestCase}; #[test] fn given_tsql_cte_when_renaming_by_token_span_then_only_relation_references_change() { @@ -112,3 +112,33 @@ fn given_tsql_cte_when_renaming_by_token_span_then_only_relation_references_chan ); } } + +#[test] +fn given_statement_when_listing_defined_ctes_then_only_with_list_entries_count() { + let test_cases = [ + DefinedCteKeysTestCase { + description: "top-level and nested WITH lists are listed", + sql: "WITH base AS (SELECT 1 AS id), other AS (WITH inner_rows AS (SELECT id FROM base) \ + SELECT id FROM inner_rows) SELECT id FROM other", + expected_keys: &["base", "other", "inner_rows"], + }, + DefinedCteKeysTestCase { + description: "named windows are not CTEs", + sql: "SELECT order_id, row_number() OVER w1 AS n FROM orders \ + WINDOW w1 AS (ORDER BY order_id), base AS (PARTITION BY order_id)", + expected_keys: &[], + }, + DefinedCteKeysTestCase { + description: "a WITH list after a named window in a subquery is still listed", + sql: "SELECT * FROM (SELECT id FROM t WINDOW w AS (ORDER BY id), v AS (ORDER BY id)) AS s \ + JOIN (WITH base AS (SELECT 1 AS id), rows_two AS (SELECT 2 AS id) SELECT id FROM base) AS b \ + ON b.id = s.id", + expected_keys: &["base", "rows_two"], + }, + ]; + for test_case in test_cases { + let keys = + defined_cte_keys(test_case.sql, SliceDialect::new(Some("duckdb"))).expect("scans"); + assert_eq!(keys, test_case.expected_keys, "{}", 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 0005b34f4e..0dbecaf8ec 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs @@ -49,6 +49,12 @@ pub(crate) struct CteRenameTestCase { pub(crate) expected_sql: Option<&'static str>, } +pub(crate) struct DefinedCteKeysTestCase { + pub(crate) description: &'static str, + pub(crate) sql: &'static str, + pub(crate) expected_keys: &'static [&'static str], +} + pub(crate) struct MarkerNamesTestCase { pub(crate) description: &'static str, pub(crate) sql: &'static str, From c730d49e1fa4d6bd9562d0c4516af51fc7bbbfca Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 6 Oct 2026 19:16:49 +0100 Subject: [PATCH 07/10] fix(test): keep reserved-name and CTE-list checks within lint rules --- .../compiler/_helpers/sql_tests/cte_rename.rs | 27 +++---------------- .../compiler/_helpers/sql_tests/planning.rs | 13 +++++---- crates/sqlbuild-analysis/src/constants.rs | 22 +++++++++++++++ .../sql_test_assembly/test_plan_test.py | 2 +- 4 files changed, 34 insertions(+), 30 deletions(-) diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_rename.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_rename.rs index 9dc9adc61a..4d8ebbbbf9 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_rename.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_rename.rs @@ -3,6 +3,7 @@ use crate::compiler::_helpers::sql_tests::cte_slices::{ SliceDialect, WithSlices, opaque_end, read_identifier, }; +use crate::constants::{WITH_KEYWORD, WITH_LIST_ENDING_CLAUSES}; use sqlbuild_sqltext::sql_scan::main::comment_end::comment_end; use sqlbuild_sqltext::sql_scan::main::skip_whitespace::skip_whitespace; use sqlbuild_sqltext::sql_scan::models::Unclosed; @@ -140,28 +141,6 @@ pub(crate) fn defined_cte_keys(sql: &str, dialect: SliceDialect) -> Option Vec { let mut lists: Vec = vec![false]; @@ -176,9 +155,9 @@ fn with_list_commas(sql: &str, code: &[&Token]) -> Vec { } _ => { if let (Some(key), Some(current)) = (token.key.as_deref(), lists.last_mut()) { - if key == "with" { + if key == WITH_KEYWORD { *current = true; - } else if LIST_ENDING_CLAUSES.contains(&key) { + } else if WITH_LIST_ENDING_CLAUSES.contains(&key) { *current = false; } } 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 98bdee42a2..508cd181ad 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs @@ -798,11 +798,14 @@ fn reserved_cte_name(query_sql: &str, dialect: SliceDialect) -> Option { defined_cte_keys(query_sql, dialect) .unwrap_or_default() .into_iter() - .find(|key| { - key == SQL_TEST_ACTUAL_CTE - || key == SQL_TEST_EXPECTED_CTE - || prefixes.iter().any(|prefix| key.starts_with(prefix)) - }) + .find(|key| is_reserved_cte_key(key, &prefixes)) +} + +fn is_reserved_cte_key(key: &str, prefixes: &[&str]) -> bool { + if key == SQL_TEST_ACTUAL_CTE || key == SQL_TEST_EXPECTED_CTE { + return true; + } + prefixes.iter().any(|prefix| key.starts_with(prefix)) } fn plan_model_test( diff --git a/crates/sqlbuild-analysis/src/constants.rs b/crates/sqlbuild-analysis/src/constants.rs index 67a67562e8..ff04bf7eb4 100644 --- a/crates/sqlbuild-analysis/src/constants.rs +++ b/crates/sqlbuild-analysis/src/constants.rs @@ -9,3 +9,25 @@ pub(crate) const TABLE_FUNCTION_TEST_MODE: &str = "table_fn"; pub(crate) const UDF_TEST_MODE: &str = "udf"; pub(crate) const UNKNOWN_SQL_TYPE: &str = "UNKNOWN"; pub(crate) const VARCHAR_SQL_TYPE: &str = "VARCHAR"; +pub(crate) const WITH_KEYWORD: &str = "with"; +/// Clause keywords that end a WITH list at their parenthesis depth. +pub(crate) const WITH_LIST_ENDING_CLAUSES: &[&str] = &[ + "select", + "from", + "where", + "group", + "having", + "qualify", + "order", + "limit", + "window", + "union", + "except", + "intersect", + "minus", + "values", + "insert", + "update", + "delete", + "merge", +]; diff --git a/tests/unit/src/sqlbuild/compiler/planner/_helpers/sql_test_assembly/test_plan_test.py b/tests/unit/src/sqlbuild/compiler/planner/_helpers/sql_test_assembly/test_plan_test.py index 943ae3ae90..75a983f290 100644 --- a/tests/unit/src/sqlbuild/compiler/planner/_helpers/sql_test_assembly/test_plan_test.py +++ b/tests/unit/src/sqlbuild/compiler/planner/_helpers/sql_test_assembly/test_plan_test.py @@ -1320,7 +1320,7 @@ def test_given_sql_analysis_enabled_when_planning_test_then_it_uses_top_level_ge helper_ctes={}, expected_model_names=("orders",), expected_chain_length=1, - expected_error_fragments=("conflicts with the generated source CTE",), + expected_error_fragments=("names starting with a SQLBuild test prefix",), expected_cte_bodies={"orders": "SELECT 1 AS id"}, ) ], From a57f926e9af5df907977909e1502cb145d4eea2a Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 6 Oct 2026 19:49:07 +0100 Subject: [PATCH 08/10] test(compile): update performance-guard fingerprints for the new unit-test SQL shape The guards hash every compiled artifact, including compiled unit-test SQL, whose shape changed intentionally (models nested once under __ref__). Compile JSON and model artifacts are unchanged. --- .../main/compile/test_dense_compile_performance.py | 12 ++++++------ .../test_fresh_process_compile_cache_performance.py | 6 +++--- .../test_fresh_process_compile_performance.py | 6 +++--- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_dense_compile_performance.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_dense_compile_performance.py index 2d801ccd1d..bbcc61bd38 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_dense_compile_performance.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_dense_compile_performance.py @@ -43,28 +43,28 @@ 1000, 13.0, 800 * _MIB, - "6ea005eef07fe6d2ec47980bc573903efa060cfde55aa5488e63b29ce846280e", + "de23b14d87e954a65c2da91918fce062d158419edd63cc8cf3dc938b3e47cd44", ), DenseCompileGuardTestCase( "dense_models_3000_all_rules", 3000, 25.0, 3 * _GIB // 2, - "3d26ef1a4d30ea96dd97d60b2e2f6c93473e354099138a01d74b136700c9f022", + "196fca3e87989eb3e89e443b7ca93bb094198538d5bde32b9d6fcca58ffc8eed", ), DenseCompileGuardTestCase( "dense_models_5000_all_rules", 5000, 42.0, 9 * _GIB // 4, - "bc6b60e90118d66f3217a5f04f104ae91f657a6356f4d19844614d1ba48b0012", + "3477a472fa76d6236ebd0c9758d79c18f67f1bf55e9bbec05b002ba3744cfd22", ), DenseCompileGuardTestCase( "dense_models_10000_all_rules", 10000, 77.0, 13 * _GIB // 4, - "913a08c2277304efacf75e293728767a3fa416f6b32f5488dca6ab5baea80668", + "f2a574af6e8068a0634fa3c078b168dff9a475ada5d71502482ad2926c90e1c3", ), ), ids=lambda case: case.description, @@ -142,7 +142,7 @@ def test_given_dense_project_when_compiling_cold_then_preserves_rules_semantics_ 11.0, 12.0, 3 * _GIB // 2, - "3d26ef1a4d30ea96dd97d60b2e2f6c93473e354099138a01d74b136700c9f022", + "196fca3e87989eb3e89e443b7ca93bb094198538d5bde32b9d6fcca58ffc8eed", "f7f8a3d881496664ec7e293144d93b8b8e486a04eba830d9cc6500d1b28147f3", 2, ), @@ -153,7 +153,7 @@ def test_given_dense_project_when_compiling_cold_then_preserves_rules_semantics_ 19.0, 21.0, 9 * _GIB // 4, - "bc6b60e90118d66f3217a5f04f104ae91f657a6356f4d19844614d1ba48b0012", + "3477a472fa76d6236ebd0c9758d79c18f67f1bf55e9bbec05b002ba3744cfd22", "571fe58493234057c7128b4929d3518532731192f6fef85da0496788826bb172", 2, ), diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_fresh_process_compile_cache_performance.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_fresh_process_compile_cache_performance.py index 4773daca5b..9fe7614c3c 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_fresh_process_compile_cache_performance.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_fresh_process_compile_cache_performance.py @@ -57,7 +57,7 @@ expected_max_rss_bytes=2 * _GIB, expected_max_cache_bytes=96 * _MIB, expected_cold_fingerprint=( - "7a1978e9f8871b2252abea5e8763c570305f97342b3b3f51e2a2992238cc3798" + "1914b788abcb452422eaf6c3a2c240ff390bab9530217285a5ca93334db7b6c3" ), expected_leaf_edit_fingerprint=( "a041d989ef3810e0840361a4f793548b428f941da2f265462a4b46a5bf4d7676" @@ -91,7 +91,7 @@ expected_max_rss_bytes=2 * _GIB, expected_max_cache_bytes=160 * _MIB, expected_cold_fingerprint=( - "d293f1e9dbe2787d21fa6e5f20c79206ad9beea1e3be63299ac0a5a1e3464ffd" + "2f1e5dfd29025c25e0dffc6e74f4c2be020a5ef133ecd081f44e0c55c24858b4" ), expected_leaf_edit_fingerprint=( "a2e0a328277276f960bef73b4fbc85f9dcacbfad73c10af2df129b78592606ba" @@ -125,7 +125,7 @@ expected_max_rss_bytes=2 * _GIB, expected_max_cache_bytes=320 * _MIB, expected_cold_fingerprint=( - "8994dd1e87cb97312d492ea66731e4f01f4da08dc2680dd6814f1e4bba23a775" + "ecdd7f2cf961fab739aa806b6d90283916cffad2007e1e2f3ba90a466db6cb75" ), expected_leaf_edit_fingerprint=( "f1f25d949e182901656fcef88c7c566254c847174b888bd35c87372aea347807" diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_fresh_process_compile_performance.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_fresh_process_compile_performance.py index eac764b865..1e4a0a879d 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_fresh_process_compile_performance.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_fresh_process_compile_performance.py @@ -39,7 +39,7 @@ expected_max_wall_seconds=17.0, expected_max_rss_bytes=int(1.5 * _GIB), expected_semantic_fingerprint=( - "24d42df4968ff361ac0e07e2dbe03abd13bc6948b6812b8188f10d7b0aa9fef4" + "d935821df54043fabc3a6911c1926578178c9b744f2a589cbef99a10d5207543" ), ), FreshProcessCompilePerformanceGuardTestCase( @@ -54,7 +54,7 @@ expected_max_wall_seconds=25.0, expected_max_rss_bytes=int(1.75 * _GIB), expected_semantic_fingerprint=( - "4fd79eff9a628608a2f9836e1325cec58f9f3d5ac870e284c734f09c505c700f" + "874cbfd473846c6a335ec2f79cb6b7b0b77e8e8d58652815fad1e9d59d423c46" ), ), FreshProcessCompilePerformanceGuardTestCase( @@ -69,7 +69,7 @@ expected_max_wall_seconds=50.0, expected_max_rss_bytes=2 * _GIB, expected_semantic_fingerprint=( - "d83928b7553bcc58564eabc4669045a63e5b314726672ada0a3d979e54ab8573" + "5ccdabeaabfdfba1ecbae80259dd3c6eb801d683df140b7fb7a249b9555308c6" ), ), ], From a24b11b042c5df872f473b58ef2d08318c923040 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 6 Oct 2026 20:01:15 +0100 Subject: [PATCH 09/10] test(compile): update after-edit guard fingerprints for the new unit-test SQL shape --- .../compile/test_dense_compile_performance.py | 4 ++-- ..._fresh_process_compile_cache_performance.py | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_dense_compile_performance.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_dense_compile_performance.py index bbcc61bd38..606879ed41 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_dense_compile_performance.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_dense_compile_performance.py @@ -143,7 +143,7 @@ def test_given_dense_project_when_compiling_cold_then_preserves_rules_semantics_ 12.0, 3 * _GIB // 2, "196fca3e87989eb3e89e443b7ca93bb094198538d5bde32b9d6fcca58ffc8eed", - "f7f8a3d881496664ec7e293144d93b8b8e486a04eba830d9cc6500d1b28147f3", + "1afe754dd8c66bc36e38a40c66e9183ecbaf365fd8d87a784069cd615c3ce64a", 2, ), DenseWarmEditCompileGuardTestCase( @@ -154,7 +154,7 @@ def test_given_dense_project_when_compiling_cold_then_preserves_rules_semantics_ 21.0, 9 * _GIB // 4, "3477a472fa76d6236ebd0c9758d79c18f67f1bf55e9bbec05b002ba3744cfd22", - "571fe58493234057c7128b4929d3518532731192f6fef85da0496788826bb172", + "dfbada4c59a3ab8ec7733784890d74c7b64e9b0adcb03364baaabef5bdba5574", 2, ), ), diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_fresh_process_compile_cache_performance.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_fresh_process_compile_cache_performance.py index 9fe7614c3c..34566e6567 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_fresh_process_compile_cache_performance.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_fresh_process_compile_cache_performance.py @@ -60,13 +60,13 @@ "1914b788abcb452422eaf6c3a2c240ff390bab9530217285a5ca93334db7b6c3" ), expected_leaf_edit_fingerprint=( - "a041d989ef3810e0840361a4f793548b428f941da2f265462a4b46a5bf4d7676" + "3a9480f952f11e16177274185e8bfda7eb0e65953a8ed1c8b21ad3cdb7d0b07d" ), expected_macro_edit_fingerprint=( - "8b62365b60a9f60afaffe6571d6849a187480d503a3114f8783050ebed46363b" + "552931889359ce882b18d4fb11fdc87341e60a8a4d2b26ad0f1635138c8b9ad8" ), expected_project_config_fingerprint=( - "d077b4676e827b41383cb199f342867995ec1a5564e859e85558a24d1201abf6" + "6bf4f742469eee2be7eb5bbf2411564c924011fea0bd43becd80db4027022dac" ), macro_call_interval=6, scoped_macros=True, @@ -94,13 +94,13 @@ "2f1e5dfd29025c25e0dffc6e74f4c2be020a5ef133ecd081f44e0c55c24858b4" ), expected_leaf_edit_fingerprint=( - "a2e0a328277276f960bef73b4fbc85f9dcacbfad73c10af2df129b78592606ba" + "708f04a565a71b3981c9beba420aff691c8467eb30c5cdedd45c2518d45b31ba" ), expected_macro_edit_fingerprint=( - "38779c13ac071bf082455f27cdc1d071060da243b9a3d47f0f34b384962b5d11" + "7dd4d45c2b21f61cbc32d33b0e0cf98721975b9f8948277cc15dc947355cfc25" ), expected_project_config_fingerprint=( - "a26f85f97e7c28d40056f95d6be646399b03b215f0a24884f4c26b7b3e996595" + "69f4fe914afc955d690e9c5b9074ca69dc58b8b7a0886a421b888385e6b066cb" ), macro_call_interval=6, scoped_macros=True, @@ -128,13 +128,13 @@ "ecdd7f2cf961fab739aa806b6d90283916cffad2007e1e2f3ba90a466db6cb75" ), expected_leaf_edit_fingerprint=( - "f1f25d949e182901656fcef88c7c566254c847174b888bd35c87372aea347807" + "f67b73b56d7ad7677cbfa649e7933e8f294f9dd7c99f3660d7c42e1d01b804ff" ), expected_macro_edit_fingerprint=( - "b8f3cc27552a0d49e97b112732a3b6331040f0dcdec6ed15cc7058a21e98aa4f" + "5554615dbc5fe9dbfd7555771167f3380222cb3da468b4313076adecef0ff9c3" ), expected_project_config_fingerprint=( - "ff3467ff64bfbfa1073a169d53576bfee8699c2529a37250c44e6cc2ecd11845" + "ebc15f5381746d060005ecc61f477b1b0338640198197f6788f9beab5da88db5" ), macro_call_interval=6, scoped_macros=True, From bd5fd21b000b5cd4523715f95e4fba0bef657300 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 6 Oct 2026 20:23:23 +0100 Subject: [PATCH 10/10] test(compile): update varied-graph guard fingerprints for the new unit-test SQL shape --- .../main/compile/test_varied_compile_cache_performance.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_varied_compile_cache_performance.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_varied_compile_cache_performance.py index b079f15144..cfbe7d947a 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_varied_compile_cache_performance.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_varied_compile_cache_performance.py @@ -44,10 +44,10 @@ 150, 250, ( - "8ef4dcbf6abc8750aee9ae740b09bfd442e6593f5176f059bdc2e1ce01ec538c", - "7c3a736acd69d910a3c612180c982e44a8a07473713f533ab8c5a90a7ecee3e4", - "43a14caf30367ca9e2b966d20ab98b83b2ab8fb36312956f7a11bf001944a0a6", - "b041d70eef587492ab936e98d527fd76895eff69e3d5224e48419e73ed426bf3", + "dd9941690ce99c8e139df0b4482153b1f6008c59ab4871c48fed9ae733f72af3", + "61a2cb11ebd6ef3799019ecadf4b6fb4517564a3e6164198980689013abcd549", + "9a5f8a278f11b1c76e03e0183e766effc454a3bd5947984155a45f2b1d96740c", + "5550d9e3efd736e314402cc42b8b4ef540105b88a5590b383b7012fdab4b2ed8", ), ), ),