From af23bcee36f22af1fd8dc3cc88ad18c1541103e4 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 5 Nov 2026 09:42:52 +0000 Subject: [PATCH 1/6] Refuse an unterminated block comment and count a flat operator chain as one depth level - The tokenizer refuses an unterminated /* at its opening, as it does an unterminated string; the refusal is the parse diagnostic SQ0001 with its span. - The AST depth guard counts a left-associative chain such as 1 + 1 + ... as one level, so a 50,000-term sum parses; real nesting (600 unary minus signs) is still refused with E_GUARD_AST_DEPTH_EXCEEDED. - Dropping a long operator chain frees it with a worklist, so it cannot overflow the stack. --- .../src/diagnostics/tests/test_render.rs | 12 +++++ .../operators/_helpers/binary_ops.rs | 47 +++++++++++++++++++ .../operators/_helpers/boolean_chains.rs | 10 ++-- .../src/expressions/operators/_helpers/mod.rs | 1 + .../src/expressions/operators/models.rs | 26 ++++++++++ .../sqlquack-syntax/src/guard/_helpers/ast.rs | 44 ++++++++++++++--- crates/sqlquack-syntax/src/guard/tests.rs | 2 + .../src/guard/tests/helpers.rs | 12 +++++ .../src/guard/tests/test_ast_guard.rs | 39 +++++++++++++++ .../src/guard/tests/test_types.rs | 7 +++ .../src/tokens/_helpers/scanning.rs | 11 +++-- .../src/tokens/tests/test_error.rs | 6 +++ 12 files changed, 198 insertions(+), 19 deletions(-) create mode 100644 crates/sqlquack-syntax/src/expressions/operators/_helpers/binary_ops.rs create mode 100644 crates/sqlquack-syntax/src/guard/tests/test_ast_guard.rs diff --git a/crates/sqlquack-core/src/diagnostics/tests/test_render.rs b/crates/sqlquack-core/src/diagnostics/tests/test_render.rs index dd30ecd07..6da1f8c50 100644 --- a/crates/sqlquack-core/src/diagnostics/tests/test_render.rs +++ b/crates/sqlquack-core/src/diagnostics/tests/test_render.rs @@ -102,6 +102,18 @@ fn given_guard_failures_when_diagnosed_then_each_guard_has_its_own_code_and_form expected_code: ("SQ0001", None), expected_label: "Unterminated string", }, + GuardTestCase { + description: "an unterminated block comment is refused with the parse code", + source: "SELECT 1 /* never closed".to_owned(), + expected_code: ("SQ0001", None), + expected_label: "Unterminated block comment", + }, + GuardTestCase { + description: "50,000 added terms are one flat chain, not 50,000 levels of depth", + source: format!("SELECT {} FROM", vec!["1"; 50_000].join(" + ")), + expected_code: ("SQ0001", None), + expected_label: "Expected table name or subquery, got From", + }, ]; for test_case in test_cases { let diagnostics = diagnose(&[unparsed(&test_case.source)], &test_case.source); diff --git a/crates/sqlquack-syntax/src/expressions/operators/_helpers/binary_ops.rs b/crates/sqlquack-syntax/src/expressions/operators/_helpers/binary_ops.rs new file mode 100644 index 000000000..53d32648c --- /dev/null +++ b/crates/sqlquack-syntax/src/expressions/operators/_helpers/binary_ops.rs @@ -0,0 +1,47 @@ +//! Reaching the [`BinaryOp`] inside an expression. + +use crate::expressions::operators::models::BinaryOp; +use crate::expressions::tree::models::Expression; + +/// The operands of every expression variant that wraps a [`BinaryOp`]. +pub(crate) fn binary_op_mut(expr: &mut Expression) -> Option<&mut BinaryOp> { + match expr { + Expression::And(op) + | Expression::Or(op) + | Expression::Add(op) + | Expression::Sub(op) + | Expression::Mul(op) + | Expression::Div(op) + | Expression::Mod(op) + | Expression::Eq(op) + | Expression::Neq(op) + | Expression::Lt(op) + | Expression::Lte(op) + | Expression::Gt(op) + | Expression::Gte(op) + | Expression::Match(op) + | Expression::BitwiseAnd(op) + | Expression::BitwiseOr(op) + | Expression::BitwiseXor(op) + | Expression::Concat(op) + | Expression::Adjacent(op) + | Expression::TsMatch(op) + | Expression::PropertyEQ(op) + | Expression::ArrayContainsAll(op) + | Expression::ArrayContainedBy(op) + | Expression::ArrayOverlaps(op) + | Expression::JSONBContainsAllTopKeys(op) + | Expression::JSONBContainsAnyTopKeys(op) + | Expression::JSONBDeleteAtPath(op) + | Expression::ExtendsLeft(op) + | Expression::ExtendsRight(op) + | Expression::Is(op) + | Expression::MemberOf(op) + | Expression::NullSafeEq(op) + | Expression::NullSafeNeq(op) + | Expression::Glob(op) + | Expression::BitwiseLeftShift(op) + | Expression::BitwiseRightShift(op) => Some(op), + _ => None, + } +} diff --git a/crates/sqlquack-syntax/src/expressions/operators/_helpers/boolean_chains.rs b/crates/sqlquack-syntax/src/expressions/operators/_helpers/boolean_chains.rs index 2d170581a..3dcc4b304 100644 --- a/crates/sqlquack-syntax/src/expressions/operators/_helpers/boolean_chains.rs +++ b/crates/sqlquack-syntax/src/expressions/operators/_helpers/boolean_chains.rs @@ -44,13 +44,9 @@ pub(crate) fn flatten_boolean_terms_owned(expr: Expression, is_and: bool) -> Vec while let Some(node) = stack.pop() { match (is_and, node) { - (true, Expression::And(op)) => { - stack.push(op.right); - stack.push(op.left); - } - (false, Expression::Or(op)) => { - stack.push(op.right); - stack.push(op.left); + (true, Expression::And(mut op)) | (false, Expression::Or(mut op)) => { + stack.push(std::mem::replace(&mut op.right, Expression::Null(Null))); + stack.push(std::mem::replace(&mut op.left, Expression::Null(Null))); } (_, other) => terms.push(other), } diff --git a/crates/sqlquack-syntax/src/expressions/operators/_helpers/mod.rs b/crates/sqlquack-syntax/src/expressions/operators/_helpers/mod.rs index fbfa07b43..9e81baec8 100644 --- a/crates/sqlquack-syntax/src/expressions/operators/_helpers/mod.rs +++ b/crates/sqlquack-syntax/src/expressions/operators/_helpers/mod.rs @@ -1,3 +1,4 @@ //! Operators helpers. +pub(crate) mod binary_ops; pub(crate) mod boolean_chains; diff --git a/crates/sqlquack-syntax/src/expressions/operators/models.rs b/crates/sqlquack-syntax/src/expressions/operators/models.rs index 73df99eac..e674c8139 100644 --- a/crates/sqlquack-syntax/src/expressions/operators/models.rs +++ b/crates/sqlquack-syntax/src/expressions/operators/models.rs @@ -5,6 +5,7 @@ use serde::Deserialize; use serde::Serialize; use crate::expressions::names::models::Identifier; +use crate::expressions::names::models::Null; use crate::expressions::names::models::TableRef; use crate::expressions::queries::models::Ordered; use crate::expressions::tree::models::Expression; @@ -161,6 +162,31 @@ pub struct BinaryOp { pub span: SourceSpan, } +/// Frees nested operators with a worklist so dropping a long chain such as `1 + 1 + ...` cannot overflow. +impl Drop for BinaryOp { + fn drop(&mut self) { + if crate::expressions::operators::_helpers::binary_ops::binary_op_mut(&mut self.left) + .is_none() + && crate::expressions::operators::_helpers::binary_ops::binary_op_mut(&mut self.right) + .is_none() + { + return; + } + let mut pending = vec![ + std::mem::replace(&mut self.left, Expression::Null(Null)), + std::mem::replace(&mut self.right, Expression::Null(Null)), + ]; + while let Some(mut expr) = pending.pop() { + if let Some(op) = + crate::expressions::operators::_helpers::binary_ops::binary_op_mut(&mut expr) + { + pending.push(std::mem::replace(&mut op.left, Expression::Null(Null))); + pending.push(std::mem::replace(&mut op.right, Expression::Null(Null))); + } + } + } +} + impl BinaryOp { pub fn new(left: Expression, right: Expression) -> Self { Self { diff --git a/crates/sqlquack-syntax/src/guard/_helpers/ast.rs b/crates/sqlquack-syntax/src/guard/_helpers/ast.rs index 574ada923..3c85899eb 100644 --- a/crates/sqlquack-syntax/src/guard/_helpers/ast.rs +++ b/crates/sqlquack-syntax/src/guard/_helpers/ast.rs @@ -72,17 +72,47 @@ pub(crate) fn push_ast_children<'a>( stack = push_connector_child(stack, &op.right, depth, ConnectorKind::Or); stack = push_connector_child(stack, &op.left, depth, ConnectorKind::Or); } - _ => { - let child_start = stack.len(); - crate::traversal::main::for_each_child::for_each_child(expr, |_, child| { - stack.push((child, depth + 1)); - }); - stack[child_start..].reverse(); - } + _ => match chain_operator(expr).filter(|op| is_commentless_binary_op(op)) { + Some(op) => { + stack.push((&op.right, depth + 1)); + let left_depth = match chain_operator(&op.left) { + Some(left) if is_commentless_binary_op(left) => depth, + _ => depth + 1, + }; + stack.push((&op.left, left_depth)); + } + None => { + let child_start = stack.len(); + crate::traversal::main::for_each_child::for_each_child(expr, |_, child| { + stack.push((child, depth + 1)); + }); + stack[child_start..].reverse(); + } + }, } stack } +/// A left-associative operator whose flat chain, such as `1 + 1 + 1`, is one level, not nesting. +pub(crate) fn chain_operator( + expr: &Expression, +) -> Option<&crate::expressions::operators::models::BinaryOp> { + match expr { + Expression::Add(op) + | Expression::Sub(op) + | Expression::Mul(op) + | Expression::Div(op) + | Expression::Mod(op) + | Expression::Concat(op) + | Expression::BitwiseAnd(op) + | Expression::BitwiseOr(op) + | Expression::BitwiseXor(op) + | Expression::BitwiseLeftShift(op) + | Expression::BitwiseRightShift(op) => Some(op), + _ => None, + } +} + pub(crate) fn push_connector_child<'a>( mut stack: Vec<(&'a Expression, usize)>, child: &'a Expression, diff --git a/crates/sqlquack-syntax/src/guard/tests.rs b/crates/sqlquack-syntax/src/guard/tests.rs index 0bac5b92b..f980bf5f1 100644 --- a/crates/sqlquack-syntax/src/guard/tests.rs +++ b/crates/sqlquack-syntax/src/guard/tests.rs @@ -1,5 +1,7 @@ #[path = "tests/helpers.rs"] mod helpers; +#[path = "tests/test_ast_guard.rs"] +mod test_ast_guard; #[path = "tests/test_token_guard.rs"] mod test_token_guard; #[path = "tests/test_types.rs"] diff --git a/crates/sqlquack-syntax/src/guard/tests/helpers.rs b/crates/sqlquack-syntax/src/guard/tests/helpers.rs index f0cca65a7..6373cfe21 100644 --- a/crates/sqlquack-syntax/src/guard/tests/helpers.rs +++ b/crates/sqlquack-syntax/src/guard/tests/helpers.rs @@ -61,3 +61,15 @@ pub(crate) fn limit_json_problems(field: &str) -> Vec { pub(crate) fn guard_json_accepted(json: &str) -> bool { serde_json::from_str::(json).is_ok() } + +/// The default-guarded Snowflake parse of `sql`, or its error message. +pub(crate) fn guarded_parse(sql: &str) -> Result<(), String> { + crate::parse_statements_with( + sql, + crate::Dialect::Snowflake, + &crate::ParseOptions::default(), + crate::CallMode::Faithful, + ) + .map(|_| ()) + .map_err(|error| error.to_string()) +} diff --git a/crates/sqlquack-syntax/src/guard/tests/test_ast_guard.rs b/crates/sqlquack-syntax/src/guard/tests/test_ast_guard.rs new file mode 100644 index 000000000..9d7222683 --- /dev/null +++ b/crates/sqlquack-syntax/src/guard/tests/test_ast_guard.rs @@ -0,0 +1,39 @@ +use crate::guard::tests::helpers::guarded_parse; +use crate::guard::tests::test_types::AstDepthTestCase; + +#[test] +fn given_a_long_flat_operator_chain_when_parsing_then_only_real_nesting_counts_as_depth() { + let test_cases = [ + AstDepthTestCase { + description: "50,000 added terms are one flat chain", + sql: || format!("SELECT {}", vec!["1"; 50_000].join(" + ")), + expected_error: None, + }, + AstDepthTestCase { + description: "a long chain mixing products, sums and concatenation stays flat", + sql: || format!("SELECT {}", vec!["2 * 3 - 1 || 'x'"; 5_000].join(" + ")), + expected_error: None, + }, + AstDepthTestCase { + description: "600 nested unary minus signs are real nesting", + sql: || format!("SELECT {}1", "- ".repeat(600)), + expected_error: Some("E_GUARD_AST_DEPTH_EXCEEDED"), + }, + ]; + for test_case in test_cases { + let outcome: Result<(), String> = guarded_parse(&(test_case.sql)()); + let code: Option = outcome.as_ref().err().map(|error| { + error + .split(|c: char| !(c.is_ascii_uppercase() || c == '_')) + .find(|word| word.starts_with("E_")) + .unwrap_or(error) + .to_owned() + }); + assert_eq!( + code.as_deref(), + test_case.expected_error, + "{}: {outcome:?}", + test_case.description + ); + } +} diff --git a/crates/sqlquack-syntax/src/guard/tests/test_types.rs b/crates/sqlquack-syntax/src/guard/tests/test_types.rs index 448060874..0f10bc7dc 100644 --- a/crates/sqlquack-syntax/src/guard/tests/test_types.rs +++ b/crates/sqlquack-syntax/src/guard/tests/test_types.rs @@ -31,3 +31,10 @@ pub(crate) struct GuardKeyTestCase { pub(crate) json: &'static str, pub(crate) expected_accepted: bool, } + +pub(crate) struct AstDepthTestCase { + pub(crate) description: &'static str, + pub(crate) sql: fn() -> String, + /// None when the default guard accepts the query; otherwise a fragment of the error. + pub(crate) expected_error: Option<&'static str>, +} diff --git a/crates/sqlquack-syntax/src/tokens/_helpers/scanning.rs b/crates/sqlquack-syntax/src/tokens/_helpers/scanning.rs index 6b57f38ef..5b9cb04f1 100644 --- a/crates/sqlquack-syntax/src/tokens/_helpers/scanning.rs +++ b/crates/sqlquack-syntax/src/tokens/_helpers/scanning.rs @@ -56,7 +56,7 @@ impl<'a, C: TokenizerCursor, T: TokenOutput> TokenizerState<'a, C, T> { pub(crate) fn tokenize(&mut self) -> Result> { while !self.is_at_end() { - self.skip_whitespace(); + self.skip_whitespace()?; if self.is_at_end() { break; } @@ -249,7 +249,8 @@ impl<'a, C: TokenizerCursor, T: TokenOutput> TokenizerState<'a, C, T> { Some((content_start, content_end)) } - pub(crate) fn skip_whitespace(&mut self) { + /// Skips whitespace and comments; an unterminated block comment is an error at its `/*`. + pub(crate) fn skip_whitespace(&mut self) -> Result<()> { let mut saw_newline = false; while !self.is_at_end() { let c = self.peek(); @@ -275,9 +276,8 @@ impl<'a, C: TokenizerCursor, T: TokenOutput> TokenizerState<'a, C, T> { if self.current + 2 < self.size && self.char_at(self.current + 2) == '+' { break; } - if self.scan_block_comment(saw_newline).is_err() { - return; - } + self.start = self.current; + self.scan_block_comment(saw_newline)?; } '/' if self.peek_next() == '/' && self.config.comments.contains_key("//") => { let prev_non_ws = if self.current > 0 { @@ -301,6 +301,7 @@ impl<'a, C: TokenizerCursor, T: TokenOutput> TokenizerState<'a, C, T> { _ => break, } } + Ok(()) } pub(crate) fn scan_hash_line_comment(&mut self) { diff --git a/crates/sqlquack-syntax/src/tokens/tests/test_error.rs b/crates/sqlquack-syntax/src/tokens/tests/test_error.rs index 788f7ca66..475566ae5 100644 --- a/crates/sqlquack-syntax/src/tokens/tests/test_error.rs +++ b/crates/sqlquack-syntax/src/tokens/tests/test_error.rs @@ -45,6 +45,12 @@ fn given_an_error_when_reading_its_position_then_line_column_and_offsets_are_kep expected_position: (Some(1), Some(14), Some(12), Some(13)), expected_message_parts: &["line 1"], }, + ErrorPositionTestCase { + description: "an unterminated block comment fails at its opening, never parsing on", + error: parse_error("SELECT /* never closed"), + expected_position: (Some(1), Some(23), Some(7), Some(22)), + expected_message_parts: &["Unterminated block comment"], + }, ]; for test_case in test_cases { From d98ac742d9e09e94ab9856c37901875fa62ccd57 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 5 Nov 2026 10:47:39 +0000 Subject: [PATCH 2/6] Keep Snowflake account parts and quoted property names as written ACCOUNTS lists in release channels, release directives and replication or failover now keep [ organization . ] account as a table reference with each part quoted separately, instead of one joined identifier. ALTER SET and UNSET property kinds upper-case only unquoted names, so a quoted name such as "v1 Q" keeps its case. --- .../parser/_helpers/ddl/snowflake_alter.rs | 12 +++++++++-- .../_helpers/ddl/snowflake_alter_account.rs | 11 +++++----- .../_helpers/ddl/snowflake_alter_objects.rs | 20 ++++++++++++++++--- .../_helpers/ddl/snowflake_alter_resources.rs | 7 ++++--- .../tests/parser/test_fixes.rs | 18 +++++++++++++++++ 5 files changed, 55 insertions(+), 13 deletions(-) diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter.rs index bcd20fa59..bdaf8b1c2 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter.rs @@ -278,7 +278,7 @@ impl Parser { } else { None }; - properties.push(property(&name.name.to_ascii_uppercase(), None, value)); + properties.push(property(&property_kind(&name), None, value)); } let comma = self.match_token(TokenType::Comma); if self.is_at_end() || self.check(TokenType::Semicolon) || !set && !comma { @@ -710,7 +710,7 @@ impl Parser { } else { self.parse_object_value()? }; - properties.push(property(&name.name.to_ascii_uppercase(), None, Some(value))); + properties.push(property(&property_kind(&name), None, Some(value))); self.match_token(TokenType::Comma); if self.is_at_end() || self.check(TokenType::Semicolon) { return Ok(properties); @@ -760,6 +760,14 @@ pub(super) fn alter_object( })) } +/// A property's kind from its name: upper-cased when unquoted, as written when quoted. +pub(super) fn property_kind(name: &Identifier) -> String { + match name.quoted { + true => name.name.clone(), + false => name.name.to_ascii_uppercase(), + } +} + /// One property of kind `kind`. pub(super) fn property( kind: &str, diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_account.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_account.rs index 4c3ab72ee..4966a610d 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_account.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_account.rs @@ -7,6 +7,7 @@ use crate::expressions::tree::models::Expression; use crate::expressions::types::models::Tuple; use crate::parser::_helpers::ddl::snowflake_alter::alter_object; use crate::parser::_helpers::ddl::snowflake_alter::property; +use crate::parser::_helpers::ddl::snowflake_alter::property_kind; use crate::parser::models::Parser; use crate::tokens::models::TokenType; use crate::tokens::types::Result; @@ -124,7 +125,7 @@ impl Parser { let name = self.parse_simple_name()?; self.expect_token(TokenType::Eq)?; let value = self.parse_group_values()?; - properties.push(property(&name.name.to_ascii_uppercase(), None, Some(value))); + properties.push(property(&property_kind(&name), None, Some(value))); if self.is_at_end() || self.check(TokenType::Semicolon) { return Ok(properties); } @@ -188,7 +189,7 @@ impl Parser { let mut properties: Vec = Vec::new(); loop { let name = self.parse_simple_name()?; - properties.push(property(&name.name.to_ascii_uppercase(), None, None)); + properties.push(property(&property_kind(&name), None, None)); if !self.match_token(TokenType::Comma) { return Ok(properties); } @@ -406,7 +407,7 @@ impl Parser { let name = self.parse_simple_name()?; self.expect_token(TokenType::Eq)?; let value = self.parse_integration_value()?; - properties.push(property(&name.name.to_ascii_uppercase(), None, Some(value))); + properties.push(property(&property_kind(&name), None, Some(value))); if self.is_at_end() || self.check(TokenType::Semicolon) { return Ok(properties); } @@ -545,7 +546,7 @@ impl Parser { } else { self.parse_object_value()? }; - properties.push(property(&name.name.to_ascii_uppercase(), None, Some(value))); + properties.push(property(&property_kind(&name), None, Some(value))); self.match_token(TokenType::Comma); if self.is_at_end() || self.check(TokenType::Semicolon) { return Ok(properties); @@ -558,7 +559,7 @@ impl Parser { let mut properties: Vec = Vec::new(); loop { let name = self.parse_simple_name()?; - properties.push(property(&name.name.to_ascii_uppercase(), None, None)); + properties.push(property(&property_kind(&name), None, None)); if !self.match_token(TokenType::Comma) { return Ok(properties); } diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_objects.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_objects.rs index a845126b2..485ac4441 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_objects.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_objects.rs @@ -7,6 +7,7 @@ use crate::expressions::tree::models::Expression; use crate::expressions::types::models::Tuple; use crate::parser::_helpers::ddl::snowflake_alter::alter_object; use crate::parser::_helpers::ddl::snowflake_alter::property; +use crate::parser::_helpers::ddl::snowflake_alter::property_kind; use crate::parser::constants::CONTACT; use crate::parser::constants::TAG; use crate::parser::models::Parser; @@ -90,13 +91,26 @@ impl Parser { /// Parse a comma-separated list of account identifiers, each `[ . ] `. fn account_list_operand(&mut self) -> Result { - let mut accounts = vec![Expression::Identifier(self.parse_simple_qualified_name()?)]; + let mut accounts = vec![self.account_operand()?]; while self.match_token(TokenType::Comma) { - accounts.push(Expression::Identifier(self.parse_simple_qualified_name()?)); + accounts.push(self.account_operand()?); } Ok(tuple(accounts)) } + /// Parse `[ . ] `, keeping each part and its quoting. + fn account_operand(&mut self) -> Result { + let first = self.parse_simple_name()?; + let mut account = TableRef::new(""); + if self.match_token(TokenType::Dot) { + account.name = self.parse_simple_name()?; + account.schema = Some(first); + } else { + account.name = first; + } + Ok(Expression::Table(Box::new(account))) + } + /// Parse a parenthesized list of account identifiers. fn parse_account_list(&mut self) -> Result { self.expect_token(TokenType::LParen)?; @@ -237,7 +251,7 @@ impl Parser { } else { None }; - properties.push(property(&name.name.to_ascii_uppercase(), None, value)); + properties.push(property(&property_kind(&name), None, value)); } let comma = self.match_token(TokenType::Comma); if self.is_at_end() || self.check(TokenType::Semicolon) || !set && !comma { diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_resources.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_resources.rs index 98459fab1..751d0560d 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_resources.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_resources.rs @@ -6,6 +6,7 @@ use crate::expressions::tree::models::Expression; use crate::expressions::types::models::Tuple; use crate::parser::_helpers::ddl::snowflake_alter::alter_object; use crate::parser::_helpers::ddl::snowflake_alter::property; +use crate::parser::_helpers::ddl::snowflake_alter::property_kind; use crate::parser::models::Parser; use crate::tokens::models::TokenType; use crate::tokens::types::Result; @@ -362,7 +363,7 @@ impl Parser { return Err(self.parse_error("Expected a literal value")); } let value = self.parse_object_value()?; - Ok(property(&name.name.to_ascii_uppercase(), None, Some(value))) + Ok(property(&property_kind(&name), None, Some(value))) } /// Parse a listing's ADD VERSION after its keywords: `[ [ IF NOT EXISTS ] ] FROM ` and properties. @@ -637,7 +638,7 @@ impl Parser { self.expect_token(TokenType::Eq)?; self.parse_resource_value()? }; - properties.push(property(&name.name.to_ascii_uppercase(), None, Some(value))); + properties.push(property(&property_kind(&name), None, Some(value))); self.match_token(TokenType::Comma); } } @@ -665,7 +666,7 @@ impl Parser { } } else { let name = self.parse_simple_name()?; - properties.push(property(&name.name.to_ascii_uppercase(), None, None)); + properties.push(property(&property_kind(&name), None, None)); } if !self.match_token(TokenType::Comma) { return Ok(properties); diff --git a/crates/sqlquack-syntax/tests/parser/test_fixes.rs b/crates/sqlquack-syntax/tests/parser/test_fixes.rs index 0eec132b0..1cc6b1967 100644 --- a/crates/sqlquack-syntax/tests/parser/test_fixes.rs +++ b/crates/sqlquack-syntax/tests/parser/test_fixes.rs @@ -1520,6 +1520,24 @@ fn given_a_snowflake_alter_of_an_application_database_or_service_kind_when_parsi pointer: "/0/alter_object/action", expected_value: json!("ENABLE REPLICATION IGNORE EDITION CHECK"), }, + ProjectionTestCase { + description: "an account keeps its unquoted organization as a separate part", + sql: "ALTER APPLICATION PACKAGE v1 MODIFY RELEASE CHANNEL c1 ADD ACCOUNTS = ( o1 . \"a Q\" )", + pointer: "/0/alter_object/operands/1/tuple/expressions/0/table/schema/quoted", + expected_value: json!(false), + }, + ProjectionTestCase { + description: "an account keeps its quoted name as written", + sql: "ALTER APPLICATION PACKAGE v1 MODIFY RELEASE CHANNEL c1 ADD ACCOUNTS = ( o1 . \"a Q\" )", + pointer: "/0/alter_object/operands/1/tuple/expressions/0/table/name/name", + expected_value: json!("a Q"), + }, + ProjectionTestCase { + description: "a quoted property name is not case-folded", + sql: "ALTER ORGANIZATION ACCOUNT UNSET \"v1 Q\", foo", + pointer: "/0/alter_object/properties/0/kind", + expected_value: json!("v1 Q"), + }, ProjectionTestCase { description: "a linked catalog change names its list", sql: "ALTER DATABASE v1 UPDATE LINKED_CATALOG REMOVE ( 'n1' ) FROM ALLOWED_NAMESPACES", From c224459eeb124e8f24c09243a7f7c91c7645aaef Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 5 Nov 2026 10:58:22 +0000 Subject: [PATCH 3/6] Parse Snowflake anonymous procedures as loosely as Snowflake does Receipts show Snowflake parses a parameter or RETURNS TABLE column given by its name alone, a name as a RUNTIME_VERSION, HANDLER, IMPORTS or PACKAGES value, and an expression CTE body after an anonymous procedure, as WITH p AS PROCEDURE ... , c AS ( 1 ) CALL p ( ). A lone part now has an unknown type; quoted strings and reserved words stay rejected. --- .../src/parser/_helpers/statements/entry.rs | 2 +- .../parser/_helpers/statements/procedures.rs | 49 ++++++++++++++++--- .../src/parser/tests/test_snowflake.rs | 13 +++++ 3 files changed, 56 insertions(+), 8 deletions(-) diff --git a/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs b/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs index a1f9f46d3..4609471af 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs @@ -1952,7 +1952,7 @@ impl Parser { ))); } let query_start = self.current; - let query = self.parse_statement()?; + let query = self.parse_cte_body(has_procedure)?; let query = self.spanned(query_start, query); self.expect_token(TokenType::RParen)?; diff --git a/crates/sqlquack-syntax/src/parser/_helpers/statements/procedures.rs b/crates/sqlquack-syntax/src/parser/_helpers/statements/procedures.rs index 223defc3d..25a8584c8 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/statements/procedures.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/procedures.rs @@ -50,6 +50,18 @@ impl Parser { Ok(Expression::CreateProcedure(Box::new(procedure))) } + /// A CTE body: a statement, or after an anonymous procedure also an expression, as `v1 AS ( 1 )`. + pub(crate) fn parse_cte_body(&mut self, after_procedure: bool) -> Result { + if after_procedure + && !self.check(TokenType::Select) + && !self.check(TokenType::With) + && !self.check(TokenType::LParen) + { + return self.parse_expression(); + } + self.parse_statement() + } + /// `RETURNS { [[NOT] NULL] | TABLE ( ... ) }` after the parameters. pub(crate) fn parse_procedure_returns( &mut self, @@ -67,7 +79,7 @@ impl Parser { Ok((Some(data_type), None)) } - /// `( [ [DEFAULT ]] [, ...] )` with no empty or trailing entries. + /// `( [ [ [DEFAULT ]]] [, ...] )` with no empty or trailing entries. pub(crate) fn parse_anonymous_procedure_parameters( &mut self, ) -> Result> { @@ -78,7 +90,7 @@ impl Parser { self.expect_token(TokenType::Comma)?; } let name = self.expect_procedure_part_name()?; - let data_type = self.parse_data_type()?; + let data_type = self.parse_procedure_part_type()?; let default = if self.match_token(TokenType::Default) { Some(self.parse_expression()?) } else { @@ -96,7 +108,7 @@ impl Parser { Ok(parameters) } - /// `( [ ] [, ...] )` after `RETURNS TABLE`. + /// `( [ []] [, ...] )` after `RETURNS TABLE`. fn parse_procedure_table_columns(&mut self) -> Result> { self.expect_token(TokenType::LParen)?; let mut columns: Vec = Vec::new(); @@ -105,12 +117,35 @@ impl Parser { self.expect_token(TokenType::Comma)?; } let name = self.expect_procedure_part_name()?; - columns.push(StructField::new(name.name, self.parse_data_type()?)); + columns.push(StructField::new( + name.name, + self.parse_procedure_part_type()?, + )); } self.expect_token(TokenType::RParen)?; Ok(columns) } + /// A parameter or column type, unknown when the name stands alone, as Snowflake parses `( v1 )`. + fn parse_procedure_part_type(&mut self) -> Result { + if self.check(TokenType::Comma) || self.check(TokenType::RParen) { + return Ok(DataType::Unknown); + } + self.parse_data_type() + } + + /// A handler option value: a string, or a name that is quoted or not a reserved word. + fn parse_procedure_option_value(&mut self) -> Result { + if self.check(TokenType::String) { + return Ok(format!("'{}'", self.expect_string()?)); + } + let name = self.expect_procedure_part_name()?; + Ok(match name.quoted { + true => format!("\"{}\"", name.name), + false => name.name, + }) + } + /// A parameter or column name: an identifier, quoted or not a reserved word. fn expect_procedure_part_name(&mut self) -> Result { if !(self.check(TokenType::Identifier) @@ -136,7 +171,7 @@ impl Parser { { let key = self.previous_text().to_ascii_uppercase(); self.expect_token(TokenType::Eq)?; - format!("{key} = '{}'", self.expect_string()?) + format!("{key} = {}", self.parse_procedure_option_value()?) } else if self.match_identifier("PACKAGES") || self.match_identifier("IMPORTS") { let key = self.previous_text().to_ascii_uppercase(); self.expect_token(TokenType::Eq)?; @@ -169,7 +204,7 @@ impl Parser { } } - /// `( '' [, ...] )`, possibly empty. + /// `( [, ...] )`, possibly empty, where each value is a string or a name. fn parse_procedure_string_list(&mut self) -> Result> { self.expect_token(TokenType::LParen)?; let mut values: Vec = Vec::new(); @@ -177,7 +212,7 @@ impl Parser { if !values.is_empty() { self.expect_token(TokenType::Comma)?; } - values.push(format!("'{}'", self.expect_string()?)); + values.push(self.parse_procedure_option_value()?); } self.expect_token(TokenType::RParen)?; Ok(values) diff --git a/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs b/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs index 3f0570b42..c6ac8e30f 100644 --- a/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs +++ b/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs @@ -612,6 +612,12 @@ fn given_unsupported_snowflake_sql_when_parsing_then_parsing_fails() { sql: "WITH p1 AS PROCEDURE () LANGUAGE SQL AS $$ BEGIN RETURN 1; END $$ CALL p1()", expected_error: "Parse error", }, + ParseErrorTestCase { + description: "a reserved word is not a procedure option value", + dialect: Some(DialectType::Snowflake), + sql: "WITH p1 AS PROCEDURE () RETURNS INT LANGUAGE JAVA HANDLER = SELECT CALL p1()", + expected_error: "Parse error", + }, ParseErrorTestCase { description: "an anonymous procedure is followed by CALL", dialect: Some(DialectType::Snowflake), @@ -759,6 +765,13 @@ fn given_neighbours_of_rejected_snowflake_sql_when_parsing_then_they_parse() { pointer: "/0", expected_kind: Ok("call"), }, + ParseKindTestCase { + description: "untyped parts, name option values and an expression CTE after a procedure parse", + dialect: Some(DialectType::Snowflake), + sql: "WITH p1 AS PROCEDURE (v1) RETURNS TABLE (c1) LANGUAGE JAVA RUNTIME_VERSION = r1 IMPORTS = (m1) HANDLER = h1, c2 AS (1) CALL p1()", + pointer: "/0/call/with/ctes/1/this", + expected_kind: Ok("literal"), + }, ]; for test_case in test_cases { From 26de8c411f7c67a88b3c5f73e99b4b56a3b5d44e Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 5 Nov 2026 11:12:08 +0000 Subject: [PATCH 4/6] Keep Snowflake interval literal qualifier precisions INTERVAL '1' YEAR(9) TO MONTH parsed as a YEAR call spanning a MONTH variable, and SECOND(9), (3) left the fractional precision as a second select item. In Snowflake's faithful modes a qualifier with a field precision is now an interval qualifier node with its fields and its leading and fractional seconds precisions. Qualifiers without a precision keep their span or simple shape. --- .../src/expressions/types/models.rs | 15 +++ .../src/parser/_helpers/functions/generic.rs | 3 + .../src/parser/_helpers/types/casts.rs | 95 +++++++++++++++++++ .../tests/parser/test_faithful.rs | 37 ++++++++ 4 files changed, 150 insertions(+) diff --git a/crates/sqlquack-syntax/src/expressions/types/models.rs b/crates/sqlquack-syntax/src/expressions/types/models.rs index bd94ad8bc..501059bef 100644 --- a/crates/sqlquack-syntax/src/expressions/types/models.rs +++ b/crates/sqlquack-syntax/src/expressions/types/models.rs @@ -464,6 +464,21 @@ pub enum IntervalUnitSpec { ExprSpan(IntervalSpanExpr), /// Expression as unit (e.g., CURRENT_DATE, CAST(GETDATE() AS DATE)) Expr(Box), + /// Snowflake qualifier with a field precision, such as `YEAR(9) TO MONTH` or `SECOND(9), (3)`. + Qualifier(IntervalQualifier), +} + +/// Snowflake interval qualifier: fields with their leading and fractional seconds precisions. +#[derive(sqlquack_syntax_derive::AstNode, Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct IntervalQualifier { + /// Leading field (e.g., DAY) + pub this: IntervalUnit, + /// Leading field precision, as 9 in `DAY(9)` + pub leading_precision: Option, + /// Trailing field after TO (e.g., SECOND) + pub expression: Option, + /// Fractional seconds precision, as 3 in `DAY TO SECOND(3)` or `SECOND(9), (3)` + pub fractional_precision: Option, } /// Interval span for ranges like HOUR TO SECOND diff --git a/crates/sqlquack-syntax/src/parser/_helpers/functions/generic.rs b/crates/sqlquack-syntax/src/parser/_helpers/functions/generic.rs index 5429b2b9a..eca7d1cd8 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/functions/generic.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/functions/generic.rs @@ -1759,6 +1759,9 @@ impl Parser { /// Try to parse interval unit - returns None if no unit present pub(crate) fn try_parse_interval_unit(&mut self) -> Result> { + if let Some(qualifier) = self.try_parse_snowflake_interval_qualifier()? { + return Ok(Some(qualifier)); + } if self.is_function_start() { let start = self.parse_primary()?; if self.check_keyword_text("TO") { diff --git a/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs b/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs index f12821c52..9716c9158 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs @@ -5,6 +5,9 @@ use crate::expressions::operators::models::Case; use crate::expressions::operators::models::Cast; use crate::expressions::tree::models::Expression; use crate::expressions::types::models::DataType; +use crate::expressions::types::models::IntervalQualifier; +use crate::expressions::types::models::IntervalUnit; +use crate::expressions::types::models::IntervalUnitSpec; use crate::parser::_helpers::types::data_types::convert_name_is_known_custom; use crate::parser::constants::CHAR; use crate::parser::constants::CHARACTER; @@ -263,6 +266,98 @@ impl Parser { } } + /// Reads Snowflake's interval literal qualifier when a field has a precision, as `SECOND(9), (3)`. + pub(crate) fn try_parse_snowflake_interval_qualifier( + &mut self, + ) -> Result> { + let precise = self.is_type_at(1, TokenType::LParen) + || self.is_to_at(1) + && self.interval_field_at(2).is_some() + && self.is_type_at(3, TokenType::LParen); + let Some(this) = self + .interval_field_at(0) + .filter(|_| precise && self.applies_snowflake_context_rules()) + else { + return Ok(None); + }; + self.skip(); + let leading_precision = self.parse_snowflake_interval_precision()?; + let mut qualifier = IntervalQualifier { + this, + leading_precision, + expression: None, + fractional_precision: None, + }; + let fraction_follows = [ + TokenType::Comma, + TokenType::LParen, + TokenType::Number, + TokenType::RParen, + ] + .into_iter() + .enumerate() + .all(|(offset, token_type)| self.is_type_at(offset, token_type)); + if let Some(end) = self.interval_field_at(1).filter(|_| self.is_to_at(0)) { + self.skip(); + self.skip(); + qualifier.expression = Some(end); + if end == IntervalUnit::Second { + qualifier.fractional_precision = self.parse_snowflake_interval_precision()?; + } + } else if this == IntervalUnit::Second && leading_precision.is_some() && fraction_follows { + self.skip(); + qualifier.fractional_precision = self.parse_snowflake_interval_precision()?; + } + Ok(Some(IntervalUnitSpec::Qualifier(qualifier))) + } + + /// Whether the token `offset` ahead has type `token_type`. + fn is_type_at(&self, offset: usize, token_type: TokenType) -> bool { + self.tokens + .get(self.current + offset) + .is_some_and(|token| token.token_type == token_type) + } + + /// Whether the token `offset` ahead is TO. + fn is_to_at(&self, offset: usize) -> bool { + self.tokens + .get(self.current + offset) + .is_some_and(|token| token.text.eq_ignore_ascii_case("TO")) + } + + /// The Snowflake interval field `offset` ahead: YEAR, MONTH, DAY, HOUR, MINUTE or SECOND. + fn interval_field_at(&self, offset: usize) -> Option { + let text = self + .tokens + .get(self.current + offset)? + .text + .to_ascii_uppercase(); + match text.as_str() { + "YEAR" => Some(IntervalUnit::Year), + "MONTH" => Some(IntervalUnit::Month), + "DAY" => Some(IntervalUnit::Day), + "HOUR" => Some(IntervalUnit::Hour), + "MINUTE" => Some(IntervalUnit::Minute), + "SECOND" => Some(IntervalUnit::Second), + _ => None, + } + } + + /// Reads an optional `( n )` interval field precision. + fn parse_snowflake_interval_precision(&mut self) -> Result> { + if !self.match_token(TokenType::LParen) { + return Ok(None); + } + let number = self.check(TokenType::Number); + let digits = match self.peek_text().parse::() { + Ok(digits) if number => digits, + _ => return Err(self.parse_error("Expected an interval precision")), + }; + self.skip(); + self.expect_token(TokenType::RParen)?; + Ok(Some(digits)) + } + /// Refuses Snowflake's INTERVAL type without a field, or with TO and no field after it. pub(crate) fn refuse_snowflake_bare_interval( &self, diff --git a/crates/sqlquack-syntax/tests/parser/test_faithful.rs b/crates/sqlquack-syntax/tests/parser/test_faithful.rs index cc49824b1..5847ad22c 100644 --- a/crates/sqlquack-syntax/tests/parser/test_faithful.rs +++ b/crates/sqlquack-syntax/tests/parser/test_faithful.rs @@ -166,3 +166,40 @@ fn given_quoted_path_keys_with_dots_when_parsing_then_they_stay_in_brackets() { ); } } + +#[test] +fn given_an_interval_qualifier_with_precisions_when_parsing_faithfully_then_both_are_kept() { + let qualifier = |this, leading, to, fraction| { + json!({"type": "qualifier", "this": this, "leading_precision": leading, + "expression": to, "fractional_precision": fraction}) + }; + let test_cases = [ + ProjectionTestCase { + description: "YEAR(9) TO MONTH keeps the leading precision on its field", + sql: "SELECT INTERVAL '1' YEAR(9) TO MONTH", + pointer: "/interval/unit", + expected_value: qualifier("Year", json!(9), json!("Month"), json!(null)), + }, + ProjectionTestCase { + description: "SECOND(9), (3) keeps the fractional precision", + sql: "SELECT INTERVAL '1' SECOND(9), (3)", + pointer: "/interval/unit", + expected_value: qualifier("Second", json!(9), json!(null), json!(3)), + }, + ProjectionTestCase { + description: "DAY TO SECOND(3) keeps the fractional precision", + sql: "SELECT INTERVAL '1' DAY TO SECOND(3)", + pointer: "/interval/unit", + expected_value: qualifier("Day", json!(null), json!("Second"), json!(3)), + }, + ]; + + for test_case in test_cases { + assert_eq!( + projection_at(test_case.sql, test_case.pointer), + test_case.expected_value, + "{}", + test_case.description + ); + } +} From 52d15aa9d5227f0fdd3c0527ab64877cfbe91d6c Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 5 Nov 2026 11:46:03 +0000 Subject: [PATCH 5/6] Free chained expression trees without recursion A 70,000-term chain of ::, [1], .a, COLLATE, IS NULL or IN, or one mixing them with = or +, overflowed the stack while its tree was dropped, even after the depth guard had refused it. The AstNode derive now generates take_expressions, which moves a node's direct expression children out. BinaryOp, Cast, CollationExpr, DotAccess, In, IsNull, IsTrueFalse, JsonExtractFunc, LikeOp, Subscript, UnaryOp, Alias and Between drop through one worklist built on it, so any node between two chain links is freed iteratively too. This replaces the BinaryOp-only worklist. --- .../src/ast_node/_helpers/mod.rs | 1 + .../src/ast_node/_helpers/takers.rs | 124 ++++++++++++++++++ .../src/ast_node/main/expand_ast_node.rs | 9 ++ .../operators/_helpers/binary_ops.rs | 47 ------- .../src/expressions/operators/_helpers/mod.rs | 1 - .../src/expressions/operators/models.rs | 26 ---- .../src/expressions/tree/_helpers/drops.rs | 54 ++++++++ .../src/expressions/tree/_helpers/mod.rs | 1 + .../src/guard/tests/test_ast_guard.rs | 65 +++++++++ crates/sqlquack-syntax/src/traversal/types.rs | 3 + 10 files changed, 257 insertions(+), 74 deletions(-) create mode 100644 crates/sqlquack-syntax-derive/src/ast_node/_helpers/takers.rs delete mode 100644 crates/sqlquack-syntax/src/expressions/operators/_helpers/binary_ops.rs create mode 100644 crates/sqlquack-syntax/src/expressions/tree/_helpers/drops.rs diff --git a/crates/sqlquack-syntax-derive/src/ast_node/_helpers/mod.rs b/crates/sqlquack-syntax-derive/src/ast_node/_helpers/mod.rs index b611d6d49..2e68c481b 100644 --- a/crates/sqlquack-syntax-derive/src/ast_node/_helpers/mod.rs +++ b/crates/sqlquack-syntax-derive/src/ast_node/_helpers/mod.rs @@ -3,4 +3,5 @@ pub(crate) mod field_types; pub(crate) mod naming; pub(crate) mod spans; +pub(crate) mod takers; pub(crate) mod visitors; diff --git a/crates/sqlquack-syntax-derive/src/ast_node/_helpers/takers.rs b/crates/sqlquack-syntax-derive/src/ast_node/_helpers/takers.rs new file mode 100644 index 000000000..d81eb11d1 --- /dev/null +++ b/crates/sqlquack-syntax-derive/src/ast_node/_helpers/takers.rs @@ -0,0 +1,124 @@ +//! Takers: move a node's direct expression children out so a tree can be freed without recursion. + +use quote::format_ident; +use quote::quote; +use syn::Data; +use syn::DeriveInput; +use syn::Field; +use syn::Fields; +use syn::Type; +use syn::Variant; + +use crate::ast_node::_helpers::field_types::container_inner; +use crate::ast_node::_helpers::field_types::is_expression; +use crate::ast_node::_helpers::field_types::is_scalar; +use crate::ast_node::_helpers::field_types::is_skipped; + +/// The body of `take_expressions`, which moves every direct expression child into `out`. +pub(crate) fn node_taker(input: &DeriveInput) -> proc_macro2::TokenStream { + match &input.data { + Data::Struct(data) => struct_takes(&data.fields), + Data::Enum(data) => { + let mut arms: Vec = Vec::new(); + for variant in &data.variants { + arms.push(variant_arm(&input.ident, variant)); + } + quote!(match self { #(#arms),* }) + } + Data::Union(_) => quote!(), + } +} + +/// Takes from every traversed field of a struct. +fn struct_takes(fields: &Fields) -> proc_macro2::TokenStream { + let mut takes: Vec = Vec::new(); + for (index, field) in fields.iter().enumerate() { + if is_skipped(field) { + continue; + } + let access = match &field.ident { + Some(ident) => quote!(&mut self.#ident), + None => { + let index = syn::Index::from(index); + quote!(&mut self.#index) + } + }; + takes.push(take(&field.ty, access)); + } + quote!(#(#takes)*) +} + +/// Whether a field can hold expressions and is traversed. +fn is_kept(field: &Field) -> bool { + !is_skipped(field) && !is_scalar(&field.ty) +} + +/// One match arm that binds an enum variant's kept fields and takes from them. +fn variant_arm(enum_name: &syn::Ident, variant: &Variant) -> proc_macro2::TokenStream { + let variant_name = &variant.ident; + let mut bindings: Vec = Vec::new(); + let mut takes: Vec = Vec::new(); + for (index, field) in variant.fields.iter().enumerate() { + let binding = match &field.ident { + Some(ident) => ident.clone(), + None => format_ident!("field_{index}"), + }; + let kept = is_kept(field); + bindings.push(match (&field.ident, kept) { + (Some(ident), true) => quote!(#ident), + (Some(ident), false) => quote!(#ident: _), + (None, true) => quote!(#binding), + (None, false) => quote!(_), + }); + if kept { + takes.push(take(&field.ty, quote!(#binding))); + } + } + match &variant.fields { + Fields::Named(_) => quote!(#enum_name::#variant_name { #(#bindings),* } => { #(#takes)* }), + Fields::Unnamed(_) => quote!(#enum_name::#variant_name(#(#bindings),*) => { #(#takes)* }), + Fields::Unit => quote!(#enum_name::#variant_name => {}), + } +} + +/// Moves the expressions in a field reached through the `&mut` place `access` into `out`. +fn take(ty: &Type, access: proc_macro2::TokenStream) -> proc_macro2::TokenStream { + if is_expression(ty) { + return quote! { + { + let slot: &mut crate::expressions::tree::models::Expression = #access; + if !matches!(slot, crate::expressions::tree::models::Expression::Null(_)) { + out.push(::core::mem::replace( + slot, + crate::expressions::tree::models::Expression::Null( + crate::expressions::names::models::Null, + ), + )); + } + } + }; + } + if let Some(inner) = container_inner(ty, "Option") { + let take = take(inner, quote!(value)); + return quote!(if let Some(value) = (#access).as_mut() { #take }); + } + if let Some(inner) = container_inner(ty, "Box") { + return take(inner, quote!(&mut **(#access))); + } + if let Some(inner) = container_inner(ty, "Vec") { + let take = take(inner, quote!(value)); + return quote!(for value in (#access).iter_mut() { #take }); + } + if let Type::Tuple(tuple) = ty { + let mut takes: Vec = Vec::new(); + for (index, element) in tuple.elems.iter().enumerate() { + let index = syn::Index::from(index); + takes.push(take(element, quote!(&mut (#access).#index))); + } + return quote!(#(#takes)*); + } + if is_scalar(ty) { + return quote!(); + } + quote!(crate::traversal::types::AstNode::take_expressions(#access, out);) +} diff --git a/crates/sqlquack-syntax-derive/src/ast_node/main/expand_ast_node.rs b/crates/sqlquack-syntax-derive/src/ast_node/main/expand_ast_node.rs index 59d2a35e0..cdebdc08d 100644 --- a/crates/sqlquack-syntax-derive/src/ast_node/main/expand_ast_node.rs +++ b/crates/sqlquack-syntax-derive/src/ast_node/main/expand_ast_node.rs @@ -6,6 +6,7 @@ use syn::DeriveInput; use crate::ast_node::_helpers::naming::serde_snake_case; use crate::ast_node::_helpers::spans::span_methods; +use crate::ast_node::_helpers::takers::node_taker; use crate::ast_node::_helpers::visitors::node_visitor; use crate::constants::EXPRESSION_TYPE; @@ -14,6 +15,7 @@ pub(crate) fn expand_ast_node(input: &DeriveInput) -> proc_macro2::TokenStream { let immutable = node_visitor(input, true); let untracked = node_visitor(input, false); let spans = span_methods(input); + let taker = node_taker(input); let serialized_variant_names = if name == EXPRESSION_TYPE { if let Data::Enum(data) = &input.data { let names = data.variants.iter().map(|variant| { @@ -58,6 +60,13 @@ pub(crate) fn expand_ast_node(input: &DeriveInput) -> proc_macro2::TokenStream { #untracked } + fn take_expressions( + &mut self, + out: &mut Vec, + ) { + #taker + } + #spans } diff --git a/crates/sqlquack-syntax/src/expressions/operators/_helpers/binary_ops.rs b/crates/sqlquack-syntax/src/expressions/operators/_helpers/binary_ops.rs deleted file mode 100644 index 53d32648c..000000000 --- a/crates/sqlquack-syntax/src/expressions/operators/_helpers/binary_ops.rs +++ /dev/null @@ -1,47 +0,0 @@ -//! Reaching the [`BinaryOp`] inside an expression. - -use crate::expressions::operators::models::BinaryOp; -use crate::expressions::tree::models::Expression; - -/// The operands of every expression variant that wraps a [`BinaryOp`]. -pub(crate) fn binary_op_mut(expr: &mut Expression) -> Option<&mut BinaryOp> { - match expr { - Expression::And(op) - | Expression::Or(op) - | Expression::Add(op) - | Expression::Sub(op) - | Expression::Mul(op) - | Expression::Div(op) - | Expression::Mod(op) - | Expression::Eq(op) - | Expression::Neq(op) - | Expression::Lt(op) - | Expression::Lte(op) - | Expression::Gt(op) - | Expression::Gte(op) - | Expression::Match(op) - | Expression::BitwiseAnd(op) - | Expression::BitwiseOr(op) - | Expression::BitwiseXor(op) - | Expression::Concat(op) - | Expression::Adjacent(op) - | Expression::TsMatch(op) - | Expression::PropertyEQ(op) - | Expression::ArrayContainsAll(op) - | Expression::ArrayContainedBy(op) - | Expression::ArrayOverlaps(op) - | Expression::JSONBContainsAllTopKeys(op) - | Expression::JSONBContainsAnyTopKeys(op) - | Expression::JSONBDeleteAtPath(op) - | Expression::ExtendsLeft(op) - | Expression::ExtendsRight(op) - | Expression::Is(op) - | Expression::MemberOf(op) - | Expression::NullSafeEq(op) - | Expression::NullSafeNeq(op) - | Expression::Glob(op) - | Expression::BitwiseLeftShift(op) - | Expression::BitwiseRightShift(op) => Some(op), - _ => None, - } -} diff --git a/crates/sqlquack-syntax/src/expressions/operators/_helpers/mod.rs b/crates/sqlquack-syntax/src/expressions/operators/_helpers/mod.rs index 9e81baec8..fbfa07b43 100644 --- a/crates/sqlquack-syntax/src/expressions/operators/_helpers/mod.rs +++ b/crates/sqlquack-syntax/src/expressions/operators/_helpers/mod.rs @@ -1,4 +1,3 @@ //! Operators helpers. -pub(crate) mod binary_ops; pub(crate) mod boolean_chains; diff --git a/crates/sqlquack-syntax/src/expressions/operators/models.rs b/crates/sqlquack-syntax/src/expressions/operators/models.rs index e674c8139..73df99eac 100644 --- a/crates/sqlquack-syntax/src/expressions/operators/models.rs +++ b/crates/sqlquack-syntax/src/expressions/operators/models.rs @@ -5,7 +5,6 @@ use serde::Deserialize; use serde::Serialize; use crate::expressions::names::models::Identifier; -use crate::expressions::names::models::Null; use crate::expressions::names::models::TableRef; use crate::expressions::queries::models::Ordered; use crate::expressions::tree::models::Expression; @@ -162,31 +161,6 @@ pub struct BinaryOp { pub span: SourceSpan, } -/// Frees nested operators with a worklist so dropping a long chain such as `1 + 1 + ...` cannot overflow. -impl Drop for BinaryOp { - fn drop(&mut self) { - if crate::expressions::operators::_helpers::binary_ops::binary_op_mut(&mut self.left) - .is_none() - && crate::expressions::operators::_helpers::binary_ops::binary_op_mut(&mut self.right) - .is_none() - { - return; - } - let mut pending = vec![ - std::mem::replace(&mut self.left, Expression::Null(Null)), - std::mem::replace(&mut self.right, Expression::Null(Null)), - ]; - while let Some(mut expr) = pending.pop() { - if let Some(op) = - crate::expressions::operators::_helpers::binary_ops::binary_op_mut(&mut expr) - { - pending.push(std::mem::replace(&mut op.left, Expression::Null(Null))); - pending.push(std::mem::replace(&mut op.right, Expression::Null(Null))); - } - } - } -} - impl BinaryOp { pub fn new(left: Expression, right: Expression) -> Self { Self { diff --git a/crates/sqlquack-syntax/src/expressions/tree/_helpers/drops.rs b/crates/sqlquack-syntax/src/expressions/tree/_helpers/drops.rs new file mode 100644 index 000000000..a771d0fe5 --- /dev/null +++ b/crates/sqlquack-syntax/src/expressions/tree/_helpers/drops.rs @@ -0,0 +1,54 @@ +//! Iterative drops for nodes that the parser chains in a loop, such as `x::INT::INT` or `v[1][1]`. + +use crate::expressions::operators::models::Alias; +use crate::expressions::operators::models::Between; +use crate::expressions::operators::models::BinaryOp; +use crate::expressions::operators::models::Cast; +use crate::expressions::operators::models::CollationExpr; +use crate::expressions::operators::models::DotAccess; +use crate::expressions::operators::models::In; +use crate::expressions::operators::models::IsNull; +use crate::expressions::operators::models::IsTrueFalse; +use crate::expressions::operators::models::LikeOp; +use crate::expressions::operators::models::Subscript; +use crate::expressions::operators::models::UnaryOp; +use crate::expressions::tree::models::Expression; +use crate::expressions::typed_functions::models::JsonExtractFunc; +use crate::traversal::types::AstNode; + +/// Frees a node's expression descendants with a worklist, so dropping a long chain cannot overflow the stack. +fn drop_descendants(node: &mut impl AstNode) { + let mut pending: Vec = Vec::new(); + node.take_expressions(&mut pending); + while let Some(mut expression) = pending.pop() { + expression.take_expressions(&mut pending); + } +} + +macro_rules! drop_iteratively { + ($($node:ty),* $(,)?) => { + $( + impl Drop for $node { + fn drop(&mut self) { + drop_descendants(self); + } + } + )* + }; +} + +drop_iteratively!( + Alias, + Between, + BinaryOp, + Cast, + CollationExpr, + DotAccess, + In, + IsNull, + IsTrueFalse, + JsonExtractFunc, + LikeOp, + Subscript, + UnaryOp, +); diff --git a/crates/sqlquack-syntax/src/expressions/tree/_helpers/mod.rs b/crates/sqlquack-syntax/src/expressions/tree/_helpers/mod.rs index f830dbc47..ac239476d 100644 --- a/crates/sqlquack-syntax/src/expressions/tree/_helpers/mod.rs +++ b/crates/sqlquack-syntax/src/expressions/tree/_helpers/mod.rs @@ -2,3 +2,4 @@ pub(crate) mod accessors; pub(crate) mod api; +pub(crate) mod drops; diff --git a/crates/sqlquack-syntax/src/guard/tests/test_ast_guard.rs b/crates/sqlquack-syntax/src/guard/tests/test_ast_guard.rs index 9d7222683..6857d9f50 100644 --- a/crates/sqlquack-syntax/src/guard/tests/test_ast_guard.rs +++ b/crates/sqlquack-syntax/src/guard/tests/test_ast_guard.rs @@ -37,3 +37,68 @@ fn given_a_long_flat_operator_chain_when_parsing_then_only_real_nesting_counts_a ); } } + +#[test] +fn given_a_70k_term_chain_when_parsing_on_a_2_mb_thread_then_it_parses_or_is_refused_without_overflow( +) { + let test_cases = [ + AstDepthTestCase { + description: "a + chain is flat and parses", + sql: || format!("SELECT 1{}", "+1".repeat(70_000)), + expected_error: None, + }, + AstDepthTestCase { + description: "a || chain is flat and parses", + sql: || format!("SELECT 'a'{}", "||'a'".repeat(70_000)), + expected_error: None, + }, + AstDepthTestCase { + description: "a :: chain is refused", + sql: || format!("SELECT 1{}", "::INT".repeat(70_000)), + expected_error: Some("E_GUARD_AST_DEPTH_EXCEEDED"), + }, + AstDepthTestCase { + description: "an = chain is refused", + sql: || format!("SELECT 1{}", "=1".repeat(70_000)), + expected_error: Some("E_GUARD_AST_DEPTH_EXCEEDED"), + }, + AstDepthTestCase { + description: "a [1] chain is refused", + sql: || format!("SELECT v{}", "[1]".repeat(70_000)), + expected_error: Some("E_GUARD_AST_DEPTH_EXCEEDED"), + }, + AstDepthTestCase { + description: "a .a chain is refused", + sql: || format!("SELECT v{}", ".a".repeat(70_000)), + expected_error: Some("E_GUARD_AST_DEPTH_EXCEEDED"), + }, + AstDepthTestCase { + description: "a chain alternating = and IS NULL is refused", + sql: || format!("SELECT 1{}", "=1 IS NULL".repeat(70_000)), + expected_error: Some("E_GUARD_AST_DEPTH_EXCEEDED"), + }, + AstDepthTestCase { + description: "a :: chain cut off by a syntax error is refused", + sql: || format!("SELECT 1{}::", "::INT".repeat(70_000)), + expected_error: Some("Expected identifier"), + }, + ]; + for test_case in test_cases { + let sql = (test_case.sql)(); + let outcome = std::thread::Builder::new() + .stack_size(2 << 20) + .spawn(move || guarded_parse(&sql)) + .expect("the parse thread starts") + .join() + .expect("the parse thread finishes"); + assert_eq!( + outcome + .as_ref() + .err() + .map(|error| error.contains(test_case.expected_error.unwrap_or("\0"))), + test_case.expected_error.map(|_| true), + "{}: {outcome:?}", + test_case.description + ); + } +} diff --git a/crates/sqlquack-syntax/src/traversal/types.rs b/crates/sqlquack-syntax/src/traversal/types.rs index a8e22f4d0..dc0e4dd02 100644 --- a/crates/sqlquack-syntax/src/traversal/types.rs +++ b/crates/sqlquack-syntax/src/traversal/types.rs @@ -18,6 +18,9 @@ pub(crate) trait AstNode { where F: FnMut(&[ChildPathSegment], &'ast Expression); + /// Moves every direct expression child into `out`, leaving NULL in its place. + fn take_expressions(&mut self, out: &mut Vec); + /// SQLQuack: the node's own recorded source range, when it has a `span` field holding one. fn recorded_span(&self) -> Option { None From f28341e6c37687759cad688d5556b31d98dc00dc Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 5 Nov 2026 11:54:09 +0000 Subject: [PATCH 6/6] Cover COLLATE, IS NULL, IN and path chains in the 70k-term drop test --- .../src/guard/tests/test_ast_guard.rs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/crates/sqlquack-syntax/src/guard/tests/test_ast_guard.rs b/crates/sqlquack-syntax/src/guard/tests/test_ast_guard.rs index 6857d9f50..2b3ed2824 100644 --- a/crates/sqlquack-syntax/src/guard/tests/test_ast_guard.rs +++ b/crates/sqlquack-syntax/src/guard/tests/test_ast_guard.rs @@ -72,6 +72,26 @@ fn given_a_70k_term_chain_when_parsing_on_a_2_mb_thread_then_it_parses_or_is_ref sql: || format!("SELECT v{}", ".a".repeat(70_000)), expected_error: Some("E_GUARD_AST_DEPTH_EXCEEDED"), }, + AstDepthTestCase { + description: "a COLLATE chain is refused", + sql: || format!("SELECT 'a'{}", " COLLATE 'en'".repeat(70_000)), + expected_error: Some("E_GUARD_AST_DEPTH_EXCEEDED"), + }, + AstDepthTestCase { + description: "an IS NULL chain is refused", + sql: || format!("SELECT 1{}", " IS NULL".repeat(70_000)), + expected_error: Some("E_GUARD_AST_DEPTH_EXCEEDED"), + }, + AstDepthTestCase { + description: "an IN chain is refused", + sql: || format!("SELECT 1{}", " IN (1)".repeat(70_000)), + expected_error: Some("E_GUARD_AST_DEPTH_EXCEEDED"), + }, + AstDepthTestCase { + description: "a : path chain is one path and parses", + sql: || format!("SELECT v{}", ":a".repeat(70_000)), + expected_error: None, + }, AstDepthTestCase { description: "a chain alternating = and IS NULL is refused", sql: || format!("SELECT 1{}", "=1 IS NULL".repeat(70_000)),