From 012c7a998870a326d4ba1c37567c70776c806045 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 11:02:34 +0100 Subject: [PATCH 1/8] feat(analysis): expose the mapped dialect normalization of analysis SQL Rules need analysis's Snowflake variant normalization together with the character map back to the authored SQL, so positions survive the rewrite. --- .../src/semantic_validation/_helpers/normalization.rs | 11 +++++++++++ .../sqlbuild-analysis/src/semantic_validation/main.rs | 1 + .../src/semantic_validation/main/normalize_mapped.rs | 7 +++++++ .../src/semantic_validation/models.rs | 7 +++++++ 4 files changed, 26 insertions(+) create mode 100644 crates/sqlbuild-analysis/src/semantic_validation/main/normalize_mapped.rs diff --git a/crates/sqlbuild-analysis/src/semantic_validation/_helpers/normalization.rs b/crates/sqlbuild-analysis/src/semantic_validation/_helpers/normalization.rs index eb86232816..25feefc33a 100644 --- a/crates/sqlbuild-analysis/src/semantic_validation/_helpers/normalization.rs +++ b/crates/sqlbuild-analysis/src/semantic_validation/_helpers/normalization.rs @@ -175,6 +175,17 @@ pub(crate) fn normalize_analysis_sql( normalize(sql, dialect, &stubs, &placeholders).map(|mapped| mapped.sql) } +pub(crate) fn normalize_dialect_sql_mapped( + sql: &str, + dialect: &str, +) -> Result<(String, Vec), String> { + let input = MappedSql::new(sql); + if !dialect.eq_ignore_ascii_case("snowflake") { + return Ok((input.sql, input.offsets)); + } + normalize_variant(input).map(|mapped| (mapped.sql, mapped.offsets)) +} + pub(crate) fn normalize_dialect_sql(sql: &str, dialect: &str) -> Result { if !dialect.eq_ignore_ascii_case("snowflake") { return Ok(sql.to_owned()); diff --git a/crates/sqlbuild-analysis/src/semantic_validation/main.rs b/crates/sqlbuild-analysis/src/semantic_validation/main.rs index a600f8fa69..9190654d75 100644 --- a/crates/sqlbuild-analysis/src/semantic_validation/main.rs +++ b/crates/sqlbuild-analysis/src/semantic_validation/main.rs @@ -6,6 +6,7 @@ pub mod new_analysis_pool; pub mod normalize; pub mod normalize_batch; pub mod normalize_dialect; +pub mod normalize_mapped; use crate::semantic_validation::models::ProjectCatalog; use polyglot_sql::{ diff --git a/crates/sqlbuild-analysis/src/semantic_validation/main/normalize_mapped.rs b/crates/sqlbuild-analysis/src/semantic_validation/main/normalize_mapped.rs new file mode 100644 index 0000000000..1587520a25 --- /dev/null +++ b/crates/sqlbuild-analysis/src/semantic_validation/main/normalize_mapped.rs @@ -0,0 +1,7 @@ +use crate::semantic_validation::models::MappedNormalization; + +/// The dialect normalization analysis applies to `sql`, mapped back to its input characters. +pub fn normalize_mapped(sql: &str, dialect: &str) -> Result { + crate::semantic_validation::_helpers::normalization::normalize_dialect_sql_mapped(sql, dialect) + .map(|(sql, offsets)| MappedNormalization { sql, offsets }) +} diff --git a/crates/sqlbuild-analysis/src/semantic_validation/models.rs b/crates/sqlbuild-analysis/src/semantic_validation/models.rs index ca907c3877..7f1805efcf 100644 --- a/crates/sqlbuild-analysis/src/semantic_validation/models.rs +++ b/crates/sqlbuild-analysis/src/semantic_validation/models.rs @@ -28,6 +28,13 @@ pub struct NormalizationInput { pub placeholders: HashMap, } +/// Normalized analysis SQL with, per normalized character, the input character it came from. +#[derive(Debug)] +pub struct MappedNormalization { + pub sql: String, + pub offsets: Vec, +} + #[derive(Debug)] pub struct PositionInput { pub authored: String, From 821a2c7cb0e377ab62eb8b4326baac9870b17873 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 11:02:35 +0100 Subject: [PATCH 2/8] perf(rules): evaluate built-in model rules over Polyglot query facts Built-in model and SQL-test rules re-parsed every model with sqlparser. They now read compact facts from one Polyglot parse per stubbed query shape: dependency calls are stubbed by occurrence, shapes are shared by the rules workers when their stubbed SQL is byte-identical, and positions map back to each model's own SQL. Models sqlparser rejected but Polyglot parses are now evaluated; SQBRDECLARATION101/102 quote the comparison's source text and follow its source position. Unparseable SQL keeps the unevaluated result with Polyglot's error. --- crates/sqlbuild-rules/Cargo.toml | 1 - crates/sqlbuild-rules/src/constants.rs | 1 - .../src/engine/_helpers/evaluation.rs | 4 +- .../engine/tests/test_empty_input_tests.rs | 2 +- .../src/engine/tests/test_evaluation.rs | 93 +- .../src/engine/tests/test_sql_scanning.rs | 120 +- .../src/engine/tests/test_types.rs | 14 +- crates/sqlbuild-rules/src/lib.rs | 1 + .../src/query_facts/_helpers/facts.rs | 710 +++++++++ .../src/query_facts/_helpers/mod.rs | 3 + .../src/query_facts/_helpers/shape.rs | 258 ++++ .../src/query_facts/_helpers/tokens.rs | 597 ++++++++ .../src/query_facts/constants.rs | 11 + .../src/query_facts/main/dependency_texts.rs | 6 + .../src/query_facts/main/mod.rs | 5 + .../src/query_facts/main/model_shape.rs | 6 + .../src/query_facts/main/parse_fixture.rs | 6 + .../src/query_facts/main/query_facts.rs | 6 + .../src/query_facts/main/stub_call.rs | 6 + crates/sqlbuild-rules/src/query_facts/mod.rs | 6 + .../sqlbuild-rules/src/query_facts/models.rs | 245 +++ .../src/rules/_helpers/dynamic_contracts.rs | 51 +- .../src/rules/_helpers/evaluation.rs | 1338 ++++------------- .../rules/_helpers/explicit_output_types.rs | 314 ++-- .../src/rules/_helpers/numeric_decisions.rs | 26 + .../src/rules/_helpers/sql_test_rules.rs | 194 +-- .../src/rules/main/compact_sql.rs | 6 + crates/sqlbuild-rules/src/rules/main/mod.rs | 1 + crates/sqlbuild-rules/src/rules/mod.rs | 1 + crates/sqlbuild-rules/src/rules/models.rs | 23 +- crates/sqlbuild-rules/src/rules/types.rs | 5 + .../cli/commands/main/rules/_test_types.py | 10 + .../commands/main/rules/test_unevaluated.py | 56 +- 33 files changed, 2540 insertions(+), 1586 deletions(-) create mode 100644 crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs create mode 100644 crates/sqlbuild-rules/src/query_facts/_helpers/mod.rs create mode 100644 crates/sqlbuild-rules/src/query_facts/_helpers/shape.rs create mode 100644 crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs create mode 100644 crates/sqlbuild-rules/src/query_facts/constants.rs create mode 100644 crates/sqlbuild-rules/src/query_facts/main/dependency_texts.rs create mode 100644 crates/sqlbuild-rules/src/query_facts/main/mod.rs create mode 100644 crates/sqlbuild-rules/src/query_facts/main/model_shape.rs create mode 100644 crates/sqlbuild-rules/src/query_facts/main/parse_fixture.rs create mode 100644 crates/sqlbuild-rules/src/query_facts/main/query_facts.rs create mode 100644 crates/sqlbuild-rules/src/query_facts/main/stub_call.rs create mode 100644 crates/sqlbuild-rules/src/query_facts/mod.rs create mode 100644 crates/sqlbuild-rules/src/query_facts/models.rs create mode 100644 crates/sqlbuild-rules/src/rules/main/compact_sql.rs create mode 100644 crates/sqlbuild-rules/src/rules/types.rs diff --git a/crates/sqlbuild-rules/Cargo.toml b/crates/sqlbuild-rules/Cargo.toml index 41b5e84013..6fb65866b2 100644 --- a/crates/sqlbuild-rules/Cargo.toml +++ b/crates/sqlbuild-rules/Cargo.toml @@ -24,7 +24,6 @@ sqlbuild-core.workspace = true sqlbuild-scopes.workspace = true sqlbuild-sqltext.workspace = true stacker.workspace = true -sqlparser.workspace = true toml.workspace = true [build-dependencies] diff --git a/crates/sqlbuild-rules/src/constants.rs b/crates/sqlbuild-rules/src/constants.rs index dd728e9a56..73e3aa809f 100644 --- a/crates/sqlbuild-rules/src/constants.rs +++ b/crates/sqlbuild-rules/src/constants.rs @@ -24,7 +24,6 @@ pub(crate) const ENFORCED_CONTRACT: &str = "enforced"; pub(crate) const SELECT_STAR_MODEL_RULE_CODE: &str = "SQBRMODEL102"; pub(crate) const BOOLEAN_TYPE: &str = "BOOLEAN"; pub(crate) const DATE_TYPE: &str = "DATE"; -pub(crate) const NEGATION_OPERATOR: &str = "-"; pub(crate) const DECLARATION_DOMAIN_COMPONENTS: usize = 3; pub(crate) const RULE_CODE_NUMBER_LENGTH: usize = 3; pub(crate) const EMPTY_FIXTURE_QUERY_TOKEN_COUNT: usize = 6; diff --git a/crates/sqlbuild-rules/src/engine/_helpers/evaluation.rs b/crates/sqlbuild-rules/src/engine/_helpers/evaluation.rs index e99d55b938..1c8e9e2bb0 100644 --- a/crates/sqlbuild-rules/src/engine/_helpers/evaluation.rs +++ b/crates/sqlbuild-rules/src/engine/_helpers/evaluation.rs @@ -10,7 +10,7 @@ use crate::rules::main::{ resolve_threshold_overrides, select, sql_test_coverage, }; use crate::rules::models::{ - ModelEvaluationRequest, ProjectEvaluationRequest, ResolvedThresholdOverride, + ModelEvaluationRequest, ProjectEvaluationRequest, QueryShapes, ResolvedThresholdOverride, }; use fensu_policy::apply_suppressions; use fensu_policy::lifecycle::errors::LifecycleError; @@ -188,6 +188,7 @@ fn evaluate_pending_models( pending_models: Vec>, context: ModelRuleBatchContext<'_>, ) -> Result, String> { + let shapes = QueryShapes::default(); let evaluate_pending = |pending: PendingModelRule<'_>| CompletedModelRule { index: pending.index, identity: pending.identity, @@ -203,6 +204,7 @@ fn evaluate_pending_models( selected: context.selected, request: context.request, threshold_overrides: context.threshold_overrides, + shapes: &shapes, }) .and_then(|faults| scoped_faults(context.selected, faults, RuleScope::evaluates_models)) }, diff --git a/crates/sqlbuild-rules/src/engine/tests/test_empty_input_tests.rs b/crates/sqlbuild-rules/src/engine/tests/test_empty_input_tests.rs index 98e029fabd..1c947d4353 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_empty_input_tests.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_empty_input_tests.rs @@ -202,7 +202,7 @@ fn given_sql_test_shapes_when_evaluating_empty_input_rule_then_flags_only_filler ), allowed_tests: json!([]), expected_messages: &[ - "SQBRTEST203 tests/unit/test_orders__empty_inputs_produce_no_rows.sql: Rule SQBRTEST203 could not be evaluated for test block 1: could not parse SQL-test fixture: sql parser error: sql parser error: Expected: an expression, found: EOF; generic fallback also failed: sql parser error: Expected: an expression, found: EOF", + "SQBRTEST203 tests/unit/test_orders__empty_inputs_produce_no_rows.sql: Rule SQBRTEST203 could not be evaluated for test block 1: could not parse SQL-test fixture: Parse error at line 1, column 42: Unexpected token: Limit", ], }, test_types::EmptyInputTestRuleTestCase { diff --git a/crates/sqlbuild-rules/src/engine/tests/test_evaluation.rs b/crates/sqlbuild-rules/src/engine/tests/test_evaluation.rs index 7e0025814d..ac84e45778 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_evaluation.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_evaluation.rs @@ -5,7 +5,8 @@ use crate::constants::NATIVE_BUILD_IDENTITY; use crate::engine::_helpers::evaluation::evaluate_json; use crate::engine::tests::helpers; use crate::engine::tests::test_types; -use crate::rules::_helpers::evaluation::normalize_rules_sql; +use crate::query_facts::main::model_shape::model_shape; +use crate::query_facts::models::ModelSource; #[test] fn given_dependency_tokens_when_classifying_imports_then_preserves_dialect_and_original_sql() @@ -13,12 +14,10 @@ fn given_dependency_tokens_when_classifying_imports_then_preserves_dialect_and_o let test_cases = [ test_types::DependencyTokenTestCase { description: "comments and escaped literals retain canonical arguments", - expected_columns: &[1], + expected_columns: &[], dialect: "duckdb", query_sql: "WITH imported AS (SELECT * FROM __ReF /* __ref('ignored') */ ('Order''s')) SELECT '__source(''ignored'')' AS label FROM imported", - expected_messages: &[ - "each __ref/__source must appear once in its own top-level import CTE", - ], + expected_messages: &[], }, test_types::DependencyTokenTestCase { description: "quoted function names are not dependency calls", @@ -57,6 +56,15 @@ fn given_dependency_tokens_when_classifying_imports_then_preserves_dialect_and_o "each __ref/__source must appear once in its own top-level import CTE", ], }, + test_types::DependencyTokenTestCase { + description: "a dependency passed to a table function is not an import CTE", + expected_columns: &[1], + dialect: "duckdb", + query_sql: "WITH imported AS (SELECT * FROM __table_fn(\"unnested\")(__ref('orders'))) SELECT order_id FROM imported", + expected_messages: &[ + "each __ref/__source must appear once in its own top-level import CTE", + ], + }, test_types::DependencyTokenTestCase { description: "duplicate dependencies remain ordered findings", expected_columns: &[1, 55], @@ -1188,65 +1196,56 @@ fn given_numeric_comparisons_when_evaluating_named_decisions_then_only_authored_ } #[test] -fn given_snowflake_parser_extensions_when_normalizing_then_preserves_source_positions() { - let test_cases = [test_types::NormalizationTestCase { - description: "normalization changes syntax only outside strings and comments", +fn given_model_sql_when_stubbing_shape_then_offsets_map_back_to_source_positions() +-> Result<(), String> { + let test_cases = [test_types::ShapeTestCase { + description: "calls in code are stubbed by occurrence; quoted and commented calls stay", dialect: "snowflake", sql: concat!( - "SELECT * EXCLUDE (\n ignored,\n obsolete\n), ", - "TRANSFORM(values, leg INT -> leg + 1),\n", - "'__table_fn(\"quoted\")(7)', /* leg INT -> unchanged */ payload:key::STRING,\n", - "CAST(amounts AS ARRAY(\n NUMBER(38, 10)\n)) AS amounts\n", - "FROM __table_fn(\"orders)archive\")(42)" + "SELECT '__ref(\"quoted\")' AS q, /* __ref(\"comment\") */ o.payload:key::STRING AS k\n", + "FROM __ref( \"orders\" ) AS o JOIN __source(\"raw\", \"items\") AS i ON i.id = o.id" ), - expected_typed_lambda: "leg -> leg + 1", - expected_quoted_call: "'__table_fn(\"quoted\")(7)'", - expected_comment: "/* leg INT -> unchanged */", - expected_table_function: "__table_fn(\"orders)archive\", 42)", - expected_other_dialect_lambda: "leg INT -> leg + 1", + expected_calls: &["__ref(\"orders\")", "__source(\"raw\",\"items\")"], + expected_fragment: "i.id = o.id", }]; for test_case in test_cases { - let normalized = normalize_rules_sql(test_case.dialect, test_case.sql); - assert_eq!( - normalized.len(), - test_case.sql.len(), - "{}", - test_case.description - ); - assert_eq!( - normalized.lines().count(), - test_case.sql.lines().count(), - "{}", - test_case.description - ); + let shape = model_shape(test_case.sql, test_case.dialect)?; + let calls: Vec<&str> = shape.calls.iter().map(|call| call.text.as_str()).collect(); + assert_eq!(calls, test_case.expected_calls, "{}", test_case.description); assert!( - normalized.contains(test_case.expected_typed_lambda), + shape.sql.contains("'__ref(\"quoted\")'"), "{}", test_case.description ); - assert!( - normalized.contains(test_case.expected_quoted_call), - "{}", - test_case.description - ); - assert!( - normalized.contains(test_case.expected_comment), - "{}", - test_case.description - ); - assert!( - normalized.contains(test_case.expected_table_function), + let normalized_start = shape + .sql + .find(test_case.expected_fragment) + .ok_or("fragment missing from the normalized SQL")?; + let normalized_start = shape.sql[..normalized_start].chars().count(); + let original_start = test_case + .sql + .find(test_case.expected_fragment) + .ok_or("fragment missing from the model SQL")?; + let source = ModelSource::new(test_case.sql, &shape.offsets); + let end = normalized_start + test_case.expected_fragment.chars().count(); + assert_eq!( + source.text(normalized_start, end), + test_case.expected_fragment, "{}", test_case.description ); - let other = normalize_rules_sql("duckdb", test_case.sql); - assert!( - other.contains(test_case.expected_other_dialect_lambda), + let line_start = test_case.sql[..original_start] + .rfind('\n') + .map_or(0, |index| index + 1); + assert_eq!( + source.line_column(normalized_start), + (2, (original_start - line_start + 1) as u64), "{}", test_case.description ); } + Ok(()) } #[test] diff --git a/crates/sqlbuild-rules/src/engine/tests/test_sql_scanning.rs b/crates/sqlbuild-rules/src/engine/tests/test_sql_scanning.rs index 359cfce2a2..92bc83d6e9 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_sql_scanning.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_sql_scanning.rs @@ -1,7 +1,7 @@ use crate::engine::tests::test_types::{ DollarQuoteSiteTestCase, LintDialectSiteTestCase, SqlScannerTestCase, }; -use crate::rules::_helpers::evaluation::normalize_rules_sql; +use crate::query_facts::main::model_shape::model_shape; use crate::rules::_helpers::numeric_decisions::compact_sql; use crate::sql_lint::_helpers::preparation::prepare; use sqlbuild_sqltext::sql_scan::main::matching_paren::matching_paren; @@ -17,10 +17,7 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann description: "plain code", fragment: "a", expected_compiler_paren: Ok(4), - expected_table_function: "SELECT * FROM __table_fn(\"o\", a x, 1)", - expected_table_function_token: "SELECT a __table_fn(\"o\", 1)", - expected_snowflake_exclude: "SELECT * , a FROM t", - expected_snowflake_comma: "SELECT a a FROM t", + expected_dependency_calls: &["__ref(\"o\")"], expected_compact: "a=aandb", expected_snowflake_compact: "a=aandb", expected_lint_site: Some("@m(a x)"), @@ -29,10 +26,7 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann description: "single quote containing open paren", fragment: "'a(b'", expected_compiler_paren: Ok(8), - expected_table_function: "SELECT * FROM __table_fn(\"o\", 'a(b' x, 1)", - expected_table_function_token: "SELECT 'a(b' __table_fn(\"o\", 1)", - expected_snowflake_exclude: "SELECT * , a FROM t", - expected_snowflake_comma: "SELECT 'a(b' a FROM t", + expected_dependency_calls: &["__ref(\"o\")"], expected_compact: "a='a(b'andb", expected_snowflake_compact: "a='a(b'andb", expected_lint_site: Some("@m('a(b' x)"), @@ -41,10 +35,7 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann description: "single quote containing close paren", fragment: "'a)b'", expected_compiler_paren: Ok(8), - expected_table_function: "SELECT * FROM __table_fn(\"o\", 'a)b' x, 1)", - expected_table_function_token: "SELECT 'a)b' __table_fn(\"o\", 1)", - expected_snowflake_exclude: "SELECT * , a FROM t", - expected_snowflake_comma: "SELECT 'a)b' a FROM t", + expected_dependency_calls: &["__ref(\"o\")"], expected_compact: "a='a)b'andb", expected_snowflake_compact: "a='a)b'andb", expected_lint_site: Some("@m('a)b' x)"), @@ -53,10 +44,7 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann description: "doubled single quote", fragment: "'it''s )'", expected_compiler_paren: Ok(12), - expected_table_function: "SELECT * FROM __table_fn(\"o\", 'it''s )' x, 1)", - expected_table_function_token: "SELECT 'it''s )' __table_fn(\"o\", 1)", - expected_snowflake_exclude: "SELECT * , a FROM t", - expected_snowflake_comma: "SELECT 'it''s )' a FROM t", + expected_dependency_calls: &["__ref(\"o\")"], expected_compact: "a='it''s )'andb", expected_snowflake_compact: "a='it''s )'andb", expected_lint_site: Some("@m('it''s )' x)"), @@ -65,10 +53,7 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann description: "backslash escaped quote before close paren", fragment: "'a\\')'", expected_compiler_paren: Ok(5), - expected_table_function: "SELECT * FROM __table_fn(\"o\", 'a\\')' x)(1)", - expected_table_function_token: "SELECT 'a\\')' __table_fn(\"o\")(1)", - expected_snowflake_exclude: "SELECT * , a FROM t", - expected_snowflake_comma: "SELECT 'a\\')' a FROM t", + expected_dependency_calls: &[], expected_compact: "a='a\\')' and b", expected_snowflake_compact: "a='a\\')'andb", expected_lint_site: Some("@m('a\\')' x)"), @@ -77,10 +62,7 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann description: "backslash escaped quote before open paren", fragment: "'a\\'('", expected_compiler_paren: Err(Unclosed::Quote), - expected_table_function: "SELECT * FROM __table_fn(\"o\", 'a\\'(' x)(1)", - expected_table_function_token: "SELECT 'a\\'(' __table_fn(\"o\")(1)", - expected_snowflake_exclude: "SELECT * , a FROM t", - expected_snowflake_comma: "SELECT 'a\\'(' a FROM t", + expected_dependency_calls: &[], expected_compact: "a='a\\'(' and b", expected_snowflake_compact: "a='a\\'('andb", expected_lint_site: Some("@m('a\\'(' x)"), @@ -89,10 +71,7 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann description: "double quote containing close paren", fragment: "\"a)b\"", expected_compiler_paren: Ok(8), - expected_table_function: "SELECT * FROM __table_fn(\"o\", \"a)b\" x, 1)", - expected_table_function_token: "SELECT \"a)b\" __table_fn(\"o\", 1)", - expected_snowflake_exclude: "SELECT * , a FROM t", - expected_snowflake_comma: "SELECT \"a)b\" a FROM t", + expected_dependency_calls: &["__ref(\"o\")"], expected_compact: "a=\"a)b\"andb", expected_snowflake_compact: "a=\"a)b\"andb", expected_lint_site: Some("@m(\"a)b\" x)"), @@ -101,10 +80,7 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann description: "backtick containing close paren", fragment: "`a)b`", expected_compiler_paren: Ok(8), - expected_table_function: "SELECT * FROM __table_fn(\"o\", `a)b` x, 1)", - expected_table_function_token: "SELECT `a)b` __table_fn(\"o\", 1)", - expected_snowflake_exclude: "SELECT * , a FROM t", - expected_snowflake_comma: "SELECT `a)b` a FROM t", + expected_dependency_calls: &["__ref(\"o\")"], expected_compact: "a=`a)b`andb", expected_snowflake_compact: "a=`a)b`andb", expected_lint_site: Some("@m(`a)"), @@ -113,10 +89,7 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann description: "doubled backtick", fragment: "`a``)b`", expected_compiler_paren: Ok(10), - expected_table_function: "SELECT * FROM __table_fn(\"o\", `a``)b` x, 1)", - expected_table_function_token: "SELECT `a``)b` __table_fn(\"o\", 1)", - expected_snowflake_exclude: "SELECT * , a FROM t", - expected_snowflake_comma: "SELECT `a``)b` a FROM t", + expected_dependency_calls: &["__ref(\"o\")"], expected_compact: "a=`a``)b`andb", expected_snowflake_compact: "a=`a``)b`andb", expected_lint_site: Some("@m(`a``)"), @@ -125,10 +98,7 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann description: "line comment", fragment: "-- )\n", expected_compiler_paren: Ok(8), - expected_table_function: "SELECT * FROM __table_fn(\"o\", -- )\n x, 1)", - expected_table_function_token: "SELECT -- )\n __table_fn(\"o\", 1)", - expected_snowflake_exclude: "SELECT * \n , a FROM t", - expected_snowflake_comma: "SELECT -- )\n a FROM t", + expected_dependency_calls: &["__ref(\"o\")"], expected_compact: "a=andb", expected_snowflake_compact: "a=andb", expected_lint_site: Some("@m(-- )\n x)"), @@ -137,10 +107,7 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann description: "block comment", fragment: "/* ) */", 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 * , a FROM t", - expected_snowflake_comma: "SELECT /* ) */ a FROM t", + expected_dependency_calls: &["__ref(\"o\")"], expected_compact: "a=andb", expected_snowflake_compact: "a=andb", expected_lint_site: Some("@m(/* ) */ x)"), @@ -149,10 +116,7 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann description: "nested block comment", fragment: "/* /* ) */ ) */", expected_compiler_paren: Ok(12), - 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", - expected_snowflake_comma: "SELECT /* /* ) */ ) */ a FROM t", + expected_dependency_calls: &["__ref(\"o\")"], expected_compact: "a=)*/andb", expected_snowflake_compact: "a=)*/andb", expected_lint_site: Some("@m(/* /* ) */ )"), @@ -161,10 +125,7 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann description: "dollar quote", fragment: "$$ ) $$", 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", - expected_snowflake_comma: "SELECT $$ ) $$ a FROM t", + expected_dependency_calls: &["__ref(\"o\")"], expected_compact: "a=$$)$$andb", expected_snowflake_compact: "a=$$)$$andb", expected_lint_site: Some("@m($$ ) $$ x)"), @@ -173,10 +134,7 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann description: "unterminated quote", fragment: "'abc", expected_compiler_paren: Err(Unclosed::Quote), - expected_table_function: "SELECT * FROM __table_fn(\"o\", 'abc x)(1)", - expected_table_function_token: "SELECT 'abc __table_fn(\"o\")(1)", - expected_snowflake_exclude: "SELECT * EXCLUDE ('abc x) , a FROM t", - expected_snowflake_comma: "SELECT 'abc a, FROM t", + expected_dependency_calls: &[], expected_compact: "a='abc and b", expected_snowflake_compact: "a='abc and b", expected_lint_site: Some(""), @@ -185,10 +143,7 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann description: "unterminated block comment", fragment: "/* abc", expected_compiler_paren: Err(Unclosed::BlockComment), - expected_table_function: "SELECT * FROM __table_fn(\"o\", /* abc x)(1)", - expected_table_function_token: "SELECT /* abc __table_fn(\"o\")(1)", - expected_snowflake_exclude: "SELECT * EXCLUDE (/* abc x) , a FROM t", - expected_snowflake_comma: "SELECT /* abc a, FROM t", + expected_dependency_calls: &[], expected_compact: "a=", expected_snowflake_compact: "a=", expected_lint_site: Some(""), @@ -197,10 +152,7 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann description: "multibyte utf8", fragment: "'é)ü' ñ", expected_compiler_paren: Ok(13), - expected_table_function: "SELECT * FROM __table_fn(\"o\", 'é)ü' ñ x, 1)", - expected_table_function_token: "SELECT 'é)ü' ñ __table_fn(\"o\", 1)", - expected_snowflake_exclude: "SELECT * , a FROM t", - expected_snowflake_comma: "SELECT 'é)ü' ñ a FROM t", + expected_dependency_calls: &["__ref(\"o\")"], expected_compact: "a='é)ü'ñandb", expected_snowflake_compact: "a='é)ü'ñandb", expected_lint_site: Some("@m('é)ü' ñ x)"), @@ -219,34 +171,18 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann "compiler matching paren: {}", test_case.description ); + let calls = model_shape(&format!("SELECT {fragment} x FROM __ref(\"o\")"), "duckdb") + .map(|shape| { + shape + .calls + .into_iter() + .map(|call| call.text) + .collect::>() + }) + .unwrap_or_default(); assert_eq!( - normalize_rules_sql( - "duckdb", - &format!("SELECT * FROM __table_fn(\"o\", {fragment} x)(1)") - ), - test_case.expected_table_function, - "rules table-function parenthesis: {}", - test_case.description - ); - assert_eq!( - normalize_rules_sql("duckdb", &format!("SELECT {fragment} __table_fn(\"o\")(1)")), - test_case.expected_table_function_token, - "rules table-function token search: {}", - test_case.description - ); - assert_eq!( - normalize_rules_sql( - "snowflake", - &format!("SELECT * EXCLUDE ({fragment} x) , a FROM t") - ), - test_case.expected_snowflake_exclude, - "rules Snowflake EXCLUDE parenthesis: {}", - test_case.description - ); - assert_eq!( - normalize_rules_sql("snowflake", &format!("SELECT {fragment} a, FROM t")), - test_case.expected_snowflake_comma, - "rules Snowflake trailing comma: {}", + calls, test_case.expected_dependency_calls, + "rules dependency calls: {}", test_case.description ); assert_eq!( diff --git a/crates/sqlbuild-rules/src/engine/tests/test_types.rs b/crates/sqlbuild-rules/src/engine/tests/test_types.rs index 6d69c0b50b..2a53c8f6d1 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_types.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_types.rs @@ -69,15 +69,12 @@ pub(crate) struct TypedContractColumnTestCase { pub(crate) expected_messages: &'static [&'static str], } -pub(crate) struct NormalizationTestCase { +pub(crate) struct ShapeTestCase { pub(crate) description: &'static str, pub(crate) dialect: &'static str, pub(crate) sql: &'static str, - pub(crate) expected_typed_lambda: &'static str, - pub(crate) expected_quoted_call: &'static str, - pub(crate) expected_comment: &'static str, - pub(crate) expected_table_function: &'static str, - pub(crate) expected_other_dialect_lambda: &'static str, + pub(crate) expected_calls: &'static [&'static str], + pub(crate) expected_fragment: &'static str, } pub(crate) struct DeepExpressionTestCase { @@ -200,10 +197,7 @@ pub(crate) struct SqlScannerTestCase { pub(crate) description: &'static str, pub(crate) fragment: &'static str, pub(crate) expected_compiler_paren: Result, - pub(crate) expected_table_function: &'static str, - pub(crate) expected_table_function_token: &'static str, - pub(crate) expected_snowflake_exclude: &'static str, - pub(crate) expected_snowflake_comma: &'static str, + pub(crate) expected_dependency_calls: &'static [&'static str], pub(crate) expected_compact: &'static str, pub(crate) expected_snowflake_compact: &'static str, /// `None` when SQL lint declines non-ASCII input; `Some("")` when no macro site closes. diff --git a/crates/sqlbuild-rules/src/lib.rs b/crates/sqlbuild-rules/src/lib.rs index 685595fba4..fdf32f0c54 100644 --- a/crates/sqlbuild-rules/src/lib.rs +++ b/crates/sqlbuild-rules/src/lib.rs @@ -13,6 +13,7 @@ pub mod fix_verification; pub mod lint_text; pub mod models; pub mod project_tree; +pub mod query_facts; pub mod rule_cache; pub mod rule_catalogue; pub mod rule_results; diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs new file mode 100644 index 0000000000..9a6ab8b9b3 --- /dev/null +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs @@ -0,0 +1,710 @@ +use crate::query_facts::_helpers::shape::stub_call; +use crate::query_facts::_helpers::tokens::{ComparisonOperator, QueryTokens, SelectTokens}; +use crate::query_facts::constants::{MAX_FUNCTION_CALL_DEPTH, TABLE_FUNCTION_STUB}; +use crate::query_facts::models::{ + CallKind, ColumnFact, ComparisonFacts, ComparisonSpan, CteFacts, ExpressionItem, ItemFacts, + ModelShape, ParsedFixture, QueryFacts, RelationFacts, SelectFacts, SetFacts, +}; +use polyglot_sql::dialects::Dialect; +use polyglot_sql::expressions::{Identifier, Literal, Select, TableRef, With}; +use polyglot_sql::{ComplexityGuardOptions, Expression, ExpressionWalk, ParseOptions}; +use sqlbuild_analysis::lineage::main::parser_dialect::parser_dialect; +use std::cell::RefCell; +use std::collections::HashMap; +use std::ops::Range; + +fn parse_query(sql: &str, dialect_name: &str) -> Result<(Expression, Dialect), String> { + let dialect_type = parser_dialect(Some(dialect_name)) + .map_err(|unknown| format!("unknown dialect {unknown}"))?; + let dialect = Dialect::get(dialect_type); + let options = ParseOptions { + complexity_guard: Some(ComplexityGuardOptions { + max_function_call_depth: Some(MAX_FUNCTION_CALL_DEPTH), + ..Default::default() + }), + }; + let mut statements = dialect + .parse_with_options(sql, &options) + .map_err(|error| error.to_string())?; + if statements.len() != 1 { + return Err("expected one statement".to_owned()); + } + let statement = statements.remove(0); + if !is_query(&statement) { + return Err("expected a query".to_owned()); + } + Ok((statement, dialect)) +} + +/// Parse a stubbed SQL-test fixture as one query. +pub(crate) fn parse_fixture( + shape: ModelShape, + dialect_name: &str, +) -> Result { + let (statement, dialect) = parse_query(&shape.sql, dialect_name)?; + let tokens = dialect + .tokenize(&shape.sql) + .map_err(|error| error.to_string())?; + let tokens = QueryTokens::new(tokens, &shape.sql); + let selects = tokens.selects(); + let order = tokens.top_level_word("ORDER").unwrap_or(tokens.len()); + let order_end = ["LIMIT", "OFFSET", "FETCH"] + .into_iter() + .filter_map(|word| tokens.top_level_word(word)) + .filter(|index| *index > order) + .min() + .unwrap_or(tokens.len()); + let projection_function = selects.first().is_some_and(|select| { + select + .items + .iter() + .any(|item| tokens.calls_function(item.clone())) + }); + Ok(ParsedFixture { + select_function: tokens.calls_function(0..order), + projection_function, + order_by_function: tokens.calls_function(order..order_end), + selects: selects.len(), + calls: shape.calls, + statement, + }) +} + +/// Parse normalized model SQL with Polyglot and read the facts built-in rules evaluate. +pub(crate) fn query_facts(sql: &str, dialect_name: &str) -> Result { + let (statement, dialect) = parse_query(sql, dialect_name)?; + let tokens = dialect.tokenize(sql).map_err(|error| error.to_string())?; + let tokens = QueryTokens::new(tokens, sql); + let builder = Builder { + selects: tokens.selects(), + tokens, + dialect, + spans: RefCell::default(), + }; + Ok(builder.query(&statement)) +} + +fn is_query(expression: &Expression) -> bool { + match expression { + Expression::Select(_) + | Expression::Union(_) + | Expression::Intersect(_) + | Expression::Except(_) + | Expression::Values(_) => true, + Expression::Subquery(subquery) => subquery.alias.is_none() && is_query(&subquery.this), + Expression::Paren(paren) => is_query(&paren.this), + _ => false, + } +} + +struct Builder<'a> { + tokens: QueryTokens<'a>, + selects: Vec, + dialect: Dialect, + spans: RefCell>>, +} + +struct ComparisonParts<'a> { + operator: ComparisonOperator, + operands: Vec<&'a Expression>, + negated: bool, +} + +struct Modifiers<'a> { + with: Option<&'a With>, + order_by: bool, + limit: bool, + fetch: bool, + locks: bool, +} + +impl Builder<'_> { + fn query(&self, expression: &Expression) -> QueryFacts { + let modifiers: Modifiers<'_> = match expression { + Expression::Select(select) => Modifiers { + with: select.with.as_ref(), + order_by: select.order_by.is_some(), + limit: select.limit.is_some() || select.offset.is_some(), + fetch: select.fetch.is_some(), + locks: !select.locks.is_empty(), + }, + Expression::Union(set) => set_modifiers( + set.with.as_ref(), + set.order_by.is_some(), + set.limit.is_some() || set.offset.is_some(), + ), + Expression::Intersect(set) => set_modifiers( + set.with.as_ref(), + set.order_by.is_some(), + set.limit.is_some() || set.offset.is_some(), + ), + Expression::Except(set) => set_modifiers( + set.with.as_ref(), + set.order_by.is_some(), + set.limit.is_some() || set.offset.is_some(), + ), + _ => set_modifiers(None, false, false), + }; + let mut ctes: Vec = Vec::new(); + for cte in modifiers.with.iter().flat_map(|with| with.ctes.iter()) { + ctes.push(CteFacts { + name: cte.alias.name.clone(), + name_start: cte.alias.span.map(|span| span.start), + columns: cte + .columns + .iter() + .map(|column| column.name.clone()) + .collect(), + query: self.query(&cte.this), + dependency_calls: dependency_calls(&cte.this), + }); + } + let body = match expression { + Expression::Subquery(subquery) => SetFacts::Query(Box::new(self.query(&subquery.this))), + Expression::Paren(paren) => SetFacts::Query(Box::new(self.query(&paren.this))), + _ => self.set(expression), + }; + QueryFacts { + ctes, + body, + has_order_by: modifiers.order_by, + has_limit: modifiers.limit, + has_fetch: modifiers.fetch, + has_locks: modifiers.locks, + } + } + + fn set(&self, expression: &Expression) -> SetFacts { + match expression { + Expression::Select(select) => SetFacts::Select(Box::new(self.select(select))), + Expression::Union(set) => self.set_operation(set.by_name, &set.left, &set.right), + Expression::Intersect(set) => self.set_operation(set.by_name, &set.left, &set.right), + Expression::Except(set) => self.set_operation(set.by_name, &set.left, &set.right), + Expression::Subquery(subquery) if subquery.alias.is_none() => { + SetFacts::Query(Box::new(self.query(&subquery.this))) + } + Expression::Paren(paren) => SetFacts::Query(Box::new(self.query(&paren.this))), + _ => SetFacts::Other, + } + } + + fn set_operation(&self, by_name: bool, left: &Expression, right: &Expression) -> SetFacts { + SetFacts::SetOperation { + by_name, + left: Box::new(self.set(left)), + right: Box::new(self.set(right)), + } + } + + fn select(&self, select: &Select) -> SelectFacts { + let tokens: Option<&SelectTokens> = self + .select_tokens(select) + .filter(|tokens| tokens.items.len() == select.expressions.len()); + let mut items: Vec = Vec::with_capacity(select.expressions.len()); + for (index, expression) in select.expressions.iter().enumerate() { + let item: Option<&Range> = tokens.map(|tokens| &tokens.items[index]); + items.push(self.item(expression, item)); + } + let mut comparisons: Vec = Vec::new(); + for join in &select.joins { + if let Some(on) = &join.on { + comparisons.extend(self.comparisons(on, None)); + } + } + for expression in &select.expressions { + let (alias, value) = match expression { + Expression::Alias(alias) => (Some(alias.alias.name.as_str()), &alias.this), + _ => (None, expression), + }; + comparisons.extend(self.case_comparisons(value, alias)); + } + for clause in [ + select.where_clause.as_ref().map(|clause| &clause.this), + select.having.as_ref().map(|clause| &clause.this), + select.qualify.as_ref().map(|clause| &clause.this), + ] + .into_iter() + .flatten() + { + comparisons.extend(self.comparisons(clause, None)); + } + SelectFacts { + items, + from: select + .from + .iter() + .flat_map(|from| from.expressions.iter()) + .map(|expression| relation(expression, &self.tokens)) + .collect(), + joins: select + .joins + .iter() + .map(|join| relation(&join.this, &self.tokens)) + .collect(), + filtered: select.where_clause.is_some() + || select.having.is_some() + || select.qualify.is_some() + || select.group_by.is_some(), + comparisons, + } + } + + fn select_tokens(&self, select: &Select) -> Option<&SelectTokens> { + let anchor = select_anchor(select)?; + self.selects + .iter() + .filter(|tokens| tokens.start < anchor && anchor < tokens.end) + .max_by_key(|tokens| tokens.start) + } + + fn item(&self, expression: &Expression, item: Option<&Range>) -> ItemFacts { + if let Expression::Star(_) = expression { + return ItemFacts::Star { + item_start: item + .filter(|item| !item.is_empty()) + .map(|item| self.tokens.start(item.start)), + star_start: item.and_then(|item| self.tokens.star_start(item)), + }; + } + let (alias, value) = match expression { + Expression::Alias(alias) => (Some(alias.alias.name.clone()), &alias.this), + _ => (None, expression), + }; + let range = item.map(|item| self.tokens.expression_range(item, alias.is_some())); + let unwrapped = unwrap_paren(value); + let cast_type = match unwrapped { + Expression::Cast(cast) | Expression::TryCast(cast) | Expression::SafeCast(cast) => { + Some( + range + .as_ref() + .and_then(|range| self.tokens.cast_type(range)) + .unwrap_or_else(|| { + self.dialect + .generate(&Expression::DataType(cast.to.clone())) + .unwrap_or_default() + }), + ) + } + _ => None, + }; + ItemFacts::Expression(ExpressionItem { + output_name: alias.or_else(|| column_name(value)), + direct_projection: column_name(value).is_some(), + direct_column: column_name(unwrapped), + cast_type, + start: range.and_then(|range| self.tokens.leaf_start(&range)), + }) + } + + fn comparisons(&self, root: &Expression, output_name: Option<&str>) -> Vec { + let mut nodes: Vec<(&Expression, ComparisonParts<'_>)> = Vec::new(); + for node in root.dfs() { + if let Some(parts) = comparison_parts(node) { + nodes.push((node, parts)); + } + } + for (node, parts) in nodes.iter().rev() { + if anchors(parts) != (None, None) { + self.span(node, parts); + } + } + let mut output: Vec = Vec::with_capacity(nodes.len()); + for (node, parts) in &nodes { + let mut fact = self.comparison(node, parts); + fact.output_name = output_name.map(str::to_owned); + output.push(fact); + } + output + } + + fn span(&self, node: &Expression, parts: &ComparisonParts<'_>) -> Option { + let key: *const Expression = node; + if let Some(span) = self.spans.borrow().get(&key) { + return *span; + } + let anchors = anchors(parts); + let generated = if anchors == (None, None) { + self.dialect.generate(node).unwrap_or_default() + } else { + String::new() + }; + let span = self + .tokens + .comparison_range(parts.operator, anchors, parts.negated, &generated); + self.spans.borrow_mut().insert(key, span); + span + } + + fn case_comparisons( + &self, + root: &Expression, + output_name: Option<&str>, + ) -> Vec { + let mut output: Vec = Vec::new(); + for node in root.dfs() { + if let Expression::Case(case) = node { + if let Some(operand) = &case.operand { + output.extend(self.comparisons(operand, output_name)); + } + for (condition, _) in &case.whens { + output.extend(self.comparisons(condition, output_name)); + } + } + } + output + } + + fn comparison(&self, node: &Expression, parts: &ComparisonParts<'_>) -> ComparisonFacts { + let ComparisonParts { + operator, + operands, + negated, + } = parts; + let (operator, negated) = (*operator, *negated); + let span = self.span(node, parts); + let mut fact = ComparisonFacts { + span, + generated_sql: if span.is_none() { + self.dialect.generate(node).unwrap_or_default() + } else { + String::new() + }, + equality: operator == ComparisonOperator::Eq + || (operator == ComparisonOperator::In && !negated), + value_list: operator == ComparisonOperator::In, + binary: !matches!( + operator, + ComparisonOperator::Between | ComparisonOperator::In + ), + ..ComparisonFacts::default() + }; + for value in node.dfs() { + match value { + Expression::Column(column) => { + fact.columns.insert(column.name.name.clone()); + } + Expression::Identifier(identifier) => { + fact.columns.insert(identifier.name.clone()); + } + Expression::Literal(literal) => { + if let Some(text) = string_literal(literal) { + fact.string_literals.push(text); + } + } + _ => {} + } + } + for operand in operands.iter().copied() { + if let Some(number) = numeric_value(operand) { + fact.numeric_literals.push(number); + } + if direct_column(operand).is_none() { + fact.modified_columns + .extend(operand.dfs().filter_map(direct_column)); + } + if direct_string_literal(operand).is_none() + && operand + .dfs() + .any(|value| direct_string_literal(value).is_some()) + { + fact.modified_string_literal = true; + } + } + fact + } +} + +fn comparison_parts(node: &Expression) -> Option> { + let (operator, operands, negated) = match node { + Expression::Eq(op) => (ComparisonOperator::Eq, vec![&op.left, &op.right], false), + Expression::Neq(op) => (ComparisonOperator::NotEq, vec![&op.left, &op.right], false), + Expression::Lt(op) => (ComparisonOperator::Lt, vec![&op.left, &op.right], false), + Expression::Lte(op) => (ComparisonOperator::LtEq, vec![&op.left, &op.right], false), + Expression::Gt(op) => (ComparisonOperator::Gt, vec![&op.left, &op.right], false), + Expression::Gte(op) => (ComparisonOperator::GtEq, vec![&op.left, &op.right], false), + Expression::Between(between) => ( + ComparisonOperator::Between, + vec![&between.this, &between.low, &between.high], + between.not, + ), + Expression::In(list) if list.query.is_none() && list.unnest.is_none() => ( + ComparisonOperator::In, + std::iter::once(&list.this) + .chain(list.expressions.iter()) + .collect(), + list.not, + ), + _ => return None, + }; + Some(ComparisonParts { + operator, + operands, + negated, + }) +} + +fn anchors(parts: &ComparisonParts<'_>) -> (Option, Option) { + let left_end = span_bounds(parts.operands[0]).map(|(_, end)| end); + let right_start = parts.operands[1..] + .iter() + .filter_map(|operand| span_bounds(operand)) + .map(|(start, _)| start) + .min(); + (left_end, right_start) +} + +fn set_modifiers(with: Option<&With>, order_by: bool, limit: bool) -> Modifiers<'_> { + Modifiers { + with, + order_by, + limit, + fetch: false, + locks: false, + } +} + +fn dependency_calls(root: &Expression) -> Vec { + root.dfs() + .filter_map(|node| match node { + Expression::Table(table) if table.schema.is_none() && table.catalog.is_none() => { + stub_call(&table.name.name) + } + _ => None, + }) + .filter(|(kind, _)| *kind != CallKind::Other) + .map(|(_, index)| index) + .collect() +} + +fn relation(expression: &Expression, tokens: &QueryTokens<'_>) -> RelationFacts { + match expression { + Expression::Alias(alias) => { + let name = Some(alias.alias.name.clone()); + match relation(&alias.this, tokens) { + RelationFacts::Call { kind, index, .. } => RelationFacts::Call { + kind, + index, + alias: name, + }, + RelationFacts::Table { + display, + parts, + start, + last, + .. + } => RelationFacts::Table { + display, + parts, + start, + last, + alias: name, + }, + other => other, + } + } + Expression::Table(table) => table_relation(table, tokens), + Expression::Pivot(pivot) if !pivot.unpivot => { + if pivot.fields.iter().any(|field| { + matches!(field, Expression::In(list) + if list.expressions.iter().any(|value| matches!(value, Expression::PivotAny(_)))) + }) { + RelationFacts::DynamicPivot + } else { + RelationFacts::Other + } + } + _ => RelationFacts::Other, + } +} + +fn table_relation(table: &TableRef, tokens: &QueryTokens<'_>) -> RelationFacts { + let alias = table.alias.as_ref().map(|alias| alias.name.clone()); + if table.schema.is_none() && table.catalog.is_none() { + if let Some((kind, index)) = stub_call(&table.name.name) { + return RelationFacts::Call { + kind, + index: Some(index), + alias, + }; + } + if table.name.name.starts_with(TABLE_FUNCTION_STUB) { + return RelationFacts::Call { + kind: CallKind::Other, + index: None, + alias, + }; + } + } + let parts: Vec<&Identifier> = [table.catalog.as_ref(), table.schema.as_ref()] + .into_iter() + .flatten() + .chain(std::iter::once(&table.name)) + .collect(); + RelationFacts::Table { + display: quoted_parts(&parts, tokens), + parts: parts.len(), + start: parts[0].span.map(|span| span.start), + last: table.name.name.to_ascii_lowercase(), + alias, + } +} + +fn quoted_parts(parts: &[&Identifier], tokens: &QueryTokens<'_>) -> String { + parts + .iter() + .map(|part| { + if let Some(text) = part + .span + .and_then(|span| tokens.slice(span.start, span.end)) + { + text.to_owned() + } else if part.quoted { + format!("\"{}\"", part.name.replace('"', "\"\"")) + } else { + part.name.clone() + } + }) + .collect::>() + .join(".") +} + +fn select_anchor(select: &Select) -> Option { + let parts = select + .expressions + .iter() + .chain(select.from.iter().flat_map(|from| from.expressions.iter())) + .chain( + select + .joins + .iter() + .flat_map(|join| std::iter::once(&join.this).chain(join.on.iter())), + ) + .chain(select.where_clause.iter().map(|clause| &clause.this)) + .chain( + select + .group_by + .iter() + .flat_map(|group| group.expressions.iter()), + ) + .chain(select.having.iter().map(|clause| &clause.this)) + .chain(select.qualify.iter().map(|clause| &clause.this)); + for part in parts { + if let Some(start) = own_span(part) { + return Some(start); + } + } + None +} + +fn own_span(expression: &Expression) -> Option { + let own = match expression { + Expression::Select(_) + | Expression::Union(_) + | Expression::Intersect(_) + | Expression::Except(_) + | Expression::Values(_) => return None, + Expression::Subquery(subquery) => { + return subquery + .alias + .as_ref() + .and_then(|alias| alias.span) + .map(|span| span.start); + } + Expression::Column(column) => column.span.or(column.name.span), + Expression::Identifier(identifier) => identifier.span, + Expression::Star(star) => star.table.as_ref().and_then(|table| table.span), + Expression::Function(function) => function.span, + Expression::Table(table) => table + .catalog + .as_ref() + .or(table.schema.as_ref()) + .unwrap_or(&table.name) + .span + .or_else(|| table.alias.as_ref().and_then(|alias| alias.span)), + Expression::Alias(alias) => { + return own_span(&alias.this).or(alias.alias.span.map(|span| span.start)); + } + _ => None, + }; + own.map(|span| span.start) + .or_else(|| expression.children().into_iter().find_map(own_span)) +} + +fn span_bounds(expression: &Expression) -> Option<(usize, usize)> { + let mut bounds: Option<(usize, usize)> = None; + for node in expression.dfs() { + let span = match node { + Expression::Column(column) => column.span.or(column.name.span), + Expression::Identifier(identifier) => identifier.span, + Expression::Star(star) => star.table.as_ref().and_then(|table| table.span), + Expression::Function(function) => function.span, + Expression::Count(count) => count.span, + Expression::Table(table) => table.name.span, + _ => None, + }; + if let Some(span) = span { + bounds = Some(bounds.map_or((span.start, span.end), |(start, end)| { + (start.min(span.start), end.max(span.end)) + })); + } + } + bounds +} + +fn unwrap_paren(mut expression: &Expression) -> &Expression { + while let Expression::Paren(paren) = expression { + expression = &paren.this; + } + expression +} + +fn column_name(expression: &Expression) -> Option { + match expression { + Expression::Column(column) => Some(column.name.name.clone()), + Expression::Identifier(identifier) => Some(identifier.name.clone()), + _ => None, + } +} + +fn direct_column(expression: &Expression) -> Option { + match unwrap_paren(expression) { + Expression::Column(column) => Some(ColumnFact { + name: column.name.name.clone(), + qualifier: column + .table + .as_ref() + .map(|table| table.name.to_ascii_lowercase()), + }), + Expression::Identifier(identifier) => Some(ColumnFact { + name: identifier.name.clone(), + qualifier: None, + }), + _ => None, + } +} + +fn string_literal(literal: &Literal) -> Option { + match literal { + Literal::String(value) => Some(format!("'{}'", value.replace('\'', "''"))), + Literal::NationalString(value) => Some(format!("N'{}'", value.replace('\'', "''"))), + _ => None, + } +} + +fn direct_string_literal(expression: &Expression) -> Option { + match unwrap_paren(expression) { + Expression::Literal(literal) => string_literal(literal), + _ => None, + } +} + +fn numeric_value(expression: &Expression) -> Option { + match unwrap_paren(expression) { + Expression::Literal(literal) => match literal.as_ref() { + Literal::Number(value) => Some(value.clone()), + _ => None, + }, + Expression::Neg(negation) => numeric_value(&negation.this).map(|value| format!("-{value}")), + _ => None, + } +} diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/mod.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/mod.rs new file mode 100644 index 0000000000..90c1051a7c --- /dev/null +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/mod.rs @@ -0,0 +1,3 @@ +pub mod facts; +pub mod shape; +pub mod tokens; diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/shape.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/shape.rs new file mode 100644 index 0000000000..f11a64ac32 --- /dev/null +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/shape.rs @@ -0,0 +1,258 @@ +use crate::query_facts::constants::{ + CALL_STUB, PLACEHOLDER_PREFIX, PLACEHOLDER_VALUE, REF_STUB, SOURCE_STUB, TABLE_FUNCTION_CALL, + TABLE_FUNCTION_STUB, UDF_CALL, UDF_STUB, +}; +use crate::query_facts::models::{CallKind, ModelCall, ModelShape}; +use crate::rules::main::compact_sql::compact_sql; +use sqlbuild_analysis::semantic_validation::main::normalize_mapped::normalize_mapped; +use sqlbuild_sqltext::sql_scan::main::comment_end::comment_end; +use sqlbuild_sqltext::sql_scan::main::matching_paren::matching_paren; +use sqlbuild_sqltext::sql_scan::main::non_code_end::non_code_end; +use sqlbuild_sqltext::sql_scan::main::quote_policy::quote_policy; +use sqlbuild_sqltext::sql_scan::models::QuotePolicy; + +pub(crate) fn model_shape(sql: &str, dialect: &str) -> Result { + let policy = QuotePolicy { + dollar_quotes: true, + ..quote_policy(dialect) + }; + let nested = matches!( + dialect.to_ascii_lowercase().as_str(), + "generic" | "postgres" | "postgresql" | "mssql" | "sqlserver" | "tsql" + ); + let (stubbed, stub_offsets, calls) = stub_calls(sql, policy, nested); + let normalized = normalize_mapped(&stubbed, dialect)?; + let end = stub_offsets.last().copied().unwrap_or_default(); + let offsets: Vec = normalized + .offsets + .iter() + .map(|offset| stub_offsets.get(*offset).copied().unwrap_or(end)) + .collect(); + Ok(ModelShape { + sql: normalized.sql, + offsets, + calls, + }) +} + +/// Every `__ref`/`__source` call in code, as the dependency text rules compare. +pub(crate) fn dependency_texts(calls: &[ModelCall]) -> Vec { + calls + .iter() + .filter(|call| call.kind != CallKind::Other) + .map(|call| call.text.clone()) + .collect() +} + +pub(crate) fn stub_call(name: &str) -> Option<(CallKind, usize)> { + for (prefix, kind) in [ + (REF_STUB, CallKind::Ref), + (SOURCE_STUB, CallKind::Source), + (CALL_STUB, CallKind::Other), + ] { + let Some(number) = name.strip_prefix(prefix) else { + continue; + }; + let Ok(number) = number.parse::() else { + return None; + }; + return Some((kind, number)); + } + None +} + +fn call_kind(word: &str) -> Option { + match word { + "__ref" => Some(CallKind::Ref), + "__source" => Some(CallKind::Source), + "__seed" | "__dbt_ref" => Some(CallKind::Other), + _ => None, + } +} + +fn stub_prefix(kind: CallKind) -> &'static str { + match kind { + CallKind::Ref => REF_STUB, + CallKind::Source => SOURCE_STUB, + CallKind::Other => CALL_STUB, + } +} + +fn is_word_byte(byte: u8) -> bool { + byte.is_ascii_alphanumeric() || byte == b'_' || byte == b'$' +} + +fn nested_comment_end(bytes: &[u8], index: usize) -> Option { + if !bytes[index..].starts_with(b"/*") { + return None; + } + let mut depth = 0_usize; + let mut cursor = index; + while cursor < bytes.len() { + if bytes[cursor..].starts_with(b"/*") { + depth += 1; + cursor += 2; + } else if bytes[cursor..].starts_with(b"*/") { + depth -= 1; + cursor += 2; + if depth == 0 { + return Some(cursor); + } + } else { + cursor += 1; + } + } + Some(bytes.len()) +} + +fn skip_space(bytes: &[u8], mut index: usize) -> usize { + loop { + while bytes.get(index).is_some_and(u8::is_ascii_whitespace) { + index += 1; + } + match comment_end(bytes, index) { + Ok(Some(end)) => index = end, + _ => return index, + } + } +} + +fn quoted_name(sql: &str, open: usize) -> Option<(String, usize)> { + let rest = sql.get(open..)?.strip_prefix("(\"")?; + let length = rest.find('"')?; + let name = &rest[..length]; + let valid = name + .bytes() + .next() + .is_some_and(|first| first.is_ascii_alphabetic() || first == b'_') + && name + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || byte == b'_'); + let close = open + 2 + length + 1; + (valid && sql.as_bytes().get(close) == Some(&b')')).then(|| (name.to_owned(), close)) +} + +fn stub_calls( + sql: &str, + policy: QuotePolicy, + nested_comments: bool, +) -> (String, Vec, Vec) { + let bytes = sql.as_bytes(); + let mut edits: Vec<(usize, usize, String)> = Vec::new(); + let mut calls: Vec = Vec::new(); + let mut index = 0; + while index < bytes.len() { + if nested_comments && let Some(end) = nested_comment_end(bytes, index) { + index = end; + continue; + } + match non_code_end(bytes, index, policy) { + Ok(Some(end)) => { + index = end; + continue; + } + Ok(None) => {} + Err(_) => break, + } + if bytes[index..].starts_with(PLACEHOLDER_PREFIX) + && bytes + .get(index + PLACEHOLDER_PREFIX.len()) + .is_some_and(|byte| is_word_byte(*byte) && *byte != b'$') + { + let start = index; + index += PLACEHOLDER_PREFIX.len(); + while index < bytes.len() && is_word_byte(bytes[index]) && bytes[index] != b'$' { + index += 1; + } + edits.push((start, index, PLACEHOLDER_VALUE.to_owned())); + continue; + } + if !(bytes[index].is_ascii_alphabetic() || bytes[index] == b'_') { + index += 1; + continue; + } + let start = index; + while index < bytes.len() && is_word_byte(bytes[index]) { + index += 1; + } + let word = sql[start..index].to_ascii_lowercase(); + if word == UDF_CALL || word == TABLE_FUNCTION_CALL { + let Some((name, close)) = quoted_name(sql, index) else { + continue; + }; + let call = close + 1; + let mut open = call; + while bytes.get(open).is_some_and(u8::is_ascii_whitespace) { + open += 1; + } + if bytes.get(open) != Some(&b'(') { + continue; + } + let stub = if word == UDF_CALL { + UDF_STUB + } else { + TABLE_FUNCTION_STUB + }; + edits.push((start, open, format!("{stub}{name}"))); + index = open; + continue; + } + let Some(kind) = call_kind(&word) else { + continue; + }; + let open = skip_space(bytes, index); + if bytes.get(open) != Some(&b'(') { + continue; + } + let Ok(close) = matching_paren(bytes, open, policy) else { + continue; + }; + let number = calls.len(); + calls.push(ModelCall { + kind, + text: format!("{word}{}", compact_sql(&sql[open..=close], policy)), + target: call_target(&sql[open + 1..close]), + }); + edits.push((start, close + 1, format!("{}{number}", stub_prefix(kind)))); + index = close + 1; + } + let mut stubbed = String::with_capacity(sql.len()); + let mut offsets: Vec = Vec::with_capacity(sql.len() + 1); + let mut character = 0; + let mut cursor = 0; + for (start, end, replacement) in edits { + for value in sql[cursor..start].chars() { + stubbed.push(value); + offsets.push(character); + character += 1; + } + stubbed.push_str(&replacement); + offsets.extend(std::iter::repeat_n(character, replacement.chars().count())); + character += sql[start..end].chars().count(); + cursor = end; + } + for value in sql[cursor..].chars() { + stubbed.push(value); + offsets.push(character); + character += 1; + } + offsets.push(character); + (stubbed, offsets, calls) +} + +fn call_target(arguments: &str) -> Option { + let argument = arguments.trim(); + for quote in ['"', '\''] { + if let Some(inner) = argument + .strip_prefix(quote) + .and_then(|value| value.strip_suffix(quote)) + { + let doubled = format!("{quote}{quote}"); + if inner.replace(&doubled, "").contains(quote) { + return None; + } + return Some(inner.replace(&doubled, "e.to_string())); + } + } + (!argument.is_empty() && argument.bytes().all(is_word_byte)).then(|| argument.to_owned()) +} diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs new file mode 100644 index 0000000000..714c8f9f04 --- /dev/null +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs @@ -0,0 +1,597 @@ +use crate::query_facts::constants::MIN_CAST_TOKENS; +use crate::query_facts::models::ComparisonSpan; +use polyglot_sql::tokens::{Token, TokenType}; +use std::cell::RefCell; +use std::collections::HashSet; +use std::ops::Range; + +const ITEM_CLAUSES: [&str; 14] = [ + "FROM", + "WHERE", + "GROUP", + "HAVING", + "QUALIFY", + "ORDER", + "LIMIT", + "UNION", + "INTERSECT", + "EXCEPT", + "MINUS", + "WINDOW", + "INTO", + "FETCH", +]; +const LEFT_BOUNDARIES: [&str; 18] = [ + "AND", "OR", "NOT", "WHERE", "ON", "WHEN", "THEN", "ELSE", "HAVING", "QUALIFY", "SELECT", "BY", + "USING", "AS", "FROM", "JOIN", "DISTINCT", "RETURN", +]; +const RIGHT_BOUNDARIES: [&str; 31] = [ + "AND", + "OR", + "THEN", + "ELSE", + "WHEN", + "FROM", + "WHERE", + "GROUP", + "ORDER", + "HAVING", + "QUALIFY", + "LIMIT", + "UNION", + "INTERSECT", + "EXCEPT", + "MINUS", + "AS", + "JOIN", + "INNER", + "LEFT", + "RIGHT", + "FULL", + "CROSS", + "NATURAL", + "ON", + "USING", + "WINDOW", + "ASC", + "DESC", + "NULLS", + "OFFSET", +]; +const NON_FUNCTION_CALLS: [&str; 41] = [ + "IN", + "CAST", + "TRY_CAST", + "SAFE_CAST", + "EXISTS", + "VALUES", + "OVER", + "FILTER", + "WITHIN", + "AS", + "FROM", + "JOIN", + "ON", + "USING", + "AND", + "OR", + "NOT", + "SELECT", + "WHERE", + "ANY", + "ALL", + "SOME", + "INTERVAL", + "EXTRACT", + "SUBSTRING", + "SUBSTR", + "TRIM", + "POSITION", + "CEIL", + "FLOOR", + "OVERLAY", + "CONVERT", + "ARRAY", + "STRUCT", + "WHEN", + "THEN", + "ELSE", + "CASE", + "IS", + "LIKE", + "BETWEEN", +]; +const PREFIX_KEYWORDS: [&str; 3] = ["NOT", "INTERVAL", "EXISTS"]; +const PREFIX_CALLS: [&str; 12] = [ + "CAST", + "TRY_CAST", + "SAFE_CAST", + "CEIL", + "FLOOR", + "SUBSTRING", + "SUBSTR", + "TRIM", + "POSITION", + "OVERLAY", + "CONVERT", + "TRY_CONVERT", +]; +const TYPED_STRING_KEYWORDS: [&str; 7] = [ + "DATE", + "TIME", + "TIMESTAMP", + "DATETIME", + "TIMESTAMP_NTZ", + "TIMESTAMP_LTZ", + "TIMESTAMP_TZ", +]; + +/// The comparison operator a comparison fact's source range is anchored on. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum ComparisonOperator { + Eq, + NotEq, + Lt, + LtEq, + Gt, + GtEq, + Between, + In, +} + +/// One SELECT keyword's extent and its projection items as token ranges. +#[derive(Debug)] +pub(crate) struct SelectTokens { + pub(crate) start: usize, + pub(crate) end: usize, + pub(crate) items: Vec>, +} + +/// The code tokens of a normalized query, without comments, over its text. +pub(crate) struct QueryTokens<'a> { + tokens: Vec, + sql: &'a str, + bytes: Vec, + used: RefCell>, +} + +impl<'a> QueryTokens<'a> { + pub(crate) fn new(tokens: Vec, sql: &'a str) -> Self { + let mut bytes: Vec = sql.char_indices().map(|(byte, _)| byte).collect(); + bytes.push(sql.len()); + Self { + sql, + bytes, + used: RefCell::default(), + tokens: tokens + .into_iter() + .filter(|token| { + !matches!( + token.token_type, + TokenType::BlockComment + | TokenType::LineComment + | TokenType::Space + | TokenType::Break + ) + }) + .collect(), + } + } + + /// The normalized text between characters `start` and `end`. + pub(crate) fn slice(&self, start: usize, end: usize) -> Option<&'a str> { + self.sql.get(*self.bytes.get(start)?..*self.bytes.get(end)?) + } + + pub(crate) fn start(&self, index: usize) -> usize { + self.tokens[index].span.start + } + + pub(crate) fn end(&self, index: usize) -> usize { + self.tokens[index].span.end + } + + fn kind(&self, index: usize) -> Option { + self.tokens.get(index).map(|token| token.token_type) + } + + fn word(&self, index: usize) -> Option { + let token = self.tokens.get(index)?; + if literal(token.token_type) { + return None; + } + token + .text + .split_whitespace() + .next() + .map(str::to_ascii_uppercase) + } + + fn is_word(&self, index: usize, words: &[&str]) -> bool { + self.word(index) + .is_some_and(|word| words.contains(&word.as_str())) + } + + fn opens(&self, index: usize) -> bool { + matches!( + self.kind(index), + Some(TokenType::LParen | TokenType::LBracket | TokenType::LBrace) + ) + } + + fn closes(&self, index: usize) -> bool { + matches!( + self.kind(index), + Some(TokenType::RParen | TokenType::RBracket | TokenType::RBrace) + ) + } + + fn matching_close(&self, open: usize) -> Option { + let mut depth = 0_usize; + for index in open..self.tokens.len() { + if self.opens(index) { + depth += 1; + } else if self.closes(index) { + depth = depth.checked_sub(1)?; + if depth == 0 { + return Some(index); + } + } + } + None + } + + pub(crate) fn len(&self) -> usize { + self.tokens.len() + } + + /// The first token at parenthesis depth 0 spelling `word`. + pub(crate) fn top_level_word(&self, word: &str) -> Option { + let mut depth = 0_usize; + for index in 0..self.tokens.len() { + if self.opens(index) { + depth += 1; + } else if self.closes(index) { + depth = depth.saturating_sub(1); + } else if depth == 0 && self.is_word(index, &[word]) { + return Some(index); + } + } + None + } + + /// Whether a function call starts in `range`. + pub(crate) fn calls_function(&self, range: Range) -> bool { + range.into_iter().any(|index| { + self.opens(index + 1) + && self.kind(index + 1) == Some(TokenType::LParen) + && self.word(index).is_some_and(|word| { + word.starts_with(|first: char| first.is_ascii_alphabetic() || first == '_') + && !NON_FUNCTION_CALLS.contains(&word.as_str()) + }) + }) + } + + /// Every SELECT keyword's region and projection items, in source order. + pub(crate) fn selects(&self) -> Vec { + (0..self.tokens.len()) + .filter(|index| self.kind(*index) == Some(TokenType::Select)) + .map(|index| self.select_tokens(index)) + .collect() + } + + fn select_tokens(&self, select: usize) -> SelectTokens { + let mut end = self.tokens.len(); + let mut depth = 0_usize; + for index in select + 1..self.tokens.len() { + if self.opens(index) { + depth += 1; + } else if self.closes(index) { + if depth == 0 { + end = index; + break; + } + depth -= 1; + } else if depth == 0 + && (self.kind(index) == Some(TokenType::Semicolon) + || (self.is_word(index, &["UNION", "INTERSECT", "MINUS"])) + || (self.is_word(index, &["EXCEPT"]) + && self.kind(index - 1) != Some(TokenType::Star))) + { + end = index; + break; + } + } + let mut cursor = select + 1; + loop { + if self.is_word(cursor, &["DISTINCT", "ALL"]) { + cursor += 1; + if self.is_word(cursor, &["ON"]) + && self.opens(cursor + 1) + && let Some(close) = self.matching_close(cursor + 1) + { + cursor = close + 1; + } + } else if self.is_word(cursor, &["TOP"]) { + cursor += 2; + } else { + break; + } + } + let mut items: Vec> = Vec::new(); + let mut item_start = cursor; + let mut depth = 0_usize; + let mut index = cursor; + while index < end { + if self.opens(index) { + depth += 1; + } else if self.closes(index) { + depth = depth.saturating_sub(1); + } else if depth == 0 { + if self.kind(index) == Some(TokenType::Comma) { + items.push(item_start..index); + item_start = index + 1; + } else if self.is_word(index, &ITEM_CLAUSES) + && !(self.is_word(index, &["EXCEPT"]) + && self.kind(index - 1) == Some(TokenType::Star)) + { + break; + } + } + index += 1; + } + if item_start < index { + items.push(item_start..index); + } + SelectTokens { + start: self.start(select), + end: if end < self.tokens.len() { + self.start(end) + } else { + usize::MAX + }, + items, + } + } + + /// The item's tokens without a trailing alias. + pub(crate) fn expression_range(&self, item: &Range, aliased: bool) -> Range { + let mut end = item.end; + if aliased && end > item.start { + end -= 1; + if end > item.start && self.is_word(end - 1, &["AS"]) { + end -= 1; + } + } + item.start..end + } + + /// The first `*` of an item. + pub(crate) fn star_start(&self, item: &Range) -> Option { + item.clone() + .find(|index| self.kind(*index) == Some(TokenType::Star)) + .map(|index| self.start(index)) + } + + /// Where the expression starts: its first token past casts, prefix operators and parentheses. + pub(crate) fn leaf_start(&self, range: &Range) -> Option { + let mut index = range.start; + while index < range.end { + let next_opens = self.opens(index + 1); + if matches!( + self.kind(index), + Some( + TokenType::LParen + | TokenType::LBracket + | TokenType::Dash + | TokenType::Plus + | TokenType::Tilde + ) + ) || self.is_word(index, &PREFIX_KEYWORDS) + || (self.is_word(index, &PREFIX_CALLS) && next_opens) + || (self.is_word(index, &["ARRAY"]) + && self.kind(index + 1) == Some(TokenType::LBracket)) + || (self.is_word(index, &TYPED_STRING_KEYWORDS) + && self.kind(index + 1) == Some(TokenType::String)) + { + index += 1; + continue; + } + if self.is_word(index, &["EXTRACT"]) && next_opens { + let close = self.matching_close(index + 1).unwrap_or(range.end); + index = (index + 2..close) + .find(|candidate| self.is_word(*candidate, &["FROM"])) + .map_or(index + 2, |from| from + 1); + continue; + } + return Some(self.start(index)); + } + None + } + + /// The source spelling of an outer `CAST(... AS type)` or `...::type` in `range`. + pub(crate) fn cast_type(&self, range: &Range) -> Option { + let mut range = range.clone(); + while range.len() >= MIN_CAST_TOKENS + && self.opens(range.start) + && self.matching_close(range.start) == Some(range.end - 1) + { + range = range.start + 1..range.end - 1; + } + if range.len() < MIN_CAST_TOKENS { + return None; + } + let (first, last) = if self.is_word(range.start, &PREFIX_CALLS[..3]) + && self.opens(range.start + 1) + && self.matching_close(range.start + 1) == Some(range.end - 1) + { + let mut depth = 0_usize; + let mut keyword = None; + for index in range.start + 1..range.end - 1 { + if self.opens(index) { + depth += 1; + } else if self.closes(index) { + depth = depth.saturating_sub(1); + } else if depth == 1 && self.is_word(index, &["AS"]) { + keyword = Some(index); + } + } + (keyword? + 1, range.end - 2) + } else { + let mut depth = 0_usize; + let mut colon = None; + for index in range.clone() { + if self.opens(index) { + depth += 1; + } else if self.closes(index) { + depth = depth.saturating_sub(1); + } else if depth == 0 && self.kind(index) == Some(TokenType::DColon) { + colon = Some(index); + } + } + (colon? + 1, range.end - 1) + }; + if first > last { + return None; + } + self.slice(self.start(first), self.end(last)) + .map(str::to_owned) + } + + fn operator(&self, index: usize, operator: ComparisonOperator) -> bool { + match operator { + ComparisonOperator::Eq => self.kind(index) == Some(TokenType::Eq), + ComparisonOperator::NotEq => self.kind(index) == Some(TokenType::Neq), + ComparisonOperator::Lt => self.kind(index) == Some(TokenType::Lt), + ComparisonOperator::LtEq => self.kind(index) == Some(TokenType::Lte), + ComparisonOperator::Gt => self.kind(index) == Some(TokenType::Gt), + ComparisonOperator::GtEq => self.kind(index) == Some(TokenType::Gte), + ComparisonOperator::Between => self.is_word(index, &["BETWEEN"]), + ComparisonOperator::In => self.is_word(index, &["IN"]), + } + } + + /// The source range and start of a comparison, anchored on its operator token. + pub(crate) fn comparison_range( + &self, + operator: ComparisonOperator, + (left_end, right_start): (Option, Option), + negated: bool, + generated: &str, + ) -> Option { + let anchor = match (left_end, right_start) { + (Some(end), _) => (0..self.tokens.len()).find(|index| { + self.start(*index) >= end + && self.operator(*index, operator) + && !self.used.borrow().contains(index) + }), + (None, Some(start)) => (0..self.tokens.len()).rev().find(|index| { + self.end(*index) <= start + && self.operator(*index, operator) + && !self.used.borrow().contains(index) + }), + (None, None) => { + let wanted = compact(generated); + (0..self.tokens.len()).find(|index| { + self.operator(*index, operator) && !self.used.borrow().contains(index) && { + let (first, last) = self.extent(*index, operator, negated); + self.slice(self.start(first), self.end(last)) + .is_some_and(|text| compact(text) == wanted) + } + }) + } + }?; + self.used.borrow_mut().insert(anchor); + let (first, last) = self.extent(anchor, operator, negated); + Some(ComparisonSpan { + start: self.start(first), + end: self.end(last), + position: self.leaf_start(&(first..last + 1)), + }) + } + + fn extent(&self, anchor: usize, operator: ComparisonOperator, negated: bool) -> (usize, usize) { + let mut first = anchor; + if negated && first > 0 && self.is_word(first - 1, &["NOT"]) { + first -= 1; + } + let mut depth = 0_usize; + while first > 0 { + let index = first - 1; + if self.closes(index) || self.is_word(index, &["END"]) { + depth += 1; + } else if self.opens(index) || self.is_word(index, &["CASE"]) { + if depth == 0 { + break; + } + depth -= 1; + } else if depth == 0 + && (matches!( + self.kind(index), + Some(TokenType::Comma | TokenType::Semicolon) + ) || self.is_word(index, &LEFT_BOUNDARIES)) + { + break; + } + first = index; + } + let mut last = anchor; + let mut between = operator == ComparisonOperator::Between; + let mut depth = 0_usize; + let mut index = anchor + 1; + while index < self.tokens.len() { + if self.opens(index) || self.is_word(index, &["CASE"]) { + depth += 1; + } else if self.closes(index) || self.is_word(index, &["END"]) { + if depth == 0 { + break; + } + depth -= 1; + } else if depth == 0 { + if matches!( + self.kind(index), + Some(TokenType::Comma | TokenType::Semicolon) + ) { + break; + } + if between && self.is_word(index, &["AND"]) { + between = false; + } else if self.is_word(index, &RIGHT_BOUNDARIES) { + break; + } + } + last = index; + index += 1; + } + (first, last) + } +} + +fn compact(text: &str) -> String { + text.chars() + .filter(|character| !character.is_whitespace()) + .flat_map(char::to_lowercase) + .collect() +} + +fn literal(token_type: TokenType) -> bool { + matches!( + token_type, + TokenType::String + | TokenType::DollarString + | TokenType::TripleDoubleQuotedString + | TokenType::TripleSingleQuotedString + | TokenType::Number + | TokenType::QuotedIdentifier + | TokenType::BitString + | TokenType::HexString + | TokenType::HexNumber + | TokenType::ByteString + | TokenType::NationalString + | TokenType::EscapeString + | TokenType::RawString + | TokenType::HeredocString + | TokenType::HeredocStringAlternative + | TokenType::UnicodeString + ) +} diff --git a/crates/sqlbuild-rules/src/query_facts/constants.rs b/crates/sqlbuild-rules/src/query_facts/constants.rs new file mode 100644 index 0000000000..c1733c094f --- /dev/null +++ b/crates/sqlbuild-rules/src/query_facts/constants.rs @@ -0,0 +1,11 @@ +pub(crate) const REF_STUB: &str = "__sqlbuild_rules_ref_"; +pub(crate) const SOURCE_STUB: &str = "__sqlbuild_rules_source_"; +pub(crate) const CALL_STUB: &str = "__sqlbuild_rules_call_"; +pub(crate) const TABLE_FUNCTION_STUB: &str = "__sqlbuild_table_function_"; +pub(crate) const UDF_STUB: &str = "__sqlbuild_udf_"; +pub(crate) const PLACEHOLDER_PREFIX: &[u8] = b"@@@"; +pub(crate) const PLACEHOLDER_VALUE: &str = "NULL"; +pub(crate) const MAX_FUNCTION_CALL_DEPTH: usize = 128; +pub(crate) const UDF_CALL: &str = "__udf"; +pub(crate) const TABLE_FUNCTION_CALL: &str = "__table_fn"; +pub(crate) const MIN_CAST_TOKENS: usize = 2; diff --git a/crates/sqlbuild-rules/src/query_facts/main/dependency_texts.rs b/crates/sqlbuild-rules/src/query_facts/main/dependency_texts.rs new file mode 100644 index 0000000000..601d8f74d2 --- /dev/null +++ b/crates/sqlbuild-rules/src/query_facts/main/dependency_texts.rs @@ -0,0 +1,6 @@ +use crate::query_facts::models::ModelCall; + +/// Every `__ref`/`__source` call in code, as the dependency text rules compare. +pub(crate) fn dependency_texts(calls: &[ModelCall]) -> Vec { + crate::query_facts::_helpers::shape::dependency_texts(calls) +} diff --git a/crates/sqlbuild-rules/src/query_facts/main/mod.rs b/crates/sqlbuild-rules/src/query_facts/main/mod.rs new file mode 100644 index 0000000000..12999bd6c5 --- /dev/null +++ b/crates/sqlbuild-rules/src/query_facts/main/mod.rs @@ -0,0 +1,5 @@ +pub mod dependency_texts; +pub mod model_shape; +pub mod parse_fixture; +pub mod query_facts; +pub mod stub_call; diff --git a/crates/sqlbuild-rules/src/query_facts/main/model_shape.rs b/crates/sqlbuild-rules/src/query_facts/main/model_shape.rs new file mode 100644 index 0000000000..ca492081a4 --- /dev/null +++ b/crates/sqlbuild-rules/src/query_facts/main/model_shape.rs @@ -0,0 +1,6 @@ +use crate::query_facts::models::ModelShape; + +/// A model's SQL with dependency calls stubbed by occurrence, mapped back to its characters. +pub(crate) fn model_shape(sql: &str, dialect: &str) -> Result { + crate::query_facts::_helpers::shape::model_shape(sql, dialect) +} diff --git a/crates/sqlbuild-rules/src/query_facts/main/parse_fixture.rs b/crates/sqlbuild-rules/src/query_facts/main/parse_fixture.rs new file mode 100644 index 0000000000..876b8da2e4 --- /dev/null +++ b/crates/sqlbuild-rules/src/query_facts/main/parse_fixture.rs @@ -0,0 +1,6 @@ +use crate::query_facts::models::{ModelShape, ParsedFixture}; + +/// Parse a stubbed SQL-test fixture as one query. +pub(crate) fn parse_fixture(shape: ModelShape, dialect: &str) -> Result { + crate::query_facts::_helpers::facts::parse_fixture(shape, dialect) +} diff --git a/crates/sqlbuild-rules/src/query_facts/main/query_facts.rs b/crates/sqlbuild-rules/src/query_facts/main/query_facts.rs new file mode 100644 index 0000000000..dba714d2c9 --- /dev/null +++ b/crates/sqlbuild-rules/src/query_facts/main/query_facts.rs @@ -0,0 +1,6 @@ +use crate::query_facts::models::QueryFacts; + +/// Parse normalized model SQL with Polyglot and read the facts built-in rules evaluate. +pub(crate) fn query_facts(sql: &str, dialect: &str) -> Result { + crate::query_facts::_helpers::facts::query_facts(sql, dialect) +} diff --git a/crates/sqlbuild-rules/src/query_facts/main/stub_call.rs b/crates/sqlbuild-rules/src/query_facts/main/stub_call.rs new file mode 100644 index 0000000000..88c4ab6cf6 --- /dev/null +++ b/crates/sqlbuild-rules/src/query_facts/main/stub_call.rs @@ -0,0 +1,6 @@ +use crate::query_facts::models::CallKind; + +/// The call kind and occurrence a stubbed relation name stands for. +pub(crate) fn stub_call(name: &str) -> Option<(CallKind, usize)> { + crate::query_facts::_helpers::shape::stub_call(name) +} diff --git a/crates/sqlbuild-rules/src/query_facts/mod.rs b/crates/sqlbuild-rules/src/query_facts/mod.rs new file mode 100644 index 0000000000..b7bf905e8f --- /dev/null +++ b/crates/sqlbuild-rules/src/query_facts/mod.rs @@ -0,0 +1,6 @@ +//! Polyglot facts built-in model and SQL-test rules read from stubbed query shapes. + +pub mod _helpers; +pub mod constants; +pub mod main; +pub mod models; diff --git a/crates/sqlbuild-rules/src/query_facts/models.rs b/crates/sqlbuild-rules/src/query_facts/models.rs new file mode 100644 index 0000000000..0f0b525597 --- /dev/null +++ b/crates/sqlbuild-rules/src/query_facts/models.rs @@ -0,0 +1,245 @@ +use polyglot_sql::Expression; +use std::collections::BTreeSet; + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum CallKind { + Ref, + Source, + Other, +} + +#[derive(Clone, Debug)] +pub(crate) struct ModelCall { + pub(crate) kind: CallKind, + pub(crate) text: String, + pub(crate) target: Option, +} + +/// A model's SQL with dependency calls stubbed by occurrence and normalized as analysis does. +#[derive(Debug)] +pub(crate) struct ModelShape { + pub(crate) sql: String, + pub(crate) offsets: Vec, + pub(crate) calls: Vec, +} + +/// A comparison's source range and where it starts as an expression, in normalized characters. +#[derive(Clone, Copy, Debug)] +pub(crate) struct ComparisonSpan { + pub(crate) start: usize, + pub(crate) end: usize, + pub(crate) position: Option, +} + +/// The facts built-in model rules read from one query, positions as normalized characters. +#[derive(Debug, Default)] +pub(crate) struct QueryFacts { + pub(crate) ctes: Vec, + pub(crate) body: SetFacts, + pub(crate) has_order_by: bool, + pub(crate) has_limit: bool, + pub(crate) has_fetch: bool, + pub(crate) has_locks: bool, +} + +#[derive(Debug)] +pub(crate) struct CteFacts { + pub(crate) name: String, + pub(crate) name_start: Option, + pub(crate) columns: Vec, + pub(crate) query: QueryFacts, + pub(crate) dependency_calls: Vec, +} + +#[derive(Debug, Default)] +pub(crate) enum SetFacts { + Select(Box), + Query(Box), + SetOperation { + by_name: bool, + left: Box, + right: Box, + }, + #[default] + Other, +} + +#[derive(Debug, Default)] +pub(crate) struct SelectFacts { + pub(crate) items: Vec, + pub(crate) from: Vec, + pub(crate) joins: Vec, + pub(crate) filtered: bool, + pub(crate) comparisons: Vec, +} + +#[derive(Debug)] +pub(crate) enum ItemFacts { + Star { + item_start: Option, + star_start: Option, + }, + Expression(ExpressionItem), +} + +#[derive(Debug)] +pub(crate) struct ExpressionItem { + pub(crate) output_name: Option, + pub(crate) direct_projection: bool, + pub(crate) direct_column: Option, + pub(crate) cast_type: Option, + pub(crate) start: Option, +} + +#[derive(Debug)] +pub(crate) enum RelationFacts { + Table { + display: String, + parts: usize, + start: Option, + last: String, + alias: Option, + }, + Call { + kind: CallKind, + index: Option, + alias: Option, + }, + DynamicPivot, + Other, +} + +#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] +pub(crate) struct ColumnFact { + pub(crate) name: String, + pub(crate) qualifier: Option, +} + +#[derive(Debug, Default)] +pub(crate) struct ComparisonFacts { + pub(crate) span: Option, + pub(crate) generated_sql: String, + pub(crate) equality: bool, + pub(crate) value_list: bool, + pub(crate) binary: bool, + pub(crate) output_name: Option, + pub(crate) columns: BTreeSet, + pub(crate) string_literals: Vec, + pub(crate) numeric_literals: Vec, + pub(crate) modified_columns: BTreeSet, + pub(crate) modified_string_literal: bool, +} + +impl QueryFacts { + /// This query, then every query its top-level CTEs and parenthesized set branches hold. + pub(crate) fn all_queries(&self) -> Vec<&QueryFacts> { + fn collect<'a>(query: &'a QueryFacts, values: &mut Vec<&'a QueryFacts>) { + values.push(query); + for cte in &query.ctes { + collect(&cte.query, values); + } + collect_set(&query.body, values); + } + fn collect_set<'a>(body: &'a SetFacts, values: &mut Vec<&'a QueryFacts>) { + match body { + SetFacts::Query(query) => collect(query, values), + SetFacts::SetOperation { left, right, .. } => { + collect_set(left, values); + collect_set(right, values); + } + SetFacts::Select(_) | SetFacts::Other => {} + } + } + let mut values: Vec<&QueryFacts> = Vec::new(); + collect(self, &mut values); + values + } + + pub(crate) fn root_select(&self) -> Option<&SelectFacts> { + match &self.body { + SetFacts::Select(select) => Some(select), + _ => None, + } + } +} + +/// An SQL-test fixture query parsed with Polyglot, with the function-call facts its rules read. +#[derive(Debug)] +pub(crate) struct ParsedFixture { + pub(crate) statement: Expression, + pub(crate) calls: Vec, + pub(crate) select_function: bool, + pub(crate) projection_function: bool, + pub(crate) order_by_function: bool, + pub(crate) selects: usize, +} + +impl ParsedFixture { + pub(crate) fn select_calls_function(&self) -> bool { + self.select_function + } + + pub(crate) fn projection_calls_function(&self) -> bool { + self.projection_function + } + + pub(crate) fn order_by_calls_function(&self) -> bool { + self.order_by_function + } + + pub(crate) fn query_count(&self) -> usize { + self.selects + } +} + +/// Character positions of one model's SQL as line and column, with its text by character. +#[derive(Debug)] +pub(crate) struct ModelSource<'a> { + sql: &'a str, + bytes: Vec, + lines: Vec, + offsets: &'a [usize], +} + +impl<'a> ModelSource<'a> { + pub(crate) fn new(sql: &'a str, offsets: &'a [usize]) -> Self { + let mut bytes: Vec = Vec::with_capacity(sql.len() + 1); + let mut lines: Vec = vec![0]; + for (character, (byte, value)) in sql.char_indices().enumerate() { + bytes.push(byte); + if value == '\n' { + lines.push(character + 1); + } + } + bytes.push(sql.len()); + Self { + sql, + bytes, + lines, + offsets, + } + } + + fn original(&self, offset: usize) -> usize { + self.offsets + .get(offset) + .copied() + .unwrap_or_else(|| self.bytes.len() - 1) + } + + /// The 1-based line and column of normalized character `offset`. + pub(crate) fn line_column(&self, offset: usize) -> (u64, u64) { + let character = self.original(offset); + let line = self.lines.partition_point(|start| *start <= character); + let column = character - self.lines[line - 1] + 1; + (line as u64, column as u64) + } + + /// The model's own text between normalized characters `start` and `end`. + pub(crate) fn text(&self, start: usize, end: usize) -> &'a str { + let last = self.bytes.len() - 1; + let from = self.bytes[self.original(start).min(last)]; + let to = self.bytes[self.original(end).min(last)]; + self.sql.get(from..to.max(from)).unwrap_or_default() + } +} diff --git a/crates/sqlbuild-rules/src/rules/_helpers/dynamic_contracts.rs b/crates/sqlbuild-rules/src/rules/_helpers/dynamic_contracts.rs index 3bd6fefdc0..7ad6246b76 100644 --- a/crates/sqlbuild-rules/src/rules/_helpers/dynamic_contracts.rs +++ b/crates/sqlbuild-rules/src/rules/_helpers/dynamic_contracts.rs @@ -2,9 +2,10 @@ use std::collections::BTreeMap; use crate::constants::{ENFORCED_CONTRACT, SELECT_STAR_MODEL_RULE_CODE}; use crate::models::{Fault, Model, RuleMetadata, RulesConfig}; +use crate::query_facts::models::{QueryFacts, RelationFacts}; +use crate::rules::_helpers::evaluation::sole_table_name; use crate::rules::_helpers::{contract_name_types, explicit_output_types, typed_contract_columns}; use crate::rules::models::FaultCollector; -use sqlparser::ast::{PivotValueSource, Query, Select, SetExpr, TableFactor}; pub(crate) fn evaluate_without_query( model: &Model, @@ -48,52 +49,20 @@ pub(crate) fn evaluate_without_query( Some(faults.into_inner()) } -pub(crate) fn allows_output_star(query: &Query, root: &Query, model: &Model) -> bool { - let Some(select) = root_select(query) else { +pub(crate) fn allows_output_star(query: &QueryFacts, root: &QueryFacts, model: &Model) -> bool { + let Some(select) = query.root_select() else { return false; }; - let dynamic_pivot_star = select.projection.len() == 1 + let dynamic_pivot_star = select.items.len() == 1 && select.from.len() == 1 - && select.from[0].joins.is_empty() - && matches!( - &select.from[0].relation, - TableFactor::Pivot { - value_source: PivotValueSource::Any(_), - .. - } - ); + && select.joins.is_empty() + && matches!(select.from[0], RelationFacts::DynamicPivot); dynamic_pivot_star || (model.dynamic_columns_proven && std::ptr::eq(query, root) - && root - .with - .as_ref() - .and_then(|with| with.cte_tables.last()) - .is_some_and(|cte| { - sole_table_name(query) - .is_some_and(|name| name.eq_ignore_ascii_case(&cte.alias.name.value)) - })) -} - -fn root_select(query: &Query) -> Option<&Select> { - let SetExpr::Select(select) = query.body.as_ref() else { - return None; - }; - Some(select) -} - -fn sole_table_name(query: &Query) -> Option { - let select = root_select(query)?; - if select.from.len() != 1 || !select.from[0].joins.is_empty() { - return None; - } - let TableFactor::Table { - name, args: None, .. - } = &select.from[0].relation - else { - return None; - }; - Some(name.to_string()) + && root.ctes.last().is_some_and(|cte| { + sole_table_name(query).is_some_and(|name| name.eq_ignore_ascii_case(&cte.name)) + })) } fn rules_need_no_query(model: &Model, selected: &BTreeMap) -> bool { diff --git a/crates/sqlbuild-rules/src/rules/_helpers/evaluation.rs b/crates/sqlbuild-rules/src/rules/_helpers/evaluation.rs index 621c051d3d..f8e62ee581 100644 --- a/crates/sqlbuild-rules/src/rules/_helpers/evaluation.rs +++ b/crates/sqlbuild-rules/src/rules/_helpers/evaluation.rs @@ -2,10 +2,17 @@ use crate::configuration::main::setting_help::setting_help; use crate::configuration::main::toml_string_array::toml_string_array; use crate::constants::MIN_CUSTOM_RULE_TEST_CASES_KEY; use crate::constants::{ - DECLARATION_DOMAIN_COMPONENTS, ENFORCED_CONTRACT, NEGATION_OPERATOR, REFERENCE_KIND, - SOURCE_REFERENCE_KIND, VIEW_MATERIALIZATION, + DECLARATION_DOMAIN_COMPONENTS, ENFORCED_CONTRACT, REFERENCE_KIND, SOURCE_REFERENCE_KIND, + VIEW_MATERIALIZATION, }; use crate::models::{Declaration, EvaluateRequest, Fault, Model, RuleMetadata, RulesConfig}; +use crate::query_facts::main::dependency_texts::dependency_texts; +use crate::query_facts::main::model_shape::model_shape; +use crate::query_facts::main::query_facts::query_facts; +use crate::query_facts::models::{ + CallKind, ColumnFact, ComparisonFacts, ItemFacts, ModelCall, ModelSource, QueryFacts, + RelationFacts, SelectFacts, +}; use crate::rules::_helpers::domain_layout::folder_layer_details; use crate::rules::_helpers::{ contract_name_types, dynamic_contracts, explicit_output_types, model_layers, numeric_decisions, @@ -16,23 +23,11 @@ use crate::rules::models::{ }; use globset::{Glob, GlobSetBuilder}; use sqlbuild_core::identifier_text::main::is_lowercase_identifier::is_lowercase_identifier; -use sqlbuild_sqltext::sql_scan::main::matching_paren::matching_paren; -use sqlbuild_sqltext::sql_scan::main::non_code_end::non_code_end; use sqlbuild_sqltext::sql_scan::main::quote_policy::quote_policy as rules_quote_policy; -use sqlbuild_sqltext::sql_scan::main::rules_dialect::rules_dialect; use sqlbuild_sqltext::sql_scan::models::QuotePolicy; -use sqlparser::ast::{ - BinaryOperator, Expr, GroupByExpr, JoinConstraint, JoinOperator, Query, Select, SelectItem, - SetExpr, Spanned, Statement, TableFactor, Value, Visit, Visitor, -}; -use sqlparser::dialect::{Dialect, GenericDialect}; -use sqlparser::parser::{Parser, ParserError, ParserOptions}; -use sqlparser::tokenizer::{Token, Tokenizer}; use std::collections::{BTreeMap, BTreeSet, HashSet}; -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)] @@ -41,47 +36,30 @@ struct Position { column: u64, } -#[derive(Default)] -struct ComparisonFact { - position: Option, - sql: String, - equality: bool, - value_list: bool, - binary: bool, - output_name: Option, - columns: BTreeSet, - string_literals: Vec, - numeric_literals: Vec, - modified_columns: BTreeSet, - modified_string_literal: bool, - source_context: SourceContext, -} - -#[derive(Clone, Eq, Ord, PartialEq, PartialOrd)] -struct ColumnFact { - name: String, - qualifier: Option, -} - #[derive(Clone, Default)] struct SourceContext { relations: BTreeSet, sole_relation: bool, } -#[derive(Default)] -struct SelectFacts { - comparisons: Vec, -} - -struct ParsedModel<'a> { - query: Query, - model: &'a Model, +pub(crate) struct ParsedModel<'a> { + pub(crate) facts: &'a QueryFacts, + pub(crate) source: ModelSource<'a>, + pub(crate) model: &'a Model, classification: ModelClassification, authored_compact_sql: String, quote_policy: QuotePolicy, } +impl ParsedModel<'_> { + fn position(&self, offset: Option) -> Option { + offset.map(|offset| { + let (line, column) = self.source.line_column(offset); + Position { line, column } + }) + } +} + struct ModelClassification { authored_dependencies: Vec, ctes: Vec, @@ -98,7 +76,6 @@ struct TestRuleEvaluation<'a> { struct CteClassification { name: String, - position: Position, dependency_import: bool, kind: CteKind, } @@ -144,6 +121,7 @@ fn evaluate_model_inner(request: ModelEvaluationRequest<'_>) -> Result) -> Result dependencies, - None => dependency_calls(&model.query_sql, &request.dialect)?, - }; - let classification = classify_model(&query, model, dependencies); + let source = ModelSource::new(&model.query_sql, &shape.offsets); + let classification = classify_model(facts, model, &shape.calls); let parsed = ParsedModel { - query, + facts, + source, model, classification, authored_compact_sql: numeric_decisions::compact_sql(&model.authored_sql, quote_policy), @@ -229,7 +192,7 @@ fn evaluate_model_inner(request: ModelEvaluationRequest<'_>) -> Result) -> Result) -> Vec { - let faults = FaultCollector::default(); - if let Some(rule) = request.selected.get("SQBRGRAPH101") { - model_layers::internal_forward_refs(request, rule, &faults); - } - if let Some(rule) = request.selected.get("SQBRDECLARATION201") { - duplicate_enums(request.request, rule, &faults); - } - if let Some(rule) = request.selected.get("SQBRDECLARATION301") { - declaration_domain_placement(request.request, &request.request.config, rule, &faults); - } - if let Some(rule) = request.selected.get("SQBRTEST301") { - custom_rule_test_coverage( - request.request, - &request.request.config, - (request.selected, rule), - &faults, - ); - } - faults.into_inner() -} - -fn requires_parsed_model(selected: &BTreeMap) -> bool { - selected.values().any(|rule| requires_model_query(rule)) -} - -fn requires_model_query(rule: &RuleMetadata) -> bool { - !rule.custom - && (rule.code.starts_with("SQBRMODEL") - || rule.code.starts_with("SQBRCONTRACT") - || matches!( - rule.code.as_str(), - "SQBRGRAPH102" - | "SQBRPROJECT101" - | "SQBRPROJECT102" - | "SQBRPROJECT103" - | "SQBRPROJECT104" - | "SQBRPROJECT105" - | "SQBRPROJECT106" - | "SQBRDECLARATION101" - | "SQBRDECLARATION102" - | "SQBRTEST201" - | "SQBRTEST202" - )) -} - -pub(super) fn parse_rule_statements( - sql: &str, - dialect_name: &str, -) -> Result, ParserError> { - parse_rule_statements_with_dependencies(sql, dialect_name, None) -} - -fn parse_rule_statements_with_dependencies( - sql: &str, - dialect_name: &str, - dependencies: Option<&mut Option>>, -) -> Result, ParserError> { - let dialect = rules_dialect(dialect_name); - match parse_with_dialect(sql, dialect.as_ref(), dependencies) { - Ok(statements) => Ok(statements), - Err(primary) if !dialect_name.eq_ignore_ascii_case("generic") => { - let fallback_sql = normalize_generic_fallback(sql); - parse_with_dialect(&fallback_sql, &GenericDialect {}, None).map_err(|fallback| { - ParserError::ParserError(format!( - "{primary}; generic fallback also failed: {fallback}" - )) - }) - } - Err(error) => Err(error), - } -} - -fn normalize_generic_fallback(sql: &str) -> String { - let policy = rules_quote_policy("generic"); - let mut bytes = sql.as_bytes().to_vec(); - let mut index = 0; - while index < bytes.len() { - let byte = bytes[index]; - let next = bytes.get(index + 1).copied(); - match non_code_end(&bytes, index, policy) { - Ok(Some(end)) => { - index = end; - continue; - } - Ok(None) => {} - Err(_) => break, - } - if byte == b'-' && next == Some(b'>') { - bytes[index] = b','; - bytes[index + 1] = b' '; - index += 2; - continue; - } else if byte == b':' - && bytes.get(index.wrapping_sub(1)) != Some(&b':') - && next != Some(b':') - { - bytes[index] = b'.'; - } - index += 1; - } - String::from_utf8(bytes).unwrap_or_else(|_| sql.to_owned()) -} - -fn parse_with_dialect( - sql: &str, - dialect: &dyn Dialect, - dependencies: Option<&mut Option>>, -) -> Result, ParserError> { - let tokens = Tokenizer::new(dialect, sql).tokenize_with_location()?; - if let Some(dependencies) = dependencies { - *dependencies = Some(dependency_calls_from_tokens( - tokens.iter().map(|token| &token.token), - )); - } - let mut parser = Parser::new(dialect) - .with_options(ParserOptions::new().with_trailing_commas(true)) - .with_recursion_limit(RULES_PARSER_RECURSION_LIMIT) - .with_tokens_with_locations(tokens); - parser.parse_statements() -} - -pub(crate) fn normalize_rules_sql(dialect: &str, sql: &str) -> String { - let policy = rules_quote_policy(dialect); - let sql = normalize_table_function_calls(sql, policy); - if !dialect.eq_ignore_ascii_case("snowflake") { - return sql; - } - let mut bytes = sql.as_bytes().to_vec(); - let mut index = 0; - while index < bytes.len() { - let byte = bytes[index]; - match non_code_end(&bytes, index, policy) { - Ok(Some(end)) => { - index = end; - continue; - } - Ok(None) => {} - Err(_) => break, - } - if byte == b',' && clause_follows(&bytes, index + 1) { - bytes[index] = b' '; - index += 1; - continue; - } - if byte.is_ascii_alphabetic() { - let start = index; - while index < bytes.len() - && (bytes[index].is_ascii_alphanumeric() || bytes[index] == b'_') - { - index += 1; - } - let word = &sql.as_bytes()[start..index]; - let mut following = index; - while bytes.get(following).is_some_and(u8::is_ascii_whitespace) { - following += 1; - } - if word.eq_ignore_ascii_case(b"EXCLUDE") - && is_star_modifier_before(sql.as_bytes(), start) - && bytes.get(following) == Some(&b'(') - && let Ok(close) = matching_paren(&bytes, following, policy) - { - blank_preserving_newlines(&mut bytes[start..=close]); - index = close + 1; - continue; - } - if word.eq_ignore_ascii_case(b"ARRAY") - && is_cast_type_before(sql.as_bytes(), start) - && bytes.get(following) == Some(&b'(') - && let Ok(close) = matching_paren(&bytes, following, policy) - { - bytes[start..index].copy_from_slice(b"TEXT "); - blank_preserving_newlines(&mut bytes[following..=close]); - index = close + 1; - continue; - } - let typed_lambda = bytes.get(following) == Some(&b'-') - && bytes.get(following + 1) == Some(&b'>') - && is_lambda_parameter_before(sql.as_bytes(), start) - && is_lambda_parameter_type(word); - if typed_lambda { - bytes[start..index].fill(b' '); - } - continue; - } - index += 1; - } - String::from_utf8(bytes).unwrap_or(sql) -} - -fn normalize_table_function_calls(sql: &str, policy: QuotePolicy) -> String { - const TABLE_FUNCTION: &[u8] = b"__table_fn("; - let mut bytes = sql.as_bytes().to_vec(); - let mut cursor = 0; - while cursor + TABLE_FUNCTION.len() <= bytes.len() { - let Some(token_start) = find_token_outside_syntax(&bytes, TABLE_FUNCTION, cursor, policy) - else { - break; - }; - let open = token_start + TABLE_FUNCTION.len() - 1; - let Ok(close) = matching_paren(&bytes, open, policy) else { - break; - }; - let mut next = close + 1; - while bytes.get(next).is_some_and(u8::is_ascii_whitespace) { - next += 1; - } - if bytes.get(next) == Some(&b'(') { - bytes[close] = b','; - bytes[next] = b' '; - } - cursor = next.saturating_add(1); - } - String::from_utf8(bytes).unwrap_or_else(|_| sql.to_owned()) -} - -fn blank_preserving_newlines(sql: &mut [u8]) { - for byte in sql { - if !matches!(*byte, b'\n' | b'\r') { - *byte = b' '; - } - } -} - -fn find_token_outside_syntax( - sql: &[u8], - token: &[u8], - start: usize, - policy: QuotePolicy, -) -> Option { - let mut index = start; - while index + token.len() <= sql.len() { - match non_code_end(sql, index, policy) { - Ok(Some(end)) => { - index = end; - continue; - } - Ok(None) => {} - Err(_) => return None, - } - if sql[index..index + token.len()].eq_ignore_ascii_case(token) { - return Some(index); - } - index += 1; - } - None -} - -fn clause_follows(sql: &[u8], start: usize) -> bool { - let mut cursor = start; - while sql.get(cursor).is_some_and(u8::is_ascii_whitespace) { - cursor += 1; - } - [ - "FROM", "WHERE", "GROUP", "HAVING", "QUALIFY", "ORDER", "LIMIT", - ] - .iter() - .any(|keyword| { - let bytes = keyword.as_bytes(); - sql.get(cursor..cursor + bytes.len()) - .is_some_and(|candidate| candidate.eq_ignore_ascii_case(bytes)) - && sql - .get(cursor + bytes.len()) - .is_none_or(|next| !next.is_ascii_alphanumeric() && *next != b'_') - }) -} - -fn is_star_modifier_before(sql: &[u8], modifier_start: usize) -> bool { - let mut cursor = modifier_start; - while cursor > 0 && sql[cursor - 1].is_ascii_whitespace() { - cursor -= 1; - } - cursor > 0 && sql[cursor - 1] == b'*' -} - -fn is_cast_type_before(sql: &[u8], type_start: usize) -> bool { - const AS_KEYWORD_LENGTH: usize = 2; - const BYTE_BEFORE_AS_OFFSET: usize = 3; - let mut cursor = type_start; - while cursor > 0 && sql[cursor - 1].is_ascii_whitespace() { - cursor -= 1; - } - cursor >= AS_KEYWORD_LENGTH - && sql[cursor - AS_KEYWORD_LENGTH..cursor].eq_ignore_ascii_case(b"AS") - && (cursor == AS_KEYWORD_LENGTH - || !sql[cursor - BYTE_BEFORE_AS_OFFSET].is_ascii_alphanumeric()) -} - -fn is_lambda_parameter_before(sql: &[u8], type_start: usize) -> bool { - let mut cursor = type_start; - while cursor > 0 && sql[cursor - 1].is_ascii_whitespace() { - cursor -= 1; - } - cursor > 0 && (sql[cursor - 1].is_ascii_alphanumeric() || sql[cursor - 1] == b'_') -} - -fn is_lambda_parameter_type(word: &[u8]) -> bool { - [ - b"INT".as_slice(), - b"INTEGER".as_slice(), - b"STRING".as_slice(), - b"VARCHAR".as_slice(), - b"BOOLEAN".as_slice(), - b"FLOAT".as_slice(), - b"DOUBLE".as_slice(), - ] - .iter() - .any(|candidate| word.eq_ignore_ascii_case(candidate)) -} - fn fault(model: &Model, rule: &RuleMetadata, position: Option<&Position>) -> Fault { Fault { unevaluated: false, @@ -604,128 +257,117 @@ pub(super) fn path_fault( } } -fn position(node: &T) -> Position { - let start = node.span().start; - Position { - line: start.line, - column: start.column, +pub(crate) fn evaluate_project_rules(request: &ProjectEvaluationRequest<'_>) -> Vec { + let faults = FaultCollector::default(); + if let Some(rule) = request.selected.get("SQBRGRAPH101") { + model_layers::internal_forward_refs(request, rule, &faults); } -} - -fn location_position(location: sqlparser::tokenizer::Location) -> Position { - Position { - line: location.line, - column: location.column, + if let Some(rule) = request.selected.get("SQBRDECLARATION201") { + duplicate_enums(request.request, rule, &faults); } + if let Some(rule) = request.selected.get("SQBRDECLARATION301") { + declaration_domain_placement(request.request, &request.request.config, rule, &faults); + } + if let Some(rule) = request.selected.get("SQBRTEST301") { + custom_rule_test_coverage( + request.request, + &request.request.config, + (request.selected, rule), + &faults, + ); + } + faults.into_inner() } -pub(super) fn root_select(query: &Query) -> Option<&Select> { - query.body.as_select() -} - -pub(super) fn top_ctes(query: &Query) -> &[sqlparser::ast::Cte] { - query - .with - .as_ref() - .map_or(&[], |with| with.cte_tables.as_slice()) +fn requires_parsed_model(selected: &BTreeMap) -> bool { + selected.values().any(|rule| requires_model_query(rule)) } -pub(super) fn group_by_empty(group: &GroupByExpr) -> bool { - matches!(group, GroupByExpr::Expressions(values, _) if values.is_empty()) +fn requires_model_query(rule: &RuleMetadata) -> bool { + !rule.custom + && (rule.code.starts_with("SQBRMODEL") + || rule.code.starts_with("SQBRCONTRACT") + || matches!( + rule.code.as_str(), + "SQBRGRAPH102" + | "SQBRPROJECT101" + | "SQBRPROJECT102" + | "SQBRPROJECT103" + | "SQBRPROJECT104" + | "SQBRPROJECT105" + | "SQBRPROJECT106" + | "SQBRDECLARATION101" + | "SQBRDECLARATION102" + | "SQBRTEST201" + | "SQBRTEST202" + )) } -fn direct_projection(item: &SelectItem, allow_star: bool) -> bool { - match item { - SelectItem::UnnamedExpr(Expr::Identifier(_) | Expr::CompoundIdentifier(_)) => true, - SelectItem::ExprWithAlias { expr, .. } => { - matches!(expr, Expr::Identifier(_) | Expr::CompoundIdentifier(_)) - } - SelectItem::Wildcard(_) | SelectItem::QualifiedWildcard(_, _) => allow_star, - _ => false, +pub(super) fn plain_select(select: &SelectFacts, allow_star: bool) -> bool { + if select.items.is_empty() || select.filtered || !select.joins.is_empty() { + return false; + } + if select.items.len() == 1 && matches!(select.items[0], ItemFacts::Star { .. }) { + return allow_star; } + select.items.iter().all( + |item| matches!(item, ItemFacts::Expression(expression) if expression.direct_projection), + ) } -fn plain_projection(query: &Query, allow_star: bool) -> bool { - if query.with.is_some() || query.order_by.is_some() || query.limit_clause.is_some() { +fn plain_projection(query: &QueryFacts, allow_star: bool) -> bool { + if !query.ctes.is_empty() || query.has_order_by || query.has_limit { return false; } - let Some(select) = root_select(query) else { - return false; - }; - plain_select(select, allow_star) + query + .root_select() + .is_some_and(|select| plain_select(select, allow_star)) } -pub(super) fn plain_select(select: &Select, allow_star: bool) -> bool { - if select.projection.is_empty() - || select.selection.is_some() - || select.having.is_some() - || select.qualify.is_some() - || !group_by_empty(&select.group_by) - || select.from.iter().any(|source| !source.joins.is_empty()) - { - return false; - } - if select.projection.len() == 1 - && matches!( - select.projection[0], - SelectItem::Wildcard(_) | SelectItem::QualifiedWildcard(_, _) - ) - { - return allow_star; +fn dependency_kind(relation: &RelationFacts) -> Option { + match relation { + RelationFacts::Call { kind, .. } if *kind != CallKind::Other => Some(*kind), + _ => None, } - select - .projection - .iter() - .all(|item| direct_projection(item, false)) } -pub(super) fn dependency_name(factor: &TableFactor) -> Option { - let TableFactor::Table { - name, - args: Some(_), - .. - } = factor - else { - return None; - }; - let value = name.to_string().to_ascii_lowercase(); - matches!(value.as_str(), "__ref" | "__source").then_some(value) +pub(super) fn reads_dependency(select: &SelectFacts) -> bool { + select.from.len() == 1 && select.joins.is_empty() && dependency_kind(&select.from[0]).is_some() } -fn dependency_import(query: &Query) -> bool { - let Some(select) = root_select(query) else { - return false; - }; - select.from.len() == 1 - && select.from[0].joins.is_empty() - && dependency_name(&select.from[0].relation).is_some() - && plain_projection(query, true) +fn dependency_import(query: &QueryFacts) -> bool { + query.root_select().is_some_and(reads_dependency) && plain_projection(query, true) } -pub(super) fn sole_table_name(query: &Query) -> Option { - let select = root_select(query)?; - if select.from.len() != 1 || !select.from[0].joins.is_empty() { +pub(super) fn sole_table_name(query: &QueryFacts) -> Option<&str> { + let select = query.root_select()?; + if select.from.len() != 1 || !select.joins.is_empty() { return None; } - let TableFactor::Table { - name, args: None, .. - } = &select.from[0].relation - else { - return None; - }; - Some(name.to_string()) + match &select.from[0] { + RelationFacts::Table { display, .. } => Some(display), + _ => None, + } } -fn classify_model( - query: &Query, - model: &Model, - authored_dependencies: Vec, -) -> ModelClassification { +fn call_texts(calls: &[ModelCall], indexes: &[usize]) -> Vec { + let mut texts: Vec = Vec::with_capacity(indexes.len()); + for index in indexes { + if let Some(call) = calls.get(*index) { + texts.push(call.text.clone()); + } + } + texts +} + +fn classify_model(query: &QueryFacts, model: &Model, calls: &[ModelCall]) -> ModelClassification { + let authored_dependencies = dependency_texts(calls); let mut logical_seen = false; - let ctes = top_ctes(query) + let ctes = query + .ctes .iter() .map(|cte| { - let dependencies = query_dependency_calls(&cte.query); + let dependencies: Vec = call_texts(calls, &cte.dependency_calls); let dependency_import = dependency_import(&cte.query); let kind = if dependencies.is_empty() { logical_seen = true; @@ -747,8 +389,7 @@ fn classify_model( } }; CteClassification { - name: cte.alias.name.value.clone(), - position: location_position(cte.alias.name.span.start), + name: cte.name.clone(), dependency_import, kind, } @@ -760,8 +401,10 @@ fn classify_model( && ctes.len() == 1 && ctes.first().is_some_and(|cte| { cte.dependency_import - && sole_table_name(query).as_deref() == Some(cte.name.as_str()) - && root_select(query).is_some_and(|select| plain_select(select, true)) + && sole_table_name(query) == Some(cte.name.as_str()) + && query + .root_select() + .is_some_and(|select| plain_select(select, true)) }); ModelClassification { @@ -771,103 +414,9 @@ fn classify_model( } } -fn dependency_calls(source: &str, dialect_name: &str) -> Result, String> { - let dialect = rules_dialect(dialect_name); - let tokens = Tokenizer::new(dialect.as_ref(), source) - .tokenize() - .map_err(|error| format!("could not tokenize model dependencies: {error}"))?; - Ok(dependency_calls_from_tokens(tokens.iter())) -} - -fn dependency_calls_from_tokens<'a>(tokens: impl Iterator) -> Vec { - let tokens = tokens - .filter(|token| !matches!(token, Token::Whitespace(_))) - .collect::>(); - let mut calls: Vec = Vec::new(); - let mut index = 0_usize; - while index < tokens.len() { - let Token::Word(word) = tokens[index] else { - index += 1; - continue; - }; - if word.quote_style.is_some() - || !matches!( - word.value.to_ascii_lowercase().as_str(), - "__ref" | "__source" - ) - || !matches!(tokens.get(index + 1), Some(Token::LParen)) - { - index += 1; - continue; - } - - let mut call = word.value.to_ascii_lowercase(); - let mut depth = 0_usize; - let mut cursor = index + 1; - while cursor < tokens.len() { - let token = tokens[cursor]; - call.push_str(&token.to_string().to_ascii_lowercase()); - match token { - Token::LParen => depth += 1, - Token::RParen => { - depth -= 1; - if depth == 0 { - calls.push(call); - cursor += 1; - break; - } - } - _ => {} - } - cursor += 1; - } - index = cursor; - } - calls -} - -fn query_dependency_calls(query: &Query) -> Vec { - #[derive(Default)] - struct Dependencies { - calls: Vec, - } - - impl Visitor for Dependencies { - type Break = (); - - fn pre_visit_table_factor(&mut self, factor: &TableFactor) -> ControlFlow { - let TableFactor::Table { - name, - args: Some(arguments), - .. - } = factor - else { - return ControlFlow::Continue(()); - }; - let dependency = name.to_string().to_ascii_lowercase(); - if !matches!(dependency.as_str(), "__ref" | "__source") { - return ControlFlow::Continue(()); - } - let arguments = arguments - .args - .iter() - .map(ToString::to_string) - .collect::>() - .join(",") - .to_ascii_lowercase(); - self.calls.push(format!("{dependency}({arguments})")); - ControlFlow::Continue(()) - } - } - - let mut dependencies = Dependencies::default(); - let _ = query.visit(&mut dependencies); - dependencies.calls -} - fn import_ctes(parsed: &ParsedModel<'_>, rule: &RuleMetadata, faults: &FaultCollector) { let mut imported: Vec = Vec::new(); - for cte in &parsed.classification.ctes { + for (cte, facts) in parsed.classification.ctes.iter().zip(&parsed.facts.ctes) { let CteKind::Import { dependencies, rejections, @@ -878,6 +427,7 @@ fn import_ctes(parsed: &ParsedModel<'_>, rule: &RuleMetadata, faults: &FaultColl if dependencies.len() == 1 { imported.extend(dependencies.iter().cloned()); } + let position = parsed.position(facts.name_start); for rejection in rejections { let message = match rejection { ImportRejection::MultipleDependencies => { @@ -893,7 +443,7 @@ fn import_ctes(parsed: &ParsedModel<'_>, rule: &RuleMetadata, faults: &FaultColl faults.push(custom_fault!( parsed.model, rule, - Some(&cte.position), + position.as_ref(), message, None, )); @@ -937,32 +487,26 @@ fn select_star( .build() .map_err(|error| error.to_string())? .is_match(&parsed.model.relative_path); - let mut import_positions: HashSet<(u64, u64)> = HashSet::new(); - for (cte, classified) in top_ctes(&parsed.query) - .iter() - .zip(&parsed.classification.ctes) - { + let mut import_stars: HashSet = HashSet::new(); + for (cte, classified) in parsed.facts.ctes.iter().zip(&parsed.classification.ctes) { if classified.dependency_import { - for value in stars_in_query(&cte.query) { - import_positions.insert((value.line, value.column)); - } + import_stars.extend(stars_in_query(&cte.query).into_iter().flatten()); } } - for query in all_queries(&parsed.query) { - let Some(select) = root_select(query) else { + for query in parsed.facts.all_queries() { + let Some(select) = query.root_select() else { continue; }; let dynamic_output_star = - dynamic_contracts::allows_output_star(query, &parsed.query, parsed.model); - let stars = stars_in_select(select); - for at in stars { - if import_positions.contains(&(at.line, at.column)) { + dynamic_contracts::allows_output_star(query, parsed.facts, parsed.model); + for star in stars_in_select(select) { + if star.is_some_and(|offset| import_stars.contains(&offset)) { continue; } if dynamic_output_star { continue; } - let lone = select.projection.len() == 1; + let lone = select.items.len() == 1; if allowed && lone { continue; } @@ -970,7 +514,7 @@ fn select_star( faults.push(custom_fault!( parsed.model, rule, - Some(&at), + parsed.position(star).as_ref(), rule.message.clone(), remediation, )); @@ -1036,45 +580,31 @@ fn view_marker(parsed: &ParsedModel<'_>, rule: &RuleMetadata, faults: &FaultColl } fn raw_qualified_tables(parsed: &ParsedModel<'_>, rule: &RuleMetadata, faults: &FaultCollector) { - for query in all_queries(&parsed.query) { - let Some(select) = root_select(query) else { + for query in parsed.facts.all_queries() { + let Some(select) = query.root_select() else { continue; }; - for source in &select.from { - qualified_table_factor(parsed.model, &source.relation, rule, faults); - for join in &source.joins { - qualified_table_factor(parsed.model, &join.relation, rule, faults); + for relation in select.from.iter().chain(&select.joins) { + if let RelationFacts::Table { + display, + parts, + start, + .. + } = relation + && *parts > 1 + { + faults.push(custom_fault!( + parsed.model, + rule, + parsed.position(*start).as_ref(), + format!("raw qualified table {display:?} bypasses the SQLBuild graph"), + None, + )); } } } } -fn qualified_table_factor( - model: &Model, - factor: &TableFactor, - rule: &RuleMetadata, - faults: &FaultCollector, -) { - let TableFactor::Table { - name, args: None, .. - } = factor - else { - return; - }; - if name.0.len() > 1 { - faults.push(custom_fault!( - model, - rule, - Some(&position(name)), - format!( - "raw qualified table {:?} bypasses the SQLBuild graph", - name.to_string() - ), - None, - )); - } -} - fn name_grammar( parsed: &ParsedModel<'_>, config: &RulesConfig, @@ -1295,54 +825,83 @@ fn evaluate_literal_rules( selected: &BTreeMap, faults: &FaultCollector, ) { - let facts = select_facts(&parsed.query); - for comparison in facts.into_iter().flat_map(|select| select.comparisons) { - if comparison.binary - && let Some(rule) = selected.get("SQBRDECLARATION101") - { - let enum_column = comparison - .columns - .iter() - .any(|name| parsed.model.enum_columns.contains(name)); - let bare_string = comparison - .string_literals + let source_imports = source_import_names(parsed.facts); + for query in parsed.facts.all_queries() { + let Some(select) = query.root_select() else { + continue; + }; + let source_context = select_source_context(select, &source_imports); + for comparison in &select.comparisons { + evaluate_comparison(parsed, selected, (comparison, &source_context), faults); + } + } +} + +fn evaluate_comparison( + parsed: &ParsedModel<'_>, + selected: &BTreeMap, + (comparison, source_context): (&ComparisonFacts, &SourceContext), + faults: &FaultCollector, +) { + let position = parsed.position(comparison.span.and_then(|span| span.position)); + let sql = comparison + .span + .map_or(comparison.generated_sql.as_str(), |span| { + parsed.source.text(span.start, span.end) + }); + if comparison.binary + && let Some(rule) = selected.get("SQBRDECLARATION101") + { + let enum_column = comparison + .columns + .iter() + .any(|name| parsed.model.enum_columns.contains(name)); + let bare_string = comparison + .string_literals + .iter() + .any(|literal| parsed.model.authored_sql.contains(literal)); + let modified_controlled_column = + comparison + .modified_columns .iter() - .any(|literal| parsed.model.authored_sql.contains(literal)); - let modified_controlled_column = comparison.modified_columns.iter().any(|column| { - parsed.model.enum_columns.contains(&column.name) - && !column.qualifier.as_ref().is_some_and(|qualifier| { - comparison.source_context.relations.contains(qualifier) - }) - && !(column.qualifier.is_none() && comparison.source_context.sole_relation) - }); - if enum_column - && (bare_string || modified_controlled_column || comparison.modified_string_literal) - { - faults.push(fault(parsed.model, rule, comparison.position.as_ref())); - } + .any(|column: &ColumnFact| { + parsed.model.enum_columns.contains(&column.name) + && !column + .qualifier + .as_ref() + .is_some_and(|qualifier| source_context.relations.contains(qualifier)) + && !(column.qualifier.is_none() && source_context.sole_relation) + }); + if enum_column + && (bare_string || modified_controlled_column || comparison.modified_string_literal) + { + faults.push(fault(parsed.model, rule, position.as_ref())); } - if let Some(rule) = selected.get("SQBRDECLARATION102") { - let magic = comparison.numeric_literals.iter().any(|literal| { - numeric_decisions::is_authored_decision(numeric_decisions::AuthoredDecision { - authored_compact_sql: &parsed.authored_compact_sql, - comparison_sql: &comparison.sql, - equality: comparison.equality, - output_name: comparison.output_name.as_deref(), - literal, - quote_policy: parsed.quote_policy, - }) - }); - if magic { - faults.push(custom_fault!( - parsed.model, - rule, - comparison.position.as_ref(), - format!("non-canonical numeric comparison: {}", comparison.sql), - comparison - .value_list - .then(|| NUMERIC_VALUE_LIST_REMEDIATION.to_owned()), - )); - } + } + if let Some(rule) = selected.get("SQBRDECLARATION102") { + let magic = comparison.numeric_literals.iter().any(|literal| { + numeric_decisions::is_authored_decision(numeric_decisions::AuthoredDecision { + authored_compact_sql: &parsed.authored_compact_sql, + comparison_sql: sql, + equality: comparison.equality, + output_name: comparison.output_name.as_deref(), + literal, + quote_policy: parsed.quote_policy, + }) + }); + if magic { + faults.push(custom_fault!( + parsed.model, + rule, + position.as_ref(), + format!( + "non-canonical numeric comparison: {}", + numeric_decisions::single_spaced(sql, parsed.quote_policy) + ), + comparison + .value_list + .then(|| NUMERIC_VALUE_LIST_REMEDIATION.to_owned()), + )); } } } @@ -1544,427 +1103,76 @@ fn custom_rule_test_coverage( } } -fn all_queries(root: &Query) -> Vec<&Query> { - fn collect_query<'a>(query: &'a Query, values: &mut Vec<&'a Query>) { - values.push(query); - if let Some(with) = &query.with { - for cte in &with.cte_tables { - collect_query(&cte.query, values); - } - } - collect_set_expr(&query.body, values); - } - fn collect_set_expr<'a>(body: &'a SetExpr, values: &mut Vec<&'a Query>) { - match body { - SetExpr::Query(query) => collect_query(query, values), - SetExpr::SetOperation { left, right, .. } => { - collect_set_expr(left, values); - collect_set_expr(right, values); - } - _ => {} - } - } - let mut values: Vec<&Query> = Vec::new(); - collect_query(root, &mut values); - values -} - -fn stars_in_query(query: &Query) -> Vec { - all_queries(query) +fn stars_in_query(query: &QueryFacts) -> Vec> { + query + .all_queries() .into_iter() - .filter_map(root_select) + .filter_map(QueryFacts::root_select) .flat_map(stars_in_select) .collect() } -fn stars_in_select(select: &Select) -> Vec { +fn stars_in_select(select: &SelectFacts) -> Vec> { select - .projection + .items .iter() .filter_map(|item| match item { - SelectItem::Wildcard(options) | SelectItem::QualifiedWildcard(_, options) => { - Some(location_position(options.wildcard_token.0.span.start)) - } - _ => None, - }) - .collect() -} - -fn select_facts(query: &Query) -> Vec { - let source_imports = source_import_names(query); - all_queries(query) - .into_iter() - .filter_map(root_select) - .map(|select| { - let source_context = select_source_context(select, &source_imports); - let mut result = SelectFacts::default(); - for source in &select.from { - for join in &source.joins { - if let Some(JoinConstraint::On(expr)) = join_constraint(&join.join_operator) { - comparison_facts(expr, &source_context, &mut result.comparisons); - } - } - } - for item in &select.projection { - match item { - SelectItem::UnnamedExpr(expression) => { - case_comparison_facts( - expression, - None, - &source_context, - &mut result.comparisons, - ); - } - SelectItem::ExprWithAlias { - expr: expression, - alias, - } => { - case_comparison_facts( - expression, - Some(&alias.value), - &source_context, - &mut result.comparisons, - ); - } - SelectItem::QualifiedWildcard(_, _) | SelectItem::Wildcard(_) => {} - } - } - for expression in [ - select.selection.as_ref(), - select.having.as_ref(), - select.qualify.as_ref(), - ] - .into_iter() - .flatten() - { - comparison_facts(expression, &source_context, &mut result.comparisons); - } - result + ItemFacts::Star { star_start, .. } => Some(*star_start), + ItemFacts::Expression(_) => None, }) .collect() } -fn source_import_names(query: &Query) -> BTreeSet { - top_ctes(query) +fn source_import_names(query: &QueryFacts) -> BTreeSet { + query + .ctes .iter() .filter(|cte| { dependency_import(&cte.query) - && root_select(&cte.query).is_some_and(|select| { - select.from.len() == 1 - && select.from[0].joins.is_empty() - && dependency_name(&select.from[0].relation).as_deref() == Some("__source") + && cte.query.root_select().is_some_and(|select| { + reads_dependency(select) + && dependency_kind(&select.from[0]) == Some(CallKind::Source) }) }) - .map(|cte| cte.alias.name.value.to_ascii_lowercase()) + .map(|cte| cte.name.to_ascii_lowercase()) .collect() } -fn select_source_context(select: &Select, source_imports: &BTreeSet) -> SourceContext { - let mut relations: BTreeSet = BTreeSet::new(); - for source in &select.from { - if let Some(name) = source_relation_name(&source.relation, source_imports) { - relations.insert(name); - } - for join in &source.joins { - if let Some(name) = source_relation_name(&join.relation, source_imports) { - relations.insert(name); - } - } - } +fn select_source_context(select: &SelectFacts, source_imports: &BTreeSet) -> SourceContext { + let relations: BTreeSet = select + .from + .iter() + .chain(&select.joins) + .filter_map(|relation| source_relation_name(relation, source_imports)) + .collect(); let sole_relation = select.from.len() == 1 - && select.from[0].joins.is_empty() - && source_relation_name(&select.from[0].relation, source_imports).is_some(); + && select.joins.is_empty() + && source_relation_name(&select.from[0], source_imports).is_some(); SourceContext { relations, sole_relation, } } -fn source_relation_name(factor: &TableFactor, source_imports: &BTreeSet) -> Option { - let TableFactor::Table { - name, args, alias, .. - } = factor - else { - return None; - }; - let relation_name = name.0.last()?.as_ident()?.value.to_ascii_lowercase(); - if args.is_none() && !source_imports.contains(&relation_name) { - return None; - } - if args.is_some() && dependency_name(factor).as_deref() != Some("__source") { - return None; - } - Some( - alias - .as_ref() - .map_or(relation_name, |value| value.name.value.to_ascii_lowercase()), - ) -} - -fn join_constraint(operator: &JoinOperator) -> Option<&JoinConstraint> { - match operator { - JoinOperator::Join(value) - | JoinOperator::Inner(value) - | JoinOperator::Left(value) - | JoinOperator::LeftOuter(value) - | JoinOperator::Right(value) - | JoinOperator::RightOuter(value) - | JoinOperator::FullOuter(value) - | JoinOperator::Semi(value) - | JoinOperator::LeftSemi(value) - | JoinOperator::RightSemi(value) - | JoinOperator::Anti(value) - | JoinOperator::LeftAnti(value) - | JoinOperator::RightAnti(value) - | JoinOperator::StraightJoin(value) - | JoinOperator::AsOf { - constraint: value, .. - } => Some(value), - _ => None, - } -} - -fn numeric_value(expression: &Expr) -> Option { - match unwrap_nested(expression) { - Expr::Value(value) => match &value.value { - Value::Number(value, _) => Some(value.to_string()), - _ => None, - }, - Expr::UnaryOp { op, expr } if op.to_string() == NEGATION_OPERATOR => { - numeric_value(expr).map(|value| format!("-{value}")) - } - _ => None, - } -} - -fn comparison_facts(root: &Expr, source_context: &SourceContext, output: &mut Vec) { - struct Comparisons<'a> { - source_context: &'a SourceContext, - output: &'a mut Vec, - } - impl Visitor for Comparisons<'_> { - type Break = (); - fn pre_visit_expr(&mut self, expression: &Expr) -> ControlFlow { - let is_comparison = match expression { - Expr::BinaryOp { op, .. } => matches!( - op, - BinaryOperator::Eq - | BinaryOperator::NotEq - | BinaryOperator::Gt - | BinaryOperator::GtEq - | BinaryOperator::Lt - | BinaryOperator::LtEq - ), - Expr::Between { .. } | Expr::InList { .. } => true, - _ => false, - }; - if is_comparison { - self.output - .push(comparison_fact(expression, self.source_context)); - } - ControlFlow::Continue(()) - } - } - let mut visitor = Comparisons { - source_context, - output, - }; - let _ = root.visit(&mut visitor); -} - -fn case_comparison_facts( - root: &Expr, - output_name: Option<&str>, - source_context: &SourceContext, - output: &mut Vec, -) { - struct CaseComparisons<'a> { - source_context: &'a SourceContext, - output: &'a mut Vec, - } - impl Visitor for CaseComparisons<'_> { - type Break = (); - - fn pre_visit_expr(&mut self, expression: &Expr) -> ControlFlow { - if let Expr::Case { - operand, - conditions, - .. - } = expression - { - if let Some(value) = operand { - comparison_facts(value, self.source_context, self.output); - } - for condition in conditions { - comparison_facts(&condition.condition, self.source_context, self.output); - } - } - ControlFlow::Continue(()) - } - } - let mut visitor = CaseComparisons { - source_context, - output, - }; - let start = visitor.output.len(); - let _ = root.visit(&mut visitor); - for fact in &mut visitor.output[start..] { - fact.output_name = output_name.map(str::to_owned); - } -} - -fn comparison_fact(root: &Expr, source_context: &SourceContext) -> ComparisonFact { - struct Values { - fact: ComparisonFact, - } - impl Visitor for Values { - type Break = (); - fn pre_visit_expr(&mut self, expression: &Expr) -> ControlFlow { - match expression { - Expr::Identifier(value) => { - self.fact.columns.insert(value.value.clone()); - } - Expr::CompoundIdentifier(values) => { - if let Some(value) = values.last() { - self.fact.columns.insert(value.value.clone()); - } - } - Expr::Value(value) => match &value.value { - Value::SingleQuotedString(_) - | Value::DoubleQuotedString(_) - | Value::TripleSingleQuotedString(_) - | Value::TripleDoubleQuotedString(_) - | Value::EscapedStringLiteral(_) - | Value::UnicodeStringLiteral(_) - | Value::NationalStringLiteral(_) => { - self.fact.string_literals.push(expression.to_string()); - } - _ => {} - }, - _ => {} - } - ControlFlow::Continue(()) - } - } - let mut visitor = Values { - fact: ComparisonFact { - position: Some(position(root)), - sql: root.to_string(), - equality: matches!( - root, - 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); - 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 { - name: value.value.clone(), - qualifier: None, - }), - Expr::CompoundIdentifier(values) => { - let name = values.last()?.value.clone(); - let qualifier = values - .get(values.len().checked_sub(2)?) - .map(|value| value.value.to_ascii_lowercase()); - Some(ColumnFact { name, qualifier }) - } +fn source_relation_name( + relation: &RelationFacts, + source_imports: &BTreeSet, +) -> Option { + match relation { + RelationFacts::Table { last, alias, .. } if source_imports.contains(last) => Some( + alias + .as_ref() + .map_or_else(|| last.clone(), |value| value.to_ascii_lowercase()), + ), + RelationFacts::Call { + kind: CallKind::Source, + alias, + .. + } => Some( + alias + .as_ref() + .map_or_else(|| "__source".to_owned(), |value| value.to_ascii_lowercase()), + ), _ => None, } } - -fn direct_string_literal(expression: &Expr) -> Option { - let Expr::Value(value) = unwrap_nested(expression) else { - return None; - }; - matches!( - value.value, - Value::SingleQuotedString(_) - | Value::DoubleQuotedString(_) - | Value::TripleSingleQuotedString(_) - | Value::TripleDoubleQuotedString(_) - | Value::EscapedStringLiteral(_) - | Value::UnicodeStringLiteral(_) - | Value::NationalStringLiteral(_) - ) - .then(|| expression.to_string()) -} - -pub(super) fn unwrap_nested(mut expression: &Expr) -> &Expr { - while let Expr::Nested(value) = expression { - expression = value; - } - expression -} - -fn column_facts(root: &Expr) -> BTreeSet { - struct Columns { - values: BTreeSet, - } - impl Visitor for Columns { - type Break = (); - fn pre_visit_expr(&mut self, expression: &Expr) -> ControlFlow { - if let Some(column) = direct_column(expression) { - self.values.insert(column); - } - ControlFlow::Continue(()) - } - } - let mut visitor = Columns { - values: BTreeSet::new(), - }; - let _ = root.visit(&mut visitor); - visitor.values -} - -fn contains_string_literal(root: &Expr) -> bool { - struct Strings { - found: bool, - } - impl Visitor for Strings { - type Break = (); - fn pre_visit_expr(&mut self, expression: &Expr) -> ControlFlow { - if direct_string_literal(expression).is_some() { - self.found = true; - return ControlFlow::Break(()); - } - ControlFlow::Continue(()) - } - } - let mut visitor = Strings { found: false }; - let _ = root.visit(&mut visitor); - visitor.found -} 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 b16fb6e14f..0002d70e6d 100644 --- a/crates/sqlbuild-rules/src/rules/_helpers/explicit_output_types.rs +++ b/crates/sqlbuild-rules/src/rules/_helpers/explicit_output_types.rs @@ -1,10 +1,10 @@ use crate::constants::ENFORCED_CONTRACT; use crate::models::{Column, Fault, Model, RuleMetadata}; -use crate::rules::_helpers::evaluation::{plain_select, root_select, sole_table_name, top_ctes}; -use crate::rules::models::FaultCollector; -use sqlparser::ast::{ - Expr, Query, Select, SelectItem, SetExpr, SetQuantifier, Spanned, TableFactor, +use crate::query_facts::models::{ItemFacts, QueryFacts, SelectFacts, SetFacts}; +use crate::rules::_helpers::evaluation::{ + ParsedModel, plain_select, reads_dependency, sole_table_name, }; +use crate::rules::models::FaultCollector; #[derive(Clone, Copy)] struct OutputScope<'a> { @@ -14,12 +14,15 @@ struct OutputScope<'a> { } struct OutputRule<'a> { + parsed: &'a ParsedModel<'a>, model: &'a Model, rule: &'a RuleMetadata, faults: &'a FaultCollector, } -pub(crate) fn evaluate(query: &Query, model: &Model, rule: &RuleMetadata, faults: &FaultCollector) { +pub(crate) fn evaluate(parsed: &ParsedModel<'_>, rule: &RuleMetadata, faults: &FaultCollector) { + let model = parsed.model; + let query = parsed.facts; if model .config .get("contract") @@ -37,6 +40,7 @@ pub(crate) fn evaluate(query: &Query, model: &Model, rule: &RuleMetadata, faults return; } let evaluator = OutputRule { + parsed, model, rule, faults, @@ -109,26 +113,24 @@ pub(crate) fn evaluate_proven_dynamic(model: &Model, rule: &RuleMetadata, faults } impl OutputRule<'_> { - fn evaluate_set(&self, set_expr: &SetExpr, scope: OutputScope<'_>) { + fn evaluate_set(&self, set_expr: &SetFacts, scope: OutputScope<'_>) { match set_expr { - SetExpr::Select(select) => self.evaluate_select(select, scope), - SetExpr::Query(query) => self.evaluate_set(&query.body, scope), - SetExpr::SetOperation { - set_quantifier, + SetFacts::Select(select) => self.evaluate_select(select, scope), + SetFacts::Query(query) => self.evaluate_set(&query.body, scope), + SetFacts::SetOperation { + by_name, left, right, - .. } => { - let operation_by_name = set_quantifier_by_name(set_quantifier); let branch_scope = OutputScope { columns: scope.columns, set_branch: true, - by_name: operation_by_name, + by_name: *by_name, }; self.evaluate_set(left, branch_scope); self.evaluate_set(right, branch_scope); } - _ => self.push( + SetFacts::Other => self.push( None, format!( "model {:?} does not expose an explicit SELECT output boundary", @@ -140,12 +142,9 @@ impl OutputRule<'_> { } } - fn evaluate_select(&self, select: &Select, scope: OutputScope<'_>) { - for (index, item) in select.projection.iter().enumerate() { - if matches!( - item, - SelectItem::Wildcard(_) | SelectItem::QualifiedWildcard(_, _) - ) { + fn evaluate_select(&self, select: &SelectFacts, scope: OutputScope<'_>) { + for (index, item) in select.items.iter().enumerate() { + let ItemFacts::Expression(expression) = item else { if self.model.dynamic_columns_proven && self .model @@ -158,68 +157,64 @@ impl OutputRule<'_> { if proven_dependency_wildcard(select, scope.columns) { continue; } + let ItemFacts::Star { item_start, .. } = item else { + continue; + }; self.push( - Some(source_position(item)), + *item_start, "wildcard output does not establish enforced contract types explicitly".into(), "Enumerate the contract columns; pass through columns with proven exact types and cast every calculated output." .into(), ); continue; + }; + let output_name = expression.output_name.clone(); + let column = if scope.by_name { + output_name + .as_deref() + .and_then(|name| column_named(scope.columns, name)) + } else { + scope.columns.get(index) + }; + let Some(column) = column.filter(|column| !column.data_type.is_empty()) else { + continue; + }; + let direct = expression.direct_column.is_some(); + let explicit_cast = expression.cast_type.is_some(); + let cast_matches_contract = expression + .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; + if passes { + continue; } - self.evaluate_projection(index, item, scope); - } - } - - fn evaluate_projection(&self, index: usize, item: &SelectItem, scope: OutputScope<'_>) { - let output_name = projection_output_name(item); - let column = if scope.by_name { - output_name - .as_deref() - .and_then(|name| column_named(scope.columns, name)) - } else { - scope.columns.get(index) - }; - let Some(column) = column.filter(|column| !column.data_type.is_empty()) else { - return; - }; - let Some(expr) = projection_expression(item) else { - return; - }; - let direct = direct_column_name(expr).is_some(); - let cast_type = explicit_cast_type(expr); - let explicit_cast = cast_type.is_some(); - let cast_matches_contract = 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; - if passes { - return; + let name = output_name.unwrap_or_else(|| column.name.clone()); + let reason = if explicit_cast && !cast_matches_contract { + "has an outer cast that does not target its declared contract type" + } else if direct && scope.set_branch { + "is in a set-operation branch that must cast each output explicitly" + } else if direct { + "is a passthrough whose type is not proven against the declared contract" + } else { + "is calculated without an outer explicit cast" + }; + self.push( + expression.start, + format!( + "output {:?} {reason}; declared type is {}", + name, column.data_type + ), + format!( + "Wrap the complete expression in CAST(... AS {}) or establish that exact type on a direct upstream column.", + column.data_type + ), + ); } - let name = output_name.unwrap_or_else(|| column.name.clone()); - let reason = if explicit_cast && !cast_matches_contract { - "has an outer cast that does not target its declared contract type" - } else if direct && scope.set_branch { - "is in a set-operation branch that must cast each output explicitly" - } else if direct { - "is a passthrough whose type is not proven against the declared contract" - } else { - "is calculated without an outer explicit cast" - }; - self.push( - Some(source_position(expr)), - format!( - "output {:?} {reason}; declared type is {}", - name, column.data_type - ), - format!( - "Wrap the complete expression in CAST(... AS {}) or establish that exact type on a direct upstream column.", - column.data_type - ), - ); } - fn push(&self, position: Option<(u64, u64)>, message: String, remediation: String) { - let (line, column) = position.unwrap_or((1, 1)); + 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 { unevaluated: false, code: self.rule.code.clone(), @@ -232,23 +227,9 @@ impl OutputRule<'_> { } } -fn proven_dependency_wildcard(select: &Select, columns: &[Column]) -> bool { - let Some(source) = select.from.first().filter(|_| select.from.len() == 1) else { - return false; - }; - if !plain_select(select, true) || !source.joins.is_empty() { - return false; - } - let TableFactor::Table { - name, - args: Some(_), - .. - } = &source.relation - else { - return false; - }; - let dependency = name.to_string().to_ascii_lowercase(); - matches!(dependency.as_str(), "__ref" | "__source") +fn proven_dependency_wildcard(select: &SelectFacts, columns: &[Column]) -> bool { + plain_select(select, true) + && reads_dependency(select) && columns .iter() .filter(|column| !column.data_type.is_empty()) @@ -256,65 +237,56 @@ fn proven_dependency_wildcard(select: &Select, columns: &[Column]) -> bool { } fn final_cte_output_boundary<'a>( - query: &'a Query, + query: &'a QueryFacts, columns: &[Column], -) -> Option<(&'a Query, Vec)> { - let final_cte = top_ctes(query).last()?; - let final_name = &final_cte.alias.name.value; +) -> Option<(&'a QueryFacts, Vec)> { + let final_cte = query.ctes.last()?; let terminal_reads_final_cte = sole_table_name(query) - .is_some_and(|name| name.eq_ignore_ascii_case(final_name)) - && root_select(query).is_some_and(|select| plain_select(select, true)) - && query.order_by.is_none() - && query.limit_clause.is_none() - && query.fetch.is_none() - && query.locks.is_empty(); + .is_some_and(|name| name.eq_ignore_ascii_case(&final_cte.name)) + && query + .root_select() + .is_some_and(|select| plain_select(select, true)) + && !query.has_order_by + && !query.has_limit + && !query.has_fetch + && !query.has_locks; if !terminal_reads_final_cte { return None; } - let select = root_select(query)?; - let cte_column_aliases: Vec = final_cte - .alias - .columns - .iter() - .map(|column| column.name.value.clone()) - .collect(); - if select.projection.len() == 1 - && matches!( - select.projection[0], - SelectItem::Wildcard(_) | SelectItem::QualifiedWildcard(_, _) - ) - { + let select = query.root_select()?; + if select.items.len() == 1 && matches!(select.items[0], ItemFacts::Star { .. }) { return Some(( &final_cte.query, columns_in_output_order_with_missing( &final_cte.query.body, columns, - &cte_column_aliases, + &final_cte.columns, ), )); } - if select.projection.len() != columns.len() { + if select.items.len() != columns.len() { return None; } let mut boundary_columns: Vec = Vec::with_capacity(columns.len()); - for item in &select.projection { - let output_name = projection_output_name(item)?; - let column = column_named(columns, &output_name)?; - let source_name = projection_expression(item).and_then(direct_column_name)?; + for item in &select.items { + let ItemFacts::Expression(expression) = item else { + return None; + }; + let column = column_named(columns, expression.output_name.as_deref()?)?; let mut boundary_column = column.clone(); - boundary_column.name = source_name; + boundary_column.name = expression.direct_column.clone()?; boundary_columns.push(boundary_column); } let ordered_columns = columns_in_output_order_with_missing( &final_cte.query.body, &boundary_columns, - &cte_column_aliases, + &final_cte.columns, ); Some((&final_cte.query, ordered_columns)) } fn columns_in_output_order_with_missing( - set_expr: &SetExpr, + set_expr: &SetFacts, columns: &[Column], cte_column_aliases: &[String], ) -> Vec { @@ -344,7 +316,7 @@ fn columns_in_output_order_with_missing( .collect() } -fn columns_in_output_order(set_expr: &SetExpr, columns: &[Column]) -> Vec { +fn columns_in_output_order(set_expr: &SetFacts, columns: &[Column]) -> Vec { let Some(output_names) = set_expression_output_names(set_expr) else { return columns.to_vec(); }; @@ -364,39 +336,33 @@ fn column_named<'a>(columns: &'a [Column], name: &str) -> Option<&'a Column> { .find(|column| column.name.eq_ignore_ascii_case(name)) } -fn set_expression_output_names(set_expr: &SetExpr) -> Option> { +fn projection_output_name(item: &ItemFacts) -> Option { + match item { + ItemFacts::Expression(expression) => expression.output_name.clone(), + ItemFacts::Star { .. } => None, + } +} + +fn set_expression_output_names(set_expr: &SetFacts) -> Option> { match set_expr { - SetExpr::Select(select) => { - let mut names: Vec = Vec::with_capacity(select.projection.len()); - for item in &select.projection { - names.push(projection_output_name(item)?); - } - Some(names) - } - SetExpr::Query(query) => set_expression_output_names(&query.body), - SetExpr::SetOperation { left, .. } => set_expression_output_names(left), - _ => None, + SetFacts::Select(select) => select.items.iter().map(projection_output_name).collect(), + SetFacts::Query(query) => set_expression_output_names(&query.body), + SetFacts::SetOperation { left, .. } => set_expression_output_names(left), + SetFacts::Other => None, } } -fn set_expression_output_names_with_missing(set_expr: &SetExpr) -> Option>> { +fn set_expression_output_names_with_missing(set_expr: &SetFacts) -> Option>> { match set_expr { - SetExpr::Select(select) => Some( - select - .projection - .iter() - .map(projection_output_name) - .collect(), - ), - SetExpr::Query(query) => set_expression_output_names_with_missing(&query.body), - SetExpr::SetOperation { + SetFacts::Select(select) => Some(select.items.iter().map(projection_output_name).collect()), + SetFacts::Query(query) => set_expression_output_names_with_missing(&query.body), + SetFacts::SetOperation { + by_name, left, right, - set_quantifier, - .. } => { let mut names = set_expression_output_names_with_missing(left)?; - if set_quantifier_by_name(set_quantifier) { + if *by_name { for right_name in set_expression_output_names_with_missing(right)? { let already_present = right_name.as_ref().is_some_and(|right_name| { names @@ -411,52 +377,10 @@ fn set_expression_output_names_with_missing(set_expr: &SetExpr) -> Option None, + SetFacts::Other => None, } } -fn set_quantifier_by_name(set_quantifier: &SetQuantifier) -> bool { - matches!( - set_quantifier, - SetQuantifier::ByName | SetQuantifier::AllByName | SetQuantifier::DistinctByName - ) -} - -fn projection_output_name(item: &SelectItem) -> Option { - match item { - SelectItem::ExprWithAlias { alias, .. } => Some(alias.value.clone()), - SelectItem::UnnamedExpr(Expr::Identifier(identifier)) => Some(identifier.value.clone()), - SelectItem::UnnamedExpr(Expr::CompoundIdentifier(identifiers)) => identifiers - .last() - .map(|identifier| identifier.value.clone()), - _ => None, - } -} - -fn projection_expression(item: &SelectItem) -> Option<&Expr> { - match item { - SelectItem::UnnamedExpr(expr) | SelectItem::ExprWithAlias { expr, .. } => Some(expr), - _ => None, - } -} - -fn direct_column_name(expr: &Expr) -> Option { - match unwrap_nested_expression(expr) { - Expr::Identifier(identifier) => Some(identifier.value.clone()), - Expr::CompoundIdentifier(identifiers) => identifiers - .last() - .map(|identifier| identifier.value.clone()), - _ => None, - } -} - -fn explicit_cast_type(expr: &Expr) -> Option { - let Expr::Cast { data_type, .. } = unwrap_nested_expression(expr) else { - return None; - }; - Some(data_type.to_string()) -} - fn type_spelling_equal(left: &str, right: &str) -> bool { let normalize = |value: &str| { value @@ -467,15 +391,3 @@ fn type_spelling_equal(left: &str, right: &str) -> bool { }; normalize(left) == normalize(right) } - -fn unwrap_nested_expression(mut expr: &Expr) -> &Expr { - while let Expr::Nested(inner) = expr { - expr = inner; - } - expr -} - -fn source_position(node: &T) -> (u64, u64) { - let start = node.span().start; - (start.line, start.column) -} diff --git a/crates/sqlbuild-rules/src/rules/_helpers/numeric_decisions.rs b/crates/sqlbuild-rules/src/rules/_helpers/numeric_decisions.rs index 8f0522b65d..1ad8e755cc 100644 --- a/crates/sqlbuild-rules/src/rules/_helpers/numeric_decisions.rs +++ b/crates/sqlbuild-rules/src/rules/_helpers/numeric_decisions.rs @@ -46,6 +46,32 @@ pub(crate) fn compact_sql(sql: &str, policy: QuotePolicy) -> String { String::from_utf8(compact).unwrap_or_default() } +/// `sql` with each whitespace run outside quoted text collapsed to one space. +pub(crate) fn single_spaced(sql: &str, policy: QuotePolicy) -> String { + let bytes = sql.as_bytes(); + let mut spaced = Vec::with_capacity(bytes.len()); + let mut index = 0; + while index < bytes.len() { + let byte = bytes[index]; + if policy.is_quote(byte) { + let end = quote_end(bytes, index, policy).unwrap_or(bytes.len()); + spaced.extend_from_slice(&bytes[index..end]); + index = end; + } else if byte.is_ascii_whitespace() { + while bytes.get(index).is_some_and(u8::is_ascii_whitespace) { + index += 1; + } + spaced.push(b' '); + } else { + spaced.push(byte); + index += 1; + } + } + String::from_utf8(spaced) + .map(|value| value.trim().to_owned()) + .unwrap_or_else(|_| sql.to_owned()) +} + pub(crate) fn is_authored_decision(decision: AuthoredDecision<'_>) -> bool { !CANONICAL_NUMERIC_DECISIONS.contains(&decision.literal) && decision diff --git a/crates/sqlbuild-rules/src/rules/_helpers/sql_test_rules.rs b/crates/sqlbuild-rules/src/rules/_helpers/sql_test_rules.rs index 6b5419602d..678022978a 100644 --- a/crates/sqlbuild-rules/src/rules/_helpers/sql_test_rules.rs +++ b/crates/sqlbuild-rules/src/rules/_helpers/sql_test_rules.rs @@ -2,18 +2,16 @@ use crate::models::{ DeclarationKind, EvaluateRequest, Fault, Model, ResourceKind, RuleMetadata, RulesConfig, ScopeResource, SqlScenarioFact, SqlTestCteFact, SqlTestFact, SqlTestMode, }; -use crate::rules::_helpers::evaluation::{ - dependency_name, group_by_empty, normalize_rules_sql, parse_rule_statements, path_fault, - unwrap_nested, -}; +use crate::query_facts::main::model_shape::model_shape; +use crate::query_facts::main::parse_fixture::parse_fixture; +use crate::query_facts::main::stub_call::stub_call; +use crate::query_facts::models::{CallKind, ParsedFixture}; +use crate::rules::_helpers::evaluation::path_fault; use crate::rules::models::ProjectEvaluationRequest; +use polyglot_sql::Expression; +use polyglot_sql::expressions::{Literal, Select}; use sqlbuild_analysis::compiler::main::sql_test_fixture_facts::fixture_facts; -use sqlparser::ast::{ - BinaryOperator, Expr, FunctionArg, FunctionArgExpr, LimitClause, Query, Select, SetExpr, - Statement, TableFactor, Value, Visit, Visitor, -}; use std::collections::{BTreeMap, HashMap}; -use std::ops::ControlFlow; use std::path::Path; const UNIT_ROOT: &str = "tests/unit"; @@ -692,64 +690,62 @@ fn empty_relation(cte: &SqlTestCteFact, dialect: &str) -> bool { if fixture_facts(&cte.name, &cte.sql).empty_fixture_marker { return true; } - let Ok(query) = fixture_query(&cte.sql, dialect) else { + let Ok(fixture) = fixture_query(&cte.sql, dialect) else { return false; }; - let SetExpr::Select(select) = query.body.as_ref() else { + let Expression::Select(select) = &fixture.statement else { return false; }; - query.with.is_none() - && (limited_to_zero_rows(&query) - || (filtered_to_zero_rows(select) && !contains_function(&query.order_by))) + select.with.is_none() + && (limited_to_zero_rows(select) + || (filtered_to_zero_rows(select, &fixture) && !fixture.order_by_calls_function())) } -fn fixture_query(sql: &str, dialect: &str) -> Result { - let mut statements = parse_rule_statements(&normalize_rules_sql(dialect, sql), dialect) - .map_err(|error| format!("could not parse SQL-test fixture: {error}"))?; - match (statements.pop(), statements.is_empty()) { - (Some(Statement::Query(query)), true) => Ok(*query), - _ => Err("SQL-test fixture must contain exactly one query".to_owned()), - } +fn fixture_query(sql: &str, dialect: &str) -> Result { + model_shape(sql, dialect) + .and_then(|shape| parse_fixture(shape, dialect)) + .map_err(|error| format!("could not parse SQL-test fixture: {error}")) } -fn limited_to_zero_rows(query: &Query) -> bool { - let limit = match &query.limit_clause { - Some(LimitClause::LimitOffset { - limit: Some(limit), .. - }) => limit, - Some(LimitClause::OffsetCommaLimit { limit, .. }) => limit, - _ => return false, - }; - numeric_literal(limit) == Some(0.0) +fn limited_to_zero_rows(select: &Select) -> bool { + select + .limit + .as_ref() + .is_some_and(|limit| numeric_literal(&limit.this) == Some(0.0)) } -fn filtered_to_zero_rows(select: &Select) -> bool { - let global_aggregate_possible = contains_function(select); +fn filtered_to_zero_rows(select: &Select, fixture: &ParsedFixture) -> bool { select.having.is_none() - && group_by_empty(&select.group_by) - && !global_aggregate_possible - && select.selection.as_ref().is_some_and(constant_false) -} - -fn constant_false(predicate: &Expr) -> bool { - match unwrap_nested(predicate) { - Expr::Value(value) => matches!(value.value, Value::Boolean(false)), - Expr::BinaryOp { - left, - op: BinaryOperator::Eq, - right, - } => numeric_literal(left) - .zip(numeric_literal(right)) + && select.group_by.is_none() + && !fixture.select_calls_function() + && select + .where_clause + .as_ref() + .is_some_and(|clause| constant_false(&clause.this)) +} + +fn unwrap_paren(mut expression: &Expression) -> &Expression { + while let Expression::Paren(paren) = expression { + expression = &paren.this; + } + expression +} + +fn constant_false(predicate: &Expression) -> bool { + match unwrap_paren(predicate) { + Expression::Boolean(value) => !value.value, + Expression::Eq(comparison) => numeric_literal(&comparison.left) + .zip(numeric_literal(&comparison.right)) .is_some_and(|(left, right)| left != right), _ => false, } } -fn numeric_literal(expression: &Expr) -> Option { - let Expr::Value(value) = unwrap_nested(expression) else { +fn numeric_literal(expression: &Expression) -> Option { + let Expression::Literal(literal) = unwrap_paren(expression) else { return None; }; - let Value::Number(number, _) = &value.value else { + let Literal::Number(number) = literal.as_ref() else { return None; }; let Ok(parsed) = number.parse::() else { @@ -759,91 +755,47 @@ fn numeric_literal(expression: &Expr) -> Option { } fn bare_row_existence(sql: &str, targets: &[String], dialect: &str) -> bool { - let Ok(query) = fixture_query(sql, dialect) else { + let Ok(fixture) = fixture_query(sql, dialect) else { return false; }; - let SetExpr::Select(select) = query.body.as_ref() else { + let Expression::Select(select) = &fixture.statement else { return false; }; - let [source] = select.from.as_slice() else { + let [source] = select + .from + .as_ref() + .map_or(&[][..], |from| from.expressions.as_slice()) + else { return false; }; - let unfiltered = select.selection.is_none() + let unfiltered = select.where_clause.is_none() && select.prewhere.is_none() && select.having.is_none() && select.qualify.is_none() - && select.connect_by.is_none() + && select.connect.is_none() && select.lateral_views.is_empty() - && group_by_empty(&select.group_by); - query.with.is_none() + && select.group_by.is_none(); + select.with.is_none() && unfiltered - && source.joins.is_empty() - && reference_target(&source.relation).is_some_and(|target| targets.contains(&target)) - && !contains_function(&select.projection) - && !contains_function(&query.order_by) - && query.limit_clause.is_none() - && query.fetch.is_none() - && query_count(&query) == 1 -} - -fn reference_target(factor: &TableFactor) -> Option { - if dependency_name(factor).as_deref() != Some("__ref") { - return None; - } - let TableFactor::Table { - args: Some(arguments), - .. - } = factor - else { + && select.joins.is_empty() + && reference_target(source, &fixture).is_some_and(|target| targets.contains(&target)) + && !fixture.projection_calls_function() + && !fixture.order_by_calls_function() + && select.limit.is_none() + && select.offset.is_none() + && select.fetch.is_none() + && fixture.query_count() == 1 +} + +fn reference_target(source: &Expression, fixture: &ParsedFixture) -> Option { + let Expression::Table(table) = source else { return None; }; - let [FunctionArg::Unnamed(FunctionArgExpr::Expr(argument))] = arguments.args.as_slice() else { + if table.schema.is_some() || table.catalog.is_some() { return None; - }; - match unwrap_nested(argument) { - Expr::Identifier(identifier) => Some(identifier.value.clone()), - Expr::Value(value) => match &value.value { - Value::SingleQuotedString(name) | Value::DoubleQuotedString(name) => Some(name.clone()), - _ => None, - }, - _ => None, } -} - -fn contains_function(node: &T) -> bool { - struct Functions; - - impl Visitor for Functions { - type Break = (); - - fn pre_visit_expr(&mut self, expression: &Expr) -> ControlFlow { - if matches!(expression, Expr::Function(_)) { - ControlFlow::Break(()) - } else { - ControlFlow::Continue(()) - } - } - } - - node.visit(&mut Functions).is_break() -} - -fn query_count(query: &Query) -> usize { - #[derive(Default)] - struct Queries { - count: usize, - } - - impl Visitor for Queries { - type Break = (); - - fn pre_visit_query(&mut self, _query: &Query) -> ControlFlow { - self.count += 1; - ControlFlow::Continue(()) - } - } - - let mut queries = Queries::default(); - let _ = query.visit(&mut queries); - queries.count + let (CallKind::Ref, index) = stub_call(&table.name.name)? else { + return None; + }; + fixture.calls.get(index)?.target.clone() } diff --git a/crates/sqlbuild-rules/src/rules/main/compact_sql.rs b/crates/sqlbuild-rules/src/rules/main/compact_sql.rs new file mode 100644 index 0000000000..a1fa2a7c57 --- /dev/null +++ b/crates/sqlbuild-rules/src/rules/main/compact_sql.rs @@ -0,0 +1,6 @@ +use sqlbuild_sqltext::sql_scan::models::QuotePolicy; + +/// `sql` lowercased without whitespace or comments outside quoted text. +pub(crate) fn compact_sql(sql: &str, policy: QuotePolicy) -> String { + crate::rules::_helpers::numeric_decisions::compact_sql(sql, policy) +} diff --git a/crates/sqlbuild-rules/src/rules/main/mod.rs b/crates/sqlbuild-rules/src/rules/main/mod.rs index a877872374..b90109699d 100644 --- a/crates/sqlbuild-rules/src/rules/main/mod.rs +++ b/crates/sqlbuild-rules/src/rules/main/mod.rs @@ -1,6 +1,7 @@ pub mod assemble_catalogue; pub mod builtin_options; pub mod catalogue; +pub mod compact_sql; pub mod evaluate; pub mod evaluate_project; pub mod fingerprint; diff --git a/crates/sqlbuild-rules/src/rules/mod.rs b/crates/sqlbuild-rules/src/rules/mod.rs index f5f7fd5d12..42ff22a67f 100644 --- a/crates/sqlbuild-rules/src/rules/mod.rs +++ b/crates/sqlbuild-rules/src/rules/mod.rs @@ -1,3 +1,4 @@ pub mod _helpers; pub mod main; pub mod models; +pub mod types; diff --git a/crates/sqlbuild-rules/src/rules/models.rs b/crates/sqlbuild-rules/src/rules/models.rs index 0d9c6ec2a9..0158b2f7e5 100644 --- a/crates/sqlbuild-rules/src/rules/models.rs +++ b/crates/sqlbuild-rules/src/rules/models.rs @@ -1,9 +1,29 @@ use std::cell::RefCell; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashMap}; +use std::sync::{Arc, Mutex, PoisonError}; use crate::models::{EvaluateRequest, Fault, Model, RuleMetadata, RulesConfig}; +use crate::rules::types::ShapeFacts; use globset::GlobSet; +/// The query shapes one rules evaluation has parsed, keyed by dialect and byte-identical SQL. +#[derive(Debug, Default)] +pub(crate) struct QueryShapes { + entries: Mutex>, +} + +impl QueryShapes { + pub(crate) fn entry(&self, dialect: &str, sql: &str) -> ShapeFacts { + let mut entries = self.entries.lock().unwrap_or_else(PoisonError::into_inner); + if let Some(entry) = entries.get(&(dialect.to_owned(), sql.to_owned())) { + return Arc::clone(entry); + } + let entry: ShapeFacts = Arc::default(); + entries.insert((dialect.to_owned(), sql.to_owned()), Arc::clone(&entry)); + entry + } +} + #[derive(Debug, Default)] pub(crate) struct FaultCollector { faults: RefCell>, @@ -16,6 +36,7 @@ pub(crate) struct ModelEvaluationRequest<'a> { pub(crate) selected: &'a BTreeMap, pub(crate) request: &'a EvaluateRequest, pub(crate) threshold_overrides: &'a [ResolvedThresholdOverride], + pub(crate) shapes: &'a QueryShapes, } #[derive(Debug)] diff --git a/crates/sqlbuild-rules/src/rules/types.rs b/crates/sqlbuild-rules/src/rules/types.rs new file mode 100644 index 0000000000..7b30b8ab47 --- /dev/null +++ b/crates/sqlbuild-rules/src/rules/types.rs @@ -0,0 +1,5 @@ +use crate::query_facts::models::QueryFacts; +use std::sync::{Arc, OnceLock}; + +/// One stubbed query shape's parsed facts, computed once by whichever worker reaches it first. +pub(crate) type ShapeFacts = Arc>>; diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py index 5256b17d0d..40e2f72c12 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py @@ -12,6 +12,16 @@ class EvaluatedPredicateCase: expected_evaluated_models: int = 1 +@dataclass(frozen=True) +class EvaluatedModelRulesCase: + description: str + sql: str + rules: tuple[str, ...] + selector: str + expected_rule_findings: tuple[tuple[str, int, int], ...] + expected_compile_findings: tuple[tuple[str, int, int], ...] + + @dataclass(frozen=True) class GroupedRulesCase: description: str diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_unevaluated.py b/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_unevaluated.py index f00ba2ddad..13c28705b1 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_unevaluated.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_unevaluated.py @@ -8,6 +8,7 @@ import pytest from tests.e2e.src.sqlbuild.cli.commands.main.rules._test_types import ( + EvaluatedModelRulesCase, EvaluatedPredicateCase, GroupedRulesCase, UnevaluatedResourceCase, @@ -42,11 +43,9 @@ ), ), UnevaluatedResourceCase( - "model the compiler accepts but the Rules parser rejects", + "built-in model rule beyond the parser budget", "models/staging/orders.sql", - "MODEL (description 'Test model orders.');\nWITH items AS (SELECT [1, 2] AS quantities)\n" - "SELECT 1 AS order_id, list_transform(i.quantities, lambda q: q + 1) AS shipped\n" - "FROM items AS i", + "MODEL (description 'Test model orders.');\nSELECT {expression} AS order_id", expected_evaluated_models=0, expected_reason="could not parse models/staging/orders.sql for rules", rule="SQBRMODEL101", @@ -104,6 +103,55 @@ def test_given_guarded_resource_when_running_rules_then_reports_failure_and_cove ) +@pytest.mark.parametrize( + "test_case", + [ + EvaluatedModelRulesCase( + "lambda model the compiler accepts gets built-in model findings", + "WITH items AS (SELECT [1, 2] AS quantities)\n" + "SELECT *, list_transform(i.quantities, lambda q: q + 1) AS shipped\n" + "FROM items AS i WHERE i.quantities[1] > 7", + ("SQBRMODEL101", "SQBRMODEL102", "SQBRDECLARATION102"), + "SQBRMODEL102", + (("SQBRMODEL102", 2, 8),), + (("SQBRMODEL102", 2, 8), ("SQBRDECLARATION102", 3, 23)), + ), + ], + ids=lambda case: case.description, +) +def test_given_model_only_polyglot_parses_when_running_rules_then_reports_model_findings( + test_case: EvaluatedModelRulesCase, + tmp_path: Path, +) -> None: + write_unevaluated_rules_project( + project_dir=tmp_path, + resource_path="models/staging/orders.sql", + resource_template="MODEL (description 'Test model orders.');\n" + test_case.sql, + selected_rules=test_case.rules, + ) + for _ in range(2): + result: subprocess.CompletedProcess[str] = run_unevaluated_rules_cli( + tmp_path, "rules", "--json", "run", test_case.selector + ) + assert result.returncode == 1, result.stdout + result.stderr + payload: dict[str, Any] = json.loads(result.stdout) + assert payload["unevaluated_resources"] == 0 + assert payload["evaluated_models"] == 1 + findings: list[tuple[str, int, int]] = [ + (finding["code"], finding["line"], finding["column"]) for finding in payload["findings"] + ] + assert findings == list(test_case.expected_rule_findings) + compiled: subprocess.CompletedProcess[str] = run_unevaluated_rules_cli( + tmp_path, "compile", "--no-cache", "--json" + ) + assert compiled.returncode == 1, compiled.stdout + compiled.stderr + diagnostics: list[tuple[str, int, int]] = [ + (item["code"], item["line"], item["column"]) + for item in json.loads(compiled.stdout)["diagnostics"] + ] + assert diagnostics == list(test_case.expected_compile_findings) + + @pytest.mark.parametrize( "test_case", [ From ab970e946589b7839cc04020b81909e1cc010b31 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 11:02:35 +0100 Subject: [PATCH 3/8] build: drop sqlparser from the workspace The rules quote policy is a static dialect table, so no crate needs sqlparser any more. --- Cargo.lock | 44 ------------------- Cargo.toml | 1 - crates/sqlbuild-sqltext/Cargo.toml | 1 - .../src/sql_scan/_helpers/rules_dialect.rs | 32 +++++--------- .../sqlbuild-sqltext/src/sql_scan/main/mod.rs | 1 - .../src/sql_scan/main/rules_dialect.rs | 6 --- 6 files changed, 10 insertions(+), 75 deletions(-) delete mode 100644 crates/sqlbuild-sqltext/src/sql_scan/main/rules_dialect.rs diff --git a/Cargo.lock b/Cargo.lock index 12d90f2595..6ea49a5baa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -878,26 +878,6 @@ dependencies = [ "crossbeam-utils", ] -[[package]] -name = "recursive" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0786a43debb760f491b1bc0269fe5e84155353c67482b9e60d0cfb596054b43e" -dependencies = [ - "recursive-proc-macro-impl", - "stacker", -] - -[[package]] -name = "recursive-proc-macro-impl" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "76009fbe0614077fc1a2ce255e3a1881a2e3a3527097d5dc6d8212c585e7e38b" -dependencies = [ - "quote", - "syn 2.0.119", -] - [[package]] name = "regex" version = "1.13.1" @@ -1327,7 +1307,6 @@ dependencies = [ "sqlbuild-core", "sqlbuild-scopes", "sqlbuild-sqltext", - "sqlparser", "stacker", "tempfile", "toml", @@ -1353,7 +1332,6 @@ dependencies = [ "rayon", "serde", "sqlbuild-core", - "sqlparser", ] [[package]] @@ -1366,28 +1344,6 @@ dependencies = [ "sqlbuild-cursor", ] -[[package]] -name = "sqlparser" -version = "0.58.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ec4b661c54b1e4b603b37873a18c59920e4c51ea8ea2cf527d925424dbd4437c" -dependencies = [ - "log", - "recursive", - "sqlparser_derive", -] - -[[package]] -name = "sqlparser_derive" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da5fc6819faabb412da764b99d3b713bb55083c11e7e0c00144d386cd6a1939c" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.119", -] - [[package]] name = "stacker" version = "0.1.25" diff --git a/Cargo.toml b/Cargo.toml index 5b26a56e45..d929f30e63 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -110,7 +110,6 @@ sqlbuild-scopes = { path = "crates/sqlbuild-scopes" } sqlbuild-sqltext = { path = "crates/sqlbuild-sqltext" } sqlbuild-state = { path = "crates/sqlbuild-state" } stacker = "0.1" -sqlparser = { version = "0.58", features = ["visitor"] } syn = { version = "2", features = ["full", "visit", "extra-traits"] } tempfile = "3" toml = "0.9" diff --git a/crates/sqlbuild-sqltext/Cargo.toml b/crates/sqlbuild-sqltext/Cargo.toml index 09864fdb60..32220c590a 100644 --- a/crates/sqlbuild-sqltext/Cargo.toml +++ b/crates/sqlbuild-sqltext/Cargo.toml @@ -11,7 +11,6 @@ rust-version.workspace = true rayon.workspace = true serde.workspace = true sqlbuild-core.workspace = true -sqlparser.workspace = true [lints] workspace = true diff --git a/crates/sqlbuild-sqltext/src/sql_scan/_helpers/rules_dialect.rs b/crates/sqlbuild-sqltext/src/sql_scan/_helpers/rules_dialect.rs index e98fe2284d..ca3716a684 100644 --- a/crates/sqlbuild-sqltext/src/sql_scan/_helpers/rules_dialect.rs +++ b/crates/sqlbuild-sqltext/src/sql_scan/_helpers/rules_dialect.rs @@ -1,32 +1,20 @@ //! Lexical quoting facts of the SQL dialects the rules engine scans. use crate::sql_scan::models::QuotePolicy; -use sqlparser::dialect::{ - BigQueryDialect, ClickHouseDialect, DatabricksDialect, Dialect, DuckDbDialect, GenericDialect, - MsSqlDialect, PostgreSqlDialect, SnowflakeDialect, -}; pub(crate) fn rules_quote_policy(dialect_name: &str) -> QuotePolicy { - let dialect = rules_dialect(dialect_name); - let backslash_escapes = dialect.supports_string_literal_backslash_escape(); + let (backtick_identifiers, backslash_escapes, double_quote_identifiers) = + match dialect_name.to_ascii_lowercase().as_str() { + "bigquery" => (true, true, false), + "clickhouse" | "snowflake" => (true, true, true), + "postgres" | "postgresql" | "mssql" | "sqlserver" | "tsql" => (false, false, true), + "databricks" => (true, false, false), + _ => (true, false, true), + }; QuotePolicy { - backtick_identifiers: dialect.is_delimited_identifier_start('`'), + backtick_identifiers, single_quote_backslash_escapes: backslash_escapes, - double_quote_backslash_escapes: backslash_escapes - && !dialect.is_delimited_identifier_start('"'), + double_quote_backslash_escapes: backslash_escapes && !double_quote_identifiers, dollar_quotes: false, } } - -pub(crate) fn rules_dialect(name: &str) -> Box { - match name.to_ascii_lowercase().as_str() { - "bigquery" => Box::new(BigQueryDialect {}), - "clickhouse" => Box::new(ClickHouseDialect {}), - "databricks" => Box::new(DatabricksDialect {}), - "duckdb" => Box::new(DuckDbDialect {}), - "postgres" | "postgresql" => Box::new(PostgreSqlDialect {}), - "mssql" | "sqlserver" | "tsql" => Box::new(MsSqlDialect {}), - "snowflake" => Box::new(SnowflakeDialect {}), - _ => Box::new(GenericDialect {}), - } -} diff --git a/crates/sqlbuild-sqltext/src/sql_scan/main/mod.rs b/crates/sqlbuild-sqltext/src/sql_scan/main/mod.rs index 69cddeb649..4735e4b172 100644 --- a/crates/sqlbuild-sqltext/src/sql_scan/main/mod.rs +++ b/crates/sqlbuild-sqltext/src/sql_scan/main/mod.rs @@ -8,6 +8,5 @@ pub mod non_code_end; pub mod quote_end; pub mod quote_policy; pub mod reads_differently_from_generic; -pub mod rules_dialect; pub mod skip_whitespace; pub mod unsupported_line_comment_prefixes; diff --git a/crates/sqlbuild-sqltext/src/sql_scan/main/rules_dialect.rs b/crates/sqlbuild-sqltext/src/sql_scan/main/rules_dialect.rs deleted file mode 100644 index 402b035dda..0000000000 --- a/crates/sqlbuild-sqltext/src/sql_scan/main/rules_dialect.rs +++ /dev/null @@ -1,6 +0,0 @@ -use sqlparser::dialect::Dialect; - -/// Return the parser dialect rules use for `name`, falling back to the generic dialect. -pub fn rules_dialect(name: &str) -> Box { - crate::sql_scan::_helpers::rules_dialect::rules_dialect(name) -} From e04c847f55d3b1c9c9961e7fc61454174af2a8c6 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 12:14:05 +0100 Subject: [PATCH 4/8] fix(rules): honour dialect comment syntax when stubbing query shapes The stub scanner only knew -- and /* */ comments, so a Snowflake // or a Snowflake/BigQuery # comment could hold a phantom __ref call, or an apostrophe in it could open a fake string that swallowed real calls. Scan with the comment and quoting syntax the rules parser used per dialect, and blank comments before normalization and parsing. --- .../src/query_facts/_helpers/shape.rs | 120 ++++++++++++------ 1 file changed, 78 insertions(+), 42 deletions(-) diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/shape.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/shape.rs index f11a64ac32..066543e71f 100644 --- a/crates/sqlbuild-rules/src/query_facts/_helpers/shape.rs +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/shape.rs @@ -5,22 +5,17 @@ use crate::query_facts::constants::{ use crate::query_facts::models::{CallKind, ModelCall, ModelShape}; use crate::rules::main::compact_sql::compact_sql; use sqlbuild_analysis::semantic_validation::main::normalize_mapped::normalize_mapped; -use sqlbuild_sqltext::sql_scan::main::comment_end::comment_end; -use sqlbuild_sqltext::sql_scan::main::matching_paren::matching_paren; -use sqlbuild_sqltext::sql_scan::main::non_code_end::non_code_end; +use sqlbuild_sqltext::sql_scan::main::dialect_matching_paren::dialect_matching_paren; +use sqlbuild_sqltext::sql_scan::main::dialect_non_code_end::dialect_non_code_end; use sqlbuild_sqltext::sql_scan::main::quote_policy::quote_policy; -use sqlbuild_sqltext::sql_scan::models::QuotePolicy; +use sqlbuild_sqltext::sql_scan::models::{LexicalSyntax, QuotePolicy}; pub(crate) fn model_shape(sql: &str, dialect: &str) -> Result { let policy = QuotePolicy { dollar_quotes: true, ..quote_policy(dialect) }; - let nested = matches!( - dialect.to_ascii_lowercase().as_str(), - "generic" | "postgres" | "postgresql" | "mssql" | "sqlserver" | "tsql" - ); - let (stubbed, stub_offsets, calls) = stub_calls(sql, policy, nested); + let (stubbed, stub_offsets, calls) = stub_calls(sql, policy, &lexical_syntax(dialect)); let normalized = normalize_mapped(&stubbed, dialect)?; let end = stub_offsets.last().copied().unwrap_or_default(); let offsets: Vec = normalized @@ -82,41 +77,71 @@ fn is_word_byte(byte: u8) -> bool { byte.is_ascii_alphanumeric() || byte == b'_' || byte == b'$' } -fn nested_comment_end(bytes: &[u8], index: usize) -> Option { - if !bytes[index..].starts_with(b"/*") { - return None; +/// The comment and quoted-text syntax the rules parser tokenized for `dialect`. +fn lexical_syntax(dialect: &str) -> LexicalSyntax { + let strings = |quotes: &[&str]| quotes.iter().map(|quote| (*quote).to_owned()).collect(); + let lines = |prefixes: &[&str]| prefixes.iter().map(|prefix| (*prefix).to_owned()).collect(); + match dialect.to_ascii_lowercase().as_str() { + "bigquery" => LexicalSyntax { + backslash_escape_quotes: strings(&["'", "\""]), + raw_string_prefix: true, + triple_quoted_strings: true, + line_comment_prefixes: lines(&["--", "#"]), + ..LexicalSyntax::default() + }, + "snowflake" => LexicalSyntax { + backslash_escape_quotes: strings(&["'"]), + line_comment_prefixes: lines(&["--", "//", "#"]), + ..LexicalSyntax::default() + }, + "clickhouse" => LexicalSyntax { + backslash_escape_quotes: strings(&["'"]), + line_comment_prefixes: lines(&["--"]), + ..LexicalSyntax::default() + }, + "databricks" | "duckdb" => LexicalSyntax { + line_comment_prefixes: lines(&["--"]), + ..LexicalSyntax::default() + }, + "mssql" | "sqlserver" | "tsql" => LexicalSyntax { + nested_block_comments: true, + line_comment_prefixes: lines(&["--"]), + ..LexicalSyntax::default() + }, + _ => LexicalSyntax { + escape_string_prefix: true, + nested_block_comments: true, + line_comment_prefixes: lines(&["--"]), + ..LexicalSyntax::default() + }, } - let mut depth = 0_usize; - let mut cursor = index; - while cursor < bytes.len() { - if bytes[cursor..].starts_with(b"/*") { - depth += 1; - cursor += 2; - } else if bytes[cursor..].starts_with(b"*/") { - depth -= 1; - cursor += 2; - if depth == 0 { - return Some(cursor); - } - } else { - cursor += 1; - } - } - Some(bytes.len()) } -fn skip_space(bytes: &[u8], mut index: usize) -> usize { +fn is_quote_start(byte: u8) -> bool { + matches!(byte, b'\'' | b'"' | b'`' | b'$') +} + +fn skip_space(bytes: &[u8], mut index: usize, syntax: &LexicalSyntax) -> usize { loop { while bytes.get(index).is_some_and(u8::is_ascii_whitespace) { index += 1; } - match comment_end(bytes, index) { + if bytes.get(index).is_none_or(|byte| is_quote_start(*byte)) { + return index; + } + match dialect_non_code_end(bytes, index, syntax) { Ok(Some(end)) => index = end, _ => return index, } } } +fn blank(text: &str) -> String { + text.chars() + .map(|value| if value == '\n' { '\n' } else { ' ' }) + .collect() +} + fn quoted_name(sql: &str, open: usize) -> Option<(String, usize)> { let rest = sql.get(open..)?.strip_prefix("(\"")?; let length = rest.find('"')?; @@ -135,19 +160,18 @@ fn quoted_name(sql: &str, open: usize) -> Option<(String, usize)> { fn stub_calls( sql: &str, policy: QuotePolicy, - nested_comments: bool, + syntax: &LexicalSyntax, ) -> (String, Vec, Vec) { let bytes = sql.as_bytes(); let mut edits: Vec<(usize, usize, String)> = Vec::new(); let mut calls: Vec = Vec::new(); let mut index = 0; while index < bytes.len() { - if nested_comments && let Some(end) = nested_comment_end(bytes, index) { - index = end; - continue; - } - match non_code_end(bytes, index, policy) { + match dialect_non_code_end(bytes, index, syntax) { Ok(Some(end)) => { + if !is_quote_start(bytes[index]) { + edits.push((index, end, blank(&sql[index..end]))); + } index = end; continue; } @@ -200,11 +224,11 @@ fn stub_calls( let Some(kind) = call_kind(&word) else { continue; }; - let open = skip_space(bytes, index); + let open = skip_space(bytes, index, syntax); if bytes.get(open) != Some(&b'(') { continue; } - let Ok(close) = matching_paren(bytes, open, policy) else { + let Ok(close) = dialect_matching_paren(bytes, open, syntax) else { continue; }; let number = calls.len(); @@ -216,6 +240,12 @@ fn stub_calls( edits.push((start, close + 1, format!("{}{number}", stub_prefix(kind)))); index = close + 1; } + let (stubbed, offsets) = apply_edits(sql, edits); + (stubbed, offsets, calls) +} + +/// `sql` with each edit applied, and per output character the input character it came from. +fn apply_edits(sql: &str, edits: Vec<(usize, usize, String)>) -> (String, Vec) { let mut stubbed = String::with_capacity(sql.len()); let mut offsets: Vec = Vec::with_capacity(sql.len() + 1); let mut character = 0; @@ -227,8 +257,14 @@ fn stub_calls( character += 1; } stubbed.push_str(&replacement); - offsets.extend(std::iter::repeat_n(character, replacement.chars().count())); - character += sql[start..end].chars().count(); + let replaced = sql[start..end].chars().count(); + let length = replacement.chars().count(); + if length == replaced { + offsets.extend(character..character + length); + } else { + offsets.extend(std::iter::repeat_n(character, length)); + } + character += replaced; cursor = end; } for value in sql[cursor..].chars() { @@ -237,7 +273,7 @@ fn stub_calls( character += 1; } offsets.push(character); - (stubbed, offsets, calls) + (stubbed, offsets) } fn call_target(arguments: &str) -> Option { From 93f46227895410339c7281c25f8087e0c7b87411 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 12:14:05 +0100 Subject: [PATCH 5/8] fix(rules): match sqlparser literal, function-call, TOP and import-star facts - SQBRDECLARATION101 compares each string literal's own source text with the authored SQL and covers double-quoted, triple-quoted, escape and Unicode strings again. - SQBRTEST203 no longer treats a parameterized cast type such as DECIMAL(18, 2) as a function call. - A T-SQL TOP (n), TOP n PERCENT or WITH TIES clause is not projection. - Stars in dependency import CTEs are exempt from SQBRMODEL102 by CTE classification, not by having a recovered source position. --- crates/sqlbuild-rules/src/engine/tests.rs | 2 + .../src/engine/tests/helpers.rs | 31 +++ .../engine/tests/test_empty_input_tests.rs | 28 +++ .../src/engine/tests/test_query_facts.rs | 176 ++++++++++++++++++ .../src/engine/tests/test_types.rs | 10 + .../src/query_facts/_helpers/facts.rs | 30 ++- .../src/query_facts/_helpers/tokens.rs | 60 +++++- .../src/query_facts/constants.rs | 1 + .../sqlbuild-rules/src/query_facts/models.rs | 9 +- .../src/rules/_helpers/evaluation.rs | 34 ++-- 10 files changed, 354 insertions(+), 27 deletions(-) create mode 100644 crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs diff --git a/crates/sqlbuild-rules/src/engine/tests.rs b/crates/sqlbuild-rules/src/engine/tests.rs index 3190903ee8..ed6cc5ba16 100644 --- a/crates/sqlbuild-rules/src/engine/tests.rs +++ b/crates/sqlbuild-rules/src/engine/tests.rs @@ -8,6 +8,8 @@ mod evaluation; mod helpers; #[path = "tests/test_incremental_rules.rs"] mod incremental_rules; +#[path = "tests/test_query_facts.rs"] +mod query_facts; #[path = "tests/test_scope_facts.rs"] mod scope_facts; #[path = "tests/test_speculative_rows.rs"] diff --git a/crates/sqlbuild-rules/src/engine/tests/helpers.rs b/crates/sqlbuild-rules/src/engine/tests/helpers.rs index 1300b5ab36..9cd7c7e08d 100644 --- a/crates/sqlbuild-rules/src/engine/tests/helpers.rs +++ b/crates/sqlbuild-rules/src/engine/tests/helpers.rs @@ -15,6 +15,7 @@ use crate::models::{ use crate::engine::_helpers::evaluation::evaluate_json; use crate::engine::_helpers::evaluation::evaluate_request as evaluate_parsed; use crate::engine::_helpers::evaluation::parse_parts; +use crate::engine::tests::test_types; pub(crate) fn request(project_dir: &TempDir, config: &Value) -> String { json!({ @@ -588,3 +589,33 @@ pub(crate) fn drop_at_once( drop(speculative); Ok(false) } +pub(crate) fn query_facts_faults( + test_case: &test_types::QueryFactsTestCase, +) -> Result, String> { + let project_dir = TempDir::new().map_err(|error| error.to_string())?; + let config = json!({"select": test_case.select, "cache": {"enabled": false}}); + let mut request: Value = + serde_json::from_str(&request(&project_dir, &config)).map_err(|error| error.to_string())?; + request["dialect"] = json!(test_case.dialect); + request["models"][0]["query_sql"] = json!(test_case.query_sql); + request["models"][0]["authored_sql"] = json!(test_case.query_sql); + request["models"][0]["enum_columns"] = json!(test_case.enum_columns); + request["models"][0]["config"] = json!({"contract": "enforced"}); + request["models"][0]["columns"] = test_case.columns.clone(); + let result: Value = serde_json::from_str(&evaluate_json(&request.to_string())?) + .map_err(|error| error.to_string())?; + let faults = result["faults"] + .as_array() + .ok_or("faults must be an array")?; + Ok(faults + .iter() + .map(|fault| { + ( + fault["code"].as_str().unwrap_or_default().to_owned(), + fault["line"].as_u64().unwrap_or_default(), + fault["column"].as_u64().unwrap_or_default(), + fault["message"].as_str().unwrap_or_default().to_owned(), + ) + }) + .collect()) +} diff --git a/crates/sqlbuild-rules/src/engine/tests/test_empty_input_tests.rs b/crates/sqlbuild-rules/src/engine/tests/test_empty_input_tests.rs index 1c947d4353..360658a9ea 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_empty_input_tests.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_empty_input_tests.rs @@ -44,6 +44,34 @@ fn given_sql_test_shapes_when_evaluating_empty_input_rule_then_flags_only_filler allowed_tests: json!([]), expected_messages: &[FLAGGED_MESSAGE], }, + test_types::EmptyInputTestRuleTestCase { + description: "a cast to a parameterized type under a false predicate is an empty mock", + test: helpers::empty_input_test_fact( + "orders__empty_inputs_produce_no_rows", + &[( + "__source__raw_orders", + "SELECT CAST(NULL AS DECIMAL(18, 2)) AS amount WHERE 1 = 0", + )], + &[], + &[NO_ROWS_ASSERTION], + ), + allowed_tests: json!([]), + expected_messages: &[FLAGGED_MESSAGE], + }, + test_types::EmptyInputTestRuleTestCase { + description: "a cast to a parameterized type keeps a bare existence check", + test: helpers::empty_input_test_fact( + "orders__empty_inputs_produce_no_rows", + &[EMPTY_ORDERS], + &[], + &[( + "__assert__empty_inputs_produce_no_rows", + "SELECT CAST(order_id AS VARCHAR(10)) AS unexpected_row FROM __ref(\"orders\")", + )], + ), + allowed_tests: json!([]), + expected_messages: &[FLAGGED_MESSAGE], + }, test_types::EmptyInputTestRuleTestCase { description: "input rows with expected output pass", test: helpers::empty_input_test_fact( diff --git a/crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs b/crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs new file mode 100644 index 0000000000..de7a7dee0b --- /dev/null +++ b/crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs @@ -0,0 +1,176 @@ +use serde_json::json; + +use crate::engine::tests::helpers; +use crate::engine::tests::test_types::QueryFactsTestCase; + +const IMPORT_RULES: &[&str] = &["SQBRMODEL101", "SQBRMODEL102"]; +const ENUM_RULE: &[&str] = &["SQBRDECLARATION101"]; +const NUMERIC_RULE: &[&str] = &["SQBRDECLARATION102"]; +const OUTPUT_TYPE_RULE: &[&str] = &["SQBRCONTRACT105"]; +const STATUS: &[&str] = &["status"]; + +#[test] +fn given_dialect_sql_when_reading_query_facts_then_findings_match_the_authored_sql() +-> Result<(), String> { + let test_cases = [ + QueryFactsTestCase { + description: "a Snowflake // comment holds no dependency call", + dialect: "snowflake", + query_sql: "WITH imported AS (\n SELECT * FROM __ref(\"orders\")\n // replaced __ref(\"orders_v1\")\n)\nSELECT order_id FROM imported", + select: IMPORT_RULES, + enum_columns: &[], + columns: json!([]), + expected_faults: &[], + }, + QueryFactsTestCase { + description: "an apostrophe in a Snowflake // comment opens no string", + dialect: "snowflake", + query_sql: "WITH imported AS (\n // don't read the archive\n SELECT * FROM __ref(\"orders\")\n)\nSELECT order_id FROM imported", + select: IMPORT_RULES, + enum_columns: &[], + columns: json!([]), + expected_faults: &[], + }, + QueryFactsTestCase { + description: "an apostrophe in a Snowflake # comment opens no string", + dialect: "snowflake", + query_sql: "WITH imported AS (\n # don't read the archive\n SELECT * FROM __ref(\"orders\")\n)\nSELECT order_id FROM imported", + select: IMPORT_RULES, + enum_columns: &[], + columns: json!([]), + expected_faults: &[], + }, + QueryFactsTestCase { + description: "an apostrophe in a BigQuery # comment opens no string", + dialect: "bigquery", + query_sql: "WITH imported AS (\n # don't read the archive\n SELECT * FROM __ref(\"orders\")\n)\nSELECT order_id FROM imported", + select: IMPORT_RULES, + enum_columns: &[], + columns: json!([]), + expected_faults: &[], + }, + QueryFactsTestCase { + description: "a DuckDB FROM-first import CTE star is exempt without a SELECT keyword", + dialect: "duckdb", + query_sql: "WITH imported AS (FROM __ref(\"orders\"))\nSELECT order_id FROM imported", + select: IMPORT_RULES, + enum_columns: &[], + columns: json!([]), + expected_faults: &[], + }, + QueryFactsTestCase { + description: "a BigQuery double-quoted string compared with an enum column", + dialect: "bigquery", + query_sql: "SELECT order_id FROM orders WHERE status = \"active\"", + select: ENUM_RULE, + enum_columns: STATUS, + columns: json!([]), + expected_faults: &[("SQBRDECLARATION101", 1, 35, "")], + }, + QueryFactsTestCase { + description: "a BigQuery triple-quoted string compared with an enum column", + dialect: "bigquery", + query_sql: "SELECT order_id FROM orders WHERE status = '''active'''", + select: ENUM_RULE, + enum_columns: STATUS, + columns: json!([]), + expected_faults: &[("SQBRDECLARATION101", 1, 35, "")], + }, + QueryFactsTestCase { + description: "a Postgres escape string compared with an enum column", + dialect: "postgres", + query_sql: "SELECT order_id FROM orders WHERE status = E'active'", + select: ENUM_RULE, + enum_columns: STATUS, + columns: json!([]), + expected_faults: &[("SQBRDECLARATION101", 1, 35, "")], + }, + QueryFactsTestCase { + description: "an interval unit is not an enum column", + dialect: "duckdb", + query_sql: "SELECT order_id FROM orders WHERE created_at > NOW() - INTERVAL 1 DAY", + select: ENUM_RULE, + enum_columns: &["day"], + columns: json!([]), + expected_faults: &[], + }, + QueryFactsTestCase { + description: "a DuckDB == comparison keeps its source position", + dialect: "duckdb", + query_sql: "SELECT order_id FROM orders WHERE amount == 42", + select: NUMERIC_RULE, + enum_columns: &[], + columns: json!([]), + expected_faults: &[("SQBRDECLARATION102", 1, 35, "amount == 42")], + }, + QueryFactsTestCase { + description: "a parenthesized T-SQL TOP argument is not projection", + dialect: "mssql", + query_sql: "SELECT TOP (5) amount + 1 AS amount FROM orders", + select: OUTPUT_TYPE_RULE, + enum_columns: &[], + columns: json!([{"name": "amount", "type": "INTEGER", "type_proven": true}]), + expected_faults: &[( + "SQBRCONTRACT105", + 1, + 16, + "calculated without an outer explicit cast", + )], + }, + QueryFactsTestCase { + description: "a T-SQL TOP PERCENT clause is not projection", + dialect: "mssql", + query_sql: "SELECT TOP 5 PERCENT amount + 1 AS amount FROM orders", + select: OUTPUT_TYPE_RULE, + enum_columns: &[], + columns: json!([{"name": "amount", "type": "INTEGER", "type_proven": true}]), + expected_faults: &[( + "SQBRCONTRACT105", + 1, + 22, + "calculated without an outer explicit cast", + )], + }, + QueryFactsTestCase { + description: "a placeholder in relation position still parses", + dialect: "duckdb", + query_sql: "SELECT order_id FROM @@@orders_table WHERE amount > 7", + select: NUMERIC_RULE, + enum_columns: &[], + columns: json!([]), + expected_faults: &[("SQBRDECLARATION102", 1, 44, "amount > 7")], + }, + QueryFactsTestCase { + description: "a placeholder name with non-ASCII word characters is replaced whole", + dialect: "duckdb", + query_sql: "SELECT order_id FROM orders WHERE amount > @@@seuil_minimé AND amount < 9", + select: NUMERIC_RULE, + enum_columns: &[], + columns: json!([]), + expected_faults: &[("SQBRDECLARATION102", 1, 64, "amount < 9")], + }, + ]; + for test_case in test_cases { + let faults = helpers::query_facts_faults(&test_case)?; + let observed: Vec<(&str, u64, u64, &str)> = faults + .iter() + .enumerate() + .map(|(index, (code, line, column, message))| { + let shown = test_case + .expected_faults + .get(index) + .map(|expected| expected.3) + .filter(|fragment| message.contains(fragment)) + .unwrap_or(message.as_str()); + (code.as_str(), *line, *column, shown) + }) + .collect(); + assert_eq!( + observed, + test_case.expected_faults.to_vec(), + "{}", + test_case.description + ); + } + Ok(()) +} diff --git a/crates/sqlbuild-rules/src/engine/tests/test_types.rs b/crates/sqlbuild-rules/src/engine/tests/test_types.rs index 2a53c8f6d1..a2501f8c01 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_types.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_types.rs @@ -272,3 +272,13 @@ pub(crate) struct SpeculativeOutcomeTestCase { pub(crate) outcome: fn() -> crate::engine::_helpers::memo::SpeculativeOutcome, pub(crate) expected_clean: bool, } + +pub(crate) struct QueryFactsTestCase { + pub(crate) description: &'static str, + pub(crate) dialect: &'static str, + pub(crate) query_sql: &'static str, + pub(crate) select: &'static [&'static str], + pub(crate) enum_columns: &'static [&'static str], + pub(crate) columns: Value, + pub(crate) expected_faults: &'static [(&'static str, u64, u64, &'static str)], +} diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs index 9a6ab8b9b3..2111b8249a 100644 --- a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs @@ -1,9 +1,11 @@ use crate::query_facts::_helpers::shape::stub_call; use crate::query_facts::_helpers::tokens::{ComparisonOperator, QueryTokens, SelectTokens}; -use crate::query_facts::constants::{MAX_FUNCTION_CALL_DEPTH, TABLE_FUNCTION_STUB}; +use crate::query_facts::constants::{ + MAX_FUNCTION_CALL_DEPTH, TABLE_FUNCTION_STUB, TRIPLE_QUOTE_LENGTH, +}; use crate::query_facts::models::{ CallKind, ColumnFact, ComparisonFacts, ComparisonSpan, CteFacts, ExpressionItem, ItemFacts, - ModelShape, ParsedFixture, QueryFacts, RelationFacts, SelectFacts, SetFacts, + ModelShape, ParsedFixture, QueryFacts, RelationFacts, SelectFacts, SetFacts, SourceText, }; use polyglot_sql::dialects::Dialect; use polyglot_sql::expressions::{Identifier, Literal, Select, TableRef, With}; @@ -378,6 +380,9 @@ impl Builder<'_> { ), ..ComparisonFacts::default() }; + let mut string_tokens: Vec<(usize, usize, &str)> = span.map_or_else(Vec::new, |span| { + self.tokens.string_tokens(span.start, span.end) + }); for value in node.dfs() { match value { Expression::Column(column) => { @@ -387,8 +392,15 @@ impl Builder<'_> { fact.columns.insert(identifier.name.clone()); } Expression::Literal(literal) => { - if let Some(text) = string_literal(literal) { - fact.string_literals.push(text); + if let Some(generated) = string_literal(literal) { + let position = string_tokens + .iter() + .position(|(_, _, text)| *text == literal.value_str()); + let range = position.map(|index| { + let (start, end, _) = string_tokens.remove(index); + (start, end) + }); + fact.string_literals.push(SourceText { range, generated }); } } _ => {} @@ -687,6 +699,15 @@ fn string_literal(literal: &Literal) -> Option { match literal { Literal::String(value) => Some(format!("'{}'", value.replace('\'', "''"))), Literal::NationalString(value) => Some(format!("N'{}'", value.replace('\'', "''"))), + Literal::TripleQuotedString(value, quote) => { + let delimiter = quote.to_string().repeat(TRIPLE_QUOTE_LENGTH); + Some(format!("{delimiter}{value}{delimiter}")) + } + Literal::EscapeString(value) => Some(if value.starts_with(['E', 'e']) { + value.clone() + } else { + format!("E'{value}'") + }), _ => None, } } @@ -694,6 +715,7 @@ fn string_literal(literal: &Literal) -> Option { fn direct_string_literal(expression: &Expression) -> Option { match unwrap_paren(expression) { Expression::Literal(literal) => string_literal(literal), + Expression::UnicodeString(unicode) => direct_string_literal(&unicode.this), _ => None, } } diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs index 714c8f9f04..19c31b01de 100644 --- a/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs @@ -262,14 +262,44 @@ impl<'a> QueryTokens<'a> { /// Whether a function call starts in `range`. pub(crate) fn calls_function(&self, range: Range) -> bool { - range.into_iter().any(|index| { - self.opens(index + 1) + let mut index = range.start; + while index < range.end { + if self.type_follows(index) { + index = self + .matching_close(index + 2) + .map_or(index + 1, |close| close + 1); + continue; + } + if self.opens(index + 1) && self.kind(index + 1) == Some(TokenType::LParen) && self.word(index).is_some_and(|word| { word.starts_with(|first: char| first.is_ascii_alphabetic() || first == '_') && !NON_FUNCTION_CALLS.contains(&word.as_str()) }) - }) + { + return true; + } + index += 1; + } + false + } + + /// Whether `index` is a cast's `AS` or `::` before a parameterized type such as `DECIMAL(18, 2)`. + fn type_follows(&self, index: usize) -> bool { + (self.is_word(index, &["AS"]) || self.kind(index) == Some(TokenType::DColon)) + && self.word(index + 1).is_some() + && self.kind(index + 2) == Some(TokenType::LParen) + } + + /// The string-literal tokens within a normalized range, as character range and parsed text. + pub(crate) fn string_tokens(&self, start: usize, end: usize) -> Vec<(usize, usize, &str)> { + self.tokens + .iter() + .filter(|token| { + token.span.start >= start && token.span.end <= end && string_token(token.token_type) + }) + .map(|token| (token.span.start, token.span.end, token.text.as_str())) + .collect() } /// Every SELECT keyword's region and projection items, in source order. @@ -313,7 +343,17 @@ impl<'a> QueryTokens<'a> { cursor = close + 1; } } else if self.is_word(cursor, &["TOP"]) { - cursor += 2; + cursor += 1; + cursor = match self.opens(cursor).then(|| self.matching_close(cursor)) { + Some(Some(close)) => close + 1, + _ => cursor + 1, + }; + if self.is_word(cursor, &["PERCENT"]) { + cursor += 1; + } + if self.is_word(cursor, &["WITH"]) && self.is_word(cursor + 1, &["TIES"]) { + cursor += 2; + } } else { break; } @@ -574,6 +614,18 @@ fn compact(text: &str) -> String { .collect() } +fn string_token(token_type: TokenType) -> bool { + matches!( + token_type, + TokenType::String + | TokenType::TripleSingleQuotedString + | TokenType::TripleDoubleQuotedString + | TokenType::EscapeString + | TokenType::UnicodeString + | TokenType::NationalString + ) +} + fn literal(token_type: TokenType) -> bool { matches!( token_type, diff --git a/crates/sqlbuild-rules/src/query_facts/constants.rs b/crates/sqlbuild-rules/src/query_facts/constants.rs index c1733c094f..564143160d 100644 --- a/crates/sqlbuild-rules/src/query_facts/constants.rs +++ b/crates/sqlbuild-rules/src/query_facts/constants.rs @@ -9,3 +9,4 @@ pub(crate) const MAX_FUNCTION_CALL_DEPTH: usize = 128; pub(crate) const UDF_CALL: &str = "__udf"; pub(crate) const TABLE_FUNCTION_CALL: &str = "__table_fn"; pub(crate) const MIN_CAST_TOKENS: usize = 2; +pub(crate) const TRIPLE_QUOTE_LENGTH: usize = 3; diff --git a/crates/sqlbuild-rules/src/query_facts/models.rs b/crates/sqlbuild-rules/src/query_facts/models.rs index 0f0b525597..2deb0ea0a7 100644 --- a/crates/sqlbuild-rules/src/query_facts/models.rs +++ b/crates/sqlbuild-rules/src/query_facts/models.rs @@ -115,6 +115,13 @@ pub(crate) struct ColumnFact { pub(crate) qualifier: Option, } +/// Text read from a model's own SQL when its normalized range is known, else Polyglot's spelling. +#[derive(Debug)] +pub(crate) struct SourceText { + pub(crate) range: Option<(usize, usize)>, + pub(crate) generated: String, +} + #[derive(Debug, Default)] pub(crate) struct ComparisonFacts { pub(crate) span: Option, @@ -124,7 +131,7 @@ pub(crate) struct ComparisonFacts { pub(crate) binary: bool, pub(crate) output_name: Option, pub(crate) columns: BTreeSet, - pub(crate) string_literals: Vec, + pub(crate) string_literals: Vec, pub(crate) numeric_literals: Vec, pub(crate) modified_columns: BTreeSet, pub(crate) modified_string_literal: bool, diff --git a/crates/sqlbuild-rules/src/rules/_helpers/evaluation.rs b/crates/sqlbuild-rules/src/rules/_helpers/evaluation.rs index f8e62ee581..e2eb76d93f 100644 --- a/crates/sqlbuild-rules/src/rules/_helpers/evaluation.rs +++ b/crates/sqlbuild-rules/src/rules/_helpers/evaluation.rs @@ -487,22 +487,25 @@ fn select_star( .build() .map_err(|error| error.to_string())? .is_match(&parsed.model.relative_path); - let mut import_stars: HashSet = HashSet::new(); + let mut import_queries: Vec<&QueryFacts> = Vec::new(); for (cte, classified) in parsed.facts.ctes.iter().zip(&parsed.classification.ctes) { if classified.dependency_import { - import_stars.extend(stars_in_query(&cte.query).into_iter().flatten()); + import_queries.extend(cte.query.all_queries()); } } for query in parsed.facts.all_queries() { let Some(select) = query.root_select() else { continue; }; + if import_queries + .iter() + .any(|imported| std::ptr::eq(*imported, query)) + { + continue; + } let dynamic_output_star = dynamic_contracts::allows_output_star(query, parsed.facts, parsed.model); for star in stars_in_select(select) { - if star.is_some_and(|offset| import_stars.contains(&offset)) { - continue; - } if dynamic_output_star { continue; } @@ -856,10 +859,14 @@ fn evaluate_comparison( .columns .iter() .any(|name| parsed.model.enum_columns.contains(name)); - let bare_string = comparison - .string_literals - .iter() - .any(|literal| parsed.model.authored_sql.contains(literal)); + let bare_string = comparison.string_literals.iter().any(|literal| { + let text = literal + .range + .map_or(literal.generated.as_str(), |(start, end)| { + parsed.source.text(start, end) + }); + parsed.model.authored_sql.contains(text) + }); let modified_controlled_column = comparison .modified_columns @@ -1103,15 +1110,6 @@ fn custom_rule_test_coverage( } } -fn stars_in_query(query: &QueryFacts) -> Vec> { - query - .all_queries() - .into_iter() - .filter_map(QueryFacts::root_select) - .flat_map(stars_in_select) - .collect() -} - fn stars_in_select(select: &SelectFacts) -> Vec> { select .items From 36486acfcebf72c2f9969783dd2929e12c1c4b1d Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 12:20:15 +0100 Subject: [PATCH 6/8] fix(rules): anchor FROM-first stars on their FROM keyword A DuckDB FROM-first query has an implicit star but no SELECT token, so its star finding fell back to 1:1. Anchor that star on the FROM keyword. A SELECT whose only located part is an unaliased subquery is now matched to its own SELECT keyword through the subquery's opening parenthesis, so its star keeps its source position too. --- .../src/engine/tests/test_query_facts.rs | 36 ++++++++++ .../src/query_facts/_helpers/facts.rs | 37 +++++++--- .../src/query_facts/_helpers/tokens.rs | 70 +++++++++++++++++++ 3 files changed, 133 insertions(+), 10 deletions(-) diff --git a/crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs b/crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs index de7a7dee0b..11b118a382 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs @@ -58,6 +58,42 @@ fn given_dialect_sql_when_reading_query_facts_then_findings_match_the_authored_s columns: json!([]), expected_faults: &[], }, + QueryFactsTestCase { + description: "a terminal DuckDB FROM-first star is anchored on its FROM keyword", + dialect: "duckdb", + query_sql: "WITH final AS (SELECT order_id FROM __ref(\"orders\"))\nFROM final", + select: IMPORT_RULES, + enum_columns: &[], + columns: json!([]), + expected_faults: &[("SQBRMODEL102", 2, 1, "SELECT * is allowed only")], + }, + QueryFactsTestCase { + description: "a FROM-first subquery does not take the outer star's position", + dialect: "duckdb", + query_sql: "WITH imported AS (SELECT * FROM __ref(\"orders\"))\nSELECT * FROM (FROM imported)", + select: &["SQBRMODEL102"], + enum_columns: &[], + columns: json!([]), + expected_faults: &[("SQBRMODEL102", 2, 8, "SELECT * is allowed only")], + }, + QueryFactsTestCase { + description: "a SELECT subquery does not take the outer star's position", + dialect: "duckdb", + query_sql: "WITH imported AS (SELECT * FROM __ref(\"orders\"))\nSELECT * FROM (SELECT * FROM imported)", + select: &["SQBRMODEL102"], + enum_columns: &[], + columns: json!([]), + expected_faults: &[("SQBRMODEL102", 2, 8, "SELECT * is allowed only")], + }, + QueryFactsTestCase { + description: "a comparison of two literals keeps its source position", + dialect: "duckdb", + query_sql: "SELECT order_id FROM orders WHERE 3 < 7", + select: NUMERIC_RULE, + enum_columns: &[], + columns: json!([]), + expected_faults: &[("SQBRDECLARATION102", 1, 35, "3 < 7")], + }, QueryFactsTestCase { description: "a BigQuery double-quoted string compared with an enum column", dialect: "bigquery", diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs index 2111b8249a..cc19d7108f 100644 --- a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs @@ -199,13 +199,16 @@ impl Builder<'_> { } fn select(&self, select: &Select) -> SelectFacts { + let from_first: Option = select_anchor(select, &self.tokens) + .and_then(|anchor| self.tokens.leading_from_keyword(anchor)); let tokens: Option<&SelectTokens> = self .select_tokens(select) - .filter(|tokens| tokens.items.len() == select.expressions.len()); + .filter(|tokens| tokens.items.len() == select.expressions.len()) + .filter(|tokens| from_first.is_none_or(|from| tokens.start > from)); let mut items: Vec = Vec::with_capacity(select.expressions.len()); for (index, expression) in select.expressions.iter().enumerate() { let item: Option<&Range> = tokens.map(|tokens| &tokens.items[index]); - items.push(self.item(expression, item)); + items.push(self.item(expression, item, from_first)); } let mut comparisons: Vec = Vec::new(); for join in &select.joins { @@ -252,20 +255,28 @@ impl Builder<'_> { } fn select_tokens(&self, select: &Select) -> Option<&SelectTokens> { - let anchor = select_anchor(select)?; + let anchor = select_anchor(select, &self.tokens)?; self.selects .iter() .filter(|tokens| tokens.start < anchor && anchor < tokens.end) .max_by_key(|tokens| tokens.start) } - fn item(&self, expression: &Expression, item: Option<&Range>) -> ItemFacts { + fn item( + &self, + expression: &Expression, + item: Option<&Range>, + from_first: Option, + ) -> ItemFacts { if let Expression::Star(_) = expression { return ItemFacts::Star { item_start: item .filter(|item| !item.is_empty()) - .map(|item| self.tokens.start(item.start)), - star_start: item.and_then(|item| self.tokens.star_start(item)), + .map(|item| self.tokens.start(item.start)) + .or(from_first), + star_start: item + .and_then(|item| self.tokens.star_start(item)) + .or(from_first), }; } let (alias, value) = match expression { @@ -580,8 +591,8 @@ fn quoted_parts(parts: &[&Identifier], tokens: &QueryTokens<'_>) -> String { .join(".") } -fn select_anchor(select: &Select) -> Option { - let parts = select +fn select_anchor(select: &Select, tokens: &QueryTokens<'_>) -> Option { + let parts: Vec<&Expression> = select .expressions .iter() .chain(select.from.iter().flat_map(|from| from.expressions.iter())) @@ -599,12 +610,18 @@ fn select_anchor(select: &Select) -> Option { .flat_map(|group| group.expressions.iter()), ) .chain(select.having.iter().map(|clause| &clause.this)) - .chain(select.qualify.iter().map(|clause| &clause.this)); - for part in parts { + .chain(select.qualify.iter().map(|clause| &clause.this)) + .collect(); + for part in &parts { if let Some(start) = own_span(part) { return Some(start); } } + for part in &parts { + if let Some(open) = span_bounds(part).and_then(|(start, _)| tokens.enclosing_open(start)) { + return Some(open); + } + } None } diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs index 19c31b01de..afde5ea27f 100644 --- a/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs @@ -226,6 +226,76 @@ impl<'a> QueryTokens<'a> { ) } + fn matching_open(&self, close: usize) -> Option { + let mut depth = 0_usize; + for index in (0..=close).rev() { + if self.closes(index) { + depth += 1; + } else if self.opens(index) { + depth = depth.checked_sub(1)?; + if depth == 0 { + return Some(index); + } + } + } + None + } + + /// Where the innermost bracket still open before character `position` starts. + pub(crate) fn enclosing_open(&self, position: usize) -> Option { + let mut depth = 0_usize; + for index in (0..self.tokens.len()).rev() { + if self.start(index) >= position { + continue; + } + if self.closes(index) { + depth += 1; + } else if self.opens(index) { + if depth == 0 { + return Some(self.start(index)); + } + depth -= 1; + } + } + None + } + + /// The `FROM` keyword starting a FROM-first query whose first relation starts at `anchor`. + pub(crate) fn leading_from_keyword(&self, anchor: usize) -> Option { + let mut index = (0..self.tokens.len()).find(|index| self.start(*index) >= anchor)?; + let mut depth = 0_usize; + while index > 0 { + index -= 1; + if self.closes(index) { + depth += 1; + } else if self.opens(index) { + depth = depth.checked_sub(1)?; + } else if depth == 0 && self.kind(index) == Some(TokenType::Select) { + return None; + } else if depth == 0 && self.is_word(index, &["FROM"]) { + return self.starts_query(index).then(|| self.start(index)); + } + } + None + } + + fn starts_query(&self, keyword: usize) -> bool { + let Some(previous) = keyword.checked_sub(1) else { + return true; + }; + if self.opens(previous) + || self.kind(previous) == Some(TokenType::Semicolon) + || self.is_word(previous, &["UNION", "INTERSECT", "EXCEPT", "MINUS", "ALL"]) + { + return true; + } + self.closes(previous) + && self + .matching_open(previous) + .and_then(|open| open.checked_sub(1)) + .is_some_and(|before| self.is_word(before, &["AS"])) + } + fn matching_close(&self, open: usize) -> Option { let mut depth = 0_usize; for index in open..self.tokens.len() { From efb93e753631fd725e36cd0432807513882f0a00 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 12:26:04 +0100 Subject: [PATCH 7/8] fix(rules): treat CONVERT and TRY_CONVERT type arguments as types SQBRTEST203 counted the leading parameterized type of a T-SQL CONVERT(VARCHAR(10), x) or TRY_CONVERT(DECIMAL(18, 2), x) as a function call, so an empty SQL Server mock or bare existence assertion was not flagged. Skip that type argument, as sqlparser's conversion parse did; function calls in the value argument still count. --- .../src/engine/tests/helpers.rs | 20 ++++- .../engine/tests/test_empty_input_tests.rs | 85 +++++++++++++++++++ .../src/query_facts/_helpers/tokens.rs | 18 ++-- 3 files changed, 117 insertions(+), 6 deletions(-) diff --git a/crates/sqlbuild-rules/src/engine/tests/helpers.rs b/crates/sqlbuild-rules/src/engine/tests/helpers.rs index 9cd7c7e08d..93a3fee44b 100644 --- a/crates/sqlbuild-rules/src/engine/tests/helpers.rs +++ b/crates/sqlbuild-rules/src/engine/tests/helpers.rs @@ -328,11 +328,29 @@ pub(crate) fn empty_input_rule_evaluation( tests: Value, allowed_tests: Value, cache_enabled: bool, +) -> Result, String> { + dialect_empty_input_rule_evaluation( + "duckdb", + project_dir, + select, + tests, + allowed_tests, + cache_enabled, + ) +} + +pub(crate) fn dialect_empty_input_rule_evaluation( + dialect: &str, + project_dir: &TempDir, + select: Value, + tests: Value, + allowed_tests: Value, + cache_enabled: bool, ) -> Result, String> { let request = json!({ "version": 1, "project_dir": project_dir.path(), - "dialect": "duckdb", + "dialect": dialect, "config": { "select": select, "thresholds": {"min_tests_per_model": 1}, diff --git a/crates/sqlbuild-rules/src/engine/tests/test_empty_input_tests.rs b/crates/sqlbuild-rules/src/engine/tests/test_empty_input_tests.rs index 360658a9ea..3c9f34d22b 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_empty_input_tests.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_empty_input_tests.rs @@ -417,3 +417,88 @@ fn given_cached_minimum_tests_when_filler_becomes_real_test_then_model_cache_is_ } Ok(()) } + +#[test] +fn given_sql_server_conversions_when_evaluating_empty_input_rule_then_type_arguments_are_not_calls() +-> Result<(), String> { + let project_dir = TempDir::new().map_err(|error| error.to_string())?; + let empty_orders = ( + "__source__raw_orders", + "SELECT NULL AS order_id WHERE 1 = 0", + ); + let test_cases = [ + test_types::EmptyInputTestRuleTestCase { + description: "a CONVERT to a parameterized type under a false predicate is an empty mock", + test: helpers::empty_input_test_fact( + "orders__empty_inputs_produce_no_rows", + &[( + "__source__raw_orders", + "SELECT CONVERT(DECIMAL(18, 2), NULL) AS amount WHERE 1 = 0", + )], + &[], + &[NO_ROWS_ASSERTION], + ), + allowed_tests: json!([]), + expected_messages: &[FLAGGED_MESSAGE], + }, + test_types::EmptyInputTestRuleTestCase { + description: "a CONVERT with a style argument is an empty mock", + test: helpers::empty_input_test_fact( + "orders__empty_inputs_produce_no_rows", + &[( + "__source__raw_orders", + "SELECT CONVERT(NUMERIC(10, 2), NULL, 1) AS amount WHERE 1 = 0", + )], + &[], + &[NO_ROWS_ASSERTION], + ), + allowed_tests: json!([]), + expected_messages: &[FLAGGED_MESSAGE], + }, + test_types::EmptyInputTestRuleTestCase { + description: "a TRY_CONVERT to a parameterized type keeps a bare existence check", + test: helpers::empty_input_test_fact( + "orders__empty_inputs_produce_no_rows", + &[empty_orders], + &[], + &[( + "__assert__empty_inputs_produce_no_rows", + "SELECT TRY_CONVERT(VARCHAR(10), order_id) AS unexpected_row FROM __ref(\"orders\")", + )], + ), + allowed_tests: json!([]), + expected_messages: &[FLAGGED_MESSAGE], + }, + test_types::EmptyInputTestRuleTestCase { + description: "a function call in a CONVERT value argument still counts", + test: helpers::empty_input_test_fact( + "orders__empty_inputs_produce_no_rows", + &[( + "__source__raw_orders", + "SELECT CONVERT(VARCHAR(10), COUNT(*)) AS amount WHERE 1 = 0", + )], + &[], + &[NO_ROWS_ASSERTION], + ), + allowed_tests: json!([]), + expected_messages: &[], + }, + ]; + + for test_case in test_cases { + let messages = helpers::dialect_empty_input_rule_evaluation( + "mssql", + &project_dir, + json!(["SQBRTEST203"]), + json!([test_case.test]), + test_case.allowed_tests, + false, + )?; + assert_eq!( + messages, test_case.expected_messages, + "{}", + test_case.description + ); + } + Ok(()) +} diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs index afde5ea27f..9e6863c110 100644 --- a/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/tokens.rs @@ -58,7 +58,7 @@ const RIGHT_BOUNDARIES: [&str; 31] = [ "NULLS", "OFFSET", ]; -const NON_FUNCTION_CALLS: [&str; 41] = [ +const NON_FUNCTION_CALLS: [&str; 42] = [ "IN", "CAST", "TRY_CAST", @@ -91,6 +91,7 @@ const NON_FUNCTION_CALLS: [&str; 41] = [ "FLOOR", "OVERLAY", "CONVERT", + "TRY_CONVERT", "ARRAY", "STRUCT", "WHEN", @@ -354,11 +355,18 @@ impl<'a> QueryTokens<'a> { false } - /// Whether `index` is a cast's `AS` or `::` before a parameterized type such as `DECIMAL(18, 2)`. + /// Whether a parameterized type such as `DECIMAL(18, 2)` follows a cast `AS`, `::` or `CONVERT(`. fn type_follows(&self, index: usize) -> bool { - (self.is_word(index, &["AS"]) || self.kind(index) == Some(TokenType::DColon)) - && self.word(index + 1).is_some() - && self.kind(index + 2) == Some(TokenType::LParen) + let parameterized = + self.word(index + 1).is_some() && self.kind(index + 2) == Some(TokenType::LParen); + let cast = self.is_word(index, &["AS"]) || self.kind(index) == Some(TokenType::DColon); + let conversion = index > 0 + && self.is_word(index - 1, &["CONVERT", "TRY_CONVERT"]) + && self.kind(index) == Some(TokenType::LParen) + && self + .matching_close(index + 2) + .is_some_and(|close| self.kind(close + 1) == Some(TokenType::Comma)); + parameterized && (cast || conversion) } /// The string-literal tokens within a normalized range, as character range and parsed text. From 464e1a623ef05f54979ee011b7e6d2c6bb526af1 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 18 Oct 2026 13:02:45 +0100 Subject: [PATCH 8/8] fix(rules): match unanchored set-operation branches to their SELECT tokens A set-operation branch whose items carry no Polyglot span, such as SELECT CAST(1 AS INTEGER), CAST('games' AS VARCHAR) after an aliased first branch, found no SELECT token region. Its SQBRCONTRACT105 cast types then fell back to Polyglot's spelling and its findings to 1:1, producing false contract findings. Such a select now takes the next unclaimed SELECT region after the previous branch, since branches and CTEs are read in source order. --- .../src/engine/tests/test_query_facts.rs | 55 +++++++++++++++++++ .../src/query_facts/_helpers/facts.rs | 31 ++++++++--- 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs b/crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs index 11b118a382..e7d29e80f8 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_query_facts.rs @@ -94,6 +94,61 @@ fn given_dialect_sql_when_reading_query_facts_then_findings_match_the_authored_s columns: json!([]), expected_faults: &[("SQBRDECLARATION102", 1, 35, "3 < 7")], }, + QueryFactsTestCase { + description: "unaliased later union branches keep their cast types", + dialect: "duckdb", + query_sql: "SELECT CAST(1 AS INTEGER) AS customer_id, CAST('books' AS VARCHAR) AS category, CAST(10.25 AS DECIMAL(12,2)) AS amount\nUNION ALL\nSELECT CAST(1 AS INTEGER), CAST('games' AS VARCHAR), CAST(20.50 AS DECIMAL(12,2))\nUNION ALL\nSELECT CAST(2 AS INTEGER), CAST('books' AS VARCHAR), CAST(7.00 AS DECIMAL(12,2))", + select: OUTPUT_TYPE_RULE, + enum_columns: &[], + columns: json!([{"name": "customer_id", "type": "INTEGER", "type_proven": true}, {"name": "category", "type": "VARCHAR", "type_proven": true}, {"name": "amount", "type": "DECIMAL(12,2)", "type_proven": true}]), + expected_faults: &[], + }, + QueryFactsTestCase { + description: "an unaliased later union branch reports its own position", + dialect: "duckdb", + query_sql: "SELECT CAST(1 AS INTEGER) AS amount\nUNION ALL\nSELECT CAST(2 AS BIGINT)", + select: OUTPUT_TYPE_RULE, + enum_columns: &[], + columns: json!([{"name": "amount", "type": "INTEGER", "type_proven": true}]), + expected_faults: &[("SQBRCONTRACT105", 3, 13, "outer cast that does not target")], + }, + QueryFactsTestCase { + description: "nested parenthesized set operations keep each branch position", + dialect: "duckdb", + query_sql: "(SELECT CAST(1 AS INTEGER) AS amount)\nUNION ALL\n((SELECT 2) EXCEPT (SELECT CAST(3 AS INTEGER)))", + select: OUTPUT_TYPE_RULE, + enum_columns: &[], + columns: json!([{"name": "amount", "type": "INTEGER", "type_proven": true}]), + expected_faults: &[( + "SQBRCONTRACT105", + 3, + 10, + "calculated without an outer explicit cast", + )], + }, + QueryFactsTestCase { + description: "an unaliased branch after a subquery-filtered branch keeps its position", + dialect: "duckdb", + query_sql: "SELECT CAST(order_id AS INTEGER) AS amount FROM orders WHERE order_id IN (SELECT order_id FROM returns)\nUNION ALL BY NAME\nSELECT CAST(4 AS SMALLINT) AS amount", + select: OUTPUT_TYPE_RULE, + enum_columns: &[], + columns: json!([{"name": "amount", "type": "INTEGER", "type_proven": true}]), + expected_faults: &[("SQBRCONTRACT105", 3, 13, "outer cast that does not target")], + }, + QueryFactsTestCase { + description: "unaliased union branches inside a final CTE keep their positions", + dialect: "duckdb", + query_sql: "WITH final AS (\n SELECT CAST(1 AS INTEGER) AS amount\n UNION ALL\n SELECT 5 + 1\n)\nSELECT * FROM final", + select: OUTPUT_TYPE_RULE, + enum_columns: &[], + columns: json!([{"name": "amount", "type": "INTEGER", "type_proven": true}]), + expected_faults: &[( + "SQBRCONTRACT105", + 4, + 10, + "calculated without an outer explicit cast", + )], + }, QueryFactsTestCase { description: "a BigQuery double-quoted string compared with an enum column", dialect: "bigquery", diff --git a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs index cc19d7108f..fe32b68f45 100644 --- a/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs +++ b/crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs @@ -11,8 +11,8 @@ use polyglot_sql::dialects::Dialect; use polyglot_sql::expressions::{Identifier, Literal, Select, TableRef, With}; use polyglot_sql::{ComplexityGuardOptions, Expression, ExpressionWalk, ParseOptions}; use sqlbuild_analysis::lineage::main::parser_dialect::parser_dialect; -use std::cell::RefCell; -use std::collections::HashMap; +use std::cell::{Cell, RefCell}; +use std::collections::{HashMap, HashSet}; use std::ops::Range; fn parse_query(sql: &str, dialect_name: &str) -> Result<(Expression, Dialect), String> { @@ -82,6 +82,8 @@ pub(crate) fn query_facts(sql: &str, dialect_name: &str) -> Result { selects: Vec, dialect: Dialect, spans: RefCell>>, + cursor: Cell, + claimed: RefCell>, } struct ComparisonParts<'a> { @@ -254,12 +258,25 @@ impl Builder<'_> { } } + /// The SELECT region around the select's first located part, else the next unclaimed one. fn select_tokens(&self, select: &Select) -> Option<&SelectTokens> { - let anchor = select_anchor(select, &self.tokens)?; - self.selects - .iter() - .filter(|tokens| tokens.start < anchor && anchor < tokens.end) - .max_by_key(|tokens| tokens.start) + let found = match select_anchor(select, &self.tokens) { + Some(anchor) => self + .selects + .iter() + .filter(|tokens| tokens.start < anchor && anchor < tokens.end) + .max_by_key(|tokens| tokens.start), + None => { + let after = self.cursor.get(); + let claimed = self.claimed.borrow(); + self.selects + .iter() + .find(|tokens| tokens.start >= after && !claimed.contains(&tokens.start)) + } + }?; + self.claimed.borrow_mut().insert(found.start); + self.cursor.set(self.cursor.get().max(found.end)); + Some(found) } fn item(