diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/mappings.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/mappings.rs index b3a2dcbe69..0018c8c5c0 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/mappings.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/mappings.rs @@ -1,6 +1,7 @@ //! Python dict semantics over ordered pairs, and the topological analysis waves. use std::collections::{BTreeSet, HashMap}; +use std::sync::OnceLock; use sha2::{Digest, Sha256}; @@ -80,7 +81,7 @@ pub(crate) struct ShapeTable { /// Each name at its first insertion, Python's dict iteration order. order: Vec, /// Digest chained over every insertion in order, so equal chains mean equal tables. - chain: [u8; 32], + chain: OnceLock<[u8; 32]>, } impl ShapeTable { @@ -92,13 +93,12 @@ impl ShapeTable { None => table.insert(name, shape.clone()), } } - table.chain = shapes_chain(&table.ordered()); table } /// The digest of every shape in order, which changes whenever the table does. pub(crate) fn chain(&self) -> [u8; 32] { - self.chain + *self.chain.get_or_init(|| shapes_chain(&self.ordered())) } fn insert(&mut self, name: &str, shape: Pairs) { @@ -126,7 +126,9 @@ impl ShapeTable { /// Python's `setdefault`. pub(crate) fn set_default(&mut self, name: &str, shape: Pairs) { if !self.shapes.contains_key(name) { - self.chain = chained(&self.chain, name, &shape); + if let Some(chain) = self.chain.get() { + self.chain = OnceLock::from(chained(chain, name, &shape)); + } self.insert(name, shape); } } diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/main/names_set_operation.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/main/names_set_operation.rs index 30c85fc3da..08ff3430f1 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/main/names_set_operation.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/main/names_set_operation.rs @@ -1,31 +1,18 @@ //! Python's `re.search(r"\b(?:UNION|INTERSECT|EXCEPT)\b", sql, re.IGNORECASE) is not None`. -use sqlbuild_core::text::main::ignorecase_key::ignorecase_key; use sqlbuild_core::text::main::is_word_char::is_word_char; use crate::assembly::analysis_session::constants::SET_OPERATION_KEYWORDS; -/// Whether `sql` names a set-operation keyword as a whole word, in any case. +/// Whether a whole word of `sql` is a set-operation keyword; only ASCII letters fold to its letters. pub fn names_set_operation(sql: &str) -> bool { - let characters: Vec = sql.chars().collect(); - for start in 0..characters.len() { - if start > 0 && is_word_char(characters[start - 1]) { - continue; - } - for keyword in SET_OPERATION_KEYWORDS { - let end = start + keyword.len(); - if end > characters.len() { - continue; - } - let matches = characters[start..end] - .iter() - .zip(keyword.chars()) - .all(|(character, letter)| ignorecase_key(*character) == letter); - let bounded = characters.get(end).is_none_or(|next| !is_word_char(*next)); - if matches && bounded { - return true; - } - } - } - false + sql.split(|character: char| !is_word_char(character)) + .any(is_set_operation_keyword) +} + +/// Whether `word` is a set-operation keyword, ignoring ASCII case. +fn is_set_operation_keyword(word: &str) -> bool { + SET_OPERATION_KEYWORDS + .iter() + .any(|keyword| word.eq_ignore_ascii_case(keyword)) } diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/tests.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/tests.rs index 1f58431987..b4f9db0702 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/tests.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/tests.rs @@ -12,6 +12,8 @@ mod helpers; mod mappings; #[path = "tests/test_model_columns.rs"] mod model_columns; +#[path = "tests/test_names_set_operation.rs"] +mod names_set_operation; #[path = "tests/test_native_dialect.rs"] mod native_dialect; #[path = "tests/test_reference_facts.rs"] diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/tests/test_names_set_operation.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/tests/test_names_set_operation.rs new file mode 100644 index 0000000000..913f56338d --- /dev/null +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/tests/test_names_set_operation.rs @@ -0,0 +1,95 @@ +use sqlbuild_core::text::main::ignorecase_key::ignorecase_key; + +use crate::assembly::analysis_session::main::names_set_operation::names_set_operation; +use crate::assembly::analysis_session::tests::test_types::{ + KeywordFoldingTestCase, NamesSetOperationTestCase, +}; + +#[test] +fn given_sql_when_checking_for_set_operations_then_matches_whole_words_in_any_case() { + let test_cases = [ + NamesSetOperationTestCase { + description: "a lowercase union is found", + sql: "select 1 union all select 2", + expected_set_operation: true, + }, + NamesSetOperationTestCase { + description: "a mixed-case intersect is found", + sql: "SELECT a FROM t InTeRsEcT SELECT a FROM u", + expected_set_operation: true, + }, + NamesSetOperationTestCase { + description: "a keyword at the end of the text is found", + sql: "select a from t\nexcept", + expected_set_operation: true, + }, + NamesSetOperationTestCase { + description: "a keyword inside an identifier is ignored", + sql: "select reunion_id, union_count, except_flag from t", + expected_set_operation: false, + }, + NamesSetOperationTestCase { + description: "a keyword followed by a non-ASCII letter is ignored", + sql: "select 1 as unioné", + expected_set_operation: false, + }, + NamesSetOperationTestCase { + description: "a keyword after a non-ASCII letter is ignored", + sql: "select 1 as éunion", + expected_set_operation: false, + }, + NamesSetOperationTestCase { + description: "a keyword after non-ASCII punctuation is found", + sql: "select 1 «union» select 2", + expected_set_operation: true, + }, + NamesSetOperationTestCase { + description: "text without a keyword is not a set operation", + sql: "select ü, ß from t", + expected_set_operation: false, + }, + ]; + + for test_case in test_cases { + assert_eq!( + names_set_operation(test_case.sql), + test_case.expected_set_operation, + "{}", + test_case.description + ); + } +} + +#[test] +fn given_non_ascii_characters_when_folded_then_none_matches_a_keyword_letter() { + let test_cases = [ + KeywordFoldingTestCase { + description: "no non-ASCII character folds to a letter of 'union'", + keyword: "union", + expected_non_ascii_folds: Vec::new(), + }, + KeywordFoldingTestCase { + description: "no non-ASCII character folds to a letter of 'intersect'", + keyword: "intersect", + expected_non_ascii_folds: Vec::new(), + }, + KeywordFoldingTestCase { + description: "no non-ASCII character folds to a letter of 'except'", + keyword: "except", + expected_non_ascii_folds: Vec::new(), + }, + ]; + + for test_case in test_cases { + let folds: Vec = (0x80..=0x10FFFF_u32) + .filter_map(char::from_u32) + .filter(|character| test_case.keyword.contains(ignorecase_key(*character))) + .collect(); + + assert_eq!( + folds, test_case.expected_non_ascii_folds, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/tests/test_types.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/tests/test_types.rs index 437696de9a..21e234622c 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/tests/test_types.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/tests/test_types.rs @@ -178,6 +178,18 @@ pub(crate) struct ReferenceFactsTestCase { pub(crate) expected_required: &'static [&'static str], } +pub(crate) struct NamesSetOperationTestCase { + pub(crate) description: &'static str, + pub(crate) sql: &'static str, + pub(crate) expected_set_operation: bool, +} + +pub(crate) struct KeywordFoldingTestCase { + pub(crate) description: &'static str, + pub(crate) keyword: &'static str, + pub(crate) expected_non_ascii_folds: Vec, +} + pub(crate) struct NativeDialectTestCase { pub(crate) description: &'static str, pub(crate) dialect: Option<&'static str>, diff --git a/crates/sqlbuild-analysis/src/compiled_project/_helpers/splice.rs b/crates/sqlbuild-analysis/src/compiled_project/_helpers/splice.rs index e5a79a1a33..f2a7150f21 100644 --- a/crates/sqlbuild-analysis/src/compiled_project/_helpers/splice.rs +++ b/crates/sqlbuild-analysis/src/compiled_project/_helpers/splice.rs @@ -3,13 +3,29 @@ use crate::compiled_project::models::{DeclarationSplice, SplicedSpan, SplicedSql}; pub(crate) fn splice_declarations(sql: &str, splices: &[DeclarationSplice]) -> SplicedSql { + if sql.is_ascii() { + return spliced(sql.len(), splices, |output, start, end| { + output.push_str(&sql[start..end]); + }); + } let characters: Vec = sql.chars().collect(); - let mut output: String = String::with_capacity(sql.len()); + spliced(characters.len(), splices, |output, start, end| { + output.extend(&characters[start..end]); + }) +} + +/// The splice over text of `length` characters, `copy` appending each kept character range. +fn spliced( + length: usize, + splices: &[DeclarationSplice], + copy: impl Fn(&mut String, usize, usize), +) -> SplicedSql { + let mut output: String = String::with_capacity(length); let mut spans: Vec = Vec::with_capacity(splices.len()); let mut cursor: usize = 0; let mut output_length: usize = 0; for splice in splices { - output.extend(&characters[cursor..splice.start]); + copy(&mut output, cursor, splice.start); output_length += splice.start - cursor; let replacement_length: usize = splice.replacement.chars().count(); output.push_str(&splice.replacement); @@ -22,7 +38,7 @@ pub(crate) fn splice_declarations(sql: &str, splices: &[DeclarationSplice]) -> S output_length += replacement_length; cursor = splice.end; } - output.extend(&characters[cursor..]); + copy(&mut output, cursor, length); SplicedSql { sql: output, spans } } diff --git a/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/diagnostics.rs b/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/diagnostics.rs index b2c81ed0d8..b4b251b058 100644 --- a/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/diagnostics.rs +++ b/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/diagnostics.rs @@ -2,7 +2,9 @@ use std::collections::{BTreeSet, HashMap, HashSet}; +use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; use sqlbuild_attachments::cursor_intrinsics::main::replace_intrinsics::replace_intrinsics; +use sqlbuild_core::panics::main::catch_compiler_panic::catch_compiler_panic; use crate::semantic_checks::_helpers::explanation::opt_outs::unneeded_opt_out; use crate::semantic_checks::_helpers::resource_sql::help::{HelpContext, resource_sql_help}; @@ -204,16 +206,31 @@ pub(crate) fn resource_sql_findings( request: &ResourceSqlRequest, catalog: &ProjectCatalog, ) -> Result, ResourceSqlFailure> { - let checked = resources(request); + let pool = || catalog.analysis_pool(); + let checked = resources(request, &pool).map_err(ResourceSqlFailure::Internal)?; if checked.is_empty() { return Ok(Vec::new()); } let dialect = request.dialect.as_deref(); - let mut normalized: HashMap<&str, String> = HashMap::new(); - for resource in &checked { - if !normalized.contains_key(resource.sql.as_str()) { - normalized.insert(&resource.sql, cleaned_sql(&resource.sql, dialect)?); - } + let mut seen: HashSet<&str> = HashSet::with_capacity(checked.len()); + let unique: Vec<&str> = checked + .iter() + .map(|resource| resource.sql.as_str()) + .filter(|sql| seen.insert(sql)) + .collect(); + let cleaned: Vec> = + pool().map_err(ResourceSqlFailure::Internal)?.install(|| { + unique + .par_iter() + .map(|sql| { + catch_compiler_panic(|| Ok(cleaned_sql(sql, dialect))) + .unwrap_or_else(|panic| Err(ResourceSqlFailure::Internal(panic))) + }) + .collect() + }); + let mut normalized: HashMap<&str, String> = HashMap::with_capacity(unique.len()); + for (sql, cleaned) in unique.into_iter().zip(cleaned) { + normalized.insert(sql, cleaned?); } let shapes: HashMap<&str, &[(String, String)]> = request .shapes diff --git a/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/resources.rs b/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/resources.rs index b0b8cd07c4..65c0767aec 100644 --- a/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/resources.rs +++ b/crates/sqlbuild-analysis/src/semantic_checks/_helpers/resource_sql/resources.rs @@ -3,6 +3,9 @@ use std::collections::{BTreeSet, HashMap, HashSet}; use std::sync::Arc; +use rayon::ThreadPool; +use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; + use crate::semantic_checks::_helpers::resource_sql::columns::resolve_binding_column; use crate::semantic_checks::_helpers::resource_sql::fixtures::{ prepared_test_sql, reference_names, @@ -68,23 +71,29 @@ fn audit_label(audit: &AuditSqlInput) -> String { label } -fn shape<'a>(request: &'a ResourceSqlRequest, name: &str) -> &'a [(String, String)] { - request - .shapes - .iter() - .find(|(shape_name, _)| shape_name == name) - .map_or(&[], |(_, columns)| columns.as_slice()) +/// Each closed shape's columns by relation name, the first shape of a name winning. +fn shapes_by_name(request: &ResourceSqlRequest) -> HashMap<&str, &[(String, String)]> { + let mut shapes: HashMap<&str, &[(String, String)]> = + HashMap::with_capacity(request.shapes.len()); + for (name, columns) in &request.shapes { + shapes.entry(name.as_str()).or_insert(columns.as_slice()); + } + shapes } /// Python's `_binds_cleanly`: a built-in single-column audit on a column its closed shape has. -fn binds_cleanly(audit: &AuditSqlInput, request: &ResourceSqlRequest) -> bool { +fn binds_cleanly( + audit: &AuditSqlInput, + request: &ResourceSqlRequest, + shapes: &HashMap<&str, &[(String, String)]>, +) -> bool { let (Some((_, target)), Some(column)) = (&audit.attached_target, &audit.attached_column) else { return false; }; if !COLUMN_ONLY_BUILT_IN_AUDITS.contains(&audit.definition_name.as_str()) || !audit.built_in { return false; } - let columns = shape(request, target); + let columns: &[(String, String)] = shapes.get(target.as_str()).copied().unwrap_or(&[]); !columns.is_empty() && resolve_binding_column( column, @@ -105,13 +114,14 @@ fn disabled_models(request: &ResourceSqlRequest) -> HashSet<&str> { } fn audit_resources(request: &ResourceSqlRequest, disabled: &HashSet<&str>) -> Vec { + let shapes: HashMap<&str, &[(String, String)]> = shapes_by_name(request); let mut resources: Vec = Vec::new(); for audit in &request.audits { let target_disabled = audit .attached_target .as_ref() .is_some_and(|(_, target)| disabled.contains(&target.as_str())); - if target_disabled || binds_cleanly(audit, request) { + if target_disabled || binds_cleanly(audit, request, &shapes) { continue; } for (sql, authored) in &audit.bodies { @@ -154,23 +164,43 @@ fn test_resource(test: &SqlTestSqlInput, prepared: &(String, Vec)) -> Re } } -fn sql_test_resources(request: &ResourceSqlRequest, disabled: &HashSet<&str>) -> Vec { - let mut prepared: HashMap<&str, (String, Vec)> = HashMap::new(); - let mut resources: Vec = Vec::new(); - for test in &request.sql_tests { - let expects_disabled = test - .expected_model_names - .iter() - .any(|name| disabled.contains(&name.as_str())); - if is_blank(&test.sql_body) || expects_disabled { - continue; - } - let sql = prepared - .entry(&test.sql_body) - .or_insert_with(|| prepared_test_sql(&test.sql_body)); - resources.push(test_resource(test, sql)); +/// Whether the test expects a model whose SQL analysis is disabled. +fn expects_disabled(test: &SqlTestSqlInput, disabled: &HashSet<&str>) -> bool { + test.expected_model_names + .iter() + .any(|name| disabled.contains(&name.as_str())) +} + +fn sql_test_resources( + request: &ResourceSqlRequest, + disabled: &HashSet<&str>, + pool: &dyn Fn() -> Result, String>, +) -> Result, String> { + let checked: Vec<&SqlTestSqlInput> = request + .sql_tests + .iter() + .filter(|test| !is_blank(&test.sql_body) && !expects_disabled(test, disabled)) + .collect(); + let mut seen: HashSet<&str> = HashSet::with_capacity(checked.len()); + let bodies: Vec<&str> = checked + .iter() + .map(|test| test.sql_body.as_str()) + .filter(|body| seen.insert(body)) + .collect(); + if bodies.is_empty() { + return Ok(Vec::new()); } - resources + let prepared: Vec<(String, Vec)> = pool()?.install(|| { + bodies + .par_iter() + .map(|body| prepared_test_sql(body)) + .collect() + }); + let prepared: HashMap<&str, (String, Vec)> = bodies.into_iter().zip(prepared).collect(); + Ok(checked + .into_iter() + .map(|test| test_resource(test, &prepared[test.sql_body.as_str()])) + .collect()) } fn hook_resources_of(model: &HookModelInput, request: &ResourceSqlRequest) -> Vec { @@ -210,10 +240,13 @@ fn hook_resources_of(model: &HookModelInput, request: &ResourceSqlRequest) -> Ve } /// Every checked resource body, deduplicated in first-seen order and filtered by the opt-out policy. -pub(crate) fn resources(request: &ResourceSqlRequest) -> Vec { +pub(crate) fn resources( + request: &ResourceSqlRequest, + pool: &dyn Fn() -> Result, String>, +) -> Result, String> { let disabled = disabled_models(request); let mut all = audit_resources(request, &disabled); - all.extend(sql_test_resources(request, &disabled)); + all.extend(sql_test_resources(request, &disabled, pool)?); for model in request .models .iter() @@ -221,13 +254,15 @@ pub(crate) fn resources(request: &ResourceSqlRequest) -> Vec { { all.extend(hook_resources_of(model, request)); } - let mut seen: HashSet = HashSet::with_capacity(all.len()); - let mut unique: Vec = Vec::with_capacity(all.len()); - for resource in all { - if seen.insert(resource.clone()) { - unique.push(resource); - } - } - unique.retain(|resource| resource.opt_out.is_none() || request.require_sql_analysis); - unique + let mut seen: HashSet<&ResourceSql> = HashSet::with_capacity(all.len()); + let first_seen: Vec = all.iter().map(|resource| seen.insert(resource)).collect(); + drop(seen); + Ok(all + .into_iter() + .zip(first_seen) + .filter(|(resource, first)| { + *first && (resource.opt_out.is_none() || request.require_sql_analysis) + }) + .map(|(resource, _)| resource) + .collect()) } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compile_driver/targets.rs b/crates/sqlbuild-python/src/bindings/_helpers/compile_driver/targets.rs index 8adc7fbb36..47226c8cda 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compile_driver/targets.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compile_driver/targets.rs @@ -1,5 +1,7 @@ //! Adapter-aware target namespace validation of a compile. +use std::collections::HashMap; + use pyo3::prelude::{ Bound, PyAny, PyAnyMethods, PyErr, PyModule, PyModuleMethods, PyResult, Python, }; @@ -403,6 +405,7 @@ pub(crate) fn project_targets( let adapter_label: String = adapter_name.str()?.to_cow()?.into_owned(); for (resource_kind, attribute) in [("model", "models"), ("seed", "seeds")] { let mut targets: Vec<(String, Bound<'_, PyAny>)> = Vec::new(); + let mut positions: HashMap = HashMap::new(); for resource in native_field(project, attribute)?.try_iter()? { let resource: Bound<'_, PyAny> = resource?; let resource_name: String = native_field(&resource, "name")? @@ -410,12 +413,12 @@ pub(crate) fn project_targets( .to_cow()? .into_owned(); let destination: Bound<'_, PyAny> = native_field(&resource, "destination")?; - match targets - .iter_mut() - .find(|(existing, _)| *existing == resource_name) - { - Some(entry) => entry.1 = destination, - None => targets.push((resource_name, destination)), + match positions.get(&resource_name) { + Some(position) => targets[*position].1 = destination, + None => { + positions.insert(resource_name.clone(), targets.len()); + targets.push((resource_name, destination)); + } } } for (resource_name, target) in targets { diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/model_loop_prepass.rs b/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/model_loop_prepass.rs index d5c9b515c0..8c707a6ceb 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/model_loop_prepass.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/model_loop_prepass.rs @@ -44,6 +44,14 @@ pub(crate) fn own_entry<'a, 'py>( /// Each model's header facts on the compile pool, or in turn when the pool cannot start. pub(crate) fn all_header_facts(headers: &[Option]) -> Vec> { + pooled(headers, facts_of) +} + +/// `each` of every item in order, run on the compile pool, or in turn when the pool cannot start. +pub(crate) fn pooled( + items: &[T], + each: impl Fn(&T) -> R + Sync + Send, +) -> Vec { let pool: &Result = POOL.get_or_init(|| { ThreadPoolBuilder::new() .num_threads(PREPASS_WORKERS) @@ -52,8 +60,8 @@ pub(crate) fn all_header_facts(headers: &[Option]) -> Vec pool.install(|| headers.par_iter().map(facts_of).collect()), - Err(_) => headers.iter().map(facts_of).collect(), + Ok(pool) => pool.install(|| items.par_iter().map(&each).collect()), + Err(_) => items.iter().map(each).collect(), } } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/source_loop.rs b/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/source_loop.rs index 31ff88cb8c..a665075ef2 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/source_loop.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/source_loop.rs @@ -20,7 +20,9 @@ use crate::bindings::_helpers::model_config::config_errors::raised_config_except use crate::bindings::_helpers::model_config::config_templates::{ PythonHost, Stop, TemplateSources, expanded, template_error, }; -use crate::bindings::_helpers::project_assembly::resources::syntax_error_message; +use crate::bindings::_helpers::project_assembly::resources::{ + syntax_error_message, syntax_error_messages, +}; use crate::bindings::_helpers::records::fields::field as native_field; use crate::bindings::_helpers::render::declaration_expansion::{ DeclarationExpansion, ExpandedDeclarations, ExpansionClasses, ExpansionConsumer, @@ -35,6 +37,9 @@ const NATIVE_CONTEXTS_ATTRIBUTE: &str = "native_contexts"; const GENERIC_DIALECT: &str = "generic"; const EXPRESSION_STATEMENTS: [&str; 3] = ["select", "with", "values"]; +/// The SQL parsed for an authored source expression and its syntax error message. +type ParsedExpression = (String, PyResult>); + /// One discovered source: its file's path, the discovered file and the entry. pub(crate) type SourceRow = (String, Py, Py); @@ -51,8 +56,13 @@ pub(crate) fn attached_sources<'py>( let (default_contract, validate, source_kind) = settings; let py: Python<'py> = context.host.py(); let source_kind: Bound<'py, PyAny> = source_kind.into_bound(py); + let mut parsed: Vec> = if validate { + parsed_expressions(py, &rows) + } else { + Vec::new() + }; let mut attached: Vec = Vec::with_capacity(rows.len()); - for (file_path, source_file, entry) in rows { + for (index, (file_path, source_file, entry)) in rows.into_iter().enumerate() { let entry: Bound<'py, PyAny> = entry.into_bound(py); let name: String = native_field(&entry, "name")?.extract()?; let consumer: Bound<'py, PyAny> = context @@ -101,7 +111,8 @@ pub(crate) fn attached_sources<'py>( return Err(compile_error(context.classes, &message, None)); } if validate { - checked_expression(context, (&name, &file_path, sql))?; + let parsed: Option = parsed.get_mut(index).and_then(Option::take); + checked_expression(context, (&name, &file_path, sql), parsed)?; } } let contract: Option = native_field(&entry, "contract")? @@ -409,13 +420,59 @@ fn replaced_spec(value: &Bound<'_, PyAny>, changes: &Bound<'_, PyDict>) -> PyRes } /// `validate_source_expression_syntax`: the expression parses as a FROM target. -fn checked_expression(context: &LoopContext<'_, '_>, source: (&str, &str, &str)) -> PyResult<()> { +fn checked_expression( + context: &LoopContext<'_, '_>, + source: (&str, &str, &str), + parsed: Option, +) -> PyResult<()> { let (name, file_path, expression) = source; - parse_checked( - context, - &format!("SELECT * FROM {}", expression_relation(expression)), - &format!("SQL syntax error in source expression '{name}' ({file_path})"), - ) + let sql: String = expression_sql(expression); + let error_prefix: String = + format!("SQL syntax error in source expression '{name}' ({file_path})"); + match parsed { + Some((parsed_sql, message)) if parsed_sql == sql => match message? { + Some(message) => Err(syntax_error(context, &error_prefix, &message)), + None => Ok(()), + }, + _ => parse_checked(context, &sql, &error_prefix), + } +} + +/// The SQL `validate_source_expression_syntax` parses for `expression`. +fn expression_sql(expression: &str) -> String { + format!("SELECT * FROM {}", expression_relation(expression)) +} + +/// Each authored source expression's parse, run together ahead of the ordered loop. +fn parsed_expressions(py: Python<'_>, rows: &[SourceRow]) -> Vec> { + let mut positions: Vec> = Vec::with_capacity(rows.len()); + let mut checks: Vec = Vec::new(); + for (_, _, entry) in rows { + let mut position: Option = None; + if let Ok(Some(expression)) = native_field(entry.bind(py), "expression") + .and_then(|value| value.extract::>()) + { + position = Some(checks.len()); + checks.push(SyntaxCheck { + sql: expression_sql(&expression), + placeholders: Vec::new(), + }); + } + positions.push(position); + } + let mut messages: Vec>>> = + syntax_error_messages(py, &checks, GENERIC_DIALECT, SyntaxMode::Parse) + .into_iter() + .map(Some) + .collect(); + positions + .into_iter() + .map(|position| { + let index: usize = position?; + let message: PyResult> = messages[index].take()?; + Some((std::mem::take(&mut checks[index].sql), message)) + }) + .collect() } /// `_validate_sql_syntax_with_message` for SQL a non-model resource owns. @@ -433,13 +490,25 @@ pub(crate) fn parse_checked( else { return Ok(()); }; + Err(syntax_error(context, error_prefix, &message)) +} + +/// The compile error raised for a non-model resource's SQL that does not parse. +fn syntax_error(context: &LoopContext<'_, '_>, error_prefix: &str, message: &str) -> PyErr { + let py: Python<'_> = context.host.py(); let keywords: Bound<'_, PyDict> = PyDict::new(py); - keywords.set_item("help", &context.classes.source_syntax_help)?; - let error: Bound<'_, PyAny> = context - .classes - .compile_error - .call((format!("{error_prefix}: {message}"),), Some(&keywords))?; - Err(PyErr::from_value(error)) + let error: PyResult> = keywords + .set_item("help", &context.classes.source_syntax_help) + .and_then(|()| { + context + .classes + .compile_error + .call((format!("{error_prefix}: {message}"),), Some(&keywords)) + }); + match error { + Ok(error) => PyErr::from_value(error), + Err(error) => error, + } } /// `_render_source_expression_relation`: the expression as a table factor. diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/sql_test_loop.rs b/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/sql_test_loop.rs index c330a2897d..c181628576 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/sql_test_loop.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compiled_project/sql_test_loop.rs @@ -608,10 +608,14 @@ impl<'py> TestScope<'_, 'py> { fn classified(&self, tests: &[ExpandedTest<'_, 'py>]) -> PyResult> { let py: Python<'py> = self.py(); let mut files: Vec<(&str, Vec)> = Vec::new(); + let mut positions: HashMap<&str, usize> = HashMap::new(); for (index, test) in tests.iter().enumerate() { - match files.iter_mut().find(|(label, _)| *label == test.file.1) { - Some((_, indexes)) => indexes.push(index), - None => files.push((&test.file.1, vec![index])), + match positions.get(test.file.1.as_str()) { + Some(position) => files[*position].1.push(index), + None => { + positions.insert(&test.file.1, files.len()); + files.push((&test.file.1, vec![index])); + } } } let store: Option> = self diff --git a/crates/sqlbuild-python/src/bindings/_helpers/contracts/contract_diagnostics.rs b/crates/sqlbuild-python/src/bindings/_helpers/contracts/contract_diagnostics.rs index c8621d2dab..2dba2e534a 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/contracts/contract_diagnostics.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/contracts/contract_diagnostics.rs @@ -1,5 +1,7 @@ //! Model contract and promotion-conflict diagnostics, built as compile's `CompilerDiagnostic`s. +use std::collections::HashMap; + use pyo3::prelude::{ Bound, PyAny, PyAnyMethods, PyModule, PyModuleMethods, PyRef, PyResult, Python, }; @@ -179,11 +181,15 @@ pub(crate) fn evaluate_model_contracts<'py>( (dialect_name, mode == IMPLICIT_COLUMN_CONTRACTS), &native, )?; + let latest: HashMap<&str, &[DiagnosticRow]> = outcomes + .iter() + .map(|(owner, rows)| (owner.as_str(), rows.as_slice())) + .collect(); let mut diagnostics: Vec> = Vec::new(); for model in native_field(project, "models")?.try_iter()? { let model: Bound<'py, PyAny> = model?; let name: String = native_field(&model, "name")?.extract()?; - let Some((_, rows)) = outcomes.iter().rev().find(|(owner, _)| *owner == name) else { + let Some(rows) = latest.get(name.as_str()).copied() else { continue; }; for row in rows { diff --git a/crates/sqlbuild-python/src/bindings/_helpers/project_assembly/resources.rs b/crates/sqlbuild-python/src/bindings/_helpers/project_assembly/resources.rs index d626d26f8c..1359df28d5 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/project_assembly/resources.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/project_assembly/resources.rs @@ -9,9 +9,11 @@ use pyo3::{PyErr, pyfunction, wrap_pyfunction}; use sqlbuild_analysis::assembly::project::main::check_sql_syntax::check_sql_syntax; use sqlbuild_analysis::assembly::project::main::sql_syntax_error::sql_syntax_error as check_syntax_error; use sqlbuild_analysis::assembly::project::models::{SyntaxCheck, SyntaxFailure, SyntaxMode}; +use sqlbuild_core::panics::main::catch_compiler_panic::catch_compiler_panic; use sqlbuild_core::panics::main::native_failure::native_failure; use crate::bindings::_helpers::boundary::panics::compiler_error; +use crate::bindings::_helpers::compiled_project::model_loop_prepass::pooled; use crate::bindings::types::CompilerDetach; const SYNTAX_CHECK_CONTEXT: &str = "native SQL syntax check"; @@ -53,6 +55,23 @@ pub(crate) fn syntax_error_message( .map_err(syntax_value_error) } +/// Each check's `syntax_error_message`, in order, parsed together on the compile pool. +pub(crate) fn syntax_error_messages( + py: Python<'_>, + checks: &[SyntaxCheck], + dialect: &str, + mode: SyntaxMode, +) -> Vec>> { + py.detach(|| { + pooled(checks, |check| { + catch_compiler_panic(|| Ok(check_syntax_error(check, dialect, mode))) + }) + }) + .into_iter() + .map(|outcome| outcome.map_err(compiler_error)?.map_err(syntax_value_error)) + .collect() +} + /// The `ValueError` Python's syntax validation raises before parsing. fn syntax_value_error(failure: SyntaxFailure) -> PyErr { match failure { diff --git a/tests/integration/src/sqlbuild/compiler/compiled_project/test_native_sources.py b/tests/integration/src/sqlbuild/compiler/compiled_project/test_native_sources.py index eed81341b3..694f8f2939 100644 --- a/tests/integration/src/sqlbuild/compiler/compiled_project/test_native_sources.py +++ b/tests/integration/src/sqlbuild/compiler/compiled_project/test_native_sources.py @@ -131,6 +131,29 @@ def test_given_source_when_compiling_then_native_project_holds_its_attached_entr expected_message="SQL syntax error in source expression 'raw_orders'", expects_help=True, ), + NativeSourceErrorTestCase( + description="first of several expressions that do not parse", + source_yml=( + "sources:\n" + ' - name: open_orders\n expression: "SELECT 1 AS id"\n' + ' - name: first_orders\n expression: "SELECT FROM WHERE"\n' + ' - name: second_orders\n expression: "SELECT , FROM"\n' + ), + expected_message="SQL syntax error in source expression 'first_orders'", + expects_help=True, + ), + NativeSourceErrorTestCase( + description="earlier source error before a later expression that does not parse", + source_yml=( + "sources:\n - name: raw_orders\n columns:\n - name: amount\n" + " meta:\n unit: ${currency}\n" + ' - name: broken_orders\n expression: "SELECT FROM WHERE"\n' + ), + expected_message=( + "source raw_orders column amount meta references unknown variable 'currency'" + ), + expects_help=False, + ), ], ids=lambda case: case.description, )