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-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-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 966ec3695..af4bd245b 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 { @@ -62,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 a63a49737..50f13d9eb 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,18 @@ 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(), + 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 0b35ed23a..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)] @@ -48,6 +50,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"`). @@ -85,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. @@ -156,8 +163,10 @@ 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>, + /// 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 41ed7cb22..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); } @@ -381,6 +391,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..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 @@ -2,12 +2,12 @@ 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, }; -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] @@ -521,6 +521,47 @@ 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: "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"), @@ -989,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"), @@ -1033,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 b26dee81d..55f726692 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, 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" )); @@ -567,6 +591,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 +625,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, @@ -685,3 +738,24 @@ pub(crate) fn check_restatements<'a>( ); problems } + +/// 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(); + 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.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/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 6d6686de6..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,7 +452,38 @@ pub struct UnobservableFile { pub diagnostics: Vec, } -/// An obligation Snowflake answers only with an internal error; never verified (decision D87). +/// 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)] pub struct Unobservable { @@ -457,6 +491,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 +501,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)] @@ -512,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 1ae3b5ac1..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| { @@ -784,6 +822,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| { @@ -964,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/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/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/schema_objects/models.rs b/crates/sqlquack-syntax/src/expressions/schema_objects/models.rs index a1fbda504..fdcb2b7ea 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)] @@ -689,6 +690,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 { @@ -707,6 +711,7 @@ impl CreateProcedure { with_options: Vec::new(), has_parens: true, use_proc_keyword: false, + returns_table: None, } } } diff --git a/crates/sqlquack-syntax/src/expressions/tree/_helpers/accessors.rs b/crates/sqlquack-syntax/src/expressions/tree/_helpers/accessors.rs index 5ccf255bc..c7f60aa1a 100644 --- a/crates/sqlquack-syntax/src/expressions/tree/_helpers/accessors.rs +++ b/crates/sqlquack-syntax/src/expressions/tree/_helpers/accessors.rs @@ -215,9 +215,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(), @@ -444,9 +445,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 86fd7e462..0a87df03b 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", @@ -964,7 +965,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 b0f63eb25..0b19be36b 100644 --- a/crates/sqlquack-syntax/src/expressions/tree/models.rs +++ b/crates/sqlquack-syntax/src/expressions/tree/models.rs @@ -809,6 +809,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/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 9e2288cfe..bc5a08b1a 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/constraints.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/constraints.rs @@ -1496,7 +1496,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; } @@ -1538,7 +1538,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, @@ -1555,7 +1555,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/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/expressions/operators.rs b/crates/sqlquack-syntax/src/parser/_helpers/expressions/operators.rs index 9a3142eff..6ed4642a6 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(); @@ -373,6 +373,7 @@ impl Parser { with_options, has_parens, use_proc_keyword, + returns_table: None, }))) } @@ -381,7 +382,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 +615,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 +1236,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/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/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/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..71ea54c33 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 }; @@ -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,69 +1930,14 @@ impl Parser { let search = self.parse_recursive_with_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) + self.parse_with_statement( + With { + ctes, + recursive, + leading_comments, + search, + }, + 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 e6decf2cd..3568a14c6 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/statements/mod.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/mod.rs @@ -3,5 +3,7 @@ pub(crate) mod analyze; pub(crate) mod dml; 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 new file mode 100644 index 000000000..28a604189 --- /dev/null +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/procedures.rs @@ -0,0 +1,239 @@ +//! 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); + 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 = 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 = 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, + mut procedure: 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(procedure); + }; + 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 = 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, 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 { + 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) { + 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/_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/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) { 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 787b752b0..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; @@ -737,14 +738,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 +903,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 { @@ -1114,6 +1107,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_TYPE + && 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() @@ -1126,6 +1130,9 @@ impl Parser { let mut after_comma = true; loop { if self.check(TokenType::RParen) { + if after_comma && !(args.is_empty() && name == TABLE_TYPE) { + return Err(self.parse_error("Expected a type parameter")); + } break; } let token = self.advance()?; diff --git a/crates/sqlquack-syntax/src/parser/constants.rs b/crates/sqlquack-syntax/src/parser/constants.rs index c41ce949b..38fbd750d 100644 --- a/crates/sqlquack-syntax/src/parser/constants.rs +++ b/crates/sqlquack-syntax/src/parser/constants.rs @@ -87,6 +87,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 53700d439..677b91890 100644 --- a/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs +++ b/crates/sqlquack-syntax/src/parser/tests/test_snowflake.rs @@ -462,6 +462,162 @@ 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: "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), + 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), @@ -480,6 +636,105 @@ fn given_unsupported_snowflake_sql_when_parsing_then_parsing_fails() { } } +#[test] +fn given_neighbours_of_rejected_snowflake_sql_when_parsing_then_they_parse() { + 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 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_snowflake_string_escapes_when_tokenizing_then_they_decode_as_documented() { let test_cases = [ @@ -537,6 +792,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 = [ 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/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 efed540ce..45c8ea2c2 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::{CallMode, Dialect}; -use crate::helpers::{accepted, cast_targets, projection_at, statements}; -use crate::test_types::{AcceptanceTestCase, CastTargetTestCase, ProjectionTestCase}; +use crate::helpers::{accepted, cast_targets, projection_at, statement_at, statements}; +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() { @@ -421,6 +423,102 @@ 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 + ); + } +} + #[test] fn given_a_snowflake_drop_of_another_object_kind_when_parsing_then_it_takes_that_kinds_options() { let test_cases = [ 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, diff --git a/docs/syntax-coverage.md b/docs/syntax-coverage.md index 1e8fbbb7d..ef55e59e2 100644 --- a/docs/syntax-coverage.md +++ b/docs/syntax-coverage.md @@ -108,6 +108,13 @@ 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` (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). + ## Cases (`cases/.jsonl`) Each line is one JSON object. Blank lines are refused. @@ -168,6 +175,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 @@ -250,7 +261,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/`. @@ -282,8 +300,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. @@ -328,7 +347,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). 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