Catch four dbt-style models Snowflake rejects (D150 round 2)

corpora/everyday/queries.toml

@@ -3176,8 +3176,8 @@ expect_repository = "refused SQ0002 node:statement"
 [[query]]
 category = "invalid_semantics"
 sql = '''SELECT customer_id FROM orders o JOIN customers c ON o.customer_id = c.customer_id'''
-expect = "refused SQ0004 statement"
-expect_repository = "refused SQ0004 statement"
+expect = "check SQ0004"
+expect_repository = "check SQ0004"
 
 [[query]]
 category = "invalid_semantics"

crates/sqlquack-core/src/diagnostics/codes.lock

@@ -23,3 +23,4 @@ SQ0205 unknown_function
 SQ0206 duplicate_alias
 SQ0207 too_many_arguments
 SQ0208 pivot_consumed_column
+SQ0209 lateral_function_join_condition

crates/sqlquack-core/src/diagnostics/constants.rs

@@ -94,6 +94,8 @@ pub const SET_OPERATION_CHAIN_HELP: &str = "split the chain: put at most 250 bra
 pub const TOO_MANY_ARGUMENTS: &str = "too_many_arguments";
 /// The name of the code of a read of a column a PIVOT consumed.
 pub const PIVOT_CONSUMED_COLUMN: &str = "pivot_consumed_column";
+/// The name of the code of a lateral table function joined with an ON condition.
+pub const LATERAL_FUNCTION_JOIN_CONDITION: &str = "lateral_function_join_condition";
 /// A relation or column name does not resolve.
 pub const UNRESOLVED_NAME: &str = "SQ0004";
 /// DuckDB cannot express the construct's meaning.
@@ -400,4 +402,17 @@ pub const CODES: &[CodeEntry] = &[
             is the only item in FROM, for names the SELECT list does not define as aliases.",
         example: r#"echo "SELECT order_id, total FROM orders PIVOT (SUM(total) FOR status IN ('shipped', 'pending')) AS p" | sqlquack check"#,
     },
+    CodeEntry {
+        code: "SQ0209",
+        name: LATERAL_FUNCTION_JOIN_CONDITION,
+        alias: None,
+        title: "A lateral table function is joined with an ON condition",
+        help: Some(
+            "remove the ON clause: a lateral table function already reads the rows before it",
+        ),
+        explanation: "Snowflake joins a LATERAL table function such as FLATTEN to the rows before it \
+            by itself, and rejects the join when it also has an ON condition, even ON TRUE \
+            (Snowflake error 2, unsupported feature). Write the join without ON, or with a comma.",
+        example: r#"echo "SELECT e.event_id, f.value FROM events e JOIN LATERAL FLATTEN(input => e.payload) f ON TRUE" | sqlquack check"#,
+    },
 ];

crates/sqlquack-core/src/refusals/_helpers/clause_location.rs

@@ -2,10 +2,14 @@
 
 use sqlquack_syntax::Expression;
 
-use crate::diagnostics::constants::{DUPLICATE_ALIAS, PIVOT_CONSUMED_COLUMN};
+use crate::diagnostics::constants::{
+    DUPLICATE_ALIAS, LATERAL_FUNCTION_JOIN_CONDITION, PIVOT_CONSUMED_COLUMN,
+};
 use crate::parsing::main::node_location::node_location;
 use crate::parsing::models::{SourceLines, SourceLocation};
-use crate::refusals::_helpers::rejections::{duplicate_alias, pivot_consumed_column};
+use crate::refusals::_helpers::rejections::{
+    duplicate_alias, lateral_join_condition, pivot_consumed_column,
+};
 
 /// The keyword that starts a QUALIFY clause.
 const QUALIFY_KEYWORD: &str = "QUALIFY";
@@ -24,6 +28,9 @@ pub(crate) fn clause_location(
         Expression::Select(select) if name == DUPLICATE_ALIAS => duplicate_alias(select)?
             .span
             .map(|span| lines.location(span.start, span.end)),
+        Expression::Select(select) if name == LATERAL_FUNCTION_JOIN_CONDITION => {
+            node_location(lateral_join_condition(select)?, lines)
+        }
         Expression::Select(select) if name == PIVOT_CONSUMED_COLUMN => {
             pivot_consumed_column(select)?
                 .span

crates/sqlquack-core/src/refusals/_helpers/rejections.rs

@@ -7,7 +7,9 @@ use sqlquack_syntax::expressions::names::models::{Column, Identifier, Star};
 use sqlquack_syntax::expressions::queries::models::Select;
 use sqlquack_syntax::{Expression, ExpressionWalk};
 
-use crate::diagnostics::constants::{DUPLICATE_ALIAS, PIVOT_CONSUMED_COLUMN, TOO_MANY_ARGUMENTS};
+use crate::diagnostics::constants::{
+    DUPLICATE_ALIAS, LATERAL_FUNCTION_JOIN_CONDITION, PIVOT_CONSUMED_COLUMN, TOO_MANY_ARGUMENTS,
+};
 use crate::refusals::constants::SNOWFLAKE_FUNCTION_ARITIES;
 
 /// The registry name of the rejection a node causes, with the reason, if it causes one.
@@ -41,6 +43,14 @@ pub(crate) fn rejection(node: &Expression) -> Option<(&'static str, String)> {
                     format!("the alias `{}` names two FROM items", alias.name),
                 )
             })
+            .or_else(|| {
+                lateral_join_condition(select).map(|_| {
+                    (
+                        LATERAL_FUNCTION_JOIN_CONDITION,
+                        "a lateral table function is joined with an ON condition".to_owned(),
+                    )
+                })
+            })
             .or_else(|| {
                 pivot_consumed_column(select).map(|column| {
                     (
@@ -89,6 +99,15 @@ pub(crate) fn duplicate_alias(select: &Select) -> Option<&Identifier> {
         .find(|alias| !seen.insert(stored_name(alias)))
 }
 
+/// The ON condition of the first join of a LATERAL table function, which Snowflake joins only without one.
+pub(crate) fn lateral_join_condition(select: &Select) -> Option<&Expression> {
+    select
+        .joins
+        .iter()
+        .filter(|join| matches!(join.this, Expression::Lateral(_)))
+        .find_map(|join| join.on.as_ref())
+}
+
 /// The first column a lone PIVOT aggregates or pivots on that its query still reads by that name.
 pub(crate) fn pivot_consumed_column(select: &Select) -> Option<&Column> {
     let [Expression::Pivot(pivot)] = select.from.as_ref()?.expressions.as_slice() else {

crates/sqlquack-core/src/refusals/tests/test_refusals.rs

@@ -163,6 +163,24 @@ fn given_sql_snowflake_parses_then_rejects_when_refusing_then_each_rejection_is_
             expected_rejections: vec![],
             expected_codes: vec![],
         },
+        RejectionTestCase {
+            description: "a lateral table function joined with an ON condition is rejected",
+            sql: "SELECT 1 FROM events e INNER JOIN LATERAL FLATTEN(input => e.payload) f ON TRUE; \
+                  SELECT 1 FROM events e LEFT JOIN LATERAL FLATTEN(input => e.payload) f ON f.index = 0",
+            expected_rejections: vec![
+                "lateral_function_join_condition",
+                "lateral_function_join_condition",
+            ],
+            expected_codes: vec!["SQ0209", "SQ0209"],
+        },
+        RejectionTestCase {
+            description: "a lateral table function joined without ON, or a lateral subquery with ON, is not",
+            sql: "SELECT 1 FROM events e JOIN LATERAL FLATTEN(input => e.payload) f; \
+                  SELECT 1 FROM events e, LATERAL FLATTEN(input => e.payload) f; \
+                  SELECT 1 FROM events e JOIN LATERAL (SELECT 1 AS a) s ON TRUE",
+            expected_rejections: vec![],
+            expected_codes: vec![],
+        },
         RejectionTestCase {
             description: "a star without ILIKE, or ILIKE without REPLACE or RENAME, is left to conversion",
             sql: "SELECT * REPLACE (price * 2 AS price) FROM orders; \

crates/sqlquack-syntax/src/parser/_helpers/statements/analyze.rs

@@ -264,6 +264,7 @@ impl Parser {
             || self.check(TokenType::Unpivot) && !self.is_unpivot_clause_start()
             || self.check(TokenType::Partition)
             || self.limit_as_snowflake_alias()
+            || self.check(TokenType::Window) && self.applies_snowflake_context_rules()
             || self.check(TokenType::Window) && {
                 let next_pos = self.current + 1;
                 next_pos >= self.tokens.len()

crates/sqlquack-syntax/src/parser/_helpers/statements/entry.rs

@@ -1610,7 +1610,7 @@ impl Parser {
 
     fn parse_select_windows(&mut self) -> Result<Option<Vec<NamedWindow>>> {
         Ok(
-            if self.check(TokenType::Window) && {
+            if !self.applies_snowflake_context_rules() && self.check(TokenType::Window) && {
                 let next_pos = self.current + 1;
                 next_pos < self.tokens.len()
                     && (self.tokens[next_pos].token_type == TokenType::Var

crates/sqlquack-syntax/tests/parser/test_snowflake_fixes.rs

@@ -1298,6 +1298,16 @@ fn given_an_operator_or_clause_snowflake_does_not_have_when_parsing_then_it_is_r
             sql: "SELECT FILTER ( [ 1 , 2 ] , x -> x > 1 ) ;",
             expected_kind: Some("select"),
         },
+        SnowflakeVerdictTestCase {
+            description: "a named WINDOW clause (recorded errno 1003 at the window's name)",
+            sql: "SELECT SUM ( 1 ) OVER w FROM t WINDOW w AS ( PARTITION BY 1 ) ;",
+            expected_kind: None,
+        },
+        SnowflakeVerdictTestCase {
+            description: "keeps a window function with its window written inline",
+            sql: "SELECT SUM ( 1 ) OVER ( PARTITION BY 1 ) FROM t ;",
+            expected_kind: Some("select"),
+        },
         SnowflakeVerdictTestCase {
             description: "keeps a single = and IN or NOT IN before a list or a query",
             sql: "SELECT 1 FROM t WHERE a = 1 AND a IN ( 1 , 2 ) AND a NOT IN ( SELECT 1 ) ;",

crates/sqlquack/src/analysis/constants.rs

@@ -7,6 +7,8 @@ pub const STATEMENT_REFUSAL: &str = "statement";
 pub const UNKNOWN_TABLE_FINDING: &str = "E200";
 /// The name check's code for a call to a function neither Snowflake nor the schema declares.
 pub const UNKNOWN_FUNCTION_FINDING: &str = "E202";
+/// The name check's code for an unqualified column more than one relation in scope has.
+pub const AMBIGUOUS_COLUMN_FINDING: &str = "E221";
 
 /// The column a scalar SQL function body is converted as.
 pub const SCALAR_RESULT_COLUMN: &str = "RESULT";

crates/sqlquack/src/analysis/main/check_source.rs

@@ -15,7 +15,9 @@ use sqlquack_semantics::query_name_findings;
 use sqlquack_semantics::types::models::Schema;
 use sqlquack_syntax::{Dialect, ParseOptions};
 
-use crate::analysis::constants::{UNKNOWN_FUNCTION_FINDING, UNKNOWN_TABLE_FINDING};
+use crate::analysis::constants::{
+    AMBIGUOUS_COLUMN_FINDING, UNKNOWN_FUNCTION_FINDING, UNKNOWN_TABLE_FINDING,
+};
 
 /// The parse failure, what Snowflake would reject and, with `schema`, the names it lacks.
 pub fn check_source(sql: &str, schema: Option<&Schema>) -> Result<Vec<Diagnostic>, String> {
@@ -54,7 +56,7 @@ fn undeclared_names(sql: &str, schema: &Schema, lines: &SourceLines) -> Vec<Refu
         .collect()
 }
 
-/// The refusal for an unknown relation or function finding; other findings are not checked here.
+/// The refusal for an unknown relation or function, or an ambiguous column; other findings are not checked here.
 fn undeclared(finding: NameFinding, lines: &SourceLines) -> Option<Refusal> {
     let location = finding
         .span
@@ -73,6 +75,14 @@ fn undeclared(finding: NameFinding, lines: &SourceLines) -> Option<Refusal> {
             category: RefusalCategory::SnowflakeRejects,
             reason: format!("no Snowflake or schema function is named `{written}`"),
         }),
+        AMBIGUOUS_COLUMN_FINDING => Some(Refusal {
+            construct: None,
+            location,
+            category: RefusalCategory::UnresolvedName,
+            reason: format!(
+                "column `{written}` is ambiguous: more than one relation in scope has it; qualify it"
+            ),
+        }),
         _ => None,
     }
 }

crates/sqlquack/src/analysis/tests/helpers.rs

@@ -785,8 +785,8 @@ const PARSED_TREE: &str = r#"{"error":false"#;
 const LOCATION_FIELD: &str = r#""query_location"#;
 
 /// Each everyday corpus query whose outcome under either policy differs from the recorded one.
-pub(crate) fn everyday_outcome_mismatches() -> Vec<String> {
-    let (root, corpus) = everyday_corpus();
+pub(crate) fn everyday_outcome_mismatches(directory: &str) -> Vec<String> {
+    let (root, corpus) = everyday_corpus(directory);
     let strict = everyday_arguments(&root, true);
     let repository = everyday_arguments(&root, false);
     corpus
@@ -819,8 +819,8 @@ pub(crate) fn everyday_outcome_mismatches() -> Vec<String> {
 }
 
 /// How many valid queries, not DDL, are recorded as translated: strict, and repository policy.
-pub(crate) fn everyday_translation_rates() -> String {
-    let (_, corpus) = everyday_corpus();
+pub(crate) fn everyday_translation_rates(directory: &str) -> String {
+    let (_, corpus) = everyday_corpus(directory);
     let valid: Vec<&EverydayQuery> = corpus
         .query
         .iter()
@@ -848,15 +848,26 @@ pub(crate) fn everyday_translation_rates() -> String {
 }
 
 /// Queries Snowflake rejects that pass SQLQuack's parse and checks, once every receipt matches.
-pub(crate) fn everyday_snowflake_rejections_not_caught() -> Vec<String> {
-    let (root, corpus) = everyday_corpus();
+pub(crate) fn everyday_snowflake_rejections_not_caught(directory: &str) -> Vec<String> {
+    let (root, corpus) = everyday_corpus(directory);
     let text =
         std::fs::read_to_string(root.join("snowflake-receipts.json")).expect("receipts read");
     let receipts: serde_json::Value = serde_json::from_str(&text).expect("receipts parse");
-    let results = receipts["results"]
-        .as_array()
-        .expect("receipts hold results");
-    for receipt in results {
+    // Results are a list, or an object keyed by query id; a receipt without a verdict is skipped.
+    let results: Vec<&serde_json::Value> = receipts["results"].as_array().map_or_else(
+        || {
+            receipts["results"]
+                .as_object()
+                .into_iter()
+                .flat_map(|by_id| by_id.values())
+                .collect()
+        },
+        |list| list.iter().collect(),
+    );
+    for receipt in results
+        .into_iter()
+        .filter(|receipt| receipt["compiles"].is_boolean())
+    {
         let sql = receipt["sql"]
             .as_str()
             .expect("a receipt names its SQL")
@@ -894,11 +905,11 @@ pub(crate) fn everyday_snowflake_rejections_not_caught() -> Vec<String> {
 }
 
 /// The everyday corpus directory and its queries.
-fn everyday_corpus() -> (std::path::PathBuf, EverydayCorpus) {
-    let root = repository_root().join(EVERYDAY_CORPUS);
+fn everyday_corpus(directory: &str) -> (std::path::PathBuf, EverydayCorpus) {
+    let root = repository_root().join(directory);
     let text = std::fs::read_to_string(root.join("queries.toml")).expect("the corpus reads");
     let corpus: EverydayCorpus = toml::from_str(&text).expect("the corpus parses");
-    assert!(corpus.query.len() > 400, "{} queries", corpus.query.len());
+    assert!(corpus.query.len() > 300, "{} queries", corpus.query.len());
     (root, corpus)
 }
 
@@ -952,8 +963,11 @@ fn everyday_outcome(sql: &str, arguments: &ConvertArguments) -> String {
     )
 }
 
-/// The corpus directory, under the repository root.
-const EVERYDAY_CORPUS: &str = "corpora/everyday";
+/// The everyday corpus directory, under the repository root.
+pub(crate) const EVERYDAY_CORPUS: &str = "corpora/everyday";
+
+/// The dbt-style model corpus directory, laid out as the everyday corpus.
+pub(crate) const EVERYDAY_MODELS_CORPUS: &str = "corpora/everyday-models";
 
 /// Corpus categories of statements no rule translates: DDL and DML.
 const EVERYDAY_NOT_TRANSLATABLE: [&str; 1] = ["ddl_dml"];

crates/sqlquack/src/analysis/tests/test_everyday_corpus.rs

@@ -1,13 +1,14 @@
 use crate::analysis::tests::helpers::{
-    everyday_outcome_mismatches, everyday_snowflake_rejections_not_caught,
-    everyday_translation_rates,
+    EVERYDAY_CORPUS, EVERYDAY_MODELS_CORPUS, everyday_outcome_mismatches,
+    everyday_snowflake_rejections_not_caught, everyday_translation_rates,
 };
 use crate::analysis::tests::test_types::EverydayCorpusTestCase;
 
 #[test]
 fn given_everyday_snowflake_queries_when_checked_and_converted_then_each_keeps_its_outcome() {
     let test_cases = [EverydayCorpusTestCase {
         description: "every corpora/everyday query parses, checks, converts or refuses as recorded, under the strict built-in policy and the repository policy",
+        directory: EVERYDAY_CORPUS,
         expected_mismatches: Vec::new(),
         expected_rates: "288 of 436 strict, 357 of 436 repository",
         // SQL Snowflake rejects (snowflake-receipts.json) that SQLQuack neither fails to parse nor
@@ -17,19 +18,55 @@ fn given_everyday_snowflake_queries_when_checked_and_converted_then_each_keeps_i
 
     for test_case in test_cases {
         assert_eq!(
-            everyday_outcome_mismatches(),
+            everyday_outcome_mismatches(test_case.directory),
             test_case.expected_mismatches,
             "{}",
             test_case.description
         );
         assert_eq!(
-            everyday_translation_rates(),
+            everyday_translation_rates(test_case.directory),
             test_case.expected_rates,
             "{}",
             test_case.description
         );
         assert_eq!(
-            everyday_snowflake_rejections_not_caught(),
+            everyday_snowflake_rejections_not_caught(test_case.directory),
+            test_case.expected_rejections_not_caught,
+            "{}",
+            test_case.description
+        );
+    }
+}
+
+#[test]
+fn given_dbt_style_snowflake_models_when_checked_and_converted_then_each_keeps_its_outcome() {
+    let test_cases = [EverydayCorpusTestCase {
+        description: "every corpora/everyday-models query parses, checks, converts or refuses as recorded, under the strict built-in policy and the repository policy",
+        directory: EVERYDAY_MODELS_CORPUS,
+        expected_mismatches: Vec::new(),
+        expected_rates: "175 of 295 strict, 247 of 295 repository",
+        // SQL Snowflake rejects that SQLQuack neither fails to parse nor flags in its checks:
+        // CHANGES without change tracking.
+        expected_rejections_not_caught: vec![
+            "select *\nfrom stg_customers\nchanges (information => default)\nat (offset => -3600)",
+        ],
+    }];
+
+    for test_case in test_cases {
+        assert_eq!(
+            everyday_outcome_mismatches(test_case.directory),
+            test_case.expected_mismatches,
+            "{}",
+            test_case.description
+        );
+        assert_eq!(
+            everyday_translation_rates(test_case.directory),
+            test_case.expected_rates,
+            "{}",
+            test_case.description
+        );
+        assert_eq!(
+            everyday_snowflake_rejections_not_caught(test_case.directory),
             test_case.expected_rejections_not_caught,
             "{}",
             test_case.description

crates/sqlquack/src/analysis/tests/test_types.rs

@@ -213,6 +213,7 @@ pub(crate) struct OverrideCallsTestCase {
 
 pub(crate) struct EverydayCorpusTestCase {
     pub(crate) description: &'static str,
+    pub(crate) directory: &'static str,
     pub(crate) expected_mismatches: Vec<String>,
     pub(crate) expected_rates: &'static str,
     pub(crate) expected_rejections_not_caught: Vec<&'static str>,