From fa90854a97521e937e6d9467e675bfdb68af18b2 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 22:36:35 +0000 Subject: [PATCH 01/10] Snowflake parser: PIVOT and UNPIVOT as Snowflake reads them From the parser-only disagreements, each checked against receipts: - a PIVOT aggregate is a call of a column: a parenthesised column or a string argument is a syntax error, as is a string alias; - a PIVOT IN list is not empty, and ANY ORDER needs BY; - PIVOT and UNPIVOT IN values take an alias without AS; - an UNPIVOT IN list holds columns, not strings, and its name column is no reserved keyword. --- .../src/parser/_helpers/queries/mod.rs | 1 + .../_helpers/queries/snowflake_pivot.rs | 60 ++++++++++++++++++ .../src/parser/_helpers/queries/tables.rs | 20 +++++- .../tests/parser/test_fixes.rs | 61 +++++++++++++++++++ 4 files changed, 140 insertions(+), 2 deletions(-) create mode 100644 crates/sqlquack-syntax/src/parser/_helpers/queries/snowflake_pivot.rs diff --git a/crates/sqlquack-syntax/src/parser/_helpers/queries/mod.rs b/crates/sqlquack-syntax/src/parser/_helpers/queries/mod.rs index d75b2b1f1..7d2d65a8b 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/queries/mod.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/queries/mod.rs @@ -5,4 +5,5 @@ pub(crate) mod insert; pub(crate) mod joins; pub(crate) mod modifiers; pub(crate) mod select; +pub(crate) mod snowflake_pivot; pub(crate) mod tables; diff --git a/crates/sqlquack-syntax/src/parser/_helpers/queries/snowflake_pivot.rs b/crates/sqlquack-syntax/src/parser/_helpers/queries/snowflake_pivot.rs new file mode 100644 index 000000000..a2a23b80e --- /dev/null +++ b/crates/sqlquack-syntax/src/parser/_helpers/queries/snowflake_pivot.rs @@ -0,0 +1,60 @@ +//! Snowflake's PIVOT and UNPIVOT rules: an aggregate of a column, non-empty lists, bare aliases. + +use crate::expressions::tree::models::Expression; +use crate::parser::models::Parser; +use crate::tokens::models::TokenType; +use crate::tokens::types::Result; + +impl Parser { + /// Refuses a Snowflake PIVOT aggregate that is no call, or that aggregates a string. + pub(crate) fn refuse_snowflake_pivot_aggregate(&self, aggregate: &Expression) -> Result<()> { + use crate::traversal::types::ExpressionWalk; + if !self.is_snowflake() { + return Ok(()); + } + let no_call = matches!( + aggregate, + Expression::Paren(_) + | Expression::Column(_) + | Expression::Literal(_) + | Expression::Tuple(_) + | Expression::Identifier(_) + ); + let of_string = aggregate.children().iter().any( + |argument| matches!(argument, Expression::Literal(literal) if literal.is_string()), + ); + match no_call || of_string { + true => Err(self.parse_error("Expected an aggregate of a column before FOR")), + false => Ok(()), + } + } + + /// Refuses Snowflake's `IN ( ANY ORDER ... )` without BY. + pub(crate) fn expect_snowflake_order_by(&self) -> Result<()> { + match self.is_snowflake() && self.check(TokenType::Order) && !self.check_next(TokenType::By) + { + true => Err(self.parse_error("Expected BY after ORDER")), + false => Ok(()), + } + } + + /// Refuses an empty Snowflake PIVOT IN list. + pub(crate) fn refuse_snowflake_empty_pivot_list(&self, values: &[Expression]) -> Result<()> { + match self.is_snowflake() && values.is_empty() { + true => Err(self.parse_error("Expected a value in the PIVOT IN list")), + false => Ok(()), + } + } + + /// A Snowflake PIVOT or UNPIVOT IN alias written without AS, or none. + pub(crate) fn parse_snowflake_bare_pivot_alias(&mut self) -> Result> { + let bare = self.is_snowflake() + && (self.check(TokenType::Var) + || self.check(TokenType::Identifier) + || self.check(TokenType::QuotedIdentifier)); + match bare { + true => Ok(self.parse_bitwise()?), + false => Ok(None), + } + } +} diff --git a/crates/sqlquack-syntax/src/parser/_helpers/queries/tables.rs b/crates/sqlquack-syntax/src/parser/_helpers/queries/tables.rs index 07d99321c..b5c3336b7 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/queries/tables.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/queries/tables.rs @@ -1068,6 +1068,7 @@ impl Parser { break; } let func = self.parse_primary()?; + self.refuse_snowflake_pivot_aggregate(&func)?; let expr = if self.match_token(TokenType::As) { let alias_name = self.expect_identifier_or_keyword()?; Expression::Alias(Box::new(Alias::new(func, Identifier::new(alias_name)))) @@ -1075,6 +1076,9 @@ impl Parser { && !self.check(TokenType::For) && !self.check(TokenType::RParen) { + if self.is_snowflake() && self.check(TokenType::String) { + return Err(self.parse_error("Expected an identifier alias in PIVOT")); + } if let Some(id) = self.parse_id_var()? { let alias_name = match &id { Expression::Identifier(ident) => ident.name.clone(), @@ -1153,6 +1157,7 @@ impl Parser { if self.match_token(TokenType::LParen) { let in_exprs = if self.match_text_seq(&["ANY"]) { + self.expect_snowflake_order_by()?; let order = self.parse_order()?; vec![Expression::PivotAny(Box::new(PivotAny { this: order.map(Box::new), @@ -1161,10 +1166,13 @@ impl Parser { let mut vals: Vec = Vec::new(); loop { if self.check(TokenType::RParen) { + self.refuse_snowflake_empty_pivot_list(&vals)?; break; } if let Some(val) = self.parse_select_or_expression()? { - let val = if self.match_token(TokenType::As) { + let val = if let Some(alias) = self.parse_snowflake_bare_pivot_alias()? { + Expression::PivotAlias(Box::new(PivotAlias { this: val, alias })) + } else if self.match_token(TokenType::As) { let alias_expr = self.parse_bitwise()?.ok_or_else(|| { self.parse_error( "Expected expression after AS in PIVOT/UNPIVOT IN clause", @@ -1242,6 +1250,9 @@ impl Parser { }; self.expect_token(TokenType::For)?; + if self.is_snowflake() && self.is_snowflake_reserved_keyword() { + return Err(self.parse_error("Expected a column name after FOR")); + } let name_column = self.parse_unquoted_identifier(Self::expect_identifier_or_keyword)?; self.expect_token(TokenType::In)?; @@ -1286,6 +1297,9 @@ impl Parser { self.parse_select_or_expression()? } } else { + if snowflake && self.check(TokenType::String) { + return Err(self.parse_error("Expected a column in the UNPIVOT IN list")); + } self.parse_select_or_expression()? }; @@ -1294,7 +1308,9 @@ impl Parser { { return Err(self.parse_error("Expected an identifier alias in UNPIVOT IN")); } - let col = if self.match_token(TokenType::As) { + let col = if let Some(alias) = self.parse_snowflake_bare_pivot_alias()? { + Expression::PivotAlias(Box::new(PivotAlias { this: col, alias })) + } else if self.match_token(TokenType::As) { let alias_expr = self.parse_bitwise()?.ok_or_else(|| { self.parse_error("Expected expression after AS in UNPIVOT IN clause") })?; diff --git a/crates/sqlquack-syntax/tests/parser/test_fixes.rs b/crates/sqlquack-syntax/tests/parser/test_fixes.rs index a363eff9f..bda6cbc41 100644 --- a/crates/sqlquack-syntax/tests/parser/test_fixes.rs +++ b/crates/sqlquack-syntax/tests/parser/test_fixes.rs @@ -1043,6 +1043,67 @@ fn given_statements_from_the_parser_only_disagreements_when_parsing_snowflake_th dialect: Dialect::Snowflake, expected_accepted: false, }, + AcceptanceTestCase { + description: + "a PIVOT aggregate is a call, not a parenthesised column (recorded errno 1003)", + sql: "SELECT 1 FROM t1 PIVOT ( ( v1 ) FOR c1 IN ( 1 ) ) ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "a PIVOT aggregate reads a column, not a string (recorded errno 1003)", + sql: "SELECT 1 FROM t1 PIVOT ( SUM ( 'v1' ) FOR c1 IN ( 1 ) ) ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "a PIVOT aggregate alias is no string", + sql: "SELECT 1 FROM t1 PIVOT ( SUM ( v1 ) 'v1' FOR c1 IN ( 1 ) ) ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "a PIVOT IN list is not empty", + sql: "SELECT 1 FROM t1 PIVOT ( SUM ( v1 ) FOR c1 IN ( ) ) ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "ANY ORDER needs BY (recorded errno 1003)", + sql: "SELECT 1 FROM t1 PIVOT ( SUM ( v1 ) FOR c1 IN ( ANY ORDER ) ) ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "ANY ORDER BY parses", + sql: "SELECT 1 FROM t1 PIVOT ( SUM ( v1 ) FOR c1 IN ( ANY ORDER BY c1 ) ) ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a PIVOT IN value takes an alias without AS (recorded semantic error)", + sql: "SELECT 1 FROM t1 PIVOT ( SUM ( v1 ) FOR c1 IN ( 1 v1 ) ) ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "an UNPIVOT IN column takes a quoted alias without AS", + sql: "SELECT 1 FROM t1 UNPIVOT ( c1 FOR v1 IN ( v1 \"SELECT\" ) ) ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "an UNPIVOT IN list holds columns, not strings (recorded errno 1003)", + sql: "SELECT 1 FROM t1 UNPIVOT ( c1 FOR v1 IN ( 'v1' ) ) ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "an UNPIVOT name column is no reserved keyword", + sql: "SELECT 1 FROM t1 UNPIVOT ( c1 FOR SELECT IN ( v1 ) ) ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, AcceptanceTestCase { description: "TABLE( ) needs an argument", sql: "SELECT * FROM TABLE ( ) ;", From c3a536cc066579a89c1941155513174612579d99 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 22:43:00 +0000 Subject: [PATCH 02/10] Snowflake parser: FIRST ( alias ) and LAST ( alias ) after a join Snowflake documents [ { FIRST | LAST } ( ) ] after a join's ON or USING. The join keeps it as first_last and first_last_alias, left out of the tree when absent; the alias refuses a reserved keyword, as recorded. The classifier names it join_first_last, and conversion, which accepts only the join fields it understands, refuses it. --- .../src/classification/_helpers/clauses.rs | 3 ++ .../src/expressions/queries/models.rs | 6 ++++ .../src/parser/_helpers/queries/joins.rs | 17 +++++++++++ .../src/parser/_helpers/queries/modifiers.rs | 2 ++ .../tests/parser/test_fixes.rs | 30 +++++++++++++++++++ 5 files changed, 58 insertions(+) diff --git a/crates/sqlquack-core/src/classification/_helpers/clauses.rs b/crates/sqlquack-core/src/classification/_helpers/clauses.rs index 6f334d6d3..c0a4ae1ae 100644 --- a/crates/sqlquack-core/src/classification/_helpers/clauses.rs +++ b/crates/sqlquack-core/src/classification/_helpers/clauses.rs @@ -188,6 +188,8 @@ fn join_constructs(join: &Join) -> Vec { comments: _, nesting_group: _, directed, + first_last, + first_last_alias: _, span: _, } = join; let mut constructs = vec![Construct::new( @@ -200,6 +202,7 @@ fn join_constructs(join: &Join) -> Vec { (match_condition.is_some(), "match_condition"), (!pivots.is_empty(), "join_pivot"), (*directed, "join_directed"), + (first_last.is_some(), "join_first_last"), ]); constructs.extend( names diff --git a/crates/sqlquack-syntax/src/expressions/queries/models.rs b/crates/sqlquack-syntax/src/expressions/queries/models.rs index 5d98591d4..c4ed23bea 100644 --- a/crates/sqlquack-syntax/src/expressions/queries/models.rs +++ b/crates/sqlquack-syntax/src/expressions/queries/models.rs @@ -658,6 +658,12 @@ pub struct Join { /// Snowflake: DIRECTED keyword in JOIN (e.g., CROSS DIRECTED JOIN) #[serde(default)] pub directed: bool, + /// Snowflake: `FIRST` or `LAST` before the join's `( alias )`; absent when none + #[serde(default, skip_serializing_if = "Option::is_none")] + pub first_last: Option, + /// Snowflake: the alias in `FIRST ( alias )` or `LAST ( alias )`; absent when none + #[serde(default, skip_serializing_if = "Option::is_none")] + pub first_last_alias: Option, /// SQLQuack: the node's source range, recorded by a located parse. #[serde(default, skip_serializing_if = "SourceSpan::is_absent")] #[ast(skip)] diff --git a/crates/sqlquack-syntax/src/parser/_helpers/queries/joins.rs b/crates/sqlquack-syntax/src/parser/_helpers/queries/joins.rs index 28c9682dd..bad2e866b 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/queries/joins.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/queries/joins.rs @@ -22,6 +22,20 @@ impl Parser { Ok((left, joins)) } + /// Snowflake's `FIRST ( alias )` or `LAST ( alias )` after a join, or none. + fn parse_snowflake_join_first_last(&mut self) -> Result<(Option, Option)> { + let side = match self.is_snowflake() && self.check_next(TokenType::LParen) { + true if self.match_token(TokenType::First) => "FIRST", + true if self.match_token(TokenType::Last) => "LAST", + _ => return Ok((None, None)), + }; + self.expect_token(TokenType::LParen)?; + let alias = self.expect_identifier_or_keyword_with_quoted()?; + self.reject_reserved_name_part(&alias)?; + self.expect_token(TokenType::RParen)?; + Ok((Some(side.to_owned()), Some(alias))) + } + /// Parses joins; chained ON/USING clauses bind right to left, as `a JOIN b JOIN c ON x ON y`. pub(crate) fn parse_joins(&mut self) -> Result> { let mut joins = Vec::with_capacity(2); @@ -104,6 +118,7 @@ impl Parser { } else { (None, Vec::new()) }; + let (first_last, first_last_alias) = self.parse_snowflake_join_first_last()?; joins.push(Join { this: table, @@ -119,6 +134,8 @@ impl Parser { comments: join_comments, nesting_group, directed, + first_last, + first_last_alias, span: self.source_range(pos_before_join_kind), }); } diff --git a/crates/sqlquack-syntax/src/parser/_helpers/queries/modifiers.rs b/crates/sqlquack-syntax/src/parser/_helpers/queries/modifiers.rs index 9ee51b85a..c112ab62f 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/queries/modifiers.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/queries/modifiers.rs @@ -1358,6 +1358,8 @@ impl Parser { comments: Vec::new(), nesting_group: 0, directed: false, + first_last: None, + first_last_alias: None, span: SourceSpan::default(), }); } diff --git a/crates/sqlquack-syntax/tests/parser/test_fixes.rs b/crates/sqlquack-syntax/tests/parser/test_fixes.rs index bda6cbc41..65467cb2e 100644 --- a/crates/sqlquack-syntax/tests/parser/test_fixes.rs +++ b/crates/sqlquack-syntax/tests/parser/test_fixes.rs @@ -682,6 +682,18 @@ fn given_a_snowflake_drop_of_another_object_kind_when_parsing_then_the_tree_name pointer: "/0/drop_schema/cascade", expected_value: json!(true), }, + ProjectionTestCase { + description: "a join keeps LAST apart from FIRST", + sql: "SELECT * FROM t1 JOIN t2 ON TRUE LAST ( a1 )", + pointer: "/0/select/joins/0/first_last", + expected_value: json!("LAST"), + }, + ProjectionTestCase { + description: "a join without FIRST or LAST leaves both out of the tree", + sql: "SELECT * FROM t1 JOIN t2 ON TRUE", + pointer: "/0/select/joins/0/first_last", + expected_value: serde_json::Value::Null, + }, ProjectionTestCase { description: "DROP SEQUENCE keeps RESTRICT apart from the default", sql: "DROP SEQUENCE v1 RESTRICT", @@ -1104,6 +1116,24 @@ fn given_statements_from_the_parser_only_disagreements_when_parsing_snowflake_th dialect: Dialect::Snowflake, expected_accepted: false, }, + AcceptanceTestCase { + description: "a join takes FIRST ( alias ) (recorded semantic error)", + sql: "SELECT * FROM t1 JOIN t2 FIRST ( v1 ) ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a directed join takes LAST ( alias ) after USING", + sql: "SELECT * FROM t1 INNER DIRECTED JOIN t2 USING ( v1 ) LAST ( v1 ) ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "FIRST ( alias ) refuses a reserved keyword (recorded errno 1003)", + sql: "SELECT * FROM t1 JOIN t2 FIRST ( SELECT ) ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, AcceptanceTestCase { description: "TABLE( ) needs an argument", sql: "SELECT * FROM TABLE ( ) ;", From f05edae74f8a011f1523e284a17e2c91e8ac57a6 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 22:46:42 +0000 Subject: [PATCH 03/10] Keep a failover account's organization and quoting apart ALTER FAILOVER GROUP / REPLICATION GROUP ADD|REMOVE accounts and ALTER CONNECTION ENABLE|DISABLE FAILOVER TO ACCOUNTS read . as a table reference: the account as its name, the organization as its schema, each keeping its own quoting. The joined string lost both the split and the quoting. Verdicts do not change. --- .../_helpers/ddl/snowflake_alter_account.rs | 12 ++++++++++-- .../sqlquack-syntax/tests/parser/test_fixes.rs | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) 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 5b66a7c03..4c3ab72ee 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 @@ -1,5 +1,6 @@ //! Parser ddl: Snowflake ALTER of account-level objects: failover and replication groups, connections, integrations and organization objects. +use crate::expressions::names::models::Identifier; use crate::expressions::names::models::TableRef; use crate::expressions::schema_objects::models::ObjectProperty; use crate::expressions::tree::models::Expression; @@ -208,8 +209,15 @@ impl Parser { fn parse_account_names(&mut self) -> Result> { let mut accounts: Vec = Vec::new(); loop { - let name = self.parse_simple_qualified_name()?; - accounts.push(Expression::Table(Box::new(TableRef::new(name.name)))); + let mut parts: Vec = vec![self.parse_simple_name()?]; + while self.match_token(TokenType::Dot) { + parts.push(self.parse_simple_name()?); + } + let mut account = TableRef::new(""); + account.name = parts.pop().unwrap_or_else(Identifier::empty); + account.schema = parts.pop(); + account.catalog = parts.pop(); + accounts.push(Expression::Table(Box::new(account))); if !self.match_token(TokenType::Comma) { return Ok(accounts); } diff --git a/crates/sqlquack-syntax/tests/parser/test_fixes.rs b/crates/sqlquack-syntax/tests/parser/test_fixes.rs index 65467cb2e..2c7f3685f 100644 --- a/crates/sqlquack-syntax/tests/parser/test_fixes.rs +++ b/crates/sqlquack-syntax/tests/parser/test_fixes.rs @@ -694,6 +694,24 @@ fn given_a_snowflake_drop_of_another_object_kind_when_parsing_then_the_tree_name pointer: "/0/select/joins/0/first_last", expected_value: serde_json::Value::Null, }, + ProjectionTestCase { + description: "a failover account keeps its organization as its own part", + sql: "ALTER FAILOVER GROUP g1 ADD \"o.1\" . a1 TO ALLOWED_ACCOUNTS", + pointer: "/0/alter_object/operands/0/table/schema/name", + expected_value: json!("o.1"), + }, + ProjectionTestCase { + description: "a failover account keeps its quoting apart from its organization's", + sql: "ALTER FAILOVER GROUP g1 ADD o1 . \"a.1\" TO ALLOWED_ACCOUNTS", + pointer: "/0/alter_object/operands/0/table/name/quoted", + expected_value: json!(true), + }, + ProjectionTestCase { + description: "a connection's failover account keeps its organization", + sql: "ALTER CONNECTION c1 DISABLE FAILOVER TO ACCOUNTS o1 . a1", + pointer: "/0/alter_object/operands/0/table/schema/name", + expected_value: json!("o1"), + }, ProjectionTestCase { description: "DROP SEQUENCE keeps RESTRICT apart from the default", sql: "DROP SEQUENCE v1 RESTRICT", From a014b1e7bb0cefde2012bf21bfbf539d962e0f66 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 22:50:37 +0000 Subject: [PATCH 04/10] Snowflake parser: TRUNCATE ERROR_TABLE ( table ), and no TRUNCATE ( name ) TRUNCATE [ TABLE ] [ IF EXISTS ] ERROR_TABLE ( ) parses, with error_table on the tree (left out when false). In SQLQuack's own modes TRUNCATE ( name ) is a bare call, not a statement, as Snowflake refuses it; the exact check's modes still read it as polyglot does. --- .../src/expressions/statements/models.rs | 4 ++++ .../src/parser/_helpers/ddl/alter.rs | 11 +++++++++ .../src/parser/_helpers/statements/entry.rs | 4 +++- .../tests/parser/test_fixes.rs | 24 +++++++++++++++++++ 4 files changed, 42 insertions(+), 1 deletion(-) diff --git a/crates/sqlquack-syntax/src/expressions/statements/models.rs b/crates/sqlquack-syntax/src/expressions/statements/models.rs index 1a6ac648f..77cd818eb 100644 --- a/crates/sqlquack-syntax/src/expressions/statements/models.rs +++ b/crates/sqlquack-syntax/src/expressions/statements/models.rs @@ -441,6 +441,9 @@ pub struct Truncate { /// Hive PARTITION clause: PARTITION(key=value, ...) #[serde(default, skip_serializing_if = "Option::is_none")] pub partition: Option>, + /// Snowflake `ERROR_TABLE (
)`: the table's error table; left out of the tree when false + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub error_table: bool, } /// A table entry in a TRUNCATE statement, with optional ONLY modifier and * suffix @@ -491,6 +494,7 @@ impl Truncate { identity: None, restrict: false, partition: None, + error_table: false, } } } diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/alter.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/alter.rs index 96b08abb2..383908ffc 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/alter.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/alter.rs @@ -1250,7 +1250,17 @@ impl Parser { let if_exists = self.match_keywords(&[TokenType::If, TokenType::Exists]); let has_only = self.match_token(TokenType::Only); + let error_table = self.is_snowflake() + && self.check_identifier("ERROR_TABLE") + && self.check_next(TokenType::LParen); + if error_table { + self.skip(); + self.skip(); + } let mut table = self.parse_object_name_ref()?; + if error_table { + self.expect_token(TokenType::RParen)?; + } if has_only { table.only = true; } @@ -1321,6 +1331,7 @@ impl Parser { identity, restrict, partition: partition.map(Box::new), + error_table, }))) } diff --git a/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs b/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs index 8f7f4f65c..36bb025e3 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs @@ -398,7 +398,9 @@ impl Parser { TokenType::Alter => self.parse_alter(), TokenType::Truncate => { - if self.check_next(TokenType::LParen) { + if self.check_next(TokenType::LParen) && self.applies_snowflake_context_rules() { + Err(self.parse_error("Expected a statement, not an expression")) + } else if self.check_next(TokenType::LParen) { self.parse_expression() } else { self.parse_truncate() diff --git a/crates/sqlquack-syntax/tests/parser/test_fixes.rs b/crates/sqlquack-syntax/tests/parser/test_fixes.rs index 2c7f3685f..3b0f443f9 100644 --- a/crates/sqlquack-syntax/tests/parser/test_fixes.rs +++ b/crates/sqlquack-syntax/tests/parser/test_fixes.rs @@ -712,6 +712,18 @@ fn given_a_snowflake_drop_of_another_object_kind_when_parsing_then_the_tree_name pointer: "/0/alter_object/operands/0/table/schema/name", expected_value: json!("o1"), }, + ProjectionTestCase { + description: "TRUNCATE ERROR_TABLE marks the error table", + sql: "TRUNCATE ERROR_TABLE ( v1 )", + pointer: "/0/truncate/error_table", + expected_value: json!(true), + }, + ProjectionTestCase { + description: "a plain TRUNCATE leaves error_table out of the tree", + sql: "TRUNCATE TABLE v1", + pointer: "/0/truncate/error_table", + expected_value: serde_json::Value::Null, + }, ProjectionTestCase { description: "DROP SEQUENCE keeps RESTRICT apart from the default", sql: "DROP SEQUENCE v1 RESTRICT", @@ -1152,6 +1164,18 @@ fn given_statements_from_the_parser_only_disagreements_when_parsing_snowflake_th dialect: Dialect::Snowflake, expected_accepted: false, }, + AcceptanceTestCase { + description: "TRUNCATE takes ERROR_TABLE ( table ) (recorded semantic error)", + sql: "TRUNCATE TABLE IF EXISTS ERROR_TABLE ( \"v1 Q\" ) ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "TRUNCATE ( name ) is no statement (recorded errno 1003)", + sql: "TRUNCATE ( v1 ) ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, AcceptanceTestCase { description: "TABLE( ) needs an argument", sql: "SELECT * FROM TABLE ( ) ;", From b393b991aae9561f09f531c0a1c9090f3e0733d4 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 23:14:35 +0000 Subject: [PATCH 05/10] DuckShop measure: create project SQL functions in DuckDB, leave Python-UDF models unmeasured - `schema --functions` writes the compiled functions beside the relations. - `snowflake --functions --schema` converts each SQL function with `sqlquack function` and creates it as a DuckDB macro before models run. A function that is refused or cannot be created makes the models calling it input_unloadable, never a mismatch. - Models calling a Python function report python_udf_not_measured and are not compared or scored. - `sqlquack function` resolves sqlbuild references (`__ref(...)` and kin) in the body to the census placeholder relations, as census does for models; DuckShop's two table functions read models through `__ref`. --- Makefile | 5 +- crates/sqlquack/src/census/main/mod.rs | 1 + .../src/census/main/resolve_references.rs | 8 + .../src/cli/_helpers/function_command.rs | 3 +- crates/sqlquack/tests/cli/helpers.rs | 7 +- crates/sqlquack/tests/cli/test_function.rs | 11 + .../duckshop_measure/_helpers/functions.py | 144 +++++++++++++ scripts/duckshop_measure/_helpers/measures.py | 58 +++++- scripts/duckshop_measure/constants.py | 7 +- scripts/duckshop_measure/main/measure.py | 38 +++- scripts/duckshop_measure/models.py | 21 ++ scripts/duckshop_measure/types.py | 9 +- .../scripts/duckshop_measure/_test_types.py | 16 ++ .../scripts/duckshop_measure/test_measure.py | 192 +++++++++++++++++- 14 files changed, 503 insertions(+), 17 deletions(-) create mode 100644 crates/sqlquack/src/census/main/resolve_references.rs create mode 100644 scripts/duckshop_measure/_helpers/functions.py diff --git a/Makefile b/Makefile index afadc1dd6..bec1ab4ff 100644 --- a/Makefile +++ b/Makefile @@ -208,7 +208,7 @@ duckshop-measure: duckshop-compiled library="$$(dirname "$$(ls "$$target"/duckdb-download/*/*/libduckdb.so | head -1)")"; \ compiled="$(abspath $(word 1,$(DUCKSHOP_COMPILED)))"; out="$(DUCKSHOP_MEASURE_DIR)"; \ uv run python -m scripts.measure_duckshop schema --environment referee/environments/duckshop.json \ - --recorded "$(DUCKSHOP_RESULTS)" --out "$$out/schema.json"; \ + --recorded "$(DUCKSHOP_RESULTS)" --functions "$$compiled/functions" --out "$$out/schema.json"; \ SQLQUACK_CONVERTED_DIR="$$out/converted" SQLQUACK_GUARDS_DIR="$$out/guards" \ LD_LIBRARY_PATH="$$library" "$$target/release/sqlquack" census --policy sqlquack.toml --json \ --names --schema "$$out/schema.json" "$$compiled/models" > "$$out/census.json"; \ @@ -216,7 +216,8 @@ duckshop-measure: duckshop-compiled --sqlquack "$$target/release/sqlquack" --library "$$library" --out "$$out/tests.json"; \ uv run python -m scripts.measure_duckshop snowflake --results "$(DUCKSHOP_RESULTS)" \ --compiled "$$compiled" --census "$$out/census.json" --converted "$$out/converted" \ - --guards "$$out/guards" --sqlquack "$$target/release/sqlquack" --library "$$library" \ + --guards "$$out/guards" --functions "$$compiled/functions" --schema "$$out/schema.json" \ + --sqlquack "$$target/release/sqlquack" --library "$$library" \ --work "$$out/work" --out "$$out/snowflake.json" # Fuzzes each of FUZZ_TARGETS from the exact check's statements for FUZZ_SECONDS: tolerant parsing, diff --git a/crates/sqlquack/src/census/main/mod.rs b/crates/sqlquack/src/census/main/mod.rs index 0003eb469..dc14bdde1 100644 --- a/crates/sqlquack/src/census/main/mod.rs +++ b/crates/sqlquack/src/census/main/mod.rs @@ -1,3 +1,4 @@ pub mod census_with_progress; pub mod inspect_corpus; pub mod render_census_text; +pub mod resolve_references; diff --git a/crates/sqlquack/src/census/main/resolve_references.rs b/crates/sqlquack/src/census/main/resolve_references.rs new file mode 100644 index 000000000..778867384 --- /dev/null +++ b/crates/sqlquack/src/census/main/resolve_references.rs @@ -0,0 +1,8 @@ +//! `resolve_references`: sqlbuild reference calls in SQL replaced as census replaces them. + +use crate::census::_helpers::placeholders::rewrite_with_references; + +/// `sql` with `__ref(…)` and its kin as the placeholder relations a census schema declares. +pub fn resolve_references(sql: &str) -> String { + rewrite_with_references(sql, None).sql +} diff --git a/crates/sqlquack/src/cli/_helpers/function_command.rs b/crates/sqlquack/src/cli/_helpers/function_command.rs index 82f839ff8..b6376b8ca 100644 --- a/crates/sqlquack/src/cli/_helpers/function_command.rs +++ b/crates/sqlquack/src/cli/_helpers/function_command.rs @@ -19,6 +19,7 @@ use sqlquack_rules::conversion::models::ConvertedFunction; use crate::analysis::main::analyze_function::analyze_function; use crate::analysis::main::parse_session::parse_session; use crate::analysis::main::session_conversion_options::session_conversion_options; +use crate::census::main::resolve_references::resolve_references; use crate::cli::_helpers::evidence::caller_choices; use crate::cli::_helpers::evidence::chosen_policy; use crate::cli::_helpers::schema::load_schema; @@ -142,7 +143,7 @@ fn convert( arguments.name )) })?; - let body = read_body(&arguments.body).map_err(Failure::Usage)?; + let body = resolve_references(&read_body(&arguments.body).map_err(Failure::Usage)?); let options = match session_conversion_options( policy.as_ref(), ( diff --git a/crates/sqlquack/tests/cli/helpers.rs b/crates/sqlquack/tests/cli/helpers.rs index 3fecc0b4c..72ed2ec3f 100644 --- a/crates/sqlquack/tests/cli/helpers.rs +++ b/crates/sqlquack/tests/cli/helpers.rs @@ -501,7 +501,8 @@ pub(crate) fn run_function_macro( ) -> FunctionMacroOutcome { let directory = tempfile::tempdir().expect("temporary directory"); let schema = format!( - "{{\"functions\": {{\"DOUBLED\": {{\"language\": \"sql\", \ + "{{\"relations\": {{\"__SQLQUACK_RELATION__FEES\": [[\"AMOUNT\", \"NUMBER(10,2)\"]]}}, \ + \"functions\": {{\"DOUBLED\": {{\"language\": \"sql\", \ \"arguments\": [[\"PRICE\", \"NUMBER(10,2)\"]], \"returns\": {returns}}}}}}}" ); std::fs::write(directory.path().join("schema.json"), schema).expect("schema written"); @@ -546,7 +547,9 @@ fn macro_values(converted: &serde_json::Value, macro_body: &str, call: &str) -> let connection = duckdb::Connection::open_in_memory().expect("DuckDB opens"); connection .execute_batch(&format!( - "CREATE MACRO \"DOUBLED\"({}) AS {macro_body}", + "CREATE TABLE \"__SQLQUACK_RELATION__FEES\" AS \ + SELECT CAST(amount AS DECIMAL(10,2)) AS \"AMOUNT\" FROM (VALUES (1.00), (2.00)) AS t(amount); \ + CREATE MACRO \"DOUBLED\"({}) AS {macro_body}", parameters.join(", ") )) .expect("the macro is created"); diff --git a/crates/sqlquack/tests/cli/test_function.rs b/crates/sqlquack/tests/cli/test_function.rs index 02c196dab..c27b98d0b 100644 --- a/crates/sqlquack/tests/cli/test_function.rs +++ b/crates/sqlquack/tests/cli/test_function.rs @@ -17,6 +17,17 @@ fn given_a_schema_declared_sql_function_when_converting_then_its_macro_runs_in_d values: vec!["2.50".to_owned()], }, }, + FunctionMacroTestCase { + description: "a sqlbuild reference in the body reads the census placeholder relation", + returns: "\"NUMBER(10,2)\"", + body: "CAST(price + (SELECT MIN(amount) FROM __ref(\"fees\")) AS NUMBER(10,2))", + call: "SELECT CAST(\"DOUBLED\"(1.25) AS VARCHAR)", + flags: &[], + expected_outcome: FunctionMacroOutcome { + exit_code: Some(0), + values: vec!["2.25".to_owned()], + }, + }, FunctionMacroTestCase { description: "a NUMBER body of another precision than declared is refused", returns: "\"NUMBER(10,2)\"", diff --git a/scripts/duckshop_measure/_helpers/functions.py b/scripts/duckshop_measure/_helpers/functions.py new file mode 100644 index 000000000..6b5cc9f69 --- /dev/null +++ b/scripts/duckshop_measure/_helpers/functions.py @@ -0,0 +1,144 @@ +"""Project functions: SQL ones converted by `sqlquack function` and created in DuckDB.""" + +from __future__ import annotations + +import argparse +import json +import os +import re +import subprocess +import sys +from pathlib import Path + +import duckdb + +from scripts.census_functions._helpers.headers import placeholder, signature +from scripts.census_functions.constants import FUNCTION_SUFFIX, SQL_LANGUAGE +from scripts.census_functions.models import Signature +from scripts.duckshop_measure._helpers.running import refusal_names +from scripts.duckshop_measure.constants import BODY_DELIMITER, PYTHON_LANGUAGE +from scripts.duckshop_measure.models import FunctionMacro, ProjectFunction +from scripts.duckshop_measure.types import FunctionConverter + + +def function_body(text: str) -> str: + """The body between a compiled function's first and last `$$`.""" + first: int = text.find(BODY_DELIMITER) + last: int = text.rfind(BODY_DELIMITER) + return text[first + len(BODY_DELIMITER) : last].strip() if last > first >= 0 else "" + + +def project_functions(directory: Path) -> list[ProjectFunction]: + """Every compiled function whose header reads, by the census's placeholder name.""" + functions: list[ProjectFunction] = [] + for path in sorted(directory.rglob(f"*{FUNCTION_SUFFIX}")): + text: str = path.read_text(encoding="utf-8") + found: Signature | None = signature(text) + if found is None: + continue + name: str = placeholder(stem=path.stem, table=not isinstance(found.returns, str)) + functions.append( + ProjectFunction(name=name, language=found.language, body=function_body(text)) + ) + return functions + + +def binary_function_converter( + *, binary: Path, library: Path, policy: Path, schema: Path +) -> FunctionConverter: + """A converter running ` function - --name … --schema ` under `policy`.""" + environment: dict[str, str] = {**os.environ, "LD_LIBRARY_PATH": str(library)} + + def convert(function: ProjectFunction) -> FunctionMacro | tuple[str, ...]: + """The macro of `function`, or its refusals; a crash or unreadable output names it.""" + completed: subprocess.CompletedProcess[str] = subprocess.run( + (str(binary), "function", "-", "--name", function.name) + + ("--schema", str(schema), "--policy", str(policy)), + input=function.body, + capture_output=True, + text=True, + check=False, + env=environment, + ) + try: + report: dict[str, object] = json.loads(completed.stdout) + except json.JSONDecodeError: + return (f"no report: exit {completed.returncode}",) + body: object = report.get("body") + if not isinstance(body, str): + return refusal_names(report.get("refusals")) or ("no macro body",) + parameters: object = report.get("parameters") + names: tuple[str, ...] = tuple(map(str, parameters)) if isinstance(parameters, list) else () + return FunctionMacro(name=function.name, parameters=names, body=body) + + return convert + + +def converted_macros( + *, functions: list[ProjectFunction], convert: FunctionConverter +) -> tuple[list[FunctionMacro], dict[str, tuple[str, ...]]]: + """Each SQL function's macro, and the refusals of those conversion refused, by name.""" + macros: list[FunctionMacro] = [] + refused: dict[str, tuple[str, ...]] = {} + for function in functions: + if function.language.lower() != SQL_LANGUAGE: + continue + converted: FunctionMacro | tuple[str, ...] = convert(function) + if isinstance(converted, FunctionMacro): + macros.append(converted) + else: + refused[function.name] = converted + return macros, refused + + +def python_function_names(functions: list[ProjectFunction]) -> frozenset[str]: + """The placeholder names of the project's Python functions.""" + return frozenset( + function.name for function in functions if function.language.lower() == PYTHON_LANGUAGE + ) + + +def create_macros(*, database: Path, macros: list[FunctionMacro]) -> dict[str, str]: + """Create each macro in `database` for models run there; DuckDB's refusals by name.""" + failed: dict[str, str] = {} + connection: duckdb.DuckDBPyConnection = duckdb.connect(str(database)) + try: + for macro in macros: + try: + connection.execute( + f'CREATE OR REPLACE MACRO "{macro.name}"({", ".join(macro.parameters)}) ' + f"AS {macro.body}" + ) + except duckdb.Error as error: + failed[macro.name] = f"{type(error).__name__}: {error}" + finally: + connection.close() + return failed + + +def python_calls(*, texts: tuple[str, ...], python: frozenset[str]) -> tuple[str, ...]: + """The Python functions any of `texts` names, sorted.""" + joined: str = "\n".join(texts) + return tuple(sorted(name for name in python if re.search(rf"\b{name}\b", joined))) + + +def project_macros( + *, arguments: argparse.Namespace, function_converter: FunctionConverter | None +) -> tuple[tuple[FunctionMacro, ...], frozenset[str], dict[str, tuple[str, ...]]]: + """The project's SQL functions as macros, its Python functions' names, and the refused.""" + if arguments.functions is None: + return (), frozenset(), {} + functions: list[ProjectFunction] = project_functions(arguments.functions) + convert: FunctionConverter = function_converter or binary_function_converter( + binary=arguments.sqlquack, + library=arguments.library, + policy=arguments.policy, + schema=arguments.schema, + ) + macros, refused = converted_macros(functions=functions, convert=convert) + print( + f"duckshop measure: {len(macros)} SQL functions created, {len(refused)} refused" + + "".join(f"\n {name}: {'; '.join(why)}" for name, why in sorted(refused.items())), + file=sys.stderr, + ) + return tuple(macros), python_function_names(functions), refused diff --git a/scripts/duckshop_measure/_helpers/measures.py b/scripts/duckshop_measure/_helpers/measures.py index a6ab6ee20..6baebb2ba 100644 --- a/scripts/duckshop_measure/_helpers/measures.py +++ b/scripts/duckshop_measure/_helpers/measures.py @@ -7,6 +7,9 @@ from concurrent.futures import ThreadPoolExecutor from pathlib import Path +from scripts.census_functions._helpers.schema_file import compiled_functions +from scripts.census_functions.constants import FUNCTIONS_KEY, RELATIONS_KEY +from scripts.duckshop_measure._helpers.functions import create_macros, python_calls from scripts.duckshop_measure._helpers.outcomes import unit_test_outcome from scripts.duckshop_measure._helpers.running import refusal_names from scripts.duckshop_measure._helpers.schema import census_schema, recorded_schema @@ -33,19 +36,40 @@ SnowflakeInputs, UnitTestOutcome, ) -from scripts.duckshop_measure.types import Comparer, Converter, Executor, RecordedOutcome +from scripts.duckshop_measure.types import ( + Comparer, + Converter, + Executor, + ModelOutcome, + RecordedOutcome, +) -def write_schema(*, environment: Path, out: Path, recorded: Path | None = None) -> None: - """Write the census schema of the environment's tables, typed as recorded with `recorded`.""" +def write_schema( + *, environment: Path, out: Path, recorded: Path | None = None, functions: Path | None = None +) -> None: + """Write the census schema of the environment's tables, with `functions`' compiled ones.""" setup: list[str] = json.loads(environment.read_text())["setup_sql"] schema: dict[str, list[list[str]]] = census_schema(setup) if recorded is not None: schema.update(recorded_schema(recorded)) - out.write_text(json.dumps(schema, indent=1) + "\n") + written: object = schema + if functions is not None: + found, unread = compiled_functions(functions) + written = {RELATIONS_KEY: schema, FUNCTIONS_KEY: found} + print(f"duckshop measure: {len(found)} functions written, {unread} unread", file=sys.stderr) + out.write_text(json.dumps(written, indent=1) + "\n") print(f"duckshop measure: {len(schema)} relations written to {out}", file=sys.stderr) +def model_python_calls( + *, inputs: SnowflakeInputs, sql: Path, refusals: tuple[str, ...] +) -> tuple[str, ...]: + """The Python functions a model's conversion or refusals name.""" + converted: tuple[str, ...] = (sql.read_text(),) if sql.exists() else () + return python_calls(texts=(*converted, *refusals), python=inputs.python_functions) + + def unit_test_outcomes( *, compiled: Path, convert: Converter, execute: Executor ) -> list[UnitTestOutcome]: @@ -96,8 +120,12 @@ def snowflake_results(*, inputs: SnowflakeInputs, run: Comparer) -> list[ModelRe ) database: Path = work / f"{target}.duckdb" refused: dict[str, str] = load_inputs(entries=entries, results=results, database=database) + failed: dict[str, str] = create_macros(database=database, macros=list(inputs.macros)) + refused.update( + (name.lower(), why) for name, why in (*inputs.unavailable_functions, *failed.items()) + ) print( - f"duckshop measure: {len(refused)} recorded relations unloadable for {target}", + f"duckshop measure: {len(refused)} inputs or functions unavailable for {target}", file=sys.stderr, ) for entry in entries: @@ -110,6 +138,11 @@ def snowflake_results(*, inputs: SnowflakeInputs, run: Comparer) -> list[ModelRe compiled=compiled, compiled_sql_path=str(sidecar["compiled_sql_path"]) ) sql: Path = converted / name + model_refusals: tuple[str, ...] = own.get( + Path(str(sidecar["compiled_sql_path"])).stem, () + ) + if model_python_calls(inputs=inputs, sql=sql, refusals=model_refusals): + continue if not sql.exists() or value_divergences(inputs=inputs, name=name): continue if sidecar["outcome"] == RecordedOutcome.ERROR: @@ -146,6 +179,21 @@ def snowflake_results(*, inputs: SnowflakeInputs, run: Comparer) -> list[ModelRe ) lenient: Path | None = inputs.lenient_converted declared = refusals if lenient is not None and (lenient / name).exists() else () + python: tuple[str, ...] = model_python_calls( + inputs=inputs, + sql=converted / name, + refusals=own.get(Path(str(sidecar["compiled_sql_path"])).stem, ()), + ) + if python: + outcomes.append( + ModelResult( + model=str(sidecar["model"]), + target=str(sidecar["target"]), + outcome=ModelOutcome.PYTHON_UDF_NOT_MEASURED, + details=python, + ) + ) + continue outcomes.append( model_result( sidecar=sidecar, diff --git a/scripts/duckshop_measure/constants.py b/scripts/duckshop_measure/constants.py index 4a412b7e4..4b9211ef1 100644 --- a/scripts/duckshop_measure/constants.py +++ b/scripts/duckshop_measure/constants.py @@ -31,7 +31,9 @@ CENSUS_TABLE_PREFIX: str = "__sqlquack_relation__" COMPILED_PREFIX: str = "target/compiled/" MISSING_TABLE: re.Pattern[str] = re.compile( - r"Table with name (__sqlquack_relation__\w+) does not exist", re.IGNORECASE + r"(?:Table|Scalar Function|Table Function) with name " + r"(__sqlquack_(?:relation|udf|table_fn)__\w+) does not exist", + re.IGNORECASE, ) MICROSECOND_SCALE: int = 6 SCALED_TYPES: frozenset[str] = frozenset({"TIME", "TIMESTAMP_NTZ", "TIMESTAMP_LTZ", "TIMESTAMP_TZ"}) @@ -61,9 +63,12 @@ "input_unrecorded", "input_unloadable", "declared_divergence", + "python_udf_not_measured", } ) WORKERS: int = 3 +BODY_DELIMITER: str = "$$" +PYTHON_LANGUAGE: str = "python" TOP_CONSTRUCTS: int = 15 JSON_SUFFIX: str = ".json" VALUE_FAILURE: str = "value" diff --git a/scripts/duckshop_measure/main/measure.py b/scripts/duckshop_measure/main/measure.py index c9e7a90f0..31a5c5f2d 100644 --- a/scripts/duckshop_measure/main/measure.py +++ b/scripts/duckshop_measure/main/measure.py @@ -6,6 +6,7 @@ import json from pathlib import Path +from scripts.duckshop_measure._helpers.functions import project_macros from scripts.duckshop_measure._helpers.measures import ( snowflake_results, unit_test_outcomes, @@ -21,7 +22,7 @@ from scripts.duckshop_measure._helpers.snowflake import policy_value_divergences, run_comparisons from scripts.duckshop_measure.constants import SCHEMA_COMMAND, SNOWFLAKE_COMMAND, TESTS_COMMAND from scripts.duckshop_measure.models import ModelResult, SnowflakeInputs, UnitTestOutcome -from scripts.duckshop_measure.types import Comparer, Converter +from scripts.duckshop_measure.types import Comparer, Converter, FunctionConverter def _measure_parser() -> argparse.ArgumentParser: @@ -38,6 +39,9 @@ def _measure_parser() -> argparse.ArgumentParser: schema.add_argument( "--recorded", type=Path, help="recorded results whose relations the schema types instead" ) + schema.add_argument( + "--functions", type=Path, help="compiled project functions the schema declares" + ) tests: argparse.ArgumentParser = commands.add_parser( TESTS_COMMAND, help="compiled unit tests converted and run on DuckDB" ) @@ -65,6 +69,12 @@ def _measure_parser() -> argparse.ArgumentParser: help="with a strict census: SQLQUACK_CONVERTED_DIR of a census under the default policy", ) snowflake.add_argument("--guards", type=Path, help="SQLQUACK_GUARDS_DIR of the census") + snowflake.add_argument( + "--functions", type=Path, help="compiled project functions, created in DuckDB before models" + ) + snowflake.add_argument( + "--schema", type=Path, help="the census schema declaring the functions (with --functions)" + ) snowflake.add_argument("--sqlquack", type=Path, required=True) snowflake.add_argument("--library", type=Path, required=True, help="libduckdb.so's directory") snowflake.add_argument("--policy", type=Path, default=Path("sqlquack.toml")) @@ -73,7 +83,12 @@ def _measure_parser() -> argparse.ArgumentParser: return parser -def _measure_snowflake(*, arguments: argparse.Namespace, comparer: Comparer | None) -> None: +def _measure_snowflake( + *, + arguments: argparse.Namespace, + comparer: Comparer | None, + function_converter: FunctionConverter | None, +) -> None: """Write the Snowflake comparison report and print its summary.""" def compare( @@ -88,6 +103,9 @@ def compare( ) arguments.work.mkdir(parents=True, exist_ok=True) + macros, python, refused = project_macros( + arguments=arguments, function_converter=function_converter + ) inputs: SnowflakeInputs = SnowflakeInputs( results=arguments.results, compiled=arguments.compiled, @@ -97,6 +115,12 @@ def compare( guards=arguments.guards, value_divergences=policy_value_divergences(arguments.policy), work=arguments.work, + macros=macros, + python_functions=python, + unavailable_functions=tuple( + (name, f"sqlquack function refused: {'; '.join(why)}") + for name, why in sorted(refused.items()) + ), ) models: list[ModelResult] = snowflake_results(inputs=inputs, run=comparer or compare) arguments.out.write_text(json.dumps(snowflake_report(models), indent=1) + "\n") @@ -108,16 +132,22 @@ def run_measure( argv: list[str] | None = None, converter: Converter | None = None, comparer: Comparer | None = None, + function_converter: FunctionConverter | None = None, ) -> int: """Run one DuckShop measure subcommand; 0 once its report is written.""" arguments: argparse.Namespace = _measure_parser().parse_args(argv) if arguments.command == SCHEMA_COMMAND: write_schema( - environment=arguments.environment, out=arguments.out, recorded=arguments.recorded + environment=arguments.environment, + out=arguments.out, + recorded=arguments.recorded, + functions=arguments.functions, ) return 0 if arguments.command == SNOWFLAKE_COMMAND: - _measure_snowflake(arguments=arguments, comparer=comparer) + _measure_snowflake( + arguments=arguments, comparer=comparer, function_converter=function_converter + ) return 0 convert: Converter = converter or binary_converter( binary=arguments.sqlquack, library=arguments.library, policy=arguments.policy diff --git a/scripts/duckshop_measure/models.py b/scripts/duckshop_measure/models.py index e96dcbed6..2a09d3417 100644 --- a/scripts/duckshop_measure/models.py +++ b/scripts/duckshop_measure/models.py @@ -49,3 +49,24 @@ class SnowflakeInputs: guards: Path | None value_divergences: frozenset[str] work: Path + macros: tuple[FunctionMacro, ...] = () + python_functions: frozenset[str] = frozenset() + unavailable_functions: tuple[tuple[str, str], ...] = () + + +@dataclass(frozen=True) +class ProjectFunction: + """One compiled project function: its census placeholder name, language and body.""" + + name: str + language: str + body: str + + +@dataclass(frozen=True) +class FunctionMacro: + """A SQL function as `sqlquack function` converted it: a DuckDB macro.""" + + name: str + parameters: tuple[str, ...] + body: str diff --git a/scripts/duckshop_measure/types.py b/scripts/duckshop_measure/types.py index 70a4005d9..7c5b0973f 100644 --- a/scripts/duckshop_measure/types.py +++ b/scripts/duckshop_measure/types.py @@ -7,7 +7,7 @@ from typing import TYPE_CHECKING, Protocol if TYPE_CHECKING: - from scripts.duckshop_measure.models import Conversion + from scripts.duckshop_measure.models import Conversion, FunctionMacro, ProjectFunction class Outcome(StrEnum): @@ -38,6 +38,7 @@ class ModelOutcome(StrEnum): INCREMENTAL = "incremental" UNRECORDABLE = "unrecordable" SNOWFLAKE_ERROR = "snowflake_error" + PYTHON_UDF_NOT_MEASURED = "python_udf_not_measured" class RecordedOutcome(StrEnum): @@ -60,6 +61,12 @@ class Executor(Protocol): def __call__(self, sql: str) -> list[tuple[object, ...]]: ... +class FunctionConverter(Protocol): + """Converts one project SQL function into a DuckDB macro, or names its refusals.""" + + def __call__(self, function: ProjectFunction) -> FunctionMacro | tuple[str, ...]: ... + + class Comparer(Protocol): """Compares converted models' DuckDB runs with their recordings; outcomes by id.""" diff --git a/tests/unit/scripts/duckshop_measure/_test_types.py b/tests/unit/scripts/duckshop_measure/_test_types.py index 6662ce1dd..eed90ec02 100644 --- a/tests/unit/scripts/duckshop_measure/_test_types.py +++ b/tests/unit/scripts/duckshop_measure/_test_types.py @@ -65,3 +65,19 @@ class SnowflakeTestCase: expected_compared: tuple[str, ...] expected_scored: int expected_outcomes: dict[str, list[str]] + + +@dataclass(frozen=True) +class FunctionSchemaTestCase: + description: str + header: str + expected_functions: dict[str, dict[str, object]] + + +@dataclass(frozen=True) +class FunctionMeasureTestCase: + description: str + models: tuple[tuple[str, str], ...] + expected_converted: list[str] + expected_compared: dict[str, object] + expected_outcomes: dict[str, list[str]] diff --git a/tests/unit/scripts/duckshop_measure/test_measure.py b/tests/unit/scripts/duckshop_measure/test_measure.py index fa0baeb62..d7450803d 100644 --- a/tests/unit/scripts/duckshop_measure/test_measure.py +++ b/tests/unit/scripts/duckshop_measure/test_measure.py @@ -23,8 +23,16 @@ with_unloadable_input, ) from scripts.duckshop_measure.main.measure import run_measure -from scripts.duckshop_measure.models import Conversion, ModelResult, UnitTestOutcome +from scripts.duckshop_measure.models import ( + Conversion, + FunctionMacro, + ModelResult, + ProjectFunction, + UnitTestOutcome, +) from tests.unit.scripts.duckshop_measure._test_types import ( + FunctionMeasureTestCase, + FunctionSchemaTestCase, LoadTestCase, ModelTestCase, OrderTestCase, @@ -293,6 +301,19 @@ def test_given_compiled_sql_when_reading_its_order_then_only_a_top_level_order_b expected_outcome="input_unrecorded", unloadable={"__sqlquack_relation__x": "ConversionException: out of range"}, ), + ModelTestCase( + description="a project function that could not be created is input_unloadable", + sidecar=_ROWS, + refusals=None, + compared={ + "outcome": "duckdb_error", + "detail": "Catalog Error: Scalar Function with name __sqlquack_udf__f does not " + "exist!", + }, + error=None, + expected_outcome="input_unloadable", + unloadable={"__sqlquack_udf__f": "sqlquack function refused: missing rule"}, + ), ModelTestCase( description="a microsecond-only match is its own outcome", sidecar=_ROWS, @@ -502,3 +523,172 @@ def compare( ) assert outcomes == test_case.expected_outcomes assert "3 models" in capsys.readouterr().out + + +@pytest.mark.parametrize( + "test_case", + ( + FunctionSchemaTestCase( + description="a SQL function is declared under its census placeholder", + header="CREATE OR REPLACE FUNCTION SHOP.PUBLIC.SKU_FAMILY(sku STRING)\nRETURNS STRING\n" + "AS $$ split_part(sku, '-', 1) $$", + expected_functions={ + "__SQLQUACK_UDF__SKU_FAMILY": { + "language": "sql", + "arguments": [["SKU", "STRING"]], + "returns": "STRING", + } + }, + ), + FunctionSchemaTestCase( + description="a Python function is declared with its language", + header="CREATE FUNCTION S.P.SKU_FAMILY(x NUMBER) RETURNS NUMBER LANGUAGE PYTHON " + "HANDLER = 'f' AS $$\ndef f(x):\n return x\n$$", + expected_functions={ + "__SQLQUACK_UDF__SKU_FAMILY": { + "language": "python", + "arguments": [["X", "NUMBER"]], + "returns": "NUMBER", + } + }, + ), + ), + ids=lambda case: case.description, +) +def test_given_compiled_functions_when_deriving_the_schema_then_they_sit_beside_the_relations( + test_case: FunctionSchemaTestCase, tmp_path: Path +) -> None: + environment: Path = tmp_path / "environment.json" + environment.write_text( + json.dumps({"setup_sql": ["CREATE TEMPORARY TABLE rel__shop__skus (sku VARCHAR)"]}) + ) + functions: Path = tmp_path / "functions" + functions.mkdir() + (functions / "sku_family.sql").write_text(test_case.header) + out: Path = tmp_path / "schema.json" + status: int = run_measure( + argv=[ + "schema", + "--environment", + str(environment), + "--functions", + str(functions), + "--out", + str(out), + ] + ) + assert (status, json.loads(out.read_text())) == ( + 0, + { + "relations": {"__SQLQUACK_RELATION__SHOP__SKUS": [["SKU", "VARCHAR"]]}, + "functions": test_case.expected_functions, + }, + ) + + +@pytest.mark.parametrize( + "test_case", + ( + FunctionMeasureTestCase( + description="a SQL function runs as a macro and a Python one leaves its model unmeasured", + models=( + ("m_sql", "SELECT \"__SQLQUACK_UDF__SKU_FAMILY\"('ab-1') AS a"), + ("m_py", 'SELECT "__SQLQUACK_UDF__PY_SCORE"(1) AS a'), + ), + expected_converted=["__SQLQUACK_UDF__SKU_FAMILY"], + expected_compared={"k_m_sql": [("AB-1",)]}, + expected_outcomes={ + "m_sql": ["matches"], + "m_py": ["python_udf_not_measured", "__SQLQUACK_UDF__PY_SCORE"], + }, + ), + ), + ids=lambda case: case.description, +) +def test_given_project_functions_when_measuring_snowflake_then_sql_ones_run_and_python_ones_are_not_measured( + test_case: FunctionMeasureTestCase, tmp_path: Path +) -> None: + results: Path = tmp_path / "results" + compiled: Path = tmp_path / "project" / "target" / "compiled" + converted: Path = tmp_path / "converted" + for directory in (results, compiled / "models", compiled / "functions", converted): + directory.mkdir(parents=True) + (compiled / "functions" / "sku_family.sql").write_text( + "CREATE FUNCTION S.P.SKU_FAMILY(sku STRING) RETURNS STRING AS $$ upper(sku) $$" + ) + (compiled / "functions" / "py_score.sql").write_text( + "CREATE FUNCTION S.P.PY_SCORE(x NUMBER) RETURNS NUMBER LANGUAGE PYTHON " + "RUNTIME_VERSION = '3.11' HANDLER = 'f' AS $$\ndef f(x):\n return x\n$$" + ) + entries: list[dict[str, object]] = [] + for model, sql in test_case.models: + (compiled / "models" / f"{model}.sql").write_text("SELECT 1 AS a") + sidecar: dict[str, object] = { + "key": f"k_{model}", + "model": model, + "target": "dev", + "outcome": "rows", + "compiled_sql_path": f"benchmarks/x/target/compiled/models/{model}.sql", + } + (results / f"k_{model}.json").write_text(json.dumps(sidecar)) + entries.append({"key": f"k_{model}", "unique_id": f"model.shop.{model}", "outcome": "rows"}) + (converted / str(compiled / "models" / f"{model}.sql").replace("/", "_")).write_text(sql) + (tmp_path / "index-core.json").write_text(json.dumps(entries)) + (tmp_path / "index-edge.json").write_text("[]") + census: Path = tmp_path / "census.json" + census.write_text(json.dumps({"blockers": {"files": []}})) + converted_names: list[str] = [] + compared: dict[str, object] = {} + + def convert_function(function: ProjectFunction) -> FunctionMacro | tuple[str, ...]: + converted_names.append(function.name) + return FunctionMacro(name=function.name, parameters=("sku",), body=function.body) + + def compare( + *, manifest: Path, comparisons: list[dict[str, object]] + ) -> dict[str, dict[str, str]]: + for comparison in comparisons: + connection: duckdb.DuckDBPyConnection = duckdb.connect(str(comparison["database"])) + sql: str = Path(str(comparison["sql"])).read_text() + compared[str(comparison["id"])] = connection.execute(sql).fetchall() + connection.close() + return {key: {"id": key, "outcome": "matches", "detail": ""} for key in compared} + + out: Path = tmp_path / "snowflake.json" + run_measure( + argv=[ + "snowflake", + "--results", + str(results), + "--compiled", + str(compiled), + "--census", + str(census), + "--converted", + str(converted), + "--functions", + str(compiled / "functions"), + "--schema", + "unused", + "--sqlquack", + "unused", + "--library", + "unused", + "--work", + str(tmp_path / "work"), + "--out", + str(out), + ], + comparer=compare, + function_converter=convert_function, + ) + listed: object = json.loads(out.read_text())["results"] + assert isinstance(listed, list) + outcomes: dict[str, list[str]] = { + str(item["model"]): [item["outcome"], *item["details"]] for item in listed + } + assert (converted_names, compared, outcomes) == ( + test_case.expected_converted, + test_case.expected_compared, + test_case.expected_outcomes, + ) From c9a02261d5b5cc65f50f63cbe6b1b1232a48ba84 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 23:38:54 +0000 Subject: [PATCH 06/10] Coverage diff: fail on cases Snowflake parsed that the parser accepted and now rejects A snapshot now lists the receipted cases Snowflake parsed, split into those the parser accepts (opaque or not) and those it rejects. diff reports base acceptances the head rejects as a regression, by case. An opaque acceptance satisfies no obligation, so a move from it to a rejection changed no form or obligation and passed unseen. --- .../src/cli/_helpers/commands.rs | 7 +++++- .../src/report/main/diff_snapshots.rs | 5 ++++ .../src/report/main/snapshot.rs | 23 +++++++++++++++++-- .../src/report/models.rs | 8 +++++++ .../src/report/tests/helpers.rs | 2 ++ .../src/report/tests/test_diff_snapshots.rs | 22 ++++++++++++++++++ docs/syntax-coverage.md | 10 +++++--- 7 files changed, 71 insertions(+), 6 deletions(-) diff --git a/crates/sqlquack-syntax-coverage/src/cli/_helpers/commands.rs b/crates/sqlquack-syntax-coverage/src/cli/_helpers/commands.rs index b55ac32e7..806de8ec8 100644 --- a/crates/sqlquack-syntax-coverage/src/cli/_helpers/commands.rs +++ b/crates/sqlquack-syntax-coverage/src/cli/_helpers/commands.rs @@ -339,9 +339,13 @@ fn read_snapshot(path: &Path) -> Result { /// Lines naming each non-empty list of the diff. fn diff_lines(diff: &SnapshotDiff) -> Vec { - let lists: [(&str, &Vec); 7] = [ + let lists: [(&str, &Vec); 8] = [ ("REGRESSION forms lost", &diff.forms_lost), ("REGRESSION obligations lost", &diff.obligations_lost), + ( + "REGRESSION Snowflake-parsed cases the parser now rejects", + &diff.parsed_cases_refused, + ), ("CATALOGUE files changed", &diff.catalogue_files_changed), ("CATALOGUE forms added", &diff.forms_added), ("CATALOGUE forms removed", &diff.forms_removed), @@ -377,6 +381,7 @@ pub(crate) fn diff( let failing: bool = [ &diff.forms_lost, &diff.obligations_lost, + &diff.parsed_cases_refused, &diff.catalogue_files_changed, &diff.forms_added, &diff.forms_removed, diff --git a/crates/sqlquack-syntax-coverage/src/report/main/diff_snapshots.rs b/crates/sqlquack-syntax-coverage/src/report/main/diff_snapshots.rs index 35a6e6347..0f6b9d04d 100644 --- a/crates/sqlquack-syntax-coverage/src/report/main/diff_snapshots.rs +++ b/crates/sqlquack-syntax-coverage/src/report/main/diff_snapshots.rs @@ -54,5 +54,10 @@ pub fn diff_snapshots(base: &Snapshot, head: &Snapshot) -> SnapshotDiff { head.satisfied_obligations .difference(&base.satisfied_obligations), ), + parsed_cases_refused: base + .parsed_cases + .intersection(&head.refused_cases) + .cloned() + .collect(), } } diff --git a/crates/sqlquack-syntax-coverage/src/report/main/snapshot.rs b/crates/sqlquack-syntax-coverage/src/report/main/snapshot.rs index dabc7b624..da9fc384c 100644 --- a/crates/sqlquack-syntax-coverage/src/report/main/snapshot.rs +++ b/crates/sqlquack-syntax-coverage/src/report/main/snapshot.rs @@ -1,9 +1,10 @@ //! `snapshot`: the claims of one check, bound to the catalogue it ran on. -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; -use crate::check::models::CheckResult; +use crate::check::models::{CheckResult, LocalVerdict}; use crate::formats::constants::{ALIASES_FILE, CATALOGUE_DIR, SNAPSHOT_FORMAT, SOURCES_FILE}; +use crate::formats::models::Phase; use crate::report::models::Snapshot; /// Whether a coverage path belongs to the catalogue rather than to obligations or evidence. @@ -11,6 +12,22 @@ fn is_catalogue_file(path: &str) -> bool { path == SOURCES_FILE || path == ALIASES_FILE || path.starts_with(&format!("{CATALOGUE_DIR}/")) } +/// The receipted cases Snowflake parsed whose local verdict `accepted` says whether it parsed too. +fn snowflake_parsed(check: &CheckResult, accepted: bool) -> BTreeSet { + check + .forms + .iter() + .flat_map(|form| &form.obligations) + .flat_map(|obligation| &obligation.cases) + .filter(|case| { + case.phase.is_some_and(|phase| phase != Phase::SyntaxError) + && matches!(case.local, LocalVerdict::Accepted { .. }) == accepted + && !matches!(case.local, LocalVerdict::Failed { .. }) + }) + .map(|case| case.case_id.clone()) + .collect() +} + /// The snapshot of `check`; `file_hashes` are the loaded files' hashes by relative path. pub fn snapshot( check: &CheckResult, @@ -38,5 +55,7 @@ pub fn snapshot( .filter(|obligation| obligation.required && obligation.reason.is_none()) .map(|obligation| obligation.id.clone()) .collect(), + parsed_cases: snowflake_parsed(check, true), + refused_cases: snowflake_parsed(check, false), } } diff --git a/crates/sqlquack-syntax-coverage/src/report/models.rs b/crates/sqlquack-syntax-coverage/src/report/models.rs index 74f473d62..4aea498f4 100644 --- a/crates/sqlquack-syntax-coverage/src/report/models.rs +++ b/crates/sqlquack-syntax-coverage/src/report/models.rs @@ -58,6 +58,12 @@ pub struct Snapshot { /// Required obligations the checker found satisfied, written in obligation order. #[serde(serialize_with = "in_obligation_order")] pub satisfied_obligations: BTreeSet, + /// Receipted cases Snowflake parsed that the parser accepts, opaque or not. + #[serde(default)] + pub parsed_cases: BTreeSet, + /// Receipted cases Snowflake parsed that the parser rejects. + #[serde(default)] + pub refused_cases: BTreeSet, } /// One snapshot against a later one; coverage and catalogue changes kept apart. @@ -74,4 +80,6 @@ pub struct SnapshotDiff { /// Obligations satisfied in the base that no longer are: regressions. pub obligations_lost: Vec, pub obligations_gained: Vec, + /// Cases Snowflake parsed that the base parser accepted, opaque or not, and the head rejects. + pub parsed_cases_refused: Vec, } diff --git a/crates/sqlquack-syntax-coverage/src/report/tests/helpers.rs b/crates/sqlquack-syntax-coverage/src/report/tests/helpers.rs index 69df0776f..4758a98dd 100644 --- a/crates/sqlquack-syntax-coverage/src/report/tests/helpers.rs +++ b/crates/sqlquack-syntax-coverage/src/report/tests/helpers.rs @@ -21,5 +21,7 @@ pub(crate) fn base() -> Snapshot { ("sf.query.with".to_owned(), FormState::Unverified), ]), satisfied_obligations: BTreeSet::from(["sf.query.select.o001".to_owned()]), + parsed_cases: BTreeSet::from(["sf.query.select.k1".to_owned()]), + refused_cases: BTreeSet::from(["sf.query.with.k2".to_owned()]), } } diff --git a/crates/sqlquack-syntax-coverage/src/report/tests/test_diff_snapshots.rs b/crates/sqlquack-syntax-coverage/src/report/tests/test_diff_snapshots.rs index 53a137fe5..8650825d4 100644 --- a/crates/sqlquack-syntax-coverage/src/report/tests/test_diff_snapshots.rs +++ b/crates/sqlquack-syntax-coverage/src/report/tests/test_diff_snapshots.rs @@ -83,6 +83,28 @@ fn given_a_later_snapshot_when_diffing_then_regressions_and_catalogue_changes_ar ..SnapshotDiff::default() }, }, + DiffTestCase { + description: "a case Snowflake parsed and the base parser accepted is now rejected", + head: |mut head| { + head.parsed_cases.remove("sf.query.select.k1"); + head.refused_cases.insert("sf.query.select.k1".to_owned()); + head + }, + expected_diff: SnapshotDiff { + parsed_cases_refused: ids(&["sf.query.select.k1"]), + ..SnapshotDiff::default() + }, + }, + DiffTestCase { + description: "a refused case now parsing, or one leaving the receipts, is no regression", + head: |mut head| { + head.refused_cases.remove("sf.query.with.k2"); + head.parsed_cases.insert("sf.query.with.k2".to_owned()); + head.parsed_cases.remove("sf.query.select.k1"); + head + }, + expected_diff: SnapshotDiff::default(), + }, DiffTestCase { description: "a verified form removed from the catalogue is also lost", head: |mut head| { diff --git a/docs/syntax-coverage.md b/docs/syntax-coverage.md index 12dc51b59..8e6ae6130 100644 --- a/docs/syntax-coverage.md +++ b/docs/syntax-coverage.md @@ -379,7 +379,8 @@ results on stdout and progress on stderr: - `boundaries --form `: the form's boundary and nesting-depth obligations, with their bounds, notes and cases. - `snapshot --catalogue-commit `: every form's state and every satisfied obligation as - JSON. It is refused unless the catalogue, `sources.toml` and `aliases.toml` in the working tree + JSON, with the receipted cases Snowflake parsed split into those the parser accepts (opaque or + not) and those it rejects. It is refused unless the catalogue, `sources.toml` and `aliases.toml` in the working tree equal those of the commit. - `diff BASE [HEAD]`: one snapshot against a later one, or against the current tree when HEAD is omitted. @@ -388,8 +389,11 @@ results on stdout and progress on stderr: `coverage/syntax/baseline.json` is a snapshot on the frozen catalogue. `make syntax-coverage`, part of `make check-ci`, runs `diff` against it and fails in two cases. The first is a coverage -regression: a form the baseline records as verified or excluded that no longer is, or a satisfied -obligation that no longer is. The second is a catalogue change: a changed sources, aliases or +regression: a form the baseline records as verified or excluded that no longer is, a satisfied +obligation that no longer is, or a receipted case Snowflake parsed that the base parser accepted +and the current one rejects. That last line catches a case that moves from an opaque acceptance, +which satisfies no obligation, to a rejection; a pull request that has one explains each case. A +snapshot taken before these case lists existed has none, so it reports no such case. The second is a catalogue change: a changed sources, aliases or catalogue file, or a form added or removed. Catalogue changes are reported separately from coverage changes. Gains pass. After a reviewed gain or catalogue change, re-record the baseline with `snapshot` and commit it with the change. A review without fingerprints binds the parser From 8be69326d72ff4c444889f099fb9368681cb75c3 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 23:39:05 +0000 Subject: [PATCH 07/10] Snowflake parser: DESCRIBE reads its object kind and keeps the name DESC WAREHOUSE v1 parsed WAREHOUSE as the target and dropped v1; DESC DATABASE and DESC SCHEMA did the same, and multi-word kinds such as COMPUTE POOL or FILE FORMAT did not parse. Snowflake DESCRIBE now reads every kind the catalogue documents, then the name and that kind's options (TYPE, REVISION, IN APPLICATION, AS RESOURCE), and refuses an unknown kind or a bare name. The shared DESCRIBE no longer drops a word that is not a property. --- .../src/parser/_helpers/ddl/scripting.rs | 14 +- .../_helpers/ddl/snowflake_statements.rs | 94 +++++++++++++ .../sqlquack-syntax/src/parser/constants.rs | 98 ++++++++++++++ .../tests/parser/test_fixes.rs | 123 ++++++++++++++++++ 4 files changed, 328 insertions(+), 1 deletion(-) diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/scripting.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/scripting.rs index 0a271877e..fc2758fe8 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/scripting.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/scripting.rs @@ -91,6 +91,12 @@ impl Parser { return self.parse_snowflake_explain_using(leading_comments); } + if !is_explain && self.is_snowflake() { + if let Some(describe) = self.parse_snowflake_describe(&leading_comments)? { + return Ok(describe); + } + } + let analyze_option: bool = is_explain && self.parse_postgres_explain_options()?; let extended = self.match_identifier("EXTENDED"); @@ -131,7 +137,10 @@ impl Parser { if properties.is_empty() { while !self.is_at_end() && !self.check(TokenType::Semicolon) { - if self.check(TokenType::Var) || self.check(TokenType::Type) || self.check_keyword() + if (self.check(TokenType::Var) + || self.check(TokenType::Type) + || self.check_keyword()) + && self.peek_nth(1).map(|token| token.token_type) == Some(TokenType::Eq) { let name = self.advance_text()?.to_lowercase(); if self.match_token(TokenType::Eq) { @@ -253,6 +262,9 @@ impl Parser { let mut parts: Vec = Vec::new(); let mut paren_depth = 0usize; while !self.is_at_end() { + if self.is_snowflake() && self.check(TokenType::String) { + return Err(self.parse_error("Expected an argument data type")); + } if self.check(TokenType::LParen) { paren_depth += 1; parts.push(self.advance_text()?); diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_statements.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_statements.rs index 37e96b944..59f099ce8 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_statements.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_statements.rs @@ -2,6 +2,7 @@ use crate::expressions::statements::models::Describe; use crate::expressions::tree::models::Expression; +use crate::parser::constants::{SHARED_DESCRIBE_KINDS, SNOWFLAKE_DESCRIBE_KINDS}; use crate::parser::models::Parser; use crate::tokens::models::TokenType; use crate::tokens::types::Result; @@ -51,6 +52,99 @@ impl Parser { }))) } + /// Snowflake's DESCRIBE of a kind the shared one does not read; None for TABLE, VIEW and its other kinds. + pub(crate) fn parse_snowflake_describe( + &mut self, + leading_comments: &[String], + ) -> Result> { + let as_resource = self.match_text_seq(&["AS", "RESOURCE"]); + let Some(kind) = SNOWFLAKE_DESCRIBE_KINDS + .iter() + .find(|kind| self.match_text_seq(kind)) + .map(|kind| kind.join(" ")) + else { + let shared = SHARED_DESCRIBE_KINDS + .iter() + .any(|kind| self.peek_text().eq_ignore_ascii_case(kind)); + return match shared && !as_resource { + true => Ok(None), + false => Err(self.parse_error("Expected an object kind after DESCRIBE")), + }; + }; + if as_resource && !matches!(kind.as_str(), "AGENT") { + return Err(self.parse_error("Expected AGENT after DESCRIBE AS RESOURCE")); + } + let target = match kind.as_str() { + "TRANSACTION" if self.check(TokenType::Number) => self.parse_primary()?, + "RESULT" + if self.check(TokenType::String) + || (self.check_identifier("LAST_QUERY_ID") + && self.check_next(TokenType::LParen)) => + { + self.parse_primary()? + } + "TRANSACTION" => return Err(self.parse_error("Expected a transaction ID")), + "RESULT" => return Err(self.parse_error("Expected a query ID or LAST_QUERY_ID()")), + _ => Expression::Table(Box::new(self.parse_object_name()?)), + }; + let properties = match kind.as_str() { + "EXTERNAL TABLE" | "ICEBERG TABLE" => { + self.parse_describe_choice("TYPE", &["COLUMNS", "STAGE"])? + } + "LISTING" => self.parse_describe_choice("REVISION", &["DRAFT", "PUBLISHED"])?, + "CONFIGURATION" | "SPECIFICATION" if self.match_text_seq(&["IN", "APPLICATION"]) => { + let start = self.current; + self.parse_object_name()?; + vec![("in_application".to_owned(), self.written_since(start))] + } + _ => Vec::new(), + }; + Ok(Some(Expression::Describe(Box::new(Describe { + target, + extended: false, + formatted: false, + kind: Some(kind), + properties, + style: as_resource.then(|| "AS RESOURCE".to_owned()), + partition: None, + leading_comments: leading_comments.to_vec(), + as_json: false, + params: Vec::new(), + })))) + } + + /// An optional ` = ` after a DESCRIBE target, as the shared DESCRIBE keeps one. + fn parse_describe_choice( + &mut self, + name: &str, + choices: &[&str], + ) -> Result> { + if !self.match_identifier(name) { + return Ok(Vec::new()); + } + self.expect_token(TokenType::Eq)?; + match choices + .iter() + .any(|choice| self.peek_text().eq_ignore_ascii_case(choice)) + { + true => Ok(vec![(name.to_ascii_lowercase(), self.advance_text()?)]), + false => { + Err(self.parse_error(format!("Expected {} after {name} =", choices.join(" or ")))) + } + } + } + + /// The tokens read since `start`, joined as written, quoted identifiers keeping their quotes. + fn written_since(&self, start: usize) -> String { + self.tokens[start..self.current] + .iter() + .map(|token| match token.token_type { + TokenType::QuotedIdentifier => format!("\"{}\"", token.text.replace('"', "\"\"")), + _ => token.text.to_string(), + }) + .collect() + } + /// Refuses a Snowflake SHOW ... LIKE pattern that is not a string. pub(crate) fn expect_snowflake_like_pattern(&self) -> Result<()> { match self.is_snowflake() && !self.check(TokenType::String) { diff --git a/crates/sqlquack-syntax/src/parser/constants.rs b/crates/sqlquack-syntax/src/parser/constants.rs index aa96172ec..e8842845e 100644 --- a/crates/sqlquack-syntax/src/parser/constants.rs +++ b/crates/sqlquack-syntax/src/parser/constants.rs @@ -440,3 +440,101 @@ pub(crate) const SNOWFLAKE_DROP_KINDS: &[&[&str]] = &[ &["VARIABLE"], &["WAREHOUSE"], ]; + +/// Snowflake DESCRIBE kinds the shared DESCRIBE does not read, each before any kind it prefixes. +pub(crate) const SNOWFLAKE_DESCRIBE_KINDS: &[&[&str]] = &[ + &["AGENT"], + &["ALERT"], + &["APPLICATION", "PACKAGE"], + &["APPLICATION", "SERVICE"], + &["APPLICATION"], + &["ARTIFACT", "REPOSITORY"], + &["AVAILABLE", "LISTING"], + &["AVAILABLE", "ORGANIZATION", "PROFILE"], + &["BACKUP", "POLICY"], + &["BACKUP", "SET"], + &["COMMITTED", "TASK"], + &["COMPUTE", "POOL"], + &["CONFIGURATION"], + &["CORTEX", "SEARCH", "SERVICE"], + &["DATABASE"], + &["DATA", "MOVEMENT", "POLICY"], + &["DATA", "MOVEMENT", "RULE"], + &["DBT", "PROJECT"], + &["DCM", "PROJECT"], + &["DYNAMIC", "TABLE"], + &["EVENT", "TABLE"], + &["EXPERIMENT"], + &["EXTERNAL", "ACCESS", "INTEGRATION"], + &["EXTERNAL", "AGENT"], + &["EXTERNAL", "CONSUMER"], + &["EXTERNAL", "TABLE"], + &["EXTERNAL", "VOLUME"], + &["FILE", "FORMAT"], + &["GATEWAY"], + &["GIT", "REPOSITORY"], + &["ICEBERG", "TABLE"], + &["LISTING"], + &["MATERIALIZED", "VIEW"], + &["MCP", "SERVER"], + &["MODEL", "MONITOR"], + &["NOTEBOOK"], + &["ONLINE", "FEATURE", "TABLE"], + &["OPENFLOW", "CONNECTOR"], + &["OPENFLOW", "DATA", "PLANE", "INTEGRATION"], + &["OPENFLOW", "DEPLOYMENT"], + &["OPENFLOW", "RUNTIME"], + &["ORGANIZATION", "PROFILE"], + &["PIPE"], + &["POSTGRES", "INSTANCE"], + &["RESTRICTED", "SESSION", "SCOPE"], + &["RESULT"], + &["RSS"], + &["SCHEMA"], + &["SEARCH", "OPTIMIZATION", "ON"], + &["SECRET"], + &["SEMANTIC", "VIEW"], + &["SEQUENCE"], + &["SERVICE"], + &["SHARE"], + &["SNAPSHOT", "POLICY"], + &["SNAPSHOT", "SET"], + &["SNAPSHOT"], + &["SPECIFICATION"], + &["STAGE"], + &["STREAMLIT"], + &["STREAM"], + &["TASK", "GRAPH"], + &["TASK"], + &["TRANSACTION"], + &["TYPE"], + &["USER"], + &["VARIABLE"], + &["WAREHOUSE"], + &["AGGREGATION", "POLICY"], + &["AUTHENTICATION", "POLICY"], + &["FEATURE", "POLICY"], + &["INGRESS", "POLICY"], + &["JOIN", "POLICY"], + &["MAINTENANCE", "POLICY"], + &["MASKING", "POLICY"], + &["MULTI", "PARTY", "APPROVAL", "POLICY"], + &["NETWORK", "POLICY"], + &["NETWORK", "RULE"], + &["PACKAGES", "POLICY"], + &["PASSWORD", "POLICY"], + &["PRIVACY", "POLICY"], + &["PROJECTION", "POLICY"], + &["ROW", "ACCESS", "POLICY"], + &["SESSION", "POLICY"], + &["STORAGE", "LIFECYCLE", "POLICY"], + &["API", "INTEGRATION"], + &["CATALOG", "INTEGRATION"], + &["NOTIFICATION", "INTEGRATION"], + &["SECURITY", "INTEGRATION"], + &["STORAGE", "INTEGRATION"], + &["INTEGRATION"], +]; + +/// The kinds the shared DESCRIBE reads for Snowflake. +pub(crate) const SHARED_DESCRIBE_KINDS: &[&str] = &["TABLE", "VIEW", "FUNCTION", "PROCEDURE"]; diff --git a/crates/sqlquack-syntax/tests/parser/test_fixes.rs b/crates/sqlquack-syntax/tests/parser/test_fixes.rs index 3b0f443f9..668fc632f 100644 --- a/crates/sqlquack-syntax/tests/parser/test_fixes.rs +++ b/crates/sqlquack-syntax/tests/parser/test_fixes.rs @@ -1474,3 +1474,126 @@ fn given_a_snowflake_alter_of_an_application_database_or_service_kind_when_parsi ); } } + +#[test] +fn given_a_snowflake_describe_when_parsing_then_it_needs_a_kind_and_a_name() { + let test_cases = [ + AcceptanceTestCase { + description: "a two-word kind is read whole (recorded semantic error)", + sql: "DESC COMPUTE POOL v1 ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "half a kind is no kind (recorded errno 1003)", + sql: "DESC POOL v1 ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "a kind needs its name (recorded errno 1003)", + sql: "DESC FILE FORMAT ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "a bare name is no kind (recorded errno 1003)", + sql: "DESC v1 ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "a word after the name is refused, not dropped", + sql: "DESC TABLE v1 v2 ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "an argument type is no string (recorded errno 1003)", + sql: "DESC FUNCTION v1 ( 'INT' ) ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "AS RESOURCE comes before AGENT only", + sql: "DESC AS RESOURCE STAGE v1 ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "a transaction is described by its number (recorded errno 1003 without)", + sql: "DESC TRANSACTION v1 ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + ]; + + for test_case in test_cases { + assert_eq!( + accepted(test_case.sql, test_case.dialect), + test_case.expected_accepted, + "{}", + test_case.description + ); + } +} + +#[test] +fn given_a_snowflake_describe_when_parsing_then_the_tree_keeps_its_kind_name_and_options() { + let test_cases = [ + ProjectionTestCase { + description: "a one-word kind keeps the name after it", + sql: "DESC WAREHOUSE v1", + pointer: "/0/describe/target/table/name/name", + expected_value: json!("v1"), + }, + ProjectionTestCase { + description: "DATABASE is the kind, not the name", + sql: "DESCRIBE DATABASE v1", + pointer: "/0/describe/kind", + expected_value: json!("DATABASE"), + }, + ProjectionTestCase { + description: "a three-word kind is named whole", + sql: "DESC CORTEX SEARCH SERVICE v1", + pointer: "/0/describe/kind", + expected_value: json!("CORTEX SEARCH SERVICE"), + }, + ProjectionTestCase { + description: "AS RESOURCE is kept", + sql: "DESC AS RESOURCE AGENT v1", + pointer: "/0/describe/style", + expected_value: json!("AS RESOURCE"), + }, + ProjectionTestCase { + description: "the application keeps its quoting", + sql: "DESC CONFIGURATION v1 IN APPLICATION \"a b\".v2", + pointer: "/0/describe/properties/0", + expected_value: json!(["in_application", "\"a b\".v2"]), + }, + ProjectionTestCase { + description: "a listing revision is kept", + sql: "DESC LISTING v1 REVISION = DRAFT", + pointer: "/0/describe/properties/0", + expected_value: json!(["revision", "DRAFT"]), + }, + ProjectionTestCase { + description: "a transaction number is the target", + sql: "DESC TRANSACTION 12", + pointer: "/0/describe/target/literal/value", + expected_value: json!("12"), + }, + ]; + + for test_case in test_cases { + assert_eq!( + statements(test_case.sql, Dialect::Snowflake, CallMode::Faithful) + .pointer(test_case.pointer) + .cloned() + .unwrap_or_default(), + test_case.expected_value, + "{}", + test_case.description + ); + } +} From 6eefa6abe1b71adf98594ca8ba40883c667c634f Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 23:58:59 +0000 Subject: [PATCH 08/10] Snowflake parser: SHOW reads its documented kind and that kind's clauses The shared SHOW folded every word up to a clause keyword into the kind, so SHOW DATABASES WITH USAGE, SHOW POOLS or SHOW OFFERS IN LISTING v1 parsed with the clauses swallowed or dropped. Snowflake SHOW now reads a documented kind, then LIKE, IN with the containers that kind takes (some kinds need one), the follow-up a kind takes (FOR PACKAGE, RUN, FOR METRIC...), STARTS WITH and LIMIT ... FROM with strings, and WITH PRIVILEGES as a list. GRANTS, PARAMETERS, USER and a few others keep the shared reading. An object name now refuses a reserved word in any dotted part, not only the first. --- .../src/parser/_helpers/ddl/mod.rs | 1 + .../src/parser/_helpers/ddl/scripting.rs | 5 + .../parser/_helpers/ddl/snowflake_alter.rs | 16 +- .../src/parser/_helpers/ddl/snowflake_show.rs | 194 ++++++++++++ .../sqlquack-syntax/src/parser/constants.rs | 287 ++++++++++++++++++ .../tests/parser/test_fixes.rs | 123 ++++++++ 6 files changed, 625 insertions(+), 1 deletion(-) create mode 100644 crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_show.rs diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/mod.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/mod.rs index 76375dcc6..1fa58e285 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/mod.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/mod.rs @@ -11,6 +11,7 @@ pub(crate) mod snowflake_alter; pub(crate) mod snowflake_alter_account; pub(crate) mod snowflake_alter_objects; pub(crate) mod snowflake_alter_resources; +pub(crate) mod snowflake_show; pub(crate) mod snowflake_statements; pub(crate) mod table_options; pub(crate) mod transactions; diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/scripting.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/scripting.rs index fc2758fe8..85de054a8 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/scripting.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/scripting.rs @@ -323,6 +323,11 @@ impl Parser { self.expect_token(TokenType::Show)?; let terse = self.match_identifier("TERSE"); + if self.is_snowflake() { + if let Some(show) = self.parse_snowflake_show(terse)? { + return Ok(show); + } + } let (this_parts, target, mutex) = self.parse_show_object_kind()?; 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 5fdb1c991..210ab356b 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter.rs @@ -109,7 +109,21 @@ impl Parser { { return Err(self.parse_error("Expected an object name")); } - self.parse_table_ref() + let start = self.current; + let name = self.parse_table_ref()?; + for pair in self.tokens[start..self.current].windows(2) { + let reserved = crate::parser::constants::SNOWFLAKE_RESERVED_KEYWORDS + .contains(&pair[1].text.to_ascii_uppercase().as_str()); + if pair[0].token_type == TokenType::Dot + && pair[1].token_type != TokenType::QuotedIdentifier + && reserved + { + return Err( + self.parse_error("Expected an object name part, not a reserved keyword") + ); + } + } + Ok(name) } /// Whether the current token can start an object name. diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_show.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_show.rs new file mode 100644 index 000000000..316d937ab --- /dev/null +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_show.rs @@ -0,0 +1,194 @@ +//! Snowflake SHOW: a documented kind, then the clauses that kind takes, each with its own operand. + +use crate::expressions::queries::models::Limit; +use crate::expressions::statements::models::Show; +use crate::expressions::tree::models::Expression; +use crate::parser::constants::{ + SHARED_SHOW_KINDS, SNOWFLAKE_SHOW_CONTAINERS, SNOWFLAKE_SHOW_EXTRA_CONTAINERS, + SNOWFLAKE_SHOW_GENERAL_CONTAINERS, SNOWFLAKE_SHOW_KINDS, SNOWFLAKE_SHOW_REQUIRED_CONTAINERS, +}; +use crate::parser::models::Parser; +use crate::tokens::models::TokenType; +use crate::tokens::types::Result; + +impl Parser { + /// A Snowflake SHOW after SHOW and TERSE; None leaves the kinds the shared SHOW reads to it. + pub(crate) fn parse_snowflake_show(&mut self, terse: bool) -> Result> { + let shared = SHARED_SHOW_KINDS + .iter() + .any(|kind| self.check_text_seq(kind)) + || self.check_next(TokenType::Dot); + if shared { + return Ok(None); + } + let Some(kind) = SNOWFLAKE_SHOW_KINDS + .iter() + .find(|kind| self.match_text_seq(kind)) + .map(|kind| kind.join(" ")) + else { + return Err(self.parse_error("Expected an object kind after SHOW")); + }; + let mut show = Show::new(kind.clone()); + show.terse = terse; + show.history = self.match_identifier("HISTORY"); + if matches!(kind.as_str(), "DELEGATED AUTHORIZATIONS BY USER") { + show.for_target = Some(self.parse_show_name()?); + return Ok(Some(Expression::Show(Box::new(show)))); + } + if matches!(kind.as_str(), "OBJECTS") + && self.match_text_seq(&["OWNED", "BY", "APPLICATION"]) + { + show.scope_kind = Some("OWNED BY APPLICATION".to_owned()); + show.scope = Some(self.parse_show_name()?); + return Ok(Some(Expression::Show(Box::new(show)))); + } + if self.match_token(TokenType::Like) { + show.like = Some(self.parse_show_string("LIKE")?); + } + let required = SNOWFLAKE_SHOW_REQUIRED_CONTAINERS + .iter() + .find(|(name, _)| *name == kind) + .map(|(_, containers)| *containers); + if required.is_some() || self.check(TokenType::In) { + self.expect_token(TokenType::In)?; + let (container, name) = self.parse_show_container(&kind)?; + let allowed = |container: &str| match required { + Some(required) => required.contains(&container), + None => { + SNOWFLAKE_SHOW_GENERAL_CONTAINERS.contains(&container) + || SNOWFLAKE_SHOW_EXTRA_CONTAINERS + .iter() + .any(|(name, extra)| *name == kind && extra.contains(&container)) + } + }; + if !allowed(container.as_deref().unwrap_or("")) { + return Err(self.parse_error(format!("Expected a container {kind} is shown in"))); + } + show.scope_kind = container; + show.scope = name; + } + let mut show = self.parse_show_follow_up(&kind, show)?; + if self.match_identifier("STARTS") { + self.expect_token(TokenType::With)?; + show.starts_with = Some(self.parse_show_string("STARTS WITH")?); + } + if self.match_token(TokenType::Limit) { + if !self.check(TokenType::Number) { + return Err(self.parse_error("Expected a row count after LIMIT")); + } + show.limit = Some(Box::new(Limit { + this: self.parse_primary()?, + percent: false, + comments: Vec::new(), + })); + if self.match_token(TokenType::From) { + show.from = Some(self.parse_show_string("FROM")?); + } + } + if show.scope_kind.is_none() && show.scope.is_none() && self.match_token(TokenType::In) { + let (container, name) = self.parse_show_container(&kind)?; + show.scope_kind = container; + show.scope = name; + } + if self.match_token(TokenType::With) { + show = self.parse_show_with(&kind, show)?; + } + if matches!(kind.as_str(), "MFA METHODS") && self.match_token(TokenType::For) { + if !self.match_identifier("USER") { + return Err(self.parse_error("Expected USER after FOR")); + } + show.for_target = Some(self.parse_show_name()?); + } + Ok(Some(Expression::Show(Box::new(show)))) + } + + /// A quoted string operand after `clause`. + fn parse_show_string(&mut self, clause: &str) -> Result { + match self.check(TokenType::String) { + true => self.parse_primary(), + false => Err(self.parse_error(format!("Expected a string after {clause}"))), + } + } + + /// An object name, kept as written. + fn parse_show_name(&mut self) -> Result { + Ok(Expression::Table(Box::new(self.parse_object_name()?))) + } + + /// The container after IN and its name; ACCOUNT, DATABASE and SCHEMA may stand alone. + fn parse_show_container(&mut self, kind: &str) -> Result<(Option, Option)> { + let Some(container) = SNOWFLAKE_SHOW_CONTAINERS + .iter() + .find(|container| self.match_text_seq(container)) + .map(|container| container.join(" ")) + else { + return Ok((None, Some(self.parse_show_name()?))); + }; + let groups = matches!(kind, "FAILOVER GROUPS" | "REPLICATION GROUPS"); + let optional = match container.as_str() { + "ACCOUNT" => !groups, + "DATABASE" | "SCHEMA" => !matches!(kind, "DATABASE ROLES" | "SCHEMA VARIABLES"), + "TABLE" | "VIEW" => true, + _ => false, + }; + let ends = self.is_at_end() + || self.check(TokenType::Semicolon) + || self.check(TokenType::Like) + || self.check(TokenType::Limit) + || self.check_identifier("STARTS"); + let name = match (container.as_str(), optional && ends) { + (_, true) => None, + ("ACCOUNT", _) if !groups => None, + _ => Some(self.parse_show_name()?), + }; + Ok((Some(container), name)) + } + + /// What some kinds take after their container: a package, run, channel, metric or version. + fn parse_show_follow_up(&mut self, kind: &str, mut show: Show) -> Result { + let artifact = show.scope_kind.as_deref() == Some("ARTIFACT REPOSITORY"); + let model = show.scope_kind.as_deref() == Some("MODEL"); + let (words, required): (&[&str], bool) = match kind { + "VERSIONS" if artifact => (&["FOR", "PACKAGE"], true), + "SHARED CONTENT" => (&["FOR", "VERSION"], true), + "RELEASE DIRECTIVES" => (&["FOR", "RELEASE", "CHANNEL"], false), + "SEMANTIC DIMENSIONS" => (&["FOR", "METRIC"], false), + "RUN METRICS" | "RUN PARAMETERS" => (&["RUN"], false), + "FUNCTIONS" if model => (&["VERSION"], false), + _ => return Ok(show), + }; + if self.match_text_seq(words) { + let name = self.parse_show_name()?; + match words { + ["FOR", ..] => show.for_target = Some(name), + _ => show.target = Some(name), + } + } else if required { + return Err( + self.parse_error(format!("Expected {} after the container", words.join(" "))) + ); + } + Ok(show) + } + + /// `WITH PRIVILEGES [ , ... ]`, or `WITH PRIMARY .`. + fn parse_show_with(&mut self, kind: &str, mut show: Show) -> Result { + if matches!(kind, "REPLICATION DATABASES") && self.match_text_seq(&["PRIMARY"]) { + show.target = Some(self.parse_show_name()?); + return Ok(show); + } + if !self.match_identifier("PRIVILEGES") { + return Err(self.parse_error("Expected PRIVILEGES after WITH")); + } + loop { + if !(self.is_identifier_token() || self.check_keyword()) { + return Err(self.parse_error("Expected a privilege")); + } + show.privileges + .push(self.advance_text()?.to_ascii_uppercase()); + if !self.match_token(TokenType::Comma) { + return Ok(show); + } + } + } +} diff --git a/crates/sqlquack-syntax/src/parser/constants.rs b/crates/sqlquack-syntax/src/parser/constants.rs index e8842845e..82ae3bfae 100644 --- a/crates/sqlquack-syntax/src/parser/constants.rs +++ b/crates/sqlquack-syntax/src/parser/constants.rs @@ -538,3 +538,290 @@ pub(crate) const SNOWFLAKE_DESCRIBE_KINDS: &[&[&str]] = &[ /// The kinds the shared DESCRIBE reads for Snowflake. pub(crate) const SHARED_DESCRIBE_KINDS: &[&str] = &["TABLE", "VIEW", "FUNCTION", "PROCEDURE"]; + +/// Snowflake SHOW kinds, each before any kind it prefixes. +pub(crate) const SNOWFLAKE_SHOW_KINDS: &[&[&str]] = &[ + &["COMPUTE", "POOL", "INSTANCE", "FAMILIES"], + &["DELEGATED", "AUTHORIZATIONS", "BY", "USER"], + &["MULTI", "PARTY", "APPROVAL", "POLICIES"], + &["OPENFLOW", "DATA", "PLANE", "INTEGRATIONS"], + &["AVAILABLE", "ORGANIZATION", "PROFILES"], + &["CORTEX", "BASE", "MODELS"], + &["DATA", "METRIC", "FUNCTIONS"], + &["DATA", "MOVEMENT", "POLICIES"], + &["DATA", "MOVEMENT", "RULES"], + &["EVENT", "ROUTING", "TABLES"], + &["EXTERNAL", "ACCESS", "INTEGRATIONS"], + &["ONLINE", "FEATURE", "TABLES"], + &["OPENFLOW", "CONNECTOR", "DEFINITIONS"], + &["ORGANIZATION", "USER", "GROUPS"], + &["RESTRICTED", "SESSION", "SCOPES"], + &["ROW", "ACCESS", "POLICIES"], + &["STORAGE", "LIFECYCLE", "POLICIES"], + &["TELEMETRY", "EVENT", "DEFINITIONS"], + &["AGGREGATION", "POLICIES"], + &["API", "INTEGRATIONS"], + &["APPLICATION", "PACKAGES"], + &["APPLICATION", "ROLES"], + &["APPLICATION", "SERVICES"], + &["APPROVED", "SPECIFICATIONS"], + &["ARTIFACT", "REPOSITORIES"], + &["AUTHENTICATION", "POLICIES"], + &["AVAILABLE", "OFFERS"], + &["BACKUP", "POLICIES"], + &["BACKUP", "SETS"], + &["CATALOG", "INTEGRATIONS"], + &["CODE", "BUNDLES"], + &["COMMITTED", "TASKS"], + &["COMPUTE", "POOLS"], + &["DATABASE", "ROLES"], + &["DBT", "PROJECTS"], + &["DCM", "PROJECTS"], + &["DECLINED", "SPECIFICATIONS"], + &["DELEGATED", "AUTHORIZATIONS"], + &["DYNAMIC", "TABLES"], + &["EVENT", "TABLES"], + &["EXTERNAL", "AGENTS"], + &["EXTERNAL", "CONSUMERS"], + &["EXTERNAL", "FUNCTIONS"], + &["EXTERNAL", "TABLES"], + &["EXTERNAL", "VOLUMES"], + &["FAILOVER", "GROUPS"], + &["FILE", "FORMATS"], + &["GIT", "BRANCHES"], + &["GIT", "REPOSITORIES"], + &["GIT", "TAGS"], + &["GLOBAL", "ACCOUNTS"], + &["HYBRID", "TABLES"], + &["ICEBERG", "TABLES"], + &["IMAGE", "REPOSITORIES"], + &["INGRESS", "POLICIES"], + &["INTERACTIVE", "TABLES"], + &["JOIN", "POLICIES"], + &["MANAGED", "ACCOUNTS"], + &["MASKING", "POLICIES"], + &["MATERIALIZED", "VIEWS"], + &["MCP", "SERVERS"], + &["MFA", "METHODS"], + &["MODEL", "MONITORS"], + &["NETWORK", "POLICIES"], + &["NETWORK", "RULES"], + &["NOTEBOOK", "PROJECTS"], + &["NOTIFICATION", "INTEGRATIONS"], + &["OPENFLOW", "CONNECTORS"], + &["OPENFLOW", "DEPLOYMENTS"], + &["OPENFLOW", "RUNTIMES"], + &["ORGANIZATION", "ACCOUNTS"], + &["ORGANIZATION", "CONTRACTS"], + &["ORGANIZATION", "PROFILES"], + &["ORGANIZATION", "USERS"], + &["PACKAGES", "POLICIES"], + &["PASSWORD", "POLICIES"], + &["PENDING", "SPECIFICATIONS"], + &["POSTGRES", "INSTANCES"], + &["PRICING", "PLANS"], + &["PRIMARY", "KEYS"], + &["PRIVACY", "POLICIES"], + &["PROJECTION", "POLICIES"], + &["RELEASE", "CHANNELS"], + &["RELEASE", "DIRECTIVES"], + &["REPLICATION", "ACCOUNTS"], + &["REPLICATION", "DATABASES"], + &["REPLICATION", "GROUPS"], + &["RESOURCE", "MONITORS"], + &["RUN", "METRICS"], + &["RUN", "PARAMETERS"], + &["SCHEMA", "VARIABLES"], + &["SECURITY", "INTEGRATIONS"], + &["SEMANTIC", "DIMENSIONS"], + &["SEMANTIC", "FACTS"], + &["SEMANTIC", "METRICS"], + &["SEMANTIC", "VIEWS"], + &["SERVICE", "CONTAINERS"], + &["SERVICE", "INSTANCES"], + &["SERVICE", "VOLUMES"], + &["SESSION", "POLICIES"], + &["SHARED", "CONTENT"], + &["SNAPSHOT", "POLICIES"], + &["SNAPSHOT", "SETS"], + &["STORAGE", "INTEGRATIONS"], + &["TASK", "GRAPHS"], + &["ACCOUNTS"], + &["AGENTS"], + &["ALERTS"], + &["APPLICATIONS"], + &["BACKUPS"], + &["CHANNELS"], + &["CLASSES"], + &["COLUMNS"], + &["CONFIGURATIONS"], + &["CONNECTIONS"], + &["CONTACTS"], + &["DATABASES"], + &["DATASETS"], + &["DEPLOYMENTS"], + &["ENDPOINTS"], + &["ENTITIES"], + &["EXPERIMENTS"], + &["FILES"], + &["FUNCTIONS"], + &["GATEWAYS"], + &["IMAGES"], + &["INDEXES"], + &["INTEGRATIONS"], + &["LISTINGS"], + &["LOCKS"], + &["MODELS"], + &["NODES"], + &["NOTEBOOKS"], + &["OBJECTS"], + &["OFFERS"], + &["PACKAGES"], + &["PIPES"], + &["PRIVILEGES"], + &["PROCEDURES"], + &["REFERENCES"], + &["REGIONS"], + &["ROLES"], + &["RSS"], + &["RUNS"], + &["SCHEMAS"], + &["SECRETS"], + &["SEQUENCES"], + &["SHARES"], + &["SNAPSHOTS"], + &["SPECIFICATIONS"], + &["STAGES"], + &["STREAMLITS"], + &["STREAMS"], + &["TABLES"], + &["TAGS"], + &["TASKS"], + &["TRANSACTIONS"], + &["TYPES"], + &["USERS"], + &["VARIABLES"], + &["VERSIONS"], + &["VIEWS"], + &["WAREHOUSES"], + &["WORKSPACES"], +]; + +/// SHOW kinds the shared SHOW still reads for Snowflake, by their leading words. +pub(crate) const SHARED_SHOW_KINDS: &[&[&str]] = &[ + &["CALLER"], + &["CORTEX", "SEARCH", "SERVICES"], + &["AVAILABLE", "LISTINGS"], + &["EVENT", "ROUTING", "TABLE", "ON"], + &["FEATURE", "POLICIES"], + &["FUTURE"], + &["GRANTS"], + &["JOB"], + &["MAINTENANCE"], + &["PARAMETERS"], + &["RULES"], + &["SERVICES"], + &["USER"], +]; + +/// The containers a Snowflake SHOW ... IN names, each before any container it prefixes. +pub(crate) const SNOWFLAKE_SHOW_CONTAINERS: &[&[&str]] = &[ + &["EVENT", "ROUTING", "TABLE"], + &["ORGANIZATION", "USER", "GROUP"], + &["APPLICATION", "PACKAGE"], + &["ARTIFACT", "REPOSITORY"], + &["BACKUP", "SET"], + &["COMPUTE", "POOL"], + &["DBT", "PROJECT"], + &["DCM", "PROJECT"], + &["FAILOVER", "GROUP"], + &["GIT", "REPOSITORY"], + &["IMAGE", "REPOSITORY"], + &["OPENFLOW", "CONNECTOR"], + &["ORGANIZATION", "PROFILE"], + &["REPLICATION", "GROUP"], + &["SNAPSHOT", "SET"], + &["ACCOUNT"], + &["APPLICATION"], + &["CLASS"], + &["DATABASE"], + &["DATASET"], + &["EXPERIMENT"], + &["GRAPH"], + &["LISTING"], + &["MODEL"], + &["SCHEMA"], + &["SERVICE"], + &["TABLE"], + &["VIEW"], +]; + +/// The containers any Snowflake SHOW ... IN may name; "" is a bare name. +pub(crate) const SNOWFLAKE_SHOW_GENERAL_CONTAINERS: &[&str] = &[ + "", + "ACCOUNT", + "APPLICATION", + "APPLICATION PACKAGE", + "CLASS", + "DATABASE", + "SCHEMA", + "TABLE", + "VIEW", +]; + +/// Further containers some Snowflake SHOW kinds may name. +pub(crate) const SNOWFLAKE_SHOW_EXTRA_CONTAINERS: &[(&str, &[&str])] = &[ + ("COMMITTED TASKS", &["GRAPH"]), + ("DATABASES", &["FAILOVER GROUP", "REPLICATION GROUP"]), + ("FUNCTIONS", &["MODEL"]), + ("LISTINGS", &["FAILOVER GROUP"]), + ("ORGANIZATION USERS", &["ORGANIZATION USER GROUP"]), + ("ROLES", &["SERVICE"]), + ("SHARES", &["FAILOVER GROUP", "REPLICATION GROUP"]), + ("SNAPSHOTS", &["SNAPSHOT SET"]), +]; + +/// Snowflake SHOW kinds that need IN and one of these containers; "" is a bare name. +pub(crate) const SNOWFLAKE_SHOW_REQUIRED_CONTAINERS: &[(&str, &[&str])] = &[ + ("APPLICATION ROLES", &["APPLICATION"]), + ("AVAILABLE OFFERS", &["LISTING"]), + ("BACKUPS", &["BACKUP SET"]), + ("DATABASE ROLES", &["DATABASE"]), + ("DEPLOYMENTS", &["DCM PROJECT"]), + ("ENDPOINTS", &["SERVICE"]), + ("ENTITIES", &["DCM PROJECT"]), + ("FILES", &["ARTIFACT REPOSITORY"]), + ("GIT BRANCHES", &["GIT REPOSITORY", ""]), + ("GIT TAGS", &["GIT REPOSITORY", ""]), + ("IMAGES", &["IMAGE REPOSITORY"]), + ("NODES", &["COMPUTE POOL"]), + ("OFFERS", &["LISTING"]), + ("PACKAGES", &["ARTIFACT REPOSITORY"]), + ("PRICING PLANS", &["LISTING"]), + ("PRIVILEGES", &["APPLICATION"]), + ("REFERENCES", &["APPLICATION"]), + ("RELEASE CHANNELS", &["APPLICATION PACKAGE", "LISTING"]), + ("RELEASE DIRECTIVES", &["APPLICATION PACKAGE"]), + ("RUN METRICS", &["EXPERIMENT"]), + ("RUN PARAMETERS", &["EXPERIMENT"]), + ("RUNS", &["EXPERIMENT"]), + ("SCHEMA VARIABLES", &["SCHEMA"]), + ("SERVICE CONTAINERS", &["SERVICE"]), + ("SERVICE INSTANCES", &["SERVICE"]), + ("SERVICE VOLUMES", &["SERVICE"]), + ("SHARED CONTENT", &["APPLICATION PACKAGE"]), + ("TELEMETRY EVENT DEFINITIONS", &["APPLICATION"]), + ( + "VERSIONS", + &[ + "APPLICATION PACKAGE", + "ARTIFACT REPOSITORY", + "DATASET", + "DBT PROJECT", + "LISTING", + "MODEL", + "OPENFLOW CONNECTOR", + "ORGANIZATION PROFILE", + ], + ), +]; diff --git a/crates/sqlquack-syntax/tests/parser/test_fixes.rs b/crates/sqlquack-syntax/tests/parser/test_fixes.rs index b960b337e..bc59ac276 100644 --- a/crates/sqlquack-syntax/tests/parser/test_fixes.rs +++ b/crates/sqlquack-syntax/tests/parser/test_fixes.rs @@ -1633,3 +1633,126 @@ fn given_a_snowflake_describe_when_parsing_then_the_tree_keeps_its_kind_name_and ); } } + +#[test] +fn given_a_snowflake_show_when_parsing_then_its_kind_and_clauses_follow_the_grammar() { + let test_cases = [ + AcceptanceTestCase { + description: "a shown kind in its container (recorded semantic error)", + sql: "SHOW OFFERS IN LISTING v1 ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a kind that needs a container refuses a bare name (recorded errno 1003)", + sql: "SHOW OFFERS IN v1 ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "half a kind is no kind (recorded errno 1003)", + sql: "SHOW POOLS ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "STARTS WITH takes a string (recorded errno 1003)", + sql: "SHOW DATABASES STARTS WITH m1 ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "WITH PRIVILEGES needs a privilege (recorded errno 1003)", + sql: "SHOW DATABASES WITH PRIVILEGES ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "a container another kind owns is refused (recorded errno 1003)", + sql: "SHOW CHANNELS IN LISTING v1 ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "a reserved word is no part of a dotted name (recorded errno 1003)", + sql: "SHOW REPLICATION DATABASES WITH PRIMARY v1 . SELECT ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "TABLE stands alone as a container (recorded semantic error)", + sql: "SHOW COLUMNS IN TABLE ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + ]; + + for test_case in test_cases { + assert_eq!( + accepted(test_case.sql, test_case.dialect), + test_case.expected_accepted, + "{}", + test_case.description + ); + } +} + +#[test] +fn given_a_snowflake_show_when_parsing_then_the_tree_keeps_kind_container_and_operands() { + let test_cases = [ + ProjectionTestCase { + description: "a multi-word kind is named whole", + sql: "SHOW COMPUTE POOL INSTANCE FAMILIES", + pointer: "/0/show/this", + expected_value: json!("COMPUTE POOL INSTANCE FAMILIES"), + }, + ProjectionTestCase { + description: "the container kind is kept", + sql: "SHOW ENTITIES IN DCM PROJECT v1", + pointer: "/0/show/scope_kind", + expected_value: json!("DCM PROJECT"), + }, + ProjectionTestCase { + description: "the package after an artifact repository is kept", + sql: "SHOW VERSIONS IN ARTIFACT REPOSITORY v1 FOR PACKAGE p1", + pointer: "/0/show/for_target/table/name/name", + expected_value: json!("p1"), + }, + ProjectionTestCase { + description: "the run of an experiment is kept", + sql: "SHOW RUN METRICS IN EXPERIMENT v1 RUN r1", + pointer: "/0/show/target/table/name/name", + expected_value: json!("r1"), + }, + ProjectionTestCase { + description: "privileges are a list", + sql: "SHOW DATABASES WITH PRIVILEGES USAGE, MODIFY", + pointer: "/0/show/privileges", + expected_value: json!(["USAGE", "MODIFY"]), + }, + ProjectionTestCase { + description: "the primary keeps its account and database apart", + sql: "SHOW REPLICATION DATABASES WITH PRIMARY a1.\"d 1\"", + pointer: "/0/show/target/table/schema/name", + expected_value: json!("a1"), + }, + ProjectionTestCase { + description: "the primary database keeps its quoting", + sql: "SHOW REPLICATION DATABASES WITH PRIMARY a1.\"d 1\"", + pointer: "/0/show/target/table/name/quoted", + expected_value: json!(true), + }, + ]; + + for test_case in test_cases { + assert_eq!( + statements(test_case.sql, Dialect::Snowflake, CallMode::Faithful) + .pointer(test_case.pointer) + .cloned() + .unwrap_or_default(), + test_case.expected_value, + "{}", + test_case.description + ); + } +} From accaaa31c681bd33d314b4d4315fd37516146b82 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 5 Nov 2026 00:10:18 +0000 Subject: [PATCH 09/10] Snowflake parser: keep SELECT ALL, aggregate DISTINCT, NTZ, transaction names and role lists From the omission audit after #46 and #48: - SELECT ALL sets select.all; it was consumed and dropped. - An aggregate of several arguments, such as APPROX_COUNT_DISTINCT ( DISTINCT a , b ), keeps distinct; the generic call dropped it. - TIMESTAMP [ ( p ) ] WITHOUT TIME ZONE is TIMESTAMP_NTZ, as its synonyms are, not bare TIMESTAMP (whose meaning follows TIMESTAMP_TYPE_MAPPING). - START TRANSACTION NAME n keeps n as the name, quoting kept, apart from NAME. - USE SECONDARY ROLES lists each role with its quoting in use.roles. --- .../src/classification/_helpers/clauses.rs | 1 + .../src/expressions/queries/models.rs | 4 ++ .../src/expressions/statements/models.rs | 3 ++ .../src/parser/_helpers/ddl/alter.rs | 14 ++++- .../src/parser/_helpers/ddl/transactions.rs | 10 ++++ .../src/parser/_helpers/functions/typed.rs | 2 +- .../src/parser/_helpers/queries/select.rs | 1 + .../src/parser/_helpers/statements/entry.rs | 5 ++ .../parser/_helpers/types/cast_base_types.rs | 21 ++++++-- .../src/parser/_helpers/types/casts.rs | 5 +- crates/sqlquack-syntax/src/parser/models.rs | 1 + .../tests/parser/test_fixes.rs | 54 +++++++++++++++++++ 12 files changed, 111 insertions(+), 10 deletions(-) diff --git a/crates/sqlquack-core/src/classification/_helpers/clauses.rs b/crates/sqlquack-core/src/classification/_helpers/clauses.rs index c0a4ae1ae..cdd981972 100644 --- a/crates/sqlquack-core/src/classification/_helpers/clauses.rs +++ b/crates/sqlquack-core/src/classification/_helpers/clauses.rs @@ -56,6 +56,7 @@ fn select_clauses(select: &Select) -> Vec { limit_by, fetch, distinct, + all: _, distinct_on, top, with, diff --git a/crates/sqlquack-syntax/src/expressions/queries/models.rs b/crates/sqlquack-syntax/src/expressions/queries/models.rs index c4ed23bea..8db6c0633 100644 --- a/crates/sqlquack-syntax/src/expressions/queries/models.rs +++ b/crates/sqlquack-syntax/src/expressions/queries/models.rs @@ -44,6 +44,9 @@ pub struct Select { pub limit_by: Option>, pub fetch: Option, pub distinct: bool, + /// Whether ALL was written where DISTINCT could be: it changes nothing, but was written. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub all: bool, pub distinct_on: Option>, pub top: Option, pub with: Option, @@ -130,6 +133,7 @@ impl Select { limit_by: None, fetch: None, distinct: false, + all: false, distinct_on: None, top: None, with: None, diff --git a/crates/sqlquack-syntax/src/expressions/statements/models.rs b/crates/sqlquack-syntax/src/expressions/statements/models.rs index 77cd818eb..6cf7feee1 100644 --- a/crates/sqlquack-syntax/src/expressions/statements/models.rs +++ b/crates/sqlquack-syntax/src/expressions/statements/models.rs @@ -506,6 +506,9 @@ pub struct Use { pub kind: Option, /// The name of the object pub this: Identifier, + /// Snowflake's secondary roles one by one, quoting kept; empty for ALL, NONE and other kinds. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub roles: Vec, } /// Kind of USE statement diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/alter.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/alter.rs index 383908ffc..dae43e905 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/alter.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/alter.rs @@ -1424,8 +1424,10 @@ impl Parser { self.skip(); let snowflake = self.is_snowflake(); let mut roles: Vec = Vec::new(); + let mut listed: Vec = Vec::new(); loop { if snowflake && self.check(TokenType::QuotedIdentifier) { + listed.push(Identifier::quoted(self.peek_text())); roles.push(self.parse_quoted_role()?); if !self.match_token(TokenType::Comma) { break; @@ -1435,6 +1437,7 @@ impl Parser { || self.check(TokenType::Identifier) { let role = self.advance_text()?; + listed.push(Identifier::new(role.clone())); roles.push(role); if !self.match_token(TokenType::Comma) { break; @@ -1451,9 +1454,14 @@ impl Parser { } else { roles.join(", ") }; + let keyword = matches!(roles.as_slice(), [role] if role.eq_ignore_ascii_case("ALL") || role.eq_ignore_ascii_case("NONE")); return Ok(Expression::Use(Box::new(Use { kind: Some(UseKind::SecondaryRoles), this: Identifier::new(name), + roles: match snowflake && !keyword { + true => listed, + false => Vec::new(), + }, }))); } } @@ -1483,7 +1491,11 @@ impl Parser { ident.name = format!("{}.{}", ident.name, second_part.name); } - Ok(Expression::Use(Box::new(Use { kind, this: ident }))) + Ok(Expression::Use(Box::new(Use { + kind, + this: ident, + roles: Vec::new(), + }))) } /// Parses BigQuery EXPORT DATA [WITH CONNECTION c] OPTIONS (...) AS SELECT ... diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/transactions.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/transactions.rs index cd0f6d365..3e0f2eaac 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/transactions.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/transactions.rs @@ -19,6 +19,16 @@ impl Parser { self.expect_token(TokenType::Transaction)?; self.refuse_snowflake_nameless_transaction()?; + if self.is_snowflake() && self.match_identifier("NAME") { + let name = self.expect_identifier_or_safe_keyword_with_quoted()?; + return Ok(Expression::Transaction(Box::new(Transaction { + this: Some(Box::new(Expression::Identifier(name))), + modes: None, + mark: Some(Box::new(Expression::Identifier(Identifier::new( + "START".to_string(), + )))), + }))); + } let mut mode_parts: Vec = Vec::new(); while self.is_identifier_token() diff --git a/crates/sqlquack-syntax/src/parser/_helpers/functions/typed.rs b/crates/sqlquack-syntax/src/parser/_helpers/functions/typed.rs index 30b89d8dc..2f4167556 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/functions/typed.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/functions/typed.rs @@ -851,7 +851,7 @@ impl Parser { name: name.to_string(), args, - distinct: false, + distinct, trailing_comments: Vec::new(), use_bracket_syntax: false, no_parens: false, diff --git a/crates/sqlquack-syntax/src/parser/_helpers/queries/select.rs b/crates/sqlquack-syntax/src/parser/_helpers/queries/select.rs index af8cff39f..a5835104f 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/queries/select.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/queries/select.rs @@ -446,6 +446,7 @@ impl Parser { limit_by: None, fetch: None, distinct: false, + all: false, distinct_on: None, top: None, with: None, diff --git a/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs b/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs index 36bb025e3..34b4b7ca7 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs @@ -1180,6 +1180,8 @@ impl Parser { let top = self.parse_select_top()?; let is_distinct_token = self.match_token(TokenType::Distinct); + let all = + !is_distinct_token && self.check(TokenType::All) && !self.check_next(TokenType::Dot); let (distinct, distinct_on) = if is_distinct_token { if self.match_token(TokenType::On) { self.expect_token(TokenType::LParen)?; @@ -1311,6 +1313,7 @@ impl Parser { hint, top, distinct, + all, distinct_on, kind, operation_modifiers, @@ -1539,6 +1542,7 @@ impl Parser { hint, top, distinct, + all, distinct_on, kind, operation_modifiers, @@ -1575,6 +1579,7 @@ impl Parser { limit_by, fetch, distinct, + all, distinct_on, top, with: None, diff --git a/crates/sqlquack-syntax/src/parser/_helpers/types/cast_base_types.rs b/crates/sqlquack-syntax/src/parser/_helpers/types/cast_base_types.rs index 3da8444e3..625fdbf36 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/types/cast_base_types.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/types/cast_base_types.rs @@ -346,10 +346,7 @@ impl Parser { } } else if self.match_keyword("WITHOUT") { self.match_time_zone_words()?; - DataType::Timestamp { - precision, - timezone: false, - } + self.timestamp_without_time_zone(precision) } else { DataType::Timestamp { precision, @@ -359,6 +356,22 @@ impl Parser { }) } + /// `TIMESTAMP WITHOUT TIME ZONE`: Snowflake's TIMESTAMP_NTZ, whatever TIMESTAMP maps to. + pub(crate) fn timestamp_without_time_zone(&self, precision: Option) -> DataType { + match (self.is_snowflake(), precision) { + (true, Some(precision)) => DataType::Custom { + name: format!("TIMESTAMPNTZ({precision})"), + }, + (true, None) => DataType::Custom { + name: "TIMESTAMPNTZ".to_owned(), + }, + (false, _) => DataType::Timestamp { + precision, + timezone: false, + }, + } + } + pub(crate) fn parse_cast_timestamp_tz_type(&mut self) -> Result { Ok({ let precision = if self.match_token(TokenType::LParen) { diff --git a/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs b/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs index c8afaf767..54dcc27d7 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs @@ -784,10 +784,7 @@ impl Parser { } } else if self.match_keyword("WITHOUT") { self.match_time_zone_words()?; - Ok(DataType::Timestamp { - precision, - timezone: false, - }) + Ok(self.timestamp_without_time_zone(precision)) } else { Ok(DataType::Timestamp { precision, diff --git a/crates/sqlquack-syntax/src/parser/models.rs b/crates/sqlquack-syntax/src/parser/models.rs index 3f1c9a91d..7a0d4936f 100644 --- a/crates/sqlquack-syntax/src/parser/models.rs +++ b/crates/sqlquack-syntax/src/parser/models.rs @@ -430,6 +430,7 @@ pub(crate) struct SelectBodyHead { pub(crate) hint: Option, pub(crate) top: Option, pub(crate) distinct: bool, + pub(crate) all: bool, pub(crate) distinct_on: Option>, pub(crate) kind: Option, pub(crate) operation_modifiers: Vec, diff --git a/crates/sqlquack-syntax/tests/parser/test_fixes.rs b/crates/sqlquack-syntax/tests/parser/test_fixes.rs index bc59ac276..cc6ad09b0 100644 --- a/crates/sqlquack-syntax/tests/parser/test_fixes.rs +++ b/crates/sqlquack-syntax/tests/parser/test_fixes.rs @@ -1756,3 +1756,57 @@ fn given_a_snowflake_show_when_parsing_then_the_tree_keeps_kind_container_and_op ); } } + +#[test] +fn given_a_documented_snowflake_element_when_parsing_then_the_tree_keeps_it() { + let test_cases = [ + ProjectionTestCase { + description: "SELECT ALL is kept, where it was dropped", + sql: "SELECT ALL * FROM t1", + pointer: "/0/select/all", + expected_value: json!(true), + }, + ProjectionTestCase { + description: "DISTINCT is kept on an aggregate of two arguments", + sql: "SELECT APPROX_COUNT_DISTINCT ( DISTINCT 1 , 1 )", + pointer: "/0/select/expressions/0/function/distinct", + expected_value: json!(true), + }, + ProjectionTestCase { + description: "TIMESTAMP WITHOUT TIME ZONE is TIMESTAMP_NTZ, not bare TIMESTAMP", + sql: "SELECT CAST(NULL AS TIMESTAMP(3) WITHOUT TIME ZONE)", + pointer: "/0/select/expressions/0/cast/to/name", + expected_value: json!("TIMESTAMPNTZ(3)"), + }, + ProjectionTestCase { + description: "a transaction name is the name, quoting kept, NAME apart", + sql: "START TRANSACTION NAME \"v1 Q\"", + pointer: "/0/transaction/this/identifier/name", + expected_value: json!("v1 Q"), + }, + ProjectionTestCase { + description: "a quoted transaction name stays quoted", + sql: "START TRANSACTION NAME \"v1 Q\"", + pointer: "/0/transaction/this/identifier/quoted", + expected_value: json!(true), + }, + ProjectionTestCase { + description: "secondary roles are listed one by one, quoting kept", + sql: "USE SECONDARY ROLES v1 , \"v2\"", + pointer: "/0/use/roles/1/quoted", + expected_value: json!(true), + }, + ]; + + for test_case in test_cases { + assert_eq!( + statements(test_case.sql, Dialect::Snowflake, CallMode::Faithful) + .pointer(test_case.pointer) + .cloned() + .unwrap_or_default(), + test_case.expected_value, + "{}", + test_case.description + ); + } +} From 174f4f1af7cf9c4137d5f350fc368df44b1696f7 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 5 Nov 2026 01:44:22 +0000 Subject: [PATCH 10/10] Polyglot exact check: review the Snowflake DESCRIBE/SHOW refusals and kept fields The gate's exact check found the batch's Snowflake trees and refusals unreviewed. SHOW of PRIMARY, UNIQUE, IMPORTED and EXPORTED KEYS goes back to the shared SHOW, which read them before; SELECT ALL and the DESCRIBE property stop apply to Snowflake only, so other dialects keep their trees. The 12 statements now refused in Snowflake (a DESCRIBE without a kind, MySQL and Hive SHOW and DESCRIBE forms) are reviewed rejections. A new kept_field entry covers a field SQLQuack keeps as written where polyglot drops or infers it: the secondary role list, the container kind of SHOW ... IN a.b, and the TIMESTAMP_NTZ target of TIMESTAMP WITHOUT TIME ZONE. --- .../polyglot-check/expected_differences.toml | 85 +++++++++++++++++++ .../src/cli/_helpers/report.rs | 3 +- .../src/expected/_helpers/nodes.rs | 41 ++++++++- .../src/expected/main/load_expected.rs | 1 + .../src/expected/models.rs | 30 ++++++- .../src/runs/main/run_corpus.rs | 22 ++++- .../src/runs/models.rs | 2 + .../tests/runs/test_exact.rs | 10 +++ .../tests/runs/test_kept_nodes.rs | 68 ++++++++++++++- .../src/parser/_helpers/ddl/scripting.rs | 3 +- .../src/parser/_helpers/statements/entry.rs | 4 +- .../sqlquack-syntax/src/parser/constants.rs | 5 +- docs/syntax.md | 8 ++ 13 files changed, 269 insertions(+), 13 deletions(-) diff --git a/corpora/polyglot-check/expected_differences.toml b/corpora/polyglot-check/expected_differences.toml index cac0aa4c1..1d9e8a947 100644 --- a/corpora/polyglot-check/expected_differences.toml +++ b/corpora/polyglot-check/expected_differences.toml @@ -912,6 +912,69 @@ dialects = ["Snowflake"] sqlquack_error = "Snowflake set operations take no LEFT, RIGHT, FULL, INNER or OUTER" note = "INNER UNION ... BY NAME is BigQuery's; Snowflake rejects a join side before a set operation (recorded errno 1003)" +[[rejection]] +id = "snowflake_describe_bare_name" +sql = "DESCRIBE x" +dialects = ["Snowflake"] +sqlquack_error = "Expected an object kind after DESCRIBE" +note = "Snowflake DESCRIBE names its object kind before the name: it refuses DESC v1 with syntax error 1003" + +[[rejection]] +id = "snowflake_show_profile" +sql = "SHOW PROFILE FOR QUERY 5" +dialects = ["Snowflake"] +sqlquack_error = "Expected an object kind after SHOW" +note = "SHOW PROFILE is MySQL's; PROFILE is no Snowflake SHOW kind" + +[[rejection]] +id = "snowflake_show_groups" +sql = "SHOW GROUPS FOR ROLE" +dialects = ["Snowflake"] +sqlquack_error = "Expected an object kind after SHOW" +note = "GROUPS is no Snowflake SHOW kind; Snowflake shows user groups as SHOW ORGANIZATION USER GROUPS" + +[[rejection]] +id = "snowflake_show_create_table" +sql = "SHOW CREATE TABLE t" +dialects = ["Snowflake"] +sqlquack_error = "Expected an object kind after SHOW" +note = "SHOW CREATE TABLE is MySQL's; Snowflake returns DDL through GET_DDL, and CREATE is no SHOW kind" + +[[rejection]] +id = "snowflake_describe_extended" +sql = "DESCRIBE EXTENDED a.b" +dialects = ["Snowflake"] +sqlquack_error = "Expected an object kind after DESCRIBE" +note = "EXTENDED is Hive's and Spark's; Snowflake DESCRIBE names an object kind, and EXTENDED is none" + +[[rejection]] +id = "snowflake_describe_formatted" +sql = "DESCRIBE FORMATTED a.b" +dialects = ["Snowflake"] +sqlquack_error = "Expected an object kind after DESCRIBE" +note = "FORMATTED is Hive's; Snowflake DESCRIBE names an object kind, and FORMATTED is none" + +[[rejection]] +id = "snowflake_describe_query" +sql = "DESCRIBE SELECT 1" +dialects = ["Snowflake"] +sqlquack_error = "Expected an object kind after DESCRIBE" +note = "Snowflake describes a query's result with DESCRIBE RESULT and a query id, not the query itself" + +[[rejection]] +id = "snowflake_describe_bare_quoted_name" +sql = "DESCRIBE \"history\"" +dialects = ["Snowflake"] +sqlquack_error = "Expected an object kind after DESCRIBE" +note = "a quoted name is still a bare name: Snowflake refuses DESC v1 with syntax error 1003" + +[[rejection]] +id = "snowflake_describe_bare_qualified_name" +sql = "DESCRIBE db.table" +dialects = ["Snowflake"] +sqlquack_error = "Expected an object kind after DESCRIBE" +note = "a qualified name is still a bare name: Snowflake refuses DESC v1 with syntax error 1003" + [[rejection]] id = "snowflake_by_name_on_intersect" sql = "SELECT 1 AS a, 2 AS b INTERSECT DISTINCT BY NAME ON (b, a) SELECT 3 AS b, 4 AS a" @@ -996,6 +1059,28 @@ marker = "+" node = "pos" note = "SELECT + 1 keeps its unary plus, a documented Snowflake operator; polyglot drops it and keeps only the operand; crates/sqlquack-syntax/tests/parser/test_fixes.rs pins the tree" +# Fields SQLQuack keeps as written where polyglot drops or infers them. The check counts an entry +# only when the statement, without whitespace and ignoring case, contains `marker`, the trees differ, +# and both are equal once every `` key is removed from each. + +[[kept_field]] +id = "snowflake_secondary_role_list" +marker = "SECONDARYROLES" +field = "roles" +note = "USE SECONDARY ROLES a, b, c lists its roles one by one with their quoting; polyglot joins them into one unquoted name; crates/sqlquack-syntax/tests/parser/test_fixes.rs pins the tree" + +[[kept_field]] +id = "snowflake_show_unnamed_container" +marker = "SHOW" +field = "scope_kind" +note = "SHOW ... IN a.b names no container kind, so SQLQuack records none; polyglot infers SCHEMA from the two-part name, which makes IN a.b and IN SCHEMA a.b one tree" + +[[kept_field]] +id = "snowflake_timestamp_without_time_zone" +marker = "WITHOUTTIMEZONE" +field = "to" +note = "Snowflake documents TIMESTAMP WITHOUT TIME ZONE as a synonym of TIMESTAMP_NTZ, so SQLQuack's cast target is TIMESTAMPNTZ, as for TIMESTAMP_NTZ; polyglot reads it as bare TIMESTAMP, whose meaning follows TIMESTAMP_TYPE_MAPPING; crates/sqlquack-syntax/tests/parser/test_fixes.rs pins the tree" + # Statements polyglot keeps as one opaque command where SQLQuack parses them into structured nodes. # The check counts an entry only when polyglot's tree is a single command, SQLQuack's tree starts # with `sqlquack_node`, and the statement, without whitespace and ignoring case, contains `marker`. diff --git a/crates/sqlquack-polyglot-check/src/cli/_helpers/report.rs b/crates/sqlquack-polyglot-check/src/cli/_helpers/report.rs index bbe376297..ce6a02368 100644 --- a/crates/sqlquack-polyglot-check/src/cli/_helpers/report.rs +++ b/crates/sqlquack-polyglot-check/src/cli/_helpers/report.rs @@ -25,7 +25,7 @@ pub(crate) fn report(label: &str, summary: &RunSummary, counts_only: bool) -> St } let _ = writeln!( text, - "{label}: {} entries, {} same trees, {} same errors, {} fixed, {} rejected, {} kept quoting, {} kept nodes, {} structured, {} differences, {} without lossless \ + "{label}: {} entries, {} same trees, {} same errors, {} fixed, {} rejected, {} kept quoting, {} kept nodes, {} kept fields, {} structured, {} differences, {} without lossless \ tokens, {} cast type names unlike polyglot's, {} faithful calls kept, {} unlisted, {} \ unexplained, {} located parses unlike faithful; parse time polyglot {:.1} ms, sqlquack \ {:.1} ms", @@ -36,6 +36,7 @@ pub(crate) fn report(label: &str, summary: &RunSummary, counts_only: bool) -> St summary.rejected.values().sum::(), summary.kept_quoting.values().sum::(), summary.kept_nodes.values().sum::(), + summary.kept_fields.values().sum::(), summary.structured.values().sum::(), summary.differences.len(), summary.tokens_lost.len(), diff --git a/crates/sqlquack-polyglot-check/src/expected/_helpers/nodes.rs b/crates/sqlquack-polyglot-check/src/expected/_helpers/nodes.rs index 4757e2736..1f5a67a54 100644 --- a/crates/sqlquack-polyglot-check/src/expected/_helpers/nodes.rs +++ b/crates/sqlquack-polyglot-check/src/expected/_helpers/nodes.rs @@ -1,4 +1,4 @@ -//! Trees that differ only by a wrapper node SQLQuack keeps and polyglot drops. +//! Trees that differ only by a wrapper node or a field SQLQuack keeps and polyglot drops or infers. use serde_json::Value; @@ -52,3 +52,42 @@ fn without_node(value: Value, node: &str) -> Value { other => other, } } + +/// Whether both rendered parses are trees that differ, and are equal once every `field` key goes from each. +pub(crate) fn differ_only_by_kept_field(polyglot: &str, sqlquack: &str, field: &str) -> bool { + let (Some(theirs), Some(ours)) = ( + polyglot.strip_prefix(OK_PREFIX), + sqlquack.strip_prefix(OK_PREFIX), + ) else { + return false; + }; + match ( + serde_json::from_str::(theirs), + serde_json::from_str::(ours), + ) { + (Ok(theirs), Ok(ours)) => { + theirs != ours && without_field(theirs, field) == without_field(ours, field) + } + (Err(_), _) | (_, Err(_)) => false, + } +} + +/// `value` with every object key `field` removed. +fn without_field(value: Value, field: &str) -> Value { + match value { + Value::Object(object) => Value::Object( + object + .into_iter() + .filter(|(key, _)| key != field) + .map(|(key, value)| (key, without_field(value, field))) + .collect(), + ), + Value::Array(items) => Value::Array( + items + .into_iter() + .map(|value| without_field(value, field)) + .collect(), + ), + other => other, + } +} diff --git a/crates/sqlquack-polyglot-check/src/expected/main/load_expected.rs b/crates/sqlquack-polyglot-check/src/expected/main/load_expected.rs index 41c4f695d..f043f61d3 100644 --- a/crates/sqlquack-polyglot-check/src/expected/main/load_expected.rs +++ b/crates/sqlquack-polyglot-check/src/expected/main/load_expected.rs @@ -15,6 +15,7 @@ pub fn load_expected(path: &Path) -> Result { rejections: file.rejections, kept_quotings: file.kept_quotings, kept_nodes: file.kept_nodes, + kept_fields: file.kept_fields, structured: file.structured, ..ExpectedDifferences::default() }; diff --git a/crates/sqlquack-polyglot-check/src/expected/models.rs b/crates/sqlquack-polyglot-check/src/expected/models.rs index 2b3ea5a43..015e7902c 100644 --- a/crates/sqlquack-polyglot-check/src/expected/models.rs +++ b/crates/sqlquack-polyglot-check/src/expected/models.rs @@ -1,6 +1,6 @@ //! Expected differences. -use crate::expected::_helpers::nodes::differ_only_by_kept_node; +use crate::expected::_helpers::nodes::{differ_only_by_kept_field, differ_only_by_kept_node}; use crate::expected::_helpers::quoting::{compact, differ_only_by_kept_quoting}; use crate::expected::constants::{COMMAND_TREE, ERROR_PREFIX, OK_PREFIX}; use std::collections::{BTreeMap, BTreeSet}; @@ -26,6 +26,9 @@ pub struct ExpectedFile { /// One entry per reviewed wrapper node SQLQuack keeps and polyglot drops. #[serde(default, rename = "kept_node")] pub kept_nodes: Vec, + /// One entry per reviewed field SQLQuack keeps as written where polyglot drops or infers it. + #[serde(default, rename = "kept_field")] + pub kept_fields: Vec, /// One entry per reviewed statement polyglot keeps as a command and SQLQuack structures. #[serde(default, rename = "structured")] pub structured: Vec, @@ -71,6 +74,20 @@ pub struct KeptNode { pub note: String, } +/// A statement both parsers parse, where SQLQuack keeps a field as written, such as a list polyglot joins into one name. +#[derive(Clone, Debug, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct KeptField { + /// A lower-case identifier. + pub id: String, + /// Text the statement contains, with whitespace removed and ignoring case, such as `SECONDARYROLES`. + pub marker: String, + /// The field, such as `roles`; without it in either tree the trees may differ in nothing. + pub field: String, + /// Why SQLQuack's field is right; a test in sqlquack-syntax pins its tree. + pub note: String, +} + /// A statement SQLQuack rejects where polyglot parses it or fails differently, recognised by its exact text and dialect. #[derive(Clone, Debug, Deserialize)] #[serde(deny_unknown_fields)] @@ -126,6 +143,8 @@ pub struct ExpectedDifferences { pub kept_quotings: Vec, /// The reviewed wrapper nodes SQLQuack keeps and polyglot drops. pub kept_nodes: Vec, + /// The reviewed fields SQLQuack keeps as written where polyglot drops or infers them. + pub kept_fields: Vec, /// The reviewed statements polyglot keeps as a command and SQLQuack structures. pub structured: Vec, } @@ -188,6 +207,15 @@ impl ExpectedDifferences { }) } + /// The entry explaining where SQLQuack keeps a field as written that polyglot drops or infers. + pub fn kept_field_for(&self, sql: &str, polyglot: &str, sqlquack: &str) -> Option<&KeptField> { + let compact = compact(sql); + self.kept_fields.iter().find(|entry| { + compact.contains(&entry.marker.to_ascii_uppercase()) + && differ_only_by_kept_field(polyglot, sqlquack, &entry.field) + }) + } + /// The entry explaining where SQLQuack structures a statement polyglot keeps as a command. pub fn structured_for( &self, diff --git a/crates/sqlquack-polyglot-check/src/runs/main/run_corpus.rs b/crates/sqlquack-polyglot-check/src/runs/main/run_corpus.rs index cd4149a5c..1ef3e6514 100644 --- a/crates/sqlquack-polyglot-check/src/runs/main/run_corpus.rs +++ b/crates/sqlquack-polyglot-check/src/runs/main/run_corpus.rs @@ -60,8 +60,16 @@ pub fn run_corpus(entries: &[CorpusEntry], expected: &ExpectedDifferences) -> Ru let rejection = expected.rejection_for(&entry.sql, &dialect, &sqlquack); let kept_quoting = expected.kept_quoting_for(&entry.sql, &polyglot, &sqlquack); let kept_node = expected.kept_node_for(&entry.sql, &polyglot, &sqlquack); + let kept_field = expected.kept_field_for(&entry.sql, &polyglot, &sqlquack); let structured = expected.structured_for(&entry.sql, &polyglot, &sqlquack); - match (fix, rejection, kept_quoting, kept_node, structured) { + match ( + fix, + rejection, + kept_quoting, + kept_node, + kept_field, + structured, + ) { (Some(fix), ..) => *summary.fixed.entry(fix.id.clone()).or_default() += 1, (None, Some(rejection), ..) => { *summary.rejected.entry(rejection.id.clone()).or_default() += 1; @@ -72,13 +80,19 @@ pub fn run_corpus(entries: &[CorpusEntry], expected: &ExpectedDifferences) -> Ru .entry(kept_quoting.id.clone()) .or_default() += 1; } - (None, None, None, Some(kept_node), _) => { + (None, None, None, Some(kept_node), ..) => { *summary.kept_nodes.entry(kept_node.id.clone()).or_default() += 1; } - (None, None, None, None, Some(structured)) => { + (None, None, None, None, Some(kept_field), _) => { + *summary + .kept_fields + .entry(kept_field.id.clone()) + .or_default() += 1; + } + (None, None, None, None, None, Some(structured)) => { *summary.structured.entry(structured.id.clone()).or_default() += 1; } - (None, None, None, None, None) => summary.differences.push(Finding { + (None, None, None, None, None, None) => summary.differences.push(Finding { from, detail: format!("polyglot: {polyglot}\nsqlquack: {sqlquack}"), }), diff --git a/crates/sqlquack-polyglot-check/src/runs/models.rs b/crates/sqlquack-polyglot-check/src/runs/models.rs index 1a8d22b57..d0c578274 100644 --- a/crates/sqlquack-polyglot-check/src/runs/models.rs +++ b/crates/sqlquack-polyglot-check/src/runs/models.rs @@ -31,6 +31,8 @@ pub struct RunSummary { pub kept_quoting: BTreeMap, /// Entries whose trees differ only by wrapper nodes polyglot drops, as a reviewed entry explains. pub kept_nodes: BTreeMap, + /// Entries whose trees differ only by a field kept as written, as a reviewed entry explains. + pub kept_fields: BTreeMap, /// Entries polyglot keeps as a command and SQLQuack structures, as a reviewed entry explains. pub structured: BTreeMap, /// Entries whose results differ. diff --git a/crates/sqlquack-polyglot-check/tests/runs/test_exact.rs b/crates/sqlquack-polyglot-check/tests/runs/test_exact.rs index 1f726a452..42baa83af 100644 --- a/crates/sqlquack-polyglot-check/tests/runs/test_exact.rs +++ b/crates/sqlquack-polyglot-check/tests/runs/test_exact.rs @@ -44,6 +44,10 @@ fn given_committed_corpora_when_parsed_by_both_parsers_then_results_are_identica ("snowflake_reserved_distinct_alias".to_owned(), 1), ("snowflake_reserved_is_alias".to_owned(), 1), ("snowflake_set_operation_join_side".to_owned(), 1), + ("snowflake_describe_bare_name".to_owned(), 1), + ("snowflake_show_profile".to_owned(), 1), + ("snowflake_show_groups".to_owned(), 1), + ("snowflake_show_create_table".to_owned(), 1), ]), differences: Vec::new(), tokens_lost: Vec::new(), @@ -72,6 +76,12 @@ fn given_committed_corpora_when_parsed_by_both_parsers_then_results_are_identica ("snowflake_frame_bare_preceding".to_owned(), 1), ("snowflake_frame_value_without_direction".to_owned(), 1), ("snowflake_frame_value_without_direction_end".to_owned(), 1), + ("snowflake_describe_bare_name".to_owned(), 1), + ("snowflake_describe_bare_qualified_name".to_owned(), 1), + ("snowflake_describe_extended".to_owned(), 1), + ("snowflake_describe_formatted".to_owned(), 1), + ("snowflake_describe_query".to_owned(), 1), + ("snowflake_describe_bare_quoted_name".to_owned(), 1), ]), differences: Vec::new(), tokens_lost: Vec::new(), diff --git a/crates/sqlquack-polyglot-check/tests/runs/test_kept_nodes.rs b/crates/sqlquack-polyglot-check/tests/runs/test_kept_nodes.rs index a05dff54f..b6f92dbb4 100644 --- a/crates/sqlquack-polyglot-check/tests/runs/test_kept_nodes.rs +++ b/crates/sqlquack-polyglot-check/tests/runs/test_kept_nodes.rs @@ -1,7 +1,7 @@ -//! Reviewed kept nodes cover only wrappers polyglot drops, in statements with the marker. +//! Reviewed kept nodes and fields cover only what polyglot drops or infers, in statements with the marker. use sqlquack_polyglot_check::corpora::models::CorpusEntry; -use sqlquack_polyglot_check::expected::models::{ExpectedDifferences, KeptNode}; +use sqlquack_polyglot_check::expected::models::{ExpectedDifferences, KeptField, KeptNode}; use sqlquack_syntax::Dialect; use crate::helpers::summary_of; @@ -137,3 +137,67 @@ fn given_trees_with_a_marker_when_matched_then_only_dropped_wrappers_are_allowed ); } } + +#[test] +fn given_trees_with_a_marker_when_matched_then_only_the_kept_field_may_differ() { + let tree = |scope: &str, name: &str| { + format!(r#"ok:[{{"show":{{"this":"TABLES","scope_kind":{scope},"scope":"{name}"}}}}]"#) + }; + let test_cases = [ + QuotingTreesTestCase { + description: "a field polyglot infers is the only difference", + sql: "SHOW TABLES IN a.b", + polyglot: tree(r#""SCHEMA""#, "a.b"), + sqlquack: tree("null", "a.b"), + expected_match: true, + }, + QuotingTreesTestCase { + description: "a field polyglot lacks is the only difference", + sql: "SHOW TABLES IN a.b", + polyglot: r#"ok:[{"show":{"this":"TABLES","scope":"a.b"}}]"#.to_owned(), + sqlquack: tree("null", "a.b"), + expected_match: true, + }, + QuotingTreesTestCase { + description: "another difference beside the field is not matched", + sql: "SHOW TABLES IN a.b", + polyglot: tree(r#""SCHEMA""#, "a.b"), + sqlquack: tree("null", "a.c"), + expected_match: false, + }, + QuotingTreesTestCase { + description: "a statement without the marker is not matched", + sql: "DESC TABLE a.b", + polyglot: tree(r#""SCHEMA""#, "a.b"), + sqlquack: tree("null", "a.b"), + expected_match: false, + }, + QuotingTreesTestCase { + description: "a polyglot error is not matched", + sql: "SHOW TABLES IN a.b", + polyglot: "err:Parse".to_owned(), + sqlquack: tree("null", "a.b"), + expected_match: false, + }, + ]; + let expected = ExpectedDifferences { + kept_fields: vec![KeptField { + id: "show_container".to_owned(), + marker: "SHOW".to_owned(), + field: "scope_kind".to_owned(), + note: "a test entry".to_owned(), + }], + ..ExpectedDifferences::default() + }; + + for test_case in test_cases { + assert_eq!( + expected + .kept_field_for(test_case.sql, &test_case.polyglot, &test_case.sqlquack) + .is_some(), + test_case.expected_match, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/scripting.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/scripting.rs index 85de054a8..641a04ca3 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/scripting.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/scripting.rs @@ -140,7 +140,8 @@ impl Parser { if (self.check(TokenType::Var) || self.check(TokenType::Type) || self.check_keyword()) - && self.peek_nth(1).map(|token| token.token_type) == Some(TokenType::Eq) + && (!self.is_snowflake() + || self.peek_nth(1).map(|token| token.token_type) == Some(TokenType::Eq)) { let name = self.advance_text()?.to_lowercase(); if self.match_token(TokenType::Eq) { diff --git a/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs b/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs index 34b4b7ca7..a19e2dd96 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs @@ -1180,8 +1180,8 @@ impl Parser { let top = self.parse_select_top()?; let is_distinct_token = self.match_token(TokenType::Distinct); - let all = - !is_distinct_token && self.check(TokenType::All) && !self.check_next(TokenType::Dot); + let all = !is_distinct_token && self.check(TokenType::All); + let all = all && self.is_snowflake() && !self.check_next(TokenType::Dot); let (distinct, distinct_on) = if is_distinct_token { if self.match_token(TokenType::On) { self.expect_token(TokenType::LParen)?; diff --git a/crates/sqlquack-syntax/src/parser/constants.rs b/crates/sqlquack-syntax/src/parser/constants.rs index 82ae3bfae..bf04526c1 100644 --- a/crates/sqlquack-syntax/src/parser/constants.rs +++ b/crates/sqlquack-syntax/src/parser/constants.rs @@ -620,7 +620,6 @@ pub(crate) const SNOWFLAKE_SHOW_KINDS: &[&[&str]] = &[ &["PENDING", "SPECIFICATIONS"], &["POSTGRES", "INSTANCES"], &["PRICING", "PLANS"], - &["PRIMARY", "KEYS"], &["PRIVACY", "POLICIES"], &["PROJECTION", "POLICIES"], &["RELEASE", "CHANNELS"], @@ -711,6 +710,10 @@ pub(crate) const SNOWFLAKE_SHOW_KINDS: &[&[&str]] = &[ pub(crate) const SHARED_SHOW_KINDS: &[&[&str]] = &[ &["CALLER"], &["CORTEX", "SEARCH", "SERVICES"], + &["EXPORTED", "KEYS"], + &["IMPORTED", "KEYS"], + &["PRIMARY", "KEYS"], + &["UNIQUE", "KEYS"], &["AVAILABLE", "LISTINGS"], &["EVENT", "ROUTING", "TABLE", "ON"], &["FEATURE", "POLICIES"], diff --git a/docs/syntax.md b/docs/syntax.md index abb308f68..f57e7256d 100644 --- a/docs/syntax.md +++ b/docs/syntax.md @@ -207,6 +207,14 @@ not different, only when the marker appears and the trees are equal once every `{ "": { "this": … } }` in SQLQuack's tree is replaced by its operand. The only entry: unary plus (`pos`), which Snowflake documents and polyglot drops. +A statement both parsers parse, where SQLQuack keeps a field as written that polyglot drops or +infers, is a `[[kept_field]]` entry: an id, a marker, the field and a reason. The check counts it as +a kept field, not different, only when the marker appears, the trees differ, and they are equal once +every `` key is removed from both. The entries: the role list of `USE SECONDARY ROLES`, which +polyglot joins into one name; the container kind of `SHOW ... IN a.b`, which polyglot infers as +SCHEMA; and the cast target of `TIMESTAMP WITHOUT TIME ZONE`, which Snowflake documents as +TIMESTAMP_NTZ and polyglot reads as bare TIMESTAMP. + The faithful parse also keeps a quoted variant path key that holds `.`, `[`, `]`, `"` or `:` in brackets (`v:a."x.y"` is the path `a["x.y"]`), where polyglot joins it with dots (`a.x.y`), which reads as two keys. The check compares variant paths written flat, so this bracketing is the only