Skip to content

Commit 9d732b9

Browse files
committed
Join every changed case to Snowflake's receipts in the coverage diff, whatever it expects
The snapshot took Snowflake's verdict from a case's checked phase, which is none for every observe case, so a receipted observe case the parser stopped accepting never reached the diff (#55's three regressions, audit 38bed22). Each case result now carries what Snowflake's parser did, by every syntax receipt of its SQL, setup and session whatever the case id: parsed when all stopped after parsing with an errno other than 1003, refused when all are a positioned 1003 or a 921, and nothing for a positionless 1003 or receipts that disagree. The diff fails on a parsed case the parser now rejects, as before, and also on a refused case it now accepts.
1 parent 77e10ee commit 9d732b9

13 files changed

Lines changed: 294 additions & 13 deletions

File tree

‎crates/sqlquack-syntax-coverage/src/check/_helpers/join.rs‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,20 +51,44 @@ fn receipts_by_case(coverage: &SyntaxCoverage) -> BTreeMap<&str, Vec<&Receipt>>
5151
receipts
5252
}
5353

54+
/// The receipts of each statement, by its SQL, setup and session hashes, whatever the case id.
55+
fn receipts_by_statement(coverage: &SyntaxCoverage) -> BTreeMap<(&str, &str, &str), Vec<&Receipt>> {
56+
let mut receipts: BTreeMap<(&str, &str, &str), Vec<&Receipt>> = BTreeMap::new();
57+
for receipt in &coverage.receipts {
58+
receipts
59+
.entry((
60+
receipt.sql_sha256.as_str(),
61+
receipt.setup_sha256.as_str(),
62+
receipt.session_sha256.as_str(),
63+
))
64+
.or_default()
65+
.push(receipt);
66+
}
67+
receipts
68+
}
69+
5470
/// Every join the checker needs, with each case judged against `parse`.
5571
pub(crate) fn join<'a>(
5672
coverage: &'a SyntaxCoverage,
5773
parse: &dyn Fn(&str) -> LocalVerdict,
5874
) -> Joined<'a> {
5975
let receipts: BTreeMap<&str, Vec<&Receipt>> = receipts_by_case(coverage);
76+
let statements: BTreeMap<(&str, &str, &str), Vec<&Receipt>> = receipts_by_statement(coverage);
6077
let mut cases: BTreeMap<&str, CaseResult> = BTreeMap::new();
6178
for case in coverage.cases.values().flatten() {
6279
let found: &[&Receipt] = receipts
6380
.get(case.case_id.as_str())
6481
.map_or(&[], Vec::as_slice);
82+
let statement: &[&Receipt] = statements
83+
.get(&(
84+
case.sql_sha256.as_str(),
85+
case.setup_sha256.as_str(),
86+
case.session_sha256.as_str(),
87+
))
88+
.map_or(&[], Vec::as_slice);
6589
cases.insert(
6690
case.case_id.as_str(),
67-
case_result(case, found, parse(&case.sql)),
91+
case_result(case, found, statement, parse(&case.sql)),
6892
);
6993
}
7094
Joined {

‎crates/sqlquack-syntax-coverage/src/check/_helpers/receipts.rs‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
33
use std::collections::{BTreeMap, BTreeSet};
44

5-
use crate::check::models::{CaseResult, LocalVerdict, Reason};
5+
use crate::check::models::{CaseResult, LocalVerdict, Reason, SnowflakeSyntax};
6+
use crate::formats::constants::{SYNTAX_ERRNO, SYNTAX_ERRNO_42601};
67
use crate::formats::models::{AccountSetting, Case, Expect, Phase, Receipt};
78

89
/// A syntax receipt bound to exactly this case's SQL, setup, session and capture, with a query id.
@@ -61,8 +62,35 @@ fn agreement(phase: Phase, local: &LocalVerdict, terminates: bool) -> Result<boo
6162
}
6263
}
6364

64-
/// The case's result against its receipts and the current parser's verdict.
65-
pub(crate) fn case_result(case: &Case, receipts: &[&Receipt], local: LocalVerdict) -> CaseResult {
65+
/// What Snowflake's parser did by the syntax receipts of the case's statement; none unless all agree.
66+
fn snowflake_syntax(statement_receipts: &[&Receipt]) -> Option<SnowflakeSyntax> {
67+
let verdicts: BTreeSet<Option<SnowflakeSyntax>> = statement_receipts
68+
.iter()
69+
.filter(|receipt| receipt.measure.is_none())
70+
.map(|receipt| match (receipt.phase, receipt.errno) {
71+
(Phase::SyntaxError, Some(errno))
72+
if errno == SYNTAX_ERRNO || errno == SYNTAX_ERRNO_42601 =>
73+
{
74+
Some(SnowflakeSyntax::Refused)
75+
}
76+
(Phase::SyntaxError, _) => None,
77+
(_, Some(errno)) if errno == SYNTAX_ERRNO => None,
78+
_ => Some(SnowflakeSyntax::Parsed),
79+
})
80+
.collect();
81+
match verdicts.len() {
82+
1 => verdicts.into_iter().next().flatten(),
83+
_ => None,
84+
}
85+
}
86+
87+
/// The case's result against its receipts, its statement's receipts and the parser's verdict.
88+
pub(crate) fn case_result(
89+
case: &Case,
90+
receipts: &[&Receipt],
91+
statement_receipts: &[&Receipt],
92+
local: LocalVerdict,
93+
) -> CaseResult {
6694
let (phase, reason, semantic_only): (Option<Phase>, Option<Reason>, bool) =
6795
match receipt_phase(case, receipts) {
6896
_ if case.expect == Some(Expect::Observe) => {
@@ -84,6 +112,7 @@ pub(crate) fn case_result(case: &Case, receipts: &[&Receipt], local: LocalVerdic
84112
local,
85113
reason,
86114
semantic_only,
115+
snowflake: snowflake_syntax(statement_receipts),
87116
account_state: receipts
88117
.iter()
89118
.filter(|receipt| admissible(case, receipt))

‎crates/sqlquack-syntax-coverage/src/check/models.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,16 @@ pub enum Reason {
7676
BlockedByHold,
7777
}
7878

79+
/// What Snowflake's parser did with a case's statement, by every receipt of its SQL, setup and session.
80+
#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Serialize)]
81+
#[serde(rename_all = "snake_case")]
82+
pub enum SnowflakeSyntax {
83+
/// Every receipt stopped after parsing or succeeded, with an errno other than 1003.
84+
Parsed,
85+
/// Every receipt is a syntax error: errno 1003 with a parse position, or errno 921.
86+
Refused,
87+
}
88+
7989
/// One case's result: its receipts against the current parser.
8090
#[derive(Clone, Debug, PartialEq, Eq, Serialize)]
8191
pub struct CaseResult {
@@ -89,6 +99,8 @@ pub struct CaseResult {
8999
pub reason: Option<Reason>,
90100
/// Agrees, while Snowflake stopped after parsing (semantic, runtime or unsupported phase).
91101
pub semantic_only: bool,
102+
/// What Snowflake's parser did, whatever the case expects; none on disagreeing or no receipts.
103+
pub snowflake: Option<SnowflakeSyntax>,
92104
/// The account parameters its admissible receipts ran under (decision D120).
93105
#[serde(skip_serializing_if = "BTreeMap::is_empty")]
94106
pub account_state: BTreeMap<String, AccountSetting>,

‎crates/sqlquack-syntax-coverage/src/check/tests/helpers.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,11 @@ pub(crate) fn reshaped(sql: &str) -> LocalVerdict {
6666
vec![structural(sql), accepted_as("another tree")].swap_remove(usize::from(sql == "SELECT 17"))
6767
}
6868

69+
/// `structural`, except that `SELECT 17` is rejected.
70+
pub(crate) fn rejecting_select_17(sql: &str) -> LocalVerdict {
71+
vec![structural(sql), LocalVerdict::Rejected].swap_remove(usize::from(sql == "SELECT 17"))
72+
}
73+
6974
/// `structural`, except that `SELECT FROM` crashes instead of being rejected.
7075
pub(crate) fn crashing_on_select_from(sql: &str) -> LocalVerdict {
7176
vec![structural(sql), crashing(sql)].swap_remove(usize::from(sql == "SELECT FROM"))

‎crates/sqlquack-syntax-coverage/src/cli/_helpers/commands.rs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,13 +339,17 @@ fn read_snapshot(path: &Path) -> Result<Snapshot, ExitCode> {
339339

340340
/// Lines naming each non-empty list of the diff.
341341
fn diff_lines(diff: &SnapshotDiff) -> Vec<String> {
342-
let lists: [(&str, &Vec<String>); 8] = [
342+
let lists: [(&str, &Vec<String>); 9] = [
343343
("REGRESSION forms lost", &diff.forms_lost),
344344
("REGRESSION obligations lost", &diff.obligations_lost),
345345
(
346346
"REGRESSION Snowflake-parsed cases the parser now rejects",
347347
&diff.parsed_cases_refused,
348348
),
349+
(
350+
"REGRESSION Snowflake syntax errors the parser now accepts",
351+
&diff.syntax_error_cases_accepted,
352+
),
349353
("CATALOGUE files changed", &diff.catalogue_files_changed),
350354
("CATALOGUE forms added", &diff.forms_added),
351355
("CATALOGUE forms removed", &diff.forms_removed),
@@ -382,6 +386,7 @@ pub(crate) fn diff(
382386
&diff.forms_lost,
383387
&diff.obligations_lost,
384388
&diff.parsed_cases_refused,
389+
&diff.syntax_error_cases_accepted,
385390
&diff.catalogue_files_changed,
386391
&diff.forms_added,
387392
&diff.forms_removed,

‎crates/sqlquack-syntax-coverage/src/formats/constants.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,3 +88,9 @@ pub const DCM_LINE: &str = "line ";
8888

8989
/// What separates an inner error's line from its position.
9090
pub const DCM_POSITION: &str = " at position ";
91+
92+
/// Snowflake's errno for a syntax error; with a parse position it is a refusal by the parser.
93+
pub const SYNTAX_ERRNO: i64 = 1003;
94+
95+
/// Snowflake's errno 921, SQLSTATE 42601, reviewed as a syntax refusal.
96+
pub const SYNTAX_ERRNO_42601: i64 = 921;

‎crates/sqlquack-syntax-coverage/src/report/main/diff_snapshots.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,5 +59,10 @@ pub fn diff_snapshots(base: &Snapshot, head: &Snapshot) -> SnapshotDiff {
5959
.intersection(&head.refused_cases)
6060
.cloned()
6161
.collect(),
62+
syntax_error_cases_accepted: base
63+
.syntax_error_cases_rejected
64+
.intersection(&head.syntax_error_cases_accepted)
65+
.cloned()
66+
.collect(),
6267
}
6368
}

‎crates/sqlquack-syntax-coverage/src/report/main/snapshot.rs‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,25 +2,28 @@
22
33
use std::collections::{BTreeMap, BTreeSet};
44

5-
use crate::check::models::{CheckResult, LocalVerdict};
5+
use crate::check::models::{CheckResult, LocalVerdict, SnowflakeSyntax};
66
use crate::formats::constants::{ALIASES_FILE, CATALOGUE_DIR, SNAPSHOT_FORMAT, SOURCES_FILE};
7-
use crate::formats::models::Phase;
87
use crate::report::models::Snapshot;
98

109
/// Whether a coverage path belongs to the catalogue rather than to obligations or evidence.
1110
fn is_catalogue_file(path: &str) -> bool {
1211
path == SOURCES_FILE || path == ALIASES_FILE || path.starts_with(&format!("{CATALOGUE_DIR}/"))
1312
}
1413

15-
/// The receipted cases Snowflake parsed whose local verdict `accepted` says whether it parsed too.
16-
fn snowflake_parsed(check: &CheckResult, accepted: bool) -> BTreeSet<String> {
14+
/// Cases whose receipts say `snowflake` and whose parser verdict is acceptance when `accepted`.
15+
fn by_snowflake(
16+
check: &CheckResult,
17+
snowflake: SnowflakeSyntax,
18+
accepted: bool,
19+
) -> BTreeSet<String> {
1720
check
1821
.forms
1922
.iter()
2023
.flat_map(|form| &form.obligations)
2124
.flat_map(|obligation| &obligation.cases)
2225
.filter(|case| {
23-
case.phase.is_some_and(|phase| phase != Phase::SyntaxError)
26+
case.snowflake == Some(snowflake)
2427
&& matches!(case.local, LocalVerdict::Accepted { .. }) == accepted
2528
&& !matches!(case.local, LocalVerdict::Failed { .. })
2629
})
@@ -55,7 +58,9 @@ pub fn snapshot(
5558
.filter(|obligation| obligation.required && obligation.reason.is_none())
5659
.map(|obligation| obligation.id.clone())
5760
.collect(),
58-
parsed_cases: snowflake_parsed(check, true),
59-
refused_cases: snowflake_parsed(check, false),
61+
parsed_cases: by_snowflake(check, SnowflakeSyntax::Parsed, true),
62+
refused_cases: by_snowflake(check, SnowflakeSyntax::Parsed, false),
63+
syntax_error_cases_rejected: by_snowflake(check, SnowflakeSyntax::Refused, false),
64+
syntax_error_cases_accepted: by_snowflake(check, SnowflakeSyntax::Refused, true),
6065
}
6166
}

‎crates/sqlquack-syntax-coverage/src/report/models.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,12 @@ pub struct Snapshot {
6464
/// Receipted cases Snowflake parsed that the parser rejects.
6565
#[serde(default)]
6666
pub refused_cases: BTreeSet<String>,
67+
/// Receipted cases Snowflake refused as syntax that the parser rejects too.
68+
#[serde(default)]
69+
pub syntax_error_cases_rejected: BTreeSet<String>,
70+
/// Receipted cases Snowflake refused as syntax that the parser accepts, opaque or not.
71+
#[serde(default)]
72+
pub syntax_error_cases_accepted: BTreeSet<String>,
6773
}
6874

6975
/// One snapshot against a later one; coverage and catalogue changes kept apart.
@@ -82,4 +88,6 @@ pub struct SnapshotDiff {
8288
pub obligations_gained: Vec<String>,
8389
/// Cases Snowflake parsed that the base parser accepted, opaque or not, and the head rejects.
8490
pub parsed_cases_refused: Vec<String>,
91+
/// Cases Snowflake refused as syntax that the base parser rejected and the head accepts.
92+
pub syntax_error_cases_accepted: Vec<String>,
8593
}

‎crates/sqlquack-syntax-coverage/src/report/tests.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,7 @@
22
mod helpers;
33
#[path = "tests/test_diff_snapshots.rs"]
44
mod test_diff_snapshots;
5+
#[path = "tests/test_parser_changes.rs"]
6+
mod test_parser_changes;
57
#[path = "tests/test_types.rs"]
68
mod test_types;

0 commit comments

Comments
 (0)