diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/analysis_cache.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/analysis_cache.rs index 21cc9eb6c3..3b4eb35339 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/analysis_cache.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/analysis_cache.rs @@ -552,6 +552,7 @@ pub(crate) fn decode_outcome( binding_validated, }, cleaned_sql: String::new(), + deferred_cleaning: None, validated_schema: schema.clone(), fused_binding_validated: true, }; diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/session.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/session.rs index 54eadd5488..74e8b174c9 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/session.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/session.rs @@ -31,9 +31,9 @@ use crate::assembly::analysis_session::_helpers::mappings::{ use crate::assembly::analysis_session::_helpers::mappings::{producers, waves}; use crate::assembly::analysis_session::constants::{UNKNOWN_NULLABILITY, UNKNOWN_TYPE}; use crate::assembly::analysis_session::models::{ - AnalysisSession, ColumnFact, ContractProof, FinishedSession, LineageFacts, LineageRow, - ModelAnalysis, ModelOutcome, ModelRequest, Phase, PivotOutcome, PivotTables, SessionModelFacts, - SessionOutcome, SessionRequest, SessionStep, + AnalysisSession, ColumnFact, ContractProof, DeferredCleaning, FinishedSession, LineageFacts, + LineageRow, ModelAnalysis, ModelOutcome, ModelRequest, Phase, PivotOutcome, PivotTables, + SessionModelFacts, SessionOutcome, SessionRequest, SessionStep, }; use crate::assembly::analysis_session::types::{Pairs, Shapes}; use crate::semantic_validation::models::ProjectCatalog; @@ -63,6 +63,7 @@ impl AnalysisSession { available_nullability: ShapeTable::from_shapes(&request.column_nullability), complete_shapes, outcomes: vec![None; request.models.len()], + pending_cleaned: vec![false; request.models.len()], waves: if dependency_ordered || request.models.is_empty() { waves } else { @@ -112,13 +113,26 @@ impl AnalysisSession { } let dynamic_contracts: Vec = self.dynamic_contracts()?; let pool: Result, String> = self.catalog.native.analysis_pool(); - let models: Vec = self + let mut models: Vec = self .outcomes .into_iter() .collect::>() .ok_or("the session left a model unanalysed")?; let (schema_additions, analysis_names) = self.catalog.into_changes(); - let request: SessionRequest = self.request; + let mut request: SessionRequest = self.request; + for ((outcome, model), pending) in models + .iter_mut() + .zip(request.models.iter_mut()) + .zip(self.pending_cleaned) + { + if pending { + outcome.deferred_cleaning = Some(DeferredCleaning { + dialect: request.dialect.clone(), + query_sql: std::mem::take(&mut model.query_sql), + placeholders: std::mem::take(&mut model.placeholders), + }); + } + } let mut model_facts: HashMap = HashMap::new(); let mut dynamic_proofs: HashMap = HashMap::new(); for ((model, outcome), proof) in request.models.iter().zip(&models).zip(&dynamic_contracts) @@ -247,6 +261,7 @@ impl AnalysisSession { self.outcomes[*model] = Some(ModelOutcome { analysis, cleaned_sql: result.cleaned_sql, + deferred_cleaning: None, validated_schema: schema, fused_binding_validated: true, }); @@ -394,8 +409,8 @@ impl AnalysisSession { /// Python's deferred binding validation, then the dataflow's shape publication. fn finish_wave(&mut self, wave: usize) -> Result<(), String> { let models: Vec = self.waves[wave].clone(); - let mut validations: Vec<(usize, String, Shapes)> = Vec::new(); let validated: &[usize] = if self.cyclic { &[] } else { &models }; + let mut revalidated: Vec<(usize, Shapes)> = Vec::new(); for model in validated { let schema: Shapes = self.binding_schema(&self.request.models[*model]); let outcome: &ModelOutcome = self.outcomes[*model] @@ -404,9 +419,18 @@ impl AnalysisSession { if !outcome.fused_binding_validated || !same_relations(&outcome.validated_schema, &schema) { - validations.push((*model, outcome.cleaned_sql.clone(), schema)); + revalidated.push((*model, schema)); } } + let pending: Vec = revalidated.iter().map(|(model, _)| *model).collect(); + self.ensure_cleaned(&pending)?; + let mut validations: Vec<(usize, String, Shapes)> = Vec::new(); + for (model, schema) in revalidated { + let outcome: &ModelOutcome = self.outcomes[model] + .as_ref() + .ok_or("a wave model was not analysed")?; + validations.push((model, outcome.cleaned_sql.clone(), schema)); + } if !validations.is_empty() { let requests: Vec<(&str, &Shapes)> = validations .iter() @@ -421,6 +445,9 @@ impl AnalysisSession { } } if self.dependency_ordered { + if self.request.case_sensitive_shapes { + self.ensure_cleaned(&models)?; + } for model in &models { self.publish(*model)?; } @@ -500,26 +527,11 @@ impl AnalysisSession { None => misses.push(position), } } - let members: Vec> = hits - .iter() - .map(|(position, _)| { - batch_member(&self.request.models[models[*position]], &schemas[*position]) - }) - .collect(); - let cleaned: Vec> = self - .catalog - .normalize_each(&self.request.dialect, &members) - .unwrap_or_else(|error| members.iter().map(|_| Err(error.clone())).collect()); - for ((position, mut outcome), sql) in hits.into_iter().zip(cleaned) { + for (position, outcome) in hits { let model: usize = models[position]; - match sql { - Ok(sql) => { - outcome.cleaned_sql = sql; - self.outcomes[model] = Some(outcome); - cache.hits[model] = true; - } - Err(_) => misses.push(position), - } + self.outcomes[model] = Some(outcome); + self.pending_cleaned[model] = true; + cache.hits[model] = true; } misses.sort_unstable(); cache.stats.hits += models.len() - misses.len(); @@ -528,6 +540,38 @@ impl AnalysisSession { Ok(misses) } + /// Normalize the cleaned SQL of the cache hits among `models` that still lack it. + fn ensure_cleaned(&mut self, models: &[usize]) -> Result<(), String> { + let pending: Vec = models + .iter() + .copied() + .filter(|model| self.pending_cleaned[*model]) + .collect(); + if pending.is_empty() { + return Ok(()); + } + let schemas: Vec = pending + .iter() + .map(|model| self.binding_schema(&self.request.models[*model])) + .collect(); + let members: Vec> = pending + .iter() + .zip(&schemas) + .map(|(model, schema)| batch_member(&self.request.models[*model], schema)) + .collect(); + let cleaned: Vec> = self + .catalog + .normalize_each(&self.request.dialect, &members)?; + for (model, sql) in pending.into_iter().zip(cleaned) { + let sql: String = sql.map_err(|reason| { + native_failure(&model_context(&self.request.models[model]), &reason) + })?; + self.outcome_mut(model)?.cleaned_sql = sql; + self.pending_cleaned[model] = false; + } + Ok(()) + } + /// Store each finished native outcome of `models` the cache did not answer. fn store_cached(&mut self, models: &[usize]) { let Some(cache) = self.cache.as_mut() else { diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/main/mod.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/main/mod.rs index f04bab938b..6fb1e98127 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/main/mod.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/main/mod.rs @@ -10,6 +10,7 @@ pub mod prove_dynamic_contracts; pub mod prove_finished_dynamic_contracts; pub mod query_columns; pub mod recover_cte_facts; +pub mod resolved_cleaned_sql; pub mod run_analysis_session; pub mod session_sharing; pub mod sql_test_helper_reads; diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/main/resolved_cleaned_sql.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/main/resolved_cleaned_sql.rs new file mode 100644 index 0000000000..58bc0c31ff --- /dev/null +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/main/resolved_cleaned_sql.rs @@ -0,0 +1,29 @@ +//! A finished model outcome's cleaned analysis SQL, normalized on first read for cache hits. + +use std::borrow::Cow; +use std::collections::HashMap; + +use sqlbuild_core::panics::main::catch_compiler_panic::catch_compiler_panic; + +use crate::assembly::analysis_session::models::{DeferredCleaning, ModelOutcome}; +use crate::semantic_validation::main::normalize::normalize_analysis_sql; +use crate::semantic_validation::models::NormalizationInput; + +/// The outcome's cleaned SQL, normalizing a cache hit's deferred input when it has one. +pub fn resolved_cleaned_sql(outcome: &ModelOutcome) -> Result, String> { + let Some(DeferredCleaning { + dialect, + query_sql, + placeholders, + }) = outcome.deferred_cleaning.as_ref() + else { + return Ok(Cow::Borrowed(&outcome.cleaned_sql)); + }; + let request: NormalizationInput = NormalizationInput { + sql: query_sql.clone(), + dialect: dialect.clone(), + stubs: HashMap::new(), + placeholders: placeholders.iter().cloned().collect(), + }; + catch_compiler_panic(|| normalize_analysis_sql(request)).map(Cow::Owned) +} diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/models.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/models.rs index a99190b5d9..c5a18ee5d3 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/models.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/models.rs @@ -213,11 +213,21 @@ pub struct ModelAnalysis { pub binding_validated: bool, } +/// The inputs a cache hit normalizes into its cleaned SQL only when a reader needs it. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DeferredCleaning { + pub(crate) dialect: String, + pub(crate) query_sql: String, + pub(crate) placeholders: Pairs, +} + /// Python's `ModelSqlAnalysis` for one model, without the placeholders Python keeps. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ModelOutcome { pub analysis: ModelAnalysis, + /// Empty while `deferred_cleaning` is set; read through `resolved_cleaned_sql`. pub cleaned_sql: String, + pub deferred_cleaning: Option, pub validated_schema: Shapes, pub fused_binding_validated: bool, } @@ -307,6 +317,8 @@ pub struct AnalysisSession { pub(crate) available_nullability: ShapeTable, pub(crate) complete_shapes: ShapeTable, pub(crate) outcomes: Vec>, + /// Cache hits whose cleaned SQL is still unnormalized. + pub(crate) pending_cleaned: Vec, pub(crate) phase: Phase, pub(crate) publications: Shapes, pub(crate) failures: Vec, diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_rename.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_rename.rs index 4d8ebbbbf9..c776859eca 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_rename.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_rename.rs @@ -83,11 +83,17 @@ pub(crate) fn rename_ctes( preserves_tokens((sql, &tokens), (&renamed, &renamed_tokens), &replaced).then_some(renamed) } -/// Case-folded names of every CTE a statement defines at any depth; `None` when unscannable. -pub(crate) fn defined_cte_keys(sql: &str, dialect: SliceDialect) -> Option> { - let tokens = match tokens(sql, dialect) { - Ok(tokens) => tokens, - Err(_) => return None, +/// Case-folded names of every CTE a statement defines at any depth; empty without `with`. +pub(crate) fn defined_cte_keys(sql: &str, dialect: SliceDialect) -> Vec { + if !sql + .as_bytes() + .windows(4) + .any(|window| window.eq_ignore_ascii_case(b"with")) + { + return Vec::new(); + } + let Ok(tokens) = tokens(sql, dialect) else { + return Vec::new(); }; let code: Vec<&Token> = tokens.iter().filter(|token| !token.comment).collect(); let text = |index: usize| code.get(index).map(|token| &sql[token.start..token.end]); @@ -138,7 +144,7 @@ pub(crate) fn defined_cte_keys(sql: &str, dialect: SliceDialect) -> Option, ) -> Result<(), String> { for (label, sql) in readers { - for key in defined_cte_keys(sql, request.slice_dialect).unwrap_or_default() { + for key in defined_cte_keys(sql, request.slice_dialect) { if let Some(index) = self.helper_indexes.get(&key) { return Err(compile_error(&format!( "SQL test '{}' defines CTE '{}' inside CTE '{label}', which redefines \ diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs index dd807f1445..272ae72a03 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs @@ -1,7 +1,7 @@ //! Coarse project-level planning for SQL-native test artifacts. use std::collections::{BTreeMap, HashMap, HashSet}; -use std::sync::{Arc, Mutex, OnceLock}; +use std::sync::{Arc, Mutex, OnceLock, PoisonError}; use std::time::Instant; use polyglot_sql::{Dialect, Expression}; @@ -91,6 +91,8 @@ struct ProjectContext { patterns: SqlTestPatterns, render_dialect: Arc, rejects_nested_with: bool, + /// Each model query's reserved CTE name, since every test reading the model checks it. + reserved_ctes: Arc>>>, } #[derive(Clone)] @@ -542,6 +544,7 @@ pub(crate) fn plan_batch(request: SqlTestPlanBatch) -> Result Option { + let cached: Option> = self + .reserved_ctes + .lock() + .unwrap_or_else(PoisonError::into_inner) + .get(query_sql) + .cloned(); + if let Some(name) = cached { + return name; + } + let name: Option = + reserved_cte_name(query_sql, SliceDialect::new(Some(&self.dialect))); + self.reserved_ctes + .lock() + .unwrap_or_else(PoisonError::into_inner) + .insert(query_sql.to_owned(), name.clone()); + name + } +} + /// The first CTE a model defines under a name reserved for generated test-query CTEs. fn reserved_cte_name(query_sql: &str, dialect: SliceDialect) -> Option { let reserved = MOCK_CTE_PREFIXES.iter().copied().chain([ @@ -705,7 +729,6 @@ fn reserved_cte_name(query_sql: &str, dialect: SliceDialect) -> Option { ]); let prefixes: Vec<&str> = reserved.collect(); defined_cte_keys(query_sql, dialect) - .unwrap_or_default() .into_iter() .find(|key| is_reserved_cte_key(key, &prefixes)) } @@ -769,8 +792,7 @@ fn plan_model_test( .model_query_overrides .get(model_name) .unwrap_or(&model.query_sql); - if let Some(name) = reserved_cte_name(query_sql, SliceDialect::new(Some(&context.dialect))) - { + if let Some(name) = context.reserved_cte_name(query_sql) { return Err(compile_error(&format!( "SQL test '{}' reads model '{model_name}', which defines CTE '{name}'; names \ starting with a SQLBuild test prefix such as __helper__ or __ref__ are reserved \ diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/rendering.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/rendering.rs index efc1b7d5f2..ff2599b2b3 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/rendering.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/rendering.rs @@ -449,7 +449,7 @@ impl RenderCteState { .collect(); let keys: HashSet = top_level.iter().map(|(name, _, _)| cte_key(name)).collect(); for (_, body, origin) in &top_level { - let nested = defined_cte_keys(body, self.dialect).unwrap_or_default(); + let nested = defined_cte_keys(body, self.dialect); if let Some(name) = nested.iter().find(|name| keys.contains(*name)) { return Err(format!( "{origin} defines CTE '{name}', which is also a CTE SQLBuild generates at the \ diff --git a/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs b/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs index 99ffa40a63..90616732f5 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs @@ -1793,7 +1793,7 @@ fn assert_names_isolated(sql: &str, dialect: &str, helper_name: &str, label: &st }); assert!(!helper_at_top_level, "{label}: {sql}"); for cte in &split.ctes { - let nested = defined_cte_keys(cte.body, slice_dialect).expect("body scans"); + let nested = defined_cte_keys(cte.body, slice_dialect); assert!( nested.iter().all(|name| !top_level.contains(name)), "{label}: {} nests {nested:?}: {sql}", diff --git a/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_cte_rename.rs b/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_cte_rename.rs index 66413678b6..73ed018515 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_cte_rename.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_cte_rename.rs @@ -135,10 +135,25 @@ fn given_statement_when_listing_defined_ctes_then_only_with_list_entries_count() ON b.id = s.id", expected_keys: &["base", "rows_two"], }, + DefinedCteKeysTestCase { + description: "lowercase and mixed-case WITH keywords are listed", + sql: "with base as (select 1 as id), Rows_Two as (WiTh deep as (select 2 as id) \ + select id from deep) select id from base", + expected_keys: &["base", "rows_two", "deep"], + }, + DefinedCteKeysTestCase { + description: "a statement without the WITH keyword defines nothing", + sql: "SELECT width, within_limit FROM orders WHERE status = 'shipped'", + expected_keys: &[], + }, + DefinedCteKeysTestCase { + description: "an unscannable statement defines nothing", + sql: "WITH base AS (SELECT 'unterminated FROM orders", + expected_keys: &[], + }, ]; for test_case in test_cases { - let keys = - defined_cte_keys(test_case.sql, SliceDialect::new(Some("duckdb"))).expect("scans"); + let keys = defined_cte_keys(test_case.sql, SliceDialect::new(Some("duckdb"))); assert_eq!(keys, test_case.expected_keys, "{}", test_case.description); } } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/analysis_session/session.rs b/crates/sqlbuild-python/src/bindings/_helpers/analysis_session/session.rs index b871dacc26..d7cbfd8d38 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/analysis_session/session.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/analysis_session/session.rs @@ -11,6 +11,7 @@ use sqlbuild_analysis::assembly::analysis_session::main::model_reference_facts:: use sqlbuild_analysis::assembly::analysis_session::main::names_set_operation::names_set_operation; use sqlbuild_analysis::assembly::analysis_session::main::prove_dynamic_contracts::prove_dynamic_contracts; use sqlbuild_analysis::assembly::analysis_session::main::prove_finished_dynamic_contracts::prove_finished_dynamic_contracts; +use sqlbuild_analysis::assembly::analysis_session::main::resolved_cleaned_sql::resolved_cleaned_sql; use sqlbuild_analysis::assembly::analysis_session::main::run_analysis_session::run_analysis_session; use sqlbuild_analysis::assembly::analysis_session::main::session_sharing::session_sharing; use sqlbuild_analysis::assembly::analysis_session::main::start_analysis_session::start_analysis_session; @@ -224,7 +225,13 @@ impl NativeModelAnalysisSession { /// The model at `index`'s outcome and proof rows, for hand-built model inputs only. fn outcome_rows(&self, index: usize) -> PyResult<(OutcomeRow, ContractRow)> { let (outcome, contract) = self.outcome(index)?; - Ok((outcome_row(outcome.clone()), contract_row(contract.clone()))) + let cleaned: String = resolved_cleaned_sql(outcome) + .map_err(compiler_error)? + .into_owned(); + Ok(( + outcome_row(outcome.clone(), cleaned), + contract_row(contract.clone()), + )) } /// The analysis cache's `(hits, misses, stored, failure)`; None without a cache. @@ -452,7 +459,7 @@ fn contract_row(outcome: PivotOutcome) -> ContractRow { } } -fn outcome_row(outcome: ModelOutcome) -> OutcomeRow { +fn outcome_row(outcome: ModelOutcome, cleaned_sql: String) -> OutcomeRow { let analysis = outcome.analysis; let (source, lineage) = match analysis.lineage { LineageFacts::Native(rows) => ("native", lineage_items(rows)), @@ -469,7 +476,7 @@ fn outcome_row(outcome: ModelOutcome) -> OutcomeRow { analysis.star_resolved, analysis.binding_diagnostics, analysis.binding_validated, - outcome.cleaned_sql, + cleaned_sql, ) } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/compiled_model_analysis.rs b/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/compiled_model_analysis.rs index a14461c56c..dfd411e73c 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/compiled_model_analysis.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/compiled_model_analysis.rs @@ -1,5 +1,6 @@ //! A compiled model's analysis, kept from the native session and projected on demand. +use std::borrow::Cow; use std::collections::HashMap; use pyo3::IntoPyObject; @@ -7,14 +8,16 @@ use pyo3::exceptions::PyValueError; use pyo3::prelude::{Bound, Py, PyAny, PyAnyMethods, PyResult, Python}; use pyo3::sync::PyOnceLock; use pyo3::types::{PyDict, PyDictMethods, PyStringMethods, PyTuple}; +use sqlbuild_analysis::assembly::analysis_session::main::resolved_cleaned_sql::resolved_cleaned_sql; use sqlbuild_analysis::assembly::analysis_session::models::{ ColumnFact, ContractProof, LineageFacts, LineageRow, ModelOutcome, PivotOutcome, }; use sqlbuild_analysis::binding_locations::main::locate_binding_diagnostics::locate_binding_diagnostics; use sqlbuild_analysis::binding_locations::models::{BindingSource, LocatedDiagnostic}; -use sqlbuild_analysis::semantic_validation::types::Expansion; +use sqlbuild_analysis::semantic_validation::types::{DiagnosticRow, Expansion}; use crate::bindings::_helpers::analysis_session::session::NativeModelAnalysisSession; +use crate::bindings::_helpers::boundary::panics::compiler_error; use crate::bindings::_helpers::compiled_project::compiled_model_view::{ CompiledViewClasses, InferredRow, }; @@ -128,12 +131,7 @@ pub(crate) fn analysis_record( if let PivotOutcome::Proof(proof) = contract { record.proof = Some(proof.clone()); } - record.binding = located( - py, - input, - &analysed.binding_diagnostics, - &outcome.cleaned_sql, - )?; + record.binding = located(py, input, outcome)?; record.binding_validated = analysed.binding_validated; } if let Some(proof) = record.proof.as_ref().filter(|proof| proof.output_proven) { @@ -147,12 +145,13 @@ pub(crate) fn analysis_record( fn located( py: Python<'_>, input: &ModelInputRecord, - rows: &[sqlbuild_analysis::semantic_validation::types::DiagnosticRow], - cleaned: &str, + outcome: &ModelOutcome, ) -> PyResult> { + let rows: &[DiagnosticRow] = &outcome.analysis.binding_diagnostics; if rows.is_empty() { return Ok(Vec::new()); } + let cleaned: Cow<'_, str> = resolved_cleaned_sql(outcome).map_err(compiler_error)?; let model_file: &Bound<'_, PyAny> = input.model_file.bind(py); let authored: String = model_file.getattr("contents")?.extract()?; let query: String = model_file.getattr("query_sql")?.extract()?; @@ -164,7 +163,7 @@ fn located( let source: BindingSource<'_> = BindingSource { authored: &authored, query: &query, - cleaned, + cleaned: &cleaned, expansion: expansion .authored .then_some((expansion.expanded_sql.bind(py).to_str()?, passes.as_slice())), diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py index 44c7815adb..da417a095e 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py @@ -467,6 +467,13 @@ class TypeFindingLocationCase: expected_diagnostics: tuple[tuple[str, str, int, int], ...] +@dataclass(frozen=True) +class CachedBindingDiagnosticCase: + description: str + query_sql: str + expected_location: tuple[str, int, int] + + @dataclass(frozen=True) class RequireSqlAnalysisCase: description: str diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/helpers.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/helpers.py index 6ff09b1f7a..a19f4d1c98 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/helpers.py @@ -4989,3 +4989,14 @@ def write_counted_error_project(*, project_dir: Path, files: dict[str, str]) -> shutil.rmtree(project_dir, ignore_errors=True) for relative_path, contents in {**FAILURE_BASE_FILES, **files}.items(): write_project_file(project_dir, relative_path, contents) + + +def compile_with_binding_error(project_dir: Path) -> dict[str, Any]: + """Compile with the analysis cache and return the JSON payload of the expected failure.""" + + result: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("--no-color", "compile", "--json") + ) + assert result.returncode == 1, result.stdout + result.stderr + payload: dict[str, Any] = json.loads(result.stdout) + return payload diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_cache_binding_diagnostics.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_cache_binding_diagnostics.py new file mode 100644 index 0000000000..2006bc3461 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_cache_binding_diagnostics.py @@ -0,0 +1,69 @@ +"""E2E coverage of binding diagnostics served from the compile analysis cache.""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.compile._test_types import ( + CachedBindingDiagnosticCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.compile.helpers import compile_with_binding_error +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project + + +@pytest.mark.parametrize( + "test_case", + ( + CachedBindingDiagnosticCase( + description="unknown column before the reference placeholder", + query_sql=( + 'SELECT\n o.order_id,\n o.quantiti AS quantity\nFROM __ref("orders") AS o\n' + ), + expected_location=("B002", 5, 5), + ), + CachedBindingDiagnosticCase( + description="unknown column after the reference placeholder", + query_sql=('SELECT\n o.order_id\nFROM __ref("orders") AS o\nWHERE o.quantiti > 1\n'), + expected_location=("B002", 6, 9), + ), + ), + ids=lambda case: case.description, +) +def test_given_cached_model_with_binding_diagnostic_when_recompiling_then_diagnostic_is_identical( + test_case: CachedBindingDiagnosticCase, tmp_path: Path +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="orders", + repo_files={ + "sqlbuild_project.toml": 'name = "orders"\nadapter = "duckdb"\n', + "models/orders.sql": ( + 'MODEL (description "Orders");\n\nSELECT 1 AS order_id, 10 AS quantity\n' + ), + "models/order_quantities.sql": ( + 'MODEL (description "Order quantities");\n\n' + test_case.query_sql + ), + }, + ) + + cold: dict[str, Any] = compile_with_binding_error(project_dir) + # An upstream comment defeats whole-project reuse; the diagnosed model's analysis stays cached. + upstream: Path = project_dir / "models" / "orders.sql" + upstream.write_text( + upstream.read_text(encoding="utf-8") + "\n-- upstream comment\n", encoding="utf-8" + ) + warm: dict[str, Any] = compile_with_binding_error(project_dir) + + assert warm["compile_timings"]["analysis_entry_cache_hits"] == 1 + assert warm["diagnostics"] == cold["diagnostics"] + code, line, column = test_case.expected_location + assert [(row["code"], row["line"], row["column"]) for row in warm["diagnostics"]] == [ + (code, line, column) + ] + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"])