Skip to content

Commit 3ffcae2

Browse files
authored
perf: skip redundant SQL normalisation and CTE scans during compile (#1088)
* perf: skip redundant SQL normalisation and CTE scans during compile * fix: resolve deferred cleaned SQL for Python readers of cached outcomes * refactor: normalise deferred SQL through the semantic validation entry point
1 parent 714e6f2 commit 3ffcae2

16 files changed

Lines changed: 277 additions & 54 deletions

File tree

‎crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/analysis_cache.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,7 @@ pub(crate) fn decode_outcome(
552552
binding_validated,
553553
},
554554
cleaned_sql: String::new(),
555+
deferred_cleaning: None,
555556
validated_schema: schema.clone(),
556557
fused_binding_validated: true,
557558
};

‎crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/session.rs‎

Lines changed: 70 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@ use crate::assembly::analysis_session::_helpers::mappings::{
3131
use crate::assembly::analysis_session::_helpers::mappings::{producers, waves};
3232
use crate::assembly::analysis_session::constants::{UNKNOWN_NULLABILITY, UNKNOWN_TYPE};
3333
use crate::assembly::analysis_session::models::{
34-
AnalysisSession, ColumnFact, ContractProof, FinishedSession, LineageFacts, LineageRow,
35-
ModelAnalysis, ModelOutcome, ModelRequest, Phase, PivotOutcome, PivotTables, SessionModelFacts,
36-
SessionOutcome, SessionRequest, SessionStep,
34+
AnalysisSession, ColumnFact, ContractProof, DeferredCleaning, FinishedSession, LineageFacts,
35+
LineageRow, ModelAnalysis, ModelOutcome, ModelRequest, Phase, PivotOutcome, PivotTables,
36+
SessionModelFacts, SessionOutcome, SessionRequest, SessionStep,
3737
};
3838
use crate::assembly::analysis_session::types::{Pairs, Shapes};
3939
use crate::semantic_validation::models::ProjectCatalog;
@@ -63,6 +63,7 @@ impl AnalysisSession {
6363
available_nullability: ShapeTable::from_shapes(&request.column_nullability),
6464
complete_shapes,
6565
outcomes: vec![None; request.models.len()],
66+
pending_cleaned: vec![false; request.models.len()],
6667
waves: if dependency_ordered || request.models.is_empty() {
6768
waves
6869
} else {
@@ -112,13 +113,26 @@ impl AnalysisSession {
112113
}
113114
let dynamic_contracts: Vec<PivotOutcome> = self.dynamic_contracts()?;
114115
let pool: Result<Arc<ThreadPool>, String> = self.catalog.native.analysis_pool();
115-
let models: Vec<ModelOutcome> = self
116+
let mut models: Vec<ModelOutcome> = self
116117
.outcomes
117118
.into_iter()
118119
.collect::<Option<_>>()
119120
.ok_or("the session left a model unanalysed")?;
120121
let (schema_additions, analysis_names) = self.catalog.into_changes();
121-
let request: SessionRequest = self.request;
122+
let mut request: SessionRequest = self.request;
123+
for ((outcome, model), pending) in models
124+
.iter_mut()
125+
.zip(request.models.iter_mut())
126+
.zip(self.pending_cleaned)
127+
{
128+
if pending {
129+
outcome.deferred_cleaning = Some(DeferredCleaning {
130+
dialect: request.dialect.clone(),
131+
query_sql: std::mem::take(&mut model.query_sql),
132+
placeholders: std::mem::take(&mut model.placeholders),
133+
});
134+
}
135+
}
122136
let mut model_facts: HashMap<String, SessionModelFacts> = HashMap::new();
123137
let mut dynamic_proofs: HashMap<String, ContractProof> = HashMap::new();
124138
for ((model, outcome), proof) in request.models.iter().zip(&models).zip(&dynamic_contracts)
@@ -247,6 +261,7 @@ impl AnalysisSession {
247261
self.outcomes[*model] = Some(ModelOutcome {
248262
analysis,
249263
cleaned_sql: result.cleaned_sql,
264+
deferred_cleaning: None,
250265
validated_schema: schema,
251266
fused_binding_validated: true,
252267
});
@@ -394,8 +409,8 @@ impl AnalysisSession {
394409
/// Python's deferred binding validation, then the dataflow's shape publication.
395410
fn finish_wave(&mut self, wave: usize) -> Result<(), String> {
396411
let models: Vec<usize> = self.waves[wave].clone();
397-
let mut validations: Vec<(usize, String, Shapes)> = Vec::new();
398412
let validated: &[usize] = if self.cyclic { &[] } else { &models };
413+
let mut revalidated: Vec<(usize, Shapes)> = Vec::new();
399414
for model in validated {
400415
let schema: Shapes = self.binding_schema(&self.request.models[*model]);
401416
let outcome: &ModelOutcome = self.outcomes[*model]
@@ -404,9 +419,18 @@ impl AnalysisSession {
404419
if !outcome.fused_binding_validated
405420
|| !same_relations(&outcome.validated_schema, &schema)
406421
{
407-
validations.push((*model, outcome.cleaned_sql.clone(), schema));
422+
revalidated.push((*model, schema));
408423
}
409424
}
425+
let pending: Vec<usize> = revalidated.iter().map(|(model, _)| *model).collect();
426+
self.ensure_cleaned(&pending)?;
427+
let mut validations: Vec<(usize, String, Shapes)> = Vec::new();
428+
for (model, schema) in revalidated {
429+
let outcome: &ModelOutcome = self.outcomes[model]
430+
.as_ref()
431+
.ok_or("a wave model was not analysed")?;
432+
validations.push((model, outcome.cleaned_sql.clone(), schema));
433+
}
410434
if !validations.is_empty() {
411435
let requests: Vec<(&str, &Shapes)> = validations
412436
.iter()
@@ -421,6 +445,9 @@ impl AnalysisSession {
421445
}
422446
}
423447
if self.dependency_ordered {
448+
if self.request.case_sensitive_shapes {
449+
self.ensure_cleaned(&models)?;
450+
}
424451
for model in &models {
425452
self.publish(*model)?;
426453
}
@@ -500,26 +527,11 @@ impl AnalysisSession {
500527
None => misses.push(position),
501528
}
502529
}
503-
let members: Vec<BatchMember<'_>> = hits
504-
.iter()
505-
.map(|(position, _)| {
506-
batch_member(&self.request.models[models[*position]], &schemas[*position])
507-
})
508-
.collect();
509-
let cleaned: Vec<Result<String, String>> = self
510-
.catalog
511-
.normalize_each(&self.request.dialect, &members)
512-
.unwrap_or_else(|error| members.iter().map(|_| Err(error.clone())).collect());
513-
for ((position, mut outcome), sql) in hits.into_iter().zip(cleaned) {
530+
for (position, outcome) in hits {
514531
let model: usize = models[position];
515-
match sql {
516-
Ok(sql) => {
517-
outcome.cleaned_sql = sql;
518-
self.outcomes[model] = Some(outcome);
519-
cache.hits[model] = true;
520-
}
521-
Err(_) => misses.push(position),
522-
}
532+
self.outcomes[model] = Some(outcome);
533+
self.pending_cleaned[model] = true;
534+
cache.hits[model] = true;
523535
}
524536
misses.sort_unstable();
525537
cache.stats.hits += models.len() - misses.len();
@@ -528,6 +540,38 @@ impl AnalysisSession {
528540
Ok(misses)
529541
}
530542

543+
/// Normalize the cleaned SQL of the cache hits among `models` that still lack it.
544+
fn ensure_cleaned(&mut self, models: &[usize]) -> Result<(), String> {
545+
let pending: Vec<usize> = models
546+
.iter()
547+
.copied()
548+
.filter(|model| self.pending_cleaned[*model])
549+
.collect();
550+
if pending.is_empty() {
551+
return Ok(());
552+
}
553+
let schemas: Vec<Shapes> = pending
554+
.iter()
555+
.map(|model| self.binding_schema(&self.request.models[*model]))
556+
.collect();
557+
let members: Vec<BatchMember<'_>> = pending
558+
.iter()
559+
.zip(&schemas)
560+
.map(|(model, schema)| batch_member(&self.request.models[*model], schema))
561+
.collect();
562+
let cleaned: Vec<Result<String, String>> = self
563+
.catalog
564+
.normalize_each(&self.request.dialect, &members)?;
565+
for (model, sql) in pending.into_iter().zip(cleaned) {
566+
let sql: String = sql.map_err(|reason| {
567+
native_failure(&model_context(&self.request.models[model]), &reason)
568+
})?;
569+
self.outcome_mut(model)?.cleaned_sql = sql;
570+
self.pending_cleaned[model] = false;
571+
}
572+
Ok(())
573+
}
574+
531575
/// Store each finished native outcome of `models` the cache did not answer.
532576
fn store_cached(&mut self, models: &[usize]) {
533577
let Some(cache) = self.cache.as_mut() else {

‎crates/sqlbuild-analysis/src/assembly/analysis_session/main/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ pub mod prove_dynamic_contracts;
1010
pub mod prove_finished_dynamic_contracts;
1111
pub mod query_columns;
1212
pub mod recover_cte_facts;
13+
pub mod resolved_cleaned_sql;
1314
pub mod run_analysis_session;
1415
pub mod session_sharing;
1516
pub mod sql_test_helper_reads;
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
//! A finished model outcome's cleaned analysis SQL, normalized on first read for cache hits.
2+
3+
use std::borrow::Cow;
4+
use std::collections::HashMap;
5+
6+
use sqlbuild_core::panics::main::catch_compiler_panic::catch_compiler_panic;
7+
8+
use crate::assembly::analysis_session::models::{DeferredCleaning, ModelOutcome};
9+
use crate::semantic_validation::main::normalize::normalize_analysis_sql;
10+
use crate::semantic_validation::models::NormalizationInput;
11+
12+
/// The outcome's cleaned SQL, normalizing a cache hit's deferred input when it has one.
13+
pub fn resolved_cleaned_sql(outcome: &ModelOutcome) -> Result<Cow<'_, str>, String> {
14+
let Some(DeferredCleaning {
15+
dialect,
16+
query_sql,
17+
placeholders,
18+
}) = outcome.deferred_cleaning.as_ref()
19+
else {
20+
return Ok(Cow::Borrowed(&outcome.cleaned_sql));
21+
};
22+
let request: NormalizationInput = NormalizationInput {
23+
sql: query_sql.clone(),
24+
dialect: dialect.clone(),
25+
stubs: HashMap::new(),
26+
placeholders: placeholders.iter().cloned().collect(),
27+
};
28+
catch_compiler_panic(|| normalize_analysis_sql(request)).map(Cow::Owned)
29+
}

‎crates/sqlbuild-analysis/src/assembly/analysis_session/models.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,11 +213,21 @@ pub struct ModelAnalysis {
213213
pub binding_validated: bool,
214214
}
215215

216+
/// The inputs a cache hit normalizes into its cleaned SQL only when a reader needs it.
217+
#[derive(Debug, Clone, PartialEq, Eq)]
218+
pub struct DeferredCleaning {
219+
pub(crate) dialect: String,
220+
pub(crate) query_sql: String,
221+
pub(crate) placeholders: Pairs,
222+
}
223+
216224
/// Python's `ModelSqlAnalysis` for one model, without the placeholders Python keeps.
217225
#[derive(Debug, Clone, PartialEq, Eq)]
218226
pub struct ModelOutcome {
219227
pub analysis: ModelAnalysis,
228+
/// Empty while `deferred_cleaning` is set; read through `resolved_cleaned_sql`.
220229
pub cleaned_sql: String,
230+
pub deferred_cleaning: Option<DeferredCleaning>,
221231
pub validated_schema: Shapes,
222232
pub fused_binding_validated: bool,
223233
}
@@ -307,6 +317,8 @@ pub struct AnalysisSession {
307317
pub(crate) available_nullability: ShapeTable,
308318
pub(crate) complete_shapes: ShapeTable,
309319
pub(crate) outcomes: Vec<Option<ModelOutcome>>,
320+
/// Cache hits whose cleaned SQL is still unnormalized.
321+
pub(crate) pending_cleaned: Vec<bool>,
310322
pub(crate) phase: Phase,
311323
pub(crate) publications: Shapes,
312324
pub(crate) failures: Vec<String>,

‎crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/cte_rename.rs‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,17 @@ pub(crate) fn rename_ctes(
8383
preserves_tokens((sql, &tokens), (&renamed, &renamed_tokens), &replaced).then_some(renamed)
8484
}
8585

86-
/// Case-folded names of every CTE a statement defines at any depth; `None` when unscannable.
87-
pub(crate) fn defined_cte_keys(sql: &str, dialect: SliceDialect) -> Option<Vec<String>> {
88-
let tokens = match tokens(sql, dialect) {
89-
Ok(tokens) => tokens,
90-
Err(_) => return None,
86+
/// Case-folded names of every CTE a statement defines at any depth; empty without `with`.
87+
pub(crate) fn defined_cte_keys(sql: &str, dialect: SliceDialect) -> Vec<String> {
88+
if !sql
89+
.as_bytes()
90+
.windows(4)
91+
.any(|window| window.eq_ignore_ascii_case(b"with"))
92+
{
93+
return Vec::new();
94+
}
95+
let Ok(tokens) = tokens(sql, dialect) else {
96+
return Vec::new();
9197
};
9298
let code: Vec<&Token> = tokens.iter().filter(|token| !token.comment).collect();
9399
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<Vec<S
138144
keys.push(key.to_string());
139145
}
140146
}
141-
Some(keys)
147+
keys
142148
}
143149

144150
/// Whether each token sits directly in a WITH list, so a comma there separates CTEs.

‎crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/helper_scope.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ impl ScopeGraph {
228228
request: &ScopeRequest<'_>,
229229
) -> Result<(), String> {
230230
for (label, sql) in readers {
231-
for key in defined_cte_keys(sql, request.slice_dialect).unwrap_or_default() {
231+
for key in defined_cte_keys(sql, request.slice_dialect) {
232232
if let Some(index) = self.helper_indexes.get(&key) {
233233
return Err(compile_error(&format!(
234234
"SQL test '{}' defines CTE '{}' inside CTE '{label}', which redefines \

‎crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! Coarse project-level planning for SQL-native test artifacts.
22
33
use std::collections::{BTreeMap, HashMap, HashSet};
4-
use std::sync::{Arc, Mutex, OnceLock};
4+
use std::sync::{Arc, Mutex, OnceLock, PoisonError};
55
use std::time::Instant;
66

77
use polyglot_sql::{Dialect, Expression};
@@ -91,6 +91,8 @@ struct ProjectContext {
9191
patterns: SqlTestPatterns,
9292
render_dialect: Arc<Dialect>,
9393
rejects_nested_with: bool,
94+
/// Each model query's reserved CTE name, since every test reading the model checks it.
95+
reserved_ctes: Arc<Mutex<HashMap<String, Option<String>>>>,
9496
}
9597

9698
#[derive(Clone)]
@@ -542,6 +544,7 @@ pub(crate) fn plan_batch(request: SqlTestPlanBatch) -> Result<SqlTestPlanBatchOu
542544
patterns: SqlTestPatterns::new(request.lexical_syntax)?,
543545
render_dialect,
544546
rejects_nested_with,
547+
reserved_ctes: Arc::new(Mutex::new(HashMap::new())),
545548
};
546549
let workers = request.workers.clamp(1, MAX_WORKERS);
547550
let pool = rayon::ThreadPoolBuilder::new()
@@ -695,6 +698,27 @@ fn plan_direct_test(
695698
})
696699
}
697700

701+
impl ProjectContext {
702+
fn reserved_cte_name(&self, query_sql: &str) -> Option<String> {
703+
let cached: Option<Option<String>> = self
704+
.reserved_ctes
705+
.lock()
706+
.unwrap_or_else(PoisonError::into_inner)
707+
.get(query_sql)
708+
.cloned();
709+
if let Some(name) = cached {
710+
return name;
711+
}
712+
let name: Option<String> =
713+
reserved_cte_name(query_sql, SliceDialect::new(Some(&self.dialect)));
714+
self.reserved_ctes
715+
.lock()
716+
.unwrap_or_else(PoisonError::into_inner)
717+
.insert(query_sql.to_owned(), name.clone());
718+
name
719+
}
720+
}
721+
698722
/// The first CTE a model defines under a name reserved for generated test-query CTEs.
699723
fn reserved_cte_name(query_sql: &str, dialect: SliceDialect) -> Option<String> {
700724
let reserved = MOCK_CTE_PREFIXES.iter().copied().chain([
@@ -705,7 +729,6 @@ fn reserved_cte_name(query_sql: &str, dialect: SliceDialect) -> Option<String> {
705729
]);
706730
let prefixes: Vec<&str> = reserved.collect();
707731
defined_cte_keys(query_sql, dialect)
708-
.unwrap_or_default()
709732
.into_iter()
710733
.find(|key| is_reserved_cte_key(key, &prefixes))
711734
}
@@ -769,8 +792,7 @@ fn plan_model_test(
769792
.model_query_overrides
770793
.get(model_name)
771794
.unwrap_or(&model.query_sql);
772-
if let Some(name) = reserved_cte_name(query_sql, SliceDialect::new(Some(&context.dialect)))
773-
{
795+
if let Some(name) = context.reserved_cte_name(query_sql) {
774796
return Err(compile_error(&format!(
775797
"SQL test '{}' reads model '{model_name}', which defines CTE '{name}'; names \
776798
starting with a SQLBuild test prefix such as __helper__ or __ref__ are reserved \

‎crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/rendering.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ impl RenderCteState {
449449
.collect();
450450
let keys: HashSet<String> = top_level.iter().map(|(name, _, _)| cte_key(name)).collect();
451451
for (_, body, origin) in &top_level {
452-
let nested = defined_cte_keys(body, self.dialect).unwrap_or_default();
452+
let nested = defined_cte_keys(body, self.dialect);
453453
if let Some(name) = nested.iter().find(|name| keys.contains(*name)) {
454454
return Err(format!(
455455
"{origin} defines CTE '{name}', which is also a CTE SQLBuild generates at the \

‎crates/sqlbuild-analysis/src/compiler/tests/helpers.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1793,7 +1793,7 @@ fn assert_names_isolated(sql: &str, dialect: &str, helper_name: &str, label: &st
17931793
});
17941794
assert!(!helper_at_top_level, "{label}: {sql}");
17951795
for cte in &split.ctes {
1796-
let nested = defined_cte_keys(cte.body, slice_dialect).expect("body scans");
1796+
let nested = defined_cte_keys(cte.body, slice_dialect);
17971797
assert!(
17981798
nested.iter().all(|name| !top_level.contains(name)),
17991799
"{label}: {} nests {nested:?}: {sql}",

0 commit comments

Comments
 (0)