diff --git a/crates/sqlquack-syntax-coverage/src/check/_helpers/contradictions.rs b/crates/sqlquack-syntax-coverage/src/check/_helpers/contradictions.rs new file mode 100644 index 000000000..8eaed4e33 --- /dev/null +++ b/crates/sqlquack-syntax-coverage/src/check/_helpers/contradictions.rs @@ -0,0 +1,64 @@ +//! Every current receipt against the parser, whether or not a catalogued case cites it (decision D141). + +use std::collections::{BTreeMap, BTreeSet}; + +use crate::check::_helpers::receipts::snowflake_syntax; +use crate::check::models::{CaseResult, LocalVerdict, SnowflakeSyntax}; +use crate::formats::models::{Receipt, SyntaxCoverage}; + +/// Whether the parser's verdict contradicts what Snowflake's parser did with the same statement. +fn contradicts(snowflake: SnowflakeSyntax, local: &LocalVerdict) -> bool { + match snowflake { + SnowflakeSyntax::Parsed => !matches!(local, LocalVerdict::Accepted { .. }), + SnowflakeSyntax::Refused => matches!(local, LocalVerdict::Accepted { .. }), + } +} + +/// The receipt case ids the parser contradicts by SQL, setup and session, and those without SQL. +pub(crate) fn receipt_contradictions( + coverage: &SyntaxCoverage, + cases: &BTreeMap<&str, CaseResult>, + parse: &dyn Fn(&str) -> LocalVerdict, +) -> (BTreeSet, BTreeSet) { + let mut statements: BTreeMap<(&str, &str, &str), Vec<&Receipt>> = BTreeMap::new(); + for receipt in &coverage.receipts { + statements + .entry(( + receipt.sql_sha256.as_str(), + receipt.setup_sha256.as_str(), + receipt.session_sha256.as_str(), + )) + .or_default() + .push(receipt); + } + let mut judged: BTreeMap<&str, LocalVerdict> = cases + .values() + .map(|case| (case.sql_sha256.as_str(), case.local.clone())) + .collect(); + let case_sql: BTreeMap<&str, &str> = coverage + .cases + .values() + .flatten() + .map(|case| (case.sql_sha256.as_str(), case.sql.as_str())) + .collect(); + let mut contradicted: BTreeSet = BTreeSet::new(); + let mut unjudged: BTreeSet = BTreeSet::new(); + for ((sql_sha256, _, _), receipts) in &statements { + let Some(snowflake) = snowflake_syntax(receipts) else { + continue; + }; + let sql: Option<&str> = case_sql + .get(sql_sha256) + .copied() + .or_else(|| coverage.receipt_sql.get(*sql_sha256).map(String::as_str)); + let Some(sql) = sql else { + unjudged.extend(receipts.iter().map(|receipt| receipt.case_id.clone())); + continue; + }; + let local: &LocalVerdict = judged.entry(sql_sha256).or_insert_with(|| parse(sql)); + if contradicts(snowflake, local) { + contradicted.extend(receipts.iter().map(|receipt| receipt.case_id.clone())); + } + } + (contradicted, unjudged) +} diff --git a/crates/sqlquack-syntax-coverage/src/check/_helpers/mod.rs b/crates/sqlquack-syntax-coverage/src/check/_helpers/mod.rs index d774bbdd0..e800470e9 100644 --- a/crates/sqlquack-syntax-coverage/src/check/_helpers/mod.rs +++ b/crates/sqlquack-syntax-coverage/src/check/_helpers/mod.rs @@ -1,3 +1,4 @@ +pub(crate) mod contradictions; pub(crate) mod fingerprints; pub(crate) mod forms; pub(crate) mod join; diff --git a/crates/sqlquack-syntax-coverage/src/check/_helpers/receipts.rs b/crates/sqlquack-syntax-coverage/src/check/_helpers/receipts.rs index 165c40a34..1ece2b1cf 100644 --- a/crates/sqlquack-syntax-coverage/src/check/_helpers/receipts.rs +++ b/crates/sqlquack-syntax-coverage/src/check/_helpers/receipts.rs @@ -63,7 +63,7 @@ fn agreement(phase: Phase, local: &LocalVerdict, terminates: bool) -> Result Option { +pub(crate) fn snowflake_syntax(statement_receipts: &[&Receipt]) -> Option { let verdicts: BTreeSet> = statement_receipts .iter() .filter(|receipt| receipt.measure.is_none()) diff --git a/crates/sqlquack-syntax-coverage/src/check/main/check_coverage.rs b/crates/sqlquack-syntax-coverage/src/check/main/check_coverage.rs index c0779d0e1..05fee1098 100644 --- a/crates/sqlquack-syntax-coverage/src/check/main/check_coverage.rs +++ b/crates/sqlquack-syntax-coverage/src/check/main/check_coverage.rs @@ -2,6 +2,7 @@ use std::collections::BTreeSet; +use crate::check::_helpers::contradictions::receipt_contradictions; use crate::check::_helpers::forms::form_result; use crate::check::_helpers::join::join; use crate::check::_helpers::references::reference_problems; @@ -31,10 +32,14 @@ pub fn check_coverage( forms.push(form_result(form, family, &context)); } } + let (contradicted, unjudged): (BTreeSet, BTreeSet) = + receipt_contradictions(coverage, &joined.cases, parse); CheckResult { parser_tree: parser_tree.map(str::to_owned), forms, unexcluded_pages: unexcluded_pages(&coverage.sources, &excluded_sources), problems: reference_problems(coverage, &joined.forms), + receipt_contradictions: contradicted, + unjudged_receipts: unjudged, } } diff --git a/crates/sqlquack-syntax-coverage/src/check/models.rs b/crates/sqlquack-syntax-coverage/src/check/models.rs index 7c6eb3b2e..9f944763e 100644 --- a/crates/sqlquack-syntax-coverage/src/check/models.rs +++ b/crates/sqlquack-syntax-coverage/src/check/models.rs @@ -162,6 +162,10 @@ pub struct CheckResult { pub unexcluded_pages: Vec, /// References between files that do not resolve; any one fails the check. pub problems: Vec, + /// Receipt case ids whose statement the parser contradicts, cited by a form or not (D141). + pub receipt_contradictions: BTreeSet, + /// Receipt case ids whose SQL no case or run report gives, so no verdict can be compared. + pub unjudged_receipts: BTreeSet, } /// The files joined for checking, with every case already judged. diff --git a/crates/sqlquack-syntax-coverage/src/check/tests/helpers.rs b/crates/sqlquack-syntax-coverage/src/check/tests/helpers.rs index 2c6bff14e..c841cb1b1 100644 --- a/crates/sqlquack-syntax-coverage/src/check/tests/helpers.rs +++ b/crates/sqlquack-syntax-coverage/src/check/tests/helpers.rs @@ -433,3 +433,17 @@ pub(crate) fn move_form(root: &Path, id: &str) { &format!("{declared}anchor = \"Syntax, edited\""), ); } + +/// Adds a receipt recording `fields` for `retired-1`, a case id no form cites, of `sql`. +pub(crate) fn retired_receipt(root: &Path, sql: &str, fields: Value) { + let case: String = case_line("retired-1", "sf.query.select.o001", sql, &[]); + append_receipt(root, &with_fields(&receipt_line(&case, "accepted"), fields)); +} + +/// Writes the run report beside the receipts, listing `sql` among its inputs. +pub(crate) fn report_input(root: &Path, sql: &str) { + let report: Value = serde_json::json!({ + "inputs": [{"sql": sql, "sql_sha256": sha256(sql)}], + }); + write(root, "receipts/run-1.report.json", &report.to_string()); +} 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 7d835c496..b75a3b1db 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 @@ -4,10 +4,12 @@ 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, 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, + rejecting, replace, report_input, reshaped, retired_receipt, rewrite_select_17, select_17, + select_17_setup_sha256, sha256, structural, unobservable, verified_tree, with_fields, write, +}; +use crate::check::tests::test_types::{ + AccountStateTestCase, CheckTestCase, Parameters, ReceiptCheckTestCase, }; -use crate::check::tests::test_types::{AccountStateTestCase, CheckTestCase, Parameters}; use crate::formats::main::load_coverage::load_coverage; #[test] @@ -1169,3 +1171,73 @@ fn given_a_receipt_with_account_state_when_checking_then_only_its_case_carries_i ); } } + +#[test] +fn given_receipts_no_form_cites_when_checking_then_each_is_judged_against_the_parser() { + let test_cases = [ + ReceiptCheckTestCase { + description: "a retired case Snowflake parsed, which the parser now rejects", + mutate: |root| { + retired_receipt(root, "SELECT 2", serde_json::json!({})); + report_input(root, "SELECT 2"); + }, + parse: rejecting, + expected_contradictions: vec!["retired-1"], + expected_unjudged: Vec::new(), + }, + ReceiptCheckTestCase { + description: "a retired case Snowflake parsed, which the parser accepts", + mutate: |root| { + retired_receipt(root, "SELECT 2", serde_json::json!({})); + report_input(root, "SELECT 2"); + }, + parse: structural, + expected_contradictions: Vec::new(), + expected_unjudged: Vec::new(), + }, + ReceiptCheckTestCase { + description: "a retired syntax error with errno 1003, which the parser accepts", + mutate: |root| { + retired_receipt( + root, + "SELECT 2", + serde_json::json!({"phase": "syntax_error", "outcome": "error", "errno": 1003}), + ); + report_input(root, "SELECT 2"); + }, + parse: structural, + expected_contradictions: vec!["retired-1"], + expected_unjudged: Vec::new(), + }, + ReceiptCheckTestCase { + description: "a retired case whose SQL no case or report gives", + mutate: |root| retired_receipt(root, "SELECT 2", serde_json::json!({})), + parse: rejecting, + expected_contradictions: Vec::new(), + expected_unjudged: vec!["retired-1"], + }, + ]; + for test_case in test_cases { + let directory = tempfile::tempdir().expect("tempdir"); + verified_tree(directory.path()); + (test_case.mutate)(directory.path()); + let coverage = load_coverage(directory.path()).expect("loads"); + let check = check_coverage(&coverage, Some(TREE), &test_case.parse); + let contradicted: Vec<&str> = check + .receipt_contradictions + .iter() + .map(String::as_str) + .filter(|id| *id == "retired-1") + .collect(); + let unjudged: Vec<&str> = check.unjudged_receipts.iter().map(String::as_str).collect(); + assert_eq!( + (contradicted, unjudged), + ( + test_case.expected_contradictions, + test_case.expected_unjudged + ), + "{}", + 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 012001a06..18f28edcf 100644 --- a/crates/sqlquack-syntax-coverage/src/check/tests/test_types.rs +++ b/crates/sqlquack-syntax-coverage/src/check/tests/test_types.rs @@ -29,3 +29,12 @@ pub(crate) struct AccountStateTestCase { /// Each case id with its parameter names and values, in case-id order. pub(crate) expected_cases: Vec<(&'static str, Parameters<'static>)>, } + +/// Receipts on a case id no form cites, and the receipt verdicts the checker must then give (D141). +pub(crate) struct ReceiptCheckTestCase { + pub(crate) description: &'static str, + pub(crate) mutate: fn(&Path), + pub(crate) parse: fn(&str) -> LocalVerdict, + pub(crate) expected_contradictions: Vec<&'static str>, + pub(crate) expected_unjudged: Vec<&'static str>, +} diff --git a/crates/sqlquack-syntax-coverage/src/cli/_helpers/commands.rs b/crates/sqlquack-syntax-coverage/src/cli/_helpers/commands.rs index 408efc4b5..20d09709c 100644 --- a/crates/sqlquack-syntax-coverage/src/cli/_helpers/commands.rs +++ b/crates/sqlquack-syntax-coverage/src/cli/_helpers/commands.rs @@ -339,8 +339,16 @@ fn read_snapshot(path: &Path) -> Result { /// Lines naming each non-empty list of the diff. fn diff_lines(diff: &SnapshotDiff) -> Vec { - let lists: [(&str, &Vec); 10] = [ + let lists: [(&str, &Vec); 12] = [ ("REGRESSION forms lost", &diff.forms_lost), + ( + "REGRESSION receipts the parser now contradicts, cited by a form or not (D141)", + &diff.receipts_contradicted, + ), + ( + "DOCUMENTED (D113) receipts the parser now contradicts, for the auditor", + &diff.documented_receipts_contradicted, + ), ("REGRESSION obligations lost", &diff.obligations_lost), ( "REGRESSION Snowflake-parsed cases the parser now rejects", @@ -360,11 +368,28 @@ fn diff_lines(diff: &SnapshotDiff) -> Vec { ("forms gained", &diff.forms_gained), ("obligations gained", &diff.obligations_gained), ]; - lists + let mut lines: Vec = lists .iter() .filter(|(_, ids)| !ids.is_empty()) .map(|(label, ids)| format!("{label} ({}): {}", ids.len(), ids.join(" "))) - .collect() + .collect(); + let counted: [(&str, usize); 2] = [ + ( + "receipts the parser already contradicted, not failing", + diff.receipts_still_contradicted.len(), + ), + ( + "receipts without SQL to judge, not failing", + diff.unjudged_receipts.len(), + ), + ]; + lines.extend( + counted + .iter() + .filter(|(_, count)| *count > 0) + .map(|(label, count)| format!("{label}: {count} (listed in --json)")), + ); + lines } /// BASE against HEAD, or against the current tree; fails on a regression or catalogue change. @@ -389,6 +414,7 @@ pub(crate) fn diff( let failing: bool = [ &diff.forms_lost, &diff.obligations_lost, + &diff.receipts_contradicted, &diff.parsed_cases_refused, &diff.syntax_error_cases_accepted, &diff.catalogue_files_changed, diff --git a/crates/sqlquack-syntax-coverage/src/formats/_helpers/retired.rs b/crates/sqlquack-syntax-coverage/src/formats/_helpers/retired.rs index 6ba6c7c3c..5734c2425 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/_helpers/retired.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/_helpers/retired.rs @@ -6,13 +6,16 @@ use std::path::{Path, PathBuf}; use serde_json::Value; use crate::formats::_helpers::files::read_text; -use crate::formats::_helpers::hashes::is_sha256; -use crate::formats::constants::{REPORT_RECEIPT_SHA256, REPORT_WITHDRAWN}; +use crate::formats::_helpers::hashes::{is_sha256, sha256_hex}; +use crate::formats::constants::{REPORT_INPUTS, REPORT_RECEIPT_SHA256, REPORT_WITHDRAWN}; use crate::formats::models::Receipt; -/// The receipt line hashes the run reports withdraw, and every problem reading them. -pub(crate) fn withdrawn_receipts(reports: &[PathBuf]) -> (BTreeSet, Vec) { +/// The receipt line hashes the run reports withdraw, the SQL of their inputs by hash, and every problem. +pub(crate) fn read_reports( + reports: &[PathBuf], +) -> (BTreeSet, BTreeMap, Vec) { let mut withdrawn: BTreeSet = BTreeSet::new(); + let mut statements: BTreeMap = BTreeMap::new(); let mut problems: Vec = Vec::new(); for path in reports { let report: Value = match read_text(path).and_then(|(text, _)| { @@ -38,8 +41,21 @@ pub(crate) fn withdrawn_receipts(reports: &[PathBuf]) -> (BTreeSet, Vec< None => problems.push(withdrawn_problem(path)), } } + let inputs: &[Value] = report[REPORT_INPUTS].as_array().map_or(&[], Vec::as_slice); + for input in inputs { + match (input["sql"].as_str(), input["sql_sha256"].as_str()) { + (Some(sql), Some(hash)) if sha256_hex(sql.as_bytes()) == hash => { + statements.insert(hash.to_owned(), sql.to_owned()); + } + (Some(_), Some(hash)) => problems.push(format!( + "{}: input {hash} is not the SHA-256 of its sql", + path.display() + )), + _ => {} + } + } } - (withdrawn, problems) + (withdrawn, statements, problems) } fn withdrawn_problem(path: &Path) -> String { diff --git a/crates/sqlquack-syntax-coverage/src/formats/_helpers/sections.rs b/crates/sqlquack-syntax-coverage/src/formats/_helpers/sections.rs index 6a622233f..17c1d2a27 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/_helpers/sections.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/_helpers/sections.rs @@ -11,7 +11,7 @@ use crate::formats::_helpers::records::{ 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::_helpers::retired::{read_reports, retire}; use crate::formats::constants::{ ALIASES_FILE, CASES_DIR, CATALOGUE_DIR, INDEXES_FILE, LIMITS_FILE, MARKS, MARKS_DIR, OBLIGATIONS_DIR, RECEIPT_REPORT_SUFFIX, RECEIPTS_DIR, REVIEWS_DIR, SOURCES_FILE, @@ -22,7 +22,7 @@ use crate::formats::models::{ Obligation, ObligationsFile, Reading, Receipt, Review, ReviewsFile, Source, SourcesFile, Unobservable, UnobservableFile, }; -use crate::formats::types::FamilyCatalogue; +use crate::formats::types::{FamilyCatalogue, LoadedReceipts}; /// What one section of the tree yields. pub(crate) struct Section { @@ -206,8 +206,8 @@ pub(crate) fn load_marks(root: &Path) -> Section> { Section::of(records, declared, problems) } -/// Every receipts file, less the receipts superseded or withdrawn by the run reports beside them. -pub(crate) fn load_receipts(root: &Path) -> Section<(Vec, Vec)> { +/// Every receipts file, less those superseded or withdrawn, with the SQL its run reports give by hash. +pub(crate) fn load_receipts(root: &Path) -> Section { let (paths, others) = listed(&root.join(RECEIPTS_DIR), "jsonl"); let mut problems: Vec = others .iter() @@ -226,11 +226,11 @@ pub(crate) fn load_receipts(root: &Path) -> Section<(Vec, Vec)> ); published.extend(receipts); } - let (withdrawn, report_problems) = withdrawn_receipts(&reports); + let (withdrawn, statements, report_problems) = read_reports(&reports); problems.extend(report_problems); let (kept, retired, retire_problems) = retire(published, &withdrawn); problems.extend(retire_problems); - Section::of((kept, retired), Vec::new(), problems) + Section::of((kept, retired, statements), Vec::new(), problems) } /// `unobservable/.toml`: optional, and each obligation listed at most once. @@ -316,7 +316,7 @@ pub(crate) struct Sections { pub(crate) catalogue: Section, pub(crate) obligations: Section>>, pub(crate) cases: Section>>, - pub(crate) receipts: Section<(Vec, Vec)>, + pub(crate) receipts: Section, pub(crate) reviews: Section>, pub(crate) unobservable: Section<(Vec, Vec)>, pub(crate) readings: Section>, diff --git a/crates/sqlquack-syntax-coverage/src/formats/constants.rs b/crates/sqlquack-syntax-coverage/src/formats/constants.rs index 0ddf80c2c..9113fa9be 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/constants.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/constants.rs @@ -72,6 +72,8 @@ pub const SNAPSHOT_FORMAT: &str = "sqlquack-syntax-coverage-snapshot/1"; pub const BASELINE_FILE: &str = "baseline.json"; /// The run-report list of receipt lines the importer withdrew, each by `receipt_sha256`. pub const REPORT_WITHDRAWN: &str = "withdrawn"; +/// The run-report list of statements the importer read, each with `sql` and `sql_sha256`. +pub const REPORT_INPUTS: &str = "inputs"; /// The field of a withdrawn entry naming the receipt line's SHA-256. pub const REPORT_RECEIPT_SHA256: &str = "receipt_sha256"; /// The importer's run report beside each receipts file. 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 92afa166c..b9827eace 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/main/load_coverage.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/main/load_coverage.rs @@ -35,6 +35,7 @@ pub fn load_coverage(root: &Path) -> Result> { cases: sections.cases.records, receipts: sections.receipts.records.0, retired_receipts: sections.receipts.records.1, + receipt_sql: sections.receipts.records.2, reviews: sections.reviews.records, unobservable: sections.unobservable.records.0, diagnostics: sections.unobservable.records.1, diff --git a/crates/sqlquack-syntax-coverage/src/formats/models.rs b/crates/sqlquack-syntax-coverage/src/formats/models.rs index 1090e3a6e..59df56c6a 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/models.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/models.rs @@ -564,6 +564,8 @@ pub struct SyntaxCoverage { pub receipts: Vec, /// SHA-256 of each published receipt line that is superseded or withdrawn. pub retired_receipts: Vec, + /// The SQL of every statement a run report read, by its SHA-256, cited by a case or not. + pub receipt_sql: BTreeMap, pub reviews: Vec, pub unobservable: Vec, pub diagnostics: Vec, diff --git a/crates/sqlquack-syntax-coverage/src/formats/types.rs b/crates/sqlquack-syntax-coverage/src/formats/types.rs index 5e1fef53b..af59e907d 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/types.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/types.rs @@ -2,7 +2,10 @@ use std::collections::BTreeMap; -use crate::formats::models::{Definition, Form}; +use crate::formats::models::{Definition, Form, Receipt}; /// Each family's forms and definitions, in file order. pub(crate) type FamilyCatalogue = BTreeMap, Vec)>; + +/// The current receipts, the retired receipt line hashes, and the run reports' SQL by its hash. +pub(crate) type LoadedReceipts = (Vec, Vec, BTreeMap); diff --git a/crates/sqlquack-syntax-coverage/src/report/main/diff_snapshots.rs b/crates/sqlquack-syntax-coverage/src/report/main/diff_snapshots.rs index 35eeae8fa..fdd36711e 100644 --- a/crates/sqlquack-syntax-coverage/src/report/main/diff_snapshots.rs +++ b/crates/sqlquack-syntax-coverage/src/report/main/diff_snapshots.rs @@ -25,7 +25,33 @@ pub fn diff_snapshots(base: &Snapshot, head: &Snapshot) -> SnapshotDiff { .intersection(&head.syntax_error_cases_accepted) .cloned() .collect(); + let empty: BTreeSet = BTreeSet::new(); + let head_contradicted: &BTreeSet = + head.receipt_contradictions.as_ref().unwrap_or(&empty); + let new_contradictions: Vec = match &base.receipt_contradictions { + Some(base_contradicted) => head_contradicted + .difference(base_contradicted) + .cloned() + .collect(), + None => Vec::new(), + }; SnapshotDiff { + receipts_contradicted: new_contradictions + .iter() + .filter(|id| !head.documented_cases.contains(*id)) + .cloned() + .collect(), + documented_receipts_contradicted: new_contradictions + .iter() + .filter(|id| head.documented_cases.contains(*id)) + .cloned() + .collect(), + receipts_still_contradicted: head_contradicted + .iter() + .filter(|id| !new_contradictions.contains(id)) + .cloned() + .collect(), + unjudged_receipts: head.unjudged_receipts.iter().flatten().cloned().collect(), catalogue_files_changed: paths .into_iter() .filter(|path| base.catalogue_files.get(*path) != head.catalogue_files.get(*path)) diff --git a/crates/sqlquack-syntax-coverage/src/report/main/snapshot.rs b/crates/sqlquack-syntax-coverage/src/report/main/snapshot.rs index 003a8e2d0..32f0ee050 100644 --- a/crates/sqlquack-syntax-coverage/src/report/main/snapshot.rs +++ b/crates/sqlquack-syntax-coverage/src/report/main/snapshot.rs @@ -65,5 +65,7 @@ pub fn snapshot( syntax_error_cases_rejected: by_snowflake(check, SnowflakeSyntax::Refused, false), syntax_error_cases_accepted: by_snowflake(check, SnowflakeSyntax::Refused, true), documented_cases: coverage.documented_cases.clone(), + receipt_contradictions: Some(check.receipt_contradictions.clone()), + unjudged_receipts: Some(check.unjudged_receipts.clone()), } } diff --git a/crates/sqlquack-syntax-coverage/src/report/models.rs b/crates/sqlquack-syntax-coverage/src/report/models.rs index 6f78a8192..dbd6ef07f 100644 --- a/crates/sqlquack-syntax-coverage/src/report/models.rs +++ b/crates/sqlquack-syntax-coverage/src/report/models.rs @@ -73,6 +73,12 @@ pub struct Snapshot { /// Cases the catalogue marks documented (D113): their acceptance against a syntax error is intended. #[serde(default)] pub documented_cases: BTreeSet, + /// Every receipt case id whose statement the parser contradicts, cited or not; none before D141. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub receipt_contradictions: Option>, + /// Receipt case ids whose SQL no case or run report gives; none before D141. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub unjudged_receipts: Option>, } /// One snapshot against a later one; coverage and catalogue changes kept apart. @@ -95,4 +101,12 @@ pub struct SnapshotDiff { pub syntax_error_cases_accepted: Vec, /// The same flips on cases marked documented (D113): listed for the auditor, not failing (D135). pub documented_cases_accepted: Vec, + /// Receipts the head contradicts and the base did not, less documented ones: regressions (D141). + pub receipts_contradicted: Vec, + /// The same on receipts of cases marked documented (D113): listed, not failing (D135). + pub documented_receipts_contradicted: Vec, + /// Receipts both contradict, or every head contradiction when the base has no receipt verdicts. + pub receipts_still_contradicted: Vec, + /// Receipts the head cannot judge because no case or run report gives their SQL. + pub unjudged_receipts: Vec, } diff --git a/crates/sqlquack-syntax-coverage/src/report/tests/helpers.rs b/crates/sqlquack-syntax-coverage/src/report/tests/helpers.rs index 830ecd882..5f538264f 100644 --- a/crates/sqlquack-syntax-coverage/src/report/tests/helpers.rs +++ b/crates/sqlquack-syntax-coverage/src/report/tests/helpers.rs @@ -34,6 +34,8 @@ pub(crate) fn base() -> Snapshot { syntax_error_cases_rejected: BTreeSet::from(["sf.query.select.k3".to_owned()]), syntax_error_cases_accepted: BTreeSet::new(), documented_cases: BTreeSet::new(), + receipt_contradictions: Some(BTreeSet::new()), + unjudged_receipts: Some(BTreeSet::new()), } } diff --git a/crates/sqlquack-syntax-coverage/src/report/tests/test_diff_snapshots.rs b/crates/sqlquack-syntax-coverage/src/report/tests/test_diff_snapshots.rs index 8650825d4..3f7e3d11b 100644 --- a/crates/sqlquack-syntax-coverage/src/report/tests/test_diff_snapshots.rs +++ b/crates/sqlquack-syntax-coverage/src/report/tests/test_diff_snapshots.rs @@ -1,8 +1,10 @@ +use std::collections::BTreeSet; + use crate::check::models::FormState; use crate::report::main::diff_snapshots::diff_snapshots; use crate::report::models::{Snapshot, SnapshotDiff}; use crate::report::tests::helpers::{base, ids}; -use crate::report::tests::test_types::DiffTestCase; +use crate::report::tests::test_types::{BaseDiffTestCase, DiffTestCase}; #[test] fn given_a_later_snapshot_when_diffing_then_regressions_and_catalogue_changes_are_named() { @@ -105,6 +107,42 @@ fn given_a_later_snapshot_when_diffing_then_regressions_and_catalogue_changes_ar }, expected_diff: SnapshotDiff::default(), }, + DiffTestCase { + description: "a receipt the head newly contradicts, cited by a form or not, is a regression", + head: |head| Snapshot { + receipt_contradictions: Some(BTreeSet::from(["retired-1".to_owned()])), + ..head + }, + expected_diff: SnapshotDiff { + receipts_contradicted: ids(&["retired-1"]), + ..SnapshotDiff::default() + }, + }, + DiffTestCase { + description: "a documented case's receipt newly contradicted is listed apart (D135)", + head: |mut head| { + head.documented_cases + .insert("sf.query.select.k3".to_owned()); + head.receipt_contradictions = + Some(BTreeSet::from(["sf.query.select.k3".to_owned()])); + head + }, + expected_diff: SnapshotDiff { + documented_receipts_contradicted: ids(&["sf.query.select.k3"]), + ..SnapshotDiff::default() + }, + }, + DiffTestCase { + description: "a receipt without SQL to judge is listed, not a regression", + head: |head| Snapshot { + unjudged_receipts: Some(BTreeSet::from(["retired-2".to_owned()])), + ..head + }, + expected_diff: SnapshotDiff { + unjudged_receipts: ids(&["retired-2"]), + ..SnapshotDiff::default() + }, + }, DiffTestCase { description: "a verified form removed from the catalogue is also lost", head: |mut head| { @@ -123,3 +161,39 @@ fn given_a_later_snapshot_when_diffing_then_regressions_and_catalogue_changes_ar assert_eq!(diff, test_case.expected_diff, "{}", test_case.description); } } + +#[test] +fn given_a_base_without_receipt_verdicts_when_diffing_then_no_contradiction_is_new() { + let test_cases = [ + BaseDiffTestCase { + description: "every head contradiction is existing against a base from before D141", + base: |base| Snapshot { + receipt_contradictions: None, + ..base + }, + head: |head| Snapshot { + receipt_contradictions: Some(BTreeSet::from(["retired-1".to_owned()])), + ..head + }, + expected_diff: SnapshotDiff { + receipts_still_contradicted: ids(&["retired-1"]), + ..SnapshotDiff::default() + }, + }, + BaseDiffTestCase { + description: "a head from before D141 lists no receipts at all", + base: |base| base, + head: |head| Snapshot { + receipt_contradictions: None, + unjudged_receipts: None, + ..head + }, + expected_diff: SnapshotDiff::default(), + }, + ]; + for test_case in test_cases { + let diff: SnapshotDiff = + diff_snapshots(&(test_case.base)(base()), &(test_case.head)(base())); + assert_eq!(diff, test_case.expected_diff, "{}", test_case.description); + } +} diff --git a/crates/sqlquack-syntax-coverage/src/report/tests/test_types.rs b/crates/sqlquack-syntax-coverage/src/report/tests/test_types.rs index ec1e10877..842e7491b 100644 --- a/crates/sqlquack-syntax-coverage/src/report/tests/test_types.rs +++ b/crates/sqlquack-syntax-coverage/src/report/tests/test_types.rs @@ -10,6 +10,14 @@ pub(crate) struct DiffTestCase { pub(crate) expected_diff: SnapshotDiff, } +/// A change to both snapshots, and the diff it must produce. +pub(crate) struct BaseDiffTestCase { + pub(crate) description: &'static str, + pub(crate) base: fn(Snapshot) -> Snapshot, + pub(crate) head: fn(Snapshot) -> Snapshot, + pub(crate) expected_diff: SnapshotDiff, +} + /// Receipts and expectations set by `mutate`, a base and a head parser, and what the diff must flag. pub(crate) struct ParserChangeTestCase { pub(crate) description: &'static str, diff --git a/crates/sqlquack-syntax/src/expressions/statements/models.rs b/crates/sqlquack-syntax/src/expressions/statements/models.rs index 7e4a29516..55435bf91 100644 --- a/crates/sqlquack-syntax/src/expressions/statements/models.rs +++ b/crates/sqlquack-syntax/src/expressions/statements/models.rs @@ -503,6 +503,9 @@ pub struct Use { /// Snowflake's secondary roles one by one, quoting kept; empty for ALL, NONE and other kinds. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub roles: Vec, + /// Snowflake's qualified name part by part, quoting kept; empty for a one-part name. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub parts: Vec, } /// Kind of USE statement diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/alter.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/alter.rs index b86cb66bf..d554ac203 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/alter.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/alter.rs @@ -1486,6 +1486,7 @@ impl Parser { true => listed, false => Vec::new(), }, + parts: Vec::new(), }))); } } @@ -1517,10 +1518,14 @@ impl Parser { let mut ident = self.expect_identifier_or_keyword_with_quoted()?; self.reject_reserved_name_part(&ident)?; + let mut parts: Vec = Vec::new(); if self.match_token(TokenType::Dot) { let second_part = self.expect_identifier_or_keyword_with_quoted()?; self.reject_reserved_name_part(&second_part)?; + if self.applies_snowflake_context_rules() { + parts = vec![ident.clone(), second_part.clone()]; + } ident.name = format!("{}.{}", ident.name, second_part.name); } @@ -1528,6 +1533,7 @@ impl Parser { kind, this: ident, roles: Vec::new(), + parts, }))) } diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/commands.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/commands.rs index 821453e71..2ccd6daa7 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/commands.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/commands.rs @@ -1445,7 +1445,6 @@ impl Parser { let (parameters, has_parens) = if self.match_token(TokenType::LParen) { let params = self.parse_function_parameters()?; self.expect_token(TokenType::RParen)?; - self.check_snowflake_routine_arguments(¶ms)?; (params, true) } else { (Vec::new(), false) diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/create_table.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/create_table.rs index b2f33a777..2eedb950c 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/create_table.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/create_table.rs @@ -399,7 +399,11 @@ impl Parser { snowflake_inline_properties, )?; - let as_select = self.parse_create_table_column_list_query(no_column_defs, &columns)?; + let as_select = self.parse_create_table_column_list_query( + no_column_defs, + &columns, + table_modifier == Some("EXTERNAL"), + )?; table_properties = self.parse_create_table_partitioning(table_properties)?; @@ -564,18 +568,23 @@ impl Parser { Ok(table_properties) } - /// The `AS ` after a column list; Snowflake requires one when a listed column has no type. + /// The `AS ` after a column list; Snowflake requires one when a non-external table's column has no type. fn parse_create_table_column_list_query( &mut self, no_column_defs: bool, columns: &[ColumnDef], + external: bool, ) -> Result> { let as_select = if !no_column_defs && self.match_token(TokenType::As) { Some(self.parse_statement()?) } else { None }; - if self.is_snowflake() && as_select.is_none() && columns.iter().any(|c| c.no_type) { + if self.is_snowflake() + && !external + && as_select.is_none() + && columns.iter().any(|c| c.no_type) + { return Err(self.parse_error("Expected a data type for the column")); } Ok(as_select) diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_dynamic.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_dynamic.rs index 83e863a64..02b90807d 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_dynamic.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_dynamic.rs @@ -51,7 +51,11 @@ const ACTIONS: &[&[&str]] = &[ impl Parser { /// Parse ALTER DYNAMIC TABLE after its kind. pub(super) fn parse_alter_dynamic_table(&mut self) -> Result { - let head = self.parse_alter_object_head()?; + let head = if self.omits_dynamic_table_name() { + (crate::expressions::names::models::TableRef::new(""), false) + } else { + self.parse_alter_object_head()? + }; let mut names: Vec = Vec::new(); while self.match_token(TokenType::Comma) { names.push(Expression::Table(Box::new(self.parse_object_name()?))); @@ -71,6 +75,16 @@ impl Parser { )) } + /// Whether the table name is omitted before `SET =` which Snowflake parses and refuses by name check. + fn omits_dynamic_table_name(&self) -> bool { + let follows = |offset: usize, types: &[TokenType]| { + self.tokens + .get(self.current + offset) + .is_some_and(|token| types.contains(&token.token_type)) + }; + (self.check_identifier("SET") || self.check(TokenType::Set)) && follows(2, &[TokenType::Eq]) + } + /// Parse what follows the words of an ALTER DYNAMIC TABLE action. fn parse_dynamic_table_action(&mut self, action: &str) -> Result { Ok(match action { @@ -80,7 +94,7 @@ impl Parser { self.expect_alter_words(&[&["ADD", "ROW", "ACCESS", "POLICY"]])?; let added = self.policy_operand()?; self.expect_token(TokenType::On)?; - return Ok((vec![name, added, self.column_list_operand()?], vec![])); + return Ok((vec![name, added, self.row_access_columns()?], vec![])); } (vec![name], vec![]) } @@ -100,7 +114,11 @@ impl Parser { let mut targets: Vec = Vec::new(); if self.match_token(TokenType::On) { loop { - targets.push(self.search_target(action.starts_with("ADD"))?); + if action.starts_with("DROP") && self.check(TokenType::String) { + targets.push(self.parse_primary()?); + } else { + targets.push(self.search_target(action.starts_with("ADD"))?); + } if !self.match_token(TokenType::Comma) { break; } @@ -108,11 +126,16 @@ impl Parser { } (targets, vec![]) } - "ADD STORAGE LIFECYCLE POLICY" | "ADD ROW ACCESS POLICY" => { + "ADD STORAGE LIFECYCLE POLICY" => { let policy = self.policy_operand()?; self.expect_token(TokenType::On)?; (vec![policy, self.column_list_operand()?], vec![]) } + "ADD ROW ACCESS POLICY" => { + let policy = self.policy_operand()?; + self.expect_token(TokenType::On)?; + (vec![policy, self.row_access_columns()?], vec![]) + } "SET AGGREGATION POLICY" => { let policy = self.policy_operand()?; let mut properties: Vec = Vec::new(); @@ -129,6 +152,15 @@ impl Parser { } (vec![policy], properties) } + "UNSET TAG" if self.is_at_end() || self.check(TokenType::Semicolon) => (vec![], vec![]), + "SET TAG" if self.check(TokenType::Eq) => { + self.skip(); + let value = self.parse_object_value()?; + ( + vec![], + vec![property("TAG", Some(Identifier::new("")), Some(value))], + ) + } "SET TAG" | "UNSET TAG" => (vec![], self.parse_tag_list(action == SET_TAG)?), "SET" | "UNSET" => (vec![], self.parse_dynamic_table_properties(action == SET)?), "ALTER" | "MODIFY" => (self.parse_dynamic_column_actions()?, vec![]), @@ -141,6 +173,15 @@ impl Parser { Ok(Expression::Table(Box::new(self.parse_object_name()?))) } + /// Parse the row access policy columns, which Snowflake parses even when empty. + fn row_access_columns(&mut self) -> Result { + if self.check(TokenType::LParen) && self.check_next(TokenType::RParen) { + self.current += 2; + return Ok(tuple(vec![])); + } + self.column_list_operand() + } + /// Parse `( [ , ... ] )`. fn column_list_operand(&mut self) -> Result { self.expect_token(TokenType::LParen)?; diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_commands.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_commands.rs index 3dcb03e15..d2f7d318b 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_commands.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_commands.rs @@ -246,24 +246,32 @@ impl Parser { /// Reads Snowflake CREATE TASK properties up to AS, refusing items that are not tags, contacts, assignments or clauses. pub(crate) fn check_snowflake_task_properties(&mut self, graph: bool) -> Result<()> { + let mut executes_as = false; while !(graph && (self.is_at_end() || self.check(TokenType::Semicolon))) && !(!graph && self.check(TokenType::As)) { self.match_token(TokenType::With); if self.match_identifier("TAG") { self.skip_snowflake_task_assignments(|parser| { - parser.expect_token(TokenType::String).map(|_| ()) + if parser.match_token(TokenType::String) { + return Ok(()); + } + parser.skip_snowflake_name() })?; } else if self.match_identifier("CONTACT") { self.skip_snowflake_task_assignments(Self::skip_snowflake_name)?; } else if self.match_identifier("TASK_DEFAULTS") { self.skip_snowflake_task_assignments(Self::skip_snowflake_task_value)?; } else if self.match_token(TokenType::After) { + if executes_as { + return Err(self.parse_error("AFTER comes before EXECUTE AS USER")); + } self.skip_snowflake_name()?; while self.match_token(TokenType::Comma) { self.skip_snowflake_name()?; } } else if self.match_text_seq(&["EXECUTE", "AS", "USER"]) { + executes_as = true; self.skip_snowflake_name()?; } else if self.match_token(TokenType::When) { if self.check(TokenType::As) { @@ -311,6 +319,8 @@ impl Parser { } self.match_token(TokenType::Dash); if self.check(TokenType::String) + || self.check(TokenType::DollarString) + || self.check(TokenType::HeredocString) || self.check(TokenType::Number) || self.check(TokenType::True) || self.check(TokenType::False) diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_create_from.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_create_from.rs index af767c26f..d161b5273 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_create_from.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_create_from.rs @@ -96,7 +96,7 @@ impl Parser { })))) } - /// Parses `MODEL ... FROM MODEL`, `GATEWAY ... FROM SPECIFICATION` or a bare `EXPERIMENT`; None for other forms. + /// Parses `MODEL ... FROM MODEL`, `{ GATEWAY | EXPERIMENT } ... FROM SPECIFICATION` or a bare `EXPERIMENT`; None for other forms. fn parse_snowflake_create_ml_object(&mut self, or_replace: bool) -> Result> { let kind = if self.check_identifier("MODEL") && !self.check_next_identifier("MONITOR") { "MODEL" @@ -138,7 +138,10 @@ impl Parser { } "MODEL" } - "GATEWAY" => { + "GATEWAY" if !self.check(TokenType::From) => { + return Err(self.parse_error("Expected FROM SPECIFICATION")); + } + _ if self.check(TokenType::From) => { self.expect_token(TokenType::From)?; self.expect_alter_words(&[&["SPECIFICATION"]])?; if !self.check(TokenType::String) && !self.check(TokenType::DollarString) { @@ -215,6 +218,13 @@ impl Parser { None, Some(value), )); + if self.check(TokenType::Comma) + && self + .peek_nth(1) + .is_some_and(|token| token.token_type != TokenType::Semicolon) + { + self.skip(); + } } Ok(Some(Expression::CreateObject(Box::new(CreateObject { kind: kind.to_owned(), diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_routines.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_routines.rs index 817f8a89f..359b08fe5 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_routines.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_routines.rs @@ -1,7 +1,6 @@ //! Snowflake function, procedure and object-name rules that the shared parsers do not apply. use crate::expressions::names::models::TableRef; -use crate::expressions::schema_objects::models::FunctionParameter; use crate::parser::constants::{ SNOWFLAKE_RESERVED_KEYWORDS, SNOWFLAKE_ROUTINE_NAME_LIST_PROPERTIES, SNOWFLAKE_ROUTINE_NAME_MAP_PROPERTIES, SNOWFLAKE_ROUTINE_NAME_PROPERTIES, @@ -97,7 +96,7 @@ impl Parser { Ok(()) } - /// In a Snowflake parse, refuses `TABLE ( ... )` with a reserved column name or a column lacking its type. + /// In a Snowflake parse, refuses `TABLE ( ... )` with a reserved column name. pub(crate) fn check_snowflake_table_columns(&self) -> Result<()> { if !self.is_snowflake() || !self.check(TokenType::Table) @@ -122,31 +121,13 @@ impl Parser { .iter() .any(|word| next.text.eq_ignore_ascii_case(word)) }); - let untyped = next.is_some_and(|next| next.token_type != TokenType::RParen) - && self - .tokens - .get(self.current + offset + 3) - .is_some_and(|after| { - matches!(after.token_type, TokenType::Comma | TokenType::RParen) - }); - if opens_column && (reserved || untyped) { - return Err(self.parse_error("Expected a column name and type in TABLE ( ... )")); + if opens_column && reserved { + return Err(self.parse_error("Expected a column name in TABLE ( ... )")); } } Ok(()) } - /// In a Snowflake parse, refuses a routine argument that lacks its name or its type. - pub(crate) fn check_snowflake_routine_arguments( - &self, - arguments: &[FunctionParameter], - ) -> Result<()> { - if self.is_snowflake() && arguments.iter().any(|argument| argument.name.is_none()) { - return Err(self.parse_error("Expected a name and a type for each argument")); - } - Ok(()) - } - /// In a Snowflake parse, refuses a ROW ACCESS POLICY body that is not ` ON ( , ... )`. pub(crate) fn check_snowflake_row_access_policy(&mut self) -> Result<()> { if !self.is_snowflake() { diff --git a/crates/sqlquack-syntax/src/parser/_helpers/expressions/operators.rs b/crates/sqlquack-syntax/src/parser/_helpers/expressions/operators.rs index 5509f79aa..b04abe71c 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/expressions/operators.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/expressions/operators.rs @@ -283,7 +283,6 @@ impl Parser { let (parameters, has_parens) = if self.match_token(TokenType::LParen) { let params = self.parse_function_parameters()?; self.expect_token(TokenType::RParen)?; - self.check_snowflake_routine_arguments(¶ms)?; (params, true) } else if self.check(TokenType::Var) && !self.check(TokenType::As) { let params = self.parse_tsql_procedure_params()?; @@ -444,7 +443,11 @@ impl Parser { let if_not_exists = self.match_keywords(&[TokenType::If, TokenType::Not, TokenType::Exists]); - let name = self.parse_table_ref()?; + let name = if self.applies_snowflake_context_rules() { + self.parse_object_name()? + } else { + self.parse_table_ref()? + }; let mut seq = CreateSequence { name, diff --git a/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs b/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs index fbb4eb55b..d00b724d5 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs @@ -265,6 +265,7 @@ impl Parser { /// Parses every remaining statement, split on semicolons, one `Expression` each. pub(crate) fn parse(&mut self) -> Result> { self.ensure_complexity_guards()?; + self.reject_snowflake_backticks()?; let mut statements: Vec = Vec::new(); while !self.is_at_end() { diff --git a/crates/sqlquack-syntax/src/parser/_helpers/statements/snowflake_context.rs b/crates/sqlquack-syntax/src/parser/_helpers/statements/snowflake_context.rs index 3951f1cf9..2c454b1d1 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/statements/snowflake_context.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/statements/snowflake_context.rs @@ -12,6 +12,25 @@ impl Parser { && !self.config.record_call_spans } + /// Refuses a backtick-quoted identifier, which Snowflake does not tokenize, in its faithful modes. + pub(crate) fn reject_snowflake_backticks(&mut self) -> Result<()> { + if !self.applies_snowflake_context_rules() { + return Ok(()); + } + let backtick = self.tokens.iter().position(|token| { + token.token_type == crate::tokens::models::TokenType::QuotedIdentifier + && self.source_text_range(token.span.start, token.span.start + 1) + == crate::parser::constants::BACKTICK + }); + match backtick { + Some(index) => { + self.current = index; + Err(self.parse_error("Snowflake does not quote identifiers with backticks")) + } + None => Ok(()), + } + } + /// Refuses TABLE( ) or IDENTIFIER( ) around nothing or a bare name, except a named argument's TABLE(name). pub(crate) fn reject_snowflake_name_wrappers(&self, statements: &[Expression]) -> Result<()> { use crate::traversal::types::ExpressionWalk; diff --git a/crates/sqlquack-syntax/src/parser/constants.rs b/crates/sqlquack-syntax/src/parser/constants.rs index 3322a89b4..ac25e0b6e 100644 --- a/crates/sqlquack-syntax/src/parser/constants.rs +++ b/crates/sqlquack-syntax/src/parser/constants.rs @@ -924,6 +924,9 @@ pub(crate) const SNOWFLAKE_INTERVAL_LEADING: std::ops::RangeInclusive = 1..= /// Snowflake's recorded fractional-second precisions after TO, as in `DAY TO SECOND ( 0 )`. pub(crate) const SNOWFLAKE_INTERVAL_FRACTION: std::ops::RangeInclusive = 0..=9; +/// The backtick that quotes identifiers in other dialects but not in Snowflake. +pub(crate) const BACKTICK: &str = "`"; + /// Snowflake function and procedure properties whose value is a string. pub(crate) const SNOWFLAKE_ROUTINE_TEXT_PROPERTIES: &[&str] = &[ "COMMENT", diff --git a/crates/sqlquack-syntax/tests/parser/test_snowflake_fixes.rs b/crates/sqlquack-syntax/tests/parser/test_snowflake_fixes.rs index 98d41c27d..099a1d391 100644 --- a/crates/sqlquack-syntax/tests/parser/test_snowflake_fixes.rs +++ b/crates/sqlquack-syntax/tests/parser/test_snowflake_fixes.rs @@ -1035,14 +1035,14 @@ fn given_a_snowflake_function_or_procedure_when_parsing_then_its_properties_are_ expected_kind: None, }, SnowflakeVerdictTestCase { - description: "an argument with a type and no name (recorded errno 1003)", + description: "an argument with a type and no name (recorded errno 90201, import 137)", sql: "DEFINE PROCEDURE d1.s1.v1 ( INT ) RETURNS INT LANGUAGE JAVASCRIPT AS 'm' ;", - expected_kind: None, + expected_kind: Some("create_procedure"), }, SnowflakeVerdictTestCase { - description: "a returned table column with no type (recorded errno 1003)", + description: "a returned table column with no type (recorded errno 90214, import 137)", sql: "DEFINE PROCEDURE d1.s1.v1 ( ) RETURNS TABLE ( v1 ) LANGUAGE SQL AS $$ BEGIN RETURN 1 ; END $$ ;", - expected_kind: None, + expected_kind: Some("create_procedure"), }, SnowflakeVerdictTestCase { description: "a reserved procedure name (recorded errno 1003)", diff --git a/crates/sqlquack-syntax/tests/parser/test_snowflake_recorded.rs b/crates/sqlquack-syntax/tests/parser/test_snowflake_recorded.rs index a3a944b1c..a7c8506e2 100644 --- a/crates/sqlquack-syntax/tests/parser/test_snowflake_recorded.rs +++ b/crates/sqlquack-syntax/tests/parser/test_snowflake_recorded.rs @@ -749,6 +749,24 @@ fn given_snowflake_create_and_begin_name_when_parsing_then_the_tree_keeps_the_el pointer: "/0/select/expressions/0/cast/to/name", expected_value: json!("PERIOD ( INT )"), }, + ProjectionTestCase { + description: "USE SCHEMA keeps the database part's quoting", + sql: "USE SCHEMA \"v1 Q\" . v1", + pointer: "/0/use/parts/0/quoted", + expected_value: json!(true), + }, + ProjectionTestCase { + description: "USE SCHEMA keeps the schema part apart from the database", + sql: "USE SCHEMA \"v1 Q\" . v1", + pointer: "/0/use/parts/1/name", + expected_value: json!("v1"), + }, + ProjectionTestCase { + description: "USE SCHEMA with a quoted dotted name has one part", + sql: "USE SCHEMA \"v1 Q.v1\"", + pointer: "/0/use/parts", + expected_value: json!(null), + }, ]; for test_case in test_cases { @@ -760,3 +778,180 @@ fn given_snowflake_create_and_begin_name_when_parsing_then_the_tree_keeps_the_el ); } } + +#[test] +fn given_receipts_contradicted_by_pr_68_when_parsing_then_acceptance_matches_snowflake() { + let test_cases = [ + AcceptanceTestCase { + description: "a function argument may omit its name (recorded errno 90201)", + sql: "CREATE FUNCTION v1 ( INT ) RETURNS INT AS 'm' ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a function argument may omit its type (recorded errno 90201)", + sql: "CREATE FUNCTION v1 ( v1 ) RETURNS INT LANGUAGE JAVASCRIPT AS 'm' ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a procedure argument may omit its type (recorded errno 90201)", + sql: "CREATE PROCEDURE v1 ( v1 ) RETURNS INT LANGUAGE SQL AS $$ BEGIN RETURN 1 ; END $$ ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a procedure's RETURNS TABLE column may omit its type (recorded errno 90214)", + sql: "CREATE PROCEDURE v1 ( ) RETURNS TABLE ( v1 ) LANGUAGE SQL AS $$ BEGIN RETURN 1 ; END $$ ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a procedure's RETURNS TABLE column may omit its name (recorded success)", + sql: "CREATE PROCEDURE v1 ( ) RETURNS TABLE ( INT ) LANGUAGE SQL AS $$ BEGIN RETURN 1 ; END $$ ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "SHOW GRANTS OF reads a kind word as the name (recorded errno 2003)", + sql: "SHOW GRANTS OF ROLE ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "SHOW GRANTS ON takes a bare name (recorded errno 2003)", + sql: "SHOW GRANTS ON v1 ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "SHOW GRANTS TO takes an empty kind (recorded errno 2003)", + sql: "SHOW GRANTS TO APPLICATION ROLE ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "SHOW GRANTS OF APPLICATION ROLE needs a name (recorded errno 1003)", + sql: "SHOW GRANTS OF APPLICATION ROLE ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "a task tag value may be a name (recorded errno 391805)", + sql: "CREATE TASK v1 WITH TAG ( v1 = m1 ) AS SELECT 1 ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a task property takes a dollar-quoted value and commas (recorded success)", + sql: "CREATE TASK v1 CONFIG = $$x$$ TIMEZONE = 'UTC', AUTOCOMMIT = TRUE AS SELECT 1", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a task's AFTER comes before EXECUTE AS USER (recorded errno 1003)", + sql: "CREATE TASK t2 WAREHOUSE = wh EXECUTE AS USER u1 AFTER t1 AS SELECT 1", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "a task's EXECUTE AS USER follows AFTER (recorded success)", + sql: "CREATE TASK t2 AFTER t1 EXECUTE AS USER u1 AS SELECT 1", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "DEFINE SEQUENCE refuses a reserved name part (recorded errno 1003)", + sql: "DEFINE SEQUENCE d1.s1.SELECT ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "an experiment may omit its TYPE (recorded errno 400022)", + sql: "CREATE EXPERIMENT v1 FROM SPECIFICATION $$$$ ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "stage options may be comma-separated (recorded success)", + sql: "CREATE STAGE s1 COMMENT = 'x', FILE_FORMAT = ( TYPE = CSV )", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "file format options may be comma-separated (recorded success)", + sql: "CREATE FILE FORMAT f1 TYPE = 'CSV' FIELD_DELIMITER = '|', SKIP_HEADER = 2", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "Snowflake has no backtick identifiers (recorded errno 1003)", + sql: "CREATE PROCEDURE v1 ( ) RETURNS INT LANGUAGE PYTHON RUNTIME_VERSION = '3.10' ARTIFACT_REPOSITORY = ` v1 ` HANDLER = 'm1.run' ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "ALTER DYNAMIC TABLE may omit the name before SET (recorded errno 926)", + sql: "ALTER DYNAMIC TABLE SET TARGET_LAG = '1 minute' ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "ALTER DYNAMIC TABLE refuses a missing name before UNSET (recorded errno 1003)", + sql: "ALTER DYNAMIC TABLE UNSET INITIALIZATION_WAREHOUSE ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "a dynamic table's UNSET TAG may be empty (recorded errno 1420)", + sql: "ALTER DYNAMIC TABLE v1 UNSET TAG ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a dynamic table's SET TAG may omit the tag name (recorded errno 1420)", + sql: "ALTER DYNAMIC TABLE v1 SET TAG = 'm' ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a dynamic table column's UNSET TAG needs a tag (recorded errno 1003)", + sql: "ALTER DYNAMIC TABLE v1 ALTER v1 UNSET TAG ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "a row access policy may name no columns (recorded errno 2003)", + sql: "ALTER DYNAMIC TABLE v1 ADD ROW ACCESS POLICY v1 ON ( ) ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a storage lifecycle policy needs columns (recorded errno 1003)", + sql: "ALTER DYNAMIC TABLE v1 ADD STORAGE LIFECYCLE POLICY v1 ON ( ) ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "DROP SEARCH OPTIMIZATION takes a string target (recorded errno 904)", + sql: "ALTER DYNAMIC TABLE v1 DROP SEARCH OPTIMIZATION ON 'v1' ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "RESUME SEARCH OPTIMIZATION refuses a string target (recorded errno 1003)", + sql: "ALTER DYNAMIC TABLE v1 RESUME SEARCH OPTIMIZATION ON 'v1' ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + ]; + + for test_case in test_cases { + assert_eq!( + accepted(test_case.sql, test_case.dialect), + test_case.expected_accepted, + "{}", + test_case.description + ); + } +} diff --git a/docs/syntax-coverage.md b/docs/syntax-coverage.md index 0cdfa6ef4..6c1ad6957 100644 --- a/docs/syntax-coverage.md +++ b/docs/syntax-coverage.md @@ -416,6 +416,24 @@ tree, so any parser commit makes its verified forms stale until it is renewed; a fingerprints goes stale only for the forms whose cases now parse differently. Either shows up as a regression. +## Receipt contradictions (decision D141) + +The case lists above only reach receipts through the catalogue: a receipt whose case id no form +cites, such as a retired or hand-written case, never counts. A snapshot therefore also records +`receipt_contradictions`: every current receipt, cited or not, whose statement the parser +contradicts. Statements are grouped by SQL, setup and session hashes and judged as above: the parser +rejects a statement Snowflake parsed, or accepts one Snowflake refused as syntax. The SQL comes from +the case that names the hash or from the `inputs` of the run reports beside the receipts, each +checked against its hash. A receipt neither gives is listed in `unjudged_receipts` and judged by +nothing. `diff` fails on a receipt the head contradicts and the base did not. A receipt of a case +marked `documented (D113)` is listed on its own line instead (decision D135). Receipts both sides +contradict, and every head contradiction when the base snapshot predates these fields, are listed in +the JSON without failing. + +The tracked baseline and receipts are a subset of the evidence, so a merge gate takes both snapshots +on the full catalogue and evidence: `snapshot` with the target branch's binary, `snapshot` with the +head's binary, then `diff BASE HEAD`. + ## Translation `syntax-coverage translation --sqlquack [--policy sqlquack.toml] [--family ]