Skip to content
Merged
Original file line number Diff line number Diff line change
@@ -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<String>, BTreeSet<String>) {
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<String> = BTreeSet::new();
let mut unjudged: BTreeSet<String> = 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)
}
1 change: 1 addition & 0 deletions crates/sqlquack-syntax-coverage/src/check/_helpers/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
pub(crate) mod contradictions;
pub(crate) mod fingerprints;
pub(crate) mod forms;
pub(crate) mod join;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ fn agreement(phase: Phase, local: &LocalVerdict, terminates: bool) -> Result<boo
}

/// What Snowflake's parser did by the syntax receipts of the case's statement; none unless all agree.
fn snowflake_syntax(statement_receipts: &[&Receipt]) -> Option<SnowflakeSyntax> {
pub(crate) fn snowflake_syntax(statement_receipts: &[&Receipt]) -> Option<SnowflakeSyntax> {
let verdicts: BTreeSet<Option<SnowflakeSyntax>> = statement_receipts
.iter()
.filter(|receipt| receipt.measure.is_none())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -31,10 +32,14 @@ pub fn check_coverage(
forms.push(form_result(form, family, &context));
}
}
let (contradicted, unjudged): (BTreeSet<String>, BTreeSet<String>) =
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,
}
}
4 changes: 4 additions & 0 deletions crates/sqlquack-syntax-coverage/src/check/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,10 @@ pub struct CheckResult {
pub unexcluded_pages: Vec<String>,
/// References between files that do not resolve; any one fails the check.
pub problems: Vec<String>,
/// Receipt case ids whose statement the parser contradicts, cited by a form or not (D141).
pub receipt_contradictions: BTreeSet<String>,
/// Receipt case ids whose SQL no case or run report gives, so no verdict can be compared.
pub unjudged_receipts: BTreeSet<String>,
}

/// The files joined for checking, with every case already judged.
Expand Down
14 changes: 14 additions & 0 deletions crates/sqlquack-syntax-coverage/src/check/tests/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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
);
}
}
9 changes: 9 additions & 0 deletions crates/sqlquack-syntax-coverage/src/check/tests/test_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
}
32 changes: 29 additions & 3 deletions crates/sqlquack-syntax-coverage/src/cli/_helpers/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,8 +339,16 @@ fn read_snapshot(path: &Path) -> Result<Snapshot, ExitCode> {

/// Lines naming each non-empty list of the diff.
fn diff_lines(diff: &SnapshotDiff) -> Vec<String> {
let lists: [(&str, &Vec<String>); 10] = [
let lists: [(&str, &Vec<String>); 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",
Expand All @@ -360,11 +368,28 @@ fn diff_lines(diff: &SnapshotDiff) -> Vec<String> {
("forms gained", &diff.forms_gained),
("obligations gained", &diff.obligations_gained),
];
lists
let mut lines: Vec<String> = 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.
Expand All @@ -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,
Expand Down
26 changes: 21 additions & 5 deletions crates/sqlquack-syntax-coverage/src/formats/_helpers/retired.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>, Vec<String>) {
/// 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<String>, BTreeMap<String, String>, Vec<String>) {
let mut withdrawn: BTreeSet<String> = BTreeSet::new();
let mut statements: BTreeMap<String, String> = BTreeMap::new();
let mut problems: Vec<String> = Vec::new();
for path in reports {
let report: Value = match read_text(path).and_then(|(text, _)| {
Expand All @@ -38,8 +41,21 @@ pub(crate) fn withdrawn_receipts(reports: &[PathBuf]) -> (BTreeSet<String>, 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 {
Expand Down
14 changes: 7 additions & 7 deletions crates/sqlquack-syntax-coverage/src/formats/_helpers/sections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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<T> {
Expand Down Expand Up @@ -206,8 +206,8 @@ pub(crate) fn load_marks(root: &Path) -> Section<Vec<Mark>> {
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<Receipt>, Vec<String>)> {
/// 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<LoadedReceipts> {
let (paths, others) = listed(&root.join(RECEIPTS_DIR), "jsonl");
let mut problems: Vec<String> = others
.iter()
Expand All @@ -226,11 +226,11 @@ pub(crate) fn load_receipts(root: &Path) -> Section<(Vec<Receipt>, Vec<String>)>
);
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/<family>.toml`: optional, and each obligation listed at most once.
Expand Down Expand Up @@ -316,7 +316,7 @@ pub(crate) struct Sections {
pub(crate) catalogue: Section<FamilyCatalogue>,
pub(crate) obligations: Section<BTreeMap<String, Vec<Obligation>>>,
pub(crate) cases: Section<BTreeMap<String, Vec<Case>>>,
pub(crate) receipts: Section<(Vec<Receipt>, Vec<String>)>,
pub(crate) receipts: Section<LoadedReceipts>,
pub(crate) reviews: Section<Vec<Review>>,
pub(crate) unobservable: Section<(Vec<Unobservable>, Vec<Diagnostic>)>,
pub(crate) readings: Section<Vec<Reading>>,
Expand Down
2 changes: 2 additions & 0 deletions crates/sqlquack-syntax-coverage/src/formats/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading
Loading