From 207d4c30b83a6b0dcb950ac9200c3e0f693ba7a3 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 1 Oct 2026 09:50:45 +0100 Subject: [PATCH 1/7] feat(test): run model SQL in SQL tests exactly as authored Split a model's leading WITH into authored CTE and body slices and lift them verbatim instead of parsing, renaming every CTE and regenerating SQL. A CTE name already lifted with different text keeps the existing nested fallback; on T-SQL, which rejects nested WITH, the test is refused with a diagnostic naming the colliding CTEs. --- .../_helpers/sql_tests/cte_namespace.rs | 106 --- .../compiler/_helpers/sql_tests/cte_slices.rs | 244 +++++++ .../compiler/_helpers/sql_tests/cte_sql.rs | 3 +- .../src/compiler/_helpers/sql_tests/mod.rs | 2 +- .../compiler/_helpers/sql_tests/planning.rs | 31 +- .../compiler/_helpers/sql_tests/rendering.rs | 625 ++++++++---------- .../src/compiler/tests.rs | 2 + .../src/compiler/tests/helpers.rs | 197 +++--- .../tests/test_sql_test_cte_slices.rs | 128 ++++ .../compiler/tests/test_sql_test_rendering.rs | 31 +- .../src/compiler/tests/test_types.rs | 11 + .../sqlbuild/compiler/pipeline/test_main.py | 5 +- .../testing/main/test_comparison_sql.py | 26 +- 13 files changed, 839 insertions(+), 572 deletions(-) delete mode 100644 crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_namespace.rs create mode 100644 crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_slices.rs create mode 100644 crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_cte_slices.rs diff --git a/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_namespace.rs b/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_namespace.rs deleted file mode 100644 index 73d729193d..0000000000 --- a/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_namespace.rs +++ /dev/null @@ -1,106 +0,0 @@ -//! Lexically scoped AST renaming before model CTEs are lifted into a shared WITH. - -use std::cell::RefCell; -use std::collections::{HashMap, HashSet}; - -use polyglot_sql::expressions::{Identifier, Null}; -use polyglot_sql::traversal::transform_map; -use polyglot_sql::{DialectType, Expression}; - -const CTE_NAME_PREFIX: &str = "__sqb_cte_"; - -/// Allocate bounded, collision-free ordinal names within one rendered query. -#[derive(Default)] -pub(crate) struct CteNamespace { - occupied: HashSet, - next: usize, -} - -impl CteNamespace { - /// Reserve generated-prefix names from all steps; no other token can collide. - pub(crate) fn reserve(&mut self, sql: &str) { - self.occupied.extend( - sql.split(|c: char| !c.is_alphanumeric() && c != '_') - .filter(|token| { - token - .as_bytes() - .get(..CTE_NAME_PREFIX.len()) - .is_some_and(|head| head.eq_ignore_ascii_case(CTE_NAME_PREFIX.as_bytes())) - }) - .map(str::to_ascii_lowercase), - ); - } - - fn allocate(&mut self) -> String { - loop { - let name = format!("{CTE_NAME_PREFIX}{:x}", self.next); - self.next += 1; - if self.occupied.insert(name.clone()) { - return name; - } - } - } - - pub(crate) fn rewrite( - &mut self, - expression: Expression, - dialect: DialectType, - ) -> polyglot_sql::Result { - let allocator = RefCell::new(self); - transform_map(expression, &|mut expression| { - let Expression::Select(select) = &mut expression else { - return Ok(expression); - }; - let Some(mut with) = select.with.take() else { - return Ok(expression); - }; - let mut names: HashMap = HashMap::new(); - for cte in &mut with.ctes { - let original = identifier_key(&cte.alias, dialect); - let replacement = allocator.borrow_mut().allocate(); - if with.recursive { - names.insert(original.clone(), replacement.clone()); - } - let body = std::mem::replace(&mut cte.this, Expression::Null(Null)); - cte.this = rewrite_relations(body, &names, dialect)?; - names.insert(original, replacement.clone()); - cte.alias = Identifier::new(replacement); - } - expression = rewrite_relations(expression, &names, dialect)?; - if let Expression::Select(select) = &mut expression { - select.with = Some(with); - } - Ok(expression) - }) - } -} - -fn rewrite_relations( - expression: Expression, - names: &HashMap, - dialect: DialectType, -) -> polyglot_sql::Result { - transform_map(expression, &|mut expression| { - if let Expression::Table(table) = &mut expression - && table.schema.is_none() - && table.catalog.is_none() - && let Some(name) = names.get(&identifier_key(&table.name, dialect)) - { - if table.alias.is_none() { - table.alias = Some(table.name.clone()); - } - table.name = Identifier::new(name); - } - Ok(expression) - }) -} - -fn identifier_key(identifier: &Identifier, dialect: DialectType) -> String { - match dialect { - DialectType::Snowflake if !identifier.quoted => identifier.name.to_uppercase(), - DialectType::Snowflake | DialectType::PostgreSQL if identifier.quoted => { - identifier.name.clone() - } - _ => identifier.name.to_lowercase(), - } -} diff --git a/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_slices.rs b/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_slices.rs new file mode 100644 index 0000000000..95dc4d98c2 --- /dev/null +++ b/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_slices.rs @@ -0,0 +1,244 @@ +//! Split a leading top-level WITH into authored CTE and body slices without reparsing. + +use std::collections::HashSet; + +use crate::compiler::_helpers::sql_tests::cte_sql::keyword_end; +use crate::rules::main::quote_policy::quote_policy; +use crate::sql_scan::main::comment_end::comment_end; +use crate::sql_scan::main::non_code_end::non_code_end; +use crate::sql_scan::main::skip_whitespace::skip_whitespace; +use crate::sql_scan::models::{QuotePolicy, Unclosed}; + +/// Dialect lexical rules for slicing authored SQL. +#[derive(Clone, Copy, Debug)] +pub(crate) struct SliceDialect { + policy: QuotePolicy, + bracket_identifiers: bool, +} + +impl SliceDialect { + /// Lexical rules for a SQL-analysis dialect name; dollar-quoted text is always opaque. + pub(crate) fn new(dialect_name: Option<&str>) -> Self { + let name = dialect_name.unwrap_or("generic"); + Self { + policy: QuotePolicy { + dollar_quotes: true, + ..quote_policy(name) + }, + bracket_identifiers: is_tsql(name), + } + } + + /// Whether the dialect rejects a WITH clause nested inside a CTE body. + pub(crate) fn rejects_nested_with(self) -> bool { + self.bracket_identifiers + } +} + +/// One authored CTE: its header (name plus optional column list) and parenthesised body. +#[derive(Debug, PartialEq, Eq)] +pub(crate) struct CteSlice<'a> { + pub(crate) header: &'a str, + pub(crate) key: String, + pub(crate) body: &'a str, +} + +/// The CTEs of a leading top-level WITH and the statement body that follows them. +#[derive(Debug, PartialEq, Eq)] +pub(crate) struct WithSlices<'a> { + pub(crate) ctes: Vec>, + pub(crate) body: &'a str, +} + +/// Authored slices of a leading WITH; `None` when absent or not liftable verbatim. +pub(crate) fn split_top_level_with( + sql: &str, + dialect: SliceDialect, +) -> Result>, Unclosed> { + let mut index = skip_ignorable(sql, 0)?; + let Some(with_end) = keyword_end(sql, index, "WITH") else { + return Ok(None); + }; + index = skip_ignorable(sql, with_end)?; + if keyword_end(sql, index, "RECURSIVE").is_some() { + return Ok(None); + } + let mut ctes: Vec> = Vec::new(); + loop { + let Some((key, name_end)) = read_identifier(sql, index, dialect)? else { + return Ok(None); + }; + let mut header_end = name_end; + let mut cursor = skip_ignorable(sql, name_end)?; + if sql.as_bytes().get(cursor) == Some(&b'(') { + header_end = matching_paren(sql, cursor, dialect)? + 1; + cursor = skip_ignorable(sql, header_end)?; + } + let Some(as_end) = keyword_end(sql, cursor, "AS") else { + return Ok(None); + }; + cursor = skip_ignorable(sql, as_end)?; + if sql.as_bytes().get(cursor) != Some(&b'(') { + return Ok(None); + } + let close = matching_paren(sql, cursor, dialect)?; + ctes.push(CteSlice { + header: &sql[index..header_end], + key, + body: &sql[cursor + 1..close], + }); + let next = skip_ignorable(sql, close + 1)?; + if sql.as_bytes().get(next) == Some(&b',') { + index = skip_ignorable(sql, next + 1)?; + continue; + } + let body = sql[skip_whitespace(sql, close + 1)..].trim_end(); + if body.is_empty() || matches!(sql.as_bytes().get(next), Some(b')' | b';') | None) { + return Ok(None); + } + return Ok(Some(WithSlices { ctes, body })); + } +} + +/// Mark the CTEs the statement body reads, directly or through other CTEs. +pub(crate) fn used_ctes(split: &WithSlices<'_>, dialect: SliceDialect) -> Vec { + let mut referenced = identifier_keys(split.body, dialect); + let mut used = vec![false; split.ctes.len()]; + for (index, cte) in split.ctes.iter().enumerate().rev() { + if referenced.contains(&cte.key) { + used[index] = true; + referenced.extend(identifier_keys(cte.body, dialect)); + } + } + used +} + +/// Case-folded identifiers of code outside comments and string literals, quoted ones unquoted. +fn identifier_keys(sql: &str, dialect: SliceDialect) -> HashSet { + let bytes = sql.as_bytes(); + let mut keys: HashSet = HashSet::new(); + let mut index = 0; + while index < bytes.len() { + match read_identifier(sql, index, dialect) { + Ok(Some((key, end))) => { + keys.insert(key); + index = end; + continue; + } + Ok(None) => {} + Err(_) => return keys, + } + match opaque_end(bytes, index, dialect) { + Ok(Some(end)) => index = end, + Ok(None) => index += sql[index..].chars().next().map_or(1, char::len_utf8), + Err(_) => return keys, + } + } + keys +} + +fn is_tsql(name: &str) -> bool { + matches!( + name.to_ascii_lowercase().as_str(), + "tsql" | "mssql" | "sqlserver" | "fabric" + ) +} + +fn read_identifier( + sql: &str, + start: usize, + dialect: SliceDialect, +) -> Result, Unclosed> { + let bytes = sql.as_bytes(); + let Some(&opening) = bytes.get(start) else { + return Ok(None); + }; + if opening == b'[' && dialect.bracket_identifiers { + let end = bracket_end(bytes, start)?; + let name = sql[start + 1..end - 1].replace("]]", "]"); + return Ok(Some((name.to_lowercase(), end))); + } + if dialect.policy.is_quote(opening) && opening != b'\'' { + let Some(end) = non_code_end(bytes, start, dialect.policy)? else { + return Ok(None); + }; + let quote = char::from(opening).to_string(); + let name = sql[start + 1..end - 1].replace("e.repeat(2), "e); + return Ok(Some((name.to_lowercase(), end))); + } + let Some(first) = sql[start..].chars().next() else { + return Ok(None); + }; + if !(first == '_' || first.is_alphabetic()) { + return Ok(None); + } + let end = sql[start..] + .char_indices() + .find(|(_, character)| !is_identifier_continue(*character)) + .map_or(sql.len(), |(offset, _)| start + offset); + Ok(Some((sql[start..end].to_lowercase(), end))) +} + +fn is_identifier_continue(character: char) -> bool { + character == '_' || character == '$' || character.is_alphanumeric() +} + +fn skip_ignorable(sql: &str, mut index: usize) -> Result { + loop { + index = skip_whitespace(sql, index); + match comment_end(sql.as_bytes(), index)? { + Some(end) => index = end, + None => return Ok(index), + } + } +} + +fn matching_paren(sql: &str, open: usize, dialect: SliceDialect) -> Result { + let bytes = sql.as_bytes(); + let mut depth = 0usize; + let mut index = open; + while index < bytes.len() { + if let Some(end) = opaque_end(bytes, index, dialect)? { + index = end; + continue; + } + match bytes[index] { + b'(' => depth += 1, + b')' => { + depth = depth.saturating_sub(1); + if depth == 0 { + return Ok(index); + } + } + _ => {} + } + index += 1; + } + Err(Unclosed::Parenthesis) +} + +fn opaque_end( + bytes: &[u8], + index: usize, + dialect: SliceDialect, +) -> Result, Unclosed> { + if dialect.bracket_identifiers && bytes[index] == b'[' { + return bracket_end(bytes, index).map(Some); + } + non_code_end(bytes, index, dialect.policy) +} + +fn bracket_end(bytes: &[u8], start: usize) -> Result { + let mut index = start + 1; + while index < bytes.len() { + if bytes[index] == b']' { + if bytes.get(index + 1) == Some(&b']') { + index += 2; + continue; + } + return Ok(index + 1); + } + index += 1; + } + Err(Unclosed::Quote) +} diff --git a/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_sql.rs b/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_sql.rs index 03f0f43005..eb335c5777 100644 --- a/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_sql.rs +++ b/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_sql.rs @@ -61,7 +61,8 @@ fn skip_leading_ignorable(sql: &str, mut index: usize) -> usize { } } -fn keyword_end(sql: &str, start: usize, keyword: &str) -> Option { +/// Return the offset after `keyword` at `start` when it is a whole word. +pub(crate) fn keyword_end(sql: &str, start: usize, keyword: &str) -> Option { let end = start + keyword.len(); if !sql.get(start..end)?.eq_ignore_ascii_case(keyword) { return None; diff --git a/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/mod.rs b/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/mod.rs index 360665ac7e..7395c6c00d 100644 --- a/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/mod.rs +++ b/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/mod.rs @@ -1,4 +1,4 @@ -pub(crate) mod cte_namespace; +pub(crate) mod cte_slices; pub(crate) mod cte_sql; pub(crate) mod expected_columns; pub(crate) mod extraction; diff --git a/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/planning.rs b/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/planning.rs index 0d0cb061af..d0203ec5aa 100644 --- a/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/planning.rs +++ b/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/planning.rs @@ -10,6 +10,7 @@ use regex::Regex; use serde::{Deserialize, Serialize}; use serde_json::Value; +use crate::compiler::_helpers::sql_tests::cte_slices::SliceDialect; use crate::compiler::_helpers::sql_tests::cte_sql::{ cte_definition_sql, leading_with_prefix_end, with_leading_ctes, with_unique_ctes, }; @@ -22,7 +23,7 @@ use crate::compiler::_helpers::sql_tests::markers::{ replace_dbt_ref_markers, replace_named_markers, }; use crate::compiler::_helpers::sql_tests::rendering::{ - AssertionStep, ChainStep, RenderRequest, StatementCache, render_comparison_sql, render_dialect, + AssertionStep, ChainStep, RenderRequest, render_comparison_sql, render_dialect, rendered_chain_steps, }; use crate::constants::{TABLE_FUNCTION_TEST_MODE, UDF_TEST_MODE}; @@ -206,7 +207,7 @@ struct ProjectContext { analysis_templates: Arc, patterns: SqlTestPatterns, render_dialect: Arc, - rendered_statements: Arc, + rejects_nested_with: bool, } #[derive(Clone)] @@ -587,6 +588,8 @@ pub(crate) fn plan_and_render_json(request_json: &str) -> Result let render_sql = request.render_sql; let include_plan = request.include_plan; let render_dialect = Arc::new(render_dialect(request.sql_analysis_dialect.as_deref())); + let rejects_nested_with = + SliceDialect::new(request.sql_analysis_dialect.as_deref()).rejects_nested_with(); let context = ProjectContext { models: request .models @@ -615,7 +618,7 @@ pub(crate) fn plan_and_render_json(request_json: &str) -> Result analysis_templates: Arc::new(Mutex::new(HashMap::new())), patterns: SqlTestPatterns::new()?, render_dialect, - rendered_statements: Arc::default(), + rejects_nested_with, }; let workers = request.workers.clamp(1, MAX_WORKERS); let pool = rayon::ThreadPoolBuilder::new() @@ -630,16 +633,24 @@ pub(crate) fn plan_and_render_json(request_json: &str) -> Result .into_par_iter() .map(|test| { let planning_start = Instant::now(); + let test_name = test.name.clone(); let mut planned = plan_test(test, &context)?; let planning_ns = planning_start.elapsed().as_nanos(); let rendering_start = Instant::now(); - let sql = render_sql.then(|| { - render_comparison_sql( - &planned.request, - &context.render_dialect, - Some(&context.rendered_statements), - ) - }); + let rendered = (render_sql || context.rejects_nested_with) + .then(|| render_comparison_sql(&planned.request)); + let sql = match rendered { + Some(Ok(sql)) => render_sql.then_some(sql), + Some(Err(message)) => { + planned.warnings.push(PlanWarning { + model_name: None, + severity: "error", + message: format!("test '{test_name}' cannot be rendered: {message}"), + }); + render_sql.then(String::new) + } + None => None, + }; if !include_plan { planned.request.chain.clear(); planned.request.assertions.clear(); diff --git a/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/rendering.rs b/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/rendering.rs index 2c021c79e9..f884e2bcd2 100644 --- a/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/rendering.rs +++ b/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/rendering.rs @@ -1,12 +1,12 @@ //! Deterministic comparison SQL rendering for planned SQL-native tests. use std::collections::{HashMap, HashSet}; -use std::hash::{BuildHasher, RandomState}; -use std::sync::{Arc, Mutex, OnceLock, PoisonError}; -use polyglot_sql::{Dialect, DialectType, Expression}; +use polyglot_sql::{Dialect, DialectType}; -use crate::compiler::_helpers::sql_tests::cte_namespace::CteNamespace; +use crate::compiler::_helpers::sql_tests::cte_slices::{ + CteSlice, SliceDialect, split_top_level_with, used_ctes, +}; use crate::compiler::_helpers::sql_tests::cte_sql::{ cte_definition_sql, leading_with_prefix_end, with_leading_ctes, }; @@ -16,7 +16,6 @@ use serde::{Deserialize, Serialize}; const DEFAULT_WORKERS: usize = 4; const MAX_WORKERS: usize = 4; const WORKER_STACK_BYTES: usize = 16 * 1024 * 1024; -const STATEMENT_CACHE_SQL_BYTES: usize = 1024 * 1024; #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] @@ -100,200 +99,296 @@ struct RenderResponse { sql: String, } -/// Bounded parses of repeated SQL reused across one single-dialect render batch. -#[derive(Default)] -pub(crate) struct StatementCache { - entries: Mutex, +/// One CTE placed in the shared top-level WITH, keeping its authored header and body text. +struct LiftedCte { + key: String, + header: String, + body: String, + authored: String, + origin: String, } -#[derive(Default)] -struct StatementEntries { - seen: HashSet, - hasher: RandomState, - parsed: HashMap>>>, - parsed_sql_bytes: usize, +impl LiftedCte { + fn conflicts(&self, key: &str, header: &str, authored: &str) -> bool { + self.key == key && (!same_header(&self.header, header) || self.authored != authored) + } } -impl StatementCache { - /// Admit SQL on its second use, within a byte budget, so unrepeated SQL is never retained. - fn entry(&self, sql: &str) -> Option>>> { - let mut entries = self.entries.lock().unwrap_or_else(PoisonError::into_inner); - if let Some(entry) = entries.parsed.get(sql) { - return Some(Arc::clone(entry)); - } - let fingerprint = entries.hasher.hash_one(sql); - if entries.seen.insert(fingerprint) - || entries.parsed_sql_bytes + sql.len() > STATEMENT_CACHE_SQL_BYTES - { - return None; - } - entries.parsed_sql_bytes += sql.len(); - Some(Arc::clone( - entries.parsed.entry(sql.to_string()).or_default(), - )) - } +/// Headers name the same CTE when equal, or equal up to case when nothing in them is quoted. +fn same_header(left: &str, right: &str) -> bool { + left == right || (left.eq_ignore_ascii_case(right) && !left.contains(['"', '`', '[', '\''])) } -struct RenderCteState<'a> { - dialect: &'a Dialect, - statements: Option<&'a StatementCache>, - lifted: Vec<(String, String)>, +struct RenderCteState { + dialect: SliceDialect, + lifted: Vec, name_counts: CteSuffixCounter, - namespace: CteNamespace, } -impl<'a> RenderCteState<'a> { - fn new( - dialect: &'a Dialect, - statements: Option<&'a StatementCache>, - chain: &[ChainStep], - assertions: &[AssertionStep], - ) -> Self { - let mut namespace = CteNamespace::default(); - for step in chain { - namespace.reserve(&step.resolved_sql); - namespace.reserve(step.comparison_body_sql.as_deref().unwrap_or_default()); - namespace.reserve(step.expected_cte_sql.as_deref().unwrap_or_default()); - for (name, sql) in step.lifted_ctes.iter().chain(&step.expected_lifted_ctes) { - namespace.reserve(name); - namespace.reserve(sql); - } - } - for assertion in assertions { - namespace.reserve(&assertion.resolved_sql); - namespace.reserve(assertion.comparison_body_sql.as_deref().unwrap_or_default()); - for (name, sql) in &assertion.lifted_ctes { - namespace.reserve(name); - namespace.reserve(sql); - } - } +impl RenderCteState { + fn new(dialect: SliceDialect) -> Self { Self { dialect, - statements, lifted: Vec::new(), name_counts: CteSuffixCounter::default(), - namespace, } } - fn lift(&mut self, sql: &str, enabled: bool, namespace: bool) -> String { + /// 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 { if !enabled || leading_with_prefix_end(sql).is_none() { - return sql.to_string(); + return Ok(sql.to_string()); } - let Some((step_ctes, body_sql)) = split_top_level_with( - sql, - self.dialect, - self.statements, - namespace.then_some(&mut self.namespace), - ) else { - return sql.to_string(); + let split = match split_top_level_with(sql, self.dialect) { + Ok(Some(split)) => split, + Ok(None) | Err(_) => return self.nested(sql, enabled, origin, &[]), }; - if step_ctes.iter().any(|(name, body)| { - existing_cte(&self.lifted, name).is_some_and(|(_, existing)| existing != body) - }) { - return sql.to_string(); + let collisions = self.collisions(split.ctes.iter()); + if !collisions.is_empty() { + let message = collision_message(origin, &collisions); + return self.nested(sql, enabled, origin, &[message]); + } + if has_duplicate_keys(split.ctes.iter().map(|cte| cte.key.as_str())) { + return self.nested(sql, enabled, origin, &[]); } - if !self.merge(&step_ctes) { - return sql.to_string(); + for cte in &split.ctes { + self.push(cte.key.clone(), cte.header, cte.body, origin); } - body_sql + Ok(split.body.to_string()) + } + + /// Return SQL that cannot be lifted, refusing it where the dialect rejects a nested WITH. + fn nested( + &self, + sql: &str, + enabled: bool, + origin: &str, + collisions: &[String], + ) -> Result { + if !(enabled + && self.dialect.rejects_nested_with() + && leading_with_prefix_end(sql).is_some()) + { + return Ok(sql.to_string()); + } + if collisions.is_empty() { + return Err(format!( + "the WITH clause of {origin} cannot be lifted into the test query, and T-SQL \ + does not allow a nested WITH; use plain `name AS (...)` CTEs without RECURSIVE \ + or MATERIALIZED" + )); + } + Err(format!( + "{}; T-SQL does not allow a nested WITH, so rename the CTE in the model or the \ + fixture so the names are unique", + collisions.join("; ") + )) } /// 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) -> String { + fn actual_step_sql(&mut self, step: &ChainStep, enabled: bool) -> Result { + let origin = format!("model '{}'", step.model_name); if step.lifted_ctes.is_empty() { - return self.lift(&step.resolved_sql, enabled, true); + return self.lift(&step.resolved_sql, enabled, &origin); } - if !self.merge(&step.lifted_ctes) { - return step.resolved_sql.clone(); + if let Some(collisions) = self.merge(&step.lifted_ctes, enabled, &origin)? { + return self.nested(&step.resolved_sql, enabled, &origin, &[collisions]); } self.lift( step.comparison_body_sql .as_deref() .unwrap_or(&step.resolved_sql), enabled, - true, + &origin, ) } /// Place an expected step's helper CTEs at top level and return its comparison body. - fn expected_step_sql(&mut self, step: &ChainStep, expected_sql: &str, enabled: bool) -> String { + fn expected_step_sql( + &mut self, + step: &ChainStep, + expected_sql: &str, + 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, false); + return self.lift(expected_sql, enabled, &origin); } - if !self.merge(&step.expected_lifted_ctes) { - return with_leading_ctes(&step.expected_lifted_ctes, expected_sql); + 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, false) + self.lift(expected_sql, enabled, &origin) } - fn merge(&mut self, ctes: &[(String, String)]) -> bool { - if ctes.iter().any(|(name, body)| { - existing_cte(&self.lifted, name).is_some_and(|(_, existing)| existing != body) - }) { - return false; - } - for (name, sql) in ctes { - if existing_cte(&self.lifted, name).is_none() { - self.lifted.push((name.clone(), sql.clone())); - } + /// Place an assertion's helper CTEs at top level and return its comparison body. + fn assertion_sql( + &mut self, + assertion: &AssertionStep, + 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]); } - true + self.lift( + assertion + .comparison_body_sql + .as_deref() + .unwrap_or(&assertion.resolved_sql), + enabled, + &origin, + ) } - fn unique_suffix(&mut self, model_name: &str) -> String { - self.name_counts.next(model_name) + /// Add generated CTEs verbatim, describing any name already lifted with different text. + fn merge( + &mut self, + ctes: &[(String, String)], + enabled: bool, + step_origin: &str, + ) -> Result, String> { + let keys: Vec = ctes.iter().map(|(name, _)| cte_key(name)).collect(); + let mut collisions: Vec<(&str, &LiftedCte)> = Vec::new(); + for ((name, sql), key) in ctes.iter().zip(&keys) { + if let Some(existing) = self.conflict(key, name, sql) { + collisions.push((name.as_str(), existing)); + } + } + if !collisions.is_empty() { + return Ok(Some(collision_message(step_origin, &collisions))); + } + for ((name, sql), key) in ctes.iter().zip(keys) { + if self.lifted.iter().any(|existing| existing.key == key) { + continue; + } + let origin = format!("CTE '{name}'"); + let body = if enabled && self.dialect.rejects_nested_with() { + self.flatten(sql, &origin)? + } else { + sql.clone() + }; + if let Some(existing) = self.conflict(&key, name, sql) { + let message = collision_message(&origin, &[(name.as_str(), existing)]); + self.nested(sql, enabled, &origin, std::slice::from_ref(&message))?; + return Ok(Some(message)); + } + self.lifted.push(LiftedCte { + key, + header: name.clone(), + body, + authored: sql.clone(), + origin, + }); + } + Ok(None) } - /// Nested model bodies must also be isolated from the shared outer scope. - fn definition(&mut self, name: &str, sql: &str, enabled: bool) -> String { - let scoped = if enabled && leading_with_prefix_end(sql).is_some() { - let (protected, identifiers) = protect_backtick_identifiers(sql); - parse_statement(&protected, self.dialect, self.statements).and_then(|expression| { - let expression = match self - .namespace - .rewrite(expression, self.dialect.dialect_type()) - { - Ok(expression) => expression, - Err(_) => return None, - }; - let sql = match self.dialect.generate(&expression) { - Ok(sql) => sql, - Err(_) => return None, - }; - Some(restore_backtick_identifiers(&sql, &identifiers)) - }) - } else { - None + /// Lift the CTEs a generated body reads ahead of it; unread CTEs are dead and left out. + fn flatten(&mut self, sql: &str, origin: &str) -> Result { + if leading_with_prefix_end(sql).is_none() { + return Ok(sql.to_string()); + } + let split = match split_top_level_with(sql, self.dialect) { + Ok(Some(split)) => split, + Ok(None) | Err(_) => return self.nested(sql, true, origin, &[]), }; - cte_definition_sql(name, scoped.as_deref().unwrap_or(sql)) + let used_flags = used_ctes(&split, self.dialect); + let used: Vec<&CteSlice<'_>> = split + .ctes + .iter() + .zip(used_flags) + .filter_map(|(cte, used)| used.then_some(cte)) + .collect(); + let collisions = self.collisions(used.iter().copied()); + if !collisions.is_empty() { + let message = collision_message(origin, &collisions); + return self.nested(sql, true, origin, &[message]); + } + for cte in used { + self.push(cte.key.clone(), cte.header, cte.body, origin); + } + Ok(split.body.to_string()) } - fn definitions(&mut self, enabled: bool) -> Vec { - let mut definitions: Vec = Vec::new(); - for (name, sql) in std::mem::take(&mut self.lifted) { - if enabled - && leading_with_prefix_end(&sql).is_some() - && let Some((ctes, body)) = split_top_level_with( - &sql, - self.dialect, - self.statements, - Some(&mut self.namespace), - ) - { - definitions.extend( - ctes.iter() - .map(|(name, sql)| self.definition(name, sql, enabled)), - ); - definitions.push(self.definition(&name, &body, enabled)); - } else { - definitions.push(self.definition(&name, &sql, enabled)); + fn collisions<'s>( + &self, + ctes: impl Iterator>, + ) -> Vec<(&'s str, &LiftedCte)> { + let mut collisions: Vec<(&'s str, &LiftedCte)> = Vec::new(); + for cte in ctes { + if let Some(existing) = self.conflict(&cte.key, cte.header, cte.body) { + collisions.push((cte.header, existing)); } } - definitions + collisions + } + + fn conflict(&self, key: &str, header: &str, body: &str) -> Option<&LiftedCte> { + self.lifted + .iter() + .find(|existing| existing.conflicts(key, header, body)) + } + + fn push(&mut self, key: String, header: &str, body: &str, origin: &str) { + if self.lifted.iter().any(|existing| existing.key == key) { + return; + } + self.lifted.push(LiftedCte { + key, + header: header.to_string(), + body: body.to_string(), + authored: body.to_string(), + origin: origin.to_string(), + }); + } + + fn unique_suffix(&mut self, model_name: &str) -> String { + self.name_counts.next(model_name) + } + + fn definitions(&mut self) -> Vec { + std::mem::take(&mut self.lifted) + .iter() + .map(|cte| cte_definition_sql(&cte.header, &cte.body)) + .collect() } } +fn collision_message(origin: &str, collisions: &[(&str, &LiftedCte)]) -> String { + collisions + .iter() + .map(|(header, existing)| { + format!( + "CTE '{header}' of {origin} collides with CTE '{}' of {}", + existing.header, existing.origin + ) + }) + .collect::>() + .join("; ") +} + +fn has_duplicate_keys<'k>(keys: impl Iterator) -> bool { + let mut seen: HashSet<&str> = HashSet::new(); + keys.into_iter().any(|key| !seen.insert(key)) +} + +/// Case-insensitive CTE identity with identifier quotes removed. +fn cte_key(name: &str) -> String { + let trimmed = name.trim(); + [('"', '"'), ('`', '`'), ('[', ']')] + .iter() + .find_map(|(open, close)| trimmed.strip_prefix(*open)?.strip_suffix(*close)) + .unwrap_or(trimmed) + .to_lowercase() +} + /// Readable, collision-free comparison CTE suffixes in step order. #[derive(Default)] struct CteSuffixCounter { @@ -334,33 +429,23 @@ pub(crate) fn render_json(request_json: &str) -> Result { .stack_size(WORKER_STACK_BYTES) .build() .map_err(|error| error.to_string())?; - let dialects: HashMap, (Dialect, StatementCache)> = request - .requests - .iter() - .map(|item| item.sql_analysis_dialect.clone()) - .collect::>() - .into_iter() - .map(|name| { - let dialect = render_dialect(name.as_deref()); - (name, (dialect, StatementCache::default())) - }) - .collect(); let responses: Vec = pool.install(|| { request .requests .into_par_iter() - .map(|request| { - let (dialect, statements) = &dialects[&request.sql_analysis_dialect]; - RenderResponse { - sql: render_comparison_sql(&request, dialect, Some(statements)), - } - }) - .collect() - }); + .map(render_response) + .collect::>() + })?; serde_json::to_string(&responses).map_err(|error| error.to_string()) } -/// Resolve the SQL-analysis dialect used to split and regenerate top-level CTEs. +fn render_response(request: RenderRequest) -> Result { + Ok(RenderResponse { + sql: render_comparison_sql(&request)?, + }) +} + +/// Resolve the SQL-analysis dialect used to read expected-row columns. pub(crate) fn render_dialect(name: Option<&str>) -> Dialect { name.and_then(Dialect::get_by_name) .unwrap_or_else(|| Dialect::get(DialectType::Generic)) @@ -369,25 +454,25 @@ pub(crate) fn render_dialect(name: Option<&str>) -> Dialect { pub(crate) fn render_difference_sample_json(request_json: &str) -> Result { let request: DifferenceSampleRequest = serde_json::from_str(request_json).map_err(|error| error.to_string())?; - let dialect = render_dialect(request.sql_analysis_dialect.as_deref()); serde_json::to_string(&RenderResponse { - sql: render_difference_sample_sql(&request, &dialect), + sql: render_difference_sample_sql(&request)?, }) .map_err(|error| error.to_string()) } -fn render_difference_sample_sql(request: &DifferenceSampleRequest, dialect: &Dialect) -> String { +fn render_difference_sample_sql(request: &DifferenceSampleRequest) -> Result { let step = &request.step; let Some(expected_input) = step.expected_cte_sql.as_deref() else { - return String::new(); + return Ok(String::new()); }; - let mut cte_state = RenderCteState::new(dialect, None, std::slice::from_ref(step), &[]); - let actual_sql = cte_state.actual_step_sql(step, request.sql_analysis_enabled); - let expected_sql = - cte_state.expected_step_sql(step, expected_input, request.sql_analysis_enabled); - let mut cte_parts = cte_state.definitions(request.sql_analysis_enabled); - cte_parts.push(cte_state.definition("__actual", &actual_sql, request.sql_analysis_enabled)); - cte_parts.push(cte_state.definition("__expected", &expected_sql, request.sql_analysis_enabled)); + let enabled = request.sql_analysis_enabled; + let mut cte_state = + RenderCteState::new(SliceDialect::new(request.sql_analysis_dialect.as_deref())); + let actual_sql = cte_state.actual_step_sql(step, enabled)?; + let expected_sql = cte_state.expected_step_sql(step, expected_input, enabled)?; + let mut cte_parts = cte_state.definitions(); + cte_parts.push(cte_definition_sql("__actual", &actual_sql)); + cte_parts.push(cte_definition_sql("__expected", &expected_sql)); let (left, right) = match request.direction { DifferenceDirection::Unexpected => ("__actual", "__expected"), DifferenceDirection::Missing => ("__expected", "__actual"), @@ -404,11 +489,11 @@ fn render_difference_sample_sql(request: &DifferenceSampleRequest, dialect: &Dia ) }; let projection = compared_projection(step); - format!( + Ok(format!( "WITH {}\n{bounded_select} FROM (SELECT {projection} FROM {left} {} SELECT {projection} FROM {right}) AS __sqlbuild_difference{limit_clause}", cte_parts.join(",\n"), request.set_difference_operator, - ) + )) } /// Columns compared for an expected-output step: the listed expected columns, else every column. @@ -418,16 +503,14 @@ fn compared_projection(step: &ChainStep) -> String { .map_or_else(|| "*".to_string(), |columns| columns.join(", ")) } -pub(crate) fn render_comparison_sql( - request: &RenderRequest, - dialect: &Dialect, - statements: Option<&StatementCache>, -) -> String { +/// Render one planned test's comparison query, lifting authored CTE text verbatim. +pub(crate) fn render_comparison_sql(request: &RenderRequest) -> Result { if request.chain.is_empty() && request.assertions.is_empty() { - return String::new(); + return Ok(String::new()); } + let enabled = request.sql_analysis_enabled; let mut cte_state = - RenderCteState::new(dialect, statements, &request.chain, &request.assertions); + RenderCteState::new(SliceDialect::new(request.sql_analysis_dialect.as_deref())); let mut comparison_ctes: Vec = Vec::new(); let mut select_parts: Vec = Vec::new(); let rendered_steps = rendered_chain_steps(&request.chain, &request.assertions); @@ -440,25 +523,16 @@ pub(crate) fn render_comparison_sql( if !rendered_steps[step_index] { continue; } - let actual_sql = cte_state.actual_step_sql(step, request.sql_analysis_enabled); - comparison_ctes.push(cte_state.definition( - &actual_cte, - &actual_sql, - request.sql_analysis_enabled, - )); + let actual_sql = cte_state.actual_step_sql(step, enabled)?; + comparison_ctes.push(cte_definition_sql(&actual_cte, &actual_sql)); if request.probe_step_index == Some(step_index) { probe_actual_cte = Some(actual_cte.clone()); } let Some(expected_input) = step.expected_cte_sql.as_deref() else { continue; }; - let expected_sql = - cte_state.expected_step_sql(step, expected_input, request.sql_analysis_enabled); - comparison_ctes.push(cte_state.definition( - &expected_cte, - &expected_sql, - request.sql_analysis_enabled, - )); + let expected_sql = cte_state.expected_step_sql(step, expected_input, enabled)?; + comparison_ctes.push(cte_definition_sql(&expected_cte, &expected_sql)); let projection = compared_projection(step); select_parts.push(format!( "SELECT {step_index} AS step_index, '{}' AS model_name, \ @@ -480,24 +554,8 @@ pub(crate) fn render_comparison_sql( { let suffix = cte_state.unique_suffix(&assertion.name); let assertion_cte = format!("__assert__{suffix}"); - let assertion_sql = - if assertion.lifted_ctes.is_empty() || cte_state.merge(&assertion.lifted_ctes) { - cte_state.lift( - assertion - .comparison_body_sql - .as_deref() - .unwrap_or(&assertion.resolved_sql), - request.sql_analysis_enabled, - true, - ) - } else { - assertion.resolved_sql.clone() - }; - comparison_ctes.push(cte_state.definition( - &assertion_cte, - &assertion_sql, - request.sql_analysis_enabled, - )); + let assertion_sql = cte_state.assertion_sql(assertion, enabled)?; + comparison_ctes.push(cte_definition_sql(&assertion_cte, &assertion_sql)); select_parts.push(format!( "SELECT {assertion_index} AS step_index, 'assertion {}' AS model_name, \ (SELECT COUNT(*) FROM {assertion_cte}) AS actual_count, \ @@ -509,146 +567,23 @@ pub(crate) fn render_comparison_sql( } if select_parts.is_empty() { - return String::new(); + return Ok(String::new()); } - let mut cte_parts = cte_state.definitions(request.sql_analysis_enabled); + let mut cte_parts = cte_state.definitions(); cte_parts.extend(comparison_ctes); if request.probe_step_index.is_some() { - return probe_actual_cte.map_or_else(String::new, |actual_cte| { + return Ok(probe_actual_cte.map_or_else(String::new, |actual_cte| { format!( "WITH {}\nSELECT * FROM {actual_cte} WHERE 1 = 0", cte_parts.join(",\n") ) - }); + })); } - format!( + Ok(format!( "WITH {}\n{}", cte_parts.join(",\n"), select_parts.join("\nUNION ALL\n") - ) -} - -fn existing_cte<'a>(lifted: &'a [(String, String)], name: &str) -> Option<&'a (String, String)> { - lifted - .iter() - .find(|(existing, _)| existing.eq_ignore_ascii_case(name)) -} - -/// Parse one statement, reusing a batch's earlier parse of the same SQL when a cache is given. -fn parse_statement( - sql: &str, - dialect: &Dialect, - statements: Option<&StatementCache>, -) -> Option { - let parse = || match dialect.parse(sql) { - Ok(mut parsed) if parsed.len() == 1 => parsed.pop(), - _ => None, - }; - match statements.and_then(|statements| statements.entry(sql)) { - Some(cached) => cached.get_or_init(parse).clone(), - None => parse(), - } -} - -fn split_top_level_with( - sql: &str, - dialect: &Dialect, - statements: Option<&StatementCache>, - namespace: Option<&mut CteNamespace>, -) -> Option<(Vec<(String, String)>, String)> { - let (protected, identifiers) = protect_backtick_identifiers(sql); - let mut expression = parse_statement(&protected, dialect, statements)?; - if let Some(namespace) = namespace { - expression = match namespace.rewrite(expression, dialect.dialect_type()) { - Ok(expression) => expression, - Err(_) => return None, - }; - } - let Expression::Select(select) = &mut expression else { - return None; - }; - let with = select.with.take()?; - if with.recursive - || with.search.is_some() - || with.ctes.iter().any(|cte| { - !cte.columns.is_empty() || cte.materialized.is_some() || !cte.key_expressions.is_empty() - }) - { - return None; - } - let mut ctes = Vec::with_capacity(with.ctes.len()); - for cte in with.ctes { - let sql = match dialect.generate(&cte.this) { - Ok(sql) => sql, - Err(_) => return None, - }; - ctes.push(( - cte.alias.name, - restore_backtick_identifiers(&sql, &identifiers), - )); - } - let body = match dialect.generate(&expression) { - Ok(body) => body, - Err(_) => return None, - }; - Some((ctes, restore_backtick_identifiers(&body, &identifiers))) -} - -fn protect_backtick_identifiers(sql: &str) -> (String, Vec<(String, String)>) { - let mut protected = String::with_capacity(sql.len()); - let mut identifiers: Vec<(String, String)> = Vec::new(); - let mut cursor = 0; - while let Some(relative_start) = sql[cursor..].find('`') { - let start = cursor + relative_start; - protected.push_str(&sql[cursor..start]); - cursor = start + 1; - let start = cursor; - loop { - let Some(relative_end) = sql[cursor..].find('`') else { - protected.push_str(&sql[start - 1..]); - return (protected, identifiers); - }; - cursor += relative_end + 1; - let remainder = &sql[cursor..]; - let whitespace_len = remainder.len() - remainder.trim_start().len(); - let dot_index = cursor + whitespace_len; - if !sql[dot_index..].starts_with('.') { - break; - } - let after_dot = dot_index + 1; - let dot_remainder = &sql[after_dot..]; - let dot_whitespace_len = dot_remainder.len() - dot_remainder.trim_start().len(); - let next_identifier = after_dot + dot_whitespace_len; - if !sql[next_identifier..].starts_with('`') { - break; - } - cursor = next_identifier + 1; - } - let identifier = &sql[start - 1..cursor]; - if !identifier[1..identifier.len() - 1].contains(['`', '.']) { - protected.push_str(identifier); - continue; - } - let placeholder = match identifiers.iter().find(|(_, value)| value == identifier) { - Some((placeholder, _)) => placeholder.clone(), - None => { - let placeholder = format!("SQB_PROTECTED_IDENTIFIER_{}", identifiers.len()); - identifiers.push((placeholder.clone(), identifier.to_string())); - placeholder - } - }; - protected.push_str(&placeholder); - } - protected.push_str(&sql[cursor..]); - (protected, identifiers) -} - -fn restore_backtick_identifiers(sql: &str, identifiers: &[(String, String)]) -> String { - identifiers - .iter() - .fold(sql.to_string(), |value, (placeholder, identifier)| { - value.replace(placeholder, identifier) - }) + )) } fn sanitize_cte_suffix(model_name: &str) -> String { diff --git a/crates/sqlbuild-rules-native/src/compiler/tests.rs b/crates/sqlbuild-rules-native/src/compiler/tests.rs index 6d65a61e11..fb62ae1929 100644 --- a/crates/sqlbuild-rules-native/src/compiler/tests.rs +++ b/crates/sqlbuild-rules-native/src/compiler/tests.rs @@ -6,6 +6,8 @@ mod model_header_tokenization; mod sql_interpolation; #[path = "tests/test_sql_references.rs"] mod sql_references; +#[path = "tests/test_sql_test_cte_slices.rs"] +mod sql_test_cte_slices; #[path = "tests/test_sql_test_expected_columns.rs"] mod sql_test_expected_columns; #[path = "tests/test_sql_test_extraction.rs"] diff --git a/crates/sqlbuild-rules-native/src/compiler/tests/helpers.rs b/crates/sqlbuild-rules-native/src/compiler/tests/helpers.rs index d54709cac0..048b2f18b0 100644 --- a/crates/sqlbuild-rules-native/src/compiler/tests/helpers.rs +++ b/crates/sqlbuild-rules-native/src/compiler/tests/helpers.rs @@ -2,8 +2,6 @@ use std::collections::HashMap; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::{Arc, Barrier, Mutex}; -use crate::compiler::_helpers::sql_tests::cte_namespace::CteNamespace; -use polyglot_sql::{Dialect, DialectType, Expression}; use serde_json::{Value, json}; use crate::compiler::_helpers::model_headers::tokenization::{ @@ -1379,69 +1377,30 @@ pub(crate) fn mock_read_through_helper_brings_its_mock_dependencies_into_scope() } true } -pub(super) fn bounded_names_are_unique() -> bool { - let dialect = Dialect::get(DialectType::DuckDB); - let long = "orders_".repeat(40); - let sql = format!( - "WITH {long} AS (SELECT 1 AS order_id), __sqb_cte_0 AS (SELECT * FROM {long}), \ - final AS (SELECT * FROM __sqb_cte_0) SELECT final.* FROM final" - ); - let reserved = "__sqb_cte_1 __SQB_CTE_2 __ref__orders __source__orders __expected__orders __actual__orders"; - let mut first = CteNamespace::default(); - first.reserve(&sql); - first.reserve(reserved); - let parsed = dialect.parse(&sql).expect("valid CTE query").remove(0); - let rewritten = first - .rewrite(parsed.clone(), dialect.dialect_type()) - .expect("valid rename"); - let Expression::Select(select) = &rewritten else { - return false; - }; - let names: Vec<_> = select - .with - .as_ref() - .expect("WITH retained") - .ctes - .iter() - .map(|cte| cte.alias.name.as_str()) - .collect(); - let second_step = first - .rewrite(parsed.clone(), dialect.dialect_type()) - .expect("second rename"); - let mut replay = CteNamespace::default(); - replay.reserve(&sql); - replay.reserve(reserved); - names.len() == 3 - && names - .iter() - .all(|name| name.len() <= 30 && name.starts_with("__sqb_cte_")) - && !names.contains(&"__sqb_cte_0") - && !names.contains(&"__sqb_cte_1") - && !names.contains(&"__sqb_cte_2") - && rewritten != second_step - && rewritten - == replay - .rewrite(parsed, dialect.dialect_type()) - .expect("deterministic rename") +fn render_one(request: Value) -> Result { + let response = crate::compiler::_helpers::sql_tests::rendering::render_json( + &json!({"requests": [request]}).to_string(), + )?; + let response: Vec = serde_json::from_str(&response).expect("render response"); + Ok(response[0]["sql"] + .as_str() + .expect("rendered query") + .to_string()) } -pub(super) fn quoted_names_keep_bindings() -> bool { - let dialect = Dialect::get(DialectType::Snowflake); - let sql = "WITH final AS (SELECT 1 AS order_id), \"final\" AS (SELECT 2 AS order_id) \ - SELECT a.order_id, b.order_id, 'final' AS label FROM FINAL a CROSS JOIN \"final\" b"; - let mut namespace = CteNamespace::default(); - namespace.reserve(sql); - let rewritten = namespace - .rewrite( - dialect.parse(sql).expect("valid quoted CTEs").remove(0), - dialect.dialect_type(), - ) - .expect("quoted rename"); - let rendered = dialect.generate(&rewritten).expect("render renamed CTEs"); - rendered.contains("FROM __sqb_cte_0 AS a") - && rendered.contains("JOIN __sqb_cte_1 AS b") - && rendered.contains("'final' AS label") +fn colliding_chain_request(dialect: &str, name: &str) -> Value { + json!({ + "sqlAnalysisDialect": dialect, + "chain": [{ + "modelName": "orders", + "resolvedSql": format!("WITH {name} AS (SELECT 2 AS order_id) SELECT final.order_id FROM final"), + "comparisonBodySql": format!("WITH {name} AS (SELECT order_id + 1 AS order_id FROM __ref__stg_orders) SELECT final.order_id FROM final"), + "liftedCtes": [["__ref__stg_orders", format!("WITH {name} AS (SELECT 1 AS order_id) SELECT * FROM {name}")]], + "expectedCteSql": "SELECT 2 AS order_id" + }] + }) } + pub(super) fn repeated_model_sql_renders_like_separate_batches() -> bool { let step = |reserved: &str| { json!({ @@ -1461,37 +1420,105 @@ pub(super) fn repeated_model_sql_renders_like_separate_batches() -> bool { serde_json::from_str::>(&response).expect("render response") }; let plain = step(""); - let reserved = step("-- __sqb_cte_0"); - let separate: Vec = [&plain, &reserved, &plain] + let commented = step("-- trailing comment"); + let separate: Vec = [&plain, &commented, &plain] .into_iter() .flat_map(|request| render(vec![request.clone()])) .collect(); - let batched = render(vec![plain.clone(), reserved.clone(), plain]); + let batched = render(vec![plain.clone(), commented.clone(), plain]); separate == batched && batched[0] != batched[1] - && batched[0]["sql"] - .as_str() - .is_some_and(|sql| sql.contains("__sqb_cte_0 AS")) + && batched[0]["sql"].as_str().is_some_and(|sql| { + sql.starts_with("WITH final AS (SELECT 2 AS order_id),\n") + && sql.contains("__actual__orders AS (SELECT final.order_id FROM final)") + }) } -pub(super) fn model_cte_names_are_isolated_across_dialects() -> bool { - [("duckdb", "final"), ("snowflake", "final"), ("postgres", "final"), ("tsql", "final"), ("bigquery", "`final`"), ("databricks", "`final`")].iter().all(|(dialect, name)| { - let request = json!({ - "requests": [{ - "sqlAnalysisDialect": dialect, - "chain": [{ +pub(super) fn colliding_model_ctes_nest_on_nested_with_dialects() -> bool { + [ + ("duckdb", "final"), + ("snowflake", "final"), + ("postgres", "final"), + ("bigquery", "`final`"), + ("databricks", "`final`"), + ] + .iter() + .all(|(dialect, name)| { + let upstream = format!("WITH {name} AS (SELECT 1 AS order_id) SELECT * FROM {name}"); + let downstream = format!( + "WITH {name} AS (SELECT order_id + 1 AS order_id FROM __ref__stg_orders) SELECT final.order_id FROM final" + ); + let sql = render_one(json!({ + "sqlAnalysisDialect": dialect, + "chain": [ + {"modelName": "stg_orders", "resolvedSql": upstream, "expectedCteSql": "SELECT 1 AS order_id"}, + { "modelName": "orders", - "resolvedSql": format!("WITH {name} AS (SELECT 2 AS order_id) SELECT final.order_id FROM final"), - "comparisonBodySql": format!("WITH {name} AS (SELECT order_id + 1 AS order_id FROM __ref__stg_orders) SELECT final.order_id FROM final"), - "liftedCtes": [["__ref__stg_orders", format!("WITH {name} AS (SELECT 1 AS order_id) SELECT * FROM {name}")]], + "resolvedSql": format!("WITH __ref__stg_orders AS ({upstream}), {}", &downstream[5..]), + "comparisonBodySql": downstream, + "liftedCtes": [["__ref__stg_orders", upstream]], "expectedCteSql": "SELECT 2 AS order_id" - }] - }] - }); - let response = crate::compiler::_helpers::sql_tests::rendering::render_json(&request.to_string()).expect("supported render dialect"); - let response: Value = serde_json::from_str(&response).expect("render response"); - let sql = response[0]["sql"].as_str().expect("rendered query"); - sql.contains("__sqb_cte_0 AS") && sql.contains("FROM __sqb_cte_0 ") - && sql.contains("__sqb_cte_1 AS") && !sql.contains("AS (WITH") + } + ] + })) + .expect("nested fallback"); + 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})")) }) } + +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", + ) +} + +pub(super) fn tsql_cte_collisions_are_refused_with_named_ctes() -> bool { + let error = render_one(colliding_chain_request("tsql", "final")).expect_err("T-SQL refusal"); + assert_eq!( + error, + "CTE 'final' of model 'orders' collides with CTE 'final' of CTE '__ref__stg_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 +} + +pub(super) fn tsql_distinct_ctes_lift_verbatim() -> bool { + let sql = render_one(json!({ + "sqlAnalysisDialect": "tsql", + "chain": [{ + "modelName": "orders", + "resolvedSql": "WITH [base rows] (order_id) AS (SELECT 1 /* ) */ AS order_id), \ + staged AS (SELECT order_id FROM [base rows]) SELECT TOP 5 order_id FROM staged", + "expectedCteSql": "SELECT 1 AS order_id" + }] + })) + .expect("distinct T-SQL CTEs lift"); + sql.starts_with( + "WITH [base rows] (order_id) AS (SELECT 1 /* ) */ AS order_id),\n\ + staged AS (SELECT order_id FROM [base rows]),\n\ + __actual__orders AS (SELECT TOP 5 order_id FROM staged),\n", + ) +} + +pub(super) fn snowflake_function_synonyms_stay_as_authored() -> bool { + let model = "WITH picked AS (SELECT STARTSWITH(name, 'A') AS matches, SUBSTR(name, 1, 2) AS prefix, \ + TIMESTAMPDIFF(day, created_at, updated_at) AS age, TRY_TO_DECIMAL(amount, 10, 2) AS total \ + FROM items) SELECT * FROM picked"; + let sql = render_one(json!({ + "sqlAnalysisDialect": "snowflake", + "chain": [{"modelName": "items", "resolvedSql": model, "expectedCteSql": "SELECT 1 AS matches"}] + })) + .expect("Snowflake render"); + let (ctes, body) = model.split_once(") SELECT").expect("authored model"); + sql.contains(&format!("{})", ctes.trim_start_matches("WITH "))) + && sql.contains(&format!("__actual__items AS (SELECT{body})")) + && !sql.contains("STARTS_WITH") + && !sql.contains("SUBSTRING") +} diff --git a/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_cte_slices.rs b/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_cte_slices.rs new file mode 100644 index 0000000000..e3d1b96c66 --- /dev/null +++ b/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_cte_slices.rs @@ -0,0 +1,128 @@ +use crate::compiler::_helpers::sql_tests::cte_slices::{SliceDialect, split_top_level_with}; +use crate::compiler::tests::test_types::CteSliceTestCase; +use crate::sql_scan::models::Unclosed; + +#[test] +fn given_leading_with_queries_when_splitting_then_authored_slices_or_none_are_returned() { + let test_cases = [ + CteSliceTestCase { + description: "plain CTEs keep their exact authored bodies", + dialect: "duckdb", + sql: "WITH a AS (SELECT 1 AS id),\n b AS ( SELECT id FROM a )\nSELECT * FROM b", + expected_slices: Ok(Some(( + vec![("a", "SELECT 1 AS id"), ("b", " SELECT id FROM a ")], + "SELECT * FROM b", + ))), + }, + CteSliceTestCase { + description: "quotes, comments and dollar quotes hide parentheses", + dialect: "snowflake", + sql: "-- lead\nWITH \"A\" AS (SELECT ')' AS x /* ) */, $$ ) $$ AS y -- )\n) SELECT * FROM \"A\"", + expected_slices: Ok(Some(( + vec![("\"A\"", "SELECT ')' AS x /* ) */, $$ ) $$ AS y -- )\n")], + "SELECT * FROM \"A\"", + ))), + }, + CteSliceTestCase { + description: "nested WITH stays inside its CTE body", + dialect: "duckdb", + sql: "WITH outer_rows AS (WITH inner_rows AS (SELECT 1 AS id) SELECT id FROM inner_rows) SELECT id FROM outer_rows", + expected_slices: Ok(Some(( + vec![( + "outer_rows", + "WITH inner_rows AS (SELECT 1 AS id) SELECT id FROM inner_rows", + )], + "SELECT id FROM outer_rows", + ))), + }, + CteSliceTestCase { + description: "column lists stay in the header", + dialect: "postgres", + sql: "WITH totals (order_id, amount) AS (SELECT 1, 2) SELECT * FROM totals", + expected_slices: Ok(Some(( + vec![("totals (order_id, amount)", "SELECT 1, 2")], + "SELECT * FROM totals", + ))), + }, + CteSliceTestCase { + description: "comments between CTEs and before the body are kept with the body", + dialect: "duckdb", + sql: "WITH a AS (SELECT 1 AS id) /* c */ , b AS (SELECT 2 AS id)\n-- body\nSELECT * FROM a, b", + expected_slices: Ok(Some(( + vec![("a", "SELECT 1 AS id"), ("b", "SELECT 2 AS id")], + "-- body\nSELECT * FROM a, b", + ))), + }, + CteSliceTestCase { + description: "BigQuery backslash escapes and backtick names", + dialect: "bigquery", + sql: "WITH `rows` AS (SELECT 'it\\'s )' AS note) SELECT * FROM `rows`", + expected_slices: Ok(Some(( + vec![("`rows`", "SELECT 'it\\'s )' AS note")], + "SELECT * FROM `rows`", + ))), + }, + CteSliceTestCase { + description: "T-SQL bracket identifiers hide parentheses", + dialect: "tsql", + sql: "WITH [odd (name]]] AS (SELECT 1 AS [x)]) SELECT * FROM [odd (name]]]", + expected_slices: Ok(Some(( + vec![("[odd (name]]]", "SELECT 1 AS [x)]")], + "SELECT * FROM [odd (name]]]", + ))), + }, + CteSliceTestCase { + description: "recursive WITH is not lifted", + dialect: "duckdb", + sql: "WITH RECURSIVE r AS (SELECT 1) SELECT * FROM r", + expected_slices: Ok(None), + }, + CteSliceTestCase { + description: "materialized CTEs are not lifted", + dialect: "postgres", + sql: "WITH r AS MATERIALIZED (SELECT 1) SELECT * FROM r", + expected_slices: Ok(None), + }, + CteSliceTestCase { + description: "unbalanced parentheses report the unclosed parenthesis", + dialect: "duckdb", + sql: "WITH r AS (SELECT (1) SELECT * FROM r", + expected_slices: Err(Unclosed::Parenthesis), + }, + CteSliceTestCase { + description: "a WITH without a following statement is not lifted", + dialect: "duckdb", + sql: "WITH r AS (SELECT 1)", + expected_slices: Ok(None), + }, + CteSliceTestCase { + description: "queries without a leading WITH are not split", + dialect: "duckdb", + sql: "SELECT 1 AS withdrawn", + expected_slices: Ok(None), + }, + ]; + + for test_case in test_cases { + let actual = + split_top_level_with(test_case.sql, SliceDialect::new(Some(test_case.dialect))).map( + |split| { + split.map(|split| { + ( + split + .ctes + .iter() + .map(|cte| (cte.header, cte.body)) + .collect::>(), + split.body, + ) + }) + }, + ); + assert_eq!( + actual, test_case.expected_slices, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_rendering.rs b/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_rendering.rs index 880425d85d..c385224d46 100644 --- a/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_rendering.rs +++ b/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_rendering.rs @@ -6,8 +6,9 @@ use crate::compiler::tests::helpers::{ sqlserver_difference_sample_projects_bracketed_columns, }; use crate::compiler::tests::helpers::{ - bounded_names_are_unique, model_cte_names_are_isolated_across_dialects, - quoted_names_keep_bindings, repeated_model_sql_renders_like_separate_batches, + 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, + tsql_cte_collisions_are_refused_with_named_ctes, tsql_distinct_ctes_lift_verbatim, }; use crate::compiler::tests::test_types::SqlTestRenderingTestCase; @@ -15,23 +16,33 @@ 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: "model CTEs are isolated across every first-class dialect", - run: model_cte_names_are_isolated_across_dialects, + description: "colliding model CTEs use the nested fallback on nested-WITH dialects", + run: colliding_model_ctes_nest_on_nested_with_dialects, expected_success: true, }, SqlTestRenderingTestCase { - description: "repeated model SQL in one batch renders like separate batches", - run: repeated_model_sql_renders_like_separate_batches, + description: "generated CTE bodies keep their own WITH nested verbatim", + run: generated_with_bodies_stay_nested_verbatim, + expected_success: true, + }, + SqlTestRenderingTestCase { + description: "T-SQL CTE collisions are refused naming the colliding CTEs", + run: tsql_cte_collisions_are_refused_with_named_ctes, expected_success: true, }, SqlTestRenderingTestCase { - description: "long and reserved CTE names are bounded, unique and deterministic", - run: bounded_names_are_unique, + description: "distinct T-SQL CTEs lift verbatim, including brackets and comments", + run: tsql_distinct_ctes_lift_verbatim, expected_success: true, }, SqlTestRenderingTestCase { - description: "quoted CTEs and literals retain their bindings", - run: quoted_names_keep_bindings, + description: "Snowflake function synonyms stay exactly as authored", + run: snowflake_function_synonyms_stay_as_authored, + expected_success: true, + }, + SqlTestRenderingTestCase { + description: "repeated model SQL in one batch renders like separate batches", + run: repeated_model_sql_renders_like_separate_batches, expected_success: true, }, SqlTestRenderingTestCase { diff --git a/crates/sqlbuild-rules-native/src/compiler/tests/test_types.rs b/crates/sqlbuild-rules-native/src/compiler/tests/test_types.rs index 895efdcd05..d3e0342c92 100644 --- a/crates/sqlbuild-rules-native/src/compiler/tests/test_types.rs +++ b/crates/sqlbuild-rules-native/src/compiler/tests/test_types.rs @@ -1,3 +1,5 @@ +use crate::sql_scan::models::Unclosed; + pub(crate) struct ModelHeaderTokenizationTestCase { pub(crate) description: &'static str, pub(crate) run: fn() -> bool, @@ -42,3 +44,12 @@ pub(crate) struct TopLevelScanTestCase { pub(crate) expected_commas: Result, String>, pub(crate) expected_from: Result, String>, } + +pub(crate) type CteSlices = Option<(Vec<(&'static str, &'static str)>, &'static str)>; + +pub(crate) struct CteSliceTestCase { + pub(crate) description: &'static str, + pub(crate) dialect: &'static str, + pub(crate) sql: &'static str, + pub(crate) expected_slices: Result, +} diff --git a/tests/integration/src/sqlbuild/compiler/pipeline/test_main.py b/tests/integration/src/sqlbuild/compiler/pipeline/test_main.py index e99faeedc4..c24bab7445 100644 --- a/tests/integration/src/sqlbuild/compiler/pipeline/test_main.py +++ b/tests/integration/src/sqlbuild/compiler/pipeline/test_main.py @@ -713,8 +713,9 @@ 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", - "__sqb_cte_0 AS (", - "__actual__fact_orders AS (", + "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)", "FROM __ref__stg_orders", "'stg_orders' AS model_name", ), diff --git a/tests/unit/src/sqlbuild/executor/testing/main/test_comparison_sql.py b/tests/unit/src/sqlbuild/executor/testing/main/test_comparison_sql.py index 29c8526925..33923872f2 100644 --- a/tests/unit/src/sqlbuild/executor/testing/main/test_comparison_sql.py +++ b/tests/unit/src/sqlbuild/executor/testing/main/test_comparison_sql.py @@ -90,15 +90,18 @@ def test_given_adapter_when_building_comparison_sql_then_it_uses_expected_set_di "test_case", [ BuildComparisonSqlTestCase( - description="Snowflake comparison lifting preserves STARTSWITH", + description="Snowflake comparison lifting keeps the authored CTE text", adapter_name="snowflake", - expected_fragments=("STARTSWITH(name, 'A')", "__sqb_cte_0 AS (", "AS picked"), + expected_fragments=( + "picked AS (SELECT STARTSWITH(name, 'A') AS matches FROM items)", + "__actual__orders AS (SELECT matches FROM picked)", + ), expected_absent_fragments=("STARTS_WITH",), ) ], ids=lambda case: case.description, ) -def test_given_snowflake_function_when_lifting_comparison_ctes_then_emits_supported_spelling( +def test_given_snowflake_function_when_lifting_comparison_ctes_then_keeps_authored_spelling( test_case: BuildComparisonSqlTestCase, ) -> None: adapter: BaseAdapter = build_comparison_test_adapter(test_case.adapter_name) @@ -205,19 +208,18 @@ def test_given_assertion_step_when_building_comparison_sql_then_it_counts_failin "test_case", [ BuildComparisonSqlTestCase( - description="matching helper CTEs retain independent actual and expected scopes", + description="identical helper CTEs are lifted once for both scopes", adapter_name="duckdb", expected_fragments=( - "__sqb_cte_0 AS", - "INPUT_VALUES AS", - "__actual__orders AS (", - "__expected__orders AS (", + "WITH input_values AS (SELECT 1 AS order_id),\n", + "__actual__orders AS (SELECT order_id FROM input_values)", + "__expected__orders AS (SELECT order_id FROM INPUT_VALUES)", ), ) ], ids=lambda case: case.description, ) -def test_given_matching_helper_ctes_when_building_comparison_sql_then_keeps_scopes_separate( +def test_given_matching_helper_ctes_when_building_comparison_sql_then_lifts_them_once( test_case: BuildComparisonSqlTestCase, ) -> None: adapter: BaseAdapter = build_comparison_test_adapter(test_case.adapter_name) @@ -255,7 +257,7 @@ def test_given_unasserted_transitive_steps_when_building_comparison_then_emits_o for expected_fragment in test_case.expected_fragments: assert expected_fragment in comparison_sql assert "__actual__stg_orders AS" not in comparison_sql - assert comparison_sql.lower().count("__sqb_cte_0 as (") == 1 + assert comparison_sql.count("shared AS (SELECT 1 AS order_id)") == 1 @pytest.mark.parametrize( @@ -350,9 +352,9 @@ def test_given_preanalyzed_step_with_authored_cte_when_building_comparison_then_ ) assert ( - comparison_sql.index("__ref__raw_orders AS") < comparison_sql.index("__sqb_cte_0 AS") + comparison_sql.index("__ref__raw_orders AS") < comparison_sql.index("picked AS") ) is test_case.expected_result - assert comparison_sql.index("__sqb_cte_0 AS") < comparison_sql.index("__actual__orders AS") + assert comparison_sql.index("picked AS") < comparison_sql.index("__actual__orders AS") assert "__actual__orders AS (\nWITH picked" not in comparison_sql From 28752f1bc931c76255e3ef0f4ee54d4248be4ee1 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 1 Oct 2026 09:50:51 +0100 Subject: [PATCH 2/7] refactor(test): remove the Snowflake STARTS_WITH spelling patch Rendered SQL-test queries now keep the authored function spelling, so the regex that rewrote STARTS_WITH back to STARTSWITH is no longer needed. --- .../planner/_helpers/sql_tests/comments.py | 29 ------------------- .../_helpers/sql_tests/native_planning.py | 14 ++------- .../main/execution/sql_test_dialect.py | 11 ------- .../testing/_helpers/difference_samples.py | 5 +--- .../testing/_helpers/expected_column_probe.py | 7 +---- .../testing/main/comparison_sql_batch.py | 10 +------ 6 files changed, 6 insertions(+), 70 deletions(-) delete mode 100644 src/sqlbuild/compiler/planner/main/execution/sql_test_dialect.py diff --git a/src/sqlbuild/compiler/planner/_helpers/sql_tests/comments.py b/src/sqlbuild/compiler/planner/_helpers/sql_tests/comments.py index 21ea2ef363..14418ad9da 100644 --- a/src/sqlbuild/compiler/planner/_helpers/sql_tests/comments.py +++ b/src/sqlbuild/compiler/planner/_helpers/sql_tests/comments.py @@ -3,18 +3,12 @@ from __future__ import annotations import re -from collections.abc import Callable - -from sqlbuild.adapter.contract.types import BuiltinAdapter _SQL_NON_CODE_PATTERN: re.Pattern[str] = re.compile( r"'(?:''|[^'])*'|\"(?:\"\"|[^\"])*\"|`(?:``|[^`])*`|" r"\$\$.*?\$\$|--[^\n]*|/\*.*?\*/", re.DOTALL, ) -_SNOWFLAKE_STARTS_WITH_PATTERN: re.Pattern[str] = re.compile( - r"\bSTARTS_WITH(?=\s*\()", re.IGNORECASE -) def uncommented_pattern_matches(*, pattern: re.Pattern[str], sql: str) -> tuple[re.Match[str], ...]: @@ -52,26 +46,3 @@ def _matches(pattern: re.Pattern[str]) -> tuple[re.Match[str], ...]: return tuple(matches) return tuple(_matches(pattern) for pattern in patterns) - - -def replace_uncommented_pattern( - *, pattern: re.Pattern[str], replacement: Callable[[re.Match[str]], str], sql: str -) -> str: - """Replace pattern matches outside comments while preserving original comments.""" - - result: str = sql - for match in reversed(uncommented_pattern_matches(pattern=pattern, sql=sql)): - result = f"{result[: match.start()]}{replacement(match)}{result[match.end() :]}" - return result - - -def restore_sql_test_dialect_function_names(*, sql: str, dialect: str | None) -> str: - """Restore warehouse-supported spellings changed by SQL analysis formatting.""" - - if dialect != BuiltinAdapter.SNOWFLAKE: - return sql - return replace_uncommented_pattern( - pattern=_SNOWFLAKE_STARTS_WITH_PATTERN, - replacement=lambda _match: "STARTSWITH", - sql=sql, - ) diff --git a/src/sqlbuild/compiler/planner/_helpers/sql_tests/native_planning.py b/src/sqlbuild/compiler/planner/_helpers/sql_tests/native_planning.py index d4a74c3bc4..d47bbfb665 100644 --- a/src/sqlbuild/compiler/planner/_helpers/sql_tests/native_planning.py +++ b/src/sqlbuild/compiler/planner/_helpers/sql_tests/native_planning.py @@ -25,9 +25,6 @@ render_test_cursor_intrinsics, ) from sqlbuild.compiler.planner.exceptions import NativeSqlTestPlanningError, PlannerInputError -from sqlbuild.compiler.planner.main.execution.sql_test_dialect import ( - restore_sql_test_dialect_function_names, -) from sqlbuild.compiler.planner.models import ( ChainStep, NativeSqlTestArtifact, @@ -137,9 +134,8 @@ def plan_sql_tests_natively( raise NativeSqlTestPlanningError( "native SQL-test planning returned an invalid batch response" ) - dialect: str | None = adapter.sql_analysis_dialect() plans: tuple[NativeSqlTestPlan, ...] = tuple( - _plan_from_payload(value=value, dialect=dialect) for value in response + _plan_from_payload(value=value) for value in response ) elapsed_ns: int = time.perf_counter_ns() - start_ns boundary_overhead_ns: int = max(0, elapsed_ns - planning_ns - rendering_ns) @@ -188,7 +184,7 @@ def _planning_error(*, error: ValueError) -> Exception: return NativeSqlTestPlanningError(f"native SQL-test planning failed: {message}") -def _plan_from_payload(*, value: object, dialect: str | None) -> NativeSqlTestPlan: +def _plan_from_payload(*, value: object) -> NativeSqlTestPlan: payload: dict[str, Any] = _object(value=value, context="result") sql: object = payload.get("sql") chain: object = payload.get("chain") @@ -206,11 +202,7 @@ def _plan_from_payload(*, value: object, dialect: str | None) -> NativeSqlTestPl assertions=tuple(_assertion_step(value=step) for step in assertions), model_names=_string_tuple(value=payload.get("modelNames"), context="model names"), warnings=tuple(_warning(value=warning) for warning in warnings), - sql=( - restore_sql_test_dialect_function_names(sql=sql, dialect=dialect) - if isinstance(sql, str) - else None - ), + sql=sql if isinstance(sql, str) else None, ) diff --git a/src/sqlbuild/compiler/planner/main/execution/sql_test_dialect.py b/src/sqlbuild/compiler/planner/main/execution/sql_test_dialect.py deleted file mode 100644 index f553df749d..0000000000 --- a/src/sqlbuild/compiler/planner/main/execution/sql_test_dialect.py +++ /dev/null @@ -1,11 +0,0 @@ -"""Adapter-specific SQL-test analysis output corrections.""" - -from sqlbuild.compiler.planner._helpers.sql_tests.comments import ( - restore_sql_test_dialect_function_names as _restore_sql_test_dialect_function_names, -) - - -def restore_sql_test_dialect_function_names(*, sql: str, dialect: str | None) -> str: - """Restore warehouse-supported function spellings after SQL analysis.""" - - return _restore_sql_test_dialect_function_names(sql=sql, dialect=dialect) diff --git a/src/sqlbuild/executor/testing/_helpers/difference_samples.py b/src/sqlbuild/executor/testing/_helpers/difference_samples.py index 24d1490128..f7b3fe9141 100644 --- a/src/sqlbuild/executor/testing/_helpers/difference_samples.py +++ b/src/sqlbuild/executor/testing/_helpers/difference_samples.py @@ -11,9 +11,6 @@ import sqlbuild._native as _native from sqlbuild.adapter.contract.classes.base_adapter import BaseAdapter from sqlbuild.adapter.contract.types import TypeDialect -from sqlbuild.compiler.planner.main.execution.sql_test_dialect import ( - restore_sql_test_dialect_function_names, -) from sqlbuild.compiler.planner.models import ChainStep, SqlTestPlanEntry from sqlbuild.diagnostics.classes.diagnostic_record_redactor import DiagnosticRecordRedactor from sqlbuild.diagnostics.main.log_debug_event import log_debug_event @@ -143,7 +140,7 @@ def build_sql_test_difference_sample_sql( sql: object = response.get("sql") if isinstance(response, dict) else None if not isinstance(sql, str): raise SqlTestRenderingError("native SQL-test difference rendering returned invalid SQL") - return restore_sql_test_dialect_function_names(sql=sql, dialect=sql_analysis_dialect) + return sql def _fetch_difference_samples( diff --git a/src/sqlbuild/executor/testing/_helpers/expected_column_probe.py b/src/sqlbuild/executor/testing/_helpers/expected_column_probe.py index 93671e5686..62b071b5a9 100644 --- a/src/sqlbuild/executor/testing/_helpers/expected_column_probe.py +++ b/src/sqlbuild/executor/testing/_helpers/expected_column_probe.py @@ -9,9 +9,6 @@ import sqlbuild._native as _native from sqlbuild.adapter.contract.classes.base_adapter import BaseAdapter -from sqlbuild.compiler.planner.main.execution.sql_test_dialect import ( - restore_sql_test_dialect_function_names, -) from sqlbuild.compiler.planner.models import SqlTestPlanEntry from sqlbuild.diagnostics.classes.diagnostic_record_redactor import DiagnosticRecordRedactor from sqlbuild.diagnostics.main.log_debug_event import log_debug_event @@ -68,9 +65,7 @@ def _probe_actual_columns( try: cursor: Any = adapter.execute( connection=connection, - sql=restore_sql_test_dialect_function_names( - sql=sql, dialect=adapter.sql_analysis_dialect() - ), + sql=sql, ) description: Any | None = getattr(cursor, "description", None) except Exception as error: diff --git a/src/sqlbuild/executor/testing/main/comparison_sql_batch.py b/src/sqlbuild/executor/testing/main/comparison_sql_batch.py index a1ea6ded17..1ba1d0d9b0 100644 --- a/src/sqlbuild/executor/testing/main/comparison_sql_batch.py +++ b/src/sqlbuild/executor/testing/main/comparison_sql_batch.py @@ -7,9 +7,6 @@ import orjson import sqlbuild._native as _native -from sqlbuild.compiler.planner.main.execution.sql_test_dialect import ( - restore_sql_test_dialect_function_names, -) from sqlbuild.compiler.planner.models import SqlTestPlanEntry from sqlbuild.executor.testing._helpers.native_requests import comparison_request from sqlbuild.executor.testing.constants import SQL_TEST_NATIVE_RENDER_WORKERS @@ -53,10 +50,5 @@ def build_sql_test_comparison_sql_batch( sql: object = item_dict.get("sql") if item_dict is not None else None if not isinstance(sql, str): raise SqlTestRenderingError("native SQL-test rendering returned an invalid result") - rendered.append( - restore_sql_test_dialect_function_names( - sql=sql, - dialect=sql_analysis_dialect, - ) - ) + rendered.append(sql) return tuple(rendered) From f175d2ec9ddbf2bec9901d18ee9e5f0b2621c26d Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 1 Oct 2026 09:50:52 +0100 Subject: [PATCH 3/7] test(test): cover SQL-test and build SQL parity Check that rendered SQL tests contain each model's build SQL verbatim, apart from fixture relation substitution, keep Snowflake function synonyms as authored, refuse T-SQL CTE collisions, and run authored CTEs with comments, dollar quotes and nested WITH through the DuckDB CLI. --- .../cli/commands/main/test/_test_types.py | 7 ++ .../main/test/test_authored_sql_parity.py | 70 ++++++++++++ .../main/test/test_chain_cte_scope.py | 9 +- .../sqlbuild/executor/testing/_test_types.py | 9 ++ .../src/sqlbuild/executor/testing/helpers.py | 102 +++++++++++++++++ .../testing/test_sql_test_build_parity.py | 105 ++++++++++++++++++ .../sql_test_assembly/test_plan_test.py | 82 +++++++++++++- 7 files changed, 381 insertions(+), 3 deletions(-) create mode 100644 tests/e2e/src/sqlbuild/cli/commands/main/test/test_authored_sql_parity.py create mode 100644 tests/integration/src/sqlbuild/executor/testing/test_sql_test_build_parity.py 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 4fd75dc99c..ab022a8c92 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 @@ -205,3 +205,10 @@ class ChainCteScopeTestCase: queries: tuple[str, ...] expected_sql: str expected_exit_code: int = 0 + + +@dataclass(frozen=True) +class AuthoredSqlParityTestCase: + description: str + compiled_test_name: str + expected_fragments: tuple[str, ...] 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 new file mode 100644 index 0000000000..ba1fe591b1 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_authored_sql_parity.py @@ -0,0 +1,70 @@ +"""`sqb test` executes model SQL exactly as authored 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 AuthoredSqlParityTestCase +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project, run_sqb +from tests.integration.src.sqlbuild.executor.testing.helpers import ( + build_authored_cte_project_files, +) + + +@pytest.mark.parametrize( + "test_case", + ( + AuthoredSqlParityTestCase( + 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" + " SELECT id AS order_id, amount, 'it''s (fine)' AS note /* ( */\n" + " FROM __source__raw_orders)", + "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)", + ), + ), + AuthoredSqlParityTestCase( + description="nested WITH and a fixture CTE collision use the nested fallback", + compiled_test_name="test_orders.sql", + expected_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)", + "__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)", + ), + ), + ), + ids=lambda case: case.description, +) +def test_given_authored_ctes_when_testing_then_passes_and_runs_the_authored_sql( + tmp_path: Path, test_case: AuthoredSqlParityTestCase +) -> None: + project: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="authored_ctes", + repo_files=build_authored_cte_project_files(), + ) + + tested: CompletedProcess[str] = run_sqb(command=("--no-color", "test"), project_dir=project) + compiled: CompletedProcess[str] = run_sqb( + command=("--no-color", "compile"), project_dir=project + ) + + assert tested.returncode == 0, tested.stdout + tested.stderr + assert "PASS=2 FAIL=0" in tested.stdout, tested.stdout + assert compiled.returncode == 0, compiled.stdout + compiled.stderr + sql: str = next( + (project / "target" / "compiled" / "tests").rglob(test_case.compiled_test_name) + ).read_text() + for fragment in test_case.expected_fragments: + assert fragment in sql, sql + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_chain_cte_scope.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_chain_cte_scope.py index e2a6712a56..91ccf82e35 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_chain_cte_scope.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_chain_cte_scope.py @@ -1,5 +1,6 @@ """Chained model CTEs retain their lexical scope through the real DuckDB CLI.""" +import re from pathlib import Path from subprocess import CompletedProcess @@ -9,6 +10,8 @@ from tests.e2e.src.sqlbuild.cli.commands.main.test.helpers import build_cte_scope_project_files from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project, run_sqb +_RELATION_MARKER: re.Pattern[str] = re.compile(r'__(ref|source)\("(\w+)"\)') + _FIRST: str = ( 'WITH final AS (SELECT order_id FROM __source("raw_orders")) SELECT final.order_id FROM final' ) @@ -74,7 +77,11 @@ def test_given_overlapping_ctes_when_testing_chain_then_each_model_uses_its_own_ sql: str = next( (project / "target" / "compiled" / "tests").rglob("order_chain.sql") ).read_text() - assert "__sqb_cte_" in sql + assert "__sqb_cte_" not in sql + upstream_queries: tuple[str, ...] = tuple( + _RELATION_MARKER.sub(r"__\1__\2", query) for query in test_case.queries[:-1] + ) + assert all(f"AS ({query})" in sql for query in upstream_queries), sql @pytest.mark.parametrize( diff --git a/tests/integration/src/sqlbuild/executor/testing/_test_types.py b/tests/integration/src/sqlbuild/executor/testing/_test_types.py index 89fb318dc3..bf13ea00ec 100644 --- a/tests/integration/src/sqlbuild/executor/testing/_test_types.py +++ b/tests/integration/src/sqlbuild/executor/testing/_test_types.py @@ -55,3 +55,12 @@ class SqlTestDiagnosticsTestCase: class ExpectedBooleanTestCase: description: str expected_result: bool + + +@dataclass(frozen=True) +class SqlTestBuildParityTestCase: + description: str + test_name: str + model_name: str + expected_whole_body_rendered: bool + expected_verbatim_fragments: tuple[str, ...] = () diff --git a/tests/integration/src/sqlbuild/executor/testing/helpers.py b/tests/integration/src/sqlbuild/executor/testing/helpers.py index c6502c2677..6fc46926d5 100644 --- a/tests/integration/src/sqlbuild/executor/testing/helpers.py +++ b/tests/integration/src/sqlbuild/executor/testing/helpers.py @@ -2,15 +2,22 @@ from __future__ import annotations +import re +from pathlib import Path from typing import Any from sqlbuild.adapters.duckdb.classes.duckdb_adapter import DuckDbAdapter from sqlbuild.compiler.compile.models import CompiledObjectKey from sqlbuild.compiler.compile.types import CompiledResourceType +from sqlbuild.compiler.pipeline.models import CompilePipelineResult from sqlbuild.compiler.planner.models import ChainStep, SqlTestPlanEntry from sqlbuild.executor.testing.main._execute import execute_sql_test +from sqlbuild.executor.testing.main.comparison_sql import build_sql_test_comparison_sql from sqlbuild.executor.testing.models import SqlTestExecutionResult from sqlbuild.executor.testing.types import SqlTestOutcome +from tests.integration.src.sqlbuild.compiler.pipeline.helpers import ( + run_compile_pipeline_for_project, +) from tests.integration.src.sqlbuild.executor.testing._test_types import ( SqlTestExecutionTestCase, ) @@ -76,3 +83,98 @@ def verify_test_result( models_by_outcome[step_result.outcome].append(step_result.model_name) failed_models: tuple[str, ...] = tuple(models_by_outcome[SqlTestOutcome.FAIL]) assert failed_models == test_case.expected_failed_models + + +_MARKER_CALL: re.Pattern[str] = re.compile( + r'__(?:ref|source|seed|dbt_ref|udf|table_fn)\(\s*"[^"]*"(?:\s*,\s*"[^"]*")*\s*\)' +) +_FIXTURE_RELATION: str = r'(?:[\w."`]+|\((?s:.*?)\))' + + +def build_sql_matches_test_body(*, build_sql: str, test_body: str) -> bool: + """Whether a test body is the build SQL byte for byte, apart from relation markers. + + A marker becomes a fixture or chain CTE name, or an inlined parenthesised fixture query. + """ + + parts: list[str] = _MARKER_CALL.split(build_sql) + return ( + re.fullmatch(_FIXTURE_RELATION.join(re.escape(part) for part in parts), test_body) + is not None + ) + + +def render_project_test_step( + *, project_dir: Path, test_name: str, model_name: str +) -> tuple[str, str, str]: + """Compile a DuckDB project and return one step's build SQL, test body and rendered test SQL.""" + + adapter: DuckDbAdapter = DuckDbAdapter() + result: CompilePipelineResult = run_compile_pipeline_for_project( + project_dir=project_dir, adapter=adapter + ) + entry: SqlTestPlanEntry = {item.name: item for item in result.plan_output.test_entries}[ + test_name + ] + step: ChainStep = {item.model_name: item for item in entry.chain}[model_name] + build_sql: str = {model.name: model.query_sql for model in result.project.models}[model_name] + rendered_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(), + ) + return build_sql, step.comparison_body_sql or step.resolved_sql, rendered_sql + + +def build_authored_cte_project_files() -> dict[str, str]: + """Build a DuckDB project whose models use comments, dollar quotes and nested WITH in CTEs.""" + + return { + "sqlbuild_project.toml": ( + 'name = "authored_ctes"\n' + 'adapter = "duckdb"\n\n' + "[connection]\n" + 'database = "authored_ctes.duckdb"\n\n' + "[settings]\n" + "sql_analysis = true\n" + ), + "sources/raw.yml": ( + "sources:\n - name: raw_orders\n schema: main\n table: raw_orders\n" + ), + "models/stg_orders.sql": ( + "MODEL (materialized table);\n\n" + "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")\n' + "),\n" + "tagged AS (SELECT order_id, amount, note, $$a ) b$$ AS tag FROM base)\n" + "SELECT order_id, amount, note, tag FROM tagged\n" + ), + "models/orders.sql": ( + "MODEL (materialized table);\n\n" + "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\n" + "),\n" + "helper_rows AS (SELECT order_id, doubled, tag FROM totals)\n" + "SELECT order_id, doubled, tag FROM helper_rows\n" + ), + "tests/unit/test_stg_orders.sql": ( + "TEST();\n\n" + "WITH\n" + "__source__raw_orders AS (SELECT 1 AS id, 10 AS amount),\n" + "__expected__stg_orders AS (\n" + " SELECT 1 AS order_id, 10 AS amount, 'it''s (fine)' AS note, 'a ) b' AS tag\n" + ")\n" + "SELECT 1\n" + ), + "tests/unit/test_orders.sql": ( + "TEST();\n\n" + "WITH\n" + "__ref__stg_orders AS (SELECT 1 AS order_id, 10 AS amount, 'x' AS tag),\n" + "helper_rows AS (SELECT 1 AS order_id, 20 AS doubled, 'x' AS tag),\n" + "__expected__orders AS (SELECT order_id, doubled, tag FROM helper_rows)\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 new file mode 100644 index 0000000000..495b7e1612 --- /dev/null +++ b/tests/integration/src/sqlbuild/executor/testing/test_sql_test_build_parity.py @@ -0,0 +1,105 @@ +"""SQL tests render the SQL `build` sends, apart from fixture relation substitution.""" + +from __future__ import annotations + +import shutil +from collections.abc import Callable +from pathlib import Path + +import pytest + +from tests.integration.src.sqlbuild.executor.testing._test_types import ( + SqlTestBuildParityTestCase, +) +from tests.integration.src.sqlbuild.executor.testing.helpers import ( + build_authored_cte_project_files, + build_sql_matches_test_body, + render_project_test_step, +) + +_WAFFLE_SHOP: Path = Path(__file__).resolve().parents[6] / "tests/e2e/fixtures/waffle_shop" + + +@pytest.mark.parametrize( + "test_case", + [ + SqlTestBuildParityTestCase( + description="staging model reading a source", + test_name="test_stg_orders", + model_name="stg_orders", + expected_whole_body_rendered=True, + ), + SqlTestBuildParityTestCase( + description="mart model joining refs and calling UDFs", + test_name="test_fact_orders", + model_name="fact_orders", + expected_whole_body_rendered=True, + ), + ], + ids=lambda case: case.description, +) +def test_given_fixture_project_test_when_rendering_then_whole_build_sql_appears_verbatim( + test_case: SqlTestBuildParityTestCase, tmp_path: Path +) -> None: + project_dir: Path = tmp_path / "waffle_shop" + shutil.copytree(_WAFFLE_SHOP, project_dir) + + build_sql, test_body, rendered_sql = render_project_test_step( + project_dir=project_dir, test_name=test_case.test_name, model_name=test_case.model_name + ) + + 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 + + +@pytest.mark.parametrize( + "test_case", + [ + SqlTestBuildParityTestCase( + description="CTE bodies with comments, quotes and dollar quotes are lifted verbatim", + test_name="test_stg_orders", + model_name="stg_orders", + expected_whole_body_rendered=False, + expected_verbatim_fragments=( + "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)", + "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)", + ), + ), + SqlTestBuildParityTestCase( + description="nested WITH stays inside its lifted CTE beside a colliding fixture CTE", + test_name="test_orders", + model_name="orders", + expected_whole_body_rendered=False, + 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)", + ), + ), + ], + ids=lambda case: case.description, +) +def test_given_model_ctes_when_rendering_then_build_sql_slices_appear_verbatim( + test_case: SqlTestBuildParityTestCase, + tmp_path: Path, + write_repo_files: Callable[[Path, dict[str, str]], None], +) -> None: + write_repo_files(tmp_path, build_authored_cte_project_files()) + + build_sql, test_body, rendered_sql = render_project_test_step( + project_dir=tmp_path, test_name=test_case.test_name, model_name=test_case.model_name + ) + + assert build_sql_matches_test_body(build_sql=build_sql, test_body=test_body), test_body + assert all(fragment in rendered_sql for fragment in test_case.expected_verbatim_fragments), ( + rendered_sql + ) + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) 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 14cbb7def1..08740712e6 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 @@ -1407,11 +1407,41 @@ def test_given_sql_analysis_assertion_when_planning_then_uses_shared_chain_ctes( dialect="snowflake", expected_sql_fragment="STARTSWITH(name, 'A')", expected_absent_sql_fragment="STARTS_WITH", - ) + ), + SqlAnalysisDialectTestCase( + description="Snowflake SUBSTR spelling is preserved", + query_sql=( + "WITH picked AS (SELECT SUBSTR(name, 1, 2) AS matches " + 'FROM __ref("items")) SELECT matches FROM picked' + ), + dialect="snowflake", + expected_sql_fragment="picked AS (SELECT SUBSTR(name, 1, 2) AS matches FROM", + expected_absent_sql_fragment="SUBSTRING", + ), + SqlAnalysisDialectTestCase( + description="Snowflake TIMESTAMPDIFF spelling is preserved", + query_sql=( + "WITH picked AS (SELECT TIMESTAMPDIFF(day, created_at, updated_at) AS matches " + 'FROM __ref("items")) SELECT matches FROM picked' + ), + dialect="snowflake", + expected_sql_fragment="TIMESTAMPDIFF(day, created_at, updated_at) AS matches", + expected_absent_sql_fragment="DATEDIFF", + ), + SqlAnalysisDialectTestCase( + description="Snowflake TRY_TO_DECIMAL spelling is preserved", + query_sql=( + "WITH picked AS (SELECT TRY_TO_DECIMAL(name, 10, 2) AS matches " + 'FROM __ref("items")) SELECT matches FROM picked' + ), + dialect="snowflake", + expected_sql_fragment="TRY_TO_DECIMAL(name, 10, 2) AS matches", + expected_absent_sql_fragment="TRY_CAST", + ), ], ids=lambda case: case.description, ) -def test_given_adapter_function_when_resolving_with_analysis_then_emits_supported_spelling( +def test_given_adapter_function_when_resolving_with_analysis_then_keeps_authored_spelling( test_case: SqlAnalysisDialectTestCase, ) -> None: compiled_test: CompiledSqlTest @@ -1454,6 +1484,54 @@ def test_given_adapter_function_when_resolving_with_analysis_then_emits_supporte assert test_case.expected_absent_sql_fragment not in comparison_sql +@pytest.mark.parametrize( + "test_case", + [ + PlanTestChainTestCase( + description="model and fixture CTEs share a name on T-SQL", + model_queries={ + "orders": ( + 'WITH helper_rows AS (SELECT order_id FROM __ref("raw_orders")) ' + "SELECT order_id FROM helper_rows" + ) + }, + mock_ref_ctes={"raw_orders": "SELECT 1 AS order_id"}, + mock_source_ctes={}, + 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_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( + test_case: PlanTestChainTestCase, +) -> None: + compiled_test: CompiledSqlTest + project: CompiledProject + compiled_test, project = build_test_and_project(test_case) + + result: SqlTestPlanResult + result, _ = plan_single_test_allowing_errors( + test=compiled_test, + project=project, + adapter=build_comparison_test_adapter("sqlserver"), + sql_analysis_enabled=True, + ) + + 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 + ) + + @pytest.mark.parametrize( "test_case", [ From fa05ea36bfd3db23e07fdd9ac8cf2f5a2e74cc20 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 1 Oct 2026 10:38:56 +0100 Subject: [PATCH 4/7] fix(test): rename colliding T-SQL CTEs and drop statement terminators On T-SQL, rename a CTE whose name collides with an already lifted CTE, together with its relation references, by token span to a fresh name when every other token is provably unchanged; refuse otherwise. Drop trailing statement terminators from lifted bodies, compare lifted CTE text ignoring surrounding whitespace, and name upstream models in collision messages. --- .../compiler/_helpers/sql_tests/cte_rename.rs | 153 ++++++++++++++++++ .../compiler/_helpers/sql_tests/cte_slices.rs | 49 +++++- .../src/compiler/_helpers/sql_tests/mod.rs | 1 + .../compiler/_helpers/sql_tests/rendering.rs | 113 ++++++++++--- .../src/compiler/tests.rs | 2 + .../src/compiler/tests/helpers.rs | 76 ++++++++- .../tests/test_sql_test_cte_rename.rs | 80 +++++++++ .../compiler/tests/test_sql_test_rendering.rs | 30 +++- .../src/compiler/tests/test_types.rs | 7 + .../main/test/test_authored_sql_parity.py | 36 +++++ .../src/sqlbuild/executor/testing/helpers.py | 60 +++++++ .../testing/test_sql_test_build_parity.py | 30 ++++ 12 files changed, 604 insertions(+), 33 deletions(-) create mode 100644 crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_rename.rs create mode 100644 crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_cte_rename.rs diff --git a/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_rename.rs b/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_rename.rs new file mode 100644 index 0000000000..9ed611f673 --- /dev/null +++ b/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_rename.rs @@ -0,0 +1,153 @@ +//! Token-span CTE renaming for dialects that reject a nested WITH. + +use crate::compiler::_helpers::sql_tests::cte_slices::{ + SliceDialect, WithSlices, opaque_end, read_identifier, +}; +use crate::sql_scan::main::comment_end::comment_end; +use crate::sql_scan::main::skip_whitespace::skip_whitespace; +use crate::sql_scan::models::Unclosed; + +const DOT: &str = "."; + +/// One lexical token of a statement: its byte span and, for identifiers, its case-folded key. +#[derive(Debug, PartialEq, Eq)] +struct Token { + start: usize, + end: usize, + key: Option, + comment: bool, +} + +/// One CTE of a split statement to rename: its index in the WITH list and its new name. +pub(crate) struct CteRename<'a> { + pub(crate) index: usize, + pub(crate) name: &'a str, +} + +/// Rename CTEs and their relation references by token span; `None` when not provably safe. +pub(crate) fn rename_ctes( + sql: &str, + split: &WithSlices<'_>, + renames: &[CteRename<'_>], + dialect: SliceDialect, +) -> Option { + let tokens = match tokens(sql, dialect) { + Ok(tokens) => tokens, + Err(_) => return None, + }; + let mut replaced: Vec<(usize, &str)> = Vec::new(); + for rename in renames { + let cte = split.ctes.get(rename.index)?; + let definition = tokens + .iter() + .position(|token| token.start == cte.header_start)?; + for (position, token) in tokens.iter().enumerate() { + if token.key.as_deref() != Some(cte.key.as_str()) { + continue; + } + if position < definition { + return None; + } + if position == definition || renamed_reference(sql, &tokens, position)? { + replaced.push((position, rename.name)); + } + } + } + replaced.sort_unstable(); + let mut renamed = String::with_capacity(sql.len()); + let mut cursor = 0; + for (position, name) in &replaced { + let token = &tokens[*position]; + renamed.push_str(&sql[cursor..token.start]); + renamed.push_str(name); + cursor = token.end; + } + renamed.push_str(&sql[cursor..]); + let renamed_tokens = match self::tokens(&renamed, dialect) { + Ok(renamed_tokens) => renamed_tokens, + Err(_) => return None, + }; + preserves_tokens((sql, &tokens), (&renamed, &renamed_tokens), &replaced).then_some(renamed) +} + +/// Rename (`Some(true)`), keep a qualified other object (`Some(false)`), or refuse (`None`). +fn renamed_reference(sql: &str, tokens: &[Token], position: usize) -> Option { + let previous = significant(tokens, position, -1); + let next = significant(tokens, position, 1); + if previous.is_some_and(|token| is_dot(sql, token)) { + return Some(false); + } + if next.is_some_and(|token| is_dot(sql, token)) { + return Some(true); + } + match previous.and_then(|token| token.key.as_deref()) { + Some("from" | "join") => Some(true), + _ => None, + } +} + +fn is_dot(sql: &str, token: &Token) -> bool { + token.key.is_none() && !token.comment && &sql[token.start..token.end] == DOT +} + +fn significant(tokens: &[Token], position: usize, step: isize) -> Option<&Token> { + let mut index = position.checked_add_signed(step)?; + while let Some(token) = tokens.get(index) { + if !token.comment { + return Some(token); + } + index = index.checked_add_signed(step)?; + } + None +} + +/// Require every untouched token of the renamed SQL to be byte-identical to the original. +fn preserves_tokens( + (sql, original): (&str, &[Token]), + (renamed_sql, renamed): (&str, &[Token]), + replaced: &[(usize, &str)], +) -> bool { + if renamed.len() != original.len() { + return false; + } + for (position, (before, after)) in original.iter().zip(renamed).enumerate() { + let after_text = &renamed_sql[after.start..after.end]; + let expected = replaced + .iter() + .find(|(index, _)| *index == position) + .map_or(&sql[before.start..before.end], |(_, name)| *name); + if after_text != expected { + return false; + } + } + true +} + +fn tokens(sql: &str, dialect: SliceDialect) -> Result, Unclosed> { + let bytes = sql.as_bytes(); + let mut tokens: Vec = Vec::new(); + let mut index = skip_whitespace(sql, 0); + while index < bytes.len() { + let comment = comment_end(bytes, index)?; + let (end, key) = if let Some(end) = comment { + (end, None) + } else if let Some((key, end)) = read_identifier(sql, index, dialect)? { + (end, Some(key)) + } else if let Some(end) = opaque_end(bytes, index, dialect)? { + (end, None) + } else { + ( + index + sql[index..].chars().next().map_or(1, char::len_utf8), + None, + ) + }; + tokens.push(Token { + start: index, + end, + key, + comment: comment.is_some(), + }); + index = skip_whitespace(sql, end); + } + Ok(tokens) +} diff --git a/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_slices.rs b/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_slices.rs index 95dc4d98c2..ba2f6f65ab 100644 --- a/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_slices.rs +++ b/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_slices.rs @@ -38,6 +38,7 @@ impl SliceDialect { /// One authored CTE: its header (name plus optional column list) and parenthesised body. #[derive(Debug, PartialEq, Eq)] pub(crate) struct CteSlice<'a> { + pub(crate) header_start: usize, pub(crate) header: &'a str, pub(crate) key: String, pub(crate) body: &'a str, @@ -83,6 +84,7 @@ pub(crate) fn split_top_level_with( } let close = matching_paren(sql, cursor, dialect)?; ctes.push(CteSlice { + header_start: index, header: &sql[index..header_end], key, body: &sql[cursor + 1..close], @@ -92,7 +94,7 @@ pub(crate) fn split_top_level_with( index = skip_ignorable(sql, next + 1)?; continue; } - let body = sql[skip_whitespace(sql, close + 1)..].trim_end(); + let body = strip_statement_terminators(&sql[skip_whitespace(sql, close + 1)..], dialect); if body.is_empty() || matches!(sql.as_bytes().get(next), Some(b')' | b';') | None) { return Ok(None); } @@ -114,7 +116,7 @@ pub(crate) fn used_ctes(split: &WithSlices<'_>, dialect: SliceDialect) -> Vec HashSet { +pub(crate) fn identifier_keys(sql: &str, dialect: SliceDialect) -> HashSet { let bytes = sql.as_bytes(); let mut keys: HashSet = HashSet::new(); let mut index = 0; @@ -144,7 +146,45 @@ fn is_tsql(name: &str) -> bool { ) } -fn read_identifier( +/// Drop trailing `;` terminators and the whitespace or comments after them. +pub(crate) fn strip_statement_terminators(sql: &str, dialect: SliceDialect) -> &str { + let bytes = sql.as_bytes(); + let mut code_end = 0; + let mut terminated = false; + let mut index = 0; + while index < bytes.len() { + match comment_end(bytes, index) { + Ok(Some(end)) => { + index = end; + continue; + } + Ok(None) => {} + Err(_) => return sql.trim_end(), + } + let next = match opaque_end(bytes, index, dialect) { + Ok(Some(end)) => end, + Ok(None) => index + sql[index..].chars().next().map_or(1, char::len_utf8), + Err(_) => return sql.trim_end(), + }; + match bytes[index] { + b';' => terminated = true, + byte if byte.is_ascii_whitespace() => {} + _ => { + code_end = next; + terminated = false; + } + } + index = next; + } + if terminated { + sql[..code_end].trim_end() + } else { + sql.trim_end() + } +} + +/// Read the identifier at `start`, returning its case-folded key and end offset. +pub(crate) fn read_identifier( sql: &str, start: usize, dialect: SliceDialect, @@ -217,7 +257,8 @@ fn matching_paren(sql: &str, open: usize, dialect: SliceDialect) -> Result bool { - self.key == key && (!same_header(&self.header, header) || self.authored != authored) + self.key == key + && (!same_header(&self.header, header) || self.authored.trim() != authored.trim()) } } @@ -123,6 +126,7 @@ struct RenderCteState { dialect: SliceDialect, lifted: Vec, name_counts: CteSuffixCounter, + renamed_ctes: usize, } impl RenderCteState { @@ -131,11 +135,13 @@ impl RenderCteState { dialect, lifted: Vec::new(), name_counts: CteSuffixCounter::default(), + renamed_ctes: 0, } } /// 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() { return Ok(sql.to_string()); } @@ -143,9 +149,12 @@ impl RenderCteState { Ok(Some(split)) => split, Ok(None) | Err(_) => return self.nested(sql, enabled, origin, &[]), }; - let collisions = self.collisions(split.ctes.iter()); - if !collisions.is_empty() { - let message = collision_message(origin, &collisions); + let colliding = self.colliding_indices(&split, &vec![true; split.ctes.len()]); + if !colliding.is_empty() { + if let Some(renamed) = self.rename_colliding(sql, &split, &colliding) { + return self.lift(&renamed, enabled, origin); + } + let message = self.collision_description(origin, &split, &colliding); return self.nested(sql, enabled, origin, &[message]); } if has_duplicate_keys(split.ctes.iter().map(|cte| cte.key.as_str())) { @@ -165,6 +174,7 @@ impl RenderCteState { origin: &str, collisions: &[String], ) -> Result { + let sql = strip_statement_terminators(sql, self.dialect); if !(enabled && self.dialect.rejects_nested_with() && leading_with_prefix_end(sql).is_some()) @@ -268,7 +278,7 @@ impl RenderCteState { if self.lifted.iter().any(|existing| existing.key == key) { continue; } - let origin = format!("CTE '{name}'"); + let origin = generated_cte_origin(name); let body = if enabled && self.dialect.rejects_nested_with() { self.flatten(sql, &origin)? } else { @@ -299,35 +309,78 @@ impl RenderCteState { Ok(Some(split)) => split, Ok(None) | Err(_) => return self.nested(sql, true, origin, &[]), }; - let used_flags = used_ctes(&split, self.dialect); - let used: Vec<&CteSlice<'_>> = split - .ctes - .iter() - .zip(used_flags) - .filter_map(|(cte, used)| used.then_some(cte)) - .collect(); - let collisions = self.collisions(used.iter().copied()); - if !collisions.is_empty() { - let message = collision_message(origin, &collisions); + let used = used_ctes(&split, self.dialect); + let colliding = self.colliding_indices(&split, &used); + if !colliding.is_empty() { + if let Some(renamed) = self.rename_colliding(sql, &split, &colliding) { + return self.flatten(&renamed, origin); + } + let message = self.collision_description(origin, &split, &colliding); return self.nested(sql, true, origin, &[message]); } - for cte in used { + for (cte, _) in split.ctes.iter().zip(used).filter(|(_, used)| *used) { self.push(cte.key.clone(), cte.header, cte.body, origin); } Ok(split.body.to_string()) } - fn collisions<'s>( + /// Indices of the selected CTEs whose names are already lifted with different text. + fn colliding_indices(&self, split: &WithSlices<'_>, selected: &[bool]) -> Vec { + let mut indices: Vec = Vec::new(); + for (index, (cte, selected)) in split.ctes.iter().zip(selected).enumerate() { + if *selected && self.conflict(&cte.key, cte.header, cte.body).is_some() { + indices.push(index); + } + } + indices + } + + fn collision_description( &self, - ctes: impl Iterator>, - ) -> Vec<(&'s str, &LiftedCte)> { - let mut collisions: Vec<(&'s str, &LiftedCte)> = Vec::new(); - for cte in ctes { + origin: &str, + split: &WithSlices<'_>, + indices: &[usize], + ) -> String { + let mut collisions: Vec<(&str, &LiftedCte)> = Vec::new(); + for cte in indices.iter().filter_map(|index| split.ctes.get(*index)) { if let Some(existing) = self.conflict(&cte.key, cte.header, cte.body) { collisions.push((cte.header, existing)); } } - collisions + collision_message(origin, &collisions) + } + + /// Where a nested WITH is rejected, rename colliding CTEs by token span to fresh names. + fn rename_colliding( + &mut self, + sql: &str, + split: &WithSlices<'_>, + indices: &[usize], + ) -> Option { + if !self.dialect.rejects_nested_with() { + return None; + } + let taken = identifier_keys(sql, self.dialect); + let names: Vec = indices.iter().map(|_| self.fresh_name(&taken)).collect(); + let renames: Vec> = indices + .iter() + .zip(&names) + .map(|(index, name)| CteRename { + index: *index, + name, + }) + .collect(); + rename_ctes(sql, split, &renames, self.dialect) + } + + fn fresh_name(&mut self, taken: &HashSet) -> String { + loop { + let name = format!("__sqb_cte_{}", self.renamed_ctes); + self.renamed_ctes += 1; + if !taken.contains(&name) && !self.lifted.iter().any(|cte| cte.key == name) { + return name; + } + } } fn conflict(&self, key: &str, header: &str, body: &str) -> Option<&LiftedCte> { @@ -374,6 +427,20 @@ fn collision_message(origin: &str, collisions: &[(&str, &LiftedCte)]) -> String .join("; ") } +/// 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 [ + ("__ref__", "model"), + ("__source__", "source"), + ("__seed__", "seed"), + ] { + if let Some(rest) = name.strip_prefix(prefix) { + return format!("{kind} '{rest}'"); + } + } + format!("CTE '{name}'") +} + fn has_duplicate_keys<'k>(keys: impl Iterator) -> bool { let mut seen: HashSet<&str> = HashSet::new(); keys.into_iter().any(|key| !seen.insert(key)) diff --git a/crates/sqlbuild-rules-native/src/compiler/tests.rs b/crates/sqlbuild-rules-native/src/compiler/tests.rs index fb62ae1929..e5221df173 100644 --- a/crates/sqlbuild-rules-native/src/compiler/tests.rs +++ b/crates/sqlbuild-rules-native/src/compiler/tests.rs @@ -6,6 +6,8 @@ mod model_header_tokenization; mod sql_interpolation; #[path = "tests/test_sql_references.rs"] mod sql_references; +#[path = "tests/test_sql_test_cte_rename.rs"] +mod sql_test_cte_rename; #[path = "tests/test_sql_test_cte_slices.rs"] mod sql_test_cte_slices; #[path = "tests/test_sql_test_expected_columns.rs"] diff --git a/crates/sqlbuild-rules-native/src/compiler/tests/helpers.rs b/crates/sqlbuild-rules-native/src/compiler/tests/helpers.rs index 048b2f18b0..9c187d1549 100644 --- a/crates/sqlbuild-rules-native/src/compiler/tests/helpers.rs +++ b/crates/sqlbuild-rules-native/src/compiler/tests/helpers.rs @@ -1478,17 +1478,87 @@ pub(super) fn generated_with_bodies_stay_nested_verbatim() -> bool { ) } -pub(super) fn tsql_cte_collisions_are_refused_with_named_ctes() -> bool { - let error = render_one(colliding_chain_request("tsql", "final")).expect_err("T-SQL refusal"); +pub(super) fn tsql_model_cte_collisions_are_renamed_by_token_span() -> bool { + let sql = render_one(colliding_chain_request("tsql", "final")).expect("T-SQL rename"); + sql.starts_with( + "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", + ) +} + +pub(super) fn tsql_unprovable_cte_renames_are_refused_with_named_ctes() -> bool { + let mut request = colliding_chain_request("tsql", "final"); + request["chain"][0]["comparisonBodySql"] = json!( + "WITH final AS (SELECT 2 AS order_id FROM __ref__stg_orders) SELECT order_id AS final FROM final" + ); + let error = render_one(request).expect_err("T-SQL refusal"); assert_eq!( error, - "CTE 'final' of model 'orders' collides with CTE 'final' of CTE '__ref__stg_orders'; \ + "CTE 'final' of model 'orders' collides with CTE 'final' of model 'stg_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 } +pub(super) fn tsql_fixture_and_model_cte_collisions_are_refused() -> bool { + let error = render_one(json!({ + "sqlAnalysisDialect": "tsql", + "chain": [{ + "modelName": "orders", + "resolvedSql": "WITH helper_rows AS (SELECT 1 AS order_id) SELECT order_id FROM helper_rows", + "expectedCteSql": "SELECT order_id FROM helper_rows", + "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 +} + +pub(super) fn tsql_identical_helper_ending_in_line_comment_is_shared() -> bool { + let sql = render_one(json!({ + "sqlAnalysisDialect": "tsql", + "chain": [{ + "modelName": "orders", + "resolvedSql": "SELECT order_id FROM __source__raw", + "liftedCtes": [["__source__raw", "WITH h AS (SELECT 1 AS order_id -- one\n) SELECT * FROM h"]], + "comparisonBodySql": "SELECT order_id FROM __source__raw", + "expectedCteSql": "SELECT order_id FROM h", + "expectedLiftedCtes": [["h", "SELECT 1 AS order_id -- one"]] + }] + })) + .expect("shared helper"); + sql.starts_with( + "WITH h AS (SELECT 1 AS order_id -- one\n),\n__source__raw AS (SELECT * FROM h),\n", + ) && sql.contains("__expected__orders AS (SELECT order_id FROM h)") +} + +pub(super) fn trailing_statement_terminators_are_dropped() -> bool { + [ + "WITH a AS (SELECT 1 AS id) SELECT * FROM a; -- done\n", + "SELECT 1 AS id ;;\n", + ] + .iter() + .all(|model| { + let sql = render_one(json!({ + "sqlAnalysisDialect": "duckdb", + "chain": [{"modelName": "m", "resolvedSql": model, "expectedCteSql": "SELECT 1 AS id"}] + })) + .expect("render"); + !sql.contains(';') + && (sql.contains("__actual__m AS (SELECT * FROM a)") + || sql.contains("__actual__m AS (SELECT 1 AS id)")) + }) +} + pub(super) fn tsql_distinct_ctes_lift_verbatim() -> bool { let sql = render_one(json!({ "sqlAnalysisDialect": "tsql", diff --git a/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_cte_rename.rs b/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_cte_rename.rs new file mode 100644 index 0000000000..8a707a7027 --- /dev/null +++ b/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_cte_rename.rs @@ -0,0 +1,80 @@ +use crate::compiler::_helpers::sql_tests::cte_rename::{CteRename, rename_ctes}; +use crate::compiler::_helpers::sql_tests::cte_slices::{SliceDialect, split_top_level_with}; +use crate::compiler::tests::test_types::CteRenameTestCase; + +#[test] +fn given_tsql_cte_when_renaming_by_token_span_then_only_relation_references_change() { + let test_cases = [ + CteRenameTestCase { + description: "definition, FROM reference and qualifier are renamed", + sql: "WITH final AS (SELECT 1 AS id) SELECT final.id FROM final", + renamed_index: 0, + expected_sql: Some( + "WITH __sqb_cte_0 AS (SELECT 1 AS id) SELECT __sqb_cte_0.id FROM __sqb_cte_0", + ), + }, + CteRenameTestCase { + description: "references in later CTEs, subqueries and joins are renamed", + sql: "WITH final AS (SELECT 1 AS id), other AS (SELECT id FROM (SELECT id FROM final) AS s \ + WHERE id IN (SELECT id FROM final)) SELECT * FROM other JOIN final ON final.id = other.id", + renamed_index: 0, + expected_sql: Some( + "WITH __sqb_cte_0 AS (SELECT 1 AS id), other AS (SELECT id FROM (SELECT id FROM __sqb_cte_0) AS s \ + WHERE id IN (SELECT id FROM __sqb_cte_0)) SELECT * FROM other JOIN __sqb_cte_0 ON __sqb_cte_0.id = other.id", + ), + }, + CteRenameTestCase { + description: "bracketed and case-variant references are renamed", + sql: "WITH [Final] AS (SELECT 1 AS id) SELECT FINAL.id FROM [final] JOIN Final AS f ON f.id = FINAL.id", + renamed_index: 0, + expected_sql: Some( + "WITH __sqb_cte_0 AS (SELECT 1 AS id) SELECT __sqb_cte_0.id FROM __sqb_cte_0 \ + JOIN __sqb_cte_0 AS f ON f.id = __sqb_cte_0.id", + ), + }, + CteRenameTestCase { + description: "qualified columns, comments and strings named like the CTE are kept", + sql: "WITH final AS (SELECT t.final, 'final' AS label FROM dbo.final AS t) -- final\n\ + SELECT final.final FROM final", + renamed_index: 0, + expected_sql: Some( + "WITH __sqb_cte_0 AS (SELECT t.final, 'final' AS label FROM dbo.final AS t) -- final\n\ + SELECT __sqb_cte_0.final FROM __sqb_cte_0", + ), + }, + CteRenameTestCase { + description: "an unqualified column alias named like the CTE is ambiguous", + sql: "WITH final AS (SELECT 1 AS id) SELECT id AS final FROM final", + renamed_index: 0, + expected_sql: None, + }, + CteRenameTestCase { + description: "a reference before the definition is ambiguous", + sql: "WITH a AS (SELECT * FROM final), final AS (SELECT 1 AS id) SELECT * FROM a, final", + renamed_index: 1, + expected_sql: None, + }, + ]; + + for test_case in test_cases { + let dialect = SliceDialect::new(Some("tsql")); + let split = split_top_level_with(test_case.sql, dialect) + .expect("balanced SQL") + .expect("leading WITH"); + let actual = rename_ctes( + test_case.sql, + &split, + &[CteRename { + index: test_case.renamed_index, + name: "__sqb_cte_0", + }], + dialect, + ); + assert_eq!( + actual.as_deref(), + test_case.expected_sql, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_rendering.rs b/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_rendering.rs index c385224d46..44053015dc 100644 --- a/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_rendering.rs +++ b/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_rendering.rs @@ -8,7 +8,11 @@ use crate::compiler::tests::helpers::{ 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, - tsql_cte_collisions_are_refused_with_named_ctes, tsql_distinct_ctes_lift_verbatim, + trailing_statement_terminators_are_dropped, tsql_distinct_ctes_lift_verbatim, + tsql_fixture_and_model_cte_collisions_are_refused, + tsql_identical_helper_ending_in_line_comment_is_shared, + tsql_model_cte_collisions_are_renamed_by_token_span, + tsql_unprovable_cte_renames_are_refused_with_named_ctes, }; use crate::compiler::tests::test_types::SqlTestRenderingTestCase; @@ -26,8 +30,28 @@ fn given_sql_rendering_cases_when_rendering_native_batches_then_expected_behavio expected_success: true, }, SqlTestRenderingTestCase { - description: "T-SQL CTE collisions are refused naming the colliding CTEs", - run: tsql_cte_collisions_are_refused_with_named_ctes, + description: "T-SQL model CTE collisions are renamed by token span", + run: tsql_model_cte_collisions_are_renamed_by_token_span, + expected_success: true, + }, + SqlTestRenderingTestCase { + description: "T-SQL renames that cannot be proven safe are refused naming the CTEs", + run: tsql_unprovable_cte_renames_are_refused_with_named_ctes, + expected_success: true, + }, + SqlTestRenderingTestCase { + description: "T-SQL fixture and model CTE collisions are refused", + run: tsql_fixture_and_model_cte_collisions_are_refused, + expected_success: true, + }, + SqlTestRenderingTestCase { + description: "T-SQL identical helpers ending in a line comment are shared", + run: tsql_identical_helper_ending_in_line_comment_is_shared, + expected_success: true, + }, + SqlTestRenderingTestCase { + description: "trailing statement terminators are dropped from lifted bodies", + run: trailing_statement_terminators_are_dropped, expected_success: true, }, SqlTestRenderingTestCase { diff --git a/crates/sqlbuild-rules-native/src/compiler/tests/test_types.rs b/crates/sqlbuild-rules-native/src/compiler/tests/test_types.rs index d3e0342c92..ae64b2147e 100644 --- a/crates/sqlbuild-rules-native/src/compiler/tests/test_types.rs +++ b/crates/sqlbuild-rules-native/src/compiler/tests/test_types.rs @@ -53,3 +53,10 @@ pub(crate) struct CteSliceTestCase { pub(crate) sql: &'static str, pub(crate) expected_slices: Result, } + +pub(crate) struct CteRenameTestCase { + pub(crate) description: &'static str, + pub(crate) sql: &'static str, + pub(crate) renamed_index: usize, + pub(crate) expected_sql: Option<&'static str>, +} 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 ba1fe591b1..8f0df214a6 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 @@ -9,6 +9,7 @@ from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project, run_sqb from tests.integration.src.sqlbuild.executor.testing.helpers import ( build_authored_cte_project_files, + build_terminated_model_project_files, ) @@ -66,5 +67,40 @@ def test_given_authored_ctes_when_testing_then_passes_and_runs_the_authored_sql( assert fragment in sql, sql +@pytest.mark.parametrize( + "test_case", + ( + AuthoredSqlParityTestCase( + description="a WITH model ending in a statement terminator", + compiled_test_name="test_m.sql", + expected_fragments=("__actual__m AS (SELECT id FROM a)",), + ), + ), + ids=lambda case: case.description, +) +def test_given_model_ending_in_semicolon_when_testing_then_terminator_is_dropped_and_passes( + tmp_path: Path, test_case: AuthoredSqlParityTestCase +) -> None: + project: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="terminated", + repo_files=build_terminated_model_project_files(), + ) + + tested: CompletedProcess[str] = run_sqb(command=("--no-color", "test"), project_dir=project) + compiled: CompletedProcess[str] = run_sqb( + command=("--no-color", "compile"), project_dir=project + ) + + assert tested.returncode == 0, tested.stdout + tested.stderr + assert "PASS=1 FAIL=0" in tested.stdout, tested.stdout + assert compiled.returncode == 0, compiled.stdout + compiled.stderr + sql: str = next( + (project / "target" / "compiled" / "tests").rglob(test_case.compiled_test_name) + ).read_text() + for fragment in test_case.expected_fragments: + assert fragment in sql, sql + + if __name__ == "__main__": pytest.main([__file__, "-vv"]) diff --git a/tests/integration/src/sqlbuild/executor/testing/helpers.py b/tests/integration/src/sqlbuild/executor/testing/helpers.py index 6fc46926d5..d907c19a6a 100644 --- a/tests/integration/src/sqlbuild/executor/testing/helpers.py +++ b/tests/integration/src/sqlbuild/executor/testing/helpers.py @@ -178,3 +178,63 @@ def build_authored_cte_project_files() -> dict[str, str]: "SELECT 1\n" ), } + + +def build_shared_cte_name_chain_entry() -> SqlTestPlanEntry: + """Build a two-model chain whose upstream and tested models both define CTE `final`.""" + + upstream: str = "WITH final AS (SELECT id FROM __source__raw) SELECT * FROM final" + tested: str = "WITH final AS (SELECT id + 1 AS id FROM __ref__stg) SELECT final.id FROM final" + source_mock: tuple[str, str] = ("__source__raw", "SELECT 1 AS id") + return SqlTestPlanEntry( + key=CompiledObjectKey(resource_type=CompiledResourceType.SQL_TEST, name="mart_chain"), + name="mart_chain", + chain=( + ChainStep( + model_name="stg", + resolved_sql=upstream, + lifted_ctes=(source_mock,), + comparison_body_sql=upstream, + expected_cte_sql="SELECT 1 AS id", + ), + ChainStep( + model_name="mart", + resolved_sql=tested, + lifted_ctes=(source_mock, ("__ref__stg", upstream)), + comparison_body_sql=tested, + expected_cte_sql="SELECT 2 AS id", + ), + ), + ) + + +def comparison_rows(*, adapter: DuckDbAdapter, connection: Any, sql: str) -> list[tuple[Any, ...]]: + """Execute rendered comparison SQL and return its count rows.""" + + return list(adapter.execute(connection=connection, sql=sql).fetchall()) + + +def build_terminated_model_project_files() -> dict[str, str]: + """Build a DuckDB project whose WITH model ends in a statement terminator.""" + + return { + "sqlbuild_project.toml": ( + 'name = "terminated"\nadapter = "duckdb"\n\n[connection]\n' + 'database = "terminated.duckdb"\n\n[settings]\nsql_analysis = true\n' + ), + "sources/raw.yml": ( + "sources:\n - name: raw_orders\n schema: main\n table: raw_orders\n" + ), + "models/m.sql": ( + "MODEL (materialized table);\n\n" + 'WITH a AS (SELECT id FROM __source("raw_orders"))\n' + "SELECT id FROM a;\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" + "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 495b7e1612..837bace792 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 @@ -5,15 +5,22 @@ import shutil from collections.abc import Callable from pathlib import Path +from typing import Any import pytest +from sqlbuild.adapters.duckdb.classes.duckdb_adapter import DuckDbAdapter +from sqlbuild.compiler.planner.models import SqlTestPlanEntry +from sqlbuild.executor.testing.main.comparison_sql import build_sql_test_comparison_sql from tests.integration.src.sqlbuild.executor.testing._test_types import ( + ExpectedBooleanTestCase, SqlTestBuildParityTestCase, ) from tests.integration.src.sqlbuild.executor.testing.helpers import ( build_authored_cte_project_files, + build_shared_cte_name_chain_entry, build_sql_matches_test_body, + comparison_rows, render_project_test_step, ) @@ -101,5 +108,28 @@ def test_given_model_ctes_when_rendering_then_build_sql_slices_appear_verbatim( ) +@pytest.mark.parametrize( + "test_case", + [ExpectedBooleanTestCase(description="T-SQL rename matches nesting", expected_result=True)], + ids=lambda case: case.description, +) +def test_given_shared_cte_name_in_chain_when_rendering_tsql_then_rename_runs_like_nesting( + test_case: ExpectedBooleanTestCase, adapter: DuckDbAdapter, connection: Any +) -> None: + entry: SqlTestPlanEntry = build_shared_cte_name_chain_entry() + + renamed_sql: str = build_sql_test_comparison_sql(test_entry=entry, sql_analysis_dialect="tsql") + 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 "AS (WITH" not in renamed_sql + assert ( + comparison_rows(adapter=adapter, connection=connection, sql=renamed_sql) + == comparison_rows(adapter=adapter, connection=connection, sql=nested_sql) + == [(0, "stg", 1, 1, 0, 0), (1, "mart", 1, 1, 0, 0)] + ) is test_case.expected_result + + if __name__ == "__main__": pytest.main([__file__, "-vv"]) From 99292c7acad2b4e14a5b96462a9bb94472488b16 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 1 Oct 2026 10:51:11 +0100 Subject: [PATCH 5/7] fix(test): keep multipart object names when renaming T-SQL CTEs A CTE name followed by a dot in a relation position is the leading part of a database or schema qualified object, so it is kept rather than renamed. A column qualifier beside such an object is ambiguous and the rename is refused. --- .../compiler/_helpers/sql_tests/cte_rename.rs | 49 +++++++++++++++---- .../tests/test_sql_test_cte_rename.rs | 34 +++++++++++++ 2 files changed, 73 insertions(+), 10 deletions(-) diff --git a/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_rename.rs b/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_rename.rs index 9ed611f673..b3981e8258 100644 --- a/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_rename.rs +++ b/crates/sqlbuild-rules-native/src/compiler/_helpers/sql_tests/cte_rename.rs @@ -41,6 +41,8 @@ pub(crate) fn rename_ctes( let definition = tokens .iter() .position(|token| token.start == cte.header_start)?; + let mut multipart_object = false; + let mut qualifier = false; for (position, token) in tokens.iter().enumerate() { if token.key.as_deref() != Some(cte.key.as_str()) { continue; @@ -48,10 +50,20 @@ pub(crate) fn rename_ctes( if position < definition { return None; } - if position == definition || renamed_reference(sql, &tokens, position)? { + let usage = if position == definition { + Usage::Relation + } else { + usage(sql, &tokens, position)? + }; + multipart_object |= usage == Usage::MultipartObject; + qualifier |= usage == Usage::Qualifier; + if matches!(usage, Usage::Relation | Usage::Qualifier) { replaced.push((position, rename.name)); } } + if multipart_object && qualifier { + return None; + } } replaced.sort_unstable(); let mut renamed = String::with_capacity(sql.len()); @@ -70,19 +82,36 @@ pub(crate) fn rename_ctes( preserves_tokens((sql, &tokens), (&renamed, &renamed_tokens), &replaced).then_some(renamed) } -/// Rename (`Some(true)`), keep a qualified other object (`Some(false)`), or refuse (`None`). -fn renamed_reference(sql: &str, tokens: &[Token], position: usize) -> Option { +/// How one occurrence of a renamed CTE's name is used. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum Usage { + /// A relation reference to the CTE, renamed. + Relation, + /// A column qualifier naming the CTE, renamed. + Qualifier, + /// The leading part of a multipart object name such as `final.dbo.orders`, kept. + MultipartObject, + /// A trailing part of a qualified name such as `t.final`, kept. + QualifiedPart, +} + +/// Classify one occurrence, or `None` when its use is ambiguous. +fn usage(sql: &str, tokens: &[Token], position: usize) -> Option { let previous = significant(tokens, position, -1); let next = significant(tokens, position, 1); if previous.is_some_and(|token| is_dot(sql, token)) { - return Some(false); - } - if next.is_some_and(|token| is_dot(sql, token)) { - return Some(true); + return Some(Usage::QualifiedPart); } - match previous.and_then(|token| token.key.as_deref()) { - Some("from" | "join") => Some(true), - _ => None, + let relation_position = matches!( + previous.and_then(|token| token.key.as_deref()), + Some("from" | "join" | "apply") + ); + let dotted = next.is_some_and(|token| is_dot(sql, token)); + match (relation_position, dotted) { + (true, true) => Some(Usage::MultipartObject), + (true, false) => Some(Usage::Relation), + (false, true) => Some(Usage::Qualifier), + (false, false) => None, } } diff --git a/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_cte_rename.rs b/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_cte_rename.rs index 8a707a7027..9e68da2843 100644 --- a/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_cte_rename.rs +++ b/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_test_cte_rename.rs @@ -42,6 +42,40 @@ fn given_tsql_cte_when_renaming_by_token_span_then_only_relation_references_chan SELECT __sqb_cte_0.final FROM __sqb_cte_0", ), }, + CteRenameTestCase { + description: "a database named like the CTE in a three-part name is kept", + sql: "WITH final AS (SELECT 1 AS id) SELECT f.id FROM final f \ + JOIN final.dbo.orders o ON o.id = f.id", + renamed_index: 0, + expected_sql: Some( + "WITH __sqb_cte_0 AS (SELECT 1 AS id) SELECT f.id FROM __sqb_cte_0 f \ + JOIN final.dbo.orders o ON o.id = f.id", + ), + }, + CteRenameTestCase { + description: "a schema named like the CTE in a two-part name is kept", + sql: "WITH final AS (SELECT 1 AS id) SELECT f.id FROM final.orders o JOIN final f ON f.id = o.id", + renamed_index: 0, + expected_sql: Some( + "WITH __sqb_cte_0 AS (SELECT 1 AS id) SELECT f.id FROM final.orders o \ + JOIN __sqb_cte_0 f ON f.id = o.id", + ), + }, + CteRenameTestCase { + description: "column qualifiers in SELECT and ON are renamed", + sql: "WITH final AS (SELECT 1 AS id) SELECT final.id FROM final JOIN orders o ON o.id = final.id", + renamed_index: 0, + expected_sql: Some( + "WITH __sqb_cte_0 AS (SELECT 1 AS id) SELECT __sqb_cte_0.id FROM __sqb_cte_0 \ + JOIN orders o ON o.id = __sqb_cte_0.id", + ), + }, + CteRenameTestCase { + description: "a qualifier beside a multipart object of the same name is ambiguous", + sql: "WITH final AS (SELECT 1 AS id) SELECT final.id FROM final.orders", + renamed_index: 0, + expected_sql: None, + }, CteRenameTestCase { description: "an unqualified column alias named like the CTE is ambiguous", sql: "WITH final AS (SELECT 1 AS id) SELECT id AS final FROM final", From 12d3f36f28a825ee575de111cac91d565a61fd9f Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 1 Oct 2026 11:27:53 +0100 Subject: [PATCH 6/7] test(compile): update pinned semantic fingerprints for authored SQL test rendering --- .../compile/test_dense_compile_performance.py | 8 +++---- ...fresh_process_compile_cache_performance.py | 24 +++++++++---------- .../test_fresh_process_compile_performance.py | 6 ++--- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_dense_compile_performance.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_dense_compile_performance.py index 2c0c98418b..9a358f069e 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 @@ -38,28 +38,28 @@ 1000, 14.0, 3 * _GIB // 2, - "093fb28b99d2da6aa98158628b758338cbfd8742fdbf43cbe1beb4a161681370", + "cfb7bc162018ddc64f9d03ddd2c316e34a333d4129906d8e0b3310399604b980", ), DenseCompileGuardTestCase( "dense_models_3000_all_rules", 3000, 45.0, 11 * _GIB // 4, - "3d387c37e25d7241c4c79b495df6f2737d57bf839fc9bf97cebc23c1c0ed1e70", + "c35ea7ec71e6e81059146bc1cd72f10e6493248dc6274aba7737d616d434585b", ), DenseCompileGuardTestCase( "dense_models_5000_all_rules", 5000, 75.0, 13 * _GIB // 4, - "dfe3610ddca5cfd995b860ac28995d75958a70626304c9879de50a380e9abe65", + "b07422282a6800de7830b9c4950f76bdc6fd555ebd3f21152b8649f976be74dc", ), DenseCompileGuardTestCase( "dense_models_10000_all_rules", 10000, 145.0, 4 * _GIB, - "dbf8679c49a22a2869511bb08123f13ab0a079bb1722cc471ec52c115f4d4794", + "7a4b6cca4a2fd4b7f5961760177f8b18133854014eaf15a980e8283df646a2cf", ), ), ids=lambda case: case.description, 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 8ec922c485..aa0f9a0f67 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=( - "c7c4d0e6449728dd6ee59538a1a482d454418f54e617f2ffb4e24fb969dabac9" + "f96a10c8d9d218c347432118e069f378140bb6f3a852676d968ad9d5f67d2589" ), expected_leaf_edit_fingerprint=( - "1a5ca199da90a44604e3f855c4ca703374315732f3b4e21084de3550f2d46de7" + "1e3adee6c9d336de987ea15e3c054726cdf0a00ba8a2b4dcc2f0953b6711cf2c" ), expected_macro_edit_fingerprint=( - "fa0368493144fd99939f3a8ea8449e0c33bf05095cdf4a18975f77243a7c749e" + "81eb4ccff1c67d77a1a4290a239007b92f21bd3112729f64a508592166ed3c39" ), expected_project_config_fingerprint=( - "d3bf664c6dcb737522fd8e42fc950e91f085a684292eaf028a65cccd4fca62ed" + "c8fbec90f4c03c1bc8c13478121a2ae4c8b9d7c18b0270102130b689a2e76549" ), 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=( - "efb34659a1f5ac37221c36ee0f21e880f95fb668339f9ea61faddc6922607b30" + "26de3005daf1b44463b25165238e83c9a8085b00d942eb4beb1b735ce5f09b57" ), expected_leaf_edit_fingerprint=( - "7d8d0283e958bd00faf77e675e84185a78ff7a366393610b67ba43f77106d823" + "e032f1c0edf2e767f356a6af4891c4dcde0d214d6f691ea1dd0336968e97b707" ), expected_macro_edit_fingerprint=( - "b8218285d114afa179c63e6da5cf519743d89a21481f52068fc5044f5f3bec36" + "9e20327a775d2f876f25621d60f8bb81b62d529823a61b8d9686f86de322790c" ), expected_project_config_fingerprint=( - "658c9cbcd8cbdaddc7dd6f06211ae830074c686101c8814afcc0f9ed53c0defd" + "b65db3cfba6cccef479df505832f7e5821f03b7a0b31e8de967b14796644c768" ), 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=( - "766d9e58c926a0aabda70280a36e816b6efe120c1948aef85375f3d2ecdfb078" + "b4e5e14d9c1a7ac2e5fc54eac8d1ff2874f2e3042952cb4b80c68c4d4bd1503c" ), expected_leaf_edit_fingerprint=( - "6d0591c75b51daf3de94a873f5faa63f262a09d09f4d59c02fe1ba8c05e5ac14" + "fa0b24b685c7f42a14588ab09e7c929332295ecc83cc9adfdc589b4cbc1c6128" ), expected_macro_edit_fingerprint=( - "1b3bd8a3040eb092312ab520f4a842a1bb930b741d7b8ddfa06b472d72eaff54" + "39079f601b624ba4d13d889e9a228c9713ec7ed09ed2127afd3b2c2b48465fbb" ), expected_project_config_fingerprint=( - "e5a20011e3c48d82301c8d65238d532ae4479638e10cd5cb3477ad9293074dd1" + "6dd7ae799420dcb8a49190356a0b97253b59a3106502cc8cf242ee75c2aedaa2" ), 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 3613afaae4..f9e223e1c3 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=( - "5d557ae6e25e895e61e07c58f711cb457bb012d9b242aab62460f9a4aed315e9" + "b763ab23427ed273feb5f80b0b03cf084c7af2f1216500bc5ad7db0be23c2ac1" ), ), FreshProcessCompilePerformanceGuardTestCase( @@ -54,7 +54,7 @@ expected_max_wall_seconds=25.0, expected_max_rss_bytes=int(1.75 * _GIB), expected_semantic_fingerprint=( - "d36434f6d80c24519e2600fca6dcae06fc01cc2c0ec8aa50c390488835a26d45" + "fc03f9e22fcf029217e77d3c6e18c9d9945e98e9425a5420a9c4756219a8fd77" ), ), FreshProcessCompilePerformanceGuardTestCase( @@ -69,7 +69,7 @@ expected_max_wall_seconds=50.0, expected_max_rss_bytes=2 * _GIB, expected_semantic_fingerprint=( - "ea9c7a46f73788b952d1da3050d81acff09059676d997346779baa600083ed42" + "34e21096cdc9c3a34655f67660de9775b0ac6cebfb5275ce2d647f245ce23f79" ), ), ], From f1d34760453230ecb975a8f31363f1e100f1e7fc Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 1 Oct 2026 12:08:19 +0100 Subject: [PATCH 7/7] test(compile): update varied cache guard fingerprints for authored SQL test rendering --- .../main/compile/test_varied_compile_cache_performance.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_varied_compile_cache_performance.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_varied_compile_cache_performance.py index 581d1a3f4c..ef9a5f99ff 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, ( - "ba45ce336d0d636f7676204165e8e6b1b5622a140511c79c94234a149f9cf6ee", - "3994a72953957311ac82d5743a7bcaa93a9b39f8fdcf7efdc85ed1fc28db7a10", - "6002880e310c08aad7a28da3f397fdc88bbd477599b04a6755344fd1af5411bd", - "1f08064c1c53f511f2acfd32f06fbfc468c156540c8a4ea3649cb826f2b3a561", + "a0fa536e6e64d9be7027ca9a52b4e84431361ec4598aeff1c721fe266fdb096d", + "50e52f18c8bc90479f83d744594cbdefeb13e26a3740ef9bb2ef077f15d2cb5a", + "a08f6c4470d65e2964c10f9c41f6d59ef92089bc5d5bea80104495edb6080d99", + "30809e387b0b096fc592f7da6e429c0c4878c41ed9cc5ea579898fd7e1029687", ), ), ),