From f7493bbcd2e552a39ee43addadc60183e2dafb94 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sat, 7 Nov 2026 23:28:19 +0000 Subject: [PATCH 1/5] feat(rules): read custom-rule SQL syntax trees from SQLQuack parsers SqlDocument is native and parses with the SQLQuack parser chosen per dialect: the native PostgreSQL parser, and the shared parser for DuckDB, MotherDuck, Snowflake and SQLite. ast() returns a SqlTreeNode tree and replaces polyglot_ast(). Other dialects, syntax errors and statements kept as unparsed text raise RuleUsageError. --- .../main/prove_dynamic_contracts.rs | 12 +- .../main/recover_cte_facts.rs | 14 +- crates/sqlbuild-analysis/src/lib.rs | 2 + .../src/sql_tree/_helpers/clauses.rs | 67 ++++++ .../src/sql_tree/_helpers/mod.rs | 7 + .../src/sql_tree/_helpers/parse.rs | 34 +++ .../src/sql_tree/_helpers/positions.rs | 19 ++ .../sql_tree/_helpers/rules_tree_parser.rs | 23 ++ .../src/sql_tree/_helpers/walk.rs | 102 +++++++++ .../src/sql_tree/constants.rs | 18 ++ .../sqlbuild-analysis/src/sql_tree/errors.rs | 34 +++ .../src/sql_tree/main/mod.rs | 3 + .../src/sql_tree/main/sql_tree_nodes.rs | 27 +++ crates/sqlbuild-analysis/src/sql_tree/mod.rs | 10 + .../sqlbuild-analysis/src/sql_tree/models.rs | 23 ++ .../sqlbuild-analysis/src/sql_tree/tests.rs | 6 + .../src/sql_tree/tests/helpers.rs | 19 ++ .../src/sql_tree/tests/test_sql_tree.rs | 110 +++++++++ .../src/sql_tree/tests/test_types.rs | 19 ++ .../src/worker_stack/main/mod.rs | 1 + .../worker_stack/main/run_on_worker_stack.rs | 17 ++ .../sqlbuild-analysis/src/worker_stack/mod.rs | 3 + .../_helpers/exported_constants/constants.rs | 3 +- .../src/bindings/_helpers/rules/mod.rs | 1 + .../bindings/_helpers/rules/registration.rs | 3 +- .../bindings/_helpers/rules/sql_document.rs | 208 ++++++++++++++++++ python-boundary.toml | 5 - .../_helpers/varied_statistics.py | 7 +- .../rules_benchmark/_helpers/custom_rules.py | 4 +- scripts/rules_benchmark/_helpers/workflow.py | 1 - .../rules/custom-rules/rule-context.md | 56 ++++- src/sqlbuild/_native.pyi | 22 +- .../rule_engine/classes/sql_document.py | 62 ------ src/sqlbuild/rule_engine/classes/sql_facts.py | 3 +- src/sqlbuild/rules/__init__.py | 4 + .../cli/commands/main/rules/_test_types.py | 9 + .../main/rules/test_rule_positions.py | 68 +++++- .../classes/sql_document/_test_types.py | 13 -- .../classes/sql_document/test_sql_document.py | 54 ----- .../rules/custom-rules/rule-context.mdx | 47 +++- 40 files changed, 960 insertions(+), 180 deletions(-) create mode 100644 crates/sqlbuild-analysis/src/sql_tree/_helpers/clauses.rs create mode 100644 crates/sqlbuild-analysis/src/sql_tree/_helpers/mod.rs create mode 100644 crates/sqlbuild-analysis/src/sql_tree/_helpers/parse.rs create mode 100644 crates/sqlbuild-analysis/src/sql_tree/_helpers/positions.rs create mode 100644 crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs create mode 100644 crates/sqlbuild-analysis/src/sql_tree/_helpers/walk.rs create mode 100644 crates/sqlbuild-analysis/src/sql_tree/constants.rs create mode 100644 crates/sqlbuild-analysis/src/sql_tree/errors.rs create mode 100644 crates/sqlbuild-analysis/src/sql_tree/main/mod.rs create mode 100644 crates/sqlbuild-analysis/src/sql_tree/main/sql_tree_nodes.rs create mode 100644 crates/sqlbuild-analysis/src/sql_tree/mod.rs create mode 100644 crates/sqlbuild-analysis/src/sql_tree/models.rs create mode 100644 crates/sqlbuild-analysis/src/sql_tree/tests.rs create mode 100644 crates/sqlbuild-analysis/src/sql_tree/tests/helpers.rs create mode 100644 crates/sqlbuild-analysis/src/sql_tree/tests/test_sql_tree.rs create mode 100644 crates/sqlbuild-analysis/src/sql_tree/tests/test_types.rs create mode 100644 crates/sqlbuild-analysis/src/worker_stack/main/mod.rs create mode 100644 crates/sqlbuild-analysis/src/worker_stack/main/run_on_worker_stack.rs create mode 100644 crates/sqlbuild-analysis/src/worker_stack/mod.rs create mode 100644 crates/sqlbuild-python/src/bindings/_helpers/rules/sql_document.rs delete mode 100644 src/sqlbuild/rule_engine/classes/sql_document.py delete mode 100644 tests/unit/src/sqlbuild/rule_engine/classes/sql_document/_test_types.py delete mode 100644 tests/unit/src/sqlbuild/rule_engine/classes/sql_document/test_sql_document.py diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/main/prove_dynamic_contracts.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/main/prove_dynamic_contracts.rs index 00af66fa58..8618571a45 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/main/prove_dynamic_contracts.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/main/prove_dynamic_contracts.rs @@ -5,15 +5,11 @@ use sqlbuild_core::constants::PANIC_MESSAGE; use crate::assembly::analysis_session::_helpers::dynamic_pivot::pivot_outcomes; use crate::assembly::analysis_session::constants::PIVOT_WORKER_STACK_BYTES; use crate::assembly::analysis_session::models::{PivotBatchRequest, PivotOutcome}; +use crate::worker_stack::main::run_on_worker_stack::run_on_worker_stack; /// Each model's proof in order; an internal failure of any proof fails them all. pub fn prove_dynamic_contracts(request: &PivotBatchRequest) -> Result, String> { - std::thread::scope(|scope| { - std::thread::Builder::new() - .stack_size(PIVOT_WORKER_STACK_BYTES) - .spawn_scoped(scope, || pivot_outcomes(&request.models, &request.tables)) - .map_err(|error| error.to_string())? - .join() - .map_err(|_| PANIC_MESSAGE.to_owned())? - }) + run_on_worker_stack(PIVOT_WORKER_STACK_BYTES, PANIC_MESSAGE, || { + pivot_outcomes(&request.models, &request.tables) + })? } diff --git a/crates/sqlbuild-analysis/src/assembly/analysis_session/main/recover_cte_facts.rs b/crates/sqlbuild-analysis/src/assembly/analysis_session/main/recover_cte_facts.rs index 92b9f05478..85849e947b 100644 --- a/crates/sqlbuild-analysis/src/assembly/analysis_session/main/recover_cte_facts.rs +++ b/crates/sqlbuild-analysis/src/assembly/analysis_session/main/recover_cte_facts.rs @@ -5,17 +5,15 @@ use crate::assembly::analysis_session::_helpers::cte_facts::{ }; use crate::assembly::analysis_session::constants::CTE_FACT_WORKER_STACK_BYTES; use crate::assembly::analysis_session::models::{CteFactRequest, CteFacts}; +use crate::worker_stack::main::run_on_worker_stack::run_on_worker_stack; /// The facts enrichment recovers for the query, or why they are left to Python. pub fn recover_cte_facts(request: &CteFactRequest) -> Result { - std::thread::scope(|scope| { - std::thread::Builder::new() - .stack_size(CTE_FACT_WORKER_STACK_BYTES) - .spawn_scoped(scope, || recovered_facts(request)) - .map_err(|error| error.to_string())? - .join() - .map_err(|_| "native CTE fact recovery panicked".to_owned())? - }) + run_on_worker_stack( + CTE_FACT_WORKER_STACK_BYTES, + "native CTE fact recovery panicked", + || recovered_facts(request), + )? } fn recovered_facts(request: &CteFactRequest) -> Result { diff --git a/crates/sqlbuild-analysis/src/lib.rs b/crates/sqlbuild-analysis/src/lib.rs index 173c375e46..29a71521e9 100644 --- a/crates/sqlbuild-analysis/src/lib.rs +++ b/crates/sqlbuild-analysis/src/lib.rs @@ -47,5 +47,7 @@ pub mod snowflake_cost; pub mod sql_macros; pub mod sql_test_attachment; pub mod sql_tokens; +pub mod sql_tree; pub mod target_paths; pub mod type_system; +pub mod worker_stack; diff --git a/crates/sqlbuild-analysis/src/sql_tree/_helpers/clauses.rs b/crates/sqlbuild-analysis/src/sql_tree/_helpers/clauses.rs new file mode 100644 index 0000000000..9fa047167c --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_tree/_helpers/clauses.rs @@ -0,0 +1,67 @@ +//! Join and CTE clauses, which SQLQuack keeps as statement fields rather than expressions. + +use sqlquack_syntax::Expression; +use sqlquack_syntax::expressions::queries::models::{Cte, Join, JoinKind, With}; +use sqlquack_syntax::tokens::models::Span; + +use crate::sql_tree::constants::{CTE_KIND, JOIN_KIND}; + +/// One clause of a statement: its kind, its span, and the expressions it owns. +pub(crate) struct Clause<'ast> { + pub(crate) kind: &'static str, + pub(crate) span: Option, + pub(crate) children: Vec<&'ast Expression>, +} + +/// The CTEs and explicit joins `expression` holds directly, CTEs first. +pub(crate) fn clauses(expression: &Expression) -> Vec> { + let (with, joins): (Option<&With>, Vec<&[Join]>) = match expression { + Expression::Select(select) => (select.with.as_ref(), vec![select.joins.as_slice()]), + Expression::Union(union) => (union.with.as_ref(), Vec::new()), + Expression::Intersect(intersect) => (intersect.with.as_ref(), Vec::new()), + Expression::Except(except) => (except.with.as_ref(), Vec::new()), + Expression::Insert(insert) => (insert.with.as_ref(), Vec::new()), + Expression::Update(update) => ( + update.with.as_ref(), + vec![update.table_joins.as_slice(), update.from_joins.as_slice()], + ), + Expression::Delete(delete) => (delete.with.as_ref(), vec![delete.joins.as_slice()]), + Expression::JoinedTable(joined) => (None, vec![joined.joins.as_slice()]), + _ => (None, Vec::new()), + }; + let ctes = with + .into_iter() + .flat_map(|with| with.ctes.iter()) + .map(cte_clause); + let joins = joins + .into_iter() + .flatten() + .filter(|join| !is_comma_item(join)) + .map(join_clause); + ctes.chain(joins).collect() +} + +fn cte_clause(cte: &Cte) -> Clause<'_> { + Clause { + kind: CTE_KIND, + span: cte.span.0, + children: vec![&cte.this], + } +} + +fn join_clause(join: &Join) -> Clause<'_> { + let mut children: Vec<&Expression> = vec![&join.this]; + children.extend(join.on.as_ref()); + children.extend(join.match_condition.as_ref()); + children.extend(join.pivots.iter()); + Clause { + kind: JOIN_KIND, + span: join.span.0, + children, + } +} + +/// A comma-separated FROM item after an explicit join, which the shared parser keeps as a join. +fn is_comma_item(join: &Join) -> bool { + join.kind == JoinKind::Implicit +} diff --git a/crates/sqlbuild-analysis/src/sql_tree/_helpers/mod.rs b/crates/sqlbuild-analysis/src/sql_tree/_helpers/mod.rs new file mode 100644 index 0000000000..0598c2c140 --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_tree/_helpers/mod.rs @@ -0,0 +1,7 @@ +//! Parser selection, parsing, the tree walk, clause nodes and source positions. + +pub(crate) mod clauses; +pub(crate) mod parse; +pub(crate) mod positions; +pub(crate) mod rules_tree_parser; +pub(crate) mod walk; diff --git a/crates/sqlbuild-analysis/src/sql_tree/_helpers/parse.rs b/crates/sqlbuild-analysis/src/sql_tree/_helpers/parse.rs new file mode 100644 index 0000000000..1fc349a628 --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_tree/_helpers/parse.rs @@ -0,0 +1,34 @@ +//! Parse SQL into statements with node spans. + +use sqlquack_syntax::{CallMode, Expression, ParseOptions}; + +use crate::sql_tree::errors::SqlTreeError; +use crate::sql_tree::models::TreeParser; + +/// The statements of `sql` under `parser`; a non-syntax PostgreSQL failure or a skipped statement is unanalysable. +pub(crate) fn parse(sql: &str, parser: TreeParser) -> Result, SqlTreeError> { + match parser { + TreeParser::Shared(dialect) => sqlquack_syntax::parse_statements_with( + sql, + dialect, + &ParseOptions::default(), + CallMode::FaithfulLocated, + ) + .map_err(|error| SqlTreeError::Parse(error.to_string())), + TreeParser::Postgres => { + let parsed = sqlquack_syntax_postgres::parse_strict(sql).map_err(|failure| { + match failure.parse_error() { + Some(error) if error.syntax => SqlTreeError::Parse(error.message.clone()), + Some(error) => SqlTreeError::Unanalysable(error.message.clone()), + None => SqlTreeError::Parse("SQL has text outside any token".to_owned()), + } + })?; + if !parsed.gaps.is_empty() { + return Err(SqlTreeError::Unanalysable( + "a statement uses syntax the PostgreSQL parser skips".to_owned(), + )); + } + Ok(parsed.statements) + } + } +} diff --git a/crates/sqlbuild-analysis/src/sql_tree/_helpers/positions.rs b/crates/sqlbuild-analysis/src/sql_tree/_helpers/positions.rs new file mode 100644 index 0000000000..b149e1ad49 --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_tree/_helpers/positions.rs @@ -0,0 +1,19 @@ +//! Character offsets as 1-based lines and columns. + +/// The character offset at which each line of `sql` starts. +pub(crate) fn line_starts(sql: &str) -> Vec { + let mut starts: Vec = vec![0]; + for (offset, character) in sql.chars().enumerate() { + if character == '\n' { + starts.push(offset + 1); + } + } + starts +} + +/// The 1-based line and column of the character at `offset`. +pub(crate) fn line_column(starts: &[usize], offset: usize) -> (usize, usize) { + let line: usize = starts.partition_point(|start| *start <= offset); + let start: usize = starts.get(line.saturating_sub(1)).copied().unwrap_or(0); + (line.max(1), offset - start + 1) +} diff --git a/crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs b/crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs new file mode 100644 index 0000000000..4a28b35f25 --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs @@ -0,0 +1,23 @@ +//! `rules_tree_parser`: the parser that reads a dialect's SQL for rules. + +use sqlquack_syntax::Dialect; + +use crate::sql_tree::constants::{ + DUCKDB_DIALECTS, POSTGRES_DIALECTS, SNOWFLAKE_DIALECT, SQLITE_DIALECT, +}; +use crate::sql_tree::models::TreeParser; + +/// The parser for `dialect`, or `None` when SQLQuack parses none of it; becomes `analysis_route` once a native module registers. +pub(crate) fn rules_tree_parser(dialect: &str) -> Option { + if POSTGRES_DIALECTS.contains(&dialect) { + return Some(TreeParser::Postgres); + } + if DUCKDB_DIALECTS.contains(&dialect) { + return Some(TreeParser::Shared(Dialect::DuckDB)); + } + match dialect { + SNOWFLAKE_DIALECT => Some(TreeParser::Shared(Dialect::Snowflake)), + SQLITE_DIALECT => Some(TreeParser::Shared(Dialect::SQLite)), + _ => None, + } +} diff --git a/crates/sqlbuild-analysis/src/sql_tree/_helpers/walk.rs b/crates/sqlbuild-analysis/src/sql_tree/_helpers/walk.rs new file mode 100644 index 0000000000..2d59d234d8 --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_tree/_helpers/walk.rs @@ -0,0 +1,102 @@ +//! The iterative pre-order walk that turns parsed statements into rule tree nodes. + +use std::collections::HashMap; + +use sqlquack_syntax::tokens::models::Span; +use sqlquack_syntax::{Expression, ExpressionWalk}; + +use crate::sql_tree::_helpers::clauses::clauses; +use crate::sql_tree::_helpers::positions::line_column; +use crate::sql_tree::constants::OPAQUE_KINDS; +use crate::sql_tree::errors::SqlTreeError; +use crate::sql_tree::models::SqlTreeNode; + +/// Child expressions mapped to the index of the clause node that owns them. +type Owners = HashMap<*const Expression, usize>; + +/// The nodes of `statements` in pre-order, walked with an explicit stack so depth cannot overflow. +pub(crate) fn walk( + statements: &[Expression], + starts: &[usize], +) -> Result, SqlTreeError> { + let mut nodes: Vec = Vec::new(); + let mut stack: Vec<(&Expression, Option)> = statements + .iter() + .rev() + .map(|statement| (statement, None)) + .collect(); + while let Some((expression, parent)) = stack.pop() { + let index: usize = nodes.len(); + let node: SqlTreeNode = expression_node(&nodes, expression, parent, starts)?; + let (clause_nodes, owners) = clause_nodes(expression, &node, index, starts); + nodes.push(node); + nodes.extend(clause_nodes); + for child in expression.children().into_iter().rev() { + let owner: usize = owners + .get(&std::ptr::from_ref(child)) + .copied() + .unwrap_or(index); + stack.push((child, Some(owner))); + } + } + Ok(nodes) +} + +/// `expression`'s node, at its own start or else at its parent's. +fn expression_node( + nodes: &[SqlTreeNode], + expression: &Expression, + parent: Option, + starts: &[usize], +) -> Result { + let kind: &'static str = expression.variant_name(); + if OPAQUE_KINDS.contains(&kind) { + return Err(SqlTreeError::Unanalysable( + "a statement uses syntax the parser keeps as unparsed text".to_owned(), + )); + } + let inherited = parent.map_or((1, 1), |parent| (nodes[parent].line, nodes[parent].column)); + let (line, column) = + own_span(expression).map_or(inherited, |span| line_column(starts, span.start)); + Ok(SqlTreeNode { + kind, + line, + column, + parent, + }) +} + +/// The clause nodes `expression` holds, numbered after `index`, and the children each one owns. +fn clause_nodes( + expression: &Expression, + node: &SqlTreeNode, + index: usize, + starts: &[usize], +) -> (Vec, Owners) { + let mut built: Vec = Vec::new(); + let mut owners: Owners = HashMap::new(); + for clause in clauses(expression) { + let clause_index: usize = index + 1 + built.len(); + let (line, column) = clause.span.map_or((node.line, node.column), |span| { + line_column(starts, span.start) + }); + built.push(SqlTreeNode { + kind: clause.kind, + line, + column, + parent: Some(index), + }); + for child in clause.children { + let _ = owners.insert(std::ptr::from_ref(child), clause_index); + } + } + (built, owners) +} + +/// The node's span; a qualified star without one starts at its qualifier. +fn own_span(expression: &Expression) -> Option { + expression.source_span().or_else(|| match expression { + Expression::Star(star) => star.table.as_ref().and_then(|table| table.span), + _ => None, + }) +} diff --git a/crates/sqlbuild-analysis/src/sql_tree/constants.rs b/crates/sqlbuild-analysis/src/sql_tree/constants.rs new file mode 100644 index 0000000000..c25d766a11 --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_tree/constants.rs @@ -0,0 +1,18 @@ +//! Node kinds the rule tree adds to SQLQuack's expression variants, and kinds it refuses. + +/// The kind of a join clause, which SQLQuack keeps as a field of its statement. +pub(crate) const JOIN_KIND: &str = "join"; +/// The kind of a common table expression, which SQLQuack keeps as a field of its statement. +pub(crate) const CTE_KIND: &str = "cte"; +/// Kinds that hold a statement the parser kept as unparsed text, so no rule can see inside it. +pub(crate) const OPAQUE_KINDS: [&str; 2] = ["command", "raw"]; +/// Dialect names read with the shared DuckDB grammar; `generic` matches the fingerprint tokenizer. +pub(crate) const DUCKDB_DIALECTS: [&str; 3] = ["duckdb", "motherduck", "generic"]; +/// Dialect names read with the native PostgreSQL parser. +pub(crate) const POSTGRES_DIALECTS: [&str; 2] = ["postgres", "postgresql"]; +/// The Snowflake dialect name. +pub(crate) const SNOWFLAKE_DIALECT: &str = "snowflake"; +/// The SQLite dialect name. +pub(crate) const SQLITE_DIALECT: &str = "sqlite"; +/// Stack for the parse-and-walk worker, so deep SQL reaches the parser's depth limit, not a stack overflow. +pub(crate) const TREE_WORKER_STACK_BYTES: usize = 16 * 1024 * 1024; diff --git a/crates/sqlbuild-analysis/src/sql_tree/errors.rs b/crates/sqlbuild-analysis/src/sql_tree/errors.rs new file mode 100644 index 0000000000..20f686eb92 --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_tree/errors.rs @@ -0,0 +1,34 @@ +//! Why a rule cannot have a SQL syntax tree. + +use std::fmt::{self, Display}; + +/// A dialect SQLQuack does not parse, SQL it rejects, or SQL it read only as opaque text. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum SqlTreeError { + /// No SQLQuack parser reads this dialect yet. + UnavailableDialect(String), + /// The parser read the SQL but kept part of it as unparsed text or refused its depth. + Unanalysable(String), + /// The parser rejected the SQL. + Parse(String), + /// The parse worker could not start or panicked. + Worker(String), +} + +impl Display for SqlTreeError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::UnavailableDialect(dialect) => write!( + formatter, + "SQL syntax trees are unavailable for the {dialect} dialect: SQLQuack does not parse it yet" + ), + Self::Unanalysable(reason) => { + write!(formatter, "SQL syntax tree is unavailable: {reason}") + } + Self::Parse(message) => write!(formatter, "SQL could not be parsed: {message}"), + Self::Worker(message) => write!(formatter, "SQL syntax tree worker failed: {message}"), + } + } +} + +impl std::error::Error for SqlTreeError {} diff --git a/crates/sqlbuild-analysis/src/sql_tree/main/mod.rs b/crates/sqlbuild-analysis/src/sql_tree/main/mod.rs new file mode 100644 index 0000000000..0c71a2f7d5 --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_tree/main/mod.rs @@ -0,0 +1,3 @@ +//! Entry points. + +pub mod sql_tree_nodes; diff --git a/crates/sqlbuild-analysis/src/sql_tree/main/sql_tree_nodes.rs b/crates/sqlbuild-analysis/src/sql_tree/main/sql_tree_nodes.rs new file mode 100644 index 0000000000..de5e7a8b94 --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_tree/main/sql_tree_nodes.rs @@ -0,0 +1,27 @@ +//! `sql_tree_nodes`: a rule's SQL syntax tree as nodes in pre-order. + +use sqlquack_syntax::Expression; + +use crate::sql_tree::_helpers::parse::parse; +use crate::sql_tree::_helpers::positions::line_starts; +use crate::sql_tree::_helpers::rules_tree_parser::rules_tree_parser; +use crate::sql_tree::_helpers::walk::walk; +use crate::sql_tree::constants::TREE_WORKER_STACK_BYTES; +use crate::sql_tree::errors::SqlTreeError; +use crate::sql_tree::models::SqlTreeNode; +use crate::worker_stack::main::run_on_worker_stack::run_on_worker_stack; + +/// Every node of `sql` parsed in `dialect`, in pre-order, joins and CTEs owning their expressions. +pub fn sql_tree_nodes(sql: &str, dialect: &str) -> Result, SqlTreeError> { + run_on_worker_stack(TREE_WORKER_STACK_BYTES, "the parser panicked", || { + tree_nodes(sql, dialect) + }) + .map_err(SqlTreeError::Worker)? +} + +fn tree_nodes(sql: &str, dialect: &str) -> Result, SqlTreeError> { + let parser = rules_tree_parser(dialect) + .ok_or_else(|| SqlTreeError::UnavailableDialect(dialect.to_owned()))?; + let statements: Vec = parse(sql, parser)?; + walk(&statements, &line_starts(sql)) +} diff --git a/crates/sqlbuild-analysis/src/sql_tree/mod.rs b/crates/sqlbuild-analysis/src/sql_tree/mod.rs new file mode 100644 index 0000000000..264e63cb56 --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_tree/mod.rs @@ -0,0 +1,10 @@ +//! SQL syntax trees for custom rules, parsed by the SQLQuack parser of each dialect. + +pub(crate) mod _helpers; +mod constants; +pub mod errors; +pub mod main; +pub mod models; + +#[cfg(test)] +mod tests; diff --git a/crates/sqlbuild-analysis/src/sql_tree/models.rs b/crates/sqlbuild-analysis/src/sql_tree/models.rs new file mode 100644 index 0000000000..22964a4c2d --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_tree/models.rs @@ -0,0 +1,23 @@ +//! Nodes of a rule's SQL syntax tree. + +/// One node: its SQLQuack kind, where it starts, and its parent's index in pre-order. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct SqlTreeNode { + /// SQLQuack's snake_case node kind, such as `select`, `subquery`, `star`, `join` or `cte`. + pub kind: &'static str, + /// 1-based line of the node's first character; a node without a span takes its parent's. + pub line: usize, + /// 1-based column, in characters, of the node's first character. + pub column: usize, + /// The parent's index; `None` for a statement. + pub parent: Option, +} + +/// The SQLQuack parser that reads a dialect's SQL for rules. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum TreeParser { + /// `sqlquack_syntax_postgres`'s native PostgreSQL parser. + Postgres, + /// `sqlquack_syntax`'s shared parser for one dialect, recording node spans. + Shared(sqlquack_syntax::Dialect), +} diff --git a/crates/sqlbuild-analysis/src/sql_tree/tests.rs b/crates/sqlbuild-analysis/src/sql_tree/tests.rs new file mode 100644 index 0000000000..e22f4b535c --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_tree/tests.rs @@ -0,0 +1,6 @@ +#[path = "tests/helpers.rs"] +mod helpers; +#[path = "tests/test_sql_tree.rs"] +mod test_sql_tree; +#[path = "tests/test_types.rs"] +mod test_types; diff --git a/crates/sqlbuild-analysis/src/sql_tree/tests/helpers.rs b/crates/sqlbuild-analysis/src/sql_tree/tests/helpers.rs new file mode 100644 index 0000000000..8338f0309f --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_tree/tests/helpers.rs @@ -0,0 +1,19 @@ +use crate::sql_tree::models::SqlTreeNode; + +const SHAPE_KINDS: [&str; 5] = ["select", "cte", "join", "subquery", "star"]; + +/// The listed kinds' nodes with their parent's kind, sorted by position, since child order is SQLQuack's. +pub(crate) fn shape( + nodes: &[SqlTreeNode], +) -> Vec<(&'static str, usize, usize, Option<&'static str>)> { + let mut shape: Vec<_> = nodes + .iter() + .filter(|node| SHAPE_KINDS.contains(&node.kind)) + .map(|node| { + let parent = node.parent.map(|parent| nodes[parent].kind); + (node.kind, node.line, node.column, parent) + }) + .collect(); + shape.sort_by_key(|(kind, line, column, _)| (*line, *column, *kind)); + shape +} diff --git a/crates/sqlbuild-analysis/src/sql_tree/tests/test_sql_tree.rs b/crates/sqlbuild-analysis/src/sql_tree/tests/test_sql_tree.rs new file mode 100644 index 0000000000..7b689821bc --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_tree/tests/test_sql_tree.rs @@ -0,0 +1,110 @@ +use crate::sql_tree::errors::SqlTreeError; +use crate::sql_tree::main::sql_tree_nodes::sql_tree_nodes; +use crate::sql_tree::tests::helpers::shape; +use crate::sql_tree::tests::test_types::{TreeFailureTestCase, TreeShapeTestCase}; + +const CTE_AND_JOIN_SQL: &str = + "WITH x AS (SELECT 1 AS id)\nSELECT *\n FROM x JOIN (SELECT 2 AS id) s ON x.id = s.id"; + +#[test] +fn given_sql_when_building_a_rule_tree_then_clauses_own_their_queries_at_their_source_positions() { + let test_cases = [ + TreeShapeTestCase { + description: "native PostgreSQL records every clause, star included", + dialect: "postgres", + sql: CTE_AND_JOIN_SQL, + expected_nodes: vec![ + ("select", 1, 1, None), + ("cte", 1, 6, Some("select")), + ("select", 1, 12, Some("cte")), + ("star", 2, 8, Some("select")), + ("join", 3, 10, Some("select")), + ("subquery", 3, 15, Some("join")), + ("select", 3, 16, Some("subquery")), + ], + }, + TreeShapeTestCase { + description: "the shared DuckDB parser leaves a bare star and a subquery's select at their parent's position", + dialect: "duckdb", + sql: CTE_AND_JOIN_SQL, + expected_nodes: vec![ + ("select", 1, 1, None), + ("star", 1, 1, Some("select")), + ("cte", 1, 6, Some("select")), + ("select", 1, 12, Some("cte")), + ("join", 3, 10, Some("select")), + ("select", 3, 15, Some("subquery")), + ("subquery", 3, 15, Some("join")), + ], + }, + TreeShapeTestCase { + description: "a qualified star starts at its qualifier; a scalar subquery's select at the subquery", + dialect: "snowflake", + sql: "SELECT o.*,\n (SELECT MAX(id) FROM orders) AS top_id\nFROM orders o", + expected_nodes: vec![ + ("select", 1, 1, None), + ("star", 1, 8, Some("select")), + ("select", 2, 3, Some("subquery")), + ("subquery", 2, 3, Some("alias")), + ], + }, + TreeShapeTestCase { + description: "a comma-separated FROM item after an explicit join is not a join", + dialect: "duckdb", + sql: "SELECT 1\nFROM a JOIN b ON a.id = b.id, (SELECT 2) c", + expected_nodes: vec![ + ("select", 1, 1, None), + ("join", 2, 8, Some("select")), + ("select", 2, 31, Some("subquery")), + ("subquery", 2, 31, Some("select")), + ], + }, + ]; + for test_case in test_cases { + let nodes = sql_tree_nodes(test_case.sql, test_case.dialect); + assert_eq!( + nodes.as_deref().map(shape), + Ok(test_case.expected_nodes), + "{}", + test_case.description + ); + } +} + +#[test] +fn given_sql_without_a_full_tree_when_building_a_rule_tree_then_the_reason_is_reported() { + let test_cases = [ + TreeFailureTestCase { + description: "a dialect SQLQuack does not parse", + dialect: "bigquery", + sql: "SELECT 1".to_owned(), + expected_error: |error| matches!(error, SqlTreeError::UnavailableDialect(name) if name == "bigquery"), + }, + TreeFailureTestCase { + description: "a PostgreSQL syntax error", + dialect: "postgres", + sql: "SELECT FROM WHERE".to_owned(), + expected_error: |error| matches!(error, SqlTreeError::Parse(_)), + }, + TreeFailureTestCase { + description: "PostgreSQL's stack depth limit is unanalysable, not a syntax error", + dialect: "postgres", + sql: format!("SELECT {}1{}", "(".repeat(10_000), ")".repeat(10_000)), + expected_error: |error| matches!(error, SqlTreeError::Unanalysable(_)), + }, + TreeFailureTestCase { + description: "a shared-parser syntax error", + dialect: "sqlite", + sql: "SELECT FROM WHERE (".to_owned(), + expected_error: |error| matches!(error, SqlTreeError::Parse(_)), + }, + ]; + for test_case in test_cases { + let error = sql_tree_nodes(&test_case.sql, test_case.dialect).err(); + assert!( + error.as_ref().is_some_and(test_case.expected_error), + "{}: {error:?}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-analysis/src/sql_tree/tests/test_types.rs b/crates/sqlbuild-analysis/src/sql_tree/tests/test_types.rs new file mode 100644 index 0000000000..ae152e0f0e --- /dev/null +++ b/crates/sqlbuild-analysis/src/sql_tree/tests/test_types.rs @@ -0,0 +1,19 @@ +//! Test case types for rule SQL trees. + +use crate::sql_tree::errors::SqlTreeError; + +/// SQL, its dialect, and the `(kind, line, column, parent kind)` of every node of the listed kinds. +pub(crate) struct TreeShapeTestCase { + pub(crate) description: &'static str, + pub(crate) dialect: &'static str, + pub(crate) sql: &'static str, + pub(crate) expected_nodes: Vec<(&'static str, usize, usize, Option<&'static str>)>, +} + +/// SQL that has no tree, and why. +pub(crate) struct TreeFailureTestCase { + pub(crate) description: &'static str, + pub(crate) dialect: &'static str, + pub(crate) sql: String, + pub(crate) expected_error: fn(&SqlTreeError) -> bool, +} diff --git a/crates/sqlbuild-analysis/src/worker_stack/main/mod.rs b/crates/sqlbuild-analysis/src/worker_stack/main/mod.rs new file mode 100644 index 0000000000..87b7ed0a5f --- /dev/null +++ b/crates/sqlbuild-analysis/src/worker_stack/main/mod.rs @@ -0,0 +1 @@ +pub mod run_on_worker_stack; diff --git a/crates/sqlbuild-analysis/src/worker_stack/main/run_on_worker_stack.rs b/crates/sqlbuild-analysis/src/worker_stack/main/run_on_worker_stack.rs new file mode 100644 index 0000000000..3a1fb0ef0c --- /dev/null +++ b/crates/sqlbuild-analysis/src/worker_stack/main/run_on_worker_stack.rs @@ -0,0 +1,17 @@ +//! `run_on_worker_stack`: one call's work on a scoped thread with its own stack size. + +/// `work`'s result from a scoped thread with `stack_bytes` of stack, or why the thread failed. +pub fn run_on_worker_stack( + stack_bytes: usize, + panic_message: &str, + work: impl FnOnce() -> T + Send, +) -> Result { + std::thread::scope(|scope| { + std::thread::Builder::new() + .stack_size(stack_bytes) + .spawn_scoped(scope, work) + .map_err(|error| error.to_string())? + .join() + .map_err(|_| panic_message.to_owned()) + }) +} diff --git a/crates/sqlbuild-analysis/src/worker_stack/mod.rs b/crates/sqlbuild-analysis/src/worker_stack/mod.rs new file mode 100644 index 0000000000..e54cd18ede --- /dev/null +++ b/crates/sqlbuild-analysis/src/worker_stack/mod.rs @@ -0,0 +1,3 @@ +//! Running deep-recursion work on a scoped thread with a larger stack. + +pub mod main; diff --git a/crates/sqlbuild-python/src/bindings/_helpers/exported_constants/constants.rs b/crates/sqlbuild-python/src/bindings/_helpers/exported_constants/constants.rs index 5366b69604..0b404933d4 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/exported_constants/constants.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/exported_constants/constants.rs @@ -18,7 +18,7 @@ use sqlbuild_core::command_output::constants::{ INVOCATION_CONTEXT_ENV, }; use sqlbuild_core::constants::{ - COLUMN_MIGRATION_TABLE_NAME, CONSUMPTION_TABLE_NAME, CTE_AST_KIND, FINGERPRINT_TABLE_NAME, + COLUMN_MIGRATION_TABLE_NAME, CONSUMPTION_TABLE_NAME, FINGERPRINT_TABLE_NAME, MIGRATION_TABLE_NAME, OLD_NAME_VIEW_TABLE_NAME, PROJECT_CONFIG_FILENAME, PYTHON_INIT_MODULE_STEM, PYTHON_NODE_ROOT, SETTINGS_SECTION, SOURCE_FRESHNESS_TABLE_NAME, STAGED_PROMOTION_MODE, TABLE_PROMOTION_MODE_SETTING_KEY, ZERO_DAY_DURATION, @@ -110,7 +110,6 @@ pub(crate) const EXPORTED_TEXTS: &[(&str, &str)] = &[ ("CONTRACT_ENFORCED", CONTRACT_ENFORCED), ("LAUNCH_RECORD_OPTION", LAUNCH_RECORD_OPTION), ("ZERO_DAY_CURSOR_DURATION", ZERO_DAY_DURATION), - ("AST_CTE_KIND", CTE_AST_KIND), ("SQL_TEXT_FIELD", SQL_TEXT_FIELD), ("SQL_DIGEST_FIELD", SQL_DIGEST_FIELD), ("SQL_PRIVATE_RECORD_FIELD", SQL_PRIVATE_RECORD_FIELD), diff --git a/crates/sqlbuild-python/src/bindings/_helpers/rules/mod.rs b/crates/sqlbuild-python/src/bindings/_helpers/rules/mod.rs index ed6a942378..12d300d1df 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/rules/mod.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/rules/mod.rs @@ -46,5 +46,6 @@ pub(crate) mod rules_config; pub(crate) mod run_lint; pub(crate) mod selection; pub(crate) mod skills; +pub(crate) mod sql_document; pub(crate) mod sql_rules; pub(crate) mod sql_test_objects; diff --git a/crates/sqlbuild-python/src/bindings/_helpers/rules/registration.rs b/crates/sqlbuild-python/src/bindings/_helpers/rules/registration.rs index d6685cc3bd..27d01ff28b 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/rules/registration.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/rules/registration.rs @@ -7,11 +7,12 @@ use pyo3::{types::PyModuleMethods, wrap_pyfunction}; use crate::bindings::_helpers::rules::{ catalogue, custom_rule_cache, custom_rule_evidence, dsl_headers, early_sql_lint, evaluation, guidance, lint, lint_expansion, lint_sql_bodies, lint_text, results, rule_api, rule_cache, - rule_plan, rule_run, rules_config, run_lint, selection, sql_rules, + rule_plan, rule_run, rules_config, run_lint, selection, sql_document, sql_rules, }; pub(crate) fn register(module: &Bound<'_, PyModule>) -> PyResult<()> { rule_api::register(module)?; + sql_document::register(module)?; crate::bindings::_helpers::rules::project_tree_queries::register(module)?; module.add_function(wrap_pyfunction!( crate::bindings::_helpers::rules::lint_headers::lint_file_role, diff --git a/crates/sqlbuild-python/src/bindings/_helpers/rules/sql_document.rs b/crates/sqlbuild-python/src/bindings/_helpers/rules/sql_document.rs new file mode 100644 index 0000000000..f9a49031ec --- /dev/null +++ b/crates/sqlbuild-python/src/bindings/_helpers/rules/sql_document.rs @@ -0,0 +1,208 @@ +//! Custom rules' SQL documents: the SQLQuack syntax tree of one SQL text and its common structures. + +use pyo3::prelude::{Bound, Py, PyAny, PyErr, PyModule, PyModuleMethods, PyResult, Python}; +use pyo3::sync::PyOnceLock; +use pyo3::types::PyTuple; +use pyo3::{IntoPyObject, pyclass, pymethods}; +use sqlbuild_analysis::sql_tree::errors::SqlTreeError; +use sqlbuild_analysis::sql_tree::main::sql_tree_nodes::sql_tree_nodes; +use sqlbuild_analysis::sql_tree::models::SqlTreeNode as TreeNode; + +use crate::bindings::_helpers::records::construct::{new_annotated_error, new_record_with}; +use crate::bindings::_helpers::records::ids::{ErrorId, RecordId}; + +/// The kind of the root node that holds a document's statements. +const DOCUMENT_KIND: &str = "document"; +/// The kind of an aggregate call, whose star argument, as in `COUNT(*)`, expands no columns. +const AGGREGATE_KIND: &str = "aggregate_function"; + +/// One node of a rule's SQL syntax tree. +#[pyclass(module = "sqlbuild._native", frozen)] +pub(crate) struct SqlTreeNode { + #[pyo3(get)] + kind: &'static str, + #[pyo3(get)] + line: usize, + #[pyo3(get)] + column: usize, + children: Vec>, +} + +#[pymethods] +impl SqlTreeNode { + /// The node's direct children, in SQLQuack's traversal order. + #[getter] + fn children<'py>(&self, py: Python<'py>) -> PyResult> { + PyTuple::new(py, self.children.iter().map(|child| child.clone_ref(py))) + } + + /// This node and every descendant, in pre-order. + fn walk(slf: Bound<'_, Self>) -> PyResult>> { + let py: Python<'_> = slf.py(); + let mut walked: Vec> = Vec::new(); + let mut stack: Vec> = vec![slf.unbind()]; + while let Some(node) = stack.pop() { + stack.extend( + node.get() + .children + .iter() + .rev() + .map(|child| child.clone_ref(py)), + ); + walked.push(node); + } + Ok(walked) + } + + fn __repr__(&self) -> String { + format!( + "SqlTreeNode(kind={:?}, line={}, column={})", + self.kind, self.line, self.column + ) + } +} + +/// One SQL text a custom rule reads, parsed on first use by its dialect's SQLQuack parser. +#[pyclass(module = "sqlbuild._native", frozen)] +pub(crate) struct SqlDocument { + #[pyo3(get)] + source: String, + dialect: String, + nodes: PyOnceLock>, + tree: PyOnceLock>, +} + +#[pymethods] +impl SqlDocument { + #[new] + #[pyo3(signature = (*, source, dialect))] + fn new(source: String, dialect: String) -> Self { + Self { + source, + dialect, + nodes: PyOnceLock::new(), + tree: PyOnceLock::new(), + } + } + + /// The syntax tree: a `document` node whose children are the statements. + fn ast(&self, py: Python<'_>) -> PyResult> { + let tree = self.tree.get_or_try_init(py, || { + let nodes: &[TreeNode] = self.parsed(py)?; + python_tree(py, nodes) + })?; + Ok(tree.clone_ref(py)) + } + + /// Every node of `kind`, compared without case, as a stable source location. + #[pyo3(signature = (*, kind))] + fn nodes<'py>(&self, py: Python<'py>, kind: &str) -> PyResult> { + self.source_nodes(py, kind, |_, _| true) + } + + /// Every common table expression. + fn ctes<'py>(&self, py: Python<'py>) -> PyResult> { + self.nodes(py, "CTE") + } + + /// Every join clause. + fn joins<'py>(&self, py: Python<'py>) -> PyResult> { + self.nodes(py, "Join") + } + + /// Every parenthesized subquery. + fn subqueries<'py>(&self, py: Python<'py>) -> PyResult> { + self.nodes(py, "Subquery") + } + + /// Every projected star, bare or qualified; a star a function call takes, such as `COUNT(*)`, is no projection. + fn star_projections<'py>(&self, py: Python<'py>) -> PyResult> { + self.source_nodes(py, "Star", |nodes, node| { + node.parent + .is_none_or(|parent| nodes[parent].kind != AGGREGATE_KIND) + }) + } +} + +impl SqlDocument { + /// The nodes of `kind` that `keep` accepts, labelled `kind`, as `SqlNode` locations. + fn source_nodes<'py>( + &self, + py: Python<'py>, + kind: &str, + keep: impl Fn(&[TreeNode], &TreeNode) -> bool, + ) -> PyResult> { + let wanted: String = kind.to_lowercase(); + let nodes: &[TreeNode] = self.parsed(py)?; + let mut found: Vec> = Vec::new(); + for node in nodes + .iter() + .filter(|node| node.kind == wanted && keep(nodes, node)) + { + found.push(new_record_with( + py, + RecordId::SqlNode, + &[ + ("kind", kind.into_pyobject(py)?.into_any()), + ("line", node.line.into_pyobject(py)?.into_any()), + ("column", node.column.into_pyobject(py)?.into_any()), + ], + )?); + } + PyTuple::new(py, found) + } + + /// The parsed nodes in pre-order; a failure raises `RuleUsageError` each time it is read. + fn parsed(&self, py: Python<'_>) -> PyResult<&[TreeNode]> { + self.nodes + .get_or_try_init(py, || { + py.detach(|| sql_tree_nodes(&self.source, &self.dialect)) + .map_err(|error| usage_error(py, &error)) + }) + .map(Vec::as_slice) + } +} + +/// The Python tree of `nodes`, built from the last node back so children exist before parents. +fn python_tree(py: Python<'_>, nodes: &[TreeNode]) -> PyResult> { + let mut children: Vec>> = (0..nodes.len()).map(|_| Vec::new()).collect(); + let mut statements: Vec> = Vec::new(); + for (index, node) in nodes.iter().enumerate().rev() { + let mut own: Vec> = std::mem::take(&mut children[index]); + own.reverse(); + let built: Py = Py::new( + py, + SqlTreeNode { + kind: node.kind, + line: node.line, + column: node.column, + children: own, + }, + )?; + match node.parent { + Some(parent) => children[parent].push(built), + None => statements.push(built), + } + } + statements.reverse(); + Py::new( + py, + SqlTreeNode { + kind: DOCUMENT_KIND, + line: 1, + column: 1, + children: statements, + }, + ) +} + +/// `RuleUsageError(message)` for a document without a tree. +fn usage_error(py: Python<'_>, error: &SqlTreeError) -> PyErr { + new_annotated_error(py, ErrorId::RuleUsage, error.to_string(), &[]) +} + +pub(crate) fn register(module: &Bound<'_, PyModule>) -> PyResult<()> { + module.add_class::()?; + module.add_class::()?; + Ok(()) +} diff --git a/python-boundary.toml b/python-boundary.toml index 5829d6322f..7b959ef22a 100644 --- a/python-boundary.toml +++ b/python-boundary.toml @@ -212,11 +212,6 @@ path = "rule_engine/classes/project_tree.py" rule = 3 note = "Project-tree fact view user rules read through RuleContext; the walk, path validation, file resolution and the children, descendants, glob, relative_parts and read_text queries are native (project_tree_*), and only the model lookup stays here." -[[stays]] -path = "rule_engine/classes/sql_document.py" -rule = 4 -note = "Lazy Polyglot SQL document user rules read through RuleContext." - [[stays]] path = "rule_engine/classes/fact_reads.py" rule = 2 diff --git a/scripts/cold_compile_performance/_helpers/varied_statistics.py b/scripts/cold_compile_performance/_helpers/varied_statistics.py index 7bb09e1fb2..edcbec5da8 100644 --- a/scripts/cold_compile_performance/_helpers/varied_statistics.py +++ b/scripts/cold_compile_performance/_helpers/varied_statistics.py @@ -8,8 +8,7 @@ VARIED_WINDOW_NODE_FRAGMENT, ) from scripts.cold_compile_performance.models import VariedProjectStatistics -from sqlbuild._native import CompiledResourceType -from sqlbuild.compiler.sql_analysis.main.import_polyglot_sql import import_polyglot_sql +from sqlbuild._native import CompiledResourceType, SqlDocument _UDF_REFERENCE: re.Pattern[str] = re.compile(r'__udf\("([a-zA-Z0-9_]+)"\)') @@ -30,8 +29,8 @@ def varied_project_statistics(payload: dict[str, object]) -> VariedProjectStatis window_models: int = 0 for model in models: query_sql: str = _UDF_REFERENCE.sub(r"\1", model["query_sql"]) - expression: Any = import_polyglot_sql().parse_one(query_sql, dialect="snowflake") - shape: tuple[str, ...] = tuple(str(node.key) for node in expression.walk()) + document: SqlDocument = SqlDocument(source=query_sql, dialect="snowflake") + shape: tuple[str, ...] = tuple(node.kind for node in document.ast().walk()) shapes.add(shape) array_models += int(any(key in VARIED_ARRAY_NODE_KEYS for key in shape)) window_models += int(any(VARIED_WINDOW_NODE_FRAGMENT in key for key in shape)) diff --git a/scripts/rules_benchmark/_helpers/custom_rules.py b/scripts/rules_benchmark/_helpers/custom_rules.py index f51ef61b8d..cfad430f9b 100644 --- a/scripts/rules_benchmark/_helpers/custom_rules.py +++ b/scripts/rules_benchmark/_helpers/custom_rules.py @@ -64,8 +64,8 @@ def check_001(*, project: Project, ctx: RuleContext) -> list[Finding]: def _model_local_rule(*, index: int) -> str: check: str = { 2: ( - "(sum(1 for node in ctx.sql.for_model(model).expanded.polyglot_ast().walk() " - 'if str(getattr(node, "key", "")).lower() == "select") ' + "(sum(1 for node in ctx.sql.for_model(model).expanded.ast().walk() " + 'if node.kind == "select") ' '>= 1 if model.name == "model_00000" else True)' ), 3: "len(ctx.graph.dependencies(model)) >= 0", diff --git a/scripts/rules_benchmark/_helpers/workflow.py b/scripts/rules_benchmark/_helpers/workflow.py index e9ad10ca0a..ac93713bdf 100644 --- a/scripts/rules_benchmark/_helpers/workflow.py +++ b/scripts/rules_benchmark/_helpers/workflow.py @@ -1114,7 +1114,6 @@ def _hardware_payload() -> dict[str, object]: def _version_payload() -> dict[str, str]: return { "sqlbuild": importlib.metadata.version("sqlbuild"), - "polyglot_sql": importlib.metadata.version("polyglot-sql-chio"), } diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/custom-rules/rule-context.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/custom-rules/rule-context.md index 9daf4094f6..fcb5d7d0f3 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/custom-rules/rule-context.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/custom-rules/rule-context.md @@ -6,6 +6,15 @@ Online: https://sqlbuild.com/docs/concepts/rules/custom-rules/rule-context/ +## Contents + +- Authored and expanded SQL +- Common SQL structures +- Declared enum types +- Syntax tree +- Project structure +- Choose the subject deliberately + `RuleContext` exposes typed, read-only views over the compiled project: - `ctx.sql`: authored and expanded SQL @@ -32,16 +41,29 @@ the expanded SQL after interpolation, declarations, and macros. ## Common SQL structures -Common structures are available through typed source nodes: +Common structures are available on expanded SQL as `SqlNode` source locations, each with a `kind`, +a `line` and a `column`: ```python for cte in sql.expanded.ctes(): ... -for star in sql.authored.star_projections(): +for star in sql.expanded.star_projections(): ... ``` +| Method | Finds | +| --- | --- | +| `ctes()` | Each common table expression, at its name | +| `joins()` | Each explicit join clause, at its join keyword, not a comma-separated `FROM` item | +| `subqueries()` | Each parenthesized subquery, at its opening parenthesis | +| `star_projections()` | Each `*` and `table.*` that expands columns, including a function argument such as `OBJECT_CONSTRUCT(*)`, but not an aggregate's `*`, as in `COUNT(*)` | +| `nodes(kind=...)` | Each syntax tree node of that kind, compared without case | + +`line` and `column` count within the document's `source`. For expanded SQL that is the query +text, not the model file. Authored SQL is the whole model file, including its `MODEL` header, so it +does not parse as SQL; read its text through `source`. + ## Declared enum types `ctx.columns.declared(model)` reports each declared column with its portable scalar type, so an @@ -54,19 +76,37 @@ if enum_types.get("fulfillment_method") != "fulfillment_method": ... ``` -## Lazy Polyglot access +## Syntax tree -Use the full Polyglot AST only when common projections are insufficient: +Use the syntax tree only when the common structures are insufficient: ```python -ast = sql.expanded.polyglot_ast() -for node in ast.walk(): - ... +for node in sql.expanded.ast().walk(): + if node.kind == "window_function": + ... ``` -AST construction is lazy. A path or contract Rule does not pay for SQL parsing merely because AST +`ast()` returns a `document` node whose children are the statements. Every `SqlTreeNode` has a +`kind`, a `line` and `column` where it starts, `children`, and `walk()`, which yields the node and +its descendants in pre-order. Kinds are SQLQuack's snake_case names, such as `select`, `subquery`, +`star`, `join`, `cte`, `column`, `function` and `window_function`. + +Parsing is lazy. A path or contract Rule does not pay for SQL parsing merely because syntax tree access exists elsewhere. +The tree comes from the SQLQuack parser SQLBuild uses for the project's dialect: + +| Dialect | Syntax tree | +| --- | --- | +| PostgreSQL | Native PostgreSQL parser | +| DuckDB, MotherDuck, Snowflake, SQLite | SQLQuack's shared parser | +| Other dialects | Unavailable | + +For the shared parser, a node the parser records no position for, such as a bare `*` or the query +inside a subquery, reports the position of its parent. Reading the tree, or any common structure, +raises `RuleUsageError` when the dialect has no syntax tree, when the SQL does not parse, or when a +statement uses syntax the parser keeps as unparsed text. + ## Project structure Use the compiler-owned tree rather than direct filesystem calls: diff --git a/src/sqlbuild/_native.pyi b/src/sqlbuild/_native.pyi index 9bba402174..063e84a383 100644 --- a/src/sqlbuild/_native.pyi +++ b/src/sqlbuild/_native.pyi @@ -210,7 +210,6 @@ CUSTOM_RULE_PROJECT_SUBJECT: str CONTRACT_ENFORCED: str LAUNCH_RECORD_OPTION: str ZERO_DAY_CURSOR_DURATION: str -AST_CTE_KIND: str SQL_TEXT_FIELD: str SQL_DIGEST_FIELD: str SQL_PRIVATE_RECORD_FIELD: str @@ -4297,6 +4296,27 @@ class NativeModelValidator: self, model: tuple[str, str], references: list[tuple[str, str, bool]] ) -> int | NativeConfigError | None: ... +class SqlDocument: + """One SQL text a custom rule reads, parsed on first use by its dialect's SQLQuack parser.""" + + source: str + def __init__(self, *, source: str, dialect: str) -> None: ... + def ast(self) -> SqlTreeNode: ... + def nodes(self, *, kind: str) -> tuple[SqlNode, ...]: ... + def ctes(self) -> tuple[SqlNode, ...]: ... + def joins(self) -> tuple[SqlNode, ...]: ... + def subqueries(self) -> tuple[SqlNode, ...]: ... + def star_projections(self) -> tuple[SqlNode, ...]: ... + +class SqlTreeNode: + """One node of a rule's SQL syntax tree.""" + + kind: str + line: int + column: int + children: tuple[SqlTreeNode, ...] + def walk(self) -> list[SqlTreeNode]: ... + class SqlCodeScanner: def __init__(self, sql: str, syntax: dict[str, object] | None, context: str) -> None: ... def non_code_end(self, index: int) -> int | None: ... diff --git a/src/sqlbuild/rule_engine/classes/sql_document.py b/src/sqlbuild/rule_engine/classes/sql_document.py deleted file mode 100644 index e609dfa8a7..0000000000 --- a/src/sqlbuild/rule_engine/classes/sql_document.py +++ /dev/null @@ -1,62 +0,0 @@ -"""Lazy SQL document view for custom rules.""" - -from typing import Any - -from sqlbuild._native import AST_CTE_KIND, RuleUsageError, SqlNode -from sqlbuild.compiler.sql_analysis.main.import_polyglot_sql import import_polyglot_sql - - -class SqlDocument: - """Lazy authored or expanded SQL view for one compiled model.""" - - def __init__(self, *, source: str, dialect: str) -> None: - self.source: str = source - self._dialect: str = dialect - self._ast: Any | None = None - - def polyglot_ast(self) -> Any: - if self._ast is None: - polyglot: Any | None = import_polyglot_sql() - if polyglot is None: - raise RuleUsageError("rules require the bundled polyglot_sql package") - self._ast = polyglot.parse_one(self.source, dialect=self._dialect) - return self._ast - - def nodes(self, *, kind: str) -> tuple[SqlNode, ...]: - ast: Any = self.polyglot_ast() - matches: list[Any] = [ - node - for node in ast.walk() - if str(getattr(node, "key", "")).casefold() == kind.casefold() - ] - if kind.casefold() == AST_CTE_KIND: - matches.extend(_cte_values(ast)) - return tuple(_source_node(kind=kind, value=node) for node in matches) - - def ctes(self) -> tuple[SqlNode, ...]: - return self.nodes(kind="CTE") - - def joins(self) -> tuple[SqlNode, ...]: - return self.nodes(kind="Join") - - def subqueries(self) -> tuple[SqlNode, ...]: - return self.nodes(kind="Subquery") - - def star_projections(self) -> tuple[SqlNode, ...]: - return self.nodes(kind="Star") - - -def _cte_values(ast: Any) -> tuple[object, ...]: - with_clause: object = getattr(ast, "args", {}).get("with") - if not isinstance(with_clause, dict): - return () - values: object = with_clause.get("ctes") - return tuple(values) if isinstance(values, list) else () - - -def _source_node(*, kind: str, value: object) -> SqlNode: - return SqlNode( - kind=kind, - line=int(getattr(value, "line", 1) or 1), - column=int(getattr(value, "column", 1) or 1), - ) diff --git a/src/sqlbuild/rule_engine/classes/sql_facts.py b/src/sqlbuild/rule_engine/classes/sql_facts.py index 8a3e994583..328bf3ec1c 100644 --- a/src/sqlbuild/rule_engine/classes/sql_facts.py +++ b/src/sqlbuild/rule_engine/classes/sql_facts.py @@ -2,8 +2,7 @@ from functools import cached_property -from sqlbuild._native import Model, ModelSql, RuleUsageError -from sqlbuild.rule_engine.classes.sql_document import SqlDocument +from sqlbuild._native import Model, ModelSql, RuleUsageError, SqlDocument from sqlbuild.rule_engine.types import RuleFactModel, RuleFactProject diff --git a/src/sqlbuild/rules/__init__.py b/src/sqlbuild/rules/__init__.py index 1ae437ada4..8b00d4eff4 100644 --- a/src/sqlbuild/rules/__init__.py +++ b/src/sqlbuild/rules/__init__.py @@ -11,7 +11,9 @@ RuleFile, RuleOption, RuleResult, + SqlDocument, SqlNode, + SqlTreeNode, ) from sqlbuild.rule_engine.main.define import rule as _rule from sqlbuild.rule_engine.main.evaluate_rule import evaluate_rule @@ -27,7 +29,9 @@ "RuleFile", "RuleOption", "RuleResult", + "SqlDocument", "SqlNode", + "SqlTreeNode", "evaluate_rule", "rule", ) 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 6e710ee77e..f4859f7693 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 @@ -375,3 +375,12 @@ class SqlRuleTargetVarsCase: commands: tuple[tuple[str, ...], ...] expected_outcomes: tuple[tuple[int, bool, bool], ...] extra_files: dict[str, str] = field(default_factory=dict) + + +@dataclass(frozen=True) +class SyntaxTreeRuleE2ETestCase: + """A model whose expanded query a custom rule reads through the syntax tree.""" + + description: str + model_sql: str + expected_messages: tuple[str, ...] diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_rule_positions.py b/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_rule_positions.py index 80ce39b09b..62f0510870 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_rule_positions.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_rule_positions.py @@ -7,7 +7,16 @@ import pytest -from tests.e2e.src.sqlbuild.cli.commands.main.rules._test_types import RulePositionE2ETestCase +from tests.e2e.src.sqlbuild.cli.commands.main.rules._test_types import ( + RulePositionE2ETestCase, + SyntaxTreeRuleE2ETestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.rules.helpers import ( + custom_rule_diagnostics, + custom_rule_source, + run_compile_cli, + write_custom_rule_project, +) from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project, run_sqb _HEADER: str = 'MODEL (description "Test model orders.", materialized table);' @@ -21,6 +30,23 @@ "CASE\n WHEN TRIM(@x) = ''\n THEN NULL\n ELSE\n TRIM(@x)\nEND\n" ) } +_TREE_RULE: str = custom_rule_source( + code="XSQBRTREE001", + body=( + " sql = ctx.sql.for_model(model).expanded\n" + " lines = sql.source.splitlines()\n" + ' selects = sum(node.kind == "select" for node in sql.ast().walk())\n' + " nodes = (*sql.ctes(), *sql.joins(), *sql.subqueries(), *sql.star_projections())\n" + " return [\n" + " ctx.finding(\n" + " subject=model,\n" + ' message=f"{node.kind}/{selects}:"\n' + " + lines[node.line - 1][node.column - 1 : node.column + 3],\n" + " )\n" + " for node in nodes\n" + " ]\n" + ), +) @pytest.mark.parametrize( @@ -93,3 +119,43 @@ def test_given_model_query_layout_when_running_rules_then_findings_use_file_posi ) assert test_case.expected_location in ran.stdout + ran.stderr, ran.stdout + ran.stderr + + +@pytest.mark.parametrize( + "test_case", + [ + SyntaxTreeRuleE2ETestCase( + description="tree nodes point at their own text in the expanded query", + model_sql=( + 'MODEL (description "Orders");\n' + "WITH o AS (SELECT 1 AS id)\n" + "SELECT o.*, (SELECT 2) AS two, COUNT(*) OVER () AS n, HASH(*) AS h\n" + "FROM o\n" + " JOIN o AS p ON o.id = p.id, o AS q\n" + ), + expected_messages=( + "CTE/3:o AS", + "Join/3:JOIN", + "Star/3:HASH", + "Star/3:o.*,", + "Subquery/3:(SEL", + ), + ) + ], + ids=lambda case: case.description, +) +def test_given_custom_rule_over_the_syntax_tree_when_compiling_then_nodes_point_at_their_source( + test_case: SyntaxTreeRuleE2ETestCase, tmp_path: Path +) -> None: + write_custom_rule_project( + project_dir=tmp_path, + selected_rules=("XSQBRTREE001",), + files=(("rules/tree.py", _TREE_RULE),), + ) + (tmp_path / "models" / "orders.sql").write_text(test_case.model_sql, encoding="utf-8") + + result: subprocess.CompletedProcess[str] = run_compile_cli(tmp_path) + + assert tuple(sorted(item["message"] for item in custom_rule_diagnostics(result))) == ( + test_case.expected_messages + ), result.stdout + result.stderr diff --git a/tests/unit/src/sqlbuild/rule_engine/classes/sql_document/_test_types.py b/tests/unit/src/sqlbuild/rule_engine/classes/sql_document/_test_types.py deleted file mode 100644 index 34b65a5f74..0000000000 --- a/tests/unit/src/sqlbuild/rule_engine/classes/sql_document/_test_types.py +++ /dev/null @@ -1,13 +0,0 @@ -"""Test cases for typed SQL document facts.""" - -from dataclasses import dataclass - - -@dataclass(frozen=True) -class SqlDocumentTestCase: - """One lazy SQL document projection expectation.""" - - description: str - source: str - expected_cte_count: int - expected_star_count: int diff --git a/tests/unit/src/sqlbuild/rule_engine/classes/sql_document/test_sql_document.py b/tests/unit/src/sqlbuild/rule_engine/classes/sql_document/test_sql_document.py deleted file mode 100644 index 07aa504670..0000000000 --- a/tests/unit/src/sqlbuild/rule_engine/classes/sql_document/test_sql_document.py +++ /dev/null @@ -1,54 +0,0 @@ -"""Behavior tests for lazy common SQL facts.""" - -from __future__ import annotations - -from unittest.mock import patch - -import pytest - -from sqlbuild.rule_engine.classes.sql_document import SqlDocument -from tests.unit.src.sqlbuild.rule_engine.classes.sql_document._test_types import ( - SqlDocumentTestCase, -) - - -@pytest.mark.parametrize( - "test_case", - [ - SqlDocumentTestCase( - description="CTE and star facts are projected from Polyglot", - source="WITH orders AS (SELECT * FROM raw_orders) SELECT * FROM orders", - expected_cte_count=1, - expected_star_count=2, - ) - ], - ids=lambda case: case.description, -) -def test_given_common_sql_structures_when_projecting_then_typed_counts_match( - test_case: SqlDocumentTestCase, -) -> None: - document: SqlDocument = SqlDocument(source=test_case.source, dialect="duckdb") - - assert len(document.ctes()) == test_case.expected_cte_count - assert len(document.star_projections()) == test_case.expected_star_count - - -@pytest.mark.parametrize( - "test_case", - [ - SqlDocumentTestCase( - description="Polyglot loading is deferred until AST access", - source="SELECT 1 AS order_id", - expected_cte_count=0, - expected_star_count=0, - ) - ], - ids=lambda case: case.description, -) -def test_given_new_sql_document_when_not_accessing_ast_then_polyglot_is_not_loaded( - test_case: SqlDocumentTestCase, -) -> None: - with patch("sqlbuild.rule_engine.classes.sql_document.import_polyglot_sql") as import_polyglot: - _ = SqlDocument(source=test_case.source, dialect="duckdb") - - assert import_polyglot.call_count == test_case.expected_cte_count diff --git a/website/src/content/docs/docs/concepts/rules/custom-rules/rule-context.mdx b/website/src/content/docs/docs/concepts/rules/custom-rules/rule-context.mdx index 6f0d16b2eb..248a890eb3 100644 --- a/website/src/content/docs/docs/concepts/rules/custom-rules/rule-context.mdx +++ b/website/src/content/docs/docs/concepts/rules/custom-rules/rule-context.mdx @@ -29,16 +29,29 @@ the expanded SQL after interpolation, declarations, and macros. ## Common SQL structures -Common structures are available through typed source nodes: +Common structures are available on expanded SQL as `SqlNode` source locations, each with a `kind`, +a `line` and a `column`: ```python for cte in sql.expanded.ctes(): ... -for star in sql.authored.star_projections(): +for star in sql.expanded.star_projections(): ... ``` +| Method | Finds | +| --- | --- | +| `ctes()` | Each common table expression, at its name | +| `joins()` | Each explicit join clause, at its join keyword, not a comma-separated `FROM` item | +| `subqueries()` | Each parenthesized subquery, at its opening parenthesis | +| `star_projections()` | Each `*` and `table.*` that expands columns, including a function argument such as `OBJECT_CONSTRUCT(*)`, but not an aggregate's `*`, as in `COUNT(*)` | +| `nodes(kind=...)` | Each syntax tree node of that kind, compared without case | + +`line` and `column` count within the document's `source`. For expanded SQL that is the query +text, not the model file. Authored SQL is the whole model file, including its `MODEL` header, so it +does not parse as SQL; read its text through `source`. + ## Declared enum types `ctx.columns.declared(model)` reports each declared column with its portable scalar type, so an @@ -51,19 +64,37 @@ if enum_types.get("fulfillment_method") != "fulfillment_method": ... ``` -## Lazy Polyglot access +## Syntax tree -Use the full Polyglot AST only when common projections are insufficient: +Use the syntax tree only when the common structures are insufficient: ```python -ast = sql.expanded.polyglot_ast() -for node in ast.walk(): - ... +for node in sql.expanded.ast().walk(): + if node.kind == "window_function": + ... ``` -AST construction is lazy. A path or contract Rule does not pay for SQL parsing merely because AST +`ast()` returns a `document` node whose children are the statements. Every `SqlTreeNode` has a +`kind`, a `line` and `column` where it starts, `children`, and `walk()`, which yields the node and +its descendants in pre-order. Kinds are SQLQuack's snake_case names, such as `select`, `subquery`, +`star`, `join`, `cte`, `column`, `function` and `window_function`. + +Parsing is lazy. A path or contract Rule does not pay for SQL parsing merely because syntax tree access exists elsewhere. +The tree comes from the SQLQuack parser SQLBuild uses for the project's dialect: + +| Dialect | Syntax tree | +| --- | --- | +| PostgreSQL | Native PostgreSQL parser | +| DuckDB, MotherDuck, Snowflake, SQLite | SQLQuack's shared parser | +| Other dialects | Unavailable | + +For the shared parser, a node the parser records no position for, such as a bare `*` or the query +inside a subquery, reports the position of its parent. Reading the tree, or any common structure, +raises `RuleUsageError` when the dialect has no syntax tree, when the SQL does not parse, or when a +statement uses syntax the parser keeps as unparsed text. + ## Project structure Use the compiler-owned tree rather than direct filesystem calls: From e18a813dedd76a2059ca5deaa523f50942d78b78 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 8 Nov 2026 00:33:25 +0000 Subject: [PATCH 2/5] docs(rules): list syntax tree errors and the rule for adding a dialect --- .../src/sql_tree/_helpers/rules_tree_parser.rs | 3 +++ .../docs/concepts/rules/custom-rules/rule-context.md | 12 +++++++++--- .../concepts/rules/custom-rules/rule-context.mdx | 12 +++++++++--- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs b/crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs index 4a28b35f25..b12c58e56f 100644 --- a/crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs +++ b/crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs @@ -1,4 +1,7 @@ //! `rules_tree_parser`: the parser that reads a dialect's SQL for rules. +//! +//! A dialect is added here, one line, only once its parser passes the depth probe with an empty +//! allowlist; BigQuery is next, after its depth work lands. use sqlquack_syntax::Dialect; diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/custom-rules/rule-context.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/custom-rules/rule-context.md index fcb5d7d0f3..c81e21ff98 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/custom-rules/rule-context.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/custom-rules/rule-context.md @@ -103,9 +103,15 @@ The tree comes from the SQLQuack parser SQLBuild uses for the project's dialect: | Other dialects | Unavailable | For the shared parser, a node the parser records no position for, such as a bare `*` or the query -inside a subquery, reports the position of its parent. Reading the tree, or any common structure, -raises `RuleUsageError` when the dialect has no syntax tree, when the SQL does not parse, or when a -statement uses syntax the parser keeps as unparsed text. +inside a subquery, reports the position of its parent. + +Reading the tree, or any common structure, raises `RuleUsageError` when: + +| Cause | Message starts with | +| --- | --- | +| The dialect has no syntax tree | `SQL syntax trees are unavailable for the dialect` | +| The SQL does not parse | `SQL could not be parsed:` | +| A statement uses syntax the parser keeps as unparsed text, or nests too deeply | `SQL syntax tree is unavailable:` | ## Project structure diff --git a/website/src/content/docs/docs/concepts/rules/custom-rules/rule-context.mdx b/website/src/content/docs/docs/concepts/rules/custom-rules/rule-context.mdx index 248a890eb3..72215349ce 100644 --- a/website/src/content/docs/docs/concepts/rules/custom-rules/rule-context.mdx +++ b/website/src/content/docs/docs/concepts/rules/custom-rules/rule-context.mdx @@ -91,9 +91,15 @@ The tree comes from the SQLQuack parser SQLBuild uses for the project's dialect: | Other dialects | Unavailable | For the shared parser, a node the parser records no position for, such as a bare `*` or the query -inside a subquery, reports the position of its parent. Reading the tree, or any common structure, -raises `RuleUsageError` when the dialect has no syntax tree, when the SQL does not parse, or when a -statement uses syntax the parser keeps as unparsed text. +inside a subquery, reports the position of its parent. + +Reading the tree, or any common structure, raises `RuleUsageError` when: + +| Cause | Message starts with | +| --- | --- | +| The dialect has no syntax tree | `SQL syntax trees are unavailable for the dialect` | +| The SQL does not parse | `SQL could not be parsed:` | +| A statement uses syntax the parser keeps as unparsed text, or nests too deeply | `SQL syntax tree is unavailable:` | ## Project structure From d53268fc35659ceeae94ff696b17a7f74808ff69 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 8 Nov 2026 00:50:37 +0000 Subject: [PATCH 3/5] fix(rules): free deep syntax trees without recursion and report parser limits as unanalysable --- .../src/sql_tree/_helpers/parse.rs | 12 +- .../src/sql_tree/constants.rs | 2 + .../src/sql_tree/tests/test_sql_tree.rs | 6 + .../bindings/_helpers/rules/sql_document.rs | 134 +++++++++--------- tests/unit/src/sqlbuild/rules/_test_types.py | 13 ++ .../src/sqlbuild/rules/test_sql_document.py | 37 +++++ 6 files changed, 139 insertions(+), 65 deletions(-) create mode 100644 tests/unit/src/sqlbuild/rules/_test_types.py create mode 100644 tests/unit/src/sqlbuild/rules/test_sql_document.py diff --git a/crates/sqlbuild-analysis/src/sql_tree/_helpers/parse.rs b/crates/sqlbuild-analysis/src/sql_tree/_helpers/parse.rs index 1fc349a628..d37c6c17d9 100644 --- a/crates/sqlbuild-analysis/src/sql_tree/_helpers/parse.rs +++ b/crates/sqlbuild-analysis/src/sql_tree/_helpers/parse.rs @@ -2,6 +2,7 @@ use sqlquack_syntax::{CallMode, Expression, ParseOptions}; +use crate::sql_tree::constants::GUARD_CODE_PREFIX; use crate::sql_tree::errors::SqlTreeError; use crate::sql_tree::models::TreeParser; @@ -14,7 +15,7 @@ pub(crate) fn parse(sql: &str, parser: TreeParser) -> Result, Sq &ParseOptions::default(), CallMode::FaithfulLocated, ) - .map_err(|error| SqlTreeError::Parse(error.to_string())), + .map_err(|error| shared_error(&error.to_string())), TreeParser::Postgres => { let parsed = sqlquack_syntax_postgres::parse_strict(sql).map_err(|failure| { match failure.parse_error() { @@ -32,3 +33,12 @@ pub(crate) fn parse(sql: &str, parser: TreeParser) -> Result, Sq } } } + +/// A shared-parser failure: a guard limit, such as nesting depth, is unanalysable, not a syntax error. +fn shared_error(message: &str) -> SqlTreeError { + if message.contains(GUARD_CODE_PREFIX) { + SqlTreeError::Unanalysable(message.to_owned()) + } else { + SqlTreeError::Parse(message.to_owned()) + } +} diff --git a/crates/sqlbuild-analysis/src/sql_tree/constants.rs b/crates/sqlbuild-analysis/src/sql_tree/constants.rs index c25d766a11..a679be39eb 100644 --- a/crates/sqlbuild-analysis/src/sql_tree/constants.rs +++ b/crates/sqlbuild-analysis/src/sql_tree/constants.rs @@ -16,3 +16,5 @@ pub(crate) const SNOWFLAKE_DIALECT: &str = "snowflake"; pub(crate) const SQLITE_DIALECT: &str = "sqlite"; /// Stack for the parse-and-walk worker, so deep SQL reaches the parser's depth limit, not a stack overflow. pub(crate) const TREE_WORKER_STACK_BYTES: usize = 16 * 1024 * 1024; +/// The prefix of the shared parser's guard codes, such as `E_GUARD_NESTING_DEPTH_EXCEEDED`. +pub(crate) const GUARD_CODE_PREFIX: &str = "E_GUARD_"; diff --git a/crates/sqlbuild-analysis/src/sql_tree/tests/test_sql_tree.rs b/crates/sqlbuild-analysis/src/sql_tree/tests/test_sql_tree.rs index 7b689821bc..e1715b2eee 100644 --- a/crates/sqlbuild-analysis/src/sql_tree/tests/test_sql_tree.rs +++ b/crates/sqlbuild-analysis/src/sql_tree/tests/test_sql_tree.rs @@ -92,6 +92,12 @@ fn given_sql_without_a_full_tree_when_building_a_rule_tree_then_the_reason_is_re sql: format!("SELECT {}1{}", "(".repeat(10_000), ")".repeat(10_000)), expected_error: |error| matches!(error, SqlTreeError::Unanalysable(_)), }, + TreeFailureTestCase { + description: "the shared parser's nesting limit is unanalysable, not a syntax error", + dialect: "duckdb", + sql: format!("SELECT {}1{}", "(".repeat(1_000), ")".repeat(1_000)), + expected_error: |error| matches!(error, SqlTreeError::Unanalysable(_)), + }, TreeFailureTestCase { description: "a shared-parser syntax error", dialect: "sqlite", diff --git a/crates/sqlbuild-python/src/bindings/_helpers/rules/sql_document.rs b/crates/sqlbuild-python/src/bindings/_helpers/rules/sql_document.rs index f9a49031ec..9b905c2e47 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/rules/sql_document.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/rules/sql_document.rs @@ -1,5 +1,7 @@ //! Custom rules' SQL documents: the SQLQuack syntax tree of one SQL text and its common structures. +use std::sync::Arc; + use pyo3::prelude::{Bound, Py, PyAny, PyErr, PyModule, PyModuleMethods, PyResult, Python}; use pyo3::sync::PyOnceLock; use pyo3::types::PyTuple; @@ -16,40 +18,62 @@ const DOCUMENT_KIND: &str = "document"; /// The kind of an aggregate call, whose star argument, as in `COUNT(*)`, expands no columns. const AGGREGATE_KIND: &str = "aggregate_function"; -/// One node of a rule's SQL syntax tree. +/// A parsed document's nodes and each node's children, shared by every node read from it. +struct Tree { + nodes: Vec, + /// Child indexes by parent index; the last entry holds the statements, the document's children. + children: Vec>, +} + +/// One node of a rule's SQL syntax tree, held as an index into its shared tree. +/// +/// No node owns another, so freeing a deep tree never recurses. #[pyclass(module = "sqlbuild._native", frozen)] pub(crate) struct SqlTreeNode { - #[pyo3(get)] - kind: &'static str, - #[pyo3(get)] - line: usize, - #[pyo3(get)] - column: usize, - children: Vec>, + tree: Arc, + index: usize, } #[pymethods] impl SqlTreeNode { + #[getter] + fn kind(&self) -> &'static str { + self.tree + .nodes + .get(self.index) + .map_or(DOCUMENT_KIND, |node| node.kind) + } + + #[getter] + fn line(&self) -> usize { + self.tree.nodes.get(self.index).map_or(1, |node| node.line) + } + + #[getter] + fn column(&self) -> usize { + self.tree + .nodes + .get(self.index) + .map_or(1, |node| node.column) + } + /// The node's direct children, in SQLQuack's traversal order. #[getter] fn children<'py>(&self, py: Python<'py>) -> PyResult> { - PyTuple::new(py, self.children.iter().map(|child| child.clone_ref(py))) + let children: PyResult>> = self.tree.children[self.index] + .iter() + .map(|&index| tree_node(py, &self.tree, index)) + .collect(); + PyTuple::new(py, children?) } /// This node and every descendant, in pre-order. - fn walk(slf: Bound<'_, Self>) -> PyResult>> { - let py: Python<'_> = slf.py(); + fn walk(&self, py: Python<'_>) -> PyResult>> { let mut walked: Vec> = Vec::new(); - let mut stack: Vec> = vec![slf.unbind()]; - while let Some(node) = stack.pop() { - stack.extend( - node.get() - .children - .iter() - .rev() - .map(|child| child.clone_ref(py)), - ); - walked.push(node); + let mut stack: Vec = vec![self.index]; + while let Some(index) = stack.pop() { + stack.extend(self.tree.children[index].iter().rev()); + walked.push(tree_node(py, &self.tree, index)?); } Ok(walked) } @@ -57,7 +81,9 @@ impl SqlTreeNode { fn __repr__(&self) -> String { format!( "SqlTreeNode(kind={:?}, line={}, column={})", - self.kind, self.line, self.column + self.kind(), + self.line(), + self.column() ) } } @@ -68,8 +94,7 @@ pub(crate) struct SqlDocument { #[pyo3(get)] source: String, dialect: String, - nodes: PyOnceLock>, - tree: PyOnceLock>, + tree: PyOnceLock>, } #[pymethods] @@ -80,18 +105,14 @@ impl SqlDocument { Self { source, dialect, - nodes: PyOnceLock::new(), tree: PyOnceLock::new(), } } /// The syntax tree: a `document` node whose children are the statements. fn ast(&self, py: Python<'_>) -> PyResult> { - let tree = self.tree.get_or_try_init(py, || { - let nodes: &[TreeNode] = self.parsed(py)?; - python_tree(py, nodes) - })?; - Ok(tree.clone_ref(py)) + let tree: &Arc = self.parsed(py)?; + tree_node(py, tree, tree.nodes.len()) } /// Every node of `kind`, compared without case, as a stable source location. @@ -133,7 +154,7 @@ impl SqlDocument { keep: impl Fn(&[TreeNode], &TreeNode) -> bool, ) -> PyResult> { let wanted: String = kind.to_lowercase(); - let nodes: &[TreeNode] = self.parsed(py)?; + let nodes: &[TreeNode] = &self.parsed(py)?.nodes; let mut found: Vec> = Vec::new(); for node in nodes .iter() @@ -152,46 +173,31 @@ impl SqlDocument { PyTuple::new(py, found) } - /// The parsed nodes in pre-order; a failure raises `RuleUsageError` each time it is read. - fn parsed(&self, py: Python<'_>) -> PyResult<&[TreeNode]> { - self.nodes - .get_or_try_init(py, || { - py.detach(|| sql_tree_nodes(&self.source, &self.dialect)) - .map_err(|error| usage_error(py, &error)) - }) - .map(Vec::as_slice) + /// The parsed tree; a failure raises `RuleUsageError` each time it is read. + fn parsed(&self, py: Python<'_>) -> PyResult<&Arc> { + self.tree.get_or_try_init(py, || { + py.detach(|| sql_tree_nodes(&self.source, &self.dialect).map(tree)) + .map_err(|error| usage_error(py, &error)) + }) } } -/// The Python tree of `nodes`, built from the last node back so children exist before parents. -fn python_tree(py: Python<'_>, nodes: &[TreeNode]) -> PyResult> { - let mut children: Vec>> = (0..nodes.len()).map(|_| Vec::new()).collect(); - let mut statements: Vec> = Vec::new(); - for (index, node) in nodes.iter().enumerate().rev() { - let mut own: Vec> = std::mem::take(&mut children[index]); - own.reverse(); - let built: Py = Py::new( - py, - SqlTreeNode { - kind: node.kind, - line: node.line, - column: node.column, - children: own, - }, - )?; - match node.parent { - Some(parent) => children[parent].push(built), - None => statements.push(built), - } +/// `nodes` with each node's children listed, in ascending index order, under its parent. +fn tree(nodes: Vec) -> Arc { + let mut children: Vec> = vec![Vec::new(); nodes.len() + 1]; + for (index, node) in nodes.iter().enumerate() { + children[node.parent.unwrap_or(nodes.len())].push(index); } - statements.reverse(); + Arc::new(Tree { nodes, children }) +} + +/// The Python node at `index` of `tree`; `tree.nodes.len()` is the document root. +fn tree_node(py: Python<'_>, tree: &Arc, index: usize) -> PyResult> { Py::new( py, SqlTreeNode { - kind: DOCUMENT_KIND, - line: 1, - column: 1, - children: statements, + tree: Arc::clone(tree), + index, }, ) } diff --git a/tests/unit/src/sqlbuild/rules/_test_types.py b/tests/unit/src/sqlbuild/rules/_test_types.py new file mode 100644 index 0000000000..2cffe01e63 --- /dev/null +++ b/tests/unit/src/sqlbuild/rules/_test_types.py @@ -0,0 +1,13 @@ +"""Test case types for the public custom-rule SQL document.""" + +from dataclasses import dataclass + + +@dataclass(frozen=True) +class DeepTreeTestCase: + """A deeply nested query whose syntax tree a rule reads and then frees.""" + + description: str + dialect: str + terms: int + expected_node_count: int diff --git a/tests/unit/src/sqlbuild/rules/test_sql_document.py b/tests/unit/src/sqlbuild/rules/test_sql_document.py new file mode 100644 index 0000000000..77ec496732 --- /dev/null +++ b/tests/unit/src/sqlbuild/rules/test_sql_document.py @@ -0,0 +1,37 @@ +"""Behavior tests for the syntax tree custom rules read through `SqlDocument`.""" + +import gc + +import pytest + +from sqlbuild.rules import SqlDocument, SqlTreeNode +from tests.unit.src.sqlbuild.rules._test_types import DeepTreeTestCase + + +@pytest.mark.parametrize( + "test_case", + [ + DeepTreeTestCase( + description="document, select, 100,000 additions and 100,001 literals", + dialect="duckdb", + terms=100_000, + expected_node_count=200_003, + ), + ], + ids=lambda case: case.description, +) +def test_given_deep_query_when_reading_and_freeing_its_tree_then_the_process_survives( + test_case: DeepTreeTestCase, +) -> None: + document: SqlDocument | None = SqlDocument( + source="SELECT " + "1 + " * test_case.terms + "1", dialect=test_case.dialect + ) + assert document is not None + tree: SqlTreeNode | None = document.ast() + assert tree is not None + walked: int = len(tree.walk()) + + del tree, document + _ = gc.collect() + + assert walked == test_case.expected_node_count From 70b65dbb4877d884556d075e5fca9163e3e71555 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 8 Nov 2026 02:29:54 +0000 Subject: [PATCH 4/5] docs(rules): keep rule tree doc comments to one line --- .../src/sql_tree/_helpers/rules_tree_parser.rs | 4 +--- .../src/bindings/_helpers/rules/sql_document.rs | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs b/crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs index b12c58e56f..26731a5256 100644 --- a/crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs +++ b/crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs @@ -1,7 +1,4 @@ //! `rules_tree_parser`: the parser that reads a dialect's SQL for rules. -//! -//! A dialect is added here, one line, only once its parser passes the depth probe with an empty -//! allowlist; BigQuery is next, after its depth work lands. use sqlquack_syntax::Dialect; @@ -12,6 +9,7 @@ use crate::sql_tree::models::TreeParser; /// The parser for `dialect`, or `None` when SQLQuack parses none of it; becomes `analysis_route` once a native module registers. pub(crate) fn rules_tree_parser(dialect: &str) -> Option { + // Add a dialect (one line) only once its parser passes the depth probe with an empty allowlist. if POSTGRES_DIALECTS.contains(&dialect) { return Some(TreeParser::Postgres); } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/rules/sql_document.rs b/crates/sqlbuild-python/src/bindings/_helpers/rules/sql_document.rs index 9b905c2e47..356f7ab175 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/rules/sql_document.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/rules/sql_document.rs @@ -25,9 +25,7 @@ struct Tree { children: Vec>, } -/// One node of a rule's SQL syntax tree, held as an index into its shared tree. -/// -/// No node owns another, so freeing a deep tree never recurses. +/// One node of a rule's SQL syntax tree: an index into its shared tree, so freeing never recurses. #[pyclass(module = "sqlbuild._native", frozen)] pub(crate) struct SqlTreeNode { tree: Arc, From 2bad73c64b15929f30242f965eb0243e624944c4 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 8 Nov 2026 03:06:47 +0000 Subject: [PATCH 5/5] docs(rules): state the rule tree dialect rule in its doc comment --- .../src/sql_tree/_helpers/rules_tree_parser.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs b/crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs index 26731a5256..b330c0a39c 100644 --- a/crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs +++ b/crates/sqlbuild-analysis/src/sql_tree/_helpers/rules_tree_parser.rs @@ -7,9 +7,8 @@ use crate::sql_tree::constants::{ }; use crate::sql_tree::models::TreeParser; -/// The parser for `dialect`, or `None` when SQLQuack parses none of it; becomes `analysis_route` once a native module registers. +/// The parser for `dialect`, or `None`; add a dialect (one line) only once its parser passes the depth probe with an empty allowlist. pub(crate) fn rules_tree_parser(dialect: &str) -> Option { - // Add a dialect (one line) only once its parser passes the depth probe with an empty allowlist. if POSTGRES_DIALECTS.contains(&dialect) { return Some(TreeParser::Postgres); }