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..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; @@ -82,6 +83,91 @@ 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 in_with_list = with_list_commas(sql, &code); + 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(",") && in_with_list[index - 1])); + 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) +} + +/// Whether each token sits directly in a WITH list, so a comma there separates CTEs. +fn with_list_commas(sql: &str, code: &[&Token]) -> 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_KEYWORD { + *current = true; + } else if WITH_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/_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/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 f26728038a..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,10 +2,18 @@ use std::collections::{BTreeMap, HashMap, HashSet}; +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, with_helper_ctes, + 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, @@ -26,6 +34,42 @@ 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, + pub(crate) mock_name: Option<&'a str>, + 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), @@ -35,11 +79,13 @@ enum ScopeNode { /// Name references between one test's helper and mock CTEs, tokenized once per test. #[derive(Default)] pub(crate) struct ScopeGraph { - 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. @@ -47,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); } } @@ -69,49 +119,137 @@ 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: with_helper_ctes(body, &fixtures.helpers), + sql: body.clone(), tokens: identifier_tokens(body, patterns), }); } } - Self { - 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 { @@ -127,46 +265,155 @@ 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 generated CTE body of one mock, with every helper CTE in scope. + /// 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()) } - /// The parenthesized body of one mock inlined in place of its relation marker. - pub(crate) fn inlined_mock_sql(&self, generated_name: &str) -> Option { - self.mock_sql(generated_name).map(|sql| format!("({sql})")) + /// 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)) + } + } } - /// 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> { + fn with_read_helpers(&self, sql: &str, tokens: &[String]) -> String { + let helpers: Vec<(String, String)> = self + .closure(tokens, false) + .into_iter() + .filter_map(|node| match node { + ScopeNode::Helper(index) => Some(( + self.helpers[index].name.clone(), + self.helpers[index].original.clone(), + )), + ScopeNode::Mock(_) => None, + }) + .collect(); + with_leading_ctes(&helpers, sql) + } + + /// 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.closure(&self.mocks[root].tokens) + self.closure(&self.mocks[root].tokens, true) .into_iter() - .filter_map(|node| match node { - ScopeNode::Mock(index) if index != root => Some(&self.mocks[index]), - _ => None, - }) + .filter_map(|node| self.dependency(node, Some(root))) .collect() } + + /// 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 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(), + )); + } + } + scope + } + + 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].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. @@ -176,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 0dfeae02ca..508cd181ad 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,11 +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)>, + pub(crate) expected: BTreeMap, + pub(crate) assertions: Vec<(String, String)>, } struct DirectTestPlan { @@ -351,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) } @@ -429,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, @@ -492,6 +503,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 +529,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, )?; } @@ -764,13 +786,43 @@ 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| 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( 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, @@ -802,16 +854,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.helpers, - &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, @@ -850,8 +908,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); @@ -862,10 +920,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, @@ -875,17 +933,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.helpers, - &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, @@ -924,7 +981,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, @@ -932,15 +989,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, @@ -1220,12 +1278,21 @@ 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 { - generated_state.insert(name, sql, request.file_label)?; + 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)?; + } } let generated_name = format!("{REF_PREFIX}{referenced_name}"); generated_state.insert( @@ -1279,7 +1346,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, @@ -1331,7 +1402,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, @@ -1340,7 +1411,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, @@ -1357,10 +1428,9 @@ fn resolve_textual_sql(request: TextualResolutionRequest<'_>) -> Result) -> Result) -> 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 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(|mock| (mock.generated_name.clone(), mock.sql.clone())), + .map(|dependency| { + ( + dependency.generated_name.to_string(), + dependency.sql.to_string(), + ) + }) + .chain(std::iter::once((generated_name, mock_sql))), ); } Ok(TextualResolution { @@ -1429,25 +1512,54 @@ fn resolve_textual_sql(request: TextualResolutionRequest<'_>) -> Result, + ctes: Vec<(String, String)>, +} + fn resolve_table_function_fixtures( sql: &str, - fixtures: &BTreeMap, - helpers: &[CteInput], + 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| (format!("({})", with_helper_ctes(body, helpers)), 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( @@ -1658,8 +1770,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/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/rendering.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/rendering.rs index 543a5d4ba6..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,18 +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, 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; @@ -124,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, @@ -142,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) { @@ -195,19 +199,24 @@ 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); - } - 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), + }; + 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())); + self.merge_inputs( + &ctes, + &format!("SELECT * FROM {model_cte}"), enabled, &origin, ) @@ -221,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. @@ -242,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. @@ -406,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() @@ -419,17 +500,35 @@ 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(); + 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 }) +} + /// 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"), @@ -533,9 +632,12 @@ 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; @@ -590,8 +699,8 @@ 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" @@ -792,7 +799,8 @@ pub(crate) fn difference_sample_lifts_generated_ctes_and_bounds_rows() -> 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" @@ -1099,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}"); } @@ -1171,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!({ @@ -1203,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" }) @@ -1215,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(), @@ -1283,7 +1381,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 +1398,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 +1413,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 +1428,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 +1440,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", ) } @@ -1352,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", @@ -1362,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 { @@ -1588,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_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_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 44053015dc..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 @@ -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_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, @@ -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 { @@ -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..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, @@ -74,3 +80,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/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/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/.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/src/sqlbuild/cli/output/_helpers/execution_result_document.py b/src/sqlbuild/cli/output/_helpers/execution_result_document.py index 3252087a36..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, @@ -825,6 +826,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, @@ -837,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]: @@ -1180,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/compile/test_dense_compile_performance.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_dense_compile_performance.py index 2d801ccd1d..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 @@ -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,8 +142,8 @@ def test_given_dense_project_when_compiling_cold_then_preserves_rules_semantics_ 11.0, 12.0, 3 * _GIB // 2, - "3d26ef1a4d30ea96dd97d60b2e2f6c93473e354099138a01d74b136700c9f022", - "f7f8a3d881496664ec7e293144d93b8b8e486a04eba830d9cc6500d1b28147f3", + "196fca3e87989eb3e89e443b7ca93bb094198538d5bde32b9d6fcca58ffc8eed", + "1afe754dd8c66bc36e38a40c66e9183ecbaf365fd8d87a784069cd615c3ce64a", 2, ), DenseWarmEditCompileGuardTestCase( @@ -153,8 +153,8 @@ def test_given_dense_project_when_compiling_cold_then_preserves_rules_semantics_ 19.0, 21.0, 9 * _GIB // 4, - "bc6b60e90118d66f3217a5f04f104ae91f657a6356f4d19844614d1ba48b0012", - "571fe58493234057c7128b4929d3518532731192f6fef85da0496788826bb172", + "3477a472fa76d6236ebd0c9758d79c18f67f1bf55e9bbec05b002ba3744cfd22", + "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 4773daca5b..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 @@ -57,16 +57,16 @@ expected_max_rss_bytes=2 * _GIB, expected_max_cache_bytes=96 * _MIB, expected_cold_fingerprint=( - "7a1978e9f8871b2252abea5e8763c570305f97342b3b3f51e2a2992238cc3798" + "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, @@ -91,16 +91,16 @@ expected_max_rss_bytes=2 * _GIB, expected_max_cache_bytes=160 * _MIB, expected_cold_fingerprint=( - "d293f1e9dbe2787d21fa6e5f20c79206ad9beea1e3be63299ac0a5a1e3464ffd" + "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, @@ -125,16 +125,16 @@ expected_max_rss_bytes=2 * _GIB, expected_max_cache_bytes=320 * _MIB, expected_cold_fingerprint=( - "8994dd1e87cb97312d492ea66731e4f01f4da08dc2680dd6814f1e4bba23a775" + "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, 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" ), ), ], 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", ), ), ), 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..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) @@ -240,3 +242,61 @@ 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, ...] + + +@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 + + +@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 e980efa666..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.""" @@ -1554,3 +1560,123 @@ 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" + ), + } + + +_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 8f0df214a6..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 @@ -20,24 +20,26 @@ 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)", + "\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__helper_rows " + "AS helper_rows)", ), ), ), @@ -71,9 +73,12 @@ 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=("__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, @@ -100,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_json_output_duration.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_json_output_duration.py new file mode 100644 index 0000000000..91a6979a4a --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_json_output_duration.py @@ -0,0 +1,104 @@ +"""`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, + 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 + + +@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 + + +@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"]) 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..b00457367e --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_model_inlining.py @@ -0,0 +1,102 @@ +"""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 ( + 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 + +_SINGLE_OCCURRENCE_FRAGMENTS: tuple[str, ...] = ( + "__ref__order_lines AS (", + "__ref__order_totals AS (", + "amount * 2 AS line_total", + "line_total + 1 AS order_total", +) +_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)", + "__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 + 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") + ).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 + 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__": + pytest.main([__file__, "-vv"]) 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/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/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/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) 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..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"}, ) ], @@ -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/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 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,