From 20bdaa5059f44e018d03c1ffd854441621980057 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 11 Nov 2026 09:48:50 +0000 Subject: [PATCH] refactor(syntax): drop the lowered call mode CallMode::Lowered reproduced Polyglot's call lowering for the removed equivalence check. Nothing outside the parser's own tests and nested procedure bodies of a lowered parse used it, so the parser now always keeps Snowflake calls as written, and the config flag that switched it goes too. Standalone type parsing, which used the flag's default, follows the same path; every corpus verdict is unchanged. The ported test that read TRY_TO_NUMBER's lowered precision and scale goes with the mode. The tests that compared the two modes now assert the node each call keeps: keyword syntax stays typed, kept calls stay calls, and DuckDB keeps its typed nodes. --- .../src/parser/_helpers/expressions/paths.rs | 2 +- .../src/parser/_helpers/functions/typed.rs | 6 +- .../parser/_helpers/statements/procedures.rs | 12 +-- .../_helpers/statements/snowflake_context.rs | 3 +- .../src/parser/main/parse_expressions.rs | 1 - .../src/parser/models.rs | 7 +- .../src/parser/tests/test_snowflake.rs | 47 --------- .../tests/parser/helpers.rs | 7 +- .../tests/parser/test_faithful.rs | 99 ++++++++++--------- .../tests/parser/test_scripting.rs | 6 +- .../tests/parser/test_types.rs | 6 +- crates/sqlquack-syntax/src/tokens/models.rs | 2 - 12 files changed, 72 insertions(+), 126 deletions(-) diff --git a/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/paths.rs b/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/paths.rs index 0edc87235..636b92af8 100644 --- a/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/paths.rs +++ b/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/paths.rs @@ -334,7 +334,7 @@ impl Parser { fn parse_json_path_dot_key(&mut self, mut path_string: String) -> Result { path_string.push('.'); - if self.config.faithful_calls && self.check(TokenType::QuotedIdentifier) { + if self.check(TokenType::QuotedIdentifier) { let part = self.advance_text()?; if self.faithful_path_key_needs_brackets(&part) || part.contains(' ') { path_string.pop(); diff --git a/crates/sqlquack-syntax-snowflake/src/parser/_helpers/functions/typed.rs b/crates/sqlquack-syntax-snowflake/src/parser/_helpers/functions/typed.rs index 1387b5bab..c121445fa 100644 --- a/crates/sqlquack-syntax-snowflake/src/parser/_helpers/functions/typed.rs +++ b/crates/sqlquack-syntax-snowflake/src/parser/_helpers/functions/typed.rs @@ -195,9 +195,7 @@ impl Parser { upper_name: &str, quoted: bool, ) -> Result { - if self.config.faithful_calls - && self.config.dialect == Some(sqlquack_syntax::tokens::models::DialectType::Snowflake) - { + if self.config.dialect == Some(sqlquack_syntax::tokens::models::DialectType::Snowflake) { return self.parse_faithful_call(name, upper_name, quoted); } self.parse_typed_function_lowered(name, upper_name, quoted) @@ -249,7 +247,7 @@ impl Parser { /// SQLQuack: whether a faithful parse keeps a path key in brackets, so the path stays unambiguous. pub(crate) fn faithful_path_key_needs_brackets(&self, key: &str) -> bool { - self.config.faithful_calls && key.contains(['.', '[', ']', '"', ':']) + key.contains(['.', '[', ']', '"', ':']) } /// SQLQuack: a call as written, unless polyglot keeps it or the typed parse is the call itself. diff --git a/crates/sqlquack-syntax-snowflake/src/parser/_helpers/statements/procedures.rs b/crates/sqlquack-syntax-snowflake/src/parser/_helpers/statements/procedures.rs index 0db5051f2..c6b878fd6 100644 --- a/crates/sqlquack-syntax-snowflake/src/parser/_helpers/statements/procedures.rs +++ b/crates/sqlquack-syntax-snowflake/src/parser/_helpers/statements/procedures.rs @@ -229,13 +229,13 @@ impl Parser { if language != Some("SQL") { return Ok(FunctionBody::StringLiteral(body)); } - let mode = if self.config.faithful_calls { - CallMode::Faithful - } else { - CallMode::Lowered - }; let statements = Dialect::get(DialectType::Snowflake) - .parse_with_config(&body, self.config.complexity_guard, mode, None) + .parse_with_config( + &body, + self.config.complexity_guard, + CallMode::Faithful, + None, + ) .map_err(|error| self.parse_error(format!("In the procedure body: {error}")))?; if statements.is_empty() { return Err(self.parse_error("Expected a procedure body")); diff --git a/crates/sqlquack-syntax-snowflake/src/parser/_helpers/statements/snowflake_context.rs b/crates/sqlquack-syntax-snowflake/src/parser/_helpers/statements/snowflake_context.rs index cce6bb042..0df729499 100644 --- a/crates/sqlquack-syntax-snowflake/src/parser/_helpers/statements/snowflake_context.rs +++ b/crates/sqlquack-syntax-snowflake/src/parser/_helpers/statements/snowflake_context.rs @@ -1,4 +1,4 @@ -//! Snowflake's statement-context rules, which the lowered mode skips. +//! Snowflake's statement-context rules. use crate::parser::models::Parser; use sqlquack_syntax::expressions::tree::models::Expression; @@ -8,7 +8,6 @@ impl Parser { /// Whether Snowflake's statement-context rules apply: in SQLQuack's own faithful modes only. pub(crate) fn applies_snowflake_context_rules(&self) -> bool { self.config.dialect == Some(sqlquack_syntax::tokens::models::DialectType::Snowflake) - && self.config.faithful_calls } /// Refuses a backtick-quoted identifier, which Snowflake does not tokenize, in its faithful modes. diff --git a/crates/sqlquack-syntax-snowflake/src/parser/main/parse_expressions.rs b/crates/sqlquack-syntax-snowflake/src/parser/main/parse_expressions.rs index d868335ae..512b63ac3 100644 --- a/crates/sqlquack-syntax-snowflake/src/parser/main/parse_expressions.rs +++ b/crates/sqlquack-syntax-snowflake/src/parser/main/parse_expressions.rs @@ -13,7 +13,6 @@ use crate::parser::_helpers::support::dialects::dialect_type; pub fn parse_expressions(tokens: Vec, dialect: Dialect) -> Result> { let config = ParserConfig { dialect: Some(dialect_type(dialect)?), - faithful_calls: true, ..Default::default() }; Parser::with_config(tokens, config).parse_expression_list() diff --git a/crates/sqlquack-syntax-snowflake/src/parser/models.rs b/crates/sqlquack-syntax-snowflake/src/parser/models.rs index 38a4cfe92..dea059fe3 100644 --- a/crates/sqlquack-syntax-snowflake/src/parser/models.rs +++ b/crates/sqlquack-syntax-snowflake/src/parser/models.rs @@ -87,7 +87,7 @@ impl Dialect { ) } - /// SQLQuack: parses with polyglot's call lowering (`faithful_calls` false) or without it. + /// SQLQuack: parses in `mode`, recording node ranges when it is located. pub(crate) fn parse_with_mode( &self, sql: &str, @@ -122,7 +122,7 @@ impl Dialect { sql: &str, complexity_guard: ComplexityGuardOptions, ) -> Result> { - self.parse_with_config(sql, complexity_guard, CallMode::Lowered, None) + self.parse_with_config(sql, complexity_guard, CallMode::Faithful, None) } pub(crate) fn parse_with_config( @@ -141,7 +141,6 @@ impl Dialect { let config = crate::parser::models::ParserConfig { dialect: Some(self.dialect_type), complexity_guard, - faithful_calls: mode != CallMode::Lowered, record_node_spans: mode == CallMode::FaithfulLocated, ..Default::default() }; @@ -437,8 +436,6 @@ pub(crate) struct ParserConfig { pub dialect: Option, /// Complexity guards for recursion-heavy parse paths. pub complexity_guard: ComplexityGuardOptions, - /// SQLQuack: in Snowflake, keep calls as written; only keyword-syntax calls become typed nodes. - pub faithful_calls: bool, /// SQLQuack: record each expression and statement node's source range (for locations). pub record_node_spans: bool, /// Whether a Snowflake CREATE SEMANTIC VIEW is being checked, where a window may PARTITION BY EXCLUDING. diff --git a/crates/sqlquack-syntax-snowflake/src/parser/tests/test_snowflake.rs b/crates/sqlquack-syntax-snowflake/src/parser/tests/test_snowflake.rs index 70688da9a..c0797cbcc 100644 --- a/crates/sqlquack-syntax-snowflake/src/parser/tests/test_snowflake.rs +++ b/crates/sqlquack-syntax-snowflake/src/parser/tests/test_snowflake.rs @@ -1,13 +1,9 @@ -use serde_json::json; - use crate::parser::tests::helpers::first_token_text; use crate::parser::tests::helpers::kind_at; use crate::parser::tests::helpers::parse_error; -use crate::parser::tests::helpers::value_at; use crate::parser::tests::test_types::DialectTokenTestCase; use crate::parser::tests::test_types::ParseErrorTestCase; use crate::parser::tests::test_types::ParseKindTestCase; -use crate::parser::tests::test_types::ParseValueTestCase; use sqlquack_syntax::tokens::models::DialectType; use sqlquack_syntax::tokens::models::TokenType; @@ -398,49 +394,6 @@ fn given_connector_generated_snowflake_sql_when_parsing_then_each_is_one_stateme } } -#[test] -fn given_try_to_number_with_numbers_when_parsing_snowflake_then_they_are_precision_and_scale() { - let test_cases = [ - ParseValueTestCase { - description: "TRY_TO_NUMBER has no format", - dialect: Some(DialectType::Snowflake), - sql: "SELECT TRY_TO_NUMBER('12.34', 9, 2)", - pointer: "/0/select/expressions/0/to_number/format", - expected_value: Ok(json!(null)), - }, - ParseValueTestCase { - description: "TRY_TO_NUMBER reads its second argument as the precision", - dialect: Some(DialectType::Snowflake), - sql: "SELECT TRY_TO_NUMBER('12.34', 9, 2)", - pointer: "/0/select/expressions/0/to_number/precision/literal/value", - expected_value: Ok(json!("9")), - }, - ParseValueTestCase { - description: "TRY_TO_NUMBER reads its third argument as the scale", - dialect: Some(DialectType::Snowflake), - sql: "SELECT TRY_TO_NUMBER('12.34', 9, 2)", - pointer: "/0/select/expressions/0/to_number/scale/literal/value", - expected_value: Ok(json!("2")), - }, - ParseValueTestCase { - description: "TRY_TO_NUMBER is the safe conversion", - dialect: Some(DialectType::Snowflake), - sql: "SELECT TRY_TO_NUMBER('12.34', 9, 2)", - pointer: "/0/select/expressions/0/to_number/safe/boolean/value", - expected_value: Ok(json!(true)), - }, - ]; - - for test_case in test_cases { - assert_eq!( - value_at(test_case.dialect, test_case.sql, test_case.pointer), - test_case.expected_value, - "{}", - test_case.description - ); - } -} - #[test] fn given_unsupported_snowflake_sql_when_parsing_then_parsing_fails() { let test_cases = [ diff --git a/crates/sqlquack-syntax-snowflake/tests/parser/helpers.rs b/crates/sqlquack-syntax-snowflake/tests/parser/helpers.rs index e8eb7ee15..0e879469a 100644 --- a/crates/sqlquack-syntax-snowflake/tests/parser/helpers.rs +++ b/crates/sqlquack-syntax-snowflake/tests/parser/helpers.rs @@ -24,7 +24,7 @@ pub(crate) fn kind(value: &Value) -> String { .unwrap_or_default() } -pub(crate) fn faithful_call(sql: &str) -> (Value, Option, String) { +pub(crate) fn faithful_call(sql: &str) -> (Value, Option) { let faithful = first_projection(sql, CallMode::Faithful); let payload = faithful .get("function") @@ -34,14 +34,9 @@ pub(crate) fn faithful_call(sql: &str) -> (Value, Option, String) { ( payload["name"].clone(), payload["args"].as_array().map(Vec::len), - kind(&first_projection(sql, CallMode::Lowered)), ) } -pub(crate) fn same_in_both_modes(sql: &str, dialect: Dialect) -> bool { - statements(sql, dialect, CallMode::Faithful) == statements(sql, dialect, CallMode::Lowered) -} - pub(crate) fn projection_at(sql: &str, pointer: &str) -> Value { first_projection(sql, CallMode::Faithful) .pointer(pointer) diff --git a/crates/sqlquack-syntax-snowflake/tests/parser/test_faithful.rs b/crates/sqlquack-syntax-snowflake/tests/parser/test_faithful.rs index 21365c68d..d91263389 100644 --- a/crates/sqlquack-syntax-snowflake/tests/parser/test_faithful.rs +++ b/crates/sqlquack-syntax-snowflake/tests/parser/test_faithful.rs @@ -1,51 +1,51 @@ use serde_json::json; -use sqlquack_syntax::Dialect; +use sqlquack_syntax::{CallMode, Dialect}; -use crate::helpers::{faithful_call, projection_at, same_in_both_modes}; -use crate::test_types::{FaithfulCallTestCase, ModeAgreementTestCase, ProjectionTestCase}; +use crate::helpers::{faithful_call, kind, projection_at, statements}; +use crate::test_types::{FaithfulCallTestCase, ProjectionKindTestCase, ProjectionTestCase}; #[test] fn given_lowered_spellings_when_parsing_faithfully_then_calls_stay_as_written() { let test_cases = [ FaithfulCallTestCase { - description: "LEN stays a call with 1 arguments and lowers to length", + description: "LEN stays a call with 1 arguments", sql: "SELECT LEN(s)", - expected_call: (json!("LEN"), Some(1), "length".to_owned()), + expected_call: (json!("LEN"), Some(1)), }, FaithfulCallTestCase { - description: "IFNULL stays a call with 2 arguments and lowers to coalesce", + description: "IFNULL stays a call with 2 arguments", sql: "SELECT IFNULL(a, b)", - expected_call: (json!("IFNULL"), Some(2), "coalesce".to_owned()), + expected_call: (json!("IFNULL"), Some(2)), }, FaithfulCallTestCase { - description: "IFF stays a call with 3 arguments and lowers to if_func", + description: "IFF stays a call with 3 arguments", sql: "SELECT IFF(c, 1, 2)", - expected_call: (json!("IFF"), Some(3), "if_func".to_owned()), + expected_call: (json!("IFF"), Some(3)), }, FaithfulCallTestCase { - description: "TRY_TO_NUMBER stays a call with 1 arguments and lowers to to_number", + description: "TRY_TO_NUMBER stays a call with 1 arguments", sql: "SELECT TRY_TO_NUMBER(s)", - expected_call: (json!("TRY_TO_NUMBER"), Some(1), "to_number".to_owned()), + expected_call: (json!("TRY_TO_NUMBER"), Some(1)), }, FaithfulCallTestCase { - description: "MOD stays a call with 2 arguments and lowers to mod_func", + description: "MOD stays a call with 2 arguments", sql: "SELECT MOD(a, b)", - expected_call: (json!("MOD"), Some(2), "mod_func".to_owned()), + expected_call: (json!("MOD"), Some(2)), }, FaithfulCallTestCase { - description: "SUM stays a call with 1 arguments and lowers to sum", + description: "SUM stays a call with 1 arguments", sql: "SELECT SUM(x)", - expected_call: (json!("SUM"), Some(1), "sum".to_owned()), + expected_call: (json!("SUM"), Some(1)), }, FaithfulCallTestCase { - description: "COUNT stays a call with 1 arguments and lowers to count", + description: "COUNT stays a call with 1 arguments", sql: "SELECT COUNT(*)", - expected_call: (json!("COUNT"), Some(1), "count".to_owned()), + expected_call: (json!("COUNT"), Some(1)), }, FaithfulCallTestCase { - description: "ROW_NUMBER stays a call with 0 arguments and lowers to row_number", + description: "ROW_NUMBER stays a call with 0 arguments", sql: "SELECT ROW_NUMBER()", - expected_call: (json!("ROW_NUMBER"), Some(0), "row_number".to_owned()), + expected_call: (json!("ROW_NUMBER"), Some(0)), }, ]; @@ -60,56 +60,67 @@ fn given_lowered_spellings_when_parsing_faithfully_then_calls_stay_as_written() } #[test] -fn given_keyword_syntax_and_kept_calls_when_parsing_faithfully_then_both_modes_agree() { +fn given_keyword_syntax_and_kept_calls_when_parsing_faithfully_then_they_keep_their_typed_node() { let test_cases = [ - ModeAgreementTestCase { - description: "SELECT EXTRACT(year FROM d) parses the same in both modes", + ProjectionKindTestCase { + description: "SELECT EXTRACT(year FROM d): keyword syntax stays a typed node", sql: "SELECT EXTRACT(year FROM d)", dialect: Dialect::Snowflake, - expected_same: true, + expected_kind: "extract", }, - ModeAgreementTestCase { - description: "SELECT POSITION('a' IN s) parses the same in both modes", + ProjectionKindTestCase { + description: "SELECT POSITION('a' IN s): keyword syntax stays a typed node", sql: "SELECT POSITION('a' IN s)", dialect: Dialect::Snowflake, - expected_same: true, + expected_kind: "str_position", }, - ModeAgreementTestCase { - description: "SELECT TRIM(BOTH ' ' FROM s) parses the same in both modes", + ProjectionKindTestCase { + description: "SELECT TRIM(BOTH ' ' FROM s): keyword syntax stays a typed node", sql: "SELECT TRIM(BOTH ' ' FROM s)", dialect: Dialect::Snowflake, - expected_same: true, + expected_kind: "trim", }, - ModeAgreementTestCase { - description: "SELECT DATEADD(day, 1, d) parses the same in both modes", + ProjectionKindTestCase { + description: + "SELECT DATEADD(day, 1, d): a call the typed parse keeps under its own name", sql: "SELECT DATEADD(day, 1, d)", dialect: Dialect::Snowflake, - expected_same: true, + expected_kind: "function", }, - ModeAgreementTestCase { - description: "SELECT DATE_PART(hour, t) parses the same in both modes", + ProjectionKindTestCase { + description: + "SELECT DATE_PART(hour, t): a call the typed parse keeps under its own name", sql: "SELECT DATE_PART(hour, t)", dialect: Dialect::Snowflake, - expected_same: true, + expected_kind: "function", }, - ModeAgreementTestCase { - description: "SELECT CONCAT(a, b) parses the same in both modes", + ProjectionKindTestCase { + description: "SELECT CONCAT(a, b): a call the typed parse keeps under its own name", sql: "SELECT CONCAT(a, b)", dialect: Dialect::Snowflake, - expected_same: true, + expected_kind: "function", }, - ModeAgreementTestCase { - description: "DuckDB calls are not changed", - sql: "SELECT length(s), coalesce(a, b)", + ProjectionKindTestCase { + description: "SELECT length(s): DuckDB keeps its typed nodes", + sql: "SELECT length(s)", dialect: Dialect::DuckDB, - expected_same: true, + expected_kind: "length", + }, + ProjectionKindTestCase { + description: "SELECT coalesce(a, b): DuckDB keeps its typed nodes", + sql: "SELECT coalesce(a, b)", + dialect: Dialect::DuckDB, + expected_kind: "coalesce", }, ]; for test_case in test_cases { assert_eq!( - same_in_both_modes(test_case.sql, test_case.dialect), - test_case.expected_same, + kind( + &statements(test_case.sql, test_case.dialect, CallMode::Faithful)[0]["select"] + ["expressions"][0] + ), + test_case.expected_kind, "{}", test_case.description ); diff --git a/crates/sqlquack-syntax-snowflake/tests/parser/test_scripting.rs b/crates/sqlquack-syntax-snowflake/tests/parser/test_scripting.rs index 3f12ce90d..8e84ee3cf 100644 --- a/crates/sqlquack-syntax-snowflake/tests/parser/test_scripting.rs +++ b/crates/sqlquack-syntax-snowflake/tests/parser/test_scripting.rs @@ -8,11 +8,7 @@ use crate::test_types::{ ScriptingVerdictTestCase, ScriptingWalkTestCase, }; -const MODES: [CallMode; 3] = [ - CallMode::Faithful, - CallMode::FaithfulLocated, - CallMode::Lowered, -]; +const MODES: [CallMode; 2] = [CallMode::Faithful, CallMode::FaithfulLocated]; #[test] fn given_scripting_blocks_when_parsing_then_members_keep_written_order_and_operands() { diff --git a/crates/sqlquack-syntax-snowflake/tests/parser/test_types.rs b/crates/sqlquack-syntax-snowflake/tests/parser/test_types.rs index 6cd99acaa..d7908b555 100644 --- a/crates/sqlquack-syntax-snowflake/tests/parser/test_types.rs +++ b/crates/sqlquack-syntax-snowflake/tests/parser/test_types.rs @@ -4,14 +4,14 @@ use sqlquack_syntax::Dialect; pub(crate) struct FaithfulCallTestCase { pub(crate) description: &'static str, pub(crate) sql: &'static str, - pub(crate) expected_call: (Value, Option, String), + pub(crate) expected_call: (Value, Option), } -pub(crate) struct ModeAgreementTestCase { +pub(crate) struct ProjectionKindTestCase { pub(crate) description: &'static str, pub(crate) sql: &'static str, pub(crate) dialect: Dialect, - pub(crate) expected_same: bool, + pub(crate) expected_kind: &'static str, } pub(crate) struct ProjectionTestCase { diff --git a/crates/sqlquack-syntax/src/tokens/models.rs b/crates/sqlquack-syntax/src/tokens/models.rs index 5d32e1d1b..c602d6115 100644 --- a/crates/sqlquack-syntax/src/tokens/models.rs +++ b/crates/sqlquack-syntax/src/tokens/models.rs @@ -46,8 +46,6 @@ pub enum Dialect { pub enum CallMode { /// Every Snowflake call keeps its name and arguments as written; SQLQuack's mode. Faithful, - /// polyglot's call lowering, used by the parser's own tests; skips statement-context rules. - Lowered, /// As [`CallMode::Faithful`], recording each node's source range (for refusal locations). FaithfulLocated, }