Skip to content

Commit bfaff24

Browse files
authored
perf: evaluate built-in rules over Polyglot query facts and drop sqlparser (#1143)
* 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. * 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. * build: drop sqlparser from the workspace The rules quote policy is a static dialect table, so no crate needs sqlparser any more. * 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. * 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. * 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. * 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. * 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.
1 parent 06de776 commit bfaff24

46 files changed

Lines changed: 3253 additions & 1669 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎Cargo.lock‎

Lines changed: 0 additions & 44 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,6 @@ sqlbuild-scopes = { path = "crates/sqlbuild-scopes" }
110110
sqlbuild-sqltext = { path = "crates/sqlbuild-sqltext" }
111111
sqlbuild-state = { path = "crates/sqlbuild-state" }
112112
stacker = "0.1"
113-
sqlparser = { version = "0.58", features = ["visitor"] }
114113
syn = { version = "2", features = ["full", "visit", "extra-traits"] }
115114
tempfile = "3"
116115
toml = "0.9"

‎crates/sqlbuild-analysis/src/semantic_validation/_helpers/normalization.rs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,17 @@ pub(crate) fn normalize_analysis_sql(
175175
normalize(sql, dialect, &stubs, &placeholders).map(|mapped| mapped.sql)
176176
}
177177

178+
pub(crate) fn normalize_dialect_sql_mapped(
179+
sql: &str,
180+
dialect: &str,
181+
) -> Result<(String, Vec<usize>), String> {
182+
let input = MappedSql::new(sql);
183+
if !dialect.eq_ignore_ascii_case("snowflake") {
184+
return Ok((input.sql, input.offsets));
185+
}
186+
normalize_variant(input).map(|mapped| (mapped.sql, mapped.offsets))
187+
}
188+
178189
pub(crate) fn normalize_dialect_sql(sql: &str, dialect: &str) -> Result<String, String> {
179190
if !dialect.eq_ignore_ascii_case("snowflake") {
180191
return Ok(sql.to_owned());

‎crates/sqlbuild-analysis/src/semantic_validation/main.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ pub mod new_analysis_pool;
66
pub mod normalize;
77
pub mod normalize_batch;
88
pub mod normalize_dialect;
9+
pub mod normalize_mapped;
910
use crate::semantic_validation::models::ProjectCatalog;
1011

1112
use polyglot_sql::{
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
use crate::semantic_validation::models::MappedNormalization;
2+
3+
/// The dialect normalization analysis applies to `sql`, mapped back to its input characters.
4+
pub fn normalize_mapped(sql: &str, dialect: &str) -> Result<MappedNormalization, String> {
5+
crate::semantic_validation::_helpers::normalization::normalize_dialect_sql_mapped(sql, dialect)
6+
.map(|(sql, offsets)| MappedNormalization { sql, offsets })
7+
}

‎crates/sqlbuild-analysis/src/semantic_validation/models.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,13 @@ pub struct NormalizationInput {
2828
pub placeholders: HashMap<String, String>,
2929
}
3030

31+
/// Normalized analysis SQL with, per normalized character, the input character it came from.
32+
#[derive(Debug)]
33+
pub struct MappedNormalization {
34+
pub sql: String,
35+
pub offsets: Vec<usize>,
36+
}
37+
3138
#[derive(Debug)]
3239
pub struct PositionInput {
3340
pub authored: String,

‎crates/sqlbuild-rules/Cargo.toml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ sqlbuild-core.workspace = true
2424
sqlbuild-scopes.workspace = true
2525
sqlbuild-sqltext.workspace = true
2626
stacker.workspace = true
27-
sqlparser.workspace = true
2827
toml.workspace = true
2928

3029
[build-dependencies]

‎crates/sqlbuild-rules/src/constants.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ pub(crate) const ENFORCED_CONTRACT: &str = "enforced";
2424
pub(crate) const SELECT_STAR_MODEL_RULE_CODE: &str = "SQBRMODEL102";
2525
pub(crate) const BOOLEAN_TYPE: &str = "BOOLEAN";
2626
pub(crate) const DATE_TYPE: &str = "DATE";
27-
pub(crate) const NEGATION_OPERATOR: &str = "-";
2827
pub(crate) const DECLARATION_DOMAIN_COMPONENTS: usize = 3;
2928
pub(crate) const RULE_CODE_NUMBER_LENGTH: usize = 3;
3029
pub(crate) const EMPTY_FIXTURE_QUERY_TOKEN_COUNT: usize = 6;

‎crates/sqlbuild-rules/src/engine/_helpers/evaluation.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use crate::rules::main::{
1010
resolve_threshold_overrides, select, sql_test_coverage,
1111
};
1212
use crate::rules::models::{
13-
ModelEvaluationRequest, ProjectEvaluationRequest, ResolvedThresholdOverride,
13+
ModelEvaluationRequest, ProjectEvaluationRequest, QueryShapes, ResolvedThresholdOverride,
1414
};
1515
use fensu_policy::apply_suppressions;
1616
use fensu_policy::lifecycle::errors::LifecycleError;
@@ -188,6 +188,7 @@ fn evaluate_pending_models(
188188
pending_models: Vec<PendingModelRule<'_>>,
189189
context: ModelRuleBatchContext<'_>,
190190
) -> Result<Vec<CompletedModelRule>, String> {
191+
let shapes = QueryShapes::default();
191192
let evaluate_pending = |pending: PendingModelRule<'_>| CompletedModelRule {
192193
index: pending.index,
193194
identity: pending.identity,
@@ -203,6 +204,7 @@ fn evaluate_pending_models(
203204
selected: context.selected,
204205
request: context.request,
205206
threshold_overrides: context.threshold_overrides,
207+
shapes: &shapes,
206208
})
207209
.and_then(|faults| scoped_faults(context.selected, faults, RuleScope::evaluates_models))
208210
},

‎crates/sqlbuild-rules/src/engine/tests.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ mod evaluation;
88
mod helpers;
99
#[path = "tests/test_incremental_rules.rs"]
1010
mod incremental_rules;
11+
#[path = "tests/test_query_facts.rs"]
12+
mod query_facts;
1113
#[path = "tests/test_scope_facts.rs"]
1214
mod scope_facts;
1315
#[path = "tests/test_speculative_rows.rs"]

0 commit comments

Comments
 (0)