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 757a894a90..129c9ec5ce 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 @@ -1,5 +1,7 @@ //! Per-model analysis cache keys over everything an analysis reads, and stored outcomes. +use crate::sql_macros::main::sql_macro_declared_outputs::sql_macro_declared_outputs; +use crate::sql_macros::models::DeclaredOutput; use std::collections::{BTreeSet, HashMap}; use sha2::{Digest, Sha256}; @@ -169,6 +171,20 @@ impl KeyHasher { } self.text(&model.pivot_sql); self.families(&model.dynamic_families); + let declared: Vec = sql_macro_declared_outputs(&model.sql_macro_checks); + self.count(declared.len()); + for output in &declared { + match &output.cte { + Some(cte) => { + self.flag(true); + self.text(cte); + } + None => self.flag(false), + } + self.text(&output.name); + self.text(&output.returns); + self.count(output.branch); + } } fn families(&mut self, families: &[DynamicFamily]) { diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/cte_facts.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/cte_facts.rs index 3fff43f207..74685b243c 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/cte_facts.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/cte_facts.rs @@ -35,6 +35,8 @@ use crate::assembly::analysis_session::types::{Pairs, Shapes}; use crate::nullability_rules::main::conditional_result_nullability::conditional_result_nullability; use crate::nullability_rules::main::first_arg_nullability::first_arg_nullability; use crate::nullability_rules::main::safe_cast_nullability::safe_cast_nullability; +use crate::sql_macros::main::sql_macro_declared_type::sql_macro_declared_type; +use crate::sql_macros::models::DeclaredOutput; use crate::type_system::main::normalize_type::normalize_type; use crate::type_system::models::{NormalizedType, TypeFamily}; @@ -58,6 +60,8 @@ pub(crate) struct RecoveryProfile<'a> { pub(crate) rules: Option<&'a Pairs>, /// Runs the adapter's own rules, those with the `python` rule id. pub(crate) callback: Option<&'a NullabilityCallback>, + /// The model's select items that are one SQL macro call each, typed by its `returns`. + pub(crate) declared: &'a [DeclaredOutput], } /// One enrichment's recovery request. @@ -129,6 +133,108 @@ pub(crate) struct LegacyAnalysis { pub(crate) lineage: Vec, } +/// The model query's output types its SQL macro call items change, given the relation types. +pub(crate) fn declared_output_types( + cleaned_sql: &str, + types: &ShapeTable, + profile: &RecoveryProfile<'_>, +) -> Fact> { + let Some(parsed) = parse_one(cleaned_sql, profile.dialect)? else { + return Ok(Vec::new()); + }; + let facts: Relations = referenced_types(&parsed, types); + let declared: Vec<(String, String)> = Context { + profile, + branches: declared_branches(&parsed), + } + .query_types(&parsed, &facts)?; + let plain: Vec<(String, String)> = Context { + profile, + branches: Vec::new(), + } + .query_types(&parsed, &facts)?; + let main: HashSet = main_output_columns(&parsed, profile.declared); + Ok(declared + .into_iter() + .filter(|(name, data_type)| { + dict_get(&plain, name) != Some(data_type) || main.contains(&casefold(name)) + }) + .collect()) +} + +/// Each select of the main query and top-level CTEs by address, with its scope and branch. +fn declared_branches(root: &Expression) -> Vec { + let mut branches: Vec = Vec::new(); + for (cte_name, has_column_aliases, body) in top_level_ctes(root) { + if !has_column_aliases + && unwrap_annotations(Some(body)).is_some_and(|body| top_level_ctes(body).is_empty()) + { + for (branch, select) in set_branches(Some(body)).into_iter().enumerate() { + branches.push(( + std::ptr::from_ref(select), + Some(cte_name.to_owned()), + branch, + )); + } + } + } + for (branch, select) in set_branches(Some(root)).into_iter().enumerate() { + branches.push((std::ptr::from_ref(select), None, branch)); + } + branches +} + +/// The folded main-query output columns its SQL macro call items stand in, in any branch. +fn main_output_columns(root: &Expression, declared: &[DeclaredOutput]) -> HashSet { + let selects: Vec<&Expression> = set_branches(Some(root)); + let by_name: bool = set_operation(root).is_some_and(|(_, _, by_name)| by_name); + let Some(first) = selects.first() else { + return HashSet::new(); + }; + let first_names: Vec<&str> = projection_names(first); + let mut columns: HashSet = HashSet::new(); + for output in declared.iter().filter(|output| output.cte.is_none()) { + if by_name { + columns.insert(casefold(&output.name)); + continue; + } + let Some(select) = selects.get(output.branch) else { + continue; + }; + let position: Option = projection_names(select) + .iter() + .position(|name| name.eq_ignore_ascii_case(&output.name)); + if let Some(column) = position.and_then(|position| first_names.get(position)) { + columns.insert(casefold(column)); + } + } + columns +} + +/// The output name of each of `select`'s projections, in order. +fn projection_names(select: &Expression) -> Vec<&str> { + select + .get_expressions() + .iter() + .map(|projection| output_name(unwrap_annotations(Some(projection)))) + .collect() +} + +/// The selects of `node`'s set operation, left to right, or `node` itself. +fn set_branches(node: Option<&Expression>) -> Vec<&Expression> { + let Some(node) = unwrap_annotations(node) else { + return Vec::new(); + }; + match set_operation(node) { + Some((left, right, _)) => { + let mut branches: Vec<&Expression> = set_branches(Some(left)); + branches.extend(set_branches(Some(right))); + branches + } + None => vec![node], + } +} + /// Python's legacy analysis of a model the native engine handed back, or why it defers. pub(crate) fn legacy_analysis(input: &LegacyInput<'_>) -> Fact { let Some(parsed) = parse_one(input.cleaned_sql, input.profile.dialect)? else { @@ -166,6 +272,7 @@ pub(crate) fn legacy_analysis(input: &LegacyInput<'_>) -> Fact { let unqualified: Option<&(String, String)> = single_resource(&resources); let context = Context { profile: &input.profile, + branches: Vec::new(), }; let (cte_types, cte_nullability) = if input.recover { ( @@ -436,6 +543,7 @@ fn passthrough_facts( } let context = Context { profile: &input.profile, + branches: Vec::new(), }; recovery.types = context.passthrough_types(root, &input_types(input))?; recovery.nullability = context.passthrough_nullability(root, &input_nullability(input))?; @@ -466,24 +574,21 @@ fn direct_cte_outputs(root: &Expression, cte_names: &HashSet) -> HashSet outputs } +/// A select's address, its top-level CTE or `None` for the main query, and its branch. +type Branch = (*const Expression, Option, usize); + /// The profile every resolver reads. struct Context<'a, 'b> { profile: &'a RecoveryProfile<'b>, + /// The selects whose SQL macro call items take their declared types. + branches: Vec, } impl Context<'_, '_> { - /// Python's `_polyglot_cte_passthrough_types_from_parsed`. - fn passthrough_types( - &self, - root: &Expression, - facts: &Relations, - ) -> Fact> { - let ctes: Vec<(&str, bool, &Expression)> = top_level_ctes(root); - if ctes.is_empty() { - return Ok(Vec::new()); - } + /// The model query's output types, through its top-level CTEs. + fn query_types(&self, root: &Expression, facts: &Relations) -> Fact> { let mut relations: Relations = referenced_facts(root, facts); - for (cte_name, has_column_aliases, body) in ctes { + for (cte_name, has_column_aliases, body) in top_level_ctes(root) { if has_column_aliases { continue; } @@ -499,6 +604,52 @@ impl Context<'_, '_> { self.direct_select_types(Some(root), &relations) } + /// Python's `_polyglot_cte_passthrough_types_from_parsed`. + fn passthrough_types( + &self, + root: &Expression, + facts: &Relations, + ) -> Fact> { + if top_level_ctes(root).is_empty() { + return Ok(Vec::new()); + } + self.query_types(root, facts) + } + + /// `types` of `select` with each of its SQL macro call items typed by its `returns`. + fn declared_types( + &self, + select: &Expression, + mut types: Vec<(String, String)>, + ) -> Vec<(String, String)> { + let address: *const Expression = std::ptr::from_ref(select); + let Some((_, cte, branch)) = self + .branches + .iter() + .find(|(branch_select, _, _)| *branch_select == address) + else { + return types; + }; + for output in self.profile.declared { + let key: String = types + .iter() + .map(|(name, _)| name) + .find(|name| name.eq_ignore_ascii_case(&output.name)) + .cloned() + .unwrap_or_else(|| output.name.clone()); + let inferred: Option<&str> = dict_get(&types, &key).map(String::as_str); + if let Some(declared) = sql_macro_declared_type( + self.profile.declared, + (cte.as_deref(), *branch, &output.name), + inferred, + self.profile.dialect, + ) { + types = dict_set(types, &key, declared); + } + } + types + } + /// Python's `_polyglot_select_output_types`. fn select_output_types( &self, @@ -689,7 +840,7 @@ impl Context<'_, '_> { inferred = dict_set(inferred, output, inferred_type); } } - Ok(inferred) + Ok(self.declared_types(select, inferred)) } /// Python's `_polyglot_direct_expression_type`. diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/enrichment.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/enrichment.rs index 2d69ab47a5..cefd99d2b9 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/enrichment.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/enrichment.rs @@ -113,6 +113,7 @@ fn legacy(input: &EnrichmentInput<'_>, cleaned: &str) -> Result = input diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/query_columns.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/query_columns.rs index 7a20a27572..f360b6b503 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/query_columns.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/query_columns.rs @@ -182,6 +182,7 @@ impl Tables<'_> { function_return_types: &self.request.function_return_types, rules: self.request.nullability_rules.as_ref(), callback: self.request.nullability_callback.as_ref(), + declared: &[], }, }) })?; 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 35bd85b0f4..d8d1e7d87a 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/session.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/_helpers/session.rs @@ -19,7 +19,7 @@ use crate::assembly::analysis_session::_helpers::compact_batch::{ Batch, BatchMember, MemberAnalysis, MemberResult, }; use crate::assembly::analysis_session::_helpers::cte_facts::{ - LegacyAnalysis, LegacyInput, RecoveryProfile, legacy_analysis, + LegacyAnalysis, LegacyInput, RecoveryProfile, declared_output_types, legacy_analysis, }; use crate::assembly::analysis_session::_helpers::dynamic_pivot::{PivotFacts, pivot_outcome}; use crate::assembly::analysis_session::_helpers::enrichment::{ @@ -40,7 +40,9 @@ use crate::assembly::analysis_session::types::{Pairs, Shapes}; use crate::semantic_validation::models::DiagnosticRow; use crate::semantic_validation::models::ProjectCatalog; use crate::sql_macros::main::check_sql_macro_arguments::check_sql_macro_arguments; -use crate::sql_macros::models::SqlMacroFinding; +use crate::sql_macros::main::sql_macro_declared_outputs::sql_macro_declared_outputs; +use crate::sql_macros::main::sql_macro_declared_type::sql_macro_declared_type; +use crate::sql_macros::models::{DeclaredOutput, SqlMacroCheck, SqlMacroFinding}; use crate::sql_macros::types::ArgumentCheckInput; /// What an internal native failure of the session names. @@ -294,6 +296,76 @@ impl AnalysisSession { self.check_sql_macros(models) } + /// Type the outputs SQL macro call items reach by the macros' `returns`, through CTEs. + fn type_sql_macro_outputs(&mut self, models: &[usize]) -> Result<(), String> { + for model in models { + let declared: Vec = + sql_macro_declared_outputs(&self.request.models[*model].sql_macro_checks); + if declared.is_empty() || self.cache_hit(*model) { + continue; + } + self.ensure_cleaned(&[*model])?; + let Some(outcome) = self.outcomes[*model].as_ref() else { + continue; + }; + let Ok(cleaned) = resolved_cleaned_sql(outcome) else { + continue; + }; + let profile = RecoveryProfile { + dialect: &self.request.dialect, + function_return_types: &self.request.function_return_types, + rules: self.request.nullability_rules.as_ref(), + callback: self.request.nullability_callback.as_ref(), + declared: &declared, + }; + let propagated: Result, String> = catch_compiler_panic(|| { + Ok(declared_output_types( + &cleaned, + &self.available_types, + &profile, + )) + }) + .map_err(|reason| { + native_failure(&model_context(&self.request.models[*model]), &reason) + })?; + let Ok(typed) = propagated else { + continue; + }; + let dialect: String = self.request.dialect.clone(); + let Some(columns) = self.outcome_mut(*model)?.analysis.columns.as_mut() else { + continue; + }; + for column in columns { + let Some((name, returns)) = typed + .iter() + .find(|(name, _)| name.eq_ignore_ascii_case(&column.name)) + else { + continue; + }; + let reached: [DeclaredOutput; 1] = [DeclaredOutput { + cte: None, + name: name.clone(), + returns: returns.clone(), + branch: 0, + }]; + if let Some(data_type) = sql_macro_declared_type( + &reached, + (None, 0, &column.name), + column.data_type.as_deref(), + &dialect, + ) { + column.data_type = Some(data_type); + } + } + } + Ok(()) + } + + /// Whether the cache answered `model` with an outcome that already holds its declared types. + fn cache_hit(&self, model: usize) -> bool { + self.cache.as_ref().is_some_and(|cache| cache.hits[model]) + } + /// Each model's SQL macro call arguments checked against its analysed types, cache hits too. fn check_sql_macros(&mut self, models: &[usize]) -> Result<(), String> { let checked: Vec = models @@ -301,7 +373,7 @@ impl AnalysisSession { .copied() .filter(|model| { let request: &ModelRequest = &self.request.models[*model]; - !request.sql_macro_checks.is_empty() && request.placeholders.is_empty() + checks_arguments(&request.sql_macro_checks) && request.placeholders.is_empty() }) .collect(); self.ensure_cleaned(&checked)?; @@ -355,6 +427,7 @@ impl AnalysisSession { function_return_types: &self.request.function_return_types, rules: self.request.nullability_rules.as_ref(), callback: self.request.nullability_callback.as_ref(), + declared: &[], }, }) }) @@ -476,6 +549,7 @@ 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(); + self.type_sql_macro_outputs(&models)?; let validated: &[usize] = if self.cyclic { &[] } else { &models }; let mut revalidated: Vec<(usize, Shapes)> = Vec::new(); for model in validated { @@ -721,6 +795,11 @@ impl AnalysisSession { } } +/// Whether any of the checks has arguments the analysed SQL must show fit. +fn checks_arguments(checks: &[SqlMacroCheck]) -> bool { + checks.iter().any(|check| !check.arguments.is_empty()) +} + fn batch_member<'a>(model: &'a ModelRequest, schema: &'a Shapes) -> BatchMember<'a> { BatchMember { query_sql: &model.query_sql, diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/main/expression_shapes.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/main/expression_shapes.rs index b65c4a0ad7..27a9755d87 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/main/expression_shapes.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/main/expression_shapes.rs @@ -66,6 +66,7 @@ pub fn expression_shapes( function_return_types: &request.function_return_types, rules: request.nullability_rules.as_ref(), callback: request.nullability_callback.as_ref(), + declared: &[], }, }) })?; diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/main/recover_cte_facts.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/main/recover_cte_facts.rs index 92b9f05478..acd8cc734a 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/main/recover_cte_facts.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/main/recover_cte_facts.rs @@ -29,6 +29,7 @@ fn recovered_facts(request: &CteFactRequest) -> Result { function_return_types: &request.function_return_types, rules: request.nullability_rules.as_ref(), callback: None, + declared: &[], }, })?; let mut direct_outputs: Vec = recovered.direct_outputs.into_iter().collect(); diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/tests/helpers.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/tests/helpers.rs index 459254d0f7..49eb329990 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/tests/helpers.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/tests/helpers.rs @@ -27,6 +27,7 @@ use crate::assembly::analysis_session::tests::test_types::{ }; use crate::assembly::analysis_session::types::{Pairs, Shapes}; use crate::semantic_validation::models::{CatalogInput, ProjectCatalog}; +use crate::sql_macros::models::{CallScope, SqlMacroCheck, TypedOutput}; const TRANSFORM_NAMES: [&str; 6] = [ "direct", @@ -179,6 +180,11 @@ pub(crate) fn cached_run(request: SessionRequest, store: NativeStore) -> CachedR (keys, hits, taken) }); CachedRun { + deferred: outcome + .models + .iter() + .map(|model| model.deferred_cleaning.is_some()) + .collect(), lines: ( steps.iter().map(step_lines).collect(), outcome.models.iter().map(described).collect(), @@ -471,6 +477,7 @@ pub(crate) fn recovered_facts(sql: &str) -> Option<(Pairs, Pairs, Vec, V function_return_types: &Vec::new(), rules: Some(&rules), callback: None, + declared: &[], }, }) .ok()?; @@ -550,6 +557,7 @@ pub(crate) fn legacy_lines(sql: &str) -> Option> { function_return_types: &Vec::new(), rules: Some(&rules), callback: None, + declared: &[], }, }) .ok()?; @@ -697,3 +705,21 @@ pub(crate) fn injected_relations( request.forced_order = Some((relations.dependency_ordered, relations.cyclic)); request } + +/// The orders request over `models`, the first typing `label` in CTE `final` as VARCHAR. +pub(crate) fn declared_label_request(models: &[ModelSpec]) -> SessionRequest { + let mut requests: Vec = model_requests(models); + requests[0].sql_macro_checks.push(SqlMacroCheck { + macro_name: "label".to_owned(), + scope: CallScope::Cte("final".to_owned()), + plain_select: true, + arguments: Vec::new(), + call: None, + output: Some(TypedOutput { + name: "label".to_owned(), + returns: "VARCHAR".to_owned(), + branch: 0, + }), + }); + orders_request(requests) +} diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/tests/test_analysis_cache.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/tests/test_analysis_cache.rs index 24c55a4b51..914908700f 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/tests/test_analysis_cache.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/tests/test_analysis_cache.rs @@ -3,13 +3,13 @@ use sqlbuild_cache::store::models::NativeStore; use crate::assembly::analysis_session::models::{ModelReference, SessionRequest}; use crate::assembly::analysis_session::tests::helpers::{ - amounts_family, cache_stats, cached_run, current_model_key, damaged, model_keys, - model_requests, orders_request, pairs, second_model, session_lines, shapes, started, - uncached_catalog, + amounts_family, cache_stats, cached_run, completed, current_model_key, damaged, + declared_label_request, described, model_keys, model_requests, orders_request, pairs, + second_model, session_lines, shapes, started, step_lines, uncached_catalog, }; use crate::assembly::analysis_session::tests::test_types::{ CacheEditTestCase, CacheKeyTestCase, CacheReuseTestCase, CachedRun, DamagedCacheTestCase, - ModelSpec, RelationDigestTestCase, + DeclaredHitTestCase, ModelSpec, RelationDigestTestCase, }; const STG_ORDERS: ModelSpec = ( @@ -73,6 +73,15 @@ const EVENTS_TOO_DEEP: ModelSpec = ( &[], ); +/// Labels orders with a function the inference does not type, in CTE `final`. +const LABELLED: ModelSpec = ( + "labelled", + "WITH final AS (SELECT order_id, bar(amount, 0, 10) AS label FROM __source(\"raw_orders\")) \ + SELECT * FROM final", + &["raw_orders"], + &[], +); + #[test] fn given_a_filled_cache_when_rerunning_then_hits_match_the_uncached_session() { let test_cases = [ @@ -519,3 +528,47 @@ fn given_a_read_relation_fact_changed_when_keying_then_the_reading_model_key_cha ); } } + +#[test] +fn given_a_declared_sql_macro_output_when_rerunning_then_the_hit_keeps_its_type_unpatched() { + let test_cases = [DeclaredHitTestCase { + description: "a call item in a final CTE, read with a star", + models: &[LABELLED], + expected_label: "label VARCHAR", + }]; + for test_case in test_cases { + let (uncached_steps, uncached_models, _) = + completed(started(declared_label_request(test_case.models))); + let cold: CachedRun = cached_run( + declared_label_request(test_case.models), + NativeStore::default(), + ); + let cold_lines = cold.lines; + let warm: CachedRun = cached_run(declared_label_request(test_case.models), cold.store); + let uncached = ( + uncached_steps.iter().map(step_lines).collect::>(), + uncached_models.iter().map(described).collect::>(), + ); + + assert!( + uncached.1[0] + .iter() + .any(|line| line.starts_with(test_case.expected_label)), + "{}: {:?}", + test_case.description, + uncached.1 + ); + assert_eq!( + (&cold_lines, &warm.lines), + (&uncached, &uncached), + "{}", + test_case.description + ); + assert_eq!( + (warm.hits, warm.deferred), + (vec![true], vec![true]), + "{}", + 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 6a3236ccd9..9568fadd92 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 @@ -144,6 +144,8 @@ pub(crate) struct CachedRun { pub(crate) keys: Vec>, /// Whether each model's outcome came from the cache. pub(crate) hits: Vec, + /// Whether each model's outcome still defers its cleaned SQL. + pub(crate) deferred: Vec, /// The catalog's schema additions and analysis names, then the sorted fact models. pub(crate) catalog: ( crate::assembly::analysis_session::types::Shapes, @@ -217,3 +219,10 @@ pub(crate) struct DeltaSessionTestCase { pub(crate) model_index: usize, pub(crate) expected_whole_session_outcome: bool, } + +/// Models whose first one declares a SQL macro output, and that output's described column. +pub(crate) struct DeclaredHitTestCase { + pub(crate) description: &'static str, + pub(crate) models: &'static [ModelSpec], + pub(crate) expected_label: &'static str, +} diff --git a/crates/sqlbuild-analysis/src/definition_changes/_helpers/sql_macro_parts.rs b/crates/sqlbuild-analysis/src/definition_changes/_helpers/sql_macro_parts.rs index 9bd4f7685f..7fdce06669 100644 --- a/crates/sqlbuild-analysis/src/definition_changes/_helpers/sql_macro_parts.rs +++ b/crates/sqlbuild-analysis/src/definition_changes/_helpers/sql_macro_parts.rs @@ -7,6 +7,7 @@ use crate::sql_macros::models::{ParameterKind, SqlMacroParameter}; pub(crate) fn parameter_text(parameter: &SqlMacroParameter) -> String { let kind: String = match ¶meter.kind { ParameterKind::Expression { data_type } => format!("expression {}", type_text(data_type)), + ParameterKind::Any => "expression any".to_owned(), ParameterKind::Constant { data_type } => format!("const {}", type_text(data_type)), ParameterKind::Collection { set, data_type } => { format!("collection {set} {}", type_text(data_type)) diff --git a/crates/sqlbuild-analysis/src/sql_macros/_helpers/arguments.rs b/crates/sqlbuild-analysis/src/sql_macros/_helpers/arguments.rs index c696813797..76769a2286 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/_helpers/arguments.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/_helpers/arguments.rs @@ -350,6 +350,11 @@ pub(crate) fn check_shape( Err(expects("a SQL expression")) } (ParameterKind::Expression { .. }, _) => Ok(()), + (ParameterKind::Any, ArgumentShape::InlineCollection | ArgumentShape::Reference { .. }) => { + Err(expects("a scalar SQL expression") + .helped("an ANY argument takes one value per row, such as a column, a literal or an expression; pass lists to LIST arguments and relations to RELATION arguments")) + } + (ParameterKind::Any, _) => Ok(()), (ParameterKind::Constant { .. }, ArgumentShape::ConstantReference { .. }) => Ok(()), (ParameterKind::Constant { .. }, _) => Err(expects("a literal, a scalar constant or an enum value") .helped(format!("pass a literal, such as 2, or a constant, such as @const(\"{name}\"); a CONST argument is known at compile time, so a column or expression cannot be passed"))), @@ -386,9 +391,10 @@ pub(crate) fn check_shape( /// Whether an argument of kind `outer`, forwarded as `@name`, fits a parameter of kind `inner`. fn forwards(outer: &ParameterKind, inner: &ParameterKind) -> bool { match inner { - ParameterKind::Expression { .. } => matches!( + ParameterKind::Expression { .. } | ParameterKind::Any => matches!( outer, ParameterKind::Expression { .. } + | ParameterKind::Any | ParameterKind::Constant { .. } | ParameterKind::Enum { .. } | ParameterKind::ColumnOf { .. } diff --git a/crates/sqlbuild-analysis/src/sql_macros/_helpers/body.rs b/crates/sqlbuild-analysis/src/sql_macros/_helpers/body.rs index 69de0a50a4..90a597e4ba 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/_helpers/body.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/_helpers/body.rs @@ -216,7 +216,7 @@ fn substituted( _ => argument.sql.clone(), } } - ParameterKind::Expression { .. } | ParameterKind::Constant { .. } + ParameterKind::Expression { .. } | ParameterKind::Any | ParameterKind::Constant { .. } if !is_atomic(&argument.sql) => { format!("({})", argument.sql) diff --git a/crates/sqlbuild-analysis/src/sql_macros/_helpers/expansion.rs b/crates/sqlbuild-analysis/src/sql_macros/_helpers/expansion.rs index 6962997c11..c8e3ed4db7 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/_helpers/expansion.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/_helpers/expansion.rs @@ -24,7 +24,7 @@ use crate::sql_macros::errors::SqlMacroError; use crate::sql_macros::models::{ ArgumentCheck, ArgumentExpectation, ArgumentShape, BodyExpansion, BoundArgument, CallPosition, CallScope, InsertedNames, ParameterKind, RenderedArgument, SqlMacroCall, SqlMacroCheck, - SqlMacroDefinition, SqlMacroForm, SqlMacroLocation, TextContext, + SqlMacroDefinition, SqlMacroForm, SqlMacroLocation, TextContext, TypedOutput, }; use crate::sql_macros::types::{ConstantShape, SqlMacroHost, SqlMacroScope}; @@ -214,24 +214,36 @@ pub(crate) fn expand_call( inserted, )); } + let site: CallSite<'_> = CallSite::new( + context.text, + scanned, + (call.start, call.end), + call.replaced_end, + ); if matches!(context.scope, SqlMacroScope::Caller) && context.stack.is_empty() { + let output: Option = match (definition.form, &definition.returns) { + (SqlMacroForm::Expression, Some(returns)) => site + .whole_item_alias() + .zip(call_branch(context.text, scanned, call.start)) + .map(|(name, branch)| TypedOutput { + name, + returns: returns.clone(), + branch, + }), + _ => None, + }; let check: SqlMacroCheck = SqlMacroCheck { macro_name: definition.name.clone(), scope: call_scope(context.text, scanned, call.start), plain_select: plain_select(context.text, scanned, call.start), arguments: argument_checks(&definition, &bound, &rendered), call: None, + output, }; - if !check.arguments.is_empty() { + if !check.arguments.is_empty() || check.output.is_some() { host.record_check(check, (call.start, call.end)); } } - let site: CallSite<'_> = CallSite::new( - context.text, - scanned, - (call.start, call.end), - call.replaced_end, - ); let line_continues: bool = context.text[call.replaced_end..] .split('\n') .next() @@ -294,7 +306,10 @@ fn rendered_argument( } host.render_declarations(scope, text)? } - (ParameterKind::Constant { .. }, ArgumentShape::ConstantReference { name }) => { + ( + ParameterKind::Constant { .. } | ParameterKind::Any, + ArgumentShape::ConstantReference { name }, + ) => { if host.constant_shape(scope, name)? != ConstantShape::Scalar { return Err(expects("a scalar constant")); } @@ -516,7 +531,9 @@ fn check_adapter_types( .iter() .map(|(_, data_type)| data_type.as_str()) .collect(), - ParameterKind::Enum { .. } | ParameterKind::ColumnOf { .. } => Vec::new(), + ParameterKind::Any | ParameterKind::Enum { .. } | ParameterKind::ColumnOf { .. } => { + Vec::new() + } }; for data_type in types { if let Some(error) = invalid( @@ -586,6 +603,11 @@ fn call_scope(text: &str, scanned: &[Token], offset: usize) -> CallScope { } } +/// The branch of its scope's set operation the call at byte `offset` of `text` stands in. +fn call_branch(text: &str, scanned: &[Token], offset: usize) -> Option { + top_level_parts(text, scanned)?.branch_of(text, scanned, offset) +} + /// Whether the call at byte `offset` of `text` stands in its scope's one select. fn plain_select(text: &str, scanned: &[Token], offset: usize) -> bool { top_level_parts(text, scanned).is_some_and(|parts| parts.plain_select(text, scanned, offset)) diff --git a/crates/sqlbuild-analysis/src/sql_macros/_helpers/header.rs b/crates/sqlbuild-analysis/src/sql_macros/_helpers/header.rs index f3dc32cafe..513b9e32d6 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/_helpers/header.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/_helpers/header.rs @@ -2,9 +2,10 @@ use crate::sql_macros::_helpers::lexing::{Token, TokenKind, closing_token, line_column, tokens}; use crate::sql_macros::constants::{ - ARGUMENTS_KEY, COLUMN_KEYWORD, COMMA, CONST_KEYWORD, DEFAULT_KEYWORD, DESCRIPTION_KEY, - ENUM_KEYWORD, HEADER_KEYWORD, INVALID_DEFINITION_CODE, LIST_KEYWORD, OF_KEYWORD, - RAW_TYPE_NAMES, RELATION_COLUMN_MIN_TOKENS, RELATION_KEYWORD, RETURNS_KEY, SET_KEYWORD, + ANY_KEYWORD, ARGUMENTS_KEY, COLUMN_KEYWORD, COMMA, CONST_KEYWORD, DEFAULT_KEYWORD, + DESCRIPTION_KEY, ENUM_KEYWORD, HEADER_KEYWORD, INVALID_DEFINITION_CODE, LIST_KEYWORD, + OF_KEYWORD, RAW_TYPE_NAMES, RELATION_COLUMN_MIN_TOKENS, RELATION_KEYWORD, RETURNS_KEY, + SET_KEYWORD, }; use crate::sql_macros::errors::LexError; use crate::sql_macros::errors::SqlMacroError; @@ -259,10 +260,33 @@ impl HeaderParser<'_> { )), }; } + if first.is_word(source, ANY_KEYWORD) { + let Some(after) = tokens.get(1) else { + return Ok(ParameterKind::Any); + }; + if after.is_word(source, CONST_KEYWORD) { + return Err(self + .error(after.start, format!("argument {name} ANY CONST: a CONST argument needs a type its values are checked against")) + .helped(format!("declare it with the type of its values, such as {name} VARCHAR CONST"))); + } + return Err(self + .error(after.start, format!("argument {name} ANY takes nothing after it; ANY accepts any scalar expression")) + .helped(format!("declare it {name} ANY, or give it a SQL type"))); + } if first.is_word(source, LIST_KEYWORD) || first.is_word(source, SET_KEYWORD) { let set: bool = first.is_word(source, SET_KEYWORD); let keyword: String = first.text(source).to_ascii_uppercase(); let element: &[Token] = &tokens[1..]; + if element.len() == 1 && element[0].is_word(source, ANY_KEYWORD) { + return Err(self + .error( + element[0].start, + format!("argument {name} {keyword} ANY: ANY is for scalar arguments only"), + ) + .helped(format!( + "declare the element type, such as {name} {keyword} VARCHAR" + ))); + } if let Some((last, element)) = element.split_last() && last.is_word(source, CONST_KEYWORD) { diff --git a/crates/sqlbuild-analysis/src/sql_macros/_helpers/literal_positions.rs b/crates/sqlbuild-analysis/src/sql_macros/_helpers/literal_positions.rs index c904ea4759..515eb037e5 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/_helpers/literal_positions.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/_helpers/literal_positions.rs @@ -39,7 +39,7 @@ pub(crate) fn non_literal_use<'r>( let parameter = definition.parameter(&reference.name)?; if !matches!( parameter.kind, - ParameterKind::Expression { .. } | ParameterKind::ColumnOf { .. } + ParameterKind::Expression { .. } | ParameterKind::Any | ParameterKind::ColumnOf { .. } ) { return None; } @@ -232,6 +232,9 @@ pub(crate) fn literal_error( Some(ParameterKind::Expression { data_type }) => { format!("declare it {name} {data_type} CONST") } + Some(ParameterKind::Any) => { + format!("declare it with a type and CONST, such as {name} VARCHAR CONST") + } _ => format!("pass the value through a CONST argument, such as {name}_value VARCHAR CONST"), }; SqlMacroError::new(LITERAL_REQUIRED_CODE, format!("{name} is used where a literal is required; {help}")) diff --git a/crates/sqlbuild-analysis/src/sql_macros/_helpers/placement.rs b/crates/sqlbuild-analysis/src/sql_macros/_helpers/placement.rs index 209ddd5283..9991907baf 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/_helpers/placement.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/_helpers/placement.rs @@ -2,7 +2,7 @@ use crate::sql_macros::_helpers::header::unquoted; use crate::sql_macros::_helpers::lexing::{Token, TokenKind, closing_token}; -use crate::sql_macros::constants::{AT_SIGN, COMMA, SELECT_KEYWORD, WITH_KEYWORD}; +use crate::sql_macros::constants::{AS_KEYWORD, AT_SIGN, COMMA, SELECT_KEYWORD, WITH_KEYWORD}; use crate::sql_macros::models::{CallPosition, CallScope}; /// Tokens before the `(` of `DISTINCT ON (...)`. @@ -11,6 +11,10 @@ const DISTINCT_ON_TOKENS: usize = 2; const QUERY_OPENERS: [&str; 4] = ["SELECT", "WITH", "VALUES", "FROM"]; /// Keywords that combine the selects of a query. const SET_OPERATIONS: [&str; 3] = ["UNION", "INTERSECT", "EXCEPT"]; +/// A star, which an `EXCEPT` column list may follow as a modifier, not a set operation. +const STAR: &str = "*"; +/// The words that may stand between a set operation and its parenthesised right branch. +const SET_OPERATION_MODIFIERS: [&str; 3] = ["ALL", "DISTINCT", "NAME"]; /// Tokens from `@` to a macro call's name: `@`, the kind, `.` and the name. const CALL_PREFIX_TOKENS: usize = 4; /// Keywords that open the clause a call stands in, read backwards from the call. @@ -252,6 +256,48 @@ impl<'a> CallSite<'a> { } } + /// The alias of the select-list item the call is the whole of, as `@sql.m(x) AS name`. + pub(crate) fn whole_item_alias(&self) -> Option { + let opens_item: bool = self.previous_token().is_some_and(|token| { + matches!( + token.text(self.sql).to_ascii_uppercase().as_str(), + "SELECT" | "DISTINCT" | "ALL" | "," + ) + }); + if !opens_item || self.position() != CallPosition::SelectList { + return None; + } + let next: usize = self.next?; + let alias_index: usize = if self.tokens[next].is_word(self.sql, AS_KEYWORD) { + next + 1 + } else { + next + }; + let alias: &Token = self.tokens.get(alias_index)?; + let alias_text: String = alias.text(self.sql).to_ascii_uppercase(); + let named: bool = match alias.kind { + TokenKind::Quoted => true, + TokenKind::Word => { + !SELECT_LIST_ENDS.contains(&alias_text.as_str()) + && !EXPRESSION_OPERATOR_WORDS.contains(&alias_text.as_str()) + && alias_text != AS_KEYWORD + } + _ => false, + }; + if !named { + return None; + } + let ends_item: bool = match self.tokens.get(alias_index + 1) { + None => true, + Some(after) => { + let text: String = after.text(self.sql).to_ascii_uppercase(); + matches!(text.as_str(), "," | ")" | ";") + || (after.kind == TokenKind::Word && SELECT_LIST_ENDS.contains(&text.as_str())) + } + }; + ends_item.then(|| unquoted(alias.text(self.sql))) + } + /// Whether the token after the call ends a select-list item. fn ends_select_item(&self) -> bool { let Some(next) = self.next_token() else { @@ -569,6 +615,74 @@ impl QueryParts { } } +impl QueryParts { + /// The set-operation branch byte `offset` stands in; `None` where it cannot be counted. + pub(crate) fn branch_of(&self, text: &str, scanned: &[Token], offset: usize) -> Option { + let range: (usize, usize) = match self.scope_of(offset) { + CallScope::WithList => return None, + CallScope::Cte(name) => self.ctes.iter().find(|cte| cte.name == name)?.body, + CallScope::Main => (self.main, text.len()), + }; + let inside: Vec<(usize, &Token)> = scanned + .iter() + .enumerate() + .filter(|(_, token)| range.0 <= token.start && token.end <= range.1) + .collect(); + if inside + .first() + .is_some_and(|(_, token)| token.is_word(text, WITH_KEYWORD)) + { + return None; + } + let mut depth: usize = 0; + let mut branch: usize = 0; + let mut at_call: Option = None; + let mut previous_at_top: Option<&Token> = None; + for (index, token) in inside { + if at_call.is_none() && token.start >= offset { + if depth != 0 { + return None; + } + at_call = Some(branch); + } + if token.kind == TokenKind::Punct { + match token.text(text) { + "(" | "[" | "{" => { + let query: bool = scanned + .get(index + 1) + .is_some_and(|next| opens_query(text, next)); + let branch_start: bool = previous_at_top.is_none_or(|previous| { + let word: &str = previous.text(text); + is_set_operation(word) + || SET_OPERATION_MODIFIERS + .iter() + .any(|modifier| word.eq_ignore_ascii_case(modifier)) + }); + if depth == 0 && query && branch_start { + return None; + } + depth += 1; + } + ")" | "]" | "}" => depth = depth.saturating_sub(1), + _ => {} + } + } + if depth == 0 { + let star_modifier: bool = + previous_at_top.is_some_and(|previous| previous.text(text) == STAR); + if token.kind == TokenKind::Word + && is_set_operation(token.text(text)) + && !star_modifier + { + branch += 1; + } + previous_at_top = Some(token); + } + } + at_call + } +} + /// Whether `token` opens a query, as the first token inside a parenthesis. fn opens_query(text: &str, token: &Token) -> bool { QUERY_OPENERS diff --git a/crates/sqlbuild-analysis/src/sql_macros/constants.rs b/crates/sqlbuild-analysis/src/sql_macros/constants.rs index e7905d424f..646741e2ad 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/constants.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/constants.rs @@ -41,7 +41,9 @@ pub(crate) const SET_KEYWORD: &str = "SET"; /// Names a parameter may not take, because `@name` already means something else. pub(crate) const RESERVED_PARAMETER_NAMES: [&str; 5] = ["sql", "py", "const", "enum", "param"]; /// Type names that would make a parameter raw, untyped text. -pub(crate) const RAW_TYPE_NAMES: [&str; 4] = ["RAW", "SQL", "ANY", "EXPRESSION"]; +pub(crate) const RAW_TYPE_NAMES: [&str; 3] = ["RAW", "SQL", "EXPRESSION"]; +/// The type of a parameter that takes any scalar expression and checks no type. +pub(crate) const ANY_KEYWORD: &str = "ANY"; /// Reference functions a SQL macro body may not call. pub(crate) const REFERENCE_FUNCTIONS: [&str; 3] = ["__ref", "__source", "__seed"]; /// The prefix of a SQL macro call. @@ -62,6 +64,8 @@ pub(crate) const CTE_NOUN: &str = "CTE"; /// Clause keywords a call position reads. pub(crate) const WITH_KEYWORD: &str = "WITH"; pub(crate) const SELECT_KEYWORD: &str = "SELECT"; +/// The keyword before an alias. +pub(crate) const AS_KEYWORD: &str = "AS"; /// Punctuation and operators the scans compare against. pub(crate) const COMMA: &str = ","; pub(crate) const AT_SIGN: &str = "@"; diff --git a/crates/sqlbuild-analysis/src/sql_macros/main/mod.rs b/crates/sqlbuild-analysis/src/sql_macros/main/mod.rs index 38f29462c4..ab74c41019 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/main/mod.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/main/mod.rs @@ -7,5 +7,8 @@ pub mod render_inline_collection; pub mod sql_macro_body_form; pub mod sql_macro_calls; pub mod sql_macro_cycle; +pub mod sql_macro_declared_outputs; +pub mod sql_macro_declared_type; +pub mod sql_macro_output_names; pub mod sql_macro_parameter_spans; pub mod with_list_sql_macro_calls; diff --git a/crates/sqlbuild-analysis/src/sql_macros/main/sql_macro_declared_outputs.rs b/crates/sqlbuild-analysis/src/sql_macros/main/sql_macro_declared_outputs.rs new file mode 100644 index 0000000000..f9c379d720 --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_macros/main/sql_macro_declared_outputs.rs @@ -0,0 +1,23 @@ +use crate::sql_macros::models::{CallScope, DeclaredOutput, SqlMacroCheck}; + +/// The select items, in any set-operation branch, of a model's top-level queries that are one call. +#[must_use] +pub fn sql_macro_declared_outputs(checks: &[SqlMacroCheck]) -> Vec { + checks + .iter() + .filter_map(|check| { + let output = check.output.as_ref()?; + let cte: Option = match &check.scope { + CallScope::Main => None, + CallScope::Cte(name) => Some(name.clone()), + CallScope::WithList => return None, + }; + Some(DeclaredOutput { + cte, + name: output.name.clone(), + returns: output.returns.clone(), + branch: output.branch, + }) + }) + .collect() +} diff --git a/crates/sqlbuild-analysis/src/sql_macros/main/sql_macro_declared_type.rs b/crates/sqlbuild-analysis/src/sql_macros/main/sql_macro_declared_type.rs new file mode 100644 index 0000000000..2e7aa03ef5 --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_macros/main/sql_macro_declared_type.rs @@ -0,0 +1,40 @@ +use crate::constants::UNKNOWN_SQL_TYPE; +use crate::sql_macros::models::DeclaredOutput; +use crate::type_system::main::normalize_type::normalize_type; +use crate::type_system::models::NormalizedType; + +/// The declared `returns` type an item takes over its inferred type: unknown or identical. +#[must_use] +pub fn sql_macro_declared_type( + declared: &[DeclaredOutput], + (cte, branch, name): (Option<&str>, usize, &str), + inferred: Option<&str>, + dialect: &str, +) -> Option { + let output: &DeclaredOutput = declared.iter().find(|output| { + output.name.eq_ignore_ascii_case(name) + && output.branch == branch + && match (&output.cte, cte) { + (None, None) => true, + (Some(declared_cte), Some(cte)) => declared_cte.eq_ignore_ascii_case(cte), + _ => false, + } + })?; + let Some(inferred) = inferred.filter(|inferred| { + !inferred.is_empty() && !inferred.eq_ignore_ascii_case(UNKNOWN_SQL_TYPE) + }) else { + return Some(output.returns.clone()); + }; + let Some(inferred_type) = normalized(inferred, dialect) else { + return Some(output.returns.clone()); + }; + (normalized(&output.returns, dialect) == Some(inferred_type)).then(|| output.returns.clone()) +} + +/// `data_type` as compared, or `None` where it does not normalise. +fn normalized(data_type: &str, dialect: &str) -> Option { + match normalize_type(data_type, dialect) { + Ok(normalization) => Some(normalization.normalized), + Err(_unnormalizable) => None, + } +} diff --git a/crates/sqlbuild-analysis/src/sql_macros/main/sql_macro_output_names.rs b/crates/sqlbuild-analysis/src/sql_macros/main/sql_macro_output_names.rs new file mode 100644 index 0000000000..5e06e4bbf5 --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_macros/main/sql_macro_output_names.rs @@ -0,0 +1,43 @@ +use crate::sql_macros::_helpers::expansion::sql_macro_calls; +use crate::sql_macros::_helpers::lexing::{Token, tokens}; +use crate::sql_macros::_helpers::placement::{CallSite, QueryParts, top_level_parts}; +use crate::sql_macros::models::{CallScope, SqlMacroCall}; + +/// The `(top-level CTE, branch, alias)` of each select item of `text` that is one SQL macro call. +#[must_use] +pub fn sql_macro_output_names(text: &str) -> Vec<(Option, usize, String)> { + let Ok(calls) = sql_macro_calls(text) else { + return Vec::new(); + }; + if calls.is_empty() { + return Vec::new(); + } + let Ok(scanned) = tokens(text) else { + return Vec::new(); + }; + let Some(parts) = top_level_parts(text, &scanned) else { + return Vec::new(); + }; + calls + .iter() + .filter(|call| !call.in_python) + .filter_map(|call| whole_item(text, (&scanned, &parts), call)) + .collect() +} + +/// The `(top-level CTE, branch, alias)` of `call` when it is a whole aliased select item. +fn whole_item( + text: &str, + (scanned, parts): (&[Token], &QueryParts), + call: &SqlMacroCall, +) -> Option<(Option, usize, String)> { + let cte: Option = match parts.scope_of(call.start) { + CallScope::Main => None, + CallScope::Cte(name) => Some(name), + CallScope::WithList => return None, + }; + let branch: usize = parts.branch_of(text, scanned, call.start)?; + CallSite::new(text, scanned, (call.start, call.end), call.replaced_end) + .whole_item_alias() + .map(|alias| (cte, branch, alias)) +} diff --git a/crates/sqlbuild-analysis/src/sql_macros/models.rs b/crates/sqlbuild-analysis/src/sql_macros/models.rs index f5d6c32ba5..8d60199c21 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/models.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/models.rs @@ -29,6 +29,8 @@ impl SqlMacroForm { pub enum ParameterKind { /// Any SQL expression of a compatible type. Expression { data_type: String }, + /// Any scalar SQL expression, whatever its type. + Any, /// A literal or a scalar constant. Constant { data_type: String }, /// An inline list or set, or a list or set constant; `set` for `SET`. @@ -46,6 +48,7 @@ impl ParameterKind { pub fn label(&self) -> String { match self { Self::Expression { data_type } => data_type.clone(), + Self::Any => "ANY".to_owned(), Self::Constant { data_type } => format!("{data_type} CONST"), Self::Collection { set, data_type } => { format!("{} {data_type}", if *set { "SET" } else { "LIST" }) @@ -253,6 +256,31 @@ pub struct SqlMacroCheck { pub arguments: Vec, /// The call's authored file location and where its text ends, when the host knows them. pub call: Option<(SqlMacroLocation, (usize, usize))>, + /// The output column an expression call is the whole select-list item of, and its type. + pub output: Option, +} + +/// A select item that is one expression macro call, typed by the macro's `returns`. +#[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +pub struct DeclaredOutput { + /// The top-level CTE the item is in, or `None` for the main query. + pub cte: Option, + /// The item's alias, unquoted. + pub name: String, + pub returns: String, + /// The select it stands in: 0, or its branch of a set operation, left to right. + pub branch: usize, +} + +/// A select-list item that is one expression macro call under an alias, as `@sql.m(x) AS name`. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct TypedOutput { + /// The alias, unquoted. + pub name: String, + /// The macro's declared `returns` type. + pub returns: String, + /// The select the item stands in: 0, or its branch of a set operation, left to right. + pub branch: usize, } /// The top-level part of the caller's query a call stands in. diff --git a/crates/sqlbuild-analysis/src/sql_macros/tests.rs b/crates/sqlbuild-analysis/src/sql_macros/tests.rs index cbdd71a321..ef65d03c09 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/tests.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/tests.rs @@ -4,5 +4,7 @@ mod helpers; mod test_definitions; #[path = "tests/test_expansion.rs"] mod test_expansion; +#[path = "tests/test_output_types.rs"] +mod test_output_types; #[path = "tests/test_types.rs"] mod test_types; diff --git a/crates/sqlbuild-analysis/src/sql_macros/tests/helpers.rs b/crates/sqlbuild-analysis/src/sql_macros/tests/helpers.rs index 8763b5a0ab..ec8f644996 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/tests/helpers.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/tests/helpers.rs @@ -4,7 +4,9 @@ use crate::sql_macros::_helpers::expansion::expand_text; use crate::sql_macros::errors::ExpansionFailure; use crate::sql_macros::errors::SqlMacroError; use crate::sql_macros::main::parse_sql_macro::parse_sql_macro; -use crate::sql_macros::models::{SqlMacroDefinition, TextContext}; +use crate::sql_macros::models::{ + CallScope, SqlMacroCheck, SqlMacroDefinition, TextContext, TypedOutput, +}; use crate::sql_macros::tests::test_types::{AsciiArguments, FakeHost}; use crate::sql_macros::types::{ConstantShape, SqlMacroHost, SqlMacroScope}; @@ -126,3 +128,19 @@ impl sqlbuild_render::macro_arguments::types::ArgumentHost for AsciiArguments { None } } + +/// A check recording output `label` of `shout()`, typed VARCHAR, at `scope`'s `branch`. +pub(super) fn typed_check(scope: CallScope, branch: usize) -> SqlMacroCheck { + SqlMacroCheck { + macro_name: "shout".to_owned(), + scope, + plain_select: true, + arguments: Vec::new(), + call: None, + output: Some(TypedOutput { + name: "label".to_owned(), + returns: "VARCHAR".to_owned(), + branch, + }), + } +} diff --git a/crates/sqlbuild-analysis/src/sql_macros/tests/test_definitions.rs b/crates/sqlbuild-analysis/src/sql_macros/tests/test_definitions.rs index c6fa3a4f3b..29c6fa4329 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/tests/test_definitions.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/tests/test_definitions.rs @@ -35,6 +35,13 @@ fn given_valid_macro_files_when_parsing_then_each_parameter_and_the_body_are_rea expected_form: "CTE-list", expected_body_position: (2, 1), }, + DefinitionTestCase { + description: "ANY parameter, with and without a default", + source: "MACRO (description \"x\", arguments (store VARCHAR, number ANY, separator ANY DEFAULT '::'), returns VARCHAR);\n@store || @separator || CAST(@number AS VARCHAR)", + expected_labels: &["VARCHAR", "ANY", "ANY"], + expected_form: "expression", + expected_body_position: (2, 1), + }, DefinitionTestCase { description: "CONST parameter as LIMIT", source: "MACRO (description \"x\", arguments (src RELATION (id BIGINT), n BIGINT CONST));\nWITH firsts AS (SELECT id FROM @src LIMIT @n)", @@ -180,6 +187,30 @@ fn given_invalid_macro_files_when_parsing_then_each_is_rejected() { expected_code: "P015", expected_message: "argument codes LIST VARCHAR CONST: a LIST argument is always known at compile time, so it takes no CONST", }, + DefinitionErrorTestCase { + description: "ANY with CONST", + source: "MACRO (description \"x\", arguments (n ANY CONST), returns BIGINT);\n@n", + expected_code: "P015", + expected_message: "argument n ANY CONST: a CONST argument needs a type its values are checked against", + }, + DefinitionErrorTestCase { + description: "ANY with more after it", + source: "MACRO (description \"x\", arguments (n ANY[]), returns BIGINT);\n@n", + expected_code: "P015", + expected_message: "argument n ANY takes nothing after it", + }, + DefinitionErrorTestCase { + description: "LIST of ANY", + source: "MACRO (description \"x\", arguments (codes LIST ANY), returns BOOLEAN);\n'a' IN @codes", + expected_code: "P015", + expected_message: "argument codes LIST ANY: ANY is for scalar arguments only", + }, + DefinitionErrorTestCase { + description: "ANY parameter as LIMIT", + source: "MACRO (description \"x\", arguments (src RELATION (id BIGINT), n ANY));\nWITH firsts AS (SELECT id FROM @src LIMIT @n)", + expected_code: "P023", + expected_message: "n is used where a literal is required; declare it with a type and CONST, such as n VARCHAR CONST", + }, DefinitionErrorTestCase { description: "array type with CONST", source: "MACRO (description \"x\", arguments (codes VARCHAR[] CONST), returns BOOLEAN);\nlen(@codes) > 0", diff --git a/crates/sqlbuild-analysis/src/sql_macros/tests/test_expansion.rs b/crates/sqlbuild-analysis/src/sql_macros/tests/test_expansion.rs index 167a3f279f..6307803193 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/tests/test_expansion.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/tests/test_expansion.rs @@ -117,6 +117,10 @@ const SPACED_DECLARATIONS: (&str, &str) = ( "spaced", "MACRO (description \"Spaced\", arguments (status VARCHAR, places INT), returns BOOLEAN);\n\n@status <> @enum (\"order_status\").CLOSED AND @places = @const (\"price_places\")\n", ); +const ORDER_KEY: (&str, &str) = ( + "order_key", + "MACRO (description \"Order key\", arguments (store_code VARCHAR, order_number ANY), returns VARCHAR);\n\n@store_code || '::' || CAST(@order_number AS VARCHAR)\n", +); const ODD: (&str, &str) = ( "odd", "MACRO (description \"Odd\", arguments (v FOO BAR), returns BIGINT);\n\n@v + 1\n", @@ -349,6 +353,13 @@ fn given_calls_of_each_form_when_expanding_then_the_ticket_examples_compile() { sql: "SELECT @sql.has_x(o.a) AS l FROM orders AS o", expected_sql: "SELECT CONTAINS(o.a, 'x') -- why x\n AS l FROM orders AS o", }, + ExpansionTestCase { + description: "ANY arguments of any type: a column, a number, a string, an expression and a scalar constant", + adapter: "duckdb", + macros: &[ORDER_KEY], + sql: "SELECT @sql.order_key(o.store, o.number) AS a, @sql.order_key(o.store, 42) AS b, @sql.order_key(o.store, 'x') AS c, @sql.order_key(o.store, o.number + 1) AS d, @sql.order_key(o.store, @const(\"price_places\")) AS e FROM orders AS o", + expected_sql: "SELECT o.store || '::' || CAST(o.number AS VARCHAR) AS a, o.store || '::' || CAST(42 AS VARCHAR) AS b, o.store || '::' || CAST('x' AS VARCHAR) AS c, o.store || '::' || CAST((o.number + 1) AS VARCHAR) AS d, o.store || '::' || CAST(2 AS VARCHAR) AS e FROM orders AS o", + }, ExpansionTestCase { description: "a body ending in a line comment, called last on its line, adds no line", adapter: "", @@ -522,6 +533,30 @@ fn given_invalid_calls_when_expanding_then_each_error_names_the_problem() { expected_code: "P017", expected_message: "argument factor of macro scaled() expects INT CONST", }, + ExpansionErrorTestCase { + description: "a list passed to an ANY argument", + adapter: "", + macros: &[ORDER_KEY], + sql: "SELECT @sql.order_key(o.store, [1, 2]) AS k FROM orders AS o", + expected_code: "P017", + expected_message: "argument order_number of macro order_key() expects ANY: a scalar SQL expression; got [1, 2]", + }, + ExpansionErrorTestCase { + description: "a list constant passed to an ANY argument", + adapter: "", + macros: &[ORDER_KEY], + sql: "SELECT @sql.order_key(o.store, @const(\"supported_countries\")) AS k FROM orders AS o", + expected_code: "P017", + expected_message: "argument order_number of macro order_key() expects ANY: a scalar constant", + }, + ExpansionErrorTestCase { + description: "a relation passed to an ANY argument", + adapter: "", + macros: &[ORDER_KEY], + sql: "SELECT @sql.order_key(o.store, __ref(\"orders\")) AS k FROM orders AS o", + expected_code: "P017", + expected_message: "argument order_number of macro order_key() expects ANY: a scalar SQL expression", + }, ExpansionErrorTestCase { description: "type the adapter cannot parse", adapter: "duckdb", diff --git a/crates/sqlbuild-analysis/src/sql_macros/tests/test_output_types.rs b/crates/sqlbuild-analysis/src/sql_macros/tests/test_output_types.rs new file mode 100644 index 0000000000..9861f19e11 --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_macros/tests/test_output_types.rs @@ -0,0 +1,143 @@ +use crate::sql_macros::main::sql_macro_declared_outputs::sql_macro_declared_outputs; +use crate::sql_macros::main::sql_macro_declared_type::sql_macro_declared_type; +use crate::sql_macros::main::sql_macro_output_names::sql_macro_output_names; +use crate::sql_macros::models::CallScope; +use crate::sql_macros::tests::helpers::typed_check; +use crate::sql_macros::tests::test_types::{OutputNamesTestCase, OutputTypeTestCase}; + +#[test] +fn given_a_typed_macro_output_when_typing_the_column_then_returns_fills_unknown_and_refines() { + let test_cases = [ + OutputTypeTestCase { + description: "unknown inferred type takes returns", + check: typed_check(CallScope::Main, 0), + item: (None, 0, "label", None), + expected_type: Some("VARCHAR"), + }, + OutputTypeTestCase { + description: "an identical type takes returns' spelling", + check: typed_check(CallScope::Main, 0), + item: (None, 0, "LABEL", Some("TEXT")), + expected_type: Some("VARCHAR"), + }, + OutputTypeTestCase { + description: "a different known family keeps the inferred type", + check: typed_check(CallScope::Main, 0), + item: (None, 0, "label", Some("BIGINT")), + expected_type: None, + }, + OutputTypeTestCase { + description: "a longer text type stays", + check: typed_check(CallScope::Main, 0), + item: (None, 0, "label", Some("VARCHAR(100)")), + expected_type: None, + }, + OutputTypeTestCase { + description: "the unknown placeholder takes returns", + check: typed_check(CallScope::Main, 0), + item: (None, 0, "label", Some("UNKNOWN")), + expected_type: Some("VARCHAR"), + }, + OutputTypeTestCase { + description: "a known time type is not unknown", + check: typed_check(CallScope::Main, 0), + item: (None, 0, "label", Some("TIME")), + expected_type: None, + }, + OutputTypeTestCase { + description: "a known array type is not unknown", + check: typed_check(CallScope::Main, 0), + item: (None, 0, "label", Some("TEXT[]")), + expected_type: None, + }, + OutputTypeTestCase { + description: "another column is untouched", + check: typed_check(CallScope::Main, 0), + item: (None, 0, "other", None), + expected_type: None, + }, + OutputTypeTestCase { + description: "a call in a CTE types that CTE's item", + check: typed_check(CallScope::Cte("base".to_owned()), 0), + item: (Some("BASE"), 0, "label", None), + expected_type: Some("VARCHAR"), + }, + OutputTypeTestCase { + description: "a call in a CTE leaves a main-query item of that name", + check: typed_check(CallScope::Cte("base".to_owned()), 0), + item: (None, 0, "label", None), + expected_type: None, + }, + OutputTypeTestCase { + description: "a call in another branch of a set operation leaves this one", + check: typed_check(CallScope::Main, 1), + item: (None, 0, "label", None), + expected_type: None, + }, + OutputTypeTestCase { + description: "a call in a later branch of a set operation types that branch", + check: typed_check(CallScope::Cte("base".to_owned()), 1), + item: (Some("base"), 1, "label", Some("TEXT")), + expected_type: Some("VARCHAR"), + }, + ]; + + for test_case in test_cases { + let (cte, branch, name, inferred) = test_case.item; + let declared = sql_macro_declared_outputs(&[test_case.check]); + let typed = sql_macro_declared_type(&declared, (cte, branch, name), inferred, "duckdb"); + + assert_eq!( + typed.as_deref(), + test_case.expected_type, + "{}", + test_case.description + ); + } +} + +#[test] +fn given_model_sql_when_reading_sql_macro_outputs_then_only_whole_aliased_items_count() { + let test_cases = [ + OutputNamesTestCase { + description: "aliased whole items, with and without AS", + sql: "SELECT @sql.a(x) AS one, @sql.b(y) two, @sql.c(z) AS \"Three\" FROM t", + expected_names: &[(None, 0, "one"), (None, 0, "two"), (None, 0, "Three")], + }, + OutputNamesTestCase { + description: "calls inside expressions or without an alias", + sql: "SELECT @sql.a(x) || 'x' AS one, UPPER(@sql.b(y)) AS two, @sql.c(z) FROM t", + expected_names: &[], + }, + OutputNamesTestCase { + description: "a call in a CTE, and ones in each branch of a set operation", + sql: "WITH c AS (SELECT @sql.a(x) AS one FROM t) SELECT @sql.b(y) AS two FROM c UNION ALL SELECT 'x' FROM c UNION ALL SELECT @sql.c(z) AS three FROM c", + expected_names: &[(Some("c"), 0, "one"), (None, 0, "two"), (None, 2, "three")], + }, + OutputNamesTestCase { + description: "a star EXCEPT modifier is not a set operation", + sql: "SELECT * EXCEPT (a) FROM t UNION ALL SELECT @sql.b(y) AS two FROM t", + expected_names: &[(None, 1, "two")], + }, + OutputNamesTestCase { + description: "calls in subqueries, a nested WITH or beside a parenthesised branch", + sql: "WITH c AS (WITH d AS (SELECT 1 AS x) SELECT @sql.a(x) AS one FROM d), e AS (SELECT * FROM (SELECT @sql.b(y) AS two FROM t) AS s) SELECT @sql.c(z) AS three FROM c UNION ALL (SELECT 'x' FROM e)", + expected_names: &[], + }, + OutputNamesTestCase { + description: "a call in the main query after CTEs", + sql: "WITH c AS (SELECT x FROM t) SELECT x, @sql.a(x) AS one FROM c", + expected_names: &[(None, 0, "one")], + }, + ]; + + for test_case in test_cases { + let names: Vec<(Option, usize, String)> = sql_macro_output_names(test_case.sql); + let found: Vec<(Option<&str>, usize, &str)> = names + .iter() + .map(|(cte, branch, name)| (cte.as_deref(), *branch, name.as_str())) + .collect(); + + assert_eq!(found, test_case.expected_names, "{}", test_case.description); + } +} diff --git a/crates/sqlbuild-analysis/src/sql_macros/tests/test_types.rs b/crates/sqlbuild-analysis/src/sql_macros/tests/test_types.rs index 493f530145..47970c78e9 100644 --- a/crates/sqlbuild-analysis/src/sql_macros/tests/test_types.rs +++ b/crates/sqlbuild-analysis/src/sql_macros/tests/test_types.rs @@ -62,3 +62,24 @@ pub(super) struct PythonArgumentTestCase { /// Argument parsing without Unicode name lookups, which these arguments never use. pub(super) struct AsciiArguments; + +/// One recorded call, one analysed select item and the type the item then has, if changed. +pub(super) struct OutputTypeTestCase { + pub(super) description: &'static str, + pub(super) check: crate::sql_macros::models::SqlMacroCheck, + /// The item's CTE (`None` for the main query), set-operation branch, name and inferred type. + pub(super) item: ( + Option<&'static str>, + usize, + &'static str, + Option<&'static str>, + ), + pub(super) expected_type: Option<&'static str>, +} + +/// Model SQL and the scope and alias of each whole-item SQL macro call. +pub(super) struct OutputNamesTestCase { + pub(super) description: &'static str, + pub(super) sql: &'static str, + pub(super) expected_names: &'static [(Option<&'static str>, usize, &'static str)], +} diff --git a/crates/sqlbuild-rules/src/engine/tests/helpers.rs b/crates/sqlbuild-rules/src/engine/tests/helpers.rs index 4bba084546..43e315b7de 100644 --- a/crates/sqlbuild-rules/src/engine/tests/helpers.rs +++ b/crates/sqlbuild-rules/src/engine/tests/helpers.rs @@ -77,6 +77,10 @@ pub(crate) fn scope_with_macros(paths: &[String]) -> Value { .enumerate() .map(|(index, path)| { let relative = path.trim_start_matches("models/commerce/_macros/"); + let relative = relative + .strip_prefix("python/") + .or_else(|| relative.strip_prefix("sql/")) + .unwrap_or(relative); let bucket_path = std::path::Path::new(relative) .parent() .and_then(std::path::Path::to_str) diff --git a/crates/sqlbuild-rules/src/engine/tests/test_domain_layout.rs b/crates/sqlbuild-rules/src/engine/tests/test_domain_layout.rs index 228a398f4f..3a4979e8d3 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_domain_layout.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_domain_layout.rs @@ -12,7 +12,72 @@ fn given_owner_and_declaration_layouts_when_evaluating_then_returns_expected_fau let repeated_paths: Vec = (0..11) .map(|_| "models/commerce/_macros/orders.py".to_owned()) .collect(); + let python_and_sql: Vec = (0..6) + .map(|index| format!("models/commerce/_macros/python/normalise/item_{index}.py")) + .chain( + (0..7).map(|index| format!("models/commerce/_macros/sql/normalise/item_{index}.sql")), + ) + .collect(); + let oversized_sql: Vec = (0..11) + .map(|index| format!("models/commerce/_macros/sql/normalise/item_{index}.sql")) + .collect(); let test_cases = [ + test_types::DomainLayoutTestCase { + description: "python and sql macro kind folders are counted apart", + code: "SQBRDECLARATION304", + models: json!([]), + thresholds: json!({}), + layout: json!({}), + scope_index: helpers::scope_with_macros(&python_and_sql), + expected_codes: &[], + expected_message_fragments: &[], + expected_absent_fragments: &[], + }, + test_types::DomainLayoutTestCase { + description: "an oversized sql macro kind folder names its real folder", + code: "SQBRDECLARATION304", + models: json!([]), + thresholds: json!({}), + layout: json!({}), + scope_index: helpers::scope_with_macros(&oversized_sql), + expected_codes: &["SQBRDECLARATION304"], + expected_message_fragments: &[ + "declaration container \"models/commerce/_macros/sql/normalise\" has 11 files", + ], + expected_absent_fragments: &[], + }, + test_types::DomainLayoutTestCase { + description: "a shared prefix across kind folders is no fault", + code: "SQBRDECLARATION306", + models: json!([]), + thresholds: json!({}), + layout: json!({}), + scope_index: helpers::scope_with_macros(&[ + "models/commerce/_macros/python/normalise_order.py".into(), + "models/commerce/_macros/sql/normalise_person.sql".into(), + ]), + expected_codes: &[], + expected_message_fragments: &[], + expected_absent_fragments: &[], + }, + test_types::DomainLayoutTestCase { + description: "sql macro files sharing a prefix move into a subfolder of sql", + code: "SQBRDECLARATION306", + models: json!([]), + thresholds: json!({}), + layout: json!({}), + scope_index: helpers::scope_with_macros(&[ + "models/commerce/_macros/sql/normalise_order.sql".into(), + "models/commerce/_macros/sql/normalise_person.sql".into(), + ]), + expected_codes: &["SQBRDECLARATION306"], + expected_message_fragments: &[ + "2 sibling SQL macro files", + "Move these files into models/commerce/_macros/sql/normalise/", + "each macro keeps its name", + ], + expected_absent_fragments: &[], + }, test_types::DomainLayoutTestCase { description: "leaf and branch models fault", code: "SQBRPROJECT202", @@ -226,7 +291,13 @@ fn given_owner_and_declaration_layouts_when_evaluating_then_returns_expected_fau .collect(); let messages = faults .iter() - .filter_map(|fault| fault["message"].as_str()) + .map(|fault| { + format!( + "{} {}", + fault["message"].as_str().unwrap_or_default(), + fault["remediation"].as_str().unwrap_or_default() + ) + }) .collect::>() .join("\n"); diff --git a/crates/sqlbuild-rules/src/rules/_helpers/domain_layout.rs b/crates/sqlbuild-rules/src/rules/_helpers/domain_layout.rs index 4b24a49267..a39aa84b97 100644 --- a/crates/sqlbuild-rules/src/rules/_helpers/domain_layout.rs +++ b/crates/sqlbuild-rules/src/rules/_helpers/domain_layout.rs @@ -10,6 +10,10 @@ const DEFAULT_MAX_ROLE_CONTAINER_DEPTH: u32 = 1; const DEFAULT_MAX_CONTAINER_FILES: u32 = 10; const MIN_MODEL_PATH_COMPONENTS: usize = 3; const MIN_BRANCH_OUTCOMES: usize = 2; +/// The kind folders a macro folder holds, each counted as its own container. +const MACRO_KIND_FOLDERS: [&str; 2] = ["python", "sql"]; +/// The kind folder of SQL macro files, whose names are their file stems. +const SQL_MACRO_KIND_FOLDER: &str = "sql"; const GENERIC_BUCKET_NAMES: [&str; 8] = [ "common", "helper", "helpers", "misc", "shared", "util", "utils", "macros", ]; @@ -379,8 +383,8 @@ fn inspect_owner( faults.extend(prefix_faults( rule, &siblings, - inspection.min_prefixes, - "owner directories", + (inspection.min_prefixes, "owner directories"), + shared_prefix_remedy, )); } for (name, child) in &node.children { @@ -412,7 +416,12 @@ fn inspect_prefix_owners( } faults.extend(inspect_prefix_owners(rule, child, minimum, label)); } - faults.extend(prefix_faults(rule, &siblings, minimum, label)); + faults.extend(prefix_faults( + rule, + &siblings, + (minimum, label), + shared_prefix_remedy, + )); faults } @@ -466,6 +475,16 @@ fn container_entry(declaration: &ScopeDeclaration) -> Option<(String, ContainerE let stem = file .rsplit_once('.') .map_or_else(|| file.to_owned(), |(value, _)| value.into()); + let kind_folder: Option<&str> = matches!(declaration.kind, DeclarationKind::Macro) + .then_some(()) + .and(declaration.path.strip_prefix(&format!("{root}/"))) + .and_then(|rest| rest.split_once('/')) + .map(|(folder, _)| folder) + .filter(|folder| MACRO_KIND_FOLDERS.contains(folder)); + let root: String = match kind_folder { + Some(folder) => format!("{root}/{folder}"), + None => root, + }; Some(( root, ContainerEntry { @@ -597,12 +616,39 @@ fn inspect_container( .or_default() .push((entry.stem.clone(), entry.path.clone())); } - for siblings in locations.values() { + let sql_macros: bool = inspection + .root + .rsplit('/') + .next() + .is_some_and(|folder| folder == SQL_MACRO_KIND_FOLDER) + && files + .iter() + .all(|entry| matches!(entry.kind, DeclarationKind::Macro)); + for (location, siblings) in &locations { + let folder: String = if location.is_empty() { + inspection.root.to_owned() + } else { + format!("{}/{location}", inspection.root) + }; + let label: &str = if sql_macros { + "SQL macro files" + } else { + "declaration files" + }; + let remedy = |prefix: &str| { + if sql_macros { + format!( + "Move these files into {folder}/{prefix}/; a SQL macro is named by its file, so each macro keeps its name and its callers stay unchanged." + ) + } else { + shared_prefix_remedy(prefix) + } + }; faults.extend(prefix_faults( rule, siblings, - inspection.min_prefixes, - "declaration files", + (inspection.min_prefixes, label), + remedy, )); } } @@ -623,11 +669,18 @@ fn container_limit(evaluation: &ProjectEvaluationRequest<'_>, kind: &Declaration threshold(evaluation, name, DEFAULT_MAX_CONTAINER_FILES) } +/// The remedy for siblings that share a token prefix. +fn shared_prefix_remedy(prefix: &str) -> String { + format!( + "Consolidate these siblings beneath {prefix}/, or rename them when the shared token prefix is not a real concern." + ) +} + fn prefix_faults( rule: &RuleMetadata, siblings: &[(String, String)], - minimum: u32, - label: &str, + (minimum, label): (u32, &str), + remedy: impl Fn(&str) -> String, ) -> Vec { let mut root = TokenNode::default(); for (name, path) in siblings { @@ -644,12 +697,8 @@ fn prefix_faults( path_fault( rule, &path, - format!( - "{count} sibling {label} encode the implicit owner or bucket {prefix:?}" - ), - format!( - "Consolidate these siblings beneath {prefix}/, or rename them when the shared token prefix is not a real concern." - ), + format!("{count} sibling {label} encode the implicit owner or bucket {prefix:?}"), + remedy(&prefix), ) }) .collect() diff --git a/crates/sqlbuild-rules/src/rules/_helpers/explicit_output_types.rs b/crates/sqlbuild-rules/src/rules/_helpers/explicit_output_types.rs index 0002d70e6d..d760285923 100644 --- a/crates/sqlbuild-rules/src/rules/_helpers/explicit_output_types.rs +++ b/crates/sqlbuild-rules/src/rules/_helpers/explicit_output_types.rs @@ -1,3 +1,7 @@ +use std::cell::OnceCell; + +use sqlbuild_analysis::sql_macros::main::sql_macro_output_names::sql_macro_output_names; + use crate::constants::ENFORCED_CONTRACT; use crate::models::{Column, Fault, Model, RuleMetadata}; use crate::query_facts::models::{ItemFacts, QueryFacts, SelectFacts, SetFacts}; @@ -11,6 +15,10 @@ struct OutputScope<'a> { columns: &'a [Column], set_branch: bool, by_name: bool, + /// Where SQL macro call items are read: the main query (`Some(None)`), a CTE, or nowhere. + macro_scope: Option>, + /// The first select's branch of the scope's set operation, left to right. + branch: usize, } struct OutputRule<'a> { @@ -18,6 +26,8 @@ struct OutputRule<'a> { model: &'a Model, rule: &'a RuleMetadata, faults: &'a FaultCollector, + /// The main query's outputs that are one SQL macro call each, read when first needed. + sql_macro_outputs: OnceCell, usize, String)>>, } pub(crate) fn evaluate(parsed: &ParsedModel<'_>, rule: &RuleMetadata, faults: &FaultCollector) { @@ -44,14 +54,18 @@ pub(crate) fn evaluate(parsed: &ParsedModel<'_>, rule: &RuleMetadata, faults: &F model, rule, faults, + sql_macro_outputs: OnceCell::new(), }; if let Some((boundary, columns)) = final_cte_output_boundary(query, &model.columns) { + let final_cte: Option<&str> = query.ctes.last().map(|cte| cte.name.as_str()); evaluator.evaluate_set( &boundary.body, OutputScope { columns: &columns, set_branch: false, by_name: false, + macro_scope: final_cte.map(Some), + branch: 0, }, ); return; @@ -63,6 +77,8 @@ pub(crate) fn evaluate(parsed: &ParsedModel<'_>, rule: &RuleMetadata, faults: &F columns: &columns, set_branch: false, by_name: false, + macro_scope: Some(None), + branch: 0, }, ); } @@ -126,9 +142,17 @@ impl OutputRule<'_> { columns: scope.columns, set_branch: true, by_name: *by_name, + macro_scope: scope.macro_scope, + branch: scope.branch, }; self.evaluate_set(left, branch_scope); - self.evaluate_set(right, branch_scope); + self.evaluate_set( + right, + OutputScope { + branch: scope.branch + set_branches(left), + ..branch_scope + }, + ); } SetFacts::Other => self.push( None, @@ -185,7 +209,11 @@ impl OutputRule<'_> { .cast_type .as_ref() .is_some_and(|data_type| type_spelling_equal(data_type, &column.data_type)); - let passes = cast_matches_contract || column.type_proven && direct && !scope.set_branch; + let passes = cast_matches_contract + || column.type_proven && direct && !scope.set_branch + || column.type_proven + && !explicit_cast + && self.sql_macro_output(scope, output_name.as_deref()); if passes { continue; } @@ -213,6 +241,25 @@ impl OutputRule<'_> { } } + /// Whether output `name` in `scope`'s select is one SQL macro call, typed by its `returns`. + fn sql_macro_output(&self, scope: OutputScope<'_>, name: Option<&str>) -> bool { + let (Some(macro_scope), Some(name)) = (scope.macro_scope, name) else { + return false; + }; + self.sql_macro_outputs + .get_or_init(|| sql_macro_output_names(&self.model.authored_sql)) + .iter() + .any(|(cte, branch, output)| { + output.eq_ignore_ascii_case(name) + && *branch == scope.branch + && match (cte.as_deref(), macro_scope) { + (None, None) => true, + (Some(cte), Some(scope)) => cte.eq_ignore_ascii_case(scope), + _ => false, + } + }) + } + fn push(&self, offset: Option, message: String, remediation: String) { let (line, column) = offset.map_or((1, 1), |offset| self.parsed.source.line_column(offset)); self.faults.push(Fault { @@ -227,6 +274,14 @@ impl OutputRule<'_> { } } +/// The selects of `set_expr`'s set operation, a nested query counting as one. +fn set_branches(set_expr: &SetFacts) -> usize { + match set_expr { + SetFacts::SetOperation { left, right, .. } => set_branches(left) + set_branches(right), + SetFacts::Select(_) | SetFacts::Query(_) | SetFacts::Other => 1, + } +} + fn proven_dependency_wildcard(select: &SelectFacts, columns: &[Column]) -> bool { plain_select(select, true) && reads_dependency(select) diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/macros/sql-macros.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/macros/sql-macros.md index c0e2718f54..5ccc5999da 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/macros/sql-macros.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/macros/sql-macros.md @@ -49,6 +49,11 @@ my_project/ A file directly under a `macros/` folder fails with `D013`, which names the subfolder to move it to. Folder scopes use the same layout, such as `models/marts/_sqlbuild/_macros/sql/`. +Rules that bound declaration folders, such as `SQBRDECLARATION304` (files per folder) and +`SQBRDECLARATION306` (shared filename prefixes), count `python/` and `sql/` as separate folders. To +split a full `sql/` folder, move files into subfolders such as `macros/sql/text/`: a SQL macro is +named by its file, so it keeps its name and its callers stay unchanged. + A SQL macro file starts with a `MACRO` header, then the body: ```sql @@ -151,6 +156,7 @@ Every parameter has a type, and may have a `DEFAULT`: | Type | Accepts | |---|---| | `BIGINT`, `VARCHAR`, `TIMESTAMP`, ... | Any SQL expression of a compatible type, evaluated per row: a column, arithmetic, a function call, a literal or a constant. | +| `ANY` | Any scalar SQL expression, whatever its type: a column, a literal, an expression or a scalar constant. Its type is not checked. Lists and relations fail with `P017`. | | ` CONST` | Only a value known at compile time: a literal, a scalar constant, an enum value, or a `CONST` parameter of a calling macro. A column or expression fails with `P017`. Scalar types only. | | `LIST `, `SET ` | An inline list or set, or a list or set constant. Always compile-time, so they take no `CONST`. They render like constants. | | `ENUM ` | A member of that enum, such as `@enum("order_status").OPEN`. | @@ -161,6 +167,13 @@ SQL types are written as in [schema declarations](../models/schemas.md) and chec adapter's SQL dialect the same way, including the dialect's array types such as `VARCHAR[]`. A runtime array value uses the array type; `LIST VARCHAR` is a list known at compile time. +Use `ANY` when the body works for values of any type, typically because it casts the argument +itself, as in `CAST(@order_ref AS VARCHAR)`, so callers can pass a number or a string. The body is +still checked, and the argument's type counts as unknown there. `ANY` is not `VARIANT`: a +`VARIANT` parameter says the macro takes semi-structured data, while `ANY` says it takes a plain +value of any type. `ANY` takes a `DEFAULT` but not `CONST`, which needs a type to check values +against, and it cannot be a `LIST` or `SET` element type. + Leave `CONST` off unless the body needs a literal. `CONST` is part of the macro's contract: callers can no longer pass a column. @@ -264,6 +277,19 @@ parameters of a calling macro, and calls in models with runtime placeholders; th column checks still run on the expanded SQL. All other errors also point at the call and at the macro file. +A select-list item that is one expression macro call under an alias, as in +`@sql.cents_to_dollars(o.amount_cents) AS amount`, has the macro's `returns` type wherever the +compiler infers no type for the expanded SQL, or exactly that type. This holds in the main query +and in any top-level CTE, including each branch of their `UNION`, `INTERSECT` or `EXCEPT`, and the +type flows on as an inferred type would: through `SELECT * FROM final`, pass-through columns and +joins, to contract checks (`K002`, `K003`), `SQBRCONTRACT105` and models that read the column. A +set operation's column combines its branches as usual, so a sibling branch of another type still +leaves it unproven, and a CTE holding one is read by column name, since `SELECT *` over it is not +typed. Such a call needs no `CAST` to satisfy a matching contract column. Any other +inferred type stands, even a close one such as `VARCHAR(100)` for `returns VARCHAR(5)`, and `K002` +reports the mismatch. A call inside a larger expression, a subquery, a nested `WITH` or a set +operation with a parenthesised branch is typed from the expanded SQL as before. + - `sqb format` formats SQL macro files, bodies of every form and the `MACRO` header, and normalises the commas around CTE-list and column-list calls. - [Rules](../rules.md) and [column lineage](../column-lineage.md) read the expanded diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_macros/helpers.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_macros/helpers.py index 19e9278d05..01e92ccaca 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_macros/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_macros/helpers.py @@ -205,3 +205,50 @@ def check_macro(arguments: str, body: str) -> dict[str, str]: f" returns BOOLEAN,\n);\n\n{body}\n" ) } + + +def python_normalise_macros(names: list[str]) -> dict[str, str]: + """One Python macro file per name in the `normalise` bucket of `macros/python/`.""" + + return { + f"macros/python/normalise/{name}.py": ( + f"def {name}(expression: str) -> str:\n" + f' """Clean a value with {name}."""\n' + ' return f"TRIM({expression})"\n' + ) + for name in names + } + + +def sql_text_macros(folder: str, names: list[str]) -> dict[str, str]: + """One lower-casing SQL macro file per name in `macros/sql//`.""" + + return { + f"macros/sql/{folder}/{name}.sql": ( + f'MACRO (description "Tidy {name}", arguments (value VARCHAR), returns VARCHAR);\n\n' + "LOWER(@value)\n" + ) + for name in names + } + + +def text_macro(name: str, returns: str, body: str) -> dict[str, str]: + """The SQL macro `name` taking one VARCHAR value and declaring `returns`.""" + + return { + f"macros/sql/{name}.sql": ( + f'MACRO (\n description "Text from a value",\n arguments (value VARCHAR),\n' + f" returns {returns},\n);\n\n{body}\n" + ) + } + + +def contract_model(name: str, columns: str, sql: str) -> dict[str, str]: + """The mart model `name` with an enforced contract over `columns`.""" + + return { + f"models/marts/{name}.sql": ( + f'MODEL (\n description "Typed {name}",\n contract enforced,\n' + f" columns ({columns}),\n);\n\n{sql}" + ) + } diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_macros/test_sql_macro_layout_rules.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_macros/test_sql_macro_layout_rules.py new file mode 100644 index 0000000000..0a978ee07d --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_macros/test_sql_macro_layout_rules.py @@ -0,0 +1,77 @@ +"""E2E coverage for declaration layout Rules over a macro folder's python/ and sql/ kind folders.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.compile.sql_macros._test_types import ( + SqlMacroCommandTestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.compile.sql_macros.helpers import ( + output, + python_normalise_macros, + run_project, + sql_text_macros, + with_rules, +) + +_LAYOUT_RULES: dict[str, str] = with_rules("SQBRDECLARATION304", "SQBRDECLARATION306") + + +@pytest.mark.parametrize( + "test_case", + [ + SqlMacroCommandTestCase( + description="six_python_and_seven_sql_files_fit_their_own_folders", + files={ + **_LAYOUT_RULES, + **python_normalise_macros(["trim", "upper", "pad", "strip", "fold", "squash"]), + **sql_text_macros( + "normalise", ["lower", "title", "slug", "ascii", "spaces", "digits", "letters"] + ), + }, + command=("compile", "--no-cache"), + expected_returncode=0, + expected_fragments=("0 errors",), + ), + SqlMacroCommandTestCase( + description="eleven_sql_files_overflow_their_real_folder", + files={ + **_LAYOUT_RULES, + **sql_text_macros("normalise", [f"tidy_{word}" for word in "abcdefghijk"]), + }, + command=("compile", "--no-cache"), + expected_returncode=1, + expected_fragments=( + "error[SQBRDECLARATION304]", + 'declaration container "macros/sql/normalise" has 11 files', + ), + ), + SqlMacroCommandTestCase( + description="sql_files_sharing_a_prefix_move_into_a_subfolder_of_sql", + files={ + **_LAYOUT_RULES, + **sql_text_macros("text", ["name_first", "name_last"]), + }, + command=("compile", "--no-cache"), + expected_returncode=1, + expected_fragments=( + "error[SQBRDECLARATION306]", + "2 sibling SQL macro files", + "Move these files into macros/sql/text/name/", + "each macro keeps its name", + ), + ), + ], + ids=lambda case: case.description, +) +def test_given_macro_kind_folders_when_checking_layout_rules_then_each_folder_counts_alone( + test_case: SqlMacroCommandTestCase, tmp_path: Path +) -> None: + _, result = run_project(tmp_path=tmp_path, files=test_case.files, command=test_case.command) + + assert result.returncode == test_case.expected_returncode, output(result) + for fragment in test_case.expected_fragments: + assert fragment in output(result) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_macros/test_sql_macro_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_macros/test_sql_macro_types.py new file mode 100644 index 0000000000..9620ef8e76 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_macros/test_sql_macro_types.py @@ -0,0 +1,506 @@ +"""E2E coverage for ANY SQL macro arguments and the declared returns type of a call.""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.compile.sql_macros._test_types import ( + SqlMacroBuildTestCase, + SqlMacroCommandTestCase, + SqlMacroCompileTestCase, + SqlMacroErrorTestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.compile.sql_macros.helpers import ( + DATABASE, + contract_model, + orders_model, + output, + run_project, + text_macro, + with_rules, +) +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import query_duckdb, run_sqb + +_ORDER_KEY: dict[str, str] = { + "macros/sql/order_key.sql": ( + 'MACRO (\n description "Order key from the country and the order",\n' + " arguments (country_code VARCHAR, order_ref ANY),\n" + " returns VARCHAR,\n);\n\n" + "@country_code || '::' || CAST(@order_ref AS VARCHAR)\n" + ) +} + +_UNPROVEN: tuple[str, ...] = ("K003", "SQBRCONTRACT105") +_TYPED_COLUMNS: str = "order_id (type BIGINT), shouted (type VARCHAR), status_bar (type VARCHAR)" +_TYPED_RULES_AND_MACROS: dict[str, str] = { + **with_rules("SQBRCONTRACT105"), + **text_macro("shouted", "VARCHAR", "UPPER(@value)"), + **text_macro("status_bar", "VARCHAR", "bar(LENGTH(@value), 0, 10)"), +} +_TYPED_CALLS: dict[str, str] = { + **_TYPED_RULES_AND_MACROS, + **contract_model( + "orders", + _TYPED_COLUMNS, + "SELECT\n o.order_id,\n @sql.shouted(o.status) AS shouted,\n" + " @sql.status_bar(o.status) AS status_bar\n" + 'FROM __ref("stg_orders") AS o\n', + ), + **contract_model( + "order_bars", + "status_bar (type VARCHAR)", + 'SELECT b.status_bar FROM __ref("orders") AS b\n', + ), +} + +_UNION_CALLS: dict[str, str] = { + **_TYPED_RULES_AND_MACROS, + **contract_model( + "orders", + "order_id (type BIGINT), status_bar (type VARCHAR)", + "WITH final AS (\n" + " SELECT CAST(o.order_id AS BIGINT) AS order_id,\n" + " @sql.status_bar(o.status) AS status_bar\n" + ' FROM __ref("stg_orders") AS o\n' + " UNION ALL\n" + " SELECT CAST(o.order_id AS BIGINT) AS order_id,\n" + " CAST(NULL AS VARCHAR) AS status_bar\n" + ' FROM __ref("stg_orders") AS o\n' + ")\nSELECT f.order_id, f.status_bar FROM final AS f\n", + ), +} + + +@pytest.mark.parametrize( + "test_case", + [ + SqlMacroBuildTestCase( + description="any_argument_takes_a_number_a_string_and_an_expression", + files={ + **_ORDER_KEY, + **orders_model( + "SELECT order_id,\n" + " @sql.order_key(country_code, order_id) AS by_number,\n" + " @sql.order_key(country_code, status) AS by_text,\n" + " @sql.order_key(country_code, amount_cents / 100) AS by_expression\n" + 'FROM __ref("stg_orders")\nWHERE order_id = 2\n' + ), + }, + query="SELECT by_number, by_text, by_expression FROM orders", + expected_rows=[("FR::2", "FR::closed", "FR::5.0")], + ), + ], + ids=lambda case: case.description, +) +def test_given_an_any_argument_when_building_then_callers_pass_values_of_any_type( + test_case: SqlMacroBuildTestCase, tmp_path: Path +) -> None: + project_dir, result = run_project(tmp_path=tmp_path, files=test_case.files, command=("build",)) + + assert result.returncode == 0, output(result) + assert "P017" not in output(result) + assert ( + query_duckdb(db_path=project_dir / DATABASE, sql=test_case.query) == test_case.expected_rows + ) + + +@pytest.mark.parametrize( + "test_case", + [ + SqlMacroErrorTestCase( + description="a_list_passed_to_an_any_argument", + files={ + **_ORDER_KEY, + **orders_model( + 'SELECT @sql.order_key(country_code, [1, 2]) AS k FROM __ref("stg_orders")\n' + ), + }, + expected_code="P017", + expected_fragments=( + "argument order_ref of macro order_key() expects ANY: a scalar SQL expression", + ), + ), + SqlMacroErrorTestCase( + description="any_with_const", + files={ + "macros/sql/order_key.sql": ( + 'MACRO (description "Key", arguments (order_ref ANY CONST), ' + "returns VARCHAR);\n\nCAST(@order_ref AS VARCHAR)\n" + ), + **orders_model('SELECT @sql.order_key(1) AS k FROM __ref("stg_orders")\n'), + }, + expected_code="P015", + expected_fragments=( + "argument order_ref ANY CONST: a CONST argument needs a type its values are " + "checked against", + ), + ), + ], + ids=lambda case: case.description, +) +def test_given_an_any_argument_misused_when_compiling_then_it_fails( + test_case: SqlMacroErrorTestCase, tmp_path: Path +) -> None: + _, result = run_project( + tmp_path=tmp_path, files=test_case.files, command=("compile", "--no-cache") + ) + + assert result.returncode == 1, output(result) + assert f"error[{test_case.expected_code}]" in output(result) + for fragment in test_case.expected_fragments: + assert fragment in output(result) + + +@pytest.mark.parametrize( + "test_case", + [ + SqlMacroCompileTestCase( + description="a_main_query_call_and_a_downstream_passthrough", + files=_TYPED_CALLS, + expected_returncode=0, + expected_fragments=("0 errors, 0 warnings",), + unexpected_fragments=_UNPROVEN, + ), + SqlMacroCompileTestCase( + description="a_call_in_a_final_cte_read_with_a_star", + files={ + **_TYPED_RULES_AND_MACROS, + **contract_model( + "orders", + _TYPED_COLUMNS, + "WITH final AS (\n SELECT\n o.order_id,\n" + " @sql.shouted(o.status) AS shouted,\n" + " @sql.status_bar(o.status) AS status_bar\n" + ' FROM __ref("stg_orders") AS o\n)\nSELECT * FROM final\n', + ), + }, + expected_returncode=0, + expected_fragments=("0 errors, 0 warnings",), + unexpected_fragments=_UNPROVEN, + ), + SqlMacroCompileTestCase( + description="a_call_through_a_two_cte_chain_and_a_join", + files={ + **_TYPED_RULES_AND_MACROS, + **contract_model( + "orders", + _TYPED_COLUMNS, + "WITH typed AS (\n SELECT\n o.order_id,\n" + " @sql.shouted(o.status) AS shouted,\n" + " @sql.status_bar(o.status) AS status_bar\n" + ' FROM __ref("stg_orders") AS o\n),\n' + "final AS (\n SELECT t.order_id, t.shouted, b.status_bar\n" + " FROM typed AS t JOIN typed AS b ON b.order_id = t.order_id\n)\n" + "SELECT * FROM final\n", + ), + }, + expected_returncode=0, + expected_fragments=("0 errors, 0 warnings",), + unexpected_fragments=_UNPROVEN, + ), + SqlMacroCompileTestCase( + description="a_call_inside_a_larger_expression_is_typed_from_its_sql", + files={ + **_TYPED_RULES_AND_MACROS, + **contract_model( + "orders", + "order_id (type BIGINT), status_bar (type VARCHAR)", + "WITH final AS (\n SELECT\n o.order_id,\n" + " COALESCE(@sql.status_bar(o.status), @sql.status_bar(o.status)) " + "AS status_bar\n" + ' FROM __ref("stg_orders") AS o\n)\nSELECT * FROM final\n', + ), + }, + expected_returncode=1, + expected_fragments=( + "warning[K003]: column 'status_bar' type could not be proven against declared " + "VARCHAR", + 'error[SQBRCONTRACT105]: output "status_bar" is calculated without an outer ' + "explicit cast", + ), + ), + SqlMacroCompileTestCase( + description="a_time_body_keeps_its_inferred_type", + files={ + **text_macro("m", "VARCHAR", "CAST('10:00:00' AS TIME)"), + **contract_model( + "orders", + "v (type VARCHAR)", + 'SELECT @sql.m(o.status) AS v FROM __ref("stg_orders") AS o\n', + ), + }, + expected_returncode=1, + expected_fragments=("error[K002]: column 'v' inferred as",), + ), + SqlMacroCompileTestCase( + description="an_array_body_keeps_its_inferred_type", + files={ + **text_macro("m", "VARCHAR", "[@value]"), + **contract_model( + "orders", + "v (type VARCHAR)", + 'SELECT @sql.m(o.status) AS v FROM __ref("stg_orders") AS o\n', + ), + }, + expected_returncode=1, + expected_fragments=("error[K002]: column 'v' inferred as",), + ), + SqlMacroCompileTestCase( + description="a_longer_text_body_keeps_its_inferred_type", + files={ + **text_macro("m", "VARCHAR(5)", "CAST(@value AS VARCHAR(100))"), + **contract_model( + "orders", + "v (type VARCHAR(5))", + 'SELECT @sql.m(o.status) AS v FROM __ref("stg_orders") AS o\n', + ), + }, + expected_returncode=1, + expected_fragments=("error[K002]: column 'v' inferred as",), + ), + SqlMacroCompileTestCase( + description="a_wider_decimal_body_keeps_its_inferred_type", + files={ + **text_macro("m", "DECIMAL(4, 1)", "CAST(LENGTH(@value) AS DECIMAL(18, 6))"), + **contract_model( + "orders", + "v (type DECIMAL(4, 1))", + 'SELECT @sql.m(o.status) AS v FROM __ref("stg_orders") AS o\n', + ), + }, + expected_returncode=1, + expected_fragments=("error[K002]: column 'v' inferred as",), + ), + SqlMacroCompileTestCase( + description="a_timestamptz_body_keeps_its_inferred_type", + files={ + **text_macro("m", "TIMESTAMP", "CAST(@value AS TIMESTAMPTZ)"), + **contract_model( + "orders", + "v (type TIMESTAMP)", + 'SELECT @sql.m(o.status) AS v FROM __ref("stg_orders") AS o\n', + ), + }, + expected_returncode=1, + expected_fragments=("error[K002]: column 'v' inferred as",), + ), + SqlMacroCompileTestCase( + description="an_unusual_outer_query_star_replace_still_compiles", + files={ + **_TYPED_RULES_AND_MACROS, + **orders_model( + "WITH final AS (\n" + " SELECT o.order_id, @sql.shouted(o.status) AS shouted\n" + ' FROM __ref("stg_orders") AS o\n' + ")\n" + "SELECT * REPLACE (UPPER(shouted) AS shouted) FROM final\n" + ), + }, + expected_returncode=0, + expected_fragments=("0 errors",), + ), + SqlMacroCompileTestCase( + description="an_unusual_outer_query_star_exclude_still_compiles", + files={ + **_TYPED_RULES_AND_MACROS, + **orders_model( + "WITH final AS (\n" + " SELECT o.order_id, @sql.shouted(o.status) AS shouted\n" + ' FROM __ref("stg_orders") AS o\n' + ")\n" + "SELECT * EXCLUDE (order_id) FROM final\n" + ), + }, + expected_returncode=0, + expected_fragments=("0 errors",), + ), + SqlMacroCompileTestCase( + description="an_unusual_outer_query_columns_expression_still_compiles", + files={ + **_TYPED_RULES_AND_MACROS, + **orders_model( + "WITH final AS (\n" + " SELECT o.order_id, @sql.shouted(o.status) AS shouted\n" + ' FROM __ref("stg_orders") AS o\n' + ")\n" + "SELECT COLUMNS('shout.*') FROM final\n" + ), + }, + expected_returncode=0, + expected_fragments=("0 errors",), + ), + SqlMacroCompileTestCase( + description="an_unusual_outer_query_star_rename_still_compiles", + files={ + **_TYPED_RULES_AND_MACROS, + **orders_model( + "WITH final AS (\n" + " SELECT o.order_id, @sql.shouted(o.status) AS shouted\n" + ' FROM __ref("stg_orders") AS o\n' + ")\n" + "SELECT * RENAME (shouted AS loud) FROM final\n" + ), + }, + expected_returncode=0, + expected_fragments=("0 errors",), + ), + SqlMacroCompileTestCase( + description="an_unusual_outer_query_full_join_using_still_compiles", + files={ + **_TYPED_RULES_AND_MACROS, + **orders_model( + "WITH final AS (\n" + " SELECT o.order_id, @sql.shouted(o.status) AS shouted\n" + ' FROM __ref("stg_orders") AS o\n' + ")\n" + "SELECT f.* FROM final AS f FULL JOIN final AS g USING (order_id)\n" + ), + }, + expected_returncode=0, + expected_fragments=("0 errors",), + ), + SqlMacroCompileTestCase( + description="a_call_in_a_union_branch_of_a_cte_types_the_union", + files=_UNION_CALLS, + expected_returncode=0, + expected_fragments=("0 errors, 0 warnings",), + unexpected_fragments=_UNPROVEN, + ), + SqlMacroCompileTestCase( + description="a_call_in_a_later_union_branch_of_the_main_query_types_the_union", + files={ + **_TYPED_RULES_AND_MACROS, + **contract_model( + "orders", + "status_bar (type VARCHAR)", + 'SELECT CAST(NULL AS VARCHAR) AS status_bar FROM __ref("stg_orders") AS o\n' + "UNION ALL\n" + 'SELECT @sql.status_bar(o.status) AS status_bar FROM __ref("stg_orders") AS o\n', + ), + }, + expected_returncode=0, + expected_fragments=("0 errors, 0 warnings",), + unexpected_fragments=_UNPROVEN, + ), + SqlMacroCompileTestCase( + description="a_union_sibling_of_another_known_type_still_reports", + files={ + **_TYPED_RULES_AND_MACROS, + **contract_model( + "orders", + "status_bar (type VARCHAR)", + "WITH final AS (\n" + ' SELECT @sql.status_bar(o.status) AS status_bar FROM __ref("stg_orders") AS o\n' + " UNION ALL\n" + " SELECT CAST(LENGTH(o.status) AS BIGINT) AS status_bar\n" + ' FROM __ref("stg_orders") AS o\n' + ")\nSELECT f.status_bar FROM final AS f\n", + ), + }, + expected_returncode=1, + expected_fragments=('error[SQBRCONTRACT105]: output "status_bar"',), + ), + SqlMacroCompileTestCase( + description="an_untyped_upstream_column_in_a_cte_takes_returns", + files={ + **with_rules("SQBRCONTRACT105"), + **text_macro("m", "BIGINT", "@value"), + "models/marts/up.sql": ( + 'MODEL (description "Up");\n\nSELECT NULL AS s FROM __ref("stg_orders") AS o\n' + ), + **contract_model( + "orders", + "v (type BIGINT)", + 'WITH c AS (SELECT @sql.m(u.s) AS v FROM __ref("up") AS u)\nSELECT v FROM c\n', + ), + }, + expected_returncode=0, + expected_fragments=("0 errors, 0 warnings",), + unexpected_fragments=_UNPROVEN, + ), + SqlMacroCompileTestCase( + description="a_call_in_a_later_main_query_branch_matching_its_sibling_is_typed", + files={ + **with_rules("SQBRCONTRACT105"), + **text_macro("m", "BIGINT", "NULL"), + **contract_model( + "orders", + "v (type BIGINT)", + 'SELECT CAST(NULL AS BIGINT) AS v FROM __ref("stg_orders") AS o\n' + "UNION ALL\n" + 'SELECT @sql.m(o.status) AS v FROM __ref("stg_orders") AS o\n', + ), + }, + expected_returncode=0, + expected_fragments=("0 errors, 0 warnings",), + unexpected_fragments=_UNPROVEN, + ), + SqlMacroCompileTestCase( + description="an_inferred_type_of_another_family_still_disagrees_through_a_cte", + files={ + **text_macro("shouted", "BIGINT", "UPPER(@value)"), + **contract_model( + "orders", + "shouted (type BIGINT)", + "WITH final AS (\n" + ' SELECT @sql.shouted(o.status) AS shouted FROM __ref("stg_orders") AS o\n' + ")\nSELECT * FROM final\n", + ), + }, + expected_returncode=1, + expected_fragments=("error[K002]: column 'shouted' inferred as",), + ), + ], + ids=lambda case: case.description, +) +def test_given_a_returns_type_when_checking_contracts_then_the_call_has_that_type( + test_case: SqlMacroCompileTestCase, tmp_path: Path +) -> None: + _, result = run_project( + tmp_path=tmp_path, files=test_case.files, command=("compile", "--no-cache") + ) + + assert result.returncode == test_case.expected_returncode, output(result) + for fragment in test_case.expected_fragments: + assert fragment in output(result) + for fragment in test_case.unexpected_fragments: + assert fragment not in output(result) + + +@pytest.mark.parametrize( + "test_case", + [ + SqlMacroCommandTestCase( + description="a_warm_compile_types_the_call_the_same_way", + files=_TYPED_CALLS, + command=("compile",), + expected_returncode=0, + expected_fragments=("0 errors, 0 warnings",), + ), + SqlMacroCommandTestCase( + description="a_warm_compile_types_a_union_branch_call_the_same_way", + files=_UNION_CALLS, + command=("compile",), + expected_returncode=0, + expected_fragments=("0 errors, 0 warnings",), + ), + ], + ids=lambda case: case.description, +) +def test_given_a_returns_type_when_compiling_warm_then_the_call_keeps_that_type( + test_case: SqlMacroCommandTestCase, tmp_path: Path +) -> None: + project_dir, cold = run_project( + tmp_path=tmp_path, files=test_case.files, command=("compile", "--no-cache") + ) + warm: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", *test_case.command), project_dir=project_dir + ) + + assert cold.returncode == 0, output(cold) + assert warm.returncode == test_case.expected_returncode, output(warm) + for fragment in test_case.expected_fragments: + assert fragment in output(warm) diff --git a/website/src/content/docs/docs/concepts/macros/sql-macros.mdx b/website/src/content/docs/docs/concepts/macros/sql-macros.mdx index f9b16676e7..c88fbe3f48 100644 --- a/website/src/content/docs/docs/concepts/macros/sql-macros.mdx +++ b/website/src/content/docs/docs/concepts/macros/sql-macros.mdx @@ -37,6 +37,11 @@ my_project/ A file directly under a `macros/` folder fails with `D013`, which names the subfolder to move it to. Folder scopes use the same layout, such as `models/marts/_sqlbuild/_macros/sql/`. +Rules that bound declaration folders, such as `SQBRDECLARATION304` (files per folder) and +`SQBRDECLARATION306` (shared filename prefixes), count `python/` and `sql/` as separate folders. To +split a full `sql/` folder, move files into subfolders such as `macros/sql/text/`: a SQL macro is +named by its file, so it keeps its name and its callers stay unchanged. + A SQL macro file starts with a `MACRO` header, then the body: ```sql @@ -139,6 +144,7 @@ Every parameter has a type, and may have a `DEFAULT`: | Type | Accepts | |---|---| | `BIGINT`, `VARCHAR`, `TIMESTAMP`, ... | Any SQL expression of a compatible type, evaluated per row: a column, arithmetic, a function call, a literal or a constant. | +| `ANY` | Any scalar SQL expression, whatever its type: a column, a literal, an expression or a scalar constant. Its type is not checked. Lists and relations fail with `P017`. | | ` CONST` | Only a value known at compile time: a literal, a scalar constant, an enum value, or a `CONST` parameter of a calling macro. A column or expression fails with `P017`. Scalar types only. | | `LIST `, `SET ` | An inline list or set, or a list or set constant. Always compile-time, so they take no `CONST`. They render like constants. | | `ENUM ` | A member of that enum, such as `@enum("order_status").OPEN`. | @@ -149,6 +155,13 @@ SQL types are written as in [schema declarations](/docs/concepts/models/schemas) adapter's SQL dialect the same way, including the dialect's array types such as `VARCHAR[]`. A runtime array value uses the array type; `LIST VARCHAR` is a list known at compile time. +Use `ANY` when the body works for values of any type, typically because it casts the argument +itself, as in `CAST(@order_ref AS VARCHAR)`, so callers can pass a number or a string. The body is +still checked, and the argument's type counts as unknown there. `ANY` is not `VARIANT`: a +`VARIANT` parameter says the macro takes semi-structured data, while `ANY` says it takes a plain +value of any type. `ANY` takes a `DEFAULT` but not `CONST`, which needs a type to check values +against, and it cannot be a `LIST` or `SET` element type. + Leave `CONST` off unless the body needs a literal. `CONST` is part of the macro's contract: callers can no longer pass a column. @@ -252,6 +265,19 @@ parameters of a calling macro, and calls in models with runtime placeholders; th column checks still run on the expanded SQL. All other errors also point at the call and at the macro file. +A select-list item that is one expression macro call under an alias, as in +`@sql.cents_to_dollars(o.amount_cents) AS amount`, has the macro's `returns` type wherever the +compiler infers no type for the expanded SQL, or exactly that type. This holds in the main query +and in any top-level CTE, including each branch of their `UNION`, `INTERSECT` or `EXCEPT`, and the +type flows on as an inferred type would: through `SELECT * FROM final`, pass-through columns and +joins, to contract checks (`K002`, `K003`), `SQBRCONTRACT105` and models that read the column. A +set operation's column combines its branches as usual, so a sibling branch of another type still +leaves it unproven, and a CTE holding one is read by column name, since `SELECT *` over it is not +typed. Such a call needs no `CAST` to satisfy a matching contract column. Any other +inferred type stands, even a close one such as `VARCHAR(100)` for `returns VARCHAR(5)`, and `K002` +reports the mismatch. A call inside a larger expression, a subquery, a nested `WITH` or a set +operation with a parenthesised branch is typed from the expanded SQL as before. + - `sqb format` formats SQL macro files, bodies of every form and the `MACRO` header, and normalises the commas around CTE-list and column-list calls. - [Rules](/docs/concepts/rules) and [column lineage](/docs/concepts/column-lineage) read the expanded