diff --git a/crates/sqlbuild-rules-native/src/compiler/tests/helpers.rs b/crates/sqlbuild-rules-native/src/compiler/tests/helpers.rs index d72e8bf7ac..f62533ed44 100644 --- a/crates/sqlbuild-rules-native/src/compiler/tests/helpers.rs +++ b/crates/sqlbuild-rules-native/src/compiler/tests/helpers.rs @@ -67,6 +67,14 @@ pub(crate) fn comments_and_quoted_text_hide_references() -> bool { == Some(vec![("ref".to_owned(), "orders".to_owned(), None, None)]) } +pub(crate) fn dollar_quoted_text_hides_references() -> bool { + extract(concat!( + "SELECT $$Customer's order -- __ref(\"ignored\") $5$$ AS label, ", + "$tag$ $$ __seed(\"also_ignored\") $tag$ AS note, price$1$ ", + "FROM __ref(orders)" + )) == Some(vec![("ref".to_owned(), "orders".to_owned(), None, None)]) +} + pub(crate) fn complex_or_malformed_sql_requests_fallback() -> bool { [ "SELECT * FROM __table_fn(\"orders\")(1)", @@ -74,6 +82,7 @@ pub(crate) fn complex_or_malformed_sql_requests_fallback() -> bool { "SELECT * FROM __ref(\"orders\"", "SELECT * FROM __ref(\"orders\") /* unterminated", "SELECT * FROM __ref(\"orders\") WHERE note = 'unterminated", + "SELECT * FROM __ref(\"orders\") WHERE note = $$unterminated", "SELECT * FROM __ref(örders)", ] .into_iter() diff --git a/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_references.rs b/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_references.rs index cda5c3de96..50ecf02ba2 100644 --- a/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_references.rs +++ b/crates/sqlbuild-rules-native/src/compiler/tests/test_sql_references.rs @@ -1,6 +1,6 @@ use crate::compiler::tests::helpers::{ comments_and_quoted_text_hide_references, complex_or_malformed_sql_requests_fallback, - simple_references_preserve_authored_order, + dollar_quoted_text_hides_references, simple_references_preserve_authored_order, }; use crate::compiler::tests::test_types::StaticSqlOperationTestCase; @@ -17,6 +17,11 @@ fn given_sql_reference_cases_when_extracting_then_expected_behavior_holds() { run: comments_and_quoted_text_hide_references, expected_success: true, }, + StaticSqlOperationTestCase { + description: "dollar-quoted text hides embedded references", + run: dollar_quoted_text_hides_references, + expected_success: true, + }, StaticSqlOperationTestCase { description: "complex and malformed SQL requests Python fallback", run: complex_or_malformed_sql_requests_fallback, diff --git a/crates/sqlbuild-rules-native/src/engine/tests/test_evaluation.rs b/crates/sqlbuild-rules-native/src/engine/tests/test_evaluation.rs index 7b904bff6f..579941bbeb 100644 --- a/crates/sqlbuild-rules-native/src/engine/tests/test_evaluation.rs +++ b/crates/sqlbuild-rules-native/src/engine/tests/test_evaluation.rs @@ -954,6 +954,60 @@ fn given_numeric_comparisons_when_evaluating_named_decisions_then_only_authored_ authored_sql: ("SELECT CASE WHEN amount > 30 THEN 'large' END AS amount_30 FROM orders"), expected_fault_count: 1, }, + test_types::NumericDecisionTestCase { + description: "authored between bounds are a named decision finding", + query_sql: "SELECT code FROM orders WHERE TRY_CAST(code AS INTEGER) BETWEEN 30 AND 61", + authored_sql: "SELECT code FROM orders WHERE TRY_CAST(code AS INTEGER) BETWEEN 30 AND 61", + expected_fault_count: 1, + }, + test_types::NumericDecisionTestCase { + description: "authored not between bounds are a named decision finding", + query_sql: "SELECT code FROM orders WHERE amount NOT BETWEEN 5 AND 10", + authored_sql: "SELECT code FROM orders WHERE amount NOT BETWEEN 5 AND 10", + expected_fault_count: 1, + }, + test_types::NumericDecisionTestCase { + description: "canonical between bounds pass", + query_sql: "SELECT code FROM orders WHERE amount BETWEEN 0 AND 1", + authored_sql: "SELECT code FROM orders WHERE amount BETWEEN 0 AND 1", + expected_fault_count: 0, + }, + test_types::NumericDecisionTestCase { + description: "expanded between constants are not authored literal findings", + query_sql: "SELECT code FROM orders WHERE amount BETWEEN 30 AND 61", + authored_sql: "SELECT code FROM orders WHERE amount BETWEEN @const(\"_low\") AND @const(\"_high\")", + expected_fault_count: 0, + }, + test_types::NumericDecisionTestCase { + description: "authored numeric in list is a named decision finding", + query_sql: "SELECT code FROM orders WHERE TRY_CAST(code AS INTEGER) IN (30, 31, 45)", + authored_sql: "SELECT code FROM orders WHERE TRY_CAST(code AS INTEGER) IN (30, 31, 45)", + expected_fault_count: 1, + }, + test_types::NumericDecisionTestCase { + description: "authored numeric not in list is a named decision finding", + query_sql: "SELECT code FROM orders WHERE amount NOT IN (30, 31)", + authored_sql: "SELECT code FROM orders WHERE amount NOT IN (30, 31)", + expected_fault_count: 1, + }, + test_types::NumericDecisionTestCase { + description: "canonical numeric in list passes", + query_sql: "SELECT code FROM orders WHERE amount IN (-1, 0, 1)", + authored_sql: "SELECT code FROM orders WHERE amount IN (-1, 0, 1)", + expected_fault_count: 0, + }, + test_types::NumericDecisionTestCase { + description: "expanded list constant is not an authored literal finding", + query_sql: "SELECT code FROM orders WHERE amount IN (30, 31, 45)", + authored_sql: "SELECT code FROM orders WHERE amount IN @const(\"_codes\")", + expected_fault_count: 0, + }, + test_types::NumericDecisionTestCase { + description: "between in projected case is a named decision finding", + query_sql: "SELECT CASE WHEN amount BETWEEN 30 AND 61 THEN 'mid' END AS band FROM orders", + authored_sql: "SELECT CASE WHEN amount BETWEEN 30 AND 61 THEN 'mid' END AS band FROM orders", + expected_fault_count: 1, + }, test_types::NumericDecisionTestCase { description: "nested comparison reports only its leaf decision", query_sql: ("SELECT CASE WHEN amount > 5 THEN status END = 'ready' AS selected FROM orders"), diff --git a/crates/sqlbuild-rules-native/src/engine/tests/test_sql_scanning.rs b/crates/sqlbuild-rules-native/src/engine/tests/test_sql_scanning.rs index f2e5c1b7b4..5d4607b0b3 100644 --- a/crates/sqlbuild-rules-native/src/engine/tests/test_sql_scanning.rs +++ b/crates/sqlbuild-rules-native/src/engine/tests/test_sql_scanning.rs @@ -170,7 +170,7 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann SqlScannerTestCase { description: "dollar quote", fragment: "$$ ) $$", - expected_compiler_paren: Ok(4), + expected_compiler_paren: Ok(10), expected_table_function: "SELECT * FROM __table_fn(\"o\", $$ ) $$ x)(1)", expected_table_function_token: "SELECT $$ ) $$ __table_fn(\"o\", 1)", expected_snowflake_exclude: "SELECT * $$ x) , a FROM t", diff --git a/crates/sqlbuild-rules-native/src/rules/_helpers/evaluation.rs b/crates/sqlbuild-rules-native/src/rules/_helpers/evaluation.rs index 01407def13..881c4f9658 100644 --- a/crates/sqlbuild-rules-native/src/rules/_helpers/evaluation.rs +++ b/crates/sqlbuild-rules-native/src/rules/_helpers/evaluation.rs @@ -30,6 +30,7 @@ use std::ops::ControlFlow; const MIXED_STAR_REMEDIATION: &str = "SELECT *, a, b mixes a passthrough star with derived columns. The * exemption permits a lone SELECT * only. Move a and b into an earlier CTE so they are computed upstream, leaving the final select a pure SELECT *. If you need those columns in the output, drop the * exemption and enumerate every column explicitly."; const RULES_PARSER_RECURSION_LIMIT: usize = 128; +const NUMERIC_VALUE_LIST_REMEDIATION: &str = "Declare the values as a list or set CONSTANT and compare through [NOT] IN @const(\"\"); only -1, 0, and 1 are self-explanatory."; #[derive(Clone)] struct Position { @@ -42,6 +43,8 @@ struct ComparisonFact { position: Option, sql: String, equality: bool, + value_list: bool, + binary: bool, output_name: Option, columns: BTreeSet, string_literals: Vec, @@ -312,6 +315,7 @@ pub(crate) fn rules_quote_policy(dialect_name: &str) -> QuotePolicy { single_quote_backslash_escapes: backslash_escapes, double_quote_backslash_escapes: backslash_escapes && !dialect.is_delimited_identifier_start('"'), + dollar_quotes: false, } } @@ -1226,7 +1230,9 @@ fn evaluate_literal_rules( ) { let facts = select_facts(&parsed.query); for comparison in facts.into_iter().flat_map(|select| select.comparisons) { - if let Some(rule) = selected.get("SQBRDECLARATION101") { + if comparison.binary + && let Some(rule) = selected.get("SQBRDECLARATION101") + { let enum_column = comparison .columns .iter() @@ -1265,7 +1271,9 @@ fn evaluate_literal_rules( rule, comparison.position.as_ref(), format!("non-canonical numeric comparison: {}", comparison.sql), - None, + comparison + .value_list + .then(|| NUMERIC_VALUE_LIST_REMEDIATION.to_owned()), )); } } @@ -1669,8 +1677,8 @@ fn comparison_facts(root: &Expr, source_context: &SourceContext, output: &mut Ve impl Visitor for Comparisons<'_> { type Break = (); fn pre_visit_expr(&mut self, expression: &Expr) -> ControlFlow { - if let Expr::BinaryOp { op, .. } = expression - && matches!( + let is_comparison = match expression { + Expr::BinaryOp { op, .. } => matches!( op, BinaryOperator::Eq | BinaryOperator::NotEq @@ -1678,8 +1686,11 @@ fn comparison_facts(root: &Expr, source_context: &SourceContext, output: &mut Ve | BinaryOperator::GtEq | BinaryOperator::Lt | BinaryOperator::LtEq - ) - { + ), + Expr::Between { .. } | Expr::InList { .. } => true, + _ => false, + }; + if is_comparison { self.output .push(comparison_fact(expression, self.source_context)); } @@ -1776,29 +1787,40 @@ fn comparison_fact(root: &Expr, source_context: &SourceContext) -> ComparisonFac Expr::BinaryOp { op: BinaryOperator::Eq, .. - } + } | Expr::InList { negated: false, .. } ), + value_list: matches!(root, Expr::InList { .. }), + binary: matches!(root, Expr::BinaryOp { .. }), source_context: source_context.clone(), ..ComparisonFact::default() }, }; let _ = root.visit(&mut visitor); - if let Expr::BinaryOp { left, right, .. } = root { - for operand in [left.as_ref(), right.as_ref()] { - if let Some(number) = numeric_value(operand) { - visitor.fact.numeric_literals.push(number); - } - if direct_column(operand).is_none() { - visitor.fact.modified_columns.extend(column_facts(operand)); - } - if direct_string_literal(operand).is_none() && contains_string_literal(operand) { - visitor.fact.modified_string_literal = true; - } + for operand in comparison_operands(root) { + if let Some(number) = numeric_value(operand) { + visitor.fact.numeric_literals.push(number); + } + if direct_column(operand).is_none() { + visitor.fact.modified_columns.extend(column_facts(operand)); + } + if direct_string_literal(operand).is_none() && contains_string_literal(operand) { + visitor.fact.modified_string_literal = true; } } visitor.fact } +fn comparison_operands(root: &Expr) -> Vec<&Expr> { + match root { + Expr::BinaryOp { left, right, .. } => vec![left.as_ref(), right.as_ref()], + Expr::Between { + expr, low, high, .. + } => vec![expr.as_ref(), low.as_ref(), high.as_ref()], + Expr::InList { expr, list, .. } => std::iter::once(expr.as_ref()).chain(list).collect(), + _ => Vec::new(), + } +} + fn direct_column(expression: &Expr) -> Option { match unwrap_nested(expression) { Expr::Identifier(value) => Some(ColumnFact { diff --git a/crates/sqlbuild-rules-native/src/sql_lint/_helpers/additional.rs b/crates/sqlbuild-rules-native/src/sql_lint/_helpers/additional.rs index 78977fb896..3d6b769bcb 100644 --- a/crates/sqlbuild-rules-native/src/sql_lint/_helpers/additional.rs +++ b/crates/sqlbuild-rules-native/src/sql_lint/_helpers/additional.rs @@ -10,6 +10,7 @@ use crate::sql_lint::_helpers::engine::{ direct_indices, is_comment, is_layout, is_query_from, query_end, significant_after, significant_before, token_depths, }; +use crate::sql_lint::constants::RELATION_MODIFIER_KEYWORDS; use crate::sql_lint::models::{AdditionalFactOptions, AdditionalQueryFacts}; const COUNT_ONE_LITERAL: &str = "1"; @@ -1242,34 +1243,92 @@ fn unused_alias_spans(query: &QuerySlice<'_>) -> Vec { let query_end = *query_end; let depth = *depth; let mut spans: Vec = Vec::new(); - for window in relation_clause.windows(2) { - if tokens[window[0]].token_type != TokenType::As || !is_identifier(&tokens[window[1]]) { - continue; - } - if significant_after(tokens, window[1]) + for (span_start, alias_index) in relation_alias_positions(tokens, relation_clause) { + if significant_after(tokens, alias_index) .is_some_and(|index| tokens[index].token_type == TokenType::LParen) { continue; } - let alias = &tokens[window[1]].text; + let alias = &tokens[alias_index].text; + let relation_name = significant_before(tokens, span_start); let used = (query_start..query_end).any(|index| { - depths[index] >= depth + index != alias_index + && Some(index) != relation_name + && depths[index] >= depth + && is_identifier(&tokens[index]) && tokens[index].text.eq_ignore_ascii_case(alias) + && significant_before(tokens, index).is_none_or(|previous| { + !matches!(tokens[previous].token_type, TokenType::Dot | TokenType::As) + }) && significant_after(tokens, index) - .is_some_and(|next| tokens[next].token_type == TokenType::Dot) + .is_none_or(|next| tokens[next].token_type != TokenType::LParen) }); if !used { spans.push(Span { - start: tokens[window[0]].span.start, - end: tokens[window[1]].span.end, - line: tokens[window[0]].span.line, - column: tokens[window[0]].span.column, + start: tokens[span_start].span.start, + end: tokens[alias_index].span.end, + line: tokens[span_start].span.line, + column: tokens[span_start].span.column, }); } } spans } +fn relation_alias_positions(tokens: &[Token], relation_clause: &[usize]) -> Vec<(usize, usize)> { + let mut aliases: Vec<(usize, usize)> = Vec::new(); + let mut segment_start = 0_usize; + let mut in_condition = false; + for (position, &index) in relation_clause.iter().enumerate() { + if matches!(tokens[index].token_type, TokenType::Join | TokenType::Comma) { + segment_start = position + 1; + in_condition = false; + continue; + } + if in_condition || is_relation_condition_start(&tokens[index]) { + in_condition = true; + continue; + } + if tokens[index].token_type == TokenType::As { + if let Some(&alias) = relation_clause.get(position + 1) + && is_identifier(&tokens[alias]) + { + aliases.push((index, alias)); + } + continue; + } + if is_implicit_relation_alias( + tokens, + &relation_clause[segment_start..], + position - segment_start, + ) { + aliases.push((index, index)); + } + } + aliases +} + +fn is_implicit_relation_alias(tokens: &[Token], segment: &[usize], position: usize) -> bool { + let candidate = &tokens[segment[position]]; + if position == 0 + || !is_identifier(candidate) + || candidate.token_type != TokenType::QuotedIdentifier + && RELATION_MODIFIER_KEYWORDS + .iter() + .any(|keyword| candidate.text.eq_ignore_ascii_case(keyword)) + { + return false; + } + let previous = &tokens[segment[position - 1]]; + if previous.token_type == TokenType::LParen { + return true; + } + is_identifier(previous) + && position + .checked_sub(QUALIFIER_PAIR_LENGTH) + .is_none_or(|before| tokens[segment[before]].token_type == TokenType::Dot) +} + fn null_rejected_left_join_spans( tokens: &[Token], direct: &[usize], diff --git a/crates/sqlbuild-rules-native/src/sql_lint/constants.rs b/crates/sqlbuild-rules-native/src/sql_lint/constants.rs index 873ef0ab42..7d1409979e 100644 --- a/crates/sqlbuild-rules-native/src/sql_lint/constants.rs +++ b/crates/sqlbuild-rules-native/src/sql_lint/constants.rs @@ -6,3 +6,48 @@ pub(crate) const VALUES_RELATION_PREFIX_TOKEN_COUNT: usize = 2; pub(crate) const POSITION_ARGUMENT_SEPARATOR: &str = ","; pub(crate) const CTE_MACRO_PLACEHOLDER_LITERAL: &str = "1"; pub(crate) const POSTFIX_CAST_PREFIX_TOKEN_COUNT: usize = 2; +pub(crate) const RELATION_MODIFIER_KEYWORDS: [&str; 43] = [ + "ANTI", + "APPLY", + "ASOF", + "AT", + "BEFORE", + "CHANGES", + "CONNECT", + "CROSS", + "EXCEPT", + "FETCH", + "FINAL", + "FOR", + "FULL", + "GROUP", + "HAVING", + "INNER", + "INTERSECT", + "JOIN", + "LATERAL", + "LEFT", + "LIMIT", + "MATCH_RECOGNIZE", + "MINUS", + "NATURAL", + "OFFSET", + "ON", + "ORDER", + "OUTER", + "PASTE", + "PIVOT", + "POSITIONAL", + "QUALIFY", + "RETURNING", + "RIGHT", + "SAMPLE", + "SEMI", + "START", + "TABLESAMPLE", + "UNION", + "UNPIVOT", + "USING", + "WHERE", + "WINDOW", +]; diff --git a/crates/sqlbuild-rules-native/src/sql_lint/tests.rs b/crates/sqlbuild-rules-native/src/sql_lint/tests.rs index f9afcf7533..61539eb14a 100644 --- a/crates/sqlbuild-rules-native/src/sql_lint/tests.rs +++ b/crates/sqlbuild-rules-native/src/sql_lint/tests.rs @@ -12,5 +12,7 @@ mod helpers; mod sql_lint; #[path = "tests/test_types.rs"] mod test_types; +#[path = "tests/test_unused_table_alias.rs"] +mod unused_table_alias; #[path = "tests/test_values_formatter.rs"] mod values_formatter; diff --git a/crates/sqlbuild-rules-native/src/sql_lint/tests/test_unused_table_alias.rs b/crates/sqlbuild-rules-native/src/sql_lint/tests/test_unused_table_alias.rs new file mode 100644 index 0000000000..940b79c4f1 --- /dev/null +++ b/crates/sqlbuild-rules-native/src/sql_lint/tests/test_unused_table_alias.rs @@ -0,0 +1,317 @@ +use crate::sql_lint::tests::{helpers, test_types}; + +#[test] +fn given_implicit_and_explicit_table_aliases_when_linting_then_unused_aliases_match() +-> Result<(), String> { + let test_cases = [ + test_types::AdditionalLintRuleTestCase { + description: "unused implicit table alias", + sql: "SELECT id FROM items unused", + rule: "SQBRSQL023", + expected_anchor: Some("unused"), + expected_replacement: Some(""), + }, + test_types::AdditionalLintRuleTestCase { + description: "unused implicit joined table alias", + sql: "SELECT o.id FROM orders o INNER JOIN customers unused ON o.customer_id = o.id", + rule: "SQBRSQL023", + expected_anchor: Some("unused"), + expected_replacement: Some(""), + }, + test_types::AdditionalLintRuleTestCase { + description: "unused implicit qualified table alias", + sql: "SELECT id FROM warehouse.analytics.items unused", + rule: "SQBRSQL023", + expected_anchor: Some("unused"), + expected_replacement: Some(""), + }, + test_types::AdditionalLintRuleTestCase { + description: "unused implicit subquery alias", + sql: "SELECT id FROM (SELECT id FROM items) unused", + rule: "SQBRSQL023", + expected_anchor: Some("unused"), + expected_replacement: Some(""), + }, + test_types::AdditionalLintRuleTestCase { + description: "unused implicit lateral table function alias", + sql: "SELECT src.id FROM items src, LATERAL FLATTEN(input => src.payload) unused", + rule: "SQBRSQL023", + expected_anchor: Some("unused"), + expected_replacement: Some(""), + }, + test_types::AdditionalLintRuleTestCase { + description: "used implicit table alias", + sql: "SELECT kept.id FROM items kept", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "used implicit joined table aliases", + sql: "SELECT o.id, c.name FROM orders o INNER JOIN customers c ON o.customer_id = c.id", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "used implicit qualified table alias", + sql: "SELECT kept.id FROM warehouse.analytics.items kept", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "used implicit subquery alias", + sql: "SELECT sub.id FROM (SELECT id FROM items) sub", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "used implicit lateral table function alias", + sql: "SELECT flattened.value FROM items src, LATERAL FLATTEN(input => src.payload) flattened", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "implicit alias used by correlated subquery", + sql: "SELECT id FROM items kept WHERE EXISTS (SELECT 1 FROM other WHERE other.id = kept.id)", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "implicit alias carrying a column list is retained", + sql: "SELECT a FROM items t(a)", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "qualified relation name is not an implicit alias", + sql: "SELECT id FROM warehouse.analytics.items", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "join modifier keywords are not implicit aliases", + sql: "SELECT o.id, c.name FROM orders o POSITIONAL JOIN customers c", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "join condition identifiers are not implicit aliases", + sql: "SELECT o.id, c.name FROM orders o INNER JOIN customers c USING (customer_id)", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + ]; + + for test_case in test_cases { + let diagnostics = helpers::diagnostics_for_rules(test_case.sql, &[test_case.rule])?; + assert_eq!( + diagnostics.len(), + usize::from(test_case.expected_anchor.is_some()), + "{}", + test_case.description + ); + let _ = test_case.expected_anchor.map(|expected_anchor| { + let diagnostic = &diagnostics[0]; + let start = diagnostic["start"].as_u64().unwrap_or_default() as usize; + let end = diagnostic["end"].as_u64().unwrap_or_default() as usize; + assert_eq!( + &test_case.sql[start..end], + expected_anchor, + "{}", + test_case.description + ); + assert_eq!( + diagnostic["fix"]["replacement"].as_str(), + test_case.expected_replacement, + "{}", + test_case.description + ); + }); + } + Ok(()) +} + +#[test] +fn given_join_modifiers_and_whole_row_uses_when_linting_duckdb_then_aliases_are_kept() +-> Result<(), String> { + let test_cases = [ + test_types::AdditionalLintRuleTestCase { + description: "unaliased relation before positional join has no alias", + sql: "SELECT * FROM orders POSITIONAL JOIN customers", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "unaliased relation before asof join has no alias", + sql: "SELECT * FROM orders ASOF JOIN prices ON orders.item_id = prices.item_id AND orders.ordered_at >= prices.valid_from", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "unaliased relation before natural join has no alias", + sql: "SELECT * FROM orders NATURAL JOIN customers", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "unaliased relation before anti join has no alias", + sql: "SELECT * FROM orders ANTI JOIN refunds USING (order_id)", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "unaliased relation before semi join has no alias", + sql: "SELECT * FROM orders SEMI JOIN refunds USING (order_id)", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "unaliased relation before cross join has no alias", + sql: "SELECT * FROM orders CROSS JOIN customers", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "implicit alias repeating the relation name is unused", + sql: "SELECT id FROM orders orders", + rule: "SQBRSQL023", + expected_anchor: Some("orders"), + expected_replacement: Some(""), + }, + test_types::AdditionalLintRuleTestCase { + description: "explicit alias repeating the relation name is unused", + sql: "SELECT id FROM orders AS orders", + rule: "SQBRSQL023", + expected_anchor: Some("AS orders"), + expected_replacement: Some(""), + }, + test_types::AdditionalLintRuleTestCase { + description: "alias repeating the relation name is used when referenced", + sql: "SELECT orders.id FROM orders orders", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "unaliased relation before window clause has no alias", + sql: "SELECT SUM(amount) OVER w FROM orders WINDOW w AS (ORDER BY id)", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "unaliased relation before tablesample has no alias", + sql: "SELECT * FROM orders TABLESAMPLE BERNOULLI (10)", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "unaliased relation before unpivot has no alias", + sql: "SELECT * FROM orders UNPIVOT (amount FOR month IN (jan, feb))", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "implicit alias used as whole-row function argument", + sql: "SELECT row_to_json(o) AS payload FROM orders o", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "explicit alias used as whole-row function argument", + sql: "SELECT row_to_json(o) AS payload FROM orders AS o", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "implicit alias used as bare projection", + sql: "SELECT o FROM orders o", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "explicit alias used as bare projection", + sql: "SELECT o FROM orders AS o", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "implicit alias used in whole-row comparison", + sql: "SELECT 1 AS present FROM orders o WHERE o IS NOT NULL", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "explicit alias used in whole-row comparison", + sql: "SELECT 1 AS present FROM orders AS o WHERE o IS NOT NULL", + rule: "SQBRSQL023", + expected_anchor: None, + expected_replacement: None, + }, + test_types::AdditionalLintRuleTestCase { + description: "unused implicit alias beside an unrelated qualified column is reported", + sql: "SELECT c.o FROM customers c CROSS JOIN orders o", + rule: "SQBRSQL023", + expected_anchor: Some("o"), + expected_replacement: Some(""), + }, + test_types::AdditionalLintRuleTestCase { + description: "unused explicit alias shadowed by an output alias is reported", + sql: "SELECT order_id AS o FROM orders AS o", + rule: "SQBRSQL023", + expected_anchor: Some("AS o"), + expected_replacement: Some(""), + }, + ]; + + for test_case in test_cases { + let diagnostics = + helpers::diagnostics_for_dialect(test_case.sql, "duckdb", &[test_case.rule])?; + assert_eq!( + diagnostics.len(), + usize::from(test_case.expected_anchor.is_some()), + "{}", + test_case.description + ); + let _ = test_case.expected_anchor.map(|expected_anchor| { + let diagnostic = &diagnostics[0]; + let start = diagnostic["start"].as_u64().unwrap_or_default() as usize; + let end = diagnostic["end"].as_u64().unwrap_or_default() as usize; + assert_eq!( + &test_case.sql[start..end], + expected_anchor, + "{}", + test_case.description + ); + assert_eq!( + diagnostic["fix"]["replacement"].as_str(), + test_case.expected_replacement, + "{}", + test_case.description + ); + }); + } + Ok(()) +} diff --git a/crates/sqlbuild-rules-native/src/sql_scan/main/non_code_end.rs b/crates/sqlbuild-rules-native/src/sql_scan/main/non_code_end.rs index cb99e49124..bf57e1b0aa 100644 --- a/crates/sqlbuild-rules-native/src/sql_scan/main/non_code_end.rs +++ b/crates/sqlbuild-rules-native/src/sql_scan/main/non_code_end.rs @@ -15,6 +15,35 @@ pub(crate) fn non_code_end( } match sql.get(index) { Some(byte) if policy.is_quote(*byte) => quote_end(sql, index, policy).map(Some), + Some(b'$') if policy.dollar_quotes => dollar_quote_end(sql, index), _ => Ok(None), } } + +/// Return the end of the `$$` or `$tag$` literal opening at `start`, if one opens there. +fn dollar_quote_end(sql: &[u8], start: usize) -> Result, Unclosed> { + if start + .checked_sub(1) + .is_some_and(|previous| continues_dollar_word(sql[previous])) + { + return Ok(None); + } + let tag_end = sql[start + 1..] + .iter() + .position(|byte| !(byte.is_ascii_alphanumeric() || *byte == b'_')) + .map_or(sql.len(), |offset| start + 1 + offset); + let tag = &sql[start + 1..tag_end]; + if sql.get(tag_end) != Some(&b'$') || tag.first().is_some_and(u8::is_ascii_digit) { + return Ok(None); + } + let delimiter = &sql[start..=tag_end]; + sql[tag_end + 1..] + .windows(delimiter.len()) + .position(|window| window == delimiter) + .map(|offset| Some(tag_end + 1 + offset + delimiter.len())) + .ok_or(Unclosed::Quote) +} + +fn continues_dollar_word(byte: u8) -> bool { + !byte.is_ascii() || byte.is_ascii_alphanumeric() || matches!(byte, b'_' | b'$') +} diff --git a/crates/sqlbuild-rules-native/src/sql_scan/models.rs b/crates/sqlbuild-rules-native/src/sql_scan/models.rs index 4f508b195c..bba4289068 100644 --- a/crates/sqlbuild-rules-native/src/sql_scan/models.rs +++ b/crates/sqlbuild-rules-native/src/sql_scan/models.rs @@ -17,14 +17,17 @@ pub(crate) struct QuotePolicy { pub(crate) single_quote_backslash_escapes: bool, /// A backslash escapes the next byte inside double-quoted text. pub(crate) double_quote_backslash_escapes: bool, + /// `$$` and `$tag$` delimit opaque string literals. + pub(crate) dollar_quotes: bool, } impl QuotePolicy { - /// Compiler policy, mirroring Python `sql_analysis`: backticks quote, backslashes are literal. + /// Compiler policy, mirroring Python `sql_analysis`: backticks and dollar quotes are text. pub(crate) const COMPILER: Self = Self { backtick_identifiers: true, single_quote_backslash_escapes: false, double_quote_backslash_escapes: false, + dollar_quotes: true, }; /// SQL lint base policy: backslashes escape; lint enables backticks per dialect. @@ -32,6 +35,7 @@ impl QuotePolicy { backtick_identifiers: false, single_quote_backslash_escapes: true, double_quote_backslash_escapes: true, + dollar_quotes: false, }; /// Return this policy with backtick-delimited identifiers enabled or disabled. diff --git a/crates/sqlbuild-rules-native/src/sql_scan/tests/test_sql_scan.rs b/crates/sqlbuild-rules-native/src/sql_scan/tests/test_sql_scan.rs index 99da540962..8079314db4 100644 --- a/crates/sqlbuild-rules-native/src/sql_scan/tests/test_sql_scan.rs +++ b/crates/sqlbuild-rules-native/src/sql_scan/tests/test_sql_scan.rs @@ -8,12 +8,14 @@ const QUOTES_ONLY: QuotePolicy = QuotePolicy { backtick_identifiers: false, single_quote_backslash_escapes: false, double_quote_backslash_escapes: false, + dollar_quotes: false, }; const BACKSLASH_ONLY: QuotePolicy = QuotePolicy { backtick_identifiers: false, single_quote_backslash_escapes: true, double_quote_backslash_escapes: false, + dollar_quotes: false, }; #[test] @@ -62,9 +64,15 @@ fn given_quote_policies_when_matching_parentheses_then_policy_decides_quote_boun expected_close: Ok(12), }, MatchingParenPolicyTestCase { - description: "dollar quotes are scanned as code", + description: "compiler policy skips dollar-quoted text", sql: "($$ ) $$ x) y", policy: QuotePolicy::COMPILER, + expected_close: Ok(10), + }, + MatchingParenPolicyTestCase { + description: "dollar quotes are code when the policy disables them", + sql: "($$ ) $$ x) y", + policy: QuotePolicy::SQL_LINT, expected_close: Ok(4), }, MatchingParenPolicyTestCase { @@ -136,6 +144,36 @@ fn given_comment_or_quote_starts_when_finding_non_code_end_then_returns_end_offs policy: QuotePolicy::SQL_LINT, expected_end: Err(Unclosed::Quote), }, + NonCodeEndTestCase { + description: "dollar quote hides apostrophes, comments and single dollars", + sql: "$$it's -- $5 $$ b", + policy: QuotePolicy::COMPILER, + expected_end: Ok(Some(15)), + }, + NonCodeEndTestCase { + description: "tagged dollar quote ends only at its own tag", + sql: "$tag$ $$ ) $tag$ b", + policy: QuotePolicy::COMPILER, + expected_end: Ok(Some(16)), + }, + NonCodeEndTestCase { + description: "unterminated dollar quote reports the quote", + sql: "$$it's", + policy: QuotePolicy::COMPILER, + expected_end: Err(Unclosed::Quote), + }, + NonCodeEndTestCase { + description: "positional parameter is code", + sql: "$1 $", + policy: QuotePolicy::COMPILER, + expected_end: Ok(None), + }, + NonCodeEndTestCase { + description: "digit-led tag is not a dollar quote", + sql: "$1$ x $1$", + policy: QuotePolicy::COMPILER, + expected_end: Ok(None), + }, NonCodeEndTestCase { description: "plain code has no non-code end", sql: "SELECT 1", diff --git a/src/sqlbuild/cli/commands/_helpers/compile/pipeline.py b/src/sqlbuild/cli/commands/_helpers/compile/pipeline.py index 18556cfe0e..cb4bf124ed 100644 --- a/src/sqlbuild/cli/commands/_helpers/compile/pipeline.py +++ b/src/sqlbuild/cli/commands/_helpers/compile/pipeline.py @@ -210,6 +210,7 @@ def _analyze_compile_project( selected_keys=selected_keys if select or exclude else None, prepared_sql=early_lint.preparation, expansion_reuse=early_lint.expansion_reuse, + no_sql_analysis=no_sql_validation, ) _ = complete_compile_phase( status=status, @@ -246,6 +247,7 @@ def _analyze_compile_project( custom_rules_ms=rules_result.custom_ms, rule_cache_hits=rules_result.cache_hits, rule_cache_misses=rules_result.cache_misses, + skipped_type_proof_rules=rules_result.skipped_type_proof_rules, ) diff --git a/src/sqlbuild/cli/commands/_helpers/rules/command.py b/src/sqlbuild/cli/commands/_helpers/rules/command.py index d04fd2d6f3..cf926e2834 100644 --- a/src/sqlbuild/cli/commands/_helpers/rules/command.py +++ b/src/sqlbuild/cli/commands/_helpers/rules/command.py @@ -25,6 +25,7 @@ from sqlbuild.rule_engine.main.load_config import load_rules_config from sqlbuild.rule_engine.main.render_result import format_result from sqlbuild.rule_engine.main.render_rule import format_rule +from sqlbuild.rule_engine.main.render_skipped_rules import format_skipped_type_proof_rules from sqlbuild.rule_engine.main.run_rules import run_rules from sqlbuild.rule_engine.main.skills import install_skills from sqlbuild.rule_engine.models import Rule, RulesConfig, RulesResult, RulesRunResult @@ -151,6 +152,11 @@ def _run_rule_selection( dialect=adapter.sql_analysis_dialect() or "generic", selected_keys=selected_keys if request.select or request.exclude else None, ) + skipped_rules_note: str | None = format_skipped_type_proof_rules( + codes=result.skipped_type_proof_rules + ) + if skipped_rules_note is not None: + print(f"note: {skipped_rules_note}", file=sys.stderr) print( format_result( result=RulesResult( diff --git a/src/sqlbuild/cli/commands/exceptions.py b/src/sqlbuild/cli/commands/exceptions.py index 29f3d0277f..0294f1403e 100644 --- a/src/sqlbuild/cli/commands/exceptions.py +++ b/src/sqlbuild/cli/commands/exceptions.py @@ -15,6 +15,13 @@ def __init__(self, message: str, *, code: str | None = None, help: str | None = self.help = help +class QueryExecutionError(CliUserError): + """The warehouse rejected or failed an ad hoc query.""" + + code: str = "C108" + exit_code: int = 2 + + class QueryDiffOutcomeError(CliUserError): """Expected query-diff failure with an explicit outcome and exit code.""" diff --git a/src/sqlbuild/cli/commands/main/inspection/_query.py b/src/sqlbuild/cli/commands/main/inspection/_query.py index 0d6c0f8a33..807a9eda40 100644 --- a/src/sqlbuild/cli/commands/main/inspection/_query.py +++ b/src/sqlbuild/cli/commands/main/inspection/_query.py @@ -6,13 +6,14 @@ from pathlib import Path from sqlbuild.adapter.contract.classes.base_adapter import BaseAdapter +from sqlbuild.adapter.contract.exceptions import AdapterUserError from sqlbuild.adapter.contract.models import QueryResult from sqlbuild.cli.commands._helpers.query.output import render_query_result from sqlbuild.cli.commands._helpers.runtime.adapters import resolve_adapter from sqlbuild.cli.commands._helpers.runtime.connection import ( resolve_project_connection_config, ) -from sqlbuild.cli.commands.exceptions import CliUserError +from sqlbuild.cli.commands.exceptions import CliUserError, QueryExecutionError from sqlbuild.compiler.discovery.main.discover import discover_project_inputs from sqlbuild.compiler.discovery.models import DiscoveredProjectInputs from sqlbuild.spec.contracts.main.resolve_effective_adapter_name import ( @@ -58,6 +59,10 @@ def run_query( connection: object = adapter.connect(connection_config) try: result: QueryResult = adapter.query(connection=connection, sql=query_sql, limit=limit) + except AdapterUserError: + raise + except Exception as error: + raise QueryExecutionError(str(getattr(error, "message", error))) from error finally: adapter.close(connection) diff --git a/src/sqlbuild/cli/commands/main/project/_compile.py b/src/sqlbuild/cli/commands/main/project/_compile.py index 049201aa3f..7818c3dd77 100644 --- a/src/sqlbuild/cli/commands/main/project/_compile.py +++ b/src/sqlbuild/cli/commands/main/project/_compile.py @@ -2,6 +2,7 @@ from __future__ import annotations +import sys import time from dataclasses import replace from pathlib import Path @@ -31,6 +32,7 @@ from sqlbuild.compiler.profiling.models import CompileTimingCollector from sqlbuild.presentation.classes.transient_status_reporter import TransientStatusReporter from sqlbuild.presentation.main.supports_color import supports_color +from sqlbuild.rule_engine.main.render_skipped_rules import format_skipped_type_proof_rules def run_compile(request: CompileCommandRequest) -> int: @@ -143,6 +145,11 @@ def _run_compile_with_status( if status is not None: status.close() + skipped_rules_note: str | None = format_skipped_type_proof_rules( + codes=analysis.skipped_type_proof_rules + ) + if skipped_rules_note is not None: + print(f"note: {skipped_rules_note}", file=sys.stderr) if json_output: print( diff --git a/src/sqlbuild/cli/compile/models.py b/src/sqlbuild/cli/compile/models.py index 98bbc05424..7a177ea7db 100644 --- a/src/sqlbuild/cli/compile/models.py +++ b/src/sqlbuild/cli/compile/models.py @@ -62,6 +62,7 @@ class CompileAnalysis: custom_rules_ms: int = 0 rule_cache_hits: int = 0 rule_cache_misses: int = 0 + skipped_type_proof_rules: tuple[str, ...] = () @dataclass(frozen=True) diff --git a/src/sqlbuild/compiler/compile/_helpers/refs/references.py b/src/sqlbuild/compiler/compile/_helpers/refs/references.py index 0e000b994b..255f6b19c9 100644 --- a/src/sqlbuild/compiler/compile/_helpers/refs/references.py +++ b/src/sqlbuild/compiler/compile/_helpers/refs/references.py @@ -34,7 +34,7 @@ _REFERENCE_PREFIX_BY_KIND: dict[SqlReferenceKind, str] = { ref_kind: prefix[:-1] for prefix, ref_kind in _REFERENCE_PREFIXES } -_REFERENCE_SCAN_PATTERN: re.Pattern[str] = re.compile(r"__|--|/\*|'|\"|`") +_REFERENCE_SCAN_PATTERN: re.Pattern[str] = re.compile(r"__|--|/\*|'|\"|`|\$") def extract_sql_references(sql: str) -> tuple[CompileSqlReference, ...]: diff --git a/src/sqlbuild/compiler/compile/_helpers/render/cursor_intrinsics.py b/src/sqlbuild/compiler/compile/_helpers/render/cursor_intrinsics.py index 230b2af9a1..f98fff0085 100644 --- a/src/sqlbuild/compiler/compile/_helpers/render/cursor_intrinsics.py +++ b/src/sqlbuild/compiler/compile/_helpers/render/cursor_intrinsics.py @@ -26,7 +26,9 @@ _IDENTIFIER_JOIN_CHARACTER: str = "_" _LINE_COMMENT_TOKEN: str = "--" _BLOCK_COMMENT_TOKEN: str = "/*" -_INTRINSIC_SCAN_PATTERN: re.Pattern[str] = re.compile(r"__cursor_start|__cursor_end|--|/\*|'|\"|`") +_INTRINSIC_SCAN_PATTERN: re.Pattern[str] = re.compile( + r"__cursor_start|__cursor_end|--|/\*|'|\"|`|\$" +) def get_validated_model_cursor_intrinsics( diff --git a/src/sqlbuild/compiler/compile/_helpers/render/declarations.py b/src/sqlbuild/compiler/compile/_helpers/render/declarations.py index d847acff0c..b9f4ee936a 100644 --- a/src/sqlbuild/compiler/compile/_helpers/render/declarations.py +++ b/src/sqlbuild/compiler/compile/_helpers/render/declarations.py @@ -65,7 +65,7 @@ r"(?P=quote)\s*\)" ) _DECLARATION_REFERENCE_START_PATTERN: re.Pattern[str] = re.compile(r"@(?Penum|const)\b") -_DECLARATION_SCAN_SPECIAL: re.Pattern[str] = re.compile(r"['\"`@/-]") +_DECLARATION_SCAN_SPECIAL: re.Pattern[str] = re.compile(r"['\"`$@/-]") _CONTEXT: str = "Enum and constant expansion" _ACCEPTED_VALUES_AUDIT: str = "accepted_values" _ENUM_REFERENCE_KIND: str = "enum" diff --git a/src/sqlbuild/compiler/compile/_helpers/render/macros.py b/src/sqlbuild/compiler/compile/_helpers/render/macros.py index f428aa948d..000bb0802b 100644 --- a/src/sqlbuild/compiler/compile/_helpers/render/macros.py +++ b/src/sqlbuild/compiler/compile/_helpers/render/macros.py @@ -68,7 +68,7 @@ _LINE_COMMENT_TOKEN: str = "--" _BLOCK_COMMENT_TOKEN: str = "/*" _STAR_IMPORT_NAME: str = "*" -_MACRO_SCAN_PATTERN: re.Pattern[str] = re.compile(r"@|--|/\*|'|\"|`") +_MACRO_SCAN_PATTERN: re.Pattern[str] = re.compile(r"@|--|/\*|'|\"|`|\$") _SYNTHETIC_PACKAGE_PREFIX: str = "_sqlbuild_project_macros_" _OBJECT_ENTRY_LENGTH: int = 2 _MACRO_MODULE_LOAD_LOCK: threading.RLock = threading.RLock() diff --git a/src/sqlbuild/compiler/compile/_helpers/render/sql_vars.py b/src/sqlbuild/compiler/compile/_helpers/render/sql_vars.py index 2d4dd10672..80f496ab11 100644 --- a/src/sqlbuild/compiler/compile/_helpers/render/sql_vars.py +++ b/src/sqlbuild/compiler/compile/_helpers/render/sql_vars.py @@ -46,7 +46,7 @@ from sqlbuild.compiler.sql_analysis.main._skip_quoted_text import skip_quoted_text from sqlbuild.sql_values.types import CollectionRendering -_SQL_INTERPOLATION_SPECIAL: re.Pattern[str] = re.compile(r"['\"`\-/@]") +_SQL_INTERPOLATION_SPECIAL: re.Pattern[str] = re.compile(r"['\"`$\-/@]") _CONTEXT: str = "SQL interpolation" _NATIVE_UNCHANGED: int = 0 diff --git a/src/sqlbuild/compiler/compile/_helpers/sql_tests/core.py b/src/sqlbuild/compiler/compile/_helpers/sql_tests/core.py index 3b49726ee3..687955d01b 100644 --- a/src/sqlbuild/compiler/compile/_helpers/sql_tests/core.py +++ b/src/sqlbuild/compiler/compile/_helpers/sql_tests/core.py @@ -72,6 +72,7 @@ _SQL_TEST_WITH_REQUIREMENT: str = "mock CTEs and one __expected__ CTE" _DIRECT_DEPENDENCY_PATH_LENGTH: int = 2 _SQL_IDENTIFIER_QUOTE_TOKENS: frozenset[str] = frozenset({'"', "`"}) +_SQL_STRING_QUOTE_TOKENS: frozenset[str] = frozenset({SQL_SINGLE_QUOTE_TOKEN, "$"}) @dataclass(frozen=True) @@ -1000,7 +1001,7 @@ def _known_cte_identifier_references(*, sql: str, names_by_key: dict[str, str]) if sql.startswith("/*", index): index = skip_block_comment(sql=sql, start=index, context=_CONTEXT) continue - if sql[index] == SQL_SINGLE_QUOTE_TOKEN: + if sql[index] in _SQL_STRING_QUOTE_TOKENS: index = skip_quoted_text(sql=sql, start=index, context=_CONTEXT) continue if sql[index] in _SQL_IDENTIFIER_QUOTE_TOKENS: diff --git a/src/sqlbuild/compiler/compile/constants.py b/src/sqlbuild/compiler/compile/constants.py index 552d76a929..38c91398ea 100644 --- a/src/sqlbuild/compiler/compile/constants.py +++ b/src/sqlbuild/compiler/compile/constants.py @@ -42,7 +42,7 @@ SQL_ARGUMENT_SEPARATOR_TOKEN: str = "," SQL_STATEMENT_TERMINATOR_TOKEN: str = ";" SQL_SINGLE_QUOTE_TOKEN: str = "'" -SQL_QUOTE_TOKENS: frozenset[str] = frozenset({"'", '"', "`"}) +SQL_QUOTE_TOKENS: frozenset[str] = frozenset({"'", '"', "`", "$"}) SQL_QUALIFIER_SEPARATOR_TOKEN: str = "." SQL_REFERENCE_NAME_QUOTE_TOKENS: frozenset[str] = frozenset({"'", '"'}) SQL_IDENTIFIER_EXTRA_TOKEN: str = "_" diff --git a/src/sqlbuild/compiler/pipeline/main/compile.py b/src/sqlbuild/compiler/pipeline/main/compile.py index 14feaead0a..91f2e04283 100644 --- a/src/sqlbuild/compiler/pipeline/main/compile.py +++ b/src/sqlbuild/compiler/pipeline/main/compile.py @@ -64,6 +64,7 @@ from sqlbuild.rule_engine.exceptions import RulesError from sqlbuild.rule_engine.main.load_config import load_rules_config from sqlbuild.rule_engine.main.render_result import format_result +from sqlbuild.rule_engine.main.render_skipped_rules import format_skipped_type_proof_rules from sqlbuild.rule_engine.main.run_rules import run_rules from sqlbuild.rule_engine.models import RulesCacheConfig, RulesConfig, RulesResult, RulesRunResult from sqlbuild.runtime.contracts.main.open_connection import open_connection_with_hooks @@ -114,11 +115,16 @@ def run_compile_pipeline( discovered_inputs=discovered_inputs, project=project, ), + no_sql_analysis=resolved_options.no_sql_validation, ) if on_progress is not None: + skipped_rules_note: str | None = format_skipped_type_proof_rules( + codes=rules_result.skipped_type_proof_rules + ) on_progress( f"Evaluated rules. (built-in {rules_result.built_in_ms / 1000:.2f}s, " f"custom {rules_result.custom_ms / 1000:.2f}s)" + + ("" if skipped_rules_note is None else f"; {skipped_rules_note}") ) if rules_result.findings: rendered: str = format_result( diff --git a/src/sqlbuild/compiler/planner/_helpers/resolve/refs.py b/src/sqlbuild/compiler/planner/_helpers/resolve/refs.py index 0d00938244..ddd53bdd28 100644 --- a/src/sqlbuild/compiler/planner/_helpers/resolve/refs.py +++ b/src/sqlbuild/compiler/planner/_helpers/resolve/refs.py @@ -47,7 +47,7 @@ _TABLE_FUNCTION_PATTERN: re.Pattern[str] = quoted_reference_call_pattern( SqlReferenceKind.TABLE_FUNCTION ) -_SQL_QUOTE_TOKENS: frozenset[str] = frozenset({"'", '"', "`"}) +_SQL_QUOTE_TOKENS: frozenset[str] = frozenset({"'", '"', "`", "$"}) _CLAUSE_KEYWORDS: frozenset[str] = frozenset( { "WHERE", diff --git a/src/sqlbuild/compiler/references/_helpers/resolution.py b/src/sqlbuild/compiler/references/_helpers/resolution.py index 53c50412ed..5e0a1fae62 100644 --- a/src/sqlbuild/compiler/references/_helpers/resolution.py +++ b/src/sqlbuild/compiler/references/_helpers/resolution.py @@ -25,7 +25,7 @@ _UNRESOLVED_TABLE_FUNCTION_PATTERN: re.Pattern[str] = re.compile( reference_call_prefix_pattern_text(SqlReferenceKind.TABLE_FUNCTION) ) -_SQL_QUOTE_TOKENS: frozenset[str] = frozenset({"'", '"', "`"}) +_SQL_QUOTE_TOKENS: frozenset[str] = frozenset({"'", '"', "`", "$"}) def assert_no_unresolved_sql_markers(*, sql: str, context: str) -> None: diff --git a/src/sqlbuild/compiler/sql_analysis/_helpers/scanning.py b/src/sqlbuild/compiler/sql_analysis/_helpers/scanning.py index bfd1320426..9dae55936e 100644 --- a/src/sqlbuild/compiler/sql_analysis/_helpers/scanning.py +++ b/src/sqlbuild/compiler/sql_analysis/_helpers/scanning.py @@ -8,19 +8,30 @@ from sqlbuild.compiler.compile.exceptions import CompileInputError from sqlbuild.compiler.sql_analysis.constants import ( SQL_CLOSE_PARENTHESIS, + SQL_DOLLAR_QUOTE_CHARACTER, SQL_ESCAPABLE_QUOTE_CHARACTERS, SQL_IDENTIFIER_PREFIX, SQL_OPEN_PARENTHESIS, - SQL_QUOTE_CHARACTERS, + SQL_TEXT_START_CHARACTERS, ) -_PAREN_SCAN_SPECIAL: re.Pattern[str] = re.compile(r"[-/'\"`()]") +_PAREN_SCAN_SPECIAL: re.Pattern[str] = re.compile(r"[-/'\"`$()]") +_DOLLAR_QUOTE_DELIMITER: re.Pattern[str] = re.compile(r"\$(?:[A-Za-z_][A-Za-z0-9_]*)?\$") def skip_quoted_text_impl(*, sql: str, start: int, context: str = "SQL") -> int: - """Skip past quoted SQL text starting at the supplied position.""" + """Skip quoted text at the position, consuming a non-opening ``$`` as one code character.""" + + end: int | None = quoted_text_end_impl(sql=sql, start=start, context=context) + return start + 1 if end is None else end + + +def quoted_text_end_impl(*, sql: str, start: int, context: str = "SQL") -> int | None: + """Return the end of quoted or dollar-quoted text starting at the position, if any.""" quote_character: str = sql[start] + if quote_character == SQL_DOLLAR_QUOTE_CHARACTER: + return _dollar_quoted_text_end(sql=sql, start=start, context=context) index: int = start + 1 while True: index = sql.find(quote_character, index) @@ -36,6 +47,24 @@ def skip_quoted_text_impl(*, sql: str, start: int, context: str = "SQL") -> int: return index + 1 +def _dollar_quoted_text_end(*, sql: str, start: int, context: str) -> int | None: + delimiter: re.Match[str] | None = _DOLLAR_QUOTE_DELIMITER.match(sql, start) + if delimiter is None or (start > 0 and _continues_dollar_word(sql[start - 1])): + return None + closing_index: int = sql.find(delimiter.group(), delimiter.end()) + if closing_index == -1: + raise CompileInputError(f"{context} contains an unclosed quoted string") + return closing_index + len(delimiter.group()) + + +def _continues_dollar_word(character: str) -> bool: + return ( + not character.isascii() + or character.isalnum() + or character in {SQL_IDENTIFIER_PREFIX, SQL_DOLLAR_QUOTE_CHARACTER} + ) + + def skip_line_comment_impl(*, sql: str, start: int) -> int: """Skip past an SQL line comment.""" @@ -62,14 +91,9 @@ def find_matching_paren_impl(*, sql: str, open_paren_index: int, context: str = if special is None: break index = special.start() - if sql.startswith("--", index): - index = skip_line_comment_impl(sql=sql, start=index) - continue - if sql.startswith("/*", index): - index = skip_block_comment_impl(sql=sql, start=index, context=context) - continue - if sql[index] in SQL_QUOTE_CHARACTERS: - index = skip_quoted_text_impl(sql=sql, start=index, context=context) + non_code_end: int | None = _non_code_end(sql=sql, index=index, context=context) + if non_code_end is not None: + index = non_code_end continue if sql[index] == SQL_OPEN_PARENTHESIS: depth += 1 @@ -87,16 +111,11 @@ def iter_code_positions_impl(*, sql: str, context: str = "SQL") -> Iterator[tupl depth: int = 0 index: int = 0 while index < len(sql): - if sql.startswith("--", index): - index = skip_line_comment_impl(sql=sql, start=index) - continue - if sql.startswith("/*", index): - index = skip_block_comment_impl(sql=sql, start=index, context=context) + non_code_end: int | None = _non_code_end(sql=sql, index=index, context=context) + if non_code_end is not None: + index = non_code_end continue character: str = sql[index] - if character in SQL_QUOTE_CHARACTERS: - index = skip_quoted_text_impl(sql=sql, start=index, context=context) - continue if character == SQL_OPEN_PARENTHESIS: depth += 1 elif character == SQL_CLOSE_PARENTHESIS: @@ -106,6 +125,16 @@ def iter_code_positions_impl(*, sql: str, context: str = "SQL") -> Iterator[tupl index += 1 +def _non_code_end(*, sql: str, index: int, context: str) -> int | None: + if sql.startswith("--", index): + return skip_line_comment_impl(sql=sql, start=index) + if sql.startswith("/*", index): + return skip_block_comment_impl(sql=sql, start=index, context=context) + if sql[index] in SQL_TEXT_START_CHARACTERS: + return quoted_text_end_impl(sql=sql, start=index, context=context) + return None + + def is_identifier_start_impl(character: str) -> bool: """Return whether a character can begin an SQL identifier.""" diff --git a/src/sqlbuild/compiler/sql_analysis/constants.py b/src/sqlbuild/compiler/sql_analysis/constants.py index 6724ecbc59..8bba4fab97 100644 --- a/src/sqlbuild/compiler/sql_analysis/constants.py +++ b/src/sqlbuild/compiler/sql_analysis/constants.py @@ -4,6 +4,7 @@ SQL_ESCAPABLE_QUOTE_CHARACTERS: frozenset[str] = frozenset({"'", '"'}) SQL_QUOTE_CHARACTERS: frozenset[str] = frozenset({*SQL_ESCAPABLE_QUOTE_CHARACTERS, "`"}) +SQL_TEXT_START_CHARACTERS: frozenset[str] = frozenset({*SQL_QUOTE_CHARACTERS, "$"}) SQL_OPEN_PARENTHESIS: str = "(" SQL_CLOSE_PARENTHESIS: str = ")" SQL_OPEN_BRACKET: str = "[" @@ -11,6 +12,7 @@ SQL_STRING_QUOTE_CHARACTER: str = "'" SQL_ESCAPE_CHARACTER: str = "\\" SQL_DOLLAR_QUOTE_DELIMITER: str = "$$" +SQL_DOLLAR_QUOTE_CHARACTER: str = "$" SQL_IDENTIFIER_PREFIX: str = "_" SQL_NULL_KEYWORD: str = "NULL" SNOWFLAKE_DIALECT_NAME: str = "snowflake" diff --git a/src/sqlbuild/rule_engine/_helpers/engine/native.py b/src/sqlbuild/rule_engine/_helpers/engine/native.py index 51ec3a3b6a..30dccae1a4 100644 --- a/src/sqlbuild/rule_engine/_helpers/engine/native.py +++ b/src/sqlbuild/rule_engine/_helpers/engine/native.py @@ -48,6 +48,7 @@ RULE_CONTEXT_PROJECT_FACT, RULE_CONTEXT_TESTS_FACT, RULES_NATIVE_API_VERSION, + TYPE_PROOF_RULE_CODES, ) from sqlbuild.rule_engine.exceptions import RulesError from sqlbuild.rule_engine.models import ( @@ -61,7 +62,6 @@ _CUSTOM_HOST_REQUIRED: str = "selected custom rules require a custom host" _NATIVE_CACHE_MISSES_PATTERN: re.Pattern[str] = re.compile(r"native_cache_misses=(\d+)") -_EXPLICIT_OUTPUT_TYPES_RULE: str = "SQBRCONTRACT105" def evaluate_native( @@ -89,7 +89,9 @@ def evaluate_native( "models": _model_payloads( project=project, dialect=dialect, - include_type_proof=_rule_selected(config=config, code=_EXPLICIT_OUTPUT_TYPES_RULE), + include_type_proof=any( + _rule_selected(config=config, code=code) for code in TYPE_PROOF_RULE_CODES + ), ), "sql_tests": _sql_test_payloads(project), "sql_scenarios": _sql_scenario_payloads(project), diff --git a/src/sqlbuild/rule_engine/_helpers/run/rules.py b/src/sqlbuild/rule_engine/_helpers/run/rules.py index 3979bca495..e9a3004feb 100644 --- a/src/sqlbuild/rule_engine/_helpers/run/rules.py +++ b/src/sqlbuild/rule_engine/_helpers/run/rules.py @@ -38,6 +38,7 @@ finalize_native_findings, native_catalogue, ) +from sqlbuild.rule_engine.constants import TYPE_PROOF_RULE_CODES from sqlbuild.rule_engine.exceptions import RulesError from sqlbuild.rule_engine.main.load_config import load_rules_config from sqlbuild.rule_engine.models import ( @@ -73,6 +74,7 @@ def evaluate_rules( selected_keys: frozenset[CompiledObjectKey] | None = None, prepared_sql: PreparedSqlLint | None = None, expansion_reuse: SqlExpansionReuse | None = None, + no_sql_analysis: bool = False, ) -> RulesRunResult: """Evaluate independent rule phases concurrently, then finalize their combined findings.""" effective_config: RulesConfig = resolve_rule_ignore_selectors( @@ -89,6 +91,12 @@ def evaluate_rules( selected: tuple[Rule, ...] = select_rules( catalogue=catalogue, config=effective_config, project_dir=resolved_project_dir ) + skipped_type_proof_rules: tuple[str, ...] = () + if no_sql_analysis or not graph.project.settings.sql_analysis: + skipped_type_proof_rules = tuple( + rule.code for rule in selected if rule.code in TYPE_PROOF_RULE_CODES + ) + selected = tuple(rule for rule in selected if rule.code not in TYPE_PROOF_RULE_CODES) native_rules: tuple[Rule, ...] = tuple(rule for rule in selected if not rule.custom) custom_rules: tuple[Rule, ...] = tuple(rule for rule in selected if rule.custom) if custom_rules: @@ -149,6 +157,7 @@ def evaluate_rules( custom_ms=result.custom_ms, cache_hits=result.cache_hits + sql_result.cache_hits, cache_misses=result.cache_misses + sql_result.cache_misses, + skipped_type_proof_rules=skipped_type_proof_rules, ) diff --git a/src/sqlbuild/rule_engine/constants.py b/src/sqlbuild/rule_engine/constants.py index ed7084a29b..3561e0ae4a 100644 --- a/src/sqlbuild/rule_engine/constants.py +++ b/src/sqlbuild/rule_engine/constants.py @@ -22,6 +22,7 @@ CONTRACT_TIMESTAMP_NAME_CODE: str = "SQBRCONTRACT103" CONTRACT_DATE_NAME_CODE: str = "SQBRCONTRACT104" EMPTY_INPUT_TEST_CODE: str = "SQBRTEST203" +TYPE_PROOF_RULE_CODES: frozenset[str] = frozenset({"SQBRCONTRACT105"}) BOOLEAN_TYPE: str = "BOOLEAN" DATE_TYPE: str = "DATE" diff --git a/src/sqlbuild/rule_engine/main/render_skipped_rules.py b/src/sqlbuild/rule_engine/main/render_skipped_rules.py new file mode 100644 index 0000000000..f883250905 --- /dev/null +++ b/src/sqlbuild/rule_engine/main/render_skipped_rules.py @@ -0,0 +1,9 @@ +"""Skipped type-proof rules note entrypoint.""" + + +def format_skipped_type_proof_rules(*, codes: tuple[str, ...]) -> str | None: + """Describe type-proof rules skipped because SQL analysis is disabled, if any.""" + + if not codes: + return None + return f"skipped type-proof rules ({', '.join(codes)}) because SQL analysis is disabled" diff --git a/src/sqlbuild/rule_engine/main/run_rules.py b/src/sqlbuild/rule_engine/main/run_rules.py index 46b7136910..c5ecda7344 100644 --- a/src/sqlbuild/rule_engine/main/run_rules.py +++ b/src/sqlbuild/rule_engine/main/run_rules.py @@ -24,6 +24,7 @@ def run_rules( selected_keys: frozenset[CompiledObjectKey] | None = None, prepared_sql: PreparedSqlLint | None = None, expansion_reuse: SqlExpansionReuse | None = None, + no_sql_analysis: bool = False, ) -> RulesRunResult: """Run selected native built-ins before selected custom Python rules.""" @@ -36,4 +37,5 @@ def run_rules( selected_keys=selected_keys, prepared_sql=prepared_sql, expansion_reuse=expansion_reuse, + no_sql_analysis=no_sql_analysis, ) diff --git a/src/sqlbuild/rule_engine/models.py b/src/sqlbuild/rule_engine/models.py index a05beb6bfe..f3651eef92 100644 --- a/src/sqlbuild/rule_engine/models.py +++ b/src/sqlbuild/rule_engine/models.py @@ -341,6 +341,7 @@ class RulesRunResult: custom_ms: int cache_hits: int = 0 cache_misses: int = 0 + skipped_type_proof_rules: tuple[str, ...] = () @dataclass(frozen=True) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py index 4fc2e65bff..b2220b0da7 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py @@ -227,3 +227,13 @@ class EmptyInputTestRuleCacheTestCase: replacement_mock: str expected_first_codes: tuple[str, ...] expected_second_codes: tuple[str, ...] + + +@dataclass(frozen=True) +class TypeProofRuleCompileTestCase: + description: str + project_toml: str + extra_args: tuple[str, ...] + expected_returncode: int + expected_rule_findings: int + expected_note_count: int diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_type_proof_rules.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_type_proof_rules.py new file mode 100644 index 0000000000..6cc5f4990d --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_type_proof_rules.py @@ -0,0 +1,141 @@ +"""E2E tests for type-proof rules when SQL analysis is disabled.""" + +from __future__ import annotations + +import json +import subprocess +from pathlib import Path +from typing import Any + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.compile._test_types import ( + TypeProofRuleCompileTestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project, run_sqb + +_SKIPPED_NOTE: str = ( + "note: skipped type-proof rules (SQBRCONTRACT105) because SQL analysis is disabled" +) +_PROJECT_TOML: str = ( + 'name = "orders"\nadapter = "duckdb"\n\n[rules]\nselect = ["SQBRCONTRACT105"]\n' +) +_ORDERS_MODEL: str = ( + "MODEL (\n" + ' description "Orders",\n' + " contract enforced,\n" + " columns (\n" + " order_id (type INTEGER, nullable false),\n" + " status (type VARCHAR),\n" + " ),\n" + ");\n\n" + "WITH final AS (\n" + " SELECT 1 AS order_id, 'placed' AS status\n" + " UNION ALL\n" + " SELECT 2 AS order_id, 'shipped' AS status\n" + ")\n" + "SELECT order_id, status FROM final\n" +) + + +@pytest.mark.parametrize( + "test_case", + ( + TypeProofRuleCompileTestCase( + description="analysis enabled evaluates type-proof rules", + project_toml=_PROJECT_TOML, + extra_args=(), + expected_returncode=1, + expected_rule_findings=4, + expected_note_count=0, + ), + TypeProofRuleCompileTestCase( + description="no-sql-analysis flag skips type-proof rules", + project_toml=_PROJECT_TOML, + extra_args=("--no-sql-analysis",), + expected_returncode=0, + expected_rule_findings=0, + expected_note_count=1, + ), + TypeProofRuleCompileTestCase( + description="project setting skips type-proof rules", + project_toml=_PROJECT_TOML + "\n[settings]\nsql_analysis = false\n", + extra_args=(), + expected_returncode=0, + expected_rule_findings=0, + expected_note_count=1, + ), + ), + ids=lambda case: case.description, +) +def test_given_typed_contract_when_compiling_then_type_proof_rules_follow_sql_analysis( + test_case: TypeProofRuleCompileTestCase, + tmp_path: Path, +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="orders", + repo_files={ + "sqlbuild_project.toml": test_case.project_toml, + "models/orders.sql": _ORDERS_MODEL, + }, + ) + + text_result: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", "compile", "--no-cache", *test_case.extra_args), + project_dir=project_dir, + ) + json_result: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", "compile", "--json", "--no-cache", *test_case.extra_args), + project_dir=project_dir, + ) + + text_output: str = text_result.stdout + text_result.stderr + assert text_result.returncode == test_case.expected_returncode, text_output + assert text_result.stderr.count(_SKIPPED_NOTE) == test_case.expected_note_count + assert _SKIPPED_NOTE not in text_result.stdout + assert text_result.stdout.count("error[SQBRCONTRACT105]") == test_case.expected_rule_findings + assert text_result.stdout.rstrip().splitlines()[-1].startswith(" Wrote:"), text_output + payload: dict[str, Any] = json.loads(json_result.stdout) + rule_codes: list[object] = [diagnostic["code"] for diagnostic in payload["diagnostics"]] + assert json_result.returncode == test_case.expected_returncode + assert rule_codes.count("SQBRCONTRACT105") == test_case.expected_rule_findings + assert json_result.stderr.count(_SKIPPED_NOTE) == test_case.expected_note_count + + +@pytest.mark.parametrize( + "test_case", + ( + TypeProofRuleCompileTestCase( + description="build without SQL analysis skips type-proof rules", + project_toml=_PROJECT_TOML, + extra_args=("--no-sql-analysis",), + expected_returncode=0, + expected_rule_findings=0, + expected_note_count=1, + ), + ), + ids=lambda case: case.description, +) +def test_given_typed_contract_when_building_without_sql_analysis_then_type_proof_rules_skip( + test_case: TypeProofRuleCompileTestCase, + tmp_path: Path, +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="orders", + repo_files={ + "sqlbuild_project.toml": test_case.project_toml, + "models/orders.sql": _ORDERS_MODEL, + }, + ) + + result: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", "build", *test_case.extra_args), + project_dir=project_dir, + ) + + output: str = result.stdout + result.stderr + assert result.returncode == test_case.expected_returncode, output + assert output.count("SQBRCONTRACT105") == test_case.expected_note_count + assert output.count(_SKIPPED_NOTE.removeprefix("note: ")) == test_case.expected_note_count diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/query/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/query/_test_types.py index 316f92543c..42d64cf8f6 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/query/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/query/_test_types.py @@ -24,3 +24,11 @@ class QueryFileErrorCliTestCase: expected_stderr_fragment: str repo_files: dict[str, str] = field(default_factory=dict) binary_files: dict[str, bytes] = field(default_factory=dict) + + +@dataclass(frozen=True) +class QueryExecutionErrorCliTestCase: + description: str + command: tuple[str, ...] + expected_stderr_fragment: str + expected_returncode: int = 2 diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/query/test_query.py b/tests/e2e/src/sqlbuild/cli/commands/main/query/test_query.py index 61c0888703..b87431f535 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/query/test_query.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/query/test_query.py @@ -7,6 +7,7 @@ from tests.e2e.src.sqlbuild.cli.commands.main.query._test_types import ( QueryCliTestCase, + QueryExecutionErrorCliTestCase, QueryFileCliTestCase, QueryFileErrorCliTestCase, ) @@ -164,3 +165,84 @@ def test_given_invalid_query_file_input_when_running_query_then_reports_actionab assert result.returncode == 1 assert test_case.expected_stderr_fragment in result.stderr + + +@pytest.mark.parametrize( + "test_case", + ( + QueryExecutionErrorCliTestCase( + description="reports a missing relation as a CLI error", + command=("query", "SELECT * FROM missing_orders"), + expected_stderr_fragment=( + "error[C108]: Catalog Error: Table with name missing_orders does not exist!" + ), + ), + QueryExecutionErrorCliTestCase( + description="reports a syntax error as a CLI error", + command=("query", "SELECT FROM WHERE"), + expected_stderr_fragment="error[C108]: Parser Error: syntax error", + ), + ), + ids=lambda case: case.description, +) +def test_given_warehouse_rejects_query_when_running_query_then_reports_cli_error( + tmp_path: Path, + test_case: QueryExecutionErrorCliTestCase, +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="query_error_project", + repo_files={ + "sqlbuild_project.toml": ( + 'name = "query_error_project"\nadapter = "duckdb"\n\n' + '[connection]\ndatabase = "query.duckdb"\n' + ), + }, + ) + + result: subprocess.CompletedProcess[str] = run_sqb( + command=test_case.command, + project_dir=project_dir, + ) + + assert result.returncode == test_case.expected_returncode + assert test_case.expected_stderr_fragment in result.stderr + assert "Traceback" not in result.stdout + result.stderr + + +@pytest.mark.parametrize( + "test_case", + ( + QueryExecutionErrorCliTestCase( + description="keeps machine-readable stdout empty when the query fails", + command=("query", "SELECT * FROM missing_orders", "--format", "json"), + expected_stderr_fragment=( + "error[C108]: Catalog Error: Table with name missing_orders does not exist!" + ), + ), + ), + ids=lambda case: case.description, +) +def test_given_json_query_fails_when_running_query_then_stdout_stays_empty( + tmp_path: Path, + test_case: QueryExecutionErrorCliTestCase, +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="query_json_error_project", + repo_files={ + "sqlbuild_project.toml": ( + 'name = "query_json_error_project"\nadapter = "duckdb"\n\n' + '[connection]\ndatabase = "query.duckdb"\n' + ), + }, + ) + + result: subprocess.CompletedProcess[str] = run_sqb( + command=test_case.command, + project_dir=project_dir, + ) + + assert result.returncode == test_case.expected_returncode + assert test_case.expected_stderr_fragment in result.stderr + assert result.stdout == "" diff --git a/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py b/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py index cae9c3ac5e..522599eb7f 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py @@ -71,6 +71,35 @@ class RulesIntegrationTestCase: expected_code: str +@dataclass(frozen=True) +class DollarQuotedLiteralBuildTestCase: + """One model projection with dollar-quoted literals and its built rows.""" + + description: str + projection: str + expected_rows: tuple[tuple[object, ...], ...] + + +@dataclass(frozen=True) +class ImplicitAliasRuleIntegrationTestCase: + """One model query and its expected unused-alias finding locations.""" + + description: str + query_sql: str + expected_locations: tuple[tuple[int, int], ...] + + +@dataclass(frozen=True) +class NumericRangeDecisionIntegrationTestCase: + """One numeric range or value-list predicate and its expected rule findings.""" + + description: str + predicate: str + expected_findings: tuple[tuple[str, str], ...] + expected_exit_code: int + constants: str = "" + + @dataclass(frozen=True) class RulePassIntegrationTestCase: """One compiler-integrated Rules command expected to pass.""" diff --git a/tests/integration/src/sqlbuild/cli/commands/main/test_dollar_quoted_literals.py b/tests/integration/src/sqlbuild/cli/commands/main/test_dollar_quoted_literals.py new file mode 100644 index 0000000000..3eae8372df --- /dev/null +++ b/tests/integration/src/sqlbuild/cli/commands/main/test_dollar_quoted_literals.py @@ -0,0 +1,66 @@ +"""Integration coverage for dollar-quoted string literals through the real CLI.""" + +from __future__ import annotations + +from pathlib import Path + +import duckdb +import pytest + +from tests.integration.src.sqlbuild.cli.commands.main._test_types import ( + DollarQuotedLiteralBuildTestCase, +) +from tests.integration.src.sqlbuild.cli.commands.main.helpers import run_build + + +@pytest.mark.parametrize( + "test_case", + [ + DollarQuotedLiteralBuildTestCase( + description="dollar-quoted literals stay opaque while real references build", + projection=( + '$$Customer\'s order -- __ref("missing_orders") costs $5$$ AS order_label,\n' + ' $note$ $$ nested __seed("missing_seed") @missing_macro() $note$ AS order_note,\n' + " o.order_id" + ), + expected_rows=( + ( + 'Customer\'s order -- __ref("missing_orders") costs $5', + ' $$ nested __seed("missing_seed") @missing_macro() ', + 1, + ), + ), + ) + ], + ids=lambda case: case.description, +) +def test_given_dollar_quoted_literal_when_building_then_literal_is_preserved_and_refs_resolve( + test_case: DollarQuotedLiteralBuildTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + (tmp_path / "sqlbuild_project.toml").write_text( + 'name = "orders"\nadapter = "duckdb"\n\n[connection]\ndatabase = "orders.duckdb"\n', + encoding="utf-8", + ) + models: Path = tmp_path / "models" + models.mkdir() + (models / "orders.sql").write_text( + 'MODEL (description "Orders", materialized table);\nSELECT 1 AS order_id\n', + encoding="utf-8", + ) + (models / "order_labels.sql").write_text( + 'MODEL (description "Order labels", materialized table);\n' + f"SELECT\n {test_case.projection}\n" + 'FROM __ref("orders") AS o\n', + encoding="utf-8", + ) + + exit_code, output = run_build(project_dir=tmp_path, flags=(), capsys=capsys) + + assert exit_code == 0, output + with duckdb.connect(str(tmp_path / "orders.duckdb"), read_only=True) as connection: + rows: list[tuple[object, ...]] = connection.execute( + "SELECT order_label, order_note, order_id FROM main.order_labels" + ).fetchall() + assert tuple(rows) == test_case.expected_rows diff --git a/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py b/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py index ec9bdab20f..98e07c6822 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py @@ -19,6 +19,8 @@ from tests.integration.src.sqlbuild.cli.commands.main._test_types import ( DynamicPivotRulesIntegrationTestCase, ExplicitContractOutputRuleIntegrationTestCase, + ImplicitAliasRuleIntegrationTestCase, + NumericRangeDecisionIntegrationTestCase, RulePassIntegrationTestCase, RulesIntegrationTestCase, TypedContractRuleIntegrationTestCase, @@ -158,6 +160,83 @@ def test_given_exact_internal_edge_exception_when_compiling_then_live_edge_passe ) +@pytest.mark.parametrize( + "test_case", + [ + NumericRangeDecisionIntegrationTestCase( + description="literal between bounds and value list are reported", + predicate=( + "TRY_CAST(code AS INTEGER) BETWEEN 30 AND 61\n" + " OR TRY_CAST(code AS INTEGER) NOT IN (30, 31, 45)" + ), + expected_findings=( + ( + "non-canonical numeric comparison: TRY_CAST(code AS INTEGER) BETWEEN 30 AND 61", + "Declare the threshold as a CONSTANT and compare through " + '@const(""); only -1, 0, and 1 are self-explanatory.', + ), + ( + "non-canonical numeric comparison: " + "TRY_CAST(code AS INTEGER) NOT IN (30, 31, 45)", + "Declare the values as a list or set CONSTANT and compare through " + '[NOT] IN @const(""); only -1, 0, and 1 are self-explanatory.', + ), + ), + expected_exit_code=1, + ), + NumericRangeDecisionIntegrationTestCase( + description="constant-backed between bounds and value list pass", + predicate=( + 'TRY_CAST(code AS INTEGER) BETWEEN @const("_low_code") AND @const("_high_code")\n' + ' OR TRY_CAST(code AS INTEGER) NOT IN @const("_held_codes")' + ), + expected_findings=(), + expected_exit_code=0, + constants=" constants (_low_code 30, _high_code 61, _held_codes [30, 31, 45]),\n", + ), + NumericRangeDecisionIntegrationTestCase( + description="canonical between bounds and value list pass", + predicate=( + "TRY_CAST(code AS INTEGER) BETWEEN 0 AND 1\n" + " OR TRY_CAST(code AS INTEGER) IN (-1, 0, 1)" + ), + expected_findings=(), + expected_exit_code=0, + ), + ], + ids=lambda case: case.description, +) +def test_given_numeric_range_or_value_list_when_compiling_then_literal_decisions_are_reported( + test_case: NumericRangeDecisionIntegrationTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + """BETWEEN bounds and IN members follow the same named-decision rule as comparisons.""" + (tmp_path / "sqlbuild_project.toml").write_text( + 'name = "orders"\nadapter = "duckdb"\n\n[rules]\nselect = ["SQBRDECLARATION102"]\n', + encoding="utf-8", + ) + model: Path = tmp_path / "models" / "orders.sql" + model.parent.mkdir() + model.write_text( + "MODEL (\n" + ' description "Orders",\n' + test_case.constants + ");\n\n" + "SELECT code\n" + "FROM (SELECT '31' AS code) AS items\n" + f"WHERE {test_case.predicate}\n", + encoding="utf-8", + ) + + exit_code: int = main(["--project-dir", str(tmp_path), "compile", "--json"]) + result: dict[str, object] = json.loads(capsys.readouterr().out) + diagnostics: list[dict[str, object]] = cast(list[dict[str, object]], result["diagnostics"]) + + assert exit_code == test_case.expected_exit_code + assert tuple((diagnostic["message"], diagnostic["help"]) for diagnostic in diagnostics) == ( + test_case.expected_findings + ) + + @pytest.mark.parametrize( "test_case", [RulePassIntegrationTestCase("constant-backed numeric decision", 0, ())], @@ -956,6 +1035,87 @@ def test_given_table_function_argument_when_running_alias_rule_then_left_alias_i assert payload["findings"] == [] +@pytest.mark.parametrize( + "test_case", + [ + ImplicitAliasRuleIntegrationTestCase( + description="unused implicit join alias is reported", + query_sql=( + "SELECT o.order_id\n" + 'FROM __ref("orders") o\n' + 'INNER JOIN __ref("customers") c ON o.customer_id = o.order_id\n' + ), + expected_locations=((4, 31),), + ), + ImplicitAliasRuleIntegrationTestCase( + description="unused implicit subquery alias is reported", + query_sql='SELECT order_id\nFROM (SELECT order_id FROM __ref("orders")) recent\n', + expected_locations=((3, 45),), + ), + ImplicitAliasRuleIntegrationTestCase( + description="implicit and explicit unused aliases are reported alike", + query_sql='SELECT order_id\nFROM __ref("orders") AS o, __ref("customers") c\n', + expected_locations=((3, 22), (3, 47)), + ), + ImplicitAliasRuleIntegrationTestCase( + description="whole-row alias uses are clean", + query_sql=( + "SELECT to_json(o) AS payload, c\n" + 'FROM __ref("orders") o\n' + 'INNER JOIN __ref("customers") AS c ON o.customer_id = c.customer_id\n' + ), + expected_locations=(), + ), + ImplicitAliasRuleIntegrationTestCase( + description="join modifiers after unaliased relations are clean", + query_sql='SELECT *\nFROM __ref("orders") POSITIONAL JOIN __ref("customers")\n', + expected_locations=(), + ), + ImplicitAliasRuleIntegrationTestCase( + description="used implicit aliases are clean", + query_sql=( + "SELECT o.order_id, c.customer_id\n" + 'FROM __ref("orders") o\n' + 'INNER JOIN __ref("customers") c ON o.customer_id = c.customer_id\n' + ), + expected_locations=(), + ), + ], + ids=lambda case: case.description, +) +def test_given_implicit_table_alias_when_running_alias_rule_then_usage_decides_finding( + test_case: ImplicitAliasRuleIntegrationTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + """Implicit aliases are held to the same unused-alias contract as AS aliases.""" + (tmp_path / "sqlbuild_project.toml").write_text( + 'name = "orders"\nadapter = "duckdb"\n', encoding="utf-8" + ) + models: Path = tmp_path / "models" + models.mkdir() + (models / "orders.sql").write_text( + 'MODEL (description "Orders");\nSELECT 1 AS order_id, 2 AS customer_id\n', + encoding="utf-8", + ) + (models / "customers.sql").write_text( + 'MODEL (description "Customers");\nSELECT 2 AS customer_id\n', encoding="utf-8" + ) + (models / "order_customers.sql").write_text( + 'MODEL (description "Order customers");\n' + test_case.query_sql, encoding="utf-8" + ) + + exit_code: int = main(["--project-dir", str(tmp_path), "rules", "--json", "run", "SQBRSQL023"]) + + payload: dict[str, object] = json.loads(capsys.readouterr().out) + findings: list[dict[str, object]] = cast(list[dict[str, object]], payload["findings"]) + assert exit_code == int(bool(test_case.expected_locations)) + assert tuple((finding["line"], finding["column"]) for finding in findings) == ( + test_case.expected_locations + ) + assert {finding["path"] for finding in findings} <= {"models/order_customers.sql"} + + @pytest.mark.parametrize( "test_case", [RulesIntegrationTestCase("ceremonial SQL test passes terminal rule", 0, "SQBRSQL035")], diff --git a/tests/unit/src/sqlbuild/cli/commands/main/inspection/query/_test_types.py b/tests/unit/src/sqlbuild/cli/commands/main/inspection/query/_test_types.py new file mode 100644 index 0000000000..3cb87d14c1 --- /dev/null +++ b/tests/unit/src/sqlbuild/cli/commands/main/inspection/query/_test_types.py @@ -0,0 +1,12 @@ +from __future__ import annotations + +from dataclasses import dataclass + + +@dataclass(frozen=True) +class QueryExecutionErrorTestCase: + description: str + sql: str + expected_message_fragment: str + expected_code: str = "C108" + expected_exit_code: int = 2 diff --git a/tests/unit/src/sqlbuild/cli/commands/main/inspection/query/test_query.py b/tests/unit/src/sqlbuild/cli/commands/main/inspection/query/test_query.py new file mode 100644 index 0000000000..16e03d9725 --- /dev/null +++ b/tests/unit/src/sqlbuild/cli/commands/main/inspection/query/test_query.py @@ -0,0 +1,46 @@ +from __future__ import annotations + +from pathlib import Path + +import duckdb +import pytest + +from sqlbuild.cli.commands.exceptions import QueryExecutionError +from sqlbuild.cli.commands.main.inspection._query import run_query +from tests.unit.src.sqlbuild.cli.commands.main.inspection.query._test_types import ( + QueryExecutionErrorTestCase, +) + + +@pytest.mark.parametrize( + "test_case", + [ + QueryExecutionErrorTestCase( + description="missing relation", + sql="SELECT * FROM missing_orders", + expected_message_fragment="Table with name missing_orders does not exist", + ), + QueryExecutionErrorTestCase( + description="syntax error", + sql="SELECT FROM WHERE", + expected_message_fragment="syntax error", + ), + ], + ids=lambda case: case.description, +) +def test_given_warehouse_rejects_query_when_running_query_then_raises_cli_error( + test_case: QueryExecutionErrorTestCase, + tmp_path: Path, +) -> None: + (tmp_path / "sqlbuild_project.toml").write_text( + 'name = "orders"\nadapter = "duckdb"\n\n[connection]\ndatabase = "orders.duckdb"\n', + encoding="utf-8", + ) + + with pytest.raises(QueryExecutionError) as raised: + run_query(project_dir=tmp_path, sql=test_case.sql) + + assert raised.value.code == test_case.expected_code + assert raised.value.exit_code == test_case.expected_exit_code + assert test_case.expected_message_fragment in raised.value.message + assert isinstance(raised.value.__cause__, duckdb.Error) diff --git a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_references.py b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_references.py index 9084472122..4d13a8b42b 100644 --- a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_references.py +++ b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_references.py @@ -27,7 +27,24 @@ (SqlReferenceKind.SOURCE, "orders", None), (SqlReferenceKind.DBT_REF, "customers", "shop"), ), - ) + ), + SqlReferenceExtractionTestCase( + description="dollar-quoted text hides embedded references", + sql=( + 'SELECT $$Customer\'s order -- __ref("ignored") $5$$ AS order_label, ' + "$note$ $$ __seed('ignored') $note$ AS order_note " + 'FROM __ref("orders")' + ), + expected_references=((SqlReferenceKind.REF, "orders", None),), + ), + SqlReferenceExtractionTestCase( + description="general scanner hides references in dollar-quoted text", + sql=( + 'SELECT $$Customer\'s order __ref("ignored")$$ AS order_label ' + 'FROM __table_fn("expand_orders")(1)' + ), + expected_references=((SqlReferenceKind.TABLE_FUNCTION, "expand_orders", None),), + ), ], ids=lambda case: case.description, ) @@ -68,6 +85,11 @@ def test_given_simple_references_when_extracting_then_returns_authored_order( sql='SELECT * FROM __ref("orders") WHERE note = \'unterminated', expected_error="unclosed quoted string", ), + SqlReferenceExtractionErrorTestCase( + description="unclosed dollar-quoted string preserves diagnostic", + sql='SELECT * FROM __ref("orders") WHERE note = $$customer\'s', + expected_error="unclosed quoted string", + ), ], ids=lambda case: case.description, ) diff --git a/tests/unit/src/sqlbuild/compiler/sql_analysis/_helpers/test_scanning.py b/tests/unit/src/sqlbuild/compiler/sql_analysis/_helpers/test_scanning.py index 3245e9b7aa..b57f732b56 100644 --- a/tests/unit/src/sqlbuild/compiler/sql_analysis/_helpers/test_scanning.py +++ b/tests/unit/src/sqlbuild/compiler/sql_analysis/_helpers/test_scanning.py @@ -42,6 +42,21 @@ quoted_sql="'a''''b'", expected_end=len("'a''''b'"), ), + SkipQuotedTextSuccessTestCase( + description="dollar quote hides apostrophes, comments and single dollars", + quoted_sql="$$customer's order -- $5 $$", + expected_end=len("$$customer's order -- $5 $$"), + ), + SkipQuotedTextSuccessTestCase( + description="tagged dollar quote ends only at its own tag", + quoted_sql="$order$ $$ it's $order$", + expected_end=len("$order$ $$ it's $order$"), + ), + SkipQuotedTextSuccessTestCase( + description="positional parameter dollar is one code character", + quoted_sql="$1", + expected_end=1, + ), ), ids=lambda case: case.description, ) @@ -64,6 +79,12 @@ def test_given_quoted_text_when_skipping_then_returns_end_position( context="SQL reference", expected_error="SQL reference contains an unclosed quoted string", ), + SkipQuotedTextErrorTestCase( + description="unclosed dollar quote", + sql="$order$ customer's $$", + context="SQL reference", + expected_error="SQL reference contains an unclosed quoted string", + ), ), ids=lambda case: case.description, ) @@ -87,6 +108,16 @@ def test_given_unclosed_quote_when_skipping_then_raises_contextual_error( sql="a'(b'`c`--d\n/*e*/f", expected_positions=((0, 0), (17, 0)), ), + IterCodePositionsTestCase( + description="dollar-quoted text is skipped", + sql="a $$'(--$$b", + expected_positions=((0, 0), (1, 0), (10, 0)), + ), + IterCodePositionsTestCase( + description="dollars inside identifiers and parameters remain code", + sql="a$$b $1", + expected_positions=tuple((index, 0) for index in range(7)), + ), IterCodePositionsTestCase( description="unbalanced close parenthesis goes below zero depth", sql=")a", diff --git a/tests/unit/src/sqlbuild/rule_engine/main/evaluate/test_evaluate.py b/tests/unit/src/sqlbuild/rule_engine/main/evaluate/test_evaluate.py index f3e97417cc..5d9d979b7d 100644 --- a/tests/unit/src/sqlbuild/rule_engine/main/evaluate/test_evaluate.py +++ b/tests/unit/src/sqlbuild/rule_engine/main/evaluate/test_evaluate.py @@ -206,6 +206,24 @@ select=("SQBRDECLARATION102",), expected_codes=("SQBRDECLARATION102",), ), + PolicyEvaluationTestCase( + description="numeric between bounds fault", + model_name="commerce__mart__orders", + relative_path="models/mart/commerce__mart__orders.sql", + sql="SELECT id FROM items WHERE item_count BETWEEN 30 AND 61", + config_values={"materialized": "table", "contract": "enforced"}, + select=("SQBRDECLARATION102",), + expected_codes=("SQBRDECLARATION102",), + ), + PolicyEvaluationTestCase( + description="numeric value list faults", + model_name="commerce__mart__orders", + relative_path="models/mart/commerce__mart__orders.sql", + sql="SELECT id FROM items WHERE item_count IN (30, 31, 45)", + config_values={"materialized": "table", "contract": "enforced"}, + select=("SQBRDECLARATION102",), + expected_codes=("SQBRDECLARATION102",), + ), PolicyEvaluationTestCase( description="canonical numeric decision passes", model_name="commerce__mart__orders", diff --git a/tests/unit/src/sqlbuild/rule_engine/main/render_skipped_rules/_test_types.py b/tests/unit/src/sqlbuild/rule_engine/main/render_skipped_rules/_test_types.py new file mode 100644 index 0000000000..cb6b90fdfe --- /dev/null +++ b/tests/unit/src/sqlbuild/rule_engine/main/render_skipped_rules/_test_types.py @@ -0,0 +1,10 @@ +from __future__ import annotations + +from dataclasses import dataclass + + +@dataclass(frozen=True) +class SkippedTypeProofRulesNoteTestCase: + description: str + codes: tuple[str, ...] + expected_note: str | None diff --git a/tests/unit/src/sqlbuild/rule_engine/main/render_skipped_rules/test_render_skipped_rules.py b/tests/unit/src/sqlbuild/rule_engine/main/render_skipped_rules/test_render_skipped_rules.py new file mode 100644 index 0000000000..812a893850 --- /dev/null +++ b/tests/unit/src/sqlbuild/rule_engine/main/render_skipped_rules/test_render_skipped_rules.py @@ -0,0 +1,32 @@ +from __future__ import annotations + +import pytest + +from sqlbuild.rule_engine.main.render_skipped_rules import format_skipped_type_proof_rules +from tests.unit.src.sqlbuild.rule_engine.main.render_skipped_rules._test_types import ( + SkippedTypeProofRulesNoteTestCase, +) + + +@pytest.mark.parametrize( + "test_case", + ( + SkippedTypeProofRulesNoteTestCase( + description="skipped rules produce one note", + codes=("SQBRCONTRACT105",), + expected_note=( + "skipped type-proof rules (SQBRCONTRACT105) because SQL analysis is disabled" + ), + ), + SkippedTypeProofRulesNoteTestCase( + description="no skipped rules produce no note", + codes=(), + expected_note=None, + ), + ), + ids=lambda case: case.description, +) +def test_given_skipped_type_proof_rules_when_formatting_note_then_names_skipped_codes( + test_case: SkippedTypeProofRulesNoteTestCase, +) -> None: + assert format_skipped_type_proof_rules(codes=test_case.codes) == test_case.expected_note diff --git a/tests/unit/src/sqlbuild/rule_engine/main/run_rules/_test_types.py b/tests/unit/src/sqlbuild/rule_engine/main/run_rules/_test_types.py new file mode 100644 index 0000000000..b9d3397c7f --- /dev/null +++ b/tests/unit/src/sqlbuild/rule_engine/main/run_rules/_test_types.py @@ -0,0 +1,12 @@ +from __future__ import annotations + +from dataclasses import dataclass + + +@dataclass(frozen=True) +class TypeProofRuleSkipTestCase: + description: str + no_sql_analysis: bool + settings_toml: str + expected_skipped_rules: tuple[str, ...] + expected_finding_codes: tuple[str, ...] diff --git a/tests/unit/src/sqlbuild/rule_engine/main/run_rules/test_run_rules.py b/tests/unit/src/sqlbuild/rule_engine/main/run_rules/test_run_rules.py new file mode 100644 index 0000000000..8f1ac856b1 --- /dev/null +++ b/tests/unit/src/sqlbuild/rule_engine/main/run_rules/test_run_rules.py @@ -0,0 +1,87 @@ +from __future__ import annotations + +from pathlib import Path + +import pytest + +from sqlbuild.adapter.contract.classes.base_adapter import BaseAdapter +from sqlbuild.cli.commands._helpers.runtime.adapters import resolve_adapter +from sqlbuild.compiler.discovery.main.discover import discover_project_inputs +from sqlbuild.compiler.discovery.models import DiscoveredProjectInputs +from sqlbuild.compiler.pipeline.main.graph import build_project_graph +from sqlbuild.compiler.pipeline.models import ProjectGraph +from sqlbuild.rule_engine.main.load_config import load_rules_config +from sqlbuild.rule_engine.main.run_rules import run_rules +from sqlbuild.rule_engine.models import RulesRunResult +from tests.unit.src.sqlbuild.rule_engine.main.run_rules._test_types import ( + TypeProofRuleSkipTestCase, +) + + +@pytest.mark.parametrize( + "test_case", + ( + TypeProofRuleSkipTestCase( + description="analysis enabled evaluates type-proof rules", + no_sql_analysis=False, + settings_toml="", + expected_skipped_rules=(), + expected_finding_codes=("SQBRCONTRACT105",) * 4, + ), + TypeProofRuleSkipTestCase( + description="run without SQL analysis skips type-proof rules", + no_sql_analysis=True, + settings_toml="", + expected_skipped_rules=("SQBRCONTRACT105",), + expected_finding_codes=(), + ), + TypeProofRuleSkipTestCase( + description="project without SQL analysis skips type-proof rules", + no_sql_analysis=False, + settings_toml="\n[settings]\nsql_analysis = false\n", + expected_skipped_rules=("SQBRCONTRACT105",), + expected_finding_codes=(), + ), + ), + ids=lambda case: case.description, +) +def test_given_sql_analysis_state_when_running_rules_then_type_proof_rules_follow_it( + test_case: TypeProofRuleSkipTestCase, + tmp_path: Path, +) -> None: + (tmp_path / "sqlbuild_project.toml").write_text( + 'name = "orders"\nadapter = "duckdb"\n\n[rules]\nselect = ["SQBRCONTRACT105"]\n' + + test_case.settings_toml, + encoding="utf-8", + ) + models: Path = tmp_path / "models" + models.mkdir() + (models / "orders.sql").write_text( + "MODEL (\n" + " contract enforced,\n" + " columns (order_id (type INTEGER), status (type VARCHAR)),\n" + ");\n" + "SELECT order_id, status FROM (SELECT 1 AS order_id, 'placed' AS status) AS raw_orders\n" + "UNION ALL\n" + "SELECT 2 AS order_id, 'shipped' AS status\n", + encoding="utf-8", + ) + discovered: DiscoveredProjectInputs = discover_project_inputs(project_dir=tmp_path) + adapter: BaseAdapter = resolve_adapter(adapter_name="duckdb", project_dir=tmp_path) + graph: ProjectGraph = build_project_graph( + discovered_inputs=discovered, + adapter=adapter, + no_sql_validation=test_case.no_sql_analysis, + ) + + result: RulesRunResult = run_rules( + graph=graph, + discovered_inputs=discovered, + config=load_rules_config(project_dir=tmp_path), + project_dir=tmp_path, + dialect="duckdb", + no_sql_analysis=test_case.no_sql_analysis, + ) + + assert result.skipped_type_proof_rules == test_case.expected_skipped_rules + assert tuple(finding.code for finding in result.findings) == test_case.expected_finding_codes diff --git a/website/src/content/docs/docs/cli/compile.mdx b/website/src/content/docs/docs/cli/compile.mdx index e6a26bc945..d5d553bbfc 100644 --- a/website/src/content/docs/docs/cli/compile.mdx +++ b/website/src/content/docs/docs/cli/compile.mdx @@ -36,7 +36,9 @@ sqb --project-dir compile [flags] 3. **SQL validation** - validates SQL syntax (when SQL analysis is enabled) 4. **Column lineage** - analyzes column-level dependencies across models (fast mode by default) 5. **Contract validation** - checks declared column contracts against inferred query output -6. **Rules** - evaluates selected native built-ins, then selected custom Python Rules +6. **Rules** - evaluates selected native built-ins, then selected custom Python Rules. Rules that + need inferred type proofs, such as `SQBRCONTRACT105`, are skipped when SQL analysis is disabled, + and compile prints one note naming them 7. **Artifact write** - writes compiled SQL to `target/compiled/` when Rules pass ## Focused compilation diff --git a/website/src/content/docs/docs/cli/query.mdx b/website/src/content/docs/docs/cli/query.mdx index 4f5fea8cba..77334b72d9 100644 --- a/website/src/content/docs/docs/cli/query.mdx +++ b/website/src/content/docs/docs/cli/query.mdx @@ -82,6 +82,13 @@ order_id,customer_id,status 2,200,completed ``` +## Errors + +When the warehouse rejects the SQL, `sqb query` prints the warehouse message as +`error[C108]: ...` on stderr and exits with code `2`. Nothing is written to stdout, so +redirected `json` or `csv` output is never partial. Invalid command input, such as a missing +query file, exits with code `1`. + ## Examples ```bash