Skip to content

Commit 464e1a6

Browse files
committed
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 efb93e7 commit 464e1a6

2 files changed

Lines changed: 79 additions & 7 deletions

File tree

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,61 @@ fn given_dialect_sql_when_reading_query_facts_then_findings_match_the_authored_s
9494
columns: json!([]),
9595
expected_faults: &[("SQBRDECLARATION102", 1, 35, "3 < 7")],
9696
},
97+
QueryFactsTestCase {
98+
description: "unaliased later union branches keep their cast types",
99+
dialect: "duckdb",
100+
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))",
101+
select: OUTPUT_TYPE_RULE,
102+
enum_columns: &[],
103+
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}]),
104+
expected_faults: &[],
105+
},
106+
QueryFactsTestCase {
107+
description: "an unaliased later union branch reports its own position",
108+
dialect: "duckdb",
109+
query_sql: "SELECT CAST(1 AS INTEGER) AS amount\nUNION ALL\nSELECT CAST(2 AS BIGINT)",
110+
select: OUTPUT_TYPE_RULE,
111+
enum_columns: &[],
112+
columns: json!([{"name": "amount", "type": "INTEGER", "type_proven": true}]),
113+
expected_faults: &[("SQBRCONTRACT105", 3, 13, "outer cast that does not target")],
114+
},
115+
QueryFactsTestCase {
116+
description: "nested parenthesized set operations keep each branch position",
117+
dialect: "duckdb",
118+
query_sql: "(SELECT CAST(1 AS INTEGER) AS amount)\nUNION ALL\n((SELECT 2) EXCEPT (SELECT CAST(3 AS INTEGER)))",
119+
select: OUTPUT_TYPE_RULE,
120+
enum_columns: &[],
121+
columns: json!([{"name": "amount", "type": "INTEGER", "type_proven": true}]),
122+
expected_faults: &[(
123+
"SQBRCONTRACT105",
124+
3,
125+
10,
126+
"calculated without an outer explicit cast",
127+
)],
128+
},
129+
QueryFactsTestCase {
130+
description: "an unaliased branch after a subquery-filtered branch keeps its position",
131+
dialect: "duckdb",
132+
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",
133+
select: OUTPUT_TYPE_RULE,
134+
enum_columns: &[],
135+
columns: json!([{"name": "amount", "type": "INTEGER", "type_proven": true}]),
136+
expected_faults: &[("SQBRCONTRACT105", 3, 13, "outer cast that does not target")],
137+
},
138+
QueryFactsTestCase {
139+
description: "unaliased union branches inside a final CTE keep their positions",
140+
dialect: "duckdb",
141+
query_sql: "WITH final AS (\n SELECT CAST(1 AS INTEGER) AS amount\n UNION ALL\n SELECT 5 + 1\n)\nSELECT * FROM final",
142+
select: OUTPUT_TYPE_RULE,
143+
enum_columns: &[],
144+
columns: json!([{"name": "amount", "type": "INTEGER", "type_proven": true}]),
145+
expected_faults: &[(
146+
"SQBRCONTRACT105",
147+
4,
148+
10,
149+
"calculated without an outer explicit cast",
150+
)],
151+
},
97152
QueryFactsTestCase {
98153
description: "a BigQuery double-quoted string compared with an enum column",
99154
dialect: "bigquery",

‎crates/sqlbuild-rules/src/query_facts/_helpers/facts.rs‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ use polyglot_sql::dialects::Dialect;
1111
use polyglot_sql::expressions::{Identifier, Literal, Select, TableRef, With};
1212
use polyglot_sql::{ComplexityGuardOptions, Expression, ExpressionWalk, ParseOptions};
1313
use sqlbuild_analysis::lineage::main::parser_dialect::parser_dialect;
14-
use std::cell::RefCell;
15-
use std::collections::HashMap;
14+
use std::cell::{Cell, RefCell};
15+
use std::collections::{HashMap, HashSet};
1616
use std::ops::Range;
1717

1818
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<QueryFacts, S
8282
tokens,
8383
dialect,
8484
spans: RefCell::default(),
85+
cursor: Cell::default(),
86+
claimed: RefCell::default(),
8587
};
8688
Ok(builder.query(&statement))
8789
}
@@ -104,6 +106,8 @@ struct Builder<'a> {
104106
selects: Vec<SelectTokens>,
105107
dialect: Dialect,
106108
spans: RefCell<HashMap<*const Expression, Option<ComparisonSpan>>>,
109+
cursor: Cell<usize>,
110+
claimed: RefCell<HashSet<usize>>,
107111
}
108112

109113
struct ComparisonParts<'a> {
@@ -254,12 +258,25 @@ impl Builder<'_> {
254258
}
255259
}
256260

261+
/// The SELECT region around the select's first located part, else the next unclaimed one.
257262
fn select_tokens(&self, select: &Select) -> Option<&SelectTokens> {
258-
let anchor = select_anchor(select, &self.tokens)?;
259-
self.selects
260-
.iter()
261-
.filter(|tokens| tokens.start < anchor && anchor < tokens.end)
262-
.max_by_key(|tokens| tokens.start)
263+
let found = match select_anchor(select, &self.tokens) {
264+
Some(anchor) => self
265+
.selects
266+
.iter()
267+
.filter(|tokens| tokens.start < anchor && anchor < tokens.end)
268+
.max_by_key(|tokens| tokens.start),
269+
None => {
270+
let after = self.cursor.get();
271+
let claimed = self.claimed.borrow();
272+
self.selects
273+
.iter()
274+
.find(|tokens| tokens.start >= after && !claimed.contains(&tokens.start))
275+
}
276+
}?;
277+
self.claimed.borrow_mut().insert(found.start);
278+
self.cursor.set(self.cursor.get().max(found.end));
279+
Some(found)
263280
}
264281

265282
fn item(

0 commit comments

Comments
 (0)