From 60262993b712fbf452c6d007b06a8ac6c2a8be9f Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 16:13:30 +0000 Subject: [PATCH 01/10] Reject an empty argument between commas in function calls Snowflake rejects calls such as TO_DECIMAL(1, ) and F(a, , b) with a syntax error; the parser skipped the empty argument and accepted them. --- .../src/parser/_helpers/functions/generic.rs | 4 +- .../parser/_helpers/statements/sessions.rs | 57 +++++++++---------- 2 files changed, 30 insertions(+), 31 deletions(-) diff --git a/crates/sqlquack-syntax/src/parser/_helpers/functions/generic.rs b/crates/sqlquack-syntax/src/parser/_helpers/functions/generic.rs index 0c5c38208..1df443a4f 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/functions/generic.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/functions/generic.rs @@ -505,8 +505,8 @@ impl Parser { if !self.match_token(TokenType::Comma) { break; } - while self.check(TokenType::Comma) { - self.skip(); + if self.check(TokenType::Comma) { + return Err(self.parse_error("Expected a function argument between commas")); } } diff --git a/crates/sqlquack-syntax/src/parser/_helpers/statements/sessions.rs b/crates/sqlquack-syntax/src/parser/_helpers/statements/sessions.rs index 48c91488d..b8f7ab6c9 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/statements/sessions.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/sessions.rs @@ -1682,35 +1682,34 @@ impl Parser { self.parse_assignment()? }; - if let Some(expr) = expr { - { - if self.match_token(TokenType::As) { - let alias_token = self.advance()?; - let alias_name = if alias_token.token_type == TokenType::QuotedIdentifier { - let raw = alias_token.text.to_string(); - let mut ident = Identifier::new(raw); - ident.quoted = true; - ident - } else { - Identifier::new(alias_token.text.clone()).with_span(alias_token.span) - }; - args.push(Expression::Alias(Box::new( - crate::expressions::operators::models::Alias { - span: SourceSpan::default(), - this: expr, - alias: alias_name, - column_aliases: Vec::new(), - alias_explicit_as: false, - alias_keyword: None, - pre_alias_comments: Vec::new(), - trailing_comments: Vec::new(), - inferred_type: None, - }, - ))); - } else { - args.push(expr); - } - } + let Some(expr) = expr else { + return Err(self.parse_error("Expected a function argument")); + }; + if self.match_token(TokenType::As) { + let alias_token = self.advance()?; + let alias_name = if alias_token.token_type == TokenType::QuotedIdentifier { + let raw = alias_token.text.to_string(); + let mut ident = Identifier::new(raw); + ident.quoted = true; + ident + } else { + Identifier::new(alias_token.text.clone()).with_span(alias_token.span) + }; + args.push(Expression::Alias(Box::new( + crate::expressions::operators::models::Alias { + span: SourceSpan::default(), + this: expr, + alias: alias_name, + column_aliases: Vec::new(), + alias_explicit_as: false, + alias_keyword: None, + pre_alias_comments: Vec::new(), + trailing_comments: Vec::new(), + inferred_type: None, + }, + ))); + } else { + args.push(expr); } if !self.match_token(TokenType::Comma) { From aec7f49727b700437e5c034224f51ffabf9eabb0 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 16:32:46 +0000 Subject: [PATCH 02/10] Reject empty and trailing-comma type parameter lists Snowflake rejects TIME(), VARBINARY(), NUMBER(), NUMBER(9,) and PERIOD() with a syntax error. CHAR(), VARCHAR() and STRING() stay accepted, as does RETURNS TABLE (). --- .../src/parser/_helpers/types/casts.rs | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs b/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs index 787b752b0..253e9bdf9 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs @@ -737,14 +737,9 @@ impl Parser { fn parse_time_data_type(&mut self) -> Result { let precision = if self.match_token(TokenType::LParen) { - if self.check(TokenType::RParen) { - self.skip(); - None - } else { - let p = self.expect_number()? as u32; - self.expect_token(TokenType::RParen)?; - Some(p) - } + let p = self.expect_number()? as u32; + self.expect_token(TokenType::RParen)?; + Some(p) } else { None }; @@ -907,10 +902,7 @@ impl Parser { fn parse_varbinary_data_type(&mut self) -> Result { if self.match_token(TokenType::LParen) { - if self.check(TokenType::RParen) { - self.skip(); - Ok(DataType::VarBinary { length: None }) - } else if self.check_identifier("MAX") { + if self.check_identifier("MAX") { self.skip(); self.expect_token(TokenType::RParen)?; Ok(DataType::Custom { @@ -1126,6 +1118,9 @@ impl Parser { let mut after_comma = true; loop { if self.check(TokenType::RParen) { + if after_comma && !(args.is_empty() && name == "TABLE") { + return Err(self.parse_error("Expected a type parameter")); + } break; } let token = self.advance()?; From 54dace57a5c608faf92682f3bd7e341d1e059830 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 16:32:46 +0000 Subject: [PATCH 03/10] Reject unquoted reserved words as object and type names Snowflake rejects DROP TABLE SELECT, DROP INDEX v1.SELECT, UNDROP ... RENAME TO SELECT, TRUNCATE SELECT, USE SELECT, DECLARE SELECT RESULTSET and CAST(x AS SELECT); quoted names such as "SELECT" stay accepted. The check applies to the names these statements parse, not to every table reference, so FROM TABLE(...) and RETURNS TABLE (...) are unaffected. Adds parser tests for the rejected statements and their accepted neighbours, including the empty-argument and type-parameter fixes. --- .../src/parser/_helpers/ddl/alter.rs | 4 +- .../src/parser/_helpers/ddl/constraints.rs | 6 +- .../parser/_helpers/expressions/operators.rs | 8 +- .../src/parser/_helpers/queries/tables.rs | 28 +++++ .../src/parser/_helpers/statements/entry.rs | 4 +- .../parser/_helpers/statements/scripting.rs | 2 +- .../src/parser/_helpers/types/casts.rs | 11 ++ .../src/parser/tests/test_snowflake.rs | 103 ++++++++++++++++++ 8 files changed, 155 insertions(+), 11 deletions(-) diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/alter.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/alter.rs index d4656cd67..623be43ee 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/alter.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/alter.rs @@ -1247,7 +1247,7 @@ impl Parser { let if_exists = self.match_keywords(&[TokenType::If, TokenType::Exists]); let has_only = self.match_token(TokenType::Only); - let mut table = self.parse_table_ref()?; + let mut table = self.parse_object_name_ref()?; if has_only { table.only = true; } @@ -1452,9 +1452,11 @@ impl Parser { }; let mut ident = self.expect_identifier_or_keyword_with_quoted()?; + self.reject_reserved_name_part(&ident)?; if self.match_token(TokenType::Dot) { let second_part = self.expect_identifier_or_keyword_with_quoted()?; + self.reject_reserved_name_part(&second_part)?; ident.name = format!("{}.{}", ident.name, second_part.name); } diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/constraints.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/constraints.rs index 746e66418..ea43a31ff 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/constraints.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/constraints.rs @@ -1478,7 +1478,7 @@ impl Parser { let mut names: Vec = Vec::new(); loop { - names.push(self.parse_table_ref()?); + names.push(self.parse_object_name_ref()?); if !self.match_token(TokenType::Comma) { break; } @@ -1520,7 +1520,7 @@ impl Parser { self.expect_token(TokenType::View)?; let if_exists = self.match_keywords(&[TokenType::If, TokenType::Exists]); - let name = self.parse_table_ref()?; + let name = self.parse_object_name_ref()?; Ok(Expression::DropView(Box::new(DropView { name, @@ -1537,7 +1537,7 @@ impl Parser { let if_exists = self.match_keywords(&[TokenType::If, TokenType::Exists]); - let name = self.parse_table_ref()?; + let name = self.parse_object_name_ref()?; let table = if self.match_token(TokenType::On) { Some(self.parse_table_ref()?) diff --git a/crates/sqlquack-syntax/src/parser/_helpers/expressions/operators.rs b/crates/sqlquack-syntax/src/parser/_helpers/expressions/operators.rs index 9a3142eff..e0ccbefc9 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/expressions/operators.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/expressions/operators.rs @@ -225,7 +225,7 @@ impl Parser { self.expect_token(TokenType::Function)?; let if_exists = self.match_keywords(&[TokenType::If, TokenType::Exists]); - let name = self.parse_table_ref()?; + let name = self.parse_object_name_ref()?; let parameters = if self.match_token(TokenType::LParen) { let mut types: Vec = Vec::new(); @@ -381,7 +381,7 @@ impl Parser { self.expect_token(TokenType::Procedure)?; let if_exists = self.match_keywords(&[TokenType::If, TokenType::Exists]); - let name = self.parse_table_ref()?; + let name = self.parse_object_name_ref()?; let parameters = if self.match_token(TokenType::LParen) { let mut types: Vec = Vec::new(); @@ -614,7 +614,7 @@ impl Parser { self.expect_token(TokenType::Sequence)?; let if_exists = self.match_keywords(&[TokenType::If, TokenType::Exists]); - let name = self.parse_table_ref()?; + let name = self.parse_object_name_ref()?; let cascade = self.match_token(TokenType::Cascade); if !cascade { @@ -1235,7 +1235,7 @@ impl Parser { self.expect_token(TokenType::Type)?; let if_exists = self.match_keywords(&[TokenType::If, TokenType::Exists]); - let name = self.parse_table_ref()?; + let name = self.parse_object_name_ref()?; let cascade = self.match_token(TokenType::Cascade); if !cascade { diff --git a/crates/sqlquack-syntax/src/parser/_helpers/queries/tables.rs b/crates/sqlquack-syntax/src/parser/_helpers/queries/tables.rs index b89156249..07d99321c 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/queries/tables.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/queries/tables.rs @@ -1379,6 +1379,34 @@ impl Parser { prev_token.span.end == curr_token.span.start } + /// An object name after DROP, UNDROP or TRUNCATE; Snowflake rejects unquoted reserved words. + pub(crate) fn parse_object_name_ref(&mut self) -> Result { + let name = self.parse_table_ref()?; + for part in [&name.catalog, &name.schema].into_iter().flatten() { + self.reject_reserved_name_part(part)?; + } + self.reject_reserved_name_part(&name.name)?; + Ok(name) + } + + /// Fails on an unquoted Snowflake reserved word used as a name part in a Snowflake parse. + pub(crate) fn reject_reserved_name_part(&self, part: &Identifier) -> Result<()> { + if matches!( + self.config.dialect, + Some(crate::tokens::models::DialectType::Snowflake) + ) && !part.quoted + && crate::parser::constants::SNOWFLAKE_RESERVED_KEYWORDS + .iter() + .any(|keyword| part.name.eq_ignore_ascii_case(keyword)) + { + return Err(self.parse_error(format!( + "Reserved word {} cannot be an unquoted object name", + part.name.to_ascii_uppercase() + ))); + } + Ok(()) + } + /// Parse a table reference (schema.table format) pub(crate) fn parse_table_ref(&mut self) -> Result { let table_ref_leading_comments = self.current_leading_comments().to_vec(); diff --git a/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs b/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs index f4015c835..ee1ce37f2 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs @@ -729,9 +729,9 @@ impl Parser { )); }; let if_exists = self.match_keywords(&[TokenType::If, TokenType::Exists]); - let name = self.parse_table_ref()?; + let name = self.parse_object_name_ref()?; let rename_to = if self.match_keywords(&[TokenType::Rename, TokenType::To]) { - Some(self.parse_table_ref()?) + Some(self.parse_object_name_ref()?) } else { None }; diff --git a/crates/sqlquack-syntax/src/parser/_helpers/statements/scripting.rs b/crates/sqlquack-syntax/src/parser/_helpers/statements/scripting.rs index 178325839..50af09994 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/statements/scripting.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/scripting.rs @@ -164,7 +164,7 @@ impl Parser { /// One DECLARE member, or with `is_let` the documented LET forms. fn parse_scripting_declaration(&mut self, is_let: bool) -> Result { let start = self.current; - let name = self.expect_identifier_or_keyword_with_quoted()?; + let name = self.expect_scripting_name()?; if self.match_text_seq(&["CURSOR", "FOR"]) { let source = match self.peek_nth(1) { Some(end) diff --git a/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs b/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs index 253e9bdf9..1076da97a 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs @@ -1106,6 +1106,17 @@ impl Parser { } fn parse_named_data_type(&mut self, name: String, raw_name: String) -> Result { + if matches!( + self.config.dialect, + Some(crate::tokens::models::DialectType::Snowflake) + ) && self.previous().token_type != TokenType::QuotedIdentifier + && name != "TABLE" + && crate::parser::constants::SNOWFLAKE_RESERVED_KEYWORDS.contains(&name.as_str()) + { + return Err(self.parse_error(format!( + "Reserved word {name} cannot be an unquoted type name" + ))); + } let is_known = convert_name_is_known_custom(&name); let custom_name = if is_known { name.clone() diff --git a/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs b/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs index 53700d439..abcb3ac21 100644 --- a/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs +++ b/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs @@ -3,6 +3,7 @@ use serde_json::json; use crate::parser::tests::helpers::first_token_text; use crate::parser::tests::helpers::kind_at; use crate::parser::tests::helpers::parse_error; +use crate::parser::tests::helpers::parsed; use crate::parser::tests::helpers::value_at; use crate::parser::tests::test_types::DialectTokenTestCase; use crate::parser::tests::test_types::ParseErrorTestCase; @@ -462,6 +463,84 @@ fn given_unsupported_snowflake_sql_when_parsing_then_parsing_fails() { sql: "SELECT ARRAY_CONSTRUCT(1, )", expected_error: "Parse error", }, + ParseErrorTestCase { + description: "an empty argument between commas is rejected", + dialect: Some(DialectType::Snowflake), + sql: "SELECT CONCAT('a', , 'b')", + expected_error: "Parse error", + }, + ParseErrorTestCase { + description: "a trailing comma in TO_DECIMAL is rejected", + dialect: Some(DialectType::Snowflake), + sql: "SELECT TO_DECIMAL(1, )", + expected_error: "Parse error", + }, + ParseErrorTestCase { + description: "an empty TIME precision is rejected", + dialect: Some(DialectType::Snowflake), + sql: "SELECT CAST(NULL AS TIME())", + expected_error: "Parse error", + }, + ParseErrorTestCase { + description: "an empty VARBINARY length is rejected", + dialect: Some(DialectType::Snowflake), + sql: "SELECT CAST(NULL AS VARBINARY())", + expected_error: "Parse error", + }, + ParseErrorTestCase { + description: "an empty NUMBER parameter list is rejected", + dialect: Some(DialectType::Snowflake), + sql: "SELECT CAST(NULL AS NUMBER())", + expected_error: "Parse error", + }, + ParseErrorTestCase { + description: "a trailing comma in NUMBER parameters is rejected", + dialect: Some(DialectType::Snowflake), + sql: "SELECT CAST(NULL AS NUMBER(9, ))", + expected_error: "Parse error", + }, + ParseErrorTestCase { + description: "a reserved word as a type name is rejected", + dialect: Some(DialectType::Snowflake), + sql: "SELECT CAST(NULL AS SELECT)", + expected_error: "Parse error", + }, + ParseErrorTestCase { + description: "a reserved word as a dropped table is rejected", + dialect: Some(DialectType::Snowflake), + sql: "DROP TABLE SELECT", + expected_error: "Parse error", + }, + ParseErrorTestCase { + description: "a reserved word as a qualified index part is rejected", + dialect: Some(DialectType::Snowflake), + sql: "DROP INDEX v1.SELECT", + expected_error: "Parse error", + }, + ParseErrorTestCase { + description: "a reserved word as an UNDROP rename target is rejected", + dialect: Some(DialectType::Snowflake), + sql: "UNDROP SNAPSHOT v1 RENAME TO SELECT", + expected_error: "Parse error", + }, + ParseErrorTestCase { + description: "a reserved word as a truncated table is rejected", + dialect: Some(DialectType::Snowflake), + sql: "TRUNCATE SELECT", + expected_error: "Parse error", + }, + ParseErrorTestCase { + description: "a reserved word as a USE target is rejected", + dialect: Some(DialectType::Snowflake), + sql: "USE SELECT.v1", + expected_error: "Parse error", + }, + ParseErrorTestCase { + description: "a reserved word as a declared name is rejected", + dialect: Some(DialectType::Snowflake), + sql: "DECLARE SELECT RESULTSET; BEGIN RETURN 1; END;", + expected_error: "Parse error", + }, ParseErrorTestCase { description: "an unquoted name% column is a known limitation", dialect: Some(DialectType::Snowflake), @@ -480,6 +559,30 @@ fn given_unsupported_snowflake_sql_when_parsing_then_parsing_fails() { } } +#[test] +fn given_neighbours_of_rejected_snowflake_sql_when_parsing_then_they_parse() { + let accepted = [ + "SELECT CAST(NULL AS VARCHAR())", + "SELECT CAST(NULL AS CHAR())", + "SELECT CAST(NULL AS TIME(9))", + "SELECT CAST(NULL AS NUMBER(9, 2))", + "SELECT CAST(NULL AS VARBINARY(8))", + "DROP TABLE \"SELECT\"", + "USE \"SELECT\".v1", + "SELECT * FROM TABLE(FLATTEN(INPUT => ARRAY_CONSTRUCT(1)))", + "CREATE PROCEDURE p1() RETURNS TABLE () LANGUAGE SQL AS $$ BEGIN RETURN 1; END $$", + "DECLARE \"SELECT\" RESULTSET; BEGIN RETURN 1; END;", + ]; + + for sql in accepted { + assert!( + parsed(Some(DialectType::Snowflake), sql).is_ok(), + "{sql}: {}", + parse_error(Some(DialectType::Snowflake), sql) + ); + } +} + #[test] fn given_snowflake_string_escapes_when_tokenizing_then_they_decode_as_documented() { let test_cases = [ From 88e0e11736c390d3d1903af38ce7f09061f8b6d3 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 16:50:19 +0000 Subject: [PATCH 04/10] Parse Snowflake anonymous procedures in WITH ... CALL WITH AS PROCEDURE (...) RETURNS ... LANGUAGE ... [options] [AS body] followed by CALL (...) [INTO :var] now parses to a structured Call that keeps its WITH clause, instead of failing. The procedure is a CreateProcedure CTE; RETURNS TABLE columns, handler options and the body are kept, and a LANGUAGE SQL body is parsed as Snowflake Scripting so a broken body fails. Snowflake CTEs also need AS and an unreserved name. All 280 recorded WITH ... AS PROCEDURE cases now agree with Snowflake's phase. --- .../src/expressions/operators/models.rs | 6 + .../src/expressions/schema_objects/models.rs | 5 + .../parser/_helpers/expressions/operators.rs | 1 + .../src/parser/_helpers/statements/entry.rs | 43 +++- .../src/parser/_helpers/statements/mod.rs | 1 + .../parser/_helpers/statements/procedures.rs | 225 ++++++++++++++++++ .../src/parser/tests/test_snowflake.rs | 86 +++++++ 7 files changed, 366 insertions(+), 1 deletion(-) create mode 100644 crates/sqlquack-syntax/src/parser/_helpers/statements/procedures.rs diff --git a/crates/sqlquack-syntax/src/expressions/operators/models.rs b/crates/sqlquack-syntax/src/expressions/operators/models.rs index 226ea41b6..73df99eac 100644 --- a/crates/sqlquack-syntax/src/expressions/operators/models.rs +++ b/crates/sqlquack-syntax/src/expressions/operators/models.rs @@ -464,6 +464,12 @@ pub struct Call { pub arguments: Vec, pub asynchronous: bool, pub hints: Vec, + /// Snowflake `WITH ... CALL`: the CTEs and anonymous procedures the call can name. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub with: Option, + /// Snowflake scripting `INTO :variable` target of the call's result. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub into: Option, } /// Regex search options shared by predicates, extraction, replacement, and position calls. diff --git a/crates/sqlquack-syntax/src/expressions/schema_objects/models.rs b/crates/sqlquack-syntax/src/expressions/schema_objects/models.rs index 304d6e053..db967931e 100644 --- a/crates/sqlquack-syntax/src/expressions/schema_objects/models.rs +++ b/crates/sqlquack-syntax/src/expressions/schema_objects/models.rs @@ -14,6 +14,7 @@ use crate::expressions::tables::models::RefreshTriggerProperty; use crate::expressions::tables::models::UniqueKeyProperty; use crate::expressions::tree::models::Expression; use crate::expressions::types::models::DataType; +use crate::expressions::types::models::StructField; /// View column definition with optional COMMENT and OPTIONS (BigQuery) #[derive(sqlquack_syntax_derive::AstNode, Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -685,6 +686,9 @@ pub struct CreateProcedure { /// Whether the short form PROC was used (instead of PROCEDURE) #[serde(default, skip_serializing_if = "std::ops::Not::not")] pub use_proc_keyword: bool, + /// Snowflake `RETURNS TABLE (...)` columns, possibly none; `return_type` is then absent. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub returns_table: Option>, } impl CreateProcedure { @@ -703,6 +707,7 @@ impl CreateProcedure { with_options: Vec::new(), has_parens: true, use_proc_keyword: false, + returns_table: None, } } } diff --git a/crates/sqlquack-syntax/src/parser/_helpers/expressions/operators.rs b/crates/sqlquack-syntax/src/parser/_helpers/expressions/operators.rs index e0ccbefc9..6ed4642a6 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/expressions/operators.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/expressions/operators.rs @@ -373,6 +373,7 @@ impl Parser { with_options, has_parens, use_proc_keyword, + returns_table: None, }))) } diff --git a/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs b/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs index ee1ce37f2..473127beb 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs @@ -1821,6 +1821,7 @@ impl Parser { let recursive = self.match_token(TokenType::Recursive); let mut ctes: Vec = Vec::new(); + let mut has_procedure = false; loop { let cte_start = self.current; @@ -1844,11 +1845,39 @@ impl Parser { Vec::new() }; - let cte_comments = if self.match_token(TokenType::As) { + let has_as = self.match_token(TokenType::As); + let cte_comments = if has_as { self.previous_trailing_comments().to_vec() } else { Vec::new() }; + if matches!( + self.config.dialect, + Some(crate::tokens::models::DialectType::Snowflake) + ) { + self.reject_reserved_name_part(&name)?; + if !has_as { + return Err(self.parse_error("Expected AS before the CTE body")); + } + } + if has_as && columns.is_empty() && self.at_anonymous_procedure() { + let procedure = self.parse_anonymous_procedure(&name)?; + has_procedure = true; + ctes.push(Cte { + alias: name, + this: procedure, + columns, + materialized: None, + key_expressions, + alias_first: true, + comments: cte_comments, + span: self.source_range(cte_start), + }); + if self.match_token(TokenType::Comma) { + continue; + } + break; + } let materialized = if self.match_token(TokenType::Materialized) { Some(true) @@ -1901,6 +1930,18 @@ impl Parser { let search = self.parse_recursive_with_search()?; + if has_procedure { + if !self.check_identifier("CALL") { + return Err(self.parse_error("Expected CALL after an anonymous procedure")); + } + return self.parse_call_with(With { + ctes, + recursive, + leading_comments, + search, + }); + } + let mut main_query = self.parse_statement()?; loop { diff --git a/crates/sqlquack-syntax/src/parser/_helpers/statements/mod.rs b/crates/sqlquack-syntax/src/parser/_helpers/statements/mod.rs index e6decf2cd..311dda77c 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/statements/mod.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/mod.rs @@ -3,5 +3,6 @@ pub(crate) mod analyze; pub(crate) mod dml; pub(crate) mod entry; +pub(crate) mod procedures; pub(crate) mod scripting; pub(crate) mod sessions; diff --git a/crates/sqlquack-syntax/src/parser/_helpers/statements/procedures.rs b/crates/sqlquack-syntax/src/parser/_helpers/statements/procedures.rs new file mode 100644 index 000000000..e5fc535d3 --- /dev/null +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/procedures.rs @@ -0,0 +1,225 @@ +//! Snowflake anonymous procedures, `WITH AS PROCEDURE ... CALL (...)`, as documented. + +use crate::expressions::names::models::Identifier; +use crate::expressions::names::models::TableRef; +use crate::expressions::operators::models::Call; +use crate::expressions::queries::models::With; +use crate::expressions::schema_objects::models::CreateProcedure; +use crate::expressions::schema_objects::models::FunctionBody; +use crate::expressions::schema_objects::models::FunctionParameter; +use crate::expressions::tree::models::Expression; +use crate::expressions::types::models::StructField; +use crate::parser::models::Dialect; +use crate::parser::models::Parser; +use crate::tokens::models::CallMode; +use crate::tokens::models::DialectType; +use crate::tokens::models::TokenType; +use crate::tokens::types::Result; + +/// Handler languages Snowflake accepts for an anonymous procedure. +const LANGUAGES: [&str; 5] = ["SQL", "JAVASCRIPT", "PYTHON", "JAVA", "SCALA"]; + +impl Parser { + /// Whether the CTE body after `AS` is a Snowflake anonymous procedure. + pub(crate) fn at_anonymous_procedure(&self) -> bool { + matches!(self.config.dialect, Some(DialectType::Snowflake)) + && self.check(TokenType::Procedure) + } + + /// The procedure after ` AS`: parameters, RETURNS, LANGUAGE, handler options and body. + pub(crate) fn parse_anonymous_procedure(&mut self, name: &Identifier) -> Result { + self.expect_token(TokenType::Procedure)?; + let parameters = self.parse_anonymous_procedure_parameters()?; + if !self.match_token(TokenType::Returns) { + return Err(self.parse_error("Expected RETURNS after the procedure parameters")); + } + let (return_type, returns_table) = if self.match_token(TokenType::Table) { + (None, Some(self.parse_procedure_table_columns()?)) + } else { + let data_type = self.parse_data_type()?; + if !self.match_keywords(&[TokenType::Not, TokenType::Null]) { + self.match_token(TokenType::Null); + } + (Some(data_type), None) + }; + if !self.match_token(TokenType::Language) { + return Err(self.parse_error("Expected LANGUAGE after the procedure return type")); + } + let language = self.expect_identifier_or_keyword()?.to_ascii_uppercase(); + if !LANGUAGES.contains(&language.as_str()) { + return Err(self.parse_error(format!("Unknown procedure language {language}"))); + } + let mut procedure = CreateProcedure::new(""); + procedure.name = TableRef::new(""); + procedure.name.name = name.clone(); + procedure.parameters = parameters; + procedure.return_type = return_type; + procedure.returns_table = returns_table; + procedure.language = Some(language); + self.parse_anonymous_procedure_options(&mut procedure)?; + Ok(Expression::CreateProcedure(Box::new(procedure))) + } + + /// `( [ [DEFAULT ]] [, ...] )` with no empty or trailing entries. + fn parse_anonymous_procedure_parameters(&mut self) -> Result> { + self.expect_token(TokenType::LParen)?; + let mut parameters = Vec::new(); + while !self.check(TokenType::RParen) { + if !parameters.is_empty() { + self.expect_token(TokenType::Comma)?; + } + let name = self.expect_procedure_part_name()?; + let data_type = self.parse_data_type()?; + let default = if self.match_token(TokenType::Default) { + Some(self.parse_expression()?) + } else { + None + }; + parameters.push(FunctionParameter { + name: Some(name), + data_type, + mode: None, + default, + mode_text: None, + }); + } + self.expect_token(TokenType::RParen)?; + Ok(parameters) + } + + /// `( [ ] [, ...] )` after `RETURNS TABLE`. + fn parse_procedure_table_columns(&mut self) -> Result> { + self.expect_token(TokenType::LParen)?; + let mut columns = Vec::new(); + while !self.check(TokenType::RParen) { + if !columns.is_empty() { + self.expect_token(TokenType::Comma)?; + } + let name = self.expect_procedure_part_name()?; + columns.push(StructField::new(name.name, self.parse_data_type()?)); + } + self.expect_token(TokenType::RParen)?; + Ok(columns) + } + + /// A parameter or column name: an identifier, quoted or not a reserved word. + fn expect_procedure_part_name(&mut self) -> Result { + if !(self.check(TokenType::Identifier) + || self.check(TokenType::QuotedIdentifier) + || self.check(TokenType::Var) + || self.check_keyword()) + { + return Err(self.parse_error("Expected a parameter or column name")); + } + let name = self.expect_identifier_or_keyword_with_quoted()?; + self.reject_reserved_name_part(&name)?; + Ok(name) + } + + /// Handler options and the `AS` body, in any order, each at most once. + fn parse_anonymous_procedure_options(&mut self, procedure: &mut CreateProcedure) -> Result<()> { + loop { + let option = if self.match_identifier("RUNTIME_VERSION") || self.match_identifier("HANDLER") + { + let key = self.previous_text().to_ascii_uppercase(); + self.expect_token(TokenType::Eq)?; + format!("{key} = '{}'", self.expect_string()?) + } else if self.match_identifier("PACKAGES") || self.match_identifier("IMPORTS") { + let key = self.previous_text().to_ascii_uppercase(); + self.expect_token(TokenType::Eq)?; + format!("{key} = ({})", self.parse_procedure_string_list()?.join(", ")) + } else if self.match_text_seq(&["CALLED", "ON", "NULL", "INPUT"]) { + "CALLED ON NULL INPUT".to_string() + } else if self.match_text_seq(&["RETURNS", "NULL", "ON", "NULL", "INPUT"]) { + "RETURNS NULL ON NULL INPUT".to_string() + } else if self.match_identifier("STRICT") { + "STRICT".to_string() + } else if procedure.body.is_none() && self.match_token(TokenType::As) { + let body = self.parse_anonymous_procedure_body(procedure.language.as_deref())?; + procedure.body = Some(body); + continue; + } else { + return Ok(()); + }; + let key = option.split(' ').next().unwrap_or_default().to_string(); + if procedure.with_options.iter().any(|seen| seen.starts_with(&key)) { + return Err(self.parse_error(format!("{key} is given more than once"))); + } + procedure.with_options.push(option); + } + } + + /// `( '' [, ...] )`, possibly empty. + fn parse_procedure_string_list(&mut self) -> Result> { + self.expect_token(TokenType::LParen)?; + let mut values = Vec::new(); + while !self.check(TokenType::RParen) { + if !values.is_empty() { + self.expect_token(TokenType::Comma)?; + } + values.push(format!("'{}'", self.expect_string()?)); + } + self.expect_token(TokenType::RParen)?; + Ok(values) + } + + /// The body string; a LANGUAGE SQL body must parse as Snowflake Scripting. + fn parse_anonymous_procedure_body(&mut self, language: Option<&str>) -> Result { + if !(self.check(TokenType::String) || self.check(TokenType::DollarString)) { + return Err(self.parse_error("Expected the procedure body as a string after AS")); + } + let body = self.advance_text()?; + if language != Some("SQL") { + return Ok(FunctionBody::StringLiteral(body)); + } + let mode = if self.config.faithful_calls { + CallMode::Faithful + } else { + CallMode::Lowered + }; + let statements = Dialect::get(DialectType::Snowflake) + .parse_with_config(&body, self.config.complexity_guard.clone(), mode) + .map_err(|error| self.parse_error(format!("In the procedure body: {error}")))?; + if statements.is_empty() { + return Err(self.parse_error("Expected a procedure body")); + } + Ok(FunctionBody::Statements(statements)) + } + + /// `CALL ( [[ =>] , ...] ) [INTO :]` after a WITH clause. + pub(crate) fn parse_call_with(&mut self, with: With) -> Result { + self.skip(); + let mut name = vec![self.expect_identifier_or_keyword_with_quoted()?]; + while self.match_token(TokenType::Dot) { + name.push(self.expect_identifier_or_keyword_with_quoted()?); + } + for part in &name { + self.reject_reserved_name_part(part)?; + } + self.expect_token(TokenType::LParen)?; + let arguments = if self.check(TokenType::RParen) { + Vec::new() + } else { + self.parse_function_arguments()? + }; + self.expect_token(TokenType::RParen)?; + let into = if self.match_token(TokenType::Into) { + self.expect_token(TokenType::Colon)?; + let variable = self.expect_identifier_or_keyword_with_quoted()?; + self.reject_reserved_name_part(&variable)?; + Some(variable) + } else { + None + }; + Ok(Expression::Call(Box::new(Call { + source_dialect: None, + name, + member: None, + arguments, + asynchronous: false, + hints: Vec::new(), + with: Some(with), + into, + }))) + } +} diff --git a/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs b/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs index abcb3ac21..f818cabf8 100644 --- a/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs +++ b/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs @@ -541,6 +541,54 @@ fn given_unsupported_snowflake_sql_when_parsing_then_parsing_fails() { sql: "DECLARE SELECT RESULTSET; BEGIN RETURN 1; END;", expected_error: "Parse error", }, + ParseErrorTestCase { + description: "an anonymous procedure needs RETURNS", + dialect: Some(DialectType::Snowflake), + sql: "WITH p1 AS PROCEDURE () LANGUAGE SQL AS $$ BEGIN RETURN 1; END $$ CALL p1()", + expected_error: "Parse error", + }, + ParseErrorTestCase { + description: "an anonymous procedure is followed by CALL", + dialect: Some(DialectType::Snowflake), + sql: "WITH p1 AS PROCEDURE () RETURNS INT LANGUAGE JAVASCRIPT AS 'm' SELECT 1", + expected_error: "Parse error", + }, + ParseErrorTestCase { + description: "a SQL procedure body must parse as a block", + dialect: Some(DialectType::Snowflake), + sql: "WITH p1 AS PROCEDURE () RETURNS INT LANGUAGE SQL AS $$ BEGIN LOOP $$ CALL p1()", + expected_error: "Parse error", + }, + ParseErrorTestCase { + description: "a reserved word is not a procedure parameter", + dialect: Some(DialectType::Snowflake), + sql: "WITH p1 AS PROCEDURE (SELECT INT) RETURNS INT LANGUAGE JAVASCRIPT AS 'm' CALL p1()", + expected_error: "Parse error", + }, + ParseErrorTestCase { + description: "a reserved word is not a RETURNS TABLE column", + dialect: Some(DialectType::Snowflake), + sql: "WITH p1 AS PROCEDURE () RETURNS TABLE (SELECT INT) LANGUAGE JAVASCRIPT AS 'm' CALL p1()", + expected_error: "Parse error", + }, + ParseErrorTestCase { + description: "a handler option is given once", + dialect: Some(DialectType::Snowflake), + sql: "WITH p1 AS PROCEDURE () RETURNS INT LANGUAGE PYTHON HANDLER = 'a' HANDLER = 'b' CALL p1()", + expected_error: "Parse error", + }, + ParseErrorTestCase { + description: "a following CTE needs AS", + dialect: Some(DialectType::Snowflake), + sql: "WITH p1 AS PROCEDURE () RETURNS INT LANGUAGE JAVASCRIPT AS 'm', c1 (SELECT 1) CALL p1()", + expected_error: "Parse error", + }, + ParseErrorTestCase { + description: "a following CTE needs a name", + dialect: Some(DialectType::Snowflake), + sql: "WITH p1 AS PROCEDURE () RETURNS INT LANGUAGE JAVASCRIPT AS 'm', AS (SELECT 1) CALL p1()", + expected_error: "Parse error", + }, ParseErrorTestCase { description: "an unquoted name% column is a known limitation", dialect: Some(DialectType::Snowflake), @@ -572,6 +620,8 @@ fn given_neighbours_of_rejected_snowflake_sql_when_parsing_then_they_parse() { "SELECT * FROM TABLE(FLATTEN(INPUT => ARRAY_CONSTRUCT(1)))", "CREATE PROCEDURE p1() RETURNS TABLE () LANGUAGE SQL AS $$ BEGIN RETURN 1; END $$", "DECLARE \"SELECT\" RESULTSET; BEGIN RETURN 1; END;", + "WITH p1 AS PROCEDURE (v1 INT, v2 INT DEFAULT 2) RETURNS INT NOT NULL LANGUAGE JAVASCRIPT CALLED ON NULL INPUT AS 'm', c1 AS (SELECT 1) CALL p1(v1 => 1) INTO :x", + "WITH p1 AS PROCEDURE () RETURNS INT LANGUAGE JAVA RUNTIME_VERSION = '11' PACKAGES = ('com.snowflake:snowpark:1.14.0') HANDLER = 'm' CALL p1()", ]; for sql in accepted { @@ -640,6 +690,42 @@ fn given_snowflake_string_escapes_when_tokenizing_then_they_decode_as_documented } } +#[test] +fn given_an_anonymous_procedure_call_when_parsing_snowflake_then_its_parts_are_kept() { + let test_cases = [ + ParseKindTestCase { + description: "the statement is a call", + dialect: Some(DialectType::Snowflake), + sql: "WITH p1 AS PROCEDURE () RETURNS VARCHAR LANGUAGE SQL AS $$ BEGIN AWAIT ALL; END; $$ CALL p1()", + pointer: "/0", + expected_kind: Ok("call"), + }, + ParseKindTestCase { + description: "the call keeps the procedure it names", + dialect: Some(DialectType::Snowflake), + sql: "WITH p1 AS PROCEDURE () RETURNS VARCHAR LANGUAGE SQL AS $$ BEGIN AWAIT ALL; END; $$ CALL p1()", + pointer: "/0/call/with/ctes/0/this", + expected_kind: Ok("create_procedure"), + }, + ParseKindTestCase { + description: "a SQL body is parsed as a scripting block", + dialect: Some(DialectType::Snowflake), + sql: "WITH p1 AS PROCEDURE () RETURNS VARCHAR LANGUAGE SQL AS $$ BEGIN AWAIT ALL; END; $$ CALL p1()", + pointer: "/0/call/with/ctes/0/this/create_procedure/body/Statements/0", + expected_kind: Ok("scripting_block"), + }, + ]; + + for test_case in test_cases { + assert_eq!( + kind_at(test_case.dialect, test_case.sql, test_case.pointer), + test_case.expected_kind.map(str::to_owned), + "{}", + test_case.description + ); + } +} + #[test] fn given_alter_session_set_items_when_parsing_then_names_and_values_are_kept() { let test_cases = [ From d704778cee9d0bbec1a82c4a80e722c9a3f411f4 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 17:16:41 +0000 Subject: [PATCH 05/10] Keep date parts, ORDER BY ALL, unary plus and parenless calls in the tree A faithful Snowflake parse now records what was written: - an unquoted date part in DATEDIFF, TIMEDIFF, TIMESTAMPDIFF, EXTRACT(part, x) and TRUNC(x, part) is a date part, not a column; TRUNC(x, c1) stays a column; - ORDER BY ALL is an OrderBy.all node with its direction, not a sort key; - unary + is a Pos node instead of being dropped; - CURRENT_ORGANIZATION_USER without parentheses is a no_parens call. Lineage checks and typing treat Pos like Neg and read OrderBy.all. The WITH clause's trailing statement moves to its own module to keep entry.rs within its size limits. --- .../src/lineage/_helpers/checks/families.rs | 2 +- .../lineage/_helpers/checks/node_checks.rs | 1 + .../lineage/_helpers/checks/scope_rules.rs | 3 + .../src/lineage/_helpers/typing/rules.rs | 2 +- .../src/expressions/queries/models.rs | 16 +++ .../expressions/tree/_helpers/accessors.rs | 14 ++- .../src/expressions/tree/_helpers/api.rs | 6 +- .../src/expressions/tree/models.rs | 2 + .../src/functions/names/constants.rs | 1 + .../_helpers/expressions/comparisons.rs | 2 +- .../parser/_helpers/expressions/grouping.rs | 13 ++- .../src/parser/_helpers/functions/typed.rs | 1 + .../src/parser/_helpers/queries/clauses.rs | 35 +++--- .../src/parser/_helpers/statements/entry.rs | 77 +----------- .../src/parser/_helpers/statements/mod.rs | 1 + .../parser/_helpers/statements/procedures.rs | 32 +++-- .../_helpers/statements/with_statement.rs | 77 ++++++++++++ .../src/parser/_helpers/types/casts.rs | 5 +- .../sqlquack-syntax/src/parser/constants.rs | 3 + .../src/parser/tests/test_snowflake.rs | 110 +++++++++++++++--- .../sqlquack-syntax/tests/parser/helpers.rs | 8 ++ .../tests/parser/test_fixes.rs | 102 +++++++++++++++- .../tests/parser/test_types.rs | 7 ++ 23 files changed, 391 insertions(+), 129 deletions(-) create mode 100644 crates/sqlquack-syntax/src/parser/_helpers/statements/with_statement.rs diff --git a/crates/sqlquack-semantics/src/lineage/_helpers/checks/families.rs b/crates/sqlquack-semantics/src/lineage/_helpers/checks/families.rs index 50e603506..59d2a5905 100644 --- a/crates/sqlquack-semantics/src/lineage/_helpers/checks/families.rs +++ b/crates/sqlquack-semantics/src/lineage/_helpers/checks/families.rs @@ -261,7 +261,7 @@ fn written_family( Expression::Literal(literal) => literal_family(literal), Expression::Boolean(_) => Family::Boolean, Expression::Alias(alias) => family(&alias.this), - Expression::Neg(unary) => family(&unary.this), + Expression::Neg(unary) | Expression::Pos(unary) => family(&unary.this), Expression::Add(operation) | Expression::Sub(operation) | Expression::Mul(operation) => { arithmetic_family(family(&operation.left), family(&operation.right)) } diff --git a/crates/sqlquack-semantics/src/lineage/_helpers/checks/node_checks.rs b/crates/sqlquack-semantics/src/lineage/_helpers/checks/node_checks.rs index 723c3827f..bdcd9bd08 100644 --- a/crates/sqlquack-semantics/src/lineage/_helpers/checks/node_checks.rs +++ b/crates/sqlquack-semantics/src/lineage/_helpers/checks/node_checks.rs @@ -409,6 +409,7 @@ fn scalar_subqueries(node: &Expression, level: NodeLevel<'_, '_>) -> Vec) -> bool { if order.siblings { return false; } + if order.all.is_some() { + return true; + } let [ordered] = order.expressions.as_slice() else { return false; }; diff --git a/crates/sqlquack-semantics/src/lineage/_helpers/typing/rules.rs b/crates/sqlquack-semantics/src/lineage/_helpers/typing/rules.rs index 71278d3fe..6d911f400 100644 --- a/crates/sqlquack-semantics/src/lineage/_helpers/typing/rules.rs +++ b/crates/sqlquack-semantics/src/lineage/_helpers/typing/rules.rs @@ -152,7 +152,7 @@ fn generic_type( Expression::ListAgg(_) | Expression::StringAgg(_) | Expression::GroupConcat(_) => { (Some(Rc::from(TEXT_TYPE)), Nullability::Unknown) } - Expression::Neg(_) => (first_type(parts), Nullability::Unknown), + Expression::Neg(_) | Expression::Pos(_) => (first_type(parts), Nullability::Unknown), Expression::Upper(_) | Expression::Lower(_) => ( Some(Rc::from(TEXT_TYPE)), ruled(NullabilityRule::FirstArgument, parts), diff --git a/crates/sqlquack-syntax/src/expressions/queries/models.rs b/crates/sqlquack-syntax/src/expressions/queries/models.rs index 7e2a2792c..5d98591d4 100644 --- a/crates/sqlquack-syntax/src/expressions/queries/models.rs +++ b/crates/sqlquack-syntax/src/expressions/queries/models.rs @@ -197,6 +197,7 @@ impl Select { expressions, siblings: false, comments: Vec::new(), + all: None, }); self } @@ -765,6 +766,21 @@ pub struct OrderBy { /// Leading comments that appeared before the ORDER BY keyword #[serde(default, skip_serializing_if = "Vec::is_empty")] pub comments: Vec, + /// DuckDB and Snowflake `ORDER BY ALL`, which sorts by every output column; `expressions` is then empty. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub all: Option, +} + +/// The direction and null placement of `ORDER BY ALL`. +#[derive(sqlquack_syntax_derive::AstNode, Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct OrderByAll { + /// Whether the sort direction is descending. + pub desc: bool, + /// `Some(true)` = NULLS FIRST, `Some(false)` = NULLS LAST, `None` = unspecified. + pub nulls_first: Option, + /// Whether ASC was explicitly written. + #[serde(default)] + pub explicit_asc: bool, } /// A sort key: ascending unless `desc`; `nulls_first` is `None` when unspecified. diff --git a/crates/sqlquack-syntax/src/expressions/tree/_helpers/accessors.rs b/crates/sqlquack-syntax/src/expressions/tree/_helpers/accessors.rs index 9f8e8908e..2928518b1 100644 --- a/crates/sqlquack-syntax/src/expressions/tree/_helpers/accessors.rs +++ b/crates/sqlquack-syntax/src/expressions/tree/_helpers/accessors.rs @@ -213,9 +213,10 @@ impl Expression { | Expression::BitwiseLeftShift(op) | Expression::BitwiseRightShift(op) => op.inferred_type.as_ref(), - Expression::Not(op) | Expression::Neg(op) | Expression::BitwiseNot(op) => { - op.inferred_type.as_ref() - } + Expression::Not(op) + | Expression::Neg(op) + | Expression::Pos(op) + | Expression::BitwiseNot(op) => op.inferred_type.as_ref(), Expression::Like(op) | Expression::ILike(op) => op.inferred_type.as_ref(), @@ -442,9 +443,10 @@ impl Expression { | Expression::BitwiseLeftShift(op) | Expression::BitwiseRightShift(op) => op.inferred_type = Some(dt), - Expression::Not(op) | Expression::Neg(op) | Expression::BitwiseNot(op) => { - op.inferred_type = Some(dt) - } + Expression::Not(op) + | Expression::Neg(op) + | Expression::Pos(op) + | Expression::BitwiseNot(op) => op.inferred_type = Some(dt), Expression::Like(op) | Expression::ILike(op) => op.inferred_type = Some(dt), diff --git a/crates/sqlquack-syntax/src/expressions/tree/_helpers/api.rs b/crates/sqlquack-syntax/src/expressions/tree/_helpers/api.rs index 5afe58b1c..0178cac96 100644 --- a/crates/sqlquack-syntax/src/expressions/tree/_helpers/api.rs +++ b/crates/sqlquack-syntax/src/expressions/tree/_helpers/api.rs @@ -79,6 +79,7 @@ impl Expression { Expression::ExtendsRight(_) => "extends_right", Expression::Not(_) => "not", Expression::Neg(_) => "neg", + Expression::Pos(_) => "pos", Expression::BitwiseNot(_) => "bitwise_not", Expression::In(_) => "in", Expression::Between(_) => "between", @@ -962,7 +963,10 @@ impl Expression { Expression::JSONColumnDef(j) => j.this.as_deref(), Expression::Hierarchy(hierarchy) => Some(&hierarchy.source), Expression::Upsert(upsert) => Some(&upsert.source), - Expression::Not(u) | Expression::Neg(u) | Expression::BitwiseNot(u) => Some(&u.this), + Expression::Not(u) + | Expression::Neg(u) + | Expression::Pos(u) + | Expression::BitwiseNot(u) => Some(&u.this), Expression::Upper(f) | Expression::Lower(f) | Expression::Length(f) diff --git a/crates/sqlquack-syntax/src/expressions/tree/models.rs b/crates/sqlquack-syntax/src/expressions/tree/models.rs index 25848ba7a..b9003f953 100644 --- a/crates/sqlquack-syntax/src/expressions/tree/models.rs +++ b/crates/sqlquack-syntax/src/expressions/tree/models.rs @@ -807,6 +807,8 @@ pub enum Expression { Not(Box), Neg(Box), + /// Unary `+`, kept so the tree records the operator as written. + Pos(Box), BitwiseNot(Box), In(Box), diff --git a/crates/sqlquack-syntax/src/functions/names/constants.rs b/crates/sqlquack-syntax/src/functions/names/constants.rs index 9fd1b6ba4..1da7ec97a 100644 --- a/crates/sqlquack-syntax/src/functions/names/constants.rs +++ b/crates/sqlquack-syntax/src/functions/names/constants.rs @@ -30,6 +30,7 @@ pub(crate) const NO_PAREN_FUNCTION_NAME_LIST: &[&str] = &[ "CURRENT_TIMESTAMP", "CURRENT_DATETIME", "CURRENT_USER", + "CURRENT_ORGANIZATION_USER", "CURRENT_ROLE", "CURRENT_SCHEMA", "CURRENT_CATALOG", diff --git a/crates/sqlquack-syntax/src/parser/_helpers/expressions/comparisons.rs b/crates/sqlquack-syntax/src/parser/_helpers/expressions/comparisons.rs index 9b7c20184..a716a0833 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/expressions/comparisons.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/expressions/comparisons.rs @@ -1792,7 +1792,7 @@ impl Parser { ) -> Result { for token in prefixes.into_iter().rev() { expr = match token { - TokenType::Plus => expr, + TokenType::Plus => Expression::Pos(Box::new(UnaryOp::new(expr))), TokenType::Dash => Expression::Neg(Box::new(UnaryOp::new(expr))), TokenType::Not => Expression::Not(Box::new(UnaryOp::new(expr))), TokenType::Tilde => Expression::BitwiseNot(Box::new(UnaryOp::new(expr))), diff --git a/crates/sqlquack-syntax/src/parser/_helpers/expressions/grouping.rs b/crates/sqlquack-syntax/src/parser/_helpers/expressions/grouping.rs index 8693d07b2..9eaf4aa90 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/expressions/grouping.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/expressions/grouping.rs @@ -1383,11 +1383,12 @@ impl Parser { } } - self.mark_order_by_all(&mut expressions); + let (expressions, all) = self.take_order_by_all(expressions); Ok(Some(Expression::OrderBy(Box::new(OrderBy { expressions, siblings: false, comments: Vec::new(), + all, })))) } @@ -1915,6 +1916,16 @@ impl Parser { let unit_index: Option = match dialect { DT::Snowflake => match upper.as_str() { "DATE_TRUNC" | "DATETRUNC" if !args.is_empty() => Some(0), + "DATEDIFF" | "TIMEDIFF" | "TIMESTAMPDIFF" if args.len() == TERNARY_ARITY => Some(0), + "EXTRACT" if args.len() == BINARY_ARITY => Some(0), + "TRUNC" | "TRUNCATE" + if args.len() == BINARY_ARITY + && matches!(&args[1], Expression::Column(c) + if c.table.is_none() && !c.name.quoted) + && self.try_parse_date_part_unit_expr(&args[1]).is_some() => + { + Some(1) + } _ => None, }, _ => None, diff --git a/crates/sqlquack-syntax/src/parser/_helpers/functions/typed.rs b/crates/sqlquack-syntax/src/parser/_helpers/functions/typed.rs index 5183f6ba0..b510552ab 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/functions/typed.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/functions/typed.rs @@ -1799,6 +1799,7 @@ impl Parser { expressions: order_exprs, siblings: false, comments: Vec::new(), + all: None, })))) } else { None diff --git a/crates/sqlquack-syntax/src/parser/_helpers/queries/clauses.rs b/crates/sqlquack-syntax/src/parser/_helpers/queries/clauses.rs index b10327773..0fc8191f0 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/queries/clauses.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/queries/clauses.rs @@ -30,6 +30,7 @@ use crate::expressions::queries::models::MatchRecognizeSemantics; use crate::expressions::queries::models::NamedWindow; use crate::expressions::queries::models::Offset; use crate::expressions::queries::models::OrderBy; +use crate::expressions::queries::models::OrderByAll; use crate::expressions::queries::models::Ordered; use crate::expressions::queries::models::Over; use crate::expressions::queries::models::Prior; @@ -482,30 +483,36 @@ impl Parser { } } - self.mark_order_by_all(&mut expressions); + let (expressions, all) = self.take_order_by_all(expressions); Ok(OrderBy { expressions, siblings, comments: Vec::new(), + all, }) } - /// Marks a query-level ORDER BY ALL as the keyword, not an identifier. - pub(crate) fn mark_order_by_all(&self, expressions: &mut [Ordered]) { - if !self + /// Takes a lone unquoted `ALL` sort key as the ORDER BY ALL keyword, not an identifier. + pub(crate) fn take_order_by_all( + &self, + mut expressions: Vec, + ) -> (Vec, Option) { + let supported = self .config .dialect - .is_some_and(|d| d.supports_order_by_all()) - { - return; - } - if let [ordered] = expressions { - if matches!(&ordered.this, Expression::Column(c) - if c.table.is_none() && !c.name.quoted && c.name.name.eq_ignore_ascii_case("all")) - { - ordered.this = Expression::Var(Box::new(Var { this: "ALL".into() })); - } + .is_some_and(|d| d.supports_order_by_all()); + let lone_all = matches!(expressions.as_slice(), [ordered] if matches!(&ordered.this, + Expression::Column(c) + if c.table.is_none() && !c.name.quoted && c.name.name.eq_ignore_ascii_case("all"))); + if !(supported && lone_all) { + return (expressions, None); } + let all = expressions.pop().map(|ordered| OrderByAll { + desc: ordered.desc, + nulls_first: ordered.nulls_first, + explicit_asc: ordered.explicit_asc, + }); + (expressions, all) } /// Parse a LIMIT count without mistaking a trailing percent sign for modulo. diff --git a/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs b/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs index 473127beb..71ea54c33 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs @@ -1930,81 +1930,14 @@ impl Parser { let search = self.parse_recursive_with_search()?; - if has_procedure { - if !self.check_identifier("CALL") { - return Err(self.parse_error("Expected CALL after an anonymous procedure")); - } - return self.parse_call_with(With { + self.parse_with_statement( + With { ctes, recursive, leading_comments, search, - }); - } - - let mut main_query = self.parse_statement()?; - - loop { - match main_query { - Expression::Paren(paren) => { - main_query = paren.this; - } - Expression::Subquery(ref sub) - if sub.alias.is_none() - && sub.order_by.is_none() - && sub.limit.is_none() - && sub.offset.is_none() => - { - if let Expression::Subquery(sub) = main_query { - main_query = sub.this; - } else { - break; - } - } - _ => break, - } - } - - let with_clause = With { - ctes, - recursive, - leading_comments, - search, - }; - match &mut main_query { - Expression::Select(ref mut select) => { - select.with = Some(with_clause); - } - Expression::Union(ref mut union) => { - union.with = Some(with_clause); - } - Expression::Intersect(ref mut intersect) => { - intersect.with = Some(with_clause); - } - Expression::Except(ref mut except) => { - except.with = Some(with_clause); - } - Expression::Update(ref mut update) => { - update.with = Some(with_clause); - } - Expression::Insert(ref mut insert) => { - insert.with = Some(with_clause); - } - Expression::Delete(ref mut delete) => { - delete.with = Some(with_clause); - } - Expression::CreateTable(ref mut ct) => { - ct.with_cte = Some(with_clause); - } - Expression::Pivot(ref mut pivot) => { - pivot.with = Some(with_clause); - } - Expression::Merge(ref mut merge) => { - merge.with_ = Some(Box::new(Expression::With(Box::new(with_clause)))); - } - _ => {} - } - - Ok(main_query) + }, + has_procedure, + ) } } diff --git a/crates/sqlquack-syntax/src/parser/_helpers/statements/mod.rs b/crates/sqlquack-syntax/src/parser/_helpers/statements/mod.rs index 311dda77c..3568a14c6 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/statements/mod.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/mod.rs @@ -6,3 +6,4 @@ pub(crate) mod entry; pub(crate) mod procedures; pub(crate) mod scripting; pub(crate) mod sessions; +pub(crate) mod with_statement; diff --git a/crates/sqlquack-syntax/src/parser/_helpers/statements/procedures.rs b/crates/sqlquack-syntax/src/parser/_helpers/statements/procedures.rs index e5fc535d3..2eef11d97 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/statements/procedures.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/procedures.rs @@ -56,14 +56,14 @@ impl Parser { procedure.return_type = return_type; procedure.returns_table = returns_table; procedure.language = Some(language); - self.parse_anonymous_procedure_options(&mut procedure)?; + let procedure = self.parse_anonymous_procedure_options(procedure)?; Ok(Expression::CreateProcedure(Box::new(procedure))) } /// `( [ [DEFAULT ]] [, ...] )` with no empty or trailing entries. fn parse_anonymous_procedure_parameters(&mut self) -> Result> { self.expect_token(TokenType::LParen)?; - let mut parameters = Vec::new(); + let mut parameters: Vec = Vec::new(); while !self.check(TokenType::RParen) { if !parameters.is_empty() { self.expect_token(TokenType::Comma)?; @@ -90,7 +90,7 @@ impl Parser { /// `( [ ] [, ...] )` after `RETURNS TABLE`. fn parse_procedure_table_columns(&mut self) -> Result> { self.expect_token(TokenType::LParen)?; - let mut columns = Vec::new(); + let mut columns: Vec = Vec::new(); while !self.check(TokenType::RParen) { if !columns.is_empty() { self.expect_token(TokenType::Comma)?; @@ -117,9 +117,13 @@ impl Parser { } /// Handler options and the `AS` body, in any order, each at most once. - fn parse_anonymous_procedure_options(&mut self, procedure: &mut CreateProcedure) -> Result<()> { + fn parse_anonymous_procedure_options( + &mut self, + mut procedure: CreateProcedure, + ) -> Result { loop { - let option = if self.match_identifier("RUNTIME_VERSION") || self.match_identifier("HANDLER") + let option = if self.match_identifier("RUNTIME_VERSION") + || self.match_identifier("HANDLER") { let key = self.previous_text().to_ascii_uppercase(); self.expect_token(TokenType::Eq)?; @@ -127,7 +131,10 @@ impl Parser { } else if self.match_identifier("PACKAGES") || self.match_identifier("IMPORTS") { let key = self.previous_text().to_ascii_uppercase(); self.expect_token(TokenType::Eq)?; - format!("{key} = ({})", self.parse_procedure_string_list()?.join(", ")) + format!( + "{key} = ({})", + self.parse_procedure_string_list()?.join(", ") + ) } else if self.match_text_seq(&["CALLED", "ON", "NULL", "INPUT"]) { "CALLED ON NULL INPUT".to_string() } else if self.match_text_seq(&["RETURNS", "NULL", "ON", "NULL", "INPUT"]) { @@ -139,10 +146,14 @@ impl Parser { procedure.body = Some(body); continue; } else { - return Ok(()); + return Ok(procedure); }; let key = option.split(' ').next().unwrap_or_default().to_string(); - if procedure.with_options.iter().any(|seen| seen.starts_with(&key)) { + if procedure + .with_options + .iter() + .any(|seen| seen.starts_with(&key)) + { return Err(self.parse_error(format!("{key} is given more than once"))); } procedure.with_options.push(option); @@ -152,7 +163,7 @@ impl Parser { /// `( '' [, ...] )`, possibly empty. fn parse_procedure_string_list(&mut self) -> Result> { self.expect_token(TokenType::LParen)?; - let mut values = Vec::new(); + let mut values: Vec = Vec::new(); while !self.check(TokenType::RParen) { if !values.is_empty() { self.expect_token(TokenType::Comma)?; @@ -188,6 +199,9 @@ impl Parser { /// `CALL ( [[ =>] , ...] ) [INTO :]` after a WITH clause. pub(crate) fn parse_call_with(&mut self, with: With) -> Result { + if !self.check_identifier("CALL") { + return Err(self.parse_error("Expected CALL after an anonymous procedure")); + } self.skip(); let mut name = vec![self.expect_identifier_or_keyword_with_quoted()?]; while self.match_token(TokenType::Dot) { diff --git a/crates/sqlquack-syntax/src/parser/_helpers/statements/with_statement.rs b/crates/sqlquack-syntax/src/parser/_helpers/statements/with_statement.rs new file mode 100644 index 000000000..7d03e434e --- /dev/null +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/with_statement.rs @@ -0,0 +1,77 @@ +//! The statement a WITH clause prefixes, with the clause attached to it. + +use crate::expressions::queries::models::With; +use crate::expressions::tree::models::Expression; +use crate::parser::models::Parser; +use crate::tokens::types::Result; + +impl Parser { + /// The statement after a WITH clause, which carries the clause; a CALL after an anonymous procedure. + pub(crate) fn parse_with_statement( + &mut self, + with_clause: With, + has_procedure: bool, + ) -> Result { + if has_procedure { + return self.parse_call_with(with_clause); + } + let mut main_query = self.parse_statement()?; + + loop { + match main_query { + Expression::Paren(paren) => { + main_query = paren.this; + } + Expression::Subquery(ref sub) + if sub.alias.is_none() + && sub.order_by.is_none() + && sub.limit.is_none() + && sub.offset.is_none() => + { + if let Expression::Subquery(sub) = main_query { + main_query = sub.this; + } else { + break; + } + } + _ => break, + } + } + + match &mut main_query { + Expression::Select(ref mut select) => { + select.with = Some(with_clause); + } + Expression::Union(ref mut union) => { + union.with = Some(with_clause); + } + Expression::Intersect(ref mut intersect) => { + intersect.with = Some(with_clause); + } + Expression::Except(ref mut except) => { + except.with = Some(with_clause); + } + Expression::Update(ref mut update) => { + update.with = Some(with_clause); + } + Expression::Insert(ref mut insert) => { + insert.with = Some(with_clause); + } + Expression::Delete(ref mut delete) => { + delete.with = Some(with_clause); + } + Expression::CreateTable(ref mut ct) => { + ct.with_cte = Some(with_clause); + } + Expression::Pivot(ref mut pivot) => { + pivot.with = Some(with_clause); + } + Expression::Merge(ref mut merge) => { + merge.with_ = Some(Box::new(Expression::With(Box::new(with_clause)))); + } + _ => {} + } + + Ok(main_query) + } +} diff --git a/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs b/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs index 1076da97a..c1d8b632a 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/types/casts.rs @@ -13,6 +13,7 @@ use crate::parser::constants::NATIONAL; use crate::parser::constants::NCHAR; use crate::parser::constants::NVARCHAR; use crate::parser::constants::REAL; +use crate::parser::constants::TABLE_TYPE; use crate::parser::models::Parser; use crate::parser::models::ParserDepthScope; use crate::tokens::models::SourceSpan; @@ -1110,7 +1111,7 @@ impl Parser { self.config.dialect, Some(crate::tokens::models::DialectType::Snowflake) ) && self.previous().token_type != TokenType::QuotedIdentifier - && name != "TABLE" + && name != TABLE_TYPE && crate::parser::constants::SNOWFLAKE_RESERVED_KEYWORDS.contains(&name.as_str()) { return Err(self.parse_error(format!( @@ -1129,7 +1130,7 @@ impl Parser { let mut after_comma = true; loop { if self.check(TokenType::RParen) { - if after_comma && !(args.is_empty() && name == "TABLE") { + if after_comma && !(args.is_empty() && name == TABLE_TYPE) { return Err(self.parse_error("Expected a type parameter")); } break; diff --git a/crates/sqlquack-syntax/src/parser/constants.rs b/crates/sqlquack-syntax/src/parser/constants.rs index 4b9936258..86bc596cb 100644 --- a/crates/sqlquack-syntax/src/parser/constants.rs +++ b/crates/sqlquack-syntax/src/parser/constants.rs @@ -90,6 +90,9 @@ pub(crate) const NATIONAL: &str = "NATIONAL"; /// The text "NCHAR", as comparisons test it. pub(crate) const NCHAR: &str = "NCHAR"; +/// The type name of `RETURNS TABLE (...)`, which may have an empty column list. +pub(crate) const TABLE_TYPE: &str = "TABLE"; + /// The text "NVARCHAR", as comparisons test it. pub(crate) const NVARCHAR: &str = "NVARCHAR"; diff --git a/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs b/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs index f818cabf8..2bdf50272 100644 --- a/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs +++ b/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs @@ -3,7 +3,6 @@ use serde_json::json; use crate::parser::tests::helpers::first_token_text; use crate::parser::tests::helpers::kind_at; use crate::parser::tests::helpers::parse_error; -use crate::parser::tests::helpers::parsed; use crate::parser::tests::helpers::value_at; use crate::parser::tests::test_types::DialectTokenTestCase; use crate::parser::tests::test_types::ParseErrorTestCase; @@ -609,26 +608,99 @@ fn given_unsupported_snowflake_sql_when_parsing_then_parsing_fails() { #[test] fn given_neighbours_of_rejected_snowflake_sql_when_parsing_then_they_parse() { - let accepted = [ - "SELECT CAST(NULL AS VARCHAR())", - "SELECT CAST(NULL AS CHAR())", - "SELECT CAST(NULL AS TIME(9))", - "SELECT CAST(NULL AS NUMBER(9, 2))", - "SELECT CAST(NULL AS VARBINARY(8))", - "DROP TABLE \"SELECT\"", - "USE \"SELECT\".v1", - "SELECT * FROM TABLE(FLATTEN(INPUT => ARRAY_CONSTRUCT(1)))", - "CREATE PROCEDURE p1() RETURNS TABLE () LANGUAGE SQL AS $$ BEGIN RETURN 1; END $$", - "DECLARE \"SELECT\" RESULTSET; BEGIN RETURN 1; END;", - "WITH p1 AS PROCEDURE (v1 INT, v2 INT DEFAULT 2) RETURNS INT NOT NULL LANGUAGE JAVASCRIPT CALLED ON NULL INPUT AS 'm', c1 AS (SELECT 1) CALL p1(v1 => 1) INTO :x", - "WITH p1 AS PROCEDURE () RETURNS INT LANGUAGE JAVA RUNTIME_VERSION = '11' PACKAGES = ('com.snowflake:snowpark:1.14.0') HANDLER = 'm' CALL p1()", + let test_cases = [ + ParseKindTestCase { + description: "SELECT CAST(NULL AS VARCHAR()) parses", + dialect: Some(DialectType::Snowflake), + sql: "SELECT CAST(NULL AS VARCHAR())", + pointer: "/0", + expected_kind: Ok("select"), + }, + ParseKindTestCase { + description: "SELECT CAST(NULL AS CHAR()) parses", + dialect: Some(DialectType::Snowflake), + sql: "SELECT CAST(NULL AS CHAR())", + pointer: "/0", + expected_kind: Ok("select"), + }, + ParseKindTestCase { + description: "SELECT CAST(NULL AS TIME(9)) parses", + dialect: Some(DialectType::Snowflake), + sql: "SELECT CAST(NULL AS TIME(9))", + pointer: "/0", + expected_kind: Ok("select"), + }, + ParseKindTestCase { + description: "SELECT CAST(NULL AS NUMBER(9, 2)) parses", + dialect: Some(DialectType::Snowflake), + sql: "SELECT CAST(NULL AS NUMBER(9, 2))", + pointer: "/0", + expected_kind: Ok("select"), + }, + ParseKindTestCase { + description: "SELECT CAST(NULL AS VARBINARY(8)) parses", + dialect: Some(DialectType::Snowflake), + sql: "SELECT CAST(NULL AS VARBINARY(8))", + pointer: "/0", + expected_kind: Ok("select"), + }, + ParseKindTestCase { + description: "DROP TABLE 'SELECT' parses", + dialect: Some(DialectType::Snowflake), + sql: "DROP TABLE \"SELECT\"", + pointer: "/0", + expected_kind: Ok("drop_table"), + }, + ParseKindTestCase { + description: "USE 'SELECT'.v1 parses", + dialect: Some(DialectType::Snowflake), + sql: "USE \"SELECT\".v1", + pointer: "/0", + expected_kind: Ok("use"), + }, + ParseKindTestCase { + description: "SELECT * FROM TABLE(FLATTEN(INPUT => ARRAY_CONSTRUCT(1))) parses", + dialect: Some(DialectType::Snowflake), + sql: "SELECT * FROM TABLE(FLATTEN(INPUT => ARRAY_CONSTRUCT(1)))", + pointer: "/0", + expected_kind: Ok("select"), + }, + ParseKindTestCase { + description: "CREATE PROCEDURE p1() RETURNS TABLE parses", + dialect: Some(DialectType::Snowflake), + sql: "CREATE PROCEDURE p1() RETURNS TABLE () LANGUAGE SQL AS $$ BEGIN RETURN 1; END $$", + pointer: "/0", + expected_kind: Ok("create_procedure"), + }, + ParseKindTestCase { + description: "DECLARE 'SELECT' RESULTSET; BEGIN RETURN 1; END; parses", + dialect: Some(DialectType::Snowflake), + sql: "DECLARE \"SELECT\" RESULTSET; BEGIN RETURN 1; END;", + pointer: "/0", + expected_kind: Ok("scripting_block"), + }, + ParseKindTestCase { + description: "WITH p1 AS PROCEDURE parses", + dialect: Some(DialectType::Snowflake), + sql: "WITH p1 AS PROCEDURE (v1 INT, v2 INT DEFAULT 2) RETURNS INT NOT NULL LANGUAGE JAVASCRIPT CALLED ON NULL INPUT AS 'm', c1 AS (SELECT 1) CALL p1(v1 => 1) INTO :x", + pointer: "/0", + expected_kind: Ok("call"), + }, + ParseKindTestCase { + description: "WITH p1 AS PROCEDURE parses", + dialect: Some(DialectType::Snowflake), + sql: "WITH p1 AS PROCEDURE () RETURNS INT LANGUAGE JAVA RUNTIME_VERSION = '11' PACKAGES = ('com.snowflake:snowpark:1.14.0') HANDLER = 'm' CALL p1()", + pointer: "/0", + expected_kind: Ok("call"), + }, ]; - for sql in accepted { - assert!( - parsed(Some(DialectType::Snowflake), sql).is_ok(), - "{sql}: {}", - parse_error(Some(DialectType::Snowflake), sql) + for test_case in test_cases { + assert_eq!( + kind_at(test_case.dialect, test_case.sql, test_case.pointer), + test_case.expected_kind.map(str::to_owned), + "{}", + test_case.description ); } } diff --git a/crates/sqlquack-syntax/tests/parser/helpers.rs b/crates/sqlquack-syntax/tests/parser/helpers.rs index c1aa3b0f0..d6bb5e873 100644 --- a/crates/sqlquack-syntax/tests/parser/helpers.rs +++ b/crates/sqlquack-syntax/tests/parser/helpers.rs @@ -46,6 +46,14 @@ pub(crate) fn projection_at(sql: &str, pointer: &str) -> Value { .unwrap_or_default() } +/// The value at `pointer` in the faithful Snowflake parse of `sql`, or null. +pub(crate) fn statement_at(sql: &str, pointer: &str) -> Value { + statements(sql, Dialect::Snowflake, CallMode::Faithful) + .pointer(pointer) + .cloned() + .unwrap_or_default() +} + /// The source text a located parse records for the node at `pointer`, or why there is none. pub(crate) fn located_text(sql: &str, pointer: &str) -> Result { let tree = statements(sql, Dialect::Snowflake, CallMode::FaithfulLocated); diff --git a/crates/sqlquack-syntax/tests/parser/test_fixes.rs b/crates/sqlquack-syntax/tests/parser/test_fixes.rs index 672d79bed..267bb9837 100644 --- a/crates/sqlquack-syntax/tests/parser/test_fixes.rs +++ b/crates/sqlquack-syntax/tests/parser/test_fixes.rs @@ -2,8 +2,10 @@ use serde_json::json; use sqlquack_syntax::Dialect; -use crate::helpers::{accepted, cast_targets, projection_at}; -use crate::test_types::{AcceptanceTestCase, CastTargetTestCase, ProjectionTestCase}; +use crate::helpers::{accepted, cast_targets, projection_at, statement_at}; +use crate::test_types::{ + AcceptanceTestCase, CastTargetTestCase, ProjectionTestCase, StatementTestCase, +}; #[test] fn given_an_index_followed_by_a_key_when_parsing_then_it_is_a_path_on_the_element() { @@ -420,3 +422,99 @@ fn given_a_select_list_when_parsing_snowflake_then_it_needs_an_item_and_top_need ); } } + +#[test] +fn given_auditor_tree_findings_when_parsing_faithfully_then_the_tree_records_what_was_written() { + let test_cases = [ + ProjectionTestCase { + description: "DATEDIFF's date part is a date part, not a column", + sql: "SELECT DATEDIFF(DAY, 1, 1)", + pointer: "/function/args/0/var/this", + expected_value: json!("DAY"), + }, + ProjectionTestCase { + description: "EXTRACT's comma form takes a date part", + sql: "SELECT EXTRACT(DAY, 1)", + pointer: "/function/args/0/var/this", + expected_value: json!("DAY"), + }, + ProjectionTestCase { + description: "TIMEDIFF's date part is a date part", + sql: "SELECT TIMEDIFF(DAY, 1, 1)", + pointer: "/function/args/0/var/this", + expected_value: json!("DAY"), + }, + ProjectionTestCase { + description: "TIMESTAMPDIFF's date part is a date part", + sql: "SELECT TIMESTAMPDIFF(DAY, 1, 1)", + pointer: "/function/args/0/var/this", + expected_value: json!("DAY"), + }, + ProjectionTestCase { + description: "TRUNC's date part is a date part", + sql: "SELECT TRUNC(1, DAY)", + pointer: "/function/args/1/var/this", + expected_value: json!("DAY"), + }, + ProjectionTestCase { + description: "TRUNC's numeric scale column stays a column", + sql: "SELECT TRUNC(1, c1)", + pointer: "/function/args/1/column/name/name", + expected_value: json!("c1"), + }, + ProjectionTestCase { + description: "unary plus keeps its operator", + sql: "SELECT + 1", + pointer: "/pos/this/literal/value", + expected_value: json!("1"), + }, + ProjectionTestCase { + description: "CURRENT_ORGANIZATION_USER without parentheses is a call", + sql: "SELECT CURRENT_ORGANIZATION_USER", + pointer: "/function/no_parens", + expected_value: json!(true), + }, + ]; + + for test_case in test_cases { + assert_eq!( + projection_at(test_case.sql, test_case.pointer), + test_case.expected_value, + "{}", + test_case.description + ); + } +} + +#[test] +fn given_order_by_all_when_parsing_then_it_is_the_keyword_with_its_direction() { + let test_cases = [ + StatementTestCase { + description: "ORDER BY ALL DESC keeps its direction", + sql: "SELECT 1 ORDER BY ALL DESC", + pointer: "/0/select/order_by/all/desc", + expected_value: json!(true), + }, + StatementTestCase { + description: "ORDER BY ALL has no sort keys", + sql: "SELECT 1 ORDER BY ALL DESC", + pointer: "/0/select/order_by/expressions", + expected_value: json!([]), + }, + StatementTestCase { + description: "a quoted ALL is a column, not the keyword", + sql: "SELECT 1 ORDER BY \"ALL\"", + pointer: "/0/select/order_by/all", + expected_value: json!(null), + }, + ]; + + for test_case in test_cases { + assert_eq!( + statement_at(test_case.sql, test_case.pointer), + test_case.expected_value, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlquack-syntax/tests/parser/test_types.rs b/crates/sqlquack-syntax/tests/parser/test_types.rs index 1c266151e..1292f1e02 100644 --- a/crates/sqlquack-syntax/tests/parser/test_types.rs +++ b/crates/sqlquack-syntax/tests/parser/test_types.rs @@ -21,6 +21,13 @@ pub(crate) struct ProjectionTestCase { pub(crate) expected_value: Value, } +pub(crate) struct StatementTestCase { + pub(crate) description: &'static str, + pub(crate) sql: &'static str, + pub(crate) pointer: &'static str, + pub(crate) expected_value: Value, +} + pub(crate) struct LocatedTestCase { pub(crate) description: &'static str, pub(crate) sql: &'static str, From 604abf0f317e847f8bd3022b82ac77a06b986a70 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 17:24:11 +0000 Subject: [PATCH 06/10] Pass the procedure body's complexity guard by copy --- .../src/parser/_helpers/statements/procedures.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/sqlquack-syntax/src/parser/_helpers/statements/procedures.rs b/crates/sqlquack-syntax/src/parser/_helpers/statements/procedures.rs index 2eef11d97..28a604189 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/statements/procedures.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/procedures.rs @@ -189,7 +189,7 @@ impl Parser { CallMode::Lowered }; let statements = Dialect::get(DialectType::Snowflake) - .parse_with_config(&body, self.config.complexity_guard.clone(), mode) + .parse_with_config(&body, self.config.complexity_guard, mode) .map_err(|error| self.parse_error(format!("In the procedure body: {error}")))?; if statements.is_empty() { return Err(self.parse_error("Expected a procedure body")); From e46a434ce383477686899a0df8c7898532228689 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 17:30:39 +0000 Subject: [PATCH 07/10] Let unobservable entries record never-submitted cases An [[unobservable]] entry may set cause = "not_submitted" for cases that were never sent to Snowflake, such as side-effecting calls a capture guard holds. It has no runs or probes, and its obligation reports unverifiable_not_submitted instead of unverifiable_internal_error, kept in the denominator and never satisfied. An absent cause keeps today's 603 meaning; any other value is refused. --- .../src/check/_helpers/forms.rs | 5 ++- .../src/check/_helpers/join.rs | 12 +++++-- .../src/check/models.rs | 6 ++-- .../src/check/tests/helpers.rs | 16 +++++++++ .../src/check/tests/test_check_coverage.rs | 26 ++++++++++++++- .../src/formats/_helpers/records.rs | 31 ++++++++++++++++- .../src/formats/models.rs | 13 +++++++- .../src/formats/tests/test_load_coverage.rs | 33 +++++++++++++++++++ docs/syntax-coverage.md | 9 ++++- 9 files changed, 139 insertions(+), 12 deletions(-) diff --git a/crates/sqlquack-syntax-coverage/src/check/_helpers/forms.rs b/crates/sqlquack-syntax-coverage/src/check/_helpers/forms.rs index 966ec3695..281f94e1b 100644 --- a/crates/sqlquack-syntax-coverage/src/check/_helpers/forms.rs +++ b/crates/sqlquack-syntax-coverage/src/check/_helpers/forms.rs @@ -16,11 +16,10 @@ fn obligation_result(obligation: &Obligation, joined: &Joined) -> ObligationResu .iter() .filter_map(|case_id| joined.cases.get(case_id).cloned()) .collect(); + let unobservable: Option = joined.unobservable.get(obligation.id.as_str()).copied(); let reason: Option = match () { () if cases.is_empty() => Some(Reason::MissingCase), - () if joined.unobservable.contains(obligation.id.as_str()) => { - Some(Reason::UnverifiableInternalError) - } + () if unobservable.is_some() => unobservable, () => cases.iter().filter_map(|case| case.reason).min(), }; ObligationResult { diff --git a/crates/sqlquack-syntax-coverage/src/check/_helpers/join.rs b/crates/sqlquack-syntax-coverage/src/check/_helpers/join.rs index a63a49737..e643b4a99 100644 --- a/crates/sqlquack-syntax-coverage/src/check/_helpers/join.rs +++ b/crates/sqlquack-syntax-coverage/src/check/_helpers/join.rs @@ -3,9 +3,9 @@ use std::collections::BTreeMap; use crate::check::_helpers::receipts::case_result; -use crate::check::models::{CaseResult, Joined, LocalVerdict}; +use crate::check::models::{CaseResult, Joined, LocalVerdict, Reason}; use crate::formats::main::obligation_form::obligation_form; -use crate::formats::models::{Form, Obligation, Receipt, SyntaxCoverage}; +use crate::formats::models::{Form, Obligation, Receipt, SyntaxCoverage, UnobservableCause}; fn forms_by_id(coverage: &SyntaxCoverage) -> BTreeMap<&str, &Form> { coverage @@ -75,7 +75,13 @@ pub(crate) fn join<'a>( unobservable: coverage .unobservable .iter() - .map(|entry| entry.obligation.as_str()) + .map(|entry| { + let reason: Reason = match entry.cause { + Some(UnobservableCause::NotSubmitted) => Reason::UnverifiableNotSubmitted, + None => Reason::UnverifiableInternalError, + }; + (entry.obligation.as_str(), reason) + }) .collect(), } } diff --git a/crates/sqlquack-syntax-coverage/src/check/models.rs b/crates/sqlquack-syntax-coverage/src/check/models.rs index 0b35ed23a..199d0e69e 100644 --- a/crates/sqlquack-syntax-coverage/src/check/models.rs +++ b/crates/sqlquack-syntax-coverage/src/check/models.rs @@ -48,6 +48,8 @@ pub enum Reason { MissingCase, /// Unverifiable: Snowflake answers the construct only with an internal error (decision D87). UnverifiableInternalError, + /// Unverifiable: the cases are never submitted, such as side-effecting calls a guard holds. + UnverifiableNotSubmitted, /// A citing case has no receipt. MissingReceipt, /// A citing extreme case awaits Snowflake's answer (`expect = "observe"`). @@ -156,8 +158,8 @@ pub(crate) struct Joined<'a> { pub(crate) citing: BTreeMap<&'a str, Vec<&'a str>>, /// Each case's result by case id. pub(crate) cases: BTreeMap<&'a str, CaseResult>, - /// Obligations listed as unobservable, which never count as satisfied. - pub(crate) unobservable: BTreeSet<&'a str>, + /// Obligations listed as unobservable, with the reason they report; never satisfied. + pub(crate) unobservable: BTreeMap<&'a str, Reason>, } /// What every form of one family is judged against besides its own obligations. diff --git a/crates/sqlquack-syntax-coverage/src/check/tests/helpers.rs b/crates/sqlquack-syntax-coverage/src/check/tests/helpers.rs index 41ed7cb22..37ddc8d6e 100644 --- a/crates/sqlquack-syntax-coverage/src/check/tests/helpers.rs +++ b/crates/sqlquack-syntax-coverage/src/check/tests/helpers.rs @@ -381,6 +381,22 @@ pub(crate) fn unobservable(root: &Path, obligation: &str, case_id: &str, sql: &s ); } +/// Lists `obligation` as never submitted, citing `case_id` with the hash of `sql`, with `runs`. +pub(crate) fn not_submitted(root: &Path, obligation: &str, case_id: &str, sql: &str, runs: &str) { + write( + root, + "unobservable/query.toml", + &format!( + "[[unobservable]]\nobligation = \"{obligation}\"\nform = \"sf.query.select\"\n\ + construct = \"a construct\"\nreason = \"not submitted: held by the capture guard\"\n\ + cause = \"not_submitted\"\n\ + cases = [{{ case_id = \"{case_id}\", sql_sha256 = \"{}\" }}]\n\ + runs = [{runs}]\nprobes = []\n", + sha256(sql) + ), + ); +} + /// Adds a note to the obligation `id` of kind `kind`, an edit the obligations file alone sees. pub(crate) fn note_obligation(root: &Path, id: &str, kind: &str) { let declared: String = format!("id = \"{id}\"\nkind = \"{kind}\"\nrequired = true\n"); diff --git a/crates/sqlquack-syntax-coverage/src/check/tests/test_check_coverage.rs b/crates/sqlquack-syntax-coverage/src/check/tests/test_check_coverage.rs index 69b650df9..07004fc37 100644 --- a/crates/sqlquack-syntax-coverage/src/check/tests/test_check_coverage.rs +++ b/crates/sqlquack-syntax-coverage/src/check/tests/test_check_coverage.rs @@ -2,7 +2,7 @@ use crate::check::main::check_coverage::check_coverage; use crate::check::models::{FormState, Reason}; use crate::check::tests::helpers::{ OTHER_TREE, SETUP, TREE, append_receipt, case_in_zone, case_line, crashing, - crashing_on_select_from, drop_lines, exclusion_review, form, hanging, move_form, + crashing_on_select_from, drop_lines, exclusion_review, form, hanging, move_form, not_submitted, note_obligation, obligation, opaque, pin_definitions, pin_fingerprints, read, receipt_line, rejecting, replace, reshaped, rewrite_select_17, select_17, select_17_setup_sha256, sha256, structural, unobservable, verified_tree, with_fields, write, @@ -521,6 +521,30 @@ fn given_a_changed_tree_when_checking_then_the_form_fails_closed_with_its_reason expected_reason: Some(Reason::UnverifiableInternalError), expected_problem: None, }, + CheckTestCase { + description: "a never-submitted obligation whose case otherwise agrees", + mutate: |root| { + not_submitted(root, "sf.query.select.o001", "select-17", "SELECT 17", "") + }, + parse: structural, + parser_tree: Some(TREE), + form: "sf.query.select", + expected_state: FormState::Unverified, + expected_reason: Some(Reason::UnverifiableNotSubmitted), + expected_problem: None, + }, + CheckTestCase { + description: "a never-submitted entry whose case hash is stale", + mutate: |root| { + not_submitted(root, "sf.query.select.o001", "select-17", "SELECT 18", "") + }, + parse: structural, + parser_tree: Some(TREE), + form: "sf.query.select", + expected_state: FormState::Unverified, + expected_reason: Some(Reason::UnverifiableNotSubmitted), + expected_problem: Some("case select-17 is not a case with that sql_sha256"), + }, CheckTestCase { description: "an unobservable entry naming an undeclared obligation", mutate: |root| unobservable(root, "sf.query.select.o009", "select-17", "SELECT 17"), diff --git a/crates/sqlquack-syntax-coverage/src/formats/_helpers/records.rs b/crates/sqlquack-syntax-coverage/src/formats/_helpers/records.rs index b26dee81d..b1c1a1e53 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/_helpers/records.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/_helpers/records.rs @@ -16,7 +16,7 @@ use crate::formats::main::obligation_form::obligation_form; use crate::formats::models::{ Alias, CaptureMethod, Case, Definition, Diagnostic, Exclusion, Expect, Form, FormKind, FormStatus, Index, Obligation, ObligationKind, Phase, Receipt, Review, Source, SourceStatus, - Unobservable, UnobservableCase, UnobservableProbe, + Unobservable, UnobservableCase, UnobservableCause, UnobservableProbe, }; fn blank(text: &str) -> bool { @@ -567,6 +567,9 @@ pub(crate) fn check_review(review: &Review) -> Vec { } pub(crate) fn check_unobservable(entry: &Unobservable, family: &str) -> Vec { + if entry.cause == Some(UnobservableCause::NotSubmitted) { + return check_not_submitted(entry, family); + } check_record( &UnobservableRecord { obligation: &entry.obligation, @@ -598,6 +601,32 @@ pub(crate) fn check_diagnostic(entry: &Diagnostic, family: &str) -> Vec ) } +/// A never-submitted entry: a reason and cases, but no runs or probes, since nothing ran. +fn check_not_submitted(entry: &Unobservable, family: &str) -> Vec { + let id: &str = &entry.obligation; + let mut problems: Vec = Vec::new(); + if obligation_form(id, family) != Some(entry.form.as_str()) { + problems.push(format!( + "unobservable {id}: not an obligation of form {} in family {family}", + entry.form + )); + } + if blank(&entry.construct) || blank(&entry.reason) || entry.cases.is_empty() { + problems.push(format!( + "unobservable {id}: needs a construct, a reason and at least one case" + )); + } + if !entry.runs.is_empty() || !entry.probes.is_empty() { + problems.push(format!( + "unobservable {id}: a not_submitted entry has no runs or probes" + )); + } + if entry.cases.iter().any(|case| !is_sha256(&case.sql_sha256)) { + problems.push(format!("unobservable {id}: a sql_sha256 is not a SHA-256")); + } + problems +} + /// The fields an unobservable entry and a diagnostic share. struct UnobservableRecord<'a> { obligation: &'a str, diff --git a/crates/sqlquack-syntax-coverage/src/formats/models.rs b/crates/sqlquack-syntax-coverage/src/formats/models.rs index 6d6686de6..780613ceb 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/models.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/models.rs @@ -449,7 +449,7 @@ pub struct UnobservableFile { pub diagnostics: Vec, } -/// An obligation Snowflake answers only with an internal error; never verified (decision D87). +/// An obligation no capture can establish a phase for; never verified (decision D87). #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] #[serde(deny_unknown_fields)] pub struct Unobservable { @@ -457,6 +457,9 @@ pub struct Unobservable { pub form: String, pub construct: String, pub reason: String, + /// Why no phase exists; absent means Snowflake answered only with an internal error (603). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub cause: Option, /// The captured cases, kept in the cases file without a receipt. pub cases: Vec, /// The capture, re-run and probe runs; none of them imported. @@ -464,6 +467,14 @@ pub struct Unobservable { pub probes: Vec, } +/// Why an unobservable obligation has no phase, other than a 603 internal error. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum UnobservableCause { + /// The cases were never submitted, such as side-effecting calls a capture guard holds. + NotSubmitted, +} + /// A 603 record on a construct no obligation requires any more; listed, never scored. #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] #[serde(deny_unknown_fields)] diff --git a/crates/sqlquack-syntax-coverage/src/formats/tests/test_load_coverage.rs b/crates/sqlquack-syntax-coverage/src/formats/tests/test_load_coverage.rs index 1ae3b5ac1..32d26eb91 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/tests/test_load_coverage.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/tests/test_load_coverage.rs @@ -784,6 +784,39 @@ fn given_a_format_fault_when_loading_then_the_problem_is_named() { }, expected_problem: "unobservable sf.query.select.o001: a sql_sha256 is not a SHA-256", }, + FormatFaultTestCase { + description: "an unobservable entry with an unknown cause", + mutate: |root| { + write( + root, + "unobservable/query.toml", + "[[unobservable]]\nobligation = \"sf.query.select.o001\"\nform = \"sf.query.select\"\nconstruct = \"c\"\nreason = \"held\"\ncause = \"held_by_guard\"\ncases = [{ case_id = \"query.select-17\", sql_sha256 = \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\" }]\nruns = []\nprobes = []\n", + ) + }, + expected_problem: "unknown variant `held_by_guard`", + }, + FormatFaultTestCase { + description: "a never-submitted entry that lists a run", + mutate: |root| { + write( + root, + "unobservable/query.toml", + "[[unobservable]]\nobligation = \"sf.query.select.o001\"\nform = \"sf.query.select\"\nconstruct = \"c\"\nreason = \"held\"\ncause = \"not_submitted\"\ncases = [{ case_id = \"query.select-17\", sql_sha256 = \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\" }]\nruns = [\"run-1\"]\nprobes = []\n", + ) + }, + expected_problem: "unobservable sf.query.select.o001: a not_submitted entry has no runs or probes", + }, + FormatFaultTestCase { + description: "a 603 entry without runs", + mutate: |root| { + write( + root, + "unobservable/query.toml", + "[[unobservable]]\nobligation = \"sf.query.select.o001\"\nform = \"sf.query.select\"\nconstruct = \"c\"\nreason = \"603\"\ncases = [{ case_id = \"query.select-17\", sql_sha256 = \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\" }]\nruns = []\nprobes = []\n", + ) + }, + expected_problem: "unobservable sf.query.select.o001: needs at least one case and one run", + }, FormatFaultTestCase { description: "an unobservable entry without cases", mutate: |root| { diff --git a/docs/syntax-coverage.md b/docs/syntax-coverage.md index 1e8fbbb7d..0407c5a59 100644 --- a/docs/syntax-coverage.md +++ b/docs/syntax-coverage.md @@ -250,7 +250,14 @@ internal error (603), so no capture can establish a phase (decision D87). It has family; each listed case must exist with that `sql_sha256` and cite the obligation; an obligation is listed at most once. Such an obligation stays in the denominator, never counts as satisfied, and is reported as `unverifiable_internal_error` ("unverifiable: Snowflake internal error"). Probe results -are diagnostics, never syntax evidence. A `[[diagnostic]]` in the same file has the same fields, +are diagnostics, never syntax evidence. + +An entry may set `cause = "not_submitted"` when its cases were never sent to Snowflake, such as +side-effecting calls a capture guard holds. Such an entry has empty `runs` and `probes`, a +`reason` naming the rule that holds the cases, and at least one case, checked as above. The +obligation stays in the denominator, never counts as satisfied, and is reported as +`unverifiable_not_submitted`. An absent `cause` means a 603 internal error; any other value is +refused. A `[[diagnostic]]` in the same file has the same fields, with `captured` in place of `cases`: a 603 record on a construct no obligation requires any more, whose retired cases are not resolved against the cases file. Diagnostics are format-checked and listed, never scored. The checker ignores `semantic-limits/` and `lane-reviews/`. From 0da14d789d2e43a82bfe8a9a0be9f73ceb5ff986 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 18:07:52 +0000 Subject: [PATCH 08/10] Hash each form's reading into its definition and accept receipt account state A form's definition hash now covers its [[reading]] from limits.toml, so editing a reading stales that form's per-form review and no other. Forms without a reading keep their hash. limits.toml is required; a duplicate reading or one naming an uncatalogued form is refused. Receipts may carry an optional account_state (D120): account parameters by name, each with a value (or null) and a basis. It is validated, shown on each case result, and changes no verdict. --- .../src/check/_helpers/fingerprints.rs | 17 ++- .../src/check/_helpers/forms.rs | 3 +- .../src/check/_helpers/join.rs | 5 + .../src/check/_helpers/receipts.rs | 10 +- .../src/check/_helpers/references.rs | 5 + .../src/check/models.rs | 9 +- .../src/check/tests/helpers.rs | 10 ++ .../src/check/tests/test_check_coverage.rs | 114 +++++++++++++++++- .../src/check/tests/test_types.rs | 12 ++ .../src/formats/_helpers/records.rs | 48 +++++++- .../src/formats/_helpers/sections.rs | 33 ++++- .../src/formats/constants.rs | 2 + .../src/formats/main/load_coverage.rs | 1 + .../src/formats/models.rs | 36 ++++++ .../src/formats/tests/helpers.rs | 21 ++++ .../src/formats/tests/test_load_coverage.rs | 80 +++++++++++- docs/syntax-coverage.md | 18 ++- 17 files changed, 399 insertions(+), 25 deletions(-) diff --git a/crates/sqlquack-syntax-coverage/src/check/_helpers/fingerprints.rs b/crates/sqlquack-syntax-coverage/src/check/_helpers/fingerprints.rs index 38c04ff8b..a82981833 100644 --- a/crates/sqlquack-syntax-coverage/src/check/_helpers/fingerprints.rs +++ b/crates/sqlquack-syntax-coverage/src/check/_helpers/fingerprints.rs @@ -4,7 +4,7 @@ use std::collections::BTreeMap; use crate::check::models::{CaseResult, LocalVerdict, ObligationResult}; use crate::formats::main::content_sha256::content_sha256; -use crate::formats::models::{Form, Obligation}; +use crate::formats::models::{Form, Obligation, Reading}; /// The local verdict as fingerprint text: `accepted` with its tree hash, `rejected`, or the failure. fn verdict_text(local: &LocalVerdict) -> String { @@ -38,8 +38,15 @@ pub(crate) fn form_fingerprint(obligations: &[ObligationResult]) -> String { content_sha256(text.as_bytes()) } -/// SHA-256 of the form's catalogue entry and own obligations as compact JSON; empty if unserializable. -pub(crate) fn form_definition(form: &Form, obligations: &[&Obligation]) -> String { - serde_json::to_vec(&(form, obligations)) - .map_or_else(|_| String::new(), |bytes| content_sha256(&bytes)) +/// SHA-256 of the form's catalogue entry, own obligations and any reading as JSON; empty on failure. +pub(crate) fn form_definition( + form: &Form, + obligations: &[&Obligation], + reading: Option<&Reading>, +) -> String { + let bytes = match reading { + Some(reading) => serde_json::to_vec(&(form, obligations, reading)), + None => serde_json::to_vec(&(form, obligations)), + }; + bytes.map_or_else(|_| String::new(), |bytes| content_sha256(&bytes)) } diff --git a/crates/sqlquack-syntax-coverage/src/check/_helpers/forms.rs b/crates/sqlquack-syntax-coverage/src/check/_helpers/forms.rs index 281f94e1b..af4bd245b 100644 --- a/crates/sqlquack-syntax-coverage/src/check/_helpers/forms.rs +++ b/crates/sqlquack-syntax-coverage/src/check/_helpers/forms.rs @@ -61,7 +61,8 @@ pub(crate) fn form_result(form: &Form, family: &str, context: &FamilyContext) -> .map(|obligation| obligation_result(obligation, context.joined)) .collect(); let fingerprint: String = form_fingerprint(&obligations); - let definition_sha256: String = form_definition(form, declared); + let reading = context.joined.readings.get(form.id.as_str()).copied(); + let definition_sha256: String = form_definition(form, declared, reading); let review: Option = review_reason(&form.id, (&fingerprint, &definition_sha256), context); let reasons: Vec = form_reasons(&obligations, review); diff --git a/crates/sqlquack-syntax-coverage/src/check/_helpers/join.rs b/crates/sqlquack-syntax-coverage/src/check/_helpers/join.rs index e643b4a99..50f13d9eb 100644 --- a/crates/sqlquack-syntax-coverage/src/check/_helpers/join.rs +++ b/crates/sqlquack-syntax-coverage/src/check/_helpers/join.rs @@ -83,5 +83,10 @@ pub(crate) fn join<'a>( (entry.obligation.as_str(), reason) }) .collect(), + readings: coverage + .readings + .iter() + .map(|reading| (reading.form.as_str(), reading)) + .collect(), } } diff --git a/crates/sqlquack-syntax-coverage/src/check/_helpers/receipts.rs b/crates/sqlquack-syntax-coverage/src/check/_helpers/receipts.rs index 0513df16d..84f269362 100644 --- a/crates/sqlquack-syntax-coverage/src/check/_helpers/receipts.rs +++ b/crates/sqlquack-syntax-coverage/src/check/_helpers/receipts.rs @@ -1,9 +1,9 @@ //! One case's result: its receipts against the current parser verdict. -use std::collections::BTreeSet; +use std::collections::{BTreeMap, BTreeSet}; use crate::check::models::{CaseResult, LocalVerdict, Reason}; -use crate::formats::models::{Case, Expect, Phase, Receipt}; +use crate::formats::models::{AccountSetting, Case, Expect, Phase, Receipt}; /// A syntax receipt bound to exactly this case's SQL, setup, session and capture, with a query id. fn admissible(case: &Case, receipt: &Receipt) -> bool { @@ -84,5 +84,11 @@ pub(crate) fn case_result(case: &Case, receipts: &[&Receipt], local: LocalVerdic local, reason, semantic_only, + account_state: receipts + .iter() + .filter(|receipt| admissible(case, receipt)) + .flat_map(|receipt| receipt.account_state.iter().flatten()) + .map(|(name, setting)| (name.clone(), setting.clone())) + .collect::>(), } } diff --git a/crates/sqlquack-syntax-coverage/src/check/_helpers/references.rs b/crates/sqlquack-syntax-coverage/src/check/_helpers/references.rs index 4847e454a..7f702c4f3 100644 --- a/crates/sqlquack-syntax-coverage/src/check/_helpers/references.rs +++ b/crates/sqlquack-syntax-coverage/src/check/_helpers/references.rs @@ -77,6 +77,11 @@ pub(crate) fn reference_problems( } } } + for reading in &coverage.readings { + if !forms.contains_key(reading.form.as_str()) { + problems.push(format!("reading {}: form is not catalogued", reading.form)); + } + } let sources: BTreeSet<&str> = coverage .sources .iter() diff --git a/crates/sqlquack-syntax-coverage/src/check/models.rs b/crates/sqlquack-syntax-coverage/src/check/models.rs index 199d0e69e..405fc97b5 100644 --- a/crates/sqlquack-syntax-coverage/src/check/models.rs +++ b/crates/sqlquack-syntax-coverage/src/check/models.rs @@ -4,7 +4,9 @@ use std::collections::{BTreeMap, BTreeSet}; use serde::{Deserialize, Serialize}; -use crate::formats::models::{Form, Obligation, ObligationKind, Phase, Review}; +use crate::formats::models::{ + AccountSetting, Form, Obligation, ObligationKind, Phase, Reading, Review, +}; /// The current parser's verdict on one case's SQL. #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] @@ -87,6 +89,9 @@ pub struct CaseResult { pub reason: Option, /// Agrees, while Snowflake stopped after parsing (semantic, runtime or unsupported phase). pub semantic_only: bool, + /// The account parameters its admissible receipts ran under (decision D120). + #[serde(skip_serializing_if = "BTreeMap::is_empty")] + pub account_state: BTreeMap, } /// One obligation's result. @@ -160,6 +165,8 @@ pub(crate) struct Joined<'a> { pub(crate) cases: BTreeMap<&'a str, CaseResult>, /// Obligations listed as unobservable, with the reason they report; never satisfied. pub(crate) unobservable: BTreeMap<&'a str, Reason>, + /// Each form's reading by form id. + pub(crate) readings: BTreeMap<&'a str, &'a Reading>, } /// What every form of one family is judged against besides its own obligations. diff --git a/crates/sqlquack-syntax-coverage/src/check/tests/helpers.rs b/crates/sqlquack-syntax-coverage/src/check/tests/helpers.rs index 37ddc8d6e..e5b8b9547 100644 --- a/crates/sqlquack-syntax-coverage/src/check/tests/helpers.rs +++ b/crates/sqlquack-syntax-coverage/src/check/tests/helpers.rs @@ -316,6 +316,16 @@ pub(crate) fn verified_tree(root: &Path) { "receipts/run-1.jsonl", &format!("{}\n", receipts.join("\n")), ); + write( + root, + "limits.toml", + &format!( + "[[reading]]\nform = \"sf.query.select\"\ngrammar_sha256 = \"{SHA_A}\"\n\ + grammar = [\"SELECT read as one expression\"]\nreason = \"r\"\n\ + [[reading]]\nform = \"sf.query.select.top\"\ngrammar_sha256 = \"{SHA_A}\"\n\ + grammar = [\"SELECT TOP read as one count\"]\nreason = \"r\"\n" + ), + ); review(root); } diff --git a/crates/sqlquack-syntax-coverage/src/check/tests/test_check_coverage.rs b/crates/sqlquack-syntax-coverage/src/check/tests/test_check_coverage.rs index 07004fc37..7d835c496 100644 --- a/crates/sqlquack-syntax-coverage/src/check/tests/test_check_coverage.rs +++ b/crates/sqlquack-syntax-coverage/src/check/tests/test_check_coverage.rs @@ -7,7 +7,7 @@ use crate::check::tests::helpers::{ rejecting, replace, reshaped, rewrite_select_17, select_17, select_17_setup_sha256, sha256, structural, unobservable, verified_tree, with_fields, write, }; -use crate::check::tests::test_types::CheckTestCase; +use crate::check::tests::test_types::{AccountStateTestCase, CheckTestCase, Parameters}; use crate::formats::main::load_coverage::load_coverage; #[test] @@ -545,6 +545,23 @@ fn given_a_changed_tree_when_checking_then_the_form_fails_closed_with_its_reason expected_reason: Some(Reason::UnverifiableNotSubmitted), expected_problem: Some("case select-17 is not a case with that sql_sha256"), }, + CheckTestCase { + description: "a reading of a form that is not catalogued", + mutate: |root| { + replace( + root, + "limits.toml", + "form = \"sf.query.select.top\"", + "form = \"sf.query.tops\"", + ) + }, + parse: structural, + parser_tree: Some(TREE), + form: "sf.query.select", + expected_state: FormState::Verified, + expected_reason: None, + expected_problem: Some("reading sf.query.tops: form is not catalogued"), + }, CheckTestCase { description: "an unobservable entry naming an undeclared obligation", mutate: |root| unobservable(root, "sf.query.select.o009", "select-17", "SELECT 17"), @@ -1013,6 +1030,37 @@ fn given_a_review_binding_each_form_when_its_family_files_change_then_only_that_ expected_reason: Some(Reason::StaleReview), expected_problem: None, }, + CheckTestCase { + description: "the form's own reading edited after review", + mutate: |root| { + pin_definitions(root); + replace( + root, + "limits.toml", + "read as one expression", + "read as a list", + ); + }, + parse: structural, + parser_tree: Some(TREE), + form: "sf.query.select", + expected_state: FormState::Unverified, + expected_reason: Some(Reason::StaleReview), + expected_problem: None, + }, + CheckTestCase { + description: "another form's reading edited in the same limits file", + mutate: |root| { + pin_definitions(root); + replace(root, "limits.toml", "read as one count", "read as a list"); + }, + parse: structural, + parser_tree: Some(TREE), + form: "sf.query.select", + expected_state: FormState::Verified, + expected_reason: None, + expected_problem: None, + }, CheckTestCase { description: "a review binding the family's files still goes stale on another form's edit", mutate: |root| note_obligation(root, "sf.query.select.top.o001", "positive"), @@ -1057,3 +1105,67 @@ fn given_a_review_binding_each_form_when_its_family_files_change_then_only_that_ ); } } + +#[test] +fn given_a_receipt_with_account_state_when_checking_then_only_its_case_carries_it() { + let test_cases = [ + AccountStateTestCase { + description: "a parameter with its value", + account_state: ",\"account_state\":{\"CORTEX_MODELS_ALLOWLIST\":\ + {\"value\":\"NONE\",\"basis\":\"D120: set by sa-01\"}}", + expected_cases: vec![ + ("select-17", vec![("CORTEX_MODELS_ALLOWLIST", Some("NONE"))]), + ("select-from", vec![]), + ("select-top", vec![]), + ], + }, + AccountStateTestCase { + description: "a parameter whose value is null for want of a usable recorded_at", + account_state: ",\"account_state\":{\"CORTEX_MODELS_ALLOWLIST\":\ + {\"value\":null,\"basis\":\"D120: no usable recorded_at\"}}", + expected_cases: vec![ + ("select-17", vec![("CORTEX_MODELS_ALLOWLIST", None)]), + ("select-from", vec![]), + ("select-top", vec![]), + ], + }, + ]; + + for test_case in test_cases { + let directory = tempfile::tempdir().expect("temporary directory"); + verified_tree(directory.path()); + replace( + directory.path(), + "receipts/run-1.jsonl", + "\"phase_basis\":\"errno\"", + &format!("\"phase_basis\":\"errno\"{}", test_case.account_state), + ); + let coverage = load_coverage(directory.path()) + .unwrap_or_else(|problems| panic!("{}: {problems:#?}", test_case.description)); + let check = check_coverage(&coverage, Some(TREE), &structural); + let cases: Vec<(&str, Parameters)> = check + .forms + .iter() + .flat_map(|form| &form.obligations) + .flat_map(|obligation| &obligation.cases) + .map(|case| { + let state = case + .account_state + .iter() + .map(|(name, setting)| (name.as_str(), setting.value.as_deref())) + .collect(); + (case.case_id.as_str(), state) + }) + .collect(); + assert_eq!(cases, test_case.expected_cases, "{}", test_case.description); + assert!( + check + .forms + .iter() + .filter(|form| form.family == "query") + .all(|form| form.state == FormState::Verified), + "{}: account state is provenance and changes no verdict", + test_case.description + ); + } +} diff --git a/crates/sqlquack-syntax-coverage/src/check/tests/test_types.rs b/crates/sqlquack-syntax-coverage/src/check/tests/test_types.rs index 9336dc2ea..012001a06 100644 --- a/crates/sqlquack-syntax-coverage/src/check/tests/test_types.rs +++ b/crates/sqlquack-syntax-coverage/src/check/tests/test_types.rs @@ -17,3 +17,15 @@ pub(crate) struct CheckTestCase { /// A fragment of a reference problem; none when there must be no problem. pub(crate) expected_problem: Option<&'static str>, } + +/// Account parameter names with their values. +pub(crate) type Parameters<'a> = Vec<(&'a str, Option<&'a str>)>; + +/// A receipt `account_state` fragment and the account state each case must then carry. +pub(crate) struct AccountStateTestCase { + pub(crate) description: &'static str, + /// Inserted after the first receipt's `phase_basis`, the receipt of `select-17`. + pub(crate) account_state: &'static str, + /// Each case id with its parameter names and values, in case-id order. + pub(crate) expected_cases: Vec<(&'static str, Parameters<'static>)>, +} diff --git a/crates/sqlquack-syntax-coverage/src/formats/_helpers/records.rs b/crates/sqlquack-syntax-coverage/src/formats/_helpers/records.rs index b1c1a1e53..b4abf1d82 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/_helpers/records.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/_helpers/records.rs @@ -14,9 +14,9 @@ use crate::formats::constants::{ }; use crate::formats::main::obligation_form::obligation_form; use crate::formats::models::{ - Alias, CaptureMethod, Case, Definition, Diagnostic, Exclusion, Expect, Form, FormKind, - FormStatus, Index, Obligation, ObligationKind, Phase, Receipt, Review, Source, SourceStatus, - Unobservable, UnobservableCase, UnobservableCause, UnobservableProbe, + AccountSetting, Alias, CaptureMethod, Case, Definition, Diagnostic, Exclusion, Expect, Form, + FormKind, FormStatus, Index, Obligation, ObligationKind, Phase, Reading, Receipt, Review, + Source, SourceStatus, Unobservable, UnobservableCase, UnobservableCause, UnobservableProbe, }; fn blank(text: &str) -> bool { @@ -354,7 +354,31 @@ pub(crate) fn check_receipt(receipt: &Receipt) -> Vec { receipt.inner_message.as_ref(), receipt.dcm_project.as_ref(), ]; - if text.iter().flatten().any(|field| identifying(field)) { + let settings: Vec<(&String, &AccountSetting)> = + receipt.account_state.iter().flatten().collect(); + if receipt + .account_state + .as_ref() + .is_some_and(BTreeMap::is_empty) + || settings.iter().any(|(name, setting)| { + blank(name) || setting.value.as_deref().is_some_and(blank) || blank(&setting.basis) + }) + { + problems.push(format!( + "receipt {id}: account_state names at least one parameter, each with a value and basis" + )); + } + let account_text = settings + .iter() + .flat_map(|(name, setting)| [Some(*name), setting.value.as_ref(), Some(&setting.basis)]) + .flatten(); + if text + .iter() + .flatten() + .copied() + .chain(account_text) + .any(|field| identifying(field)) + { problems.push(format!( "receipt {id}: holds an account, host, URL or address" )); @@ -714,3 +738,19 @@ pub(crate) fn check_restatements<'a>( ); problems } + +/// A reading pins the page grammar it reads and gives a non-empty reading and reason. +pub(crate) fn check_reading(reading: &Reading) -> Vec { + let id: &str = &reading.form; + let mut problems: Vec = Vec::new(); + if !is_sha256(&reading.grammar_sha256) { + problems.push(format!("reading {id}: grammar_sha256 is not a SHA-256")); + } + if reading.grammar.is_empty() + || reading.grammar.iter().any(|line| blank(line)) + || blank(&reading.reason) + { + problems.push(format!("reading {id}: needs a grammar and a reason")); + } + problems +} diff --git a/crates/sqlquack-syntax-coverage/src/formats/_helpers/sections.rs b/crates/sqlquack-syntax-coverage/src/formats/_helpers/sections.rs index 73811f3a5..07b222ef4 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/_helpers/sections.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/_helpers/sections.rs @@ -8,18 +8,18 @@ use crate::formats::_helpers::files::{ }; use crate::formats::_helpers::records::{ check_alias, check_case, check_definition, check_diagnostic, check_form, check_index, - check_obligation, check_receipt, check_restatements, check_review, check_source, + check_obligation, check_reading, check_receipt, check_restatements, check_review, check_source, check_unobservable, }; use crate::formats::_helpers::retired::{retire, withdrawn_receipts}; use crate::formats::constants::{ - ALIASES_FILE, CASES_DIR, CATALOGUE_DIR, INDEXES_FILE, OBLIGATIONS_DIR, RECEIPT_REPORT_SUFFIX, - RECEIPTS_DIR, REVIEWS_DIR, SOURCES_FILE, UNOBSERVABLE_DIR, + ALIASES_FILE, CASES_DIR, CATALOGUE_DIR, INDEXES_FILE, LIMITS_FILE, OBLIGATIONS_DIR, + RECEIPT_REPORT_SUFFIX, RECEIPTS_DIR, REVIEWS_DIR, SOURCES_FILE, UNOBSERVABLE_DIR, }; use crate::formats::models::{ - Alias, AliasesFile, Case, CatalogueFile, Diagnostic, Index, IndexesFile, Obligation, - ObligationsFile, Receipt, Review, ReviewsFile, Source, SourcesFile, Unobservable, - UnobservableFile, + Alias, AliasesFile, Case, CatalogueFile, Diagnostic, Index, IndexesFile, LimitsFile, + Obligation, ObligationsFile, Reading, Receipt, Review, ReviewsFile, Source, SourcesFile, + Unobservable, UnobservableFile, }; use crate::formats::types::FamilyCatalogue; @@ -249,6 +249,24 @@ pub(crate) fn load_unobservable(root: &Path) -> Section<(Vec, Vec< Section::of((records, diagnostics), Vec::new(), problems) } +/// The readings in `limits.toml`, which must exist: without it no form's definition holds its reading. +pub(crate) fn load_readings(root: &Path) -> Section> { + match read_toml::(&root.join(LIMITS_FILE)) { + Ok((file, _)) => { + let mut problems: Vec = file.readings.iter().flat_map(check_reading).collect(); + let mut seen: BTreeSet<&str> = BTreeSet::new(); + problems.extend( + file.readings + .iter() + .filter(|reading| !seen.insert(reading.form.as_str())) + .map(|reading| format!("reading {}: listed twice", reading.form)), + ); + Section::of(file.readings, Vec::new(), problems) + } + Err(problem) => Section::of(Vec::new(), Vec::new(), vec![problem]), + } +} + pub(crate) fn load_reviews(root: &Path) -> Section> { let (paths, others) = listed(&root.join(REVIEWS_DIR), "toml"); let mut problems: Vec = others.iter().map(|path| stray(path, "toml")).collect(); @@ -277,6 +295,7 @@ pub(crate) struct Sections { pub(crate) receipts: Section<(Vec, Vec)>, pub(crate) reviews: Section>, pub(crate) unobservable: Section<(Vec, Vec)>, + pub(crate) readings: Section>, } impl Sections { @@ -292,6 +311,7 @@ impl Sections { &self.receipts.problems, &self.reviews.problems, &self.unobservable.problems, + &self.readings.problems, ] .into_iter() .flatten() @@ -327,6 +347,7 @@ pub(crate) fn load_sections(root: &Path) -> Sections { receipts: load_receipts(root), reviews: load_reviews(root), unobservable: load_unobservable(root), + readings: load_readings(root), sources, } } diff --git a/crates/sqlquack-syntax-coverage/src/formats/constants.rs b/crates/sqlquack-syntax-coverage/src/formats/constants.rs index dc9c46c56..a8b13e9a0 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/constants.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/constants.rs @@ -20,6 +20,8 @@ pub const RECEIPTS_DIR: &str = "receipts"; pub const REVIEWS_DIR: &str = "reviews"; /// Obligations Snowflake answers only with an internal error, one optional TOML file per family. pub const UNOBSERVABLE_DIR: &str = "unobservable"; +/// Hand-added limits, suppressions and grammar readings; the checker loads the readings. +pub const LIMITS_FILE: &str = "limits.toml"; /// Every family, each with a catalogue, an obligation and a case file. pub const FAMILIES: [&str; 10] = [ "query", diff --git a/crates/sqlquack-syntax-coverage/src/formats/main/load_coverage.rs b/crates/sqlquack-syntax-coverage/src/formats/main/load_coverage.rs index ff2c233ee..32e8c851f 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/main/load_coverage.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/main/load_coverage.rs @@ -36,6 +36,7 @@ pub fn load_coverage(root: &Path) -> Result> { reviews: sections.reviews.records, unobservable: sections.unobservable.records.0, diagnostics: sections.unobservable.records.1, + readings: sections.readings.records, file_hashes, }) } diff --git a/crates/sqlquack-syntax-coverage/src/formats/models.rs b/crates/sqlquack-syntax-coverage/src/formats/models.rs index 780613ceb..97db6155b 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/models.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/models.rs @@ -343,6 +343,9 @@ pub struct Receipt { /// The position the inner error points at, when its text gives one. #[serde(default)] pub definition_position: Option, + /// Account parameters the capture ran under, by name (decision D120); provenance, never phase. + #[serde(default)] + pub account_state: Option>, } #[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)] @@ -449,6 +452,37 @@ pub struct UnobservableFile { pub diagnostics: Vec, } +/// One account parameter a receipt's capture ran under, and why it held that value. +#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +pub struct AccountSetting { + /// Null when the receipt has no usable `recorded_at` to read the value at. + pub value: Option, + pub basis: String, +} + +/// `limits.toml`. The checker reads only the readings; the obligation builder owns the other tables. +#[derive(Clone, Debug, PartialEq, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +pub struct LimitsFile { + #[serde(default, rename = "limit")] + pub limits: Vec, + #[serde(default, rename = "suppress")] + pub suppressions: Vec, + #[serde(default, rename = "reading")] + pub readings: Vec, +} + +/// How a form's page grammar is read where it is ambiguous; part of the form's definition (D118). +#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +pub struct Reading { + pub form: String, + pub grammar_sha256: String, + pub grammar: Vec, + pub reason: String, +} + /// An obligation no capture can establish a phase for; never verified (decision D87). #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] #[serde(deny_unknown_fields)] @@ -523,6 +557,8 @@ pub struct SyntaxCoverage { pub reviews: Vec, pub unobservable: Vec, pub diagnostics: Vec, + /// Each form's reading from `limits.toml`, hashed into its definition. + pub readings: Vec, /// SHA-256 of the sources, aliases, catalogue and obligation files, by relative path. pub file_hashes: BTreeMap, } diff --git a/crates/sqlquack-syntax-coverage/src/formats/tests/helpers.rs b/crates/sqlquack-syntax-coverage/src/formats/tests/helpers.rs index 110a1290b..a0183cb23 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/tests/helpers.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/tests/helpers.rs @@ -74,6 +74,17 @@ pub(crate) fn receipt_line() -> String { ) } +/// Where a receipt line takes an `account_state` field. +pub(crate) const ACCOUNT_STATE_AT: &str = "\"phase_basis\":\"ran\""; + +/// `ACCOUNT_STATE_AT` followed by one account parameter whose basis is `basis`. +pub(crate) fn account_state(basis: &str) -> String { + format!( + "{ACCOUNT_STATE_AT},\"account_state\":{{\"CORTEX_MODELS_ALLOWLIST\":\ + {{\"basis\":\"{basis}\",\"value\":\"NONE\"}}}}" + ) +} + /// `receipt_line()` as a failed DCM PLAN of its case, its inner error ending the whole message. pub(crate) fn dcm_line(inner_message: &str, position: &str) -> String { let message: String = format!( @@ -160,6 +171,16 @@ pub(crate) fn valid_tree(root: &Path) { ), ); write(root, "cases/query.jsonl", &format!("{}\n", case_line())); + write( + root, + "limits.toml", + &format!( + "[[limit]]\nform = \"sf.query.select\"\nkind = \"boundary_above\"\n\ + [[suppress]]\nform = \"sf.query.select\"\nreason = \"r\"\n\ + [[reading]]\nform = \"sf.query.select\"\ngrammar_sha256 = \"{SHA_A}\"\n\ + grammar = [\"SELECT \"]\nreason = \"r\"\n" + ), + ); write( root, "receipts/first.jsonl", diff --git a/crates/sqlquack-syntax-coverage/src/formats/tests/test_load_coverage.rs b/crates/sqlquack-syntax-coverage/src/formats/tests/test_load_coverage.rs index 32d26eb91..475bdd3c4 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/tests/test_load_coverage.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/tests/test_load_coverage.rs @@ -4,8 +4,8 @@ use std::path::Path; use crate::formats::constants::{COVERAGE_ROOT, FAMILIES}; use crate::formats::main::load_coverage::load_coverage; use crate::formats::tests::helpers::{ - dcm_line, line_sha256, receipt_line, replace, restated_page, superseding_line, valid_tree, - write, + ACCOUNT_STATE_AT, account_state, dcm_line, line_sha256, receipt_line, replace, restated_page, + superseding_line, valid_tree, write, }; use crate::formats::tests::test_types::{ CommittedTestCase, FormatFaultTestCase, LoadTestCase, LoadedCounts, @@ -234,6 +234,18 @@ fn given_a_valid_tree_when_loading_then_every_record_is_kept() { ..counts(0) }, }, + LoadTestCase { + description: "a receipt recording the account parameter its capture ran under", + tree: |root| { + valid_tree(root); + write( + root, + "receipts/first.jsonl", + &receipt_line().replace(ACCOUNT_STATE_AT, &account_state("D120")), + ); + }, + expected_counts: counts(1), + }, LoadTestCase { description: "a tree without renames or reviews yet", tree: |root| { @@ -755,6 +767,32 @@ fn given_a_format_fault_when_loading_then_the_problem_is_named() { }, expected_problem: "query-1: declared twice", }, + FormatFaultTestCase { + description: "no limits.toml, so no form's reading can be hashed", + mutate: |root| std::fs::remove_file(root.join("limits.toml")).expect("remove"), + expected_problem: "limits.toml: No such file", + }, + FormatFaultTestCase { + description: "a form read twice", + mutate: |root| { + let limits = std::fs::read_to_string(root.join("limits.toml")).expect("read"); + let reading = &limits[limits.find("[[reading]]").expect("reading")..]; + write(root, "limits.toml", &format!("{limits}{reading}")); + }, + expected_problem: "reading sf.query.select: listed twice", + }, + FormatFaultTestCase { + description: "a misspelt table in limits.toml", + mutate: |root| { + let limits = std::fs::read_to_string(root.join("limits.toml")).expect("read"); + write( + root, + "limits.toml", + &limits.replace("[[reading]]", "[[readings]]"), + ); + }, + expected_problem: "unknown field `readings`", + }, FormatFaultTestCase { description: "an unobservable obligation of another form", mutate: |root| { @@ -997,6 +1035,44 @@ fn given_a_format_fault_when_loading_then_the_problem_is_named() { }, expected_problem: "cases/query.jsonl:1:", }, + FormatFaultTestCase { + description: "an account parameter without its basis", + mutate: |root| { + write( + root, + "receipts/first.jsonl", + &receipt_line().replace(ACCOUNT_STATE_AT, &account_state(" ")), + ) + }, + expected_problem: "receipt query.select-17: account_state names at least one parameter", + }, + FormatFaultTestCase { + description: "an account parameter whose basis holds an address", + mutate: |root| { + write( + root, + "receipts/first.jsonl", + &receipt_line().replace( + ACCOUNT_STATE_AT, + &account_state("set by someone@example.com"), + ), + ) + }, + expected_problem: "receipt query.select-17: holds an account, host, URL or address", + }, + FormatFaultTestCase { + description: "an account parameter with an unknown field", + mutate: |root| { + write( + root, + "receipts/first.jsonl", + &receipt_line() + .replace(ACCOUNT_STATE_AT, &account_state("D120")) + .replace("\"value\":\"NONE\"", "\"value\":\"NONE\",\"scope\":\"x\""), + ) + }, + expected_problem: "unknown field `scope`", + }, FormatFaultTestCase { description: "a receipt holding an address", mutate: |root| { diff --git a/docs/syntax-coverage.md b/docs/syntax-coverage.md index 0407c5a59..1075a43cf 100644 --- a/docs/syntax-coverage.md +++ b/docs/syntax-coverage.md @@ -108,6 +108,12 @@ placeholder. Add prose-documented limits by hand, with their anchor. Where Snowf limit, the obligation's `note` says `no documented bound`, and its `depths` list the depths that were tested. Nothing claims to be exhaustive. +`limits.toml` holds the hand-added `[[limit]]` and `[[suppress]]` rows the obligation builder +reads, and the `[[reading]]` rows: how an ambiguous page grammar is read, with `form`, +`grammar_sha256`, `grammar` (a non-empty list of lines) and `reason`. The checker requires the +file, refuses any other table, a reading of a form that is not catalogued, and a form read twice, +and hashes each form's reading into its definition (below). + ## Cases (`cases/.jsonl`) Each line is one JSON object. Blank lines are refused. @@ -168,6 +174,10 @@ committed one. `statements_in_request`, `completed_statements`, `after_sql` and `category`. `completed_statements` is the number of statements in the request that completed before it stopped. It is provenance only and does not change the phase. +- Optional `account_state`: the account parameters the capture ran under, by name, each as + `{ "value": …, "basis": … }` with both non-empty, such as + `{ "CORTEX_MODELS_ALLOWLIST": { "value": "NONE", "basis": "D120: …" } }` (decision D120). It + is provenance only and does not change the phase; when present it names at least one parameter. - Optional `reason_category` and `reason_category_basis`, always together, and only on a phase after parsing (`semantic_error`, `runtime_error` or `unsupported_feature`). The category is one of `missing_object`, `invalid_option_or_value`, `type` or `other`. The basis says what the @@ -289,8 +299,9 @@ A `[[review]]` has these fields: hash when it was reviewed, as `{ "
" = "" }`, one for every listed form. A review with `definitions` binds each form by its definition and fingerprint alone and has no `catalogue_sha256`, `obligations_sha256` or exclusions. An edit to another form in the same - catalogue or obligations file leaves it current; any change to the form's own catalogue entry, - obligations or fingerprint makes it stale for that form (decision D118). + catalogue or obligations file, or to another form's reading, leaves it current; any change to + the form's own catalogue entry, obligations, reading or fingerprint makes it stale for that form + (decision D118). - `[[review.exclusion]]`: exactly one of `form` (a form id of the review's family) or `source` (a page id, used for out-of-scope and syntax-less pages), plus a `reason`. A page review excludes pages only, and a family review excludes forms only. @@ -335,7 +346,8 @@ followed by a tab. `family --json` prints each form's `fingerprint`. A parser ch every case of a form parsing into the same tree, and every rejection still a rejection, leaves its fingerprint and so its review unchanged; a changed tree or verdict, or an added or removed case, changes it. A form's definition hash is the SHA-256 of the compact JSON array of its catalogue -entry and its own obligations in file order, as loaded; `family --json` prints it as +entry and its own obligations in file order, as loaded, followed by its `[[reading]]` from +`limits.toml` when it has one; `family --json` prints it as `definition_sha256`. A form is excluded only by an approved review that is current in the same way. Absent forms keep their ids and are never counted. The headline denominator is the catalogued forms: 3,348 on the frozen catalogue `e863d7de` (decision D81). From 75a66c759588a3ddaffc7911ea4766cb5c8c33dc Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 18:17:15 +0000 Subject: [PATCH 09/10] Accept prose and lexical readings A reading's grammar_sha256 may be a prose heading, as its form's is, and a lexical form's reading may have no grammar lines when its reason explains it. 54 readings in the catalogue have one or both. --- .../src/formats/_helpers/records.rs | 21 ++++++++++++------- docs/syntax-coverage.md | 3 ++- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/crates/sqlquack-syntax-coverage/src/formats/_helpers/records.rs b/crates/sqlquack-syntax-coverage/src/formats/_helpers/records.rs index b4abf1d82..55f726692 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/_helpers/records.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/_helpers/records.rs @@ -739,18 +739,23 @@ pub(crate) fn check_restatements<'a>( problems } -/// A reading pins the page grammar it reads and gives a non-empty reading and reason. +/// A reading pins the page grammar it reads, by hash or prose heading, and gives its reason. pub(crate) fn check_reading(reading: &Reading) -> Vec { let id: &str = &reading.form; let mut problems: Vec = Vec::new(); - if !is_sha256(&reading.grammar_sha256) { - problems.push(format!("reading {id}: grammar_sha256 is not a SHA-256")); + let prose: bool = reading + .grammar_sha256 + .strip_prefix(PROSE_HASH_PREFIX) + .is_some_and(|heading| !blank(heading)); + if !prose && !is_sha256(&reading.grammar_sha256) { + problems.push(format!( + "reading {id}: grammar_sha256 is neither a SHA-256 nor a prose heading" + )); } - if reading.grammar.is_empty() - || reading.grammar.iter().any(|line| blank(line)) - || blank(&reading.reason) - { - problems.push(format!("reading {id}: needs a grammar and a reason")); + if reading.grammar.iter().any(|line| blank(line)) || blank(&reading.reason) { + problems.push(format!( + "reading {id}: needs a reason and no blank grammar line" + )); } problems } diff --git a/docs/syntax-coverage.md b/docs/syntax-coverage.md index 1075a43cf..ef55e59e2 100644 --- a/docs/syntax-coverage.md +++ b/docs/syntax-coverage.md @@ -110,7 +110,8 @@ were tested. Nothing claims to be exhaustive. `limits.toml` holds the hand-added `[[limit]]` and `[[suppress]]` rows the obligation builder reads, and the `[[reading]]` rows: how an ambiguous page grammar is read, with `form`, -`grammar_sha256`, `grammar` (a non-empty list of lines) and `reason`. The checker requires the +`grammar_sha256` (the form's grammar hash, or `prose:` for a prose form), `grammar` (its +lines, none blank; empty for a lexical form the `reason` alone explains) and `reason`. The checker requires the file, refuses any other table, a reading of a form that is not catalogued, and a form read twice, and hashes each form's reading into its definition (below). From 7a7a1b2a91279a11821fe7c9ed8ce422ad8e01e6 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 18:52:31 +0000 Subject: [PATCH 10/10] Explain the polyglot differences the parser fixes introduce Unary plus is now a kept_node entry: the trees must match once SQLQuack's pos wrappers are removed. Snowflake-only rejections cover a CTE without AS, a reserved CTE or TRUNCATE name, an empty named argument, and the reserved NOT in a typed-expression type, each pinned by a parser test. --- .../polyglot-check/expected_differences.toml | 54 ++++++- .../src/cli/_helpers/report.rs | 3 +- .../src/expected/_helpers/mod.rs | 1 + .../src/expected/_helpers/nodes.rs | 54 +++++++ .../src/expected/constants.rs | 2 + .../src/expected/main/load_expected.rs | 1 + .../src/expected/models.rs | 29 ++++ .../src/runs/main/run_corpus.rs | 16 +- .../src/runs/models.rs | 2 + crates/sqlquack-polyglot-check/tests/runs.rs | 2 + .../tests/runs/test_exact.rs | 8 +- .../tests/runs/test_kept_nodes.rs | 139 ++++++++++++++++++ .../tests/runs/test_types.rs | 7 + .../src/parser/tests/test_snowflake.rs | 30 ++++ .../parser/tests/test_typed_expressions.rs | 6 + docs/syntax.md | 6 + 16 files changed, 351 insertions(+), 9 deletions(-) create mode 100644 crates/sqlquack-polyglot-check/src/expected/_helpers/nodes.rs create mode 100644 crates/sqlquack-polyglot-check/tests/runs/test_kept_nodes.rs diff --git a/corpora/polyglot-check/expected_differences.toml b/corpora/polyglot-check/expected_differences.toml index 994318bd5..cda7baea5 100644 --- a/corpora/polyglot-check/expected_differences.toml +++ b/corpora/polyglot-check/expected_differences.toml @@ -782,10 +782,52 @@ note = "Snowflake accepts a bare trailing LIMIT (recorded as accepted) and reads [[rejection]] id = "malformed_typed_type_text" sql = "SELECT __polyglot_typed(amount, 'NOT A TYPE (') FROM orders" -dialects = ["Snowflake", "DuckDB"] +dialects = ["DuckDB"] sqlquack_error = "Unexpected token after data type" note = "malformed typed-expression type text errors instead of degrading to an ordinary call" +[[rejection]] +id = "snowflake_reserved_type_name" +sql = "SELECT __polyglot_typed(amount, 'NOT A TYPE (') FROM orders" +dialects = ["Snowflake"] +sqlquack_error = "Reserved word NOT cannot be an unquoted type name" +note = "the same malformed type text fails earlier in Snowflake, where NOT is reserved and cannot name a type" + +[[rejection]] +id = "snowflake_cte_without_as" +sql = "WITH t (SELECT 1 AS c) SELECT c FROM t" +dialects = ["Snowflake"] +sqlquack_error = "Expected AS before the CTE body" +note = "Snowflake needs AS between a CTE's name and its body: it refuses WITH v1 ( SELECT 1 ) SELECT 1 with syntax error 1003 (case sf.query.with.select.k8b29420d51a4)" + +[[rejection]] +id = "snowflake_cte_without_as_prose" +sql = " with the correct AST shape." +dialects = ["Snowflake"] +sqlquack_error = "Expected AS before the CTE body" +note = "prose read as a CTE without AS; polyglot fails here too, with another error" + +[[rejection]] +id = "snowflake_reserved_cte_name" +sql = "WITH all AS (SELECT 1 AS count) SELECT all.count FROM all" +dialects = ["Snowflake"] +sqlquack_error = "Reserved word ALL cannot be an unquoted object name" +note = "ALL is a Snowflake reserved word, so it cannot name a CTE unquoted; Snowflake refuses WITH SELECT AS ( SELECT 1 ) SELECT 1 with syntax error 1003" + +[[rejection]] +id = "snowflake_reserved_truncate_target" +sql = "TRUNCATE TABLE table" +dialects = ["Snowflake"] +sqlquack_error = "Reserved word TABLE cannot be an unquoted object name" +note = "TABLE is a Snowflake reserved word, so it cannot name a table unquoted" + +[[rejection]] +id = "snowflake_empty_call_argument" +sql = "SELECT ROUND(SCALE => 1, EXPR => 2.25, , ROUNDING_MODE => 'HALF_TO_EVEN') AS value" +dialects = ["Snowflake"] +sqlquack_error = "Expected a function argument between commas" +note = "Snowflake refuses an empty argument between commas with syntax error 1003; polyglot drops it" + [[rejection]] id = "snowflake_bare_values_cte" sql = "WITH RECURSIVE T(n) AS (VALUES (1) UNION ALL SELECT n + 1 FROM t WHERE n < 100) SELECT SUM(n) FROM t" @@ -895,6 +937,16 @@ id = "star_exclude_quoted_name" marker = "EXCLUDE(\"" note = "SELECT * EXCLUDE (\"note\") excludes the case-sensitive column note, not NOTE; polyglot drops the quotes of a star's EXCLUDE names" +# Wrapper nodes SQLQuack keeps where polyglot drops them. The check counts an entry only when the +# statement, without whitespace and ignoring case, contains `marker`, and both trees are equal once +# every `{ "": { "this": … } }` in SQLQuack's tree is replaced by its operand. + +[[kept_node]] +id = "unary_plus" +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" + # 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 f69f25448..bbe376297 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, {} structured, {} differences, {} without lossless \ + "{label}: {} entries, {} same trees, {} same errors, {} fixed, {} rejected, {} kept quoting, {} kept nodes, {} 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", @@ -35,6 +35,7 @@ pub(crate) fn report(label: &str, summary: &RunSummary, counts_only: bool) -> St summary.fixed.values().sum::(), summary.rejected.values().sum::(), summary.kept_quoting.values().sum::(), + summary.kept_nodes.values().sum::(), summary.structured.values().sum::(), summary.differences.len(), summary.tokens_lost.len(), diff --git a/crates/sqlquack-polyglot-check/src/expected/_helpers/mod.rs b/crates/sqlquack-polyglot-check/src/expected/_helpers/mod.rs index 1383efda7..d78aaa9ab 100644 --- a/crates/sqlquack-polyglot-check/src/expected/_helpers/mod.rs +++ b/crates/sqlquack-polyglot-check/src/expected/_helpers/mod.rs @@ -1 +1,2 @@ +pub(crate) mod nodes; pub(crate) mod quoting; diff --git a/crates/sqlquack-polyglot-check/src/expected/_helpers/nodes.rs b/crates/sqlquack-polyglot-check/src/expected/_helpers/nodes.rs new file mode 100644 index 000000000..4757e2736 --- /dev/null +++ b/crates/sqlquack-polyglot-check/src/expected/_helpers/nodes.rs @@ -0,0 +1,54 @@ +//! Trees that differ only by a wrapper node SQLQuack keeps and polyglot drops. + +use serde_json::Value; + +use crate::expected::constants::{OK_PREFIX, THIS_KEY}; + +/// Whether both rendered parses are trees that are equal once SQLQuack's `node` wrappers go. +pub(crate) fn differ_only_by_kept_node(polyglot: &str, sqlquack: &str, node: &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_node(ours, node) == theirs, + (Err(_), _) | (_, Err(_)) => false, + } +} + +/// `value` with every `{ node: { "this": inner } }` replaced by `inner`. +fn without_node(value: Value, node: &str) -> Value { + match value { + Value::Object(mut object) => { + let wrapped: bool = object.len() == 1 + && object + .get(node) + .and_then(Value::as_object) + .is_some_and(|inner| inner.len() == 1 && inner.contains_key(THIS_KEY)); + match object.remove(node).filter(|_| wrapped) { + Some(Value::Object(mut inner)) => { + without_node(inner.remove(THIS_KEY).unwrap_or(Value::Null), node) + } + Some(other) => other, + None => Value::Object( + object + .into_iter() + .map(|(key, value)| (key, without_node(value, node))) + .collect(), + ), + } + } + Value::Array(items) => Value::Array( + items + .into_iter() + .map(|value| without_node(value, node)) + .collect(), + ), + other => other, + } +} diff --git a/crates/sqlquack-polyglot-check/src/expected/constants.rs b/crates/sqlquack-polyglot-check/src/expected/constants.rs index 029924ec4..e2300c7c0 100644 --- a/crates/sqlquack-polyglot-check/src/expected/constants.rs +++ b/crates/sqlquack-polyglot-check/src/expected/constants.rs @@ -8,5 +8,7 @@ pub const OK_PREFIX: &str = "ok:"; pub const COMMAND_TREE: &str = "[{\"command\":"; /// The field naming an identifier in a serialised tree. pub const NAME_KEY: &str = "name"; +/// The field holding the operand of a wrapper node such as unary plus. +pub const THIS_KEY: &str = "this"; /// The field recording whether an identifier was quoted. pub const QUOTED_KEY: &str = "quoted"; 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 d28bb7977..41c4f695d 100644 --- a/crates/sqlquack-polyglot-check/src/expected/main/load_expected.rs +++ b/crates/sqlquack-polyglot-check/src/expected/main/load_expected.rs @@ -14,6 +14,7 @@ pub fn load_expected(path: &Path) -> Result { fixes: file.fixes, rejections: file.rejections, kept_quotings: file.kept_quotings, + kept_nodes: file.kept_nodes, 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 329f7355e..2b3ea5a43 100644 --- a/crates/sqlquack-polyglot-check/src/expected/models.rs +++ b/crates/sqlquack-polyglot-check/src/expected/models.rs @@ -1,5 +1,6 @@ //! Expected differences. +use crate::expected::_helpers::nodes::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}; @@ -22,6 +23,9 @@ pub struct ExpectedFile { /// One entry per reviewed name whose quoting polyglot drops and SQLQuack keeps. #[serde(default, rename = "kept_quoting")] pub kept_quotings: Vec, + /// One entry per reviewed wrapper node SQLQuack keeps and polyglot drops. + #[serde(default, rename = "kept_node")] + pub kept_nodes: Vec, /// One entry per reviewed statement polyglot keeps as a command and SQLQuack structures. #[serde(default, rename = "structured")] pub structured: Vec, @@ -53,6 +57,20 @@ pub struct KeptQuoting { pub note: String, } +/// A statement both parsers parse, where SQLQuack keeps a wrapper node, such as unary plus, polyglot drops. +#[derive(Clone, Debug, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct KeptNode { + /// A lower-case identifier. + pub id: String, + /// Text the statement contains, with whitespace removed and ignoring case, such as `+`. + pub marker: String, + /// The wrapper node SQLQuack keeps, such as `pos`; its only field is `this`. + pub node: String, + /// Why SQLQuack keeps it; without its wrappers the trees may differ in nothing. + 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)] @@ -106,6 +124,8 @@ pub struct ExpectedDifferences { pub rejections: Vec, /// The reviewed names whose quoting polyglot drops. pub kept_quotings: Vec, + /// The reviewed wrapper nodes SQLQuack keeps and polyglot drops. + pub kept_nodes: Vec, /// The reviewed statements polyglot keeps as a command and SQLQuack structures. pub structured: Vec, } @@ -159,6 +179,15 @@ impl ExpectedDifferences { .filter(|_| differ_only_by_kept_quoting(polyglot, sqlquack)) } + /// The entry explaining where SQLQuack keeps a wrapper node polyglot drops. + pub fn kept_node_for(&self, sql: &str, polyglot: &str, sqlquack: &str) -> Option<&KeptNode> { + let compact = compact(sql); + self.kept_nodes.iter().find(|entry| { + compact.contains(&entry.marker.to_ascii_uppercase()) + && differ_only_by_kept_node(polyglot, sqlquack, &entry.node) + }) + } + /// 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 984e98fd0..cd4149a5c 100644 --- a/crates/sqlquack-polyglot-check/src/runs/main/run_corpus.rs +++ b/crates/sqlquack-polyglot-check/src/runs/main/run_corpus.rs @@ -59,22 +59,26 @@ pub fn run_corpus(entries: &[CorpusEntry], expected: &ExpectedDifferences) -> Ru let dialect = format!("{:?}", entry.dialect); 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 structured = expected.structured_for(&entry.sql, &polyglot, &sqlquack); - match (fix, rejection, kept_quoting, structured) { - (Some(fix), _, _, _) => *summary.fixed.entry(fix.id.clone()).or_default() += 1, - (None, Some(rejection), _, _) => { + match (fix, rejection, kept_quoting, kept_node, structured) { + (Some(fix), ..) => *summary.fixed.entry(fix.id.clone()).or_default() += 1, + (None, Some(rejection), ..) => { *summary.rejected.entry(rejection.id.clone()).or_default() += 1; } - (None, None, Some(kept_quoting), _) => { + (None, None, Some(kept_quoting), ..) => { *summary .kept_quoting .entry(kept_quoting.id.clone()) .or_default() += 1; } - (None, None, None, Some(structured)) => { + (None, None, None, Some(kept_node), _) => { + *summary.kept_nodes.entry(kept_node.id.clone()).or_default() += 1; + } + (None, None, None, None, Some(structured)) => { *summary.structured.entry(structured.id.clone()).or_default() += 1; } - (None, None, None, None) => summary.differences.push(Finding { + (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 828789273..1a8d22b57 100644 --- a/crates/sqlquack-polyglot-check/src/runs/models.rs +++ b/crates/sqlquack-polyglot-check/src/runs/models.rs @@ -29,6 +29,8 @@ pub struct RunSummary { pub rejected: BTreeMap, /// Entries whose trees differ only by quoting polyglot drops, as a reviewed entry explains. pub kept_quoting: BTreeMap, + /// Entries whose trees differ only by wrapper nodes polyglot drops, as a reviewed entry explains. + pub kept_nodes: 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.rs b/crates/sqlquack-polyglot-check/tests/runs.rs index 95fe3ed20..650114413 100644 --- a/crates/sqlquack-polyglot-check/tests/runs.rs +++ b/crates/sqlquack-polyglot-check/tests/runs.rs @@ -2,6 +2,8 @@ mod helpers; #[path = "runs/test_exact.rs"] mod test_exact; +#[path = "runs/test_kept_nodes.rs"] +mod test_kept_nodes; #[path = "runs/test_kept_quoting.rs"] mod test_kept_quoting; #[path = "runs/test_rejections.rs"] diff --git a/crates/sqlquack-polyglot-check/tests/runs/test_exact.rs b/crates/sqlquack-polyglot-check/tests/runs/test_exact.rs index f10f77983..e603bceae 100644 --- a/crates/sqlquack-polyglot-check/tests/runs/test_exact.rs +++ b/crates/sqlquack-polyglot-check/tests/runs/test_exact.rs @@ -19,8 +19,10 @@ fn given_committed_corpora_when_parsed_by_both_parsers_then_results_are_identica expected_outcome: ExactOutcome { loaded: true, rejected: BTreeMap::from([ - ("malformed_typed_type_text".to_owned(), 2), + ("malformed_typed_type_text".to_owned(), 1), ("snowflake_bare_values_cte".to_owned(), 1), + ("snowflake_cte_without_as_prose".to_owned(), 1), + ("snowflake_reserved_type_name".to_owned(), 1), ("snowflake_by_name_on_except".to_owned(), 1), ("snowflake_by_name_on_intersect".to_owned(), 1), ("snowflake_empty_select_list".to_owned(), 1), @@ -54,6 +56,10 @@ fn given_committed_corpora_when_parsed_by_both_parsers_then_results_are_identica expected_outcome: ExactOutcome { loaded: true, rejected: BTreeMap::from([ + ("snowflake_cte_without_as".to_owned(), 1), + ("snowflake_empty_call_argument".to_owned(), 1), + ("snowflake_reserved_cte_name".to_owned(), 1), + ("snowflake_reserved_truncate_target".to_owned(), 1), ( "snowflake_empty_select_list_trailing_order_by".to_owned(), 1, diff --git a/crates/sqlquack-polyglot-check/tests/runs/test_kept_nodes.rs b/crates/sqlquack-polyglot-check/tests/runs/test_kept_nodes.rs new file mode 100644 index 000000000..a05dff54f --- /dev/null +++ b/crates/sqlquack-polyglot-check/tests/runs/test_kept_nodes.rs @@ -0,0 +1,139 @@ +//! Reviewed kept nodes cover only wrappers polyglot drops, in statements with the marker. + +use sqlquack_polyglot_check::corpora::models::CorpusEntry; +use sqlquack_polyglot_check::expected::models::{ExpectedDifferences, KeptNode}; +use sqlquack_syntax::Dialect; + +use crate::helpers::summary_of; +use crate::test_types::{KeptNodeTestCase, QuotingTreesTestCase}; + +#[test] +fn given_unary_plus_when_compared_then_only_the_kept_operator_is_explained() { + let test_cases = [ + KeptNodeTestCase { + description: "a unary plus on a literal is a kept node", + sql: "SELECT + 1", + expected_kept_nodes: 1, + expected_differences: 0, + }, + KeptNodeTestCase { + description: "a unary plus on a column is a kept node", + sql: "SELECT +IF FROM t", + expected_kept_nodes: 1, + expected_differences: 0, + }, + KeptNodeTestCase { + description: "a unary minus gives identical trees", + sql: "SELECT - 1", + expected_kept_nodes: 0, + expected_differences: 0, + }, + KeptNodeTestCase { + description: "a binary plus gives identical trees", + sql: "SELECT 1 + 2", + expected_kept_nodes: 0, + expected_differences: 0, + }, + ]; + + for test_case in test_cases { + let summary = summary_of(&[CorpusEntry { + dialect: Dialect::Snowflake, + sql: test_case.sql.to_owned(), + from: test_case.description.to_owned(), + }]); + assert_eq!( + ( + summary.kept_nodes.values().sum::(), + summary.differences.len() + ), + ( + test_case.expected_kept_nodes, + test_case.expected_differences + ), + "{}", + test_case.description + ); + } +} + +#[test] +fn given_trees_with_a_marker_when_matched_then_only_dropped_wrappers_are_allowed() { + let column = |name: &str| format!(r#"{{"column":{{"name":"{name}"}}}}"#); + let pos = |inner: String| format!(r#"{{"pos":{{"this":{inner}}}}}"#); + let tree = |first: String, second: String| format!(r#"ok:[{{"select":[{first},{second}]}}]"#); + let test_cases = [ + QuotingTreesTestCase { + description: "a wrapper polyglot drops is the only difference", + sql: "SELECT +a, b", + polyglot: tree(column("a"), column("b")), + sqlquack: tree(pos(column("a")), column("b")), + expected_match: true, + }, + QuotingTreesTestCase { + description: "nested wrappers polyglot drops are the only difference", + sql: "SELECT + +a, b", + polyglot: tree(column("a"), column("b")), + sqlquack: tree(pos(pos(column("a"))), column("b")), + expected_match: true, + }, + QuotingTreesTestCase { + description: "a different name beside the wrapper is not matched", + sql: "SELECT +a, b", + polyglot: tree(column("a"), column("c")), + sqlquack: tree(pos(column("a")), column("b")), + expected_match: false, + }, + QuotingTreesTestCase { + description: "a wrapper polyglot keeps and SQLQuack drops is not matched", + sql: "SELECT +a, b", + polyglot: tree(pos(column("a")), column("b")), + sqlquack: tree(column("a"), column("b")), + expected_match: false, + }, + QuotingTreesTestCase { + description: "a wrapper with a field besides its operand is not matched", + sql: "SELECT +a, b", + polyglot: tree(column("a"), column("b")), + sqlquack: tree( + format!(r#"{{"pos":{{"this":{},"span":null}}}}"#, column("a")), + column("b"), + ), + expected_match: false, + }, + QuotingTreesTestCase { + description: "a statement without the marker is not matched", + sql: "SELECT a, b", + polyglot: tree(column("a"), column("b")), + sqlquack: tree(pos(column("a")), column("b")), + expected_match: false, + }, + QuotingTreesTestCase { + description: "a polyglot error is not matched", + sql: "SELECT +a, b", + polyglot: "err:Parse".to_owned(), + sqlquack: tree(pos(column("a")), column("b")), + expected_match: false, + }, + ]; + let expected = ExpectedDifferences { + kept_nodes: vec![KeptNode { + id: "unary_plus".to_owned(), + marker: "+".to_owned(), + node: "pos".to_owned(), + note: "a test entry".to_owned(), + }], + ..ExpectedDifferences::default() + }; + + for test_case in test_cases { + assert_eq!( + expected + .kept_node_for(test_case.sql, &test_case.polyglot, &test_case.sqlquack) + .is_some(), + test_case.expected_match, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlquack-polyglot-check/tests/runs/test_types.rs b/crates/sqlquack-polyglot-check/tests/runs/test_types.rs index 050d11294..4ec7d7e1a 100644 --- a/crates/sqlquack-polyglot-check/tests/runs/test_types.rs +++ b/crates/sqlquack-polyglot-check/tests/runs/test_types.rs @@ -29,6 +29,13 @@ pub(crate) struct RejectionTestCase { pub(crate) expected_differences: usize, } +pub(crate) struct KeptNodeTestCase { + pub(crate) description: &'static str, + pub(crate) sql: &'static str, + pub(crate) expected_kept_nodes: usize, + pub(crate) expected_differences: usize, +} + pub(crate) struct KeptQuotingTestCase { pub(crate) description: &'static str, pub(crate) sql: &'static str, diff --git a/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs b/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs index 2bdf50272..677b91890 100644 --- a/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs +++ b/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs @@ -540,6 +540,36 @@ fn given_unsupported_snowflake_sql_when_parsing_then_parsing_fails() { sql: "DECLARE SELECT RESULTSET; BEGIN RETURN 1; END;", expected_error: "Parse error", }, + ParseErrorTestCase { + description: "a CTE name needs AS before its body", + dialect: Some(DialectType::Snowflake), + sql: "WITH t (SELECT 1 AS c) SELECT c FROM t", + expected_error: "Expected AS before the CTE body", + }, + ParseErrorTestCase { + description: "prose read as a CTE needs AS before its body", + dialect: Some(DialectType::Snowflake), + sql: " with the correct AST shape.", + expected_error: "Expected AS before the CTE body", + }, + ParseErrorTestCase { + description: "a reserved word cannot name a CTE", + dialect: Some(DialectType::Snowflake), + sql: "WITH all AS (SELECT 1 AS count) SELECT all.count FROM all", + expected_error: "Reserved word ALL cannot be an unquoted object name", + }, + ParseErrorTestCase { + description: "a reserved word cannot name a truncated table", + dialect: Some(DialectType::Snowflake), + sql: "TRUNCATE TABLE table", + expected_error: "Reserved word TABLE cannot be an unquoted object name", + }, + ParseErrorTestCase { + description: "an empty named argument between commas is rejected", + dialect: Some(DialectType::Snowflake), + sql: "SELECT ROUND(SCALE => 1, EXPR => 2.25, , ROUNDING_MODE => 'HALF_TO_EVEN') AS value", + expected_error: "Expected a function argument between commas", + }, ParseErrorTestCase { description: "an anonymous procedure needs RETURNS", dialect: Some(DialectType::Snowflake), diff --git a/crates/sqlquack-syntax/src/parser/tests/test_typed_expressions.rs b/crates/sqlquack-syntax/src/parser/tests/test_typed_expressions.rs index 01434573c..798b29a78 100644 --- a/crates/sqlquack-syntax/src/parser/tests/test_typed_expressions.rs +++ b/crates/sqlquack-syntax/src/parser/tests/test_typed_expressions.rs @@ -98,6 +98,12 @@ fn given_a_typed_expression_with_a_malformed_type_when_parsing_then_the_statemen sql: "SELECT __polyglot_typed(amount, 'NOT A TYPE (') FROM orders", expected_error: "Unexpected token after data type", }, + ParseErrorTestCase { + description: "a reserved word as the type fails first in Snowflake", + dialect: Some(DialectType::Snowflake), + sql: "SELECT __polyglot_typed(amount, 'NOT A TYPE (') FROM orders", + expected_error: "Reserved word NOT cannot be an unquoted type name", + }, ParseErrorTestCase { description: "a type with trailing tokens fails", dialect: Some(DialectType::DuckDB), diff --git a/docs/syntax.md b/docs/syntax.md index 0167340c6..abb308f68 100644 --- a/docs/syntax.md +++ b/docs/syntax.md @@ -201,6 +201,12 @@ reason. The check counts it as kept quoting, not different, only when the marker two trees differ in nothing but identifiers that are unquoted in polyglot's tree and quoted in SQLQuack's. The only entry: a star's `EXCLUDE ("note")`, whose quotes make the name case-sensitive. +A statement both parsers parse, where SQLQuack keeps a wrapper node polyglot drops, is a +`[[kept_node]]` entry: an id, a marker, the node and a reason. The check counts it as a kept node, +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. + 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