diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/extraction.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/extraction.rs index e070c2d140..9f7007301d 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/extraction.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/extraction.rs @@ -35,6 +35,8 @@ const DIRECT_NAMES: [(&str, &str, &str); 3] = [ "__table_fn_expected__", ), ]; +const TEST_CONTEXT: &str = "SQL test"; +const SCENARIO_CONTEXT: &str = "SQL scenario"; const DECLARATION_CALLS: [&str; 4] = ["enum", "const", "var", "param"]; /// Longest projection the alias help repeats verbatim. const ALIAS_HELP_EXPRESSION_LIMIT: usize = 80; @@ -67,6 +69,9 @@ struct TestRequest { /// Read only the CTE names and body offsets of the authored block, before any expansion. #[serde(default)] authored: bool, + /// The authored text is a scenario rather than a SQL test block. + #[serde(default)] + scenario: bool, } /// One extraction error, with help and the offending text where the author can act on it. @@ -189,7 +194,7 @@ fn extract_test(test: &TestRequest, syntax: &LexicalSyntax) -> Result = scan_ctes(&test.sql, &test.file_label, syntax)?.ctes; + let ctes: Vec = scan_ctes(&test.sql, &test.file_label, TEST_CONTEXT, syntax)?.ctes; let scope = TestScope { file: &test.file_label, syntax, @@ -204,7 +209,12 @@ fn extract_test(test: &TestRequest, syntax: &LexicalSyntax) -> Result Result { - match scan_ctes(&test.sql, &test.file_label, syntax) { + let context: &str = if test.scenario { + SCENARIO_CONTEXT + } else { + TEST_CONTEXT + }; + match scan_ctes(&test.sql, &test.file_label, context, syntax) { Ok(scanned) => Ok(Classified::Authored { ctes: scanned .ctes @@ -218,13 +228,30 @@ fn authored_ctes(test: &TestRequest, syntax: &LexicalSyntax) -> Result { + file: &'a str, + context: &'a str, + syntax: &'a LexicalSyntax, +} + /// The top-level CTEs of one test, with the code-point offset of each stripped body. struct ScannedCtes { ctes: Vec, body_starts: Vec, } -fn scan_ctes(sql: &str, file: &str, syntax: &LexicalSyntax) -> Result { +fn scan_ctes( + sql: &str, + file: &str, + context: &str, + syntax: &LexicalSyntax, +) -> Result { + let owner = CteOwner { + file, + context, + syntax, + }; let mut body_starts: Vec = Vec::new(); let mut index = skip_ignorable(sql, 0, syntax)?; index = consume_keyword(sql, index, "WITH").ok_or_else(|| { @@ -237,7 +264,7 @@ fn scan_ctes(sql: &str, file: &str, syntax: &LexicalSyntax) -> Result = Vec::new(); let mut seen: HashSet = HashSet::new(); loop { - let (name, end) = read_cte_name(sql, index, file, syntax)?; + let (name, end) = read_cte_name(sql, index, &owner)?; if !seen.insert(name.clone()) { return Err(format!("SQL test '{file}' defines duplicate CTE '{name}'").into()); } @@ -252,8 +279,11 @@ fn scan_ctes(sql: &str, file: &str, syntax: &LexicalSyntax) -> Result Result, ) -> Result<(String, usize), Failure> { + let (file, context, syntax) = (owner.file, owner.context, owner.syntax); if let Some(quote) = byte_at(sql, start).filter(|byte| matches!(byte, b'"' | b'`' | b'[')) { let end = if quote == b'[' { sql[start..] @@ -290,11 +320,10 @@ fn read_cte_name( } else { skip_non_code(sql, start, syntax)? }; - return Err(cte_name_failure(sql, start, end, file)); + return Err(cte_name_failure(sql, start, end, owner)); } - let Some(first) = char_at(sql, start).filter(|first| *first == '_' || first.is_alphabetic()) - else { - return Err(format!("SQL test '{file}' expected a CTE name").into()); + let Some(first) = char_at(sql, start).filter(|first| is_identifier_continue(*first)) else { + return Err(format!("{context} '{file}' expected a CTE name").into()); }; let mut end = start + first.len_utf8(); while let Some(character) = @@ -302,16 +331,48 @@ fn read_cte_name( { end += character.len_utf8(); } - if sql[start..end] - .bytes() - .all(|byte| byte.is_ascii_alphanumeric() || byte == b'_') + if (first == '_' || first.is_ascii_alphabetic()) + && sql[start..end] + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || byte == b'_') { return Ok((sql[start..end].to_owned(), end)); } - Err(cte_name_failure(sql, start, end, file)) + Err(cte_name_failure(sql, start, end, owner)) +} + +/// Python's error for `AS MATERIALIZED` or `AS NOT MATERIALIZED`, located at the hint. +fn materialization_failure( + sql: &str, + start: usize, + owner: &CteOwner<'_>, + name: &str, +) -> Result, Failure> { + let (hint, end) = match consume_keyword(sql, start, "NOT") { + Some(not_end) => { + let index = skip_ignorable(sql, not_end, owner.syntax)?; + match consume_keyword(sql, index, "MATERIALIZED") { + Some(end) => ("NOT MATERIALIZED", end), + None => return Ok(None), + } + } + None => match consume_keyword(sql, start, "MATERIALIZED") { + Some(end) => ("MATERIALIZED", end), + None => return Ok(None), + }, + }; + Ok(Some(Failure { + message: format!( + "{} '{}' CTE '{name}' must not use AS {hint}; materialization hints are not supported in {} CTEs", + owner.context, owner.file, owner.context + ), + help: Some(format!("remove {hint} and write {name} AS (...)")), + token: Some((sql[start..end].to_owned(), sql[..start].chars().count())), + })) } -fn cte_name_failure(sql: &str, start: usize, end: usize, file: &str) -> Failure { +fn cte_name_failure(sql: &str, start: usize, end: usize, owner: &CteOwner<'_>) -> Failure { + let (file, context) = (owner.file, owner.context); let token = &sql[start..end]; let shown = if matches!(byte_at(sql, start), Some(b'"' | b'`' | b'[')) { token.to_owned() @@ -338,7 +399,7 @@ fn cte_name_failure(sql: &str, start: usize, end: usize, file: &str) -> Failure } Failure { message: format!( - "SQL test '{file}' CTE name {shown} must be an unquoted identifier of ASCII letters, digits and underscores" + "{context} '{file}' CTE name {shown} must be an unquoted identifier of ASCII letters, digits and underscores" ), help: Some(format!( "rename the CTE, for example {suggestion}; quoted CTE names and names with $ or non-ASCII characters are not supported" diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/scenario_extraction.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/scenario_extraction.rs index d5d4f066e2..fd8c6488b1 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/scenario_extraction.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/scenario_extraction.rs @@ -31,12 +31,10 @@ struct ClassifiedScenario { assertion_names: Vec, } -/// One scenario's scanner error, or its classification after any Polyglot independence check. +/// One scenario's error, or its classification after any Polyglot independence check. #[derive(Debug, Default, Serialize)] #[serde(rename_all = "camelCase")] struct ScenarioOutcome { - #[serde(skip_serializing_if = "Option::is_none")] - scan_error: Option, #[serde(skip_serializing_if = "Option::is_none")] independence: Option>, #[serde(skip_serializing_if = "Option::is_none")] @@ -54,7 +52,7 @@ pub(crate) fn extract_scenario_json( match scan_top_level_ctes(sql, file, RelationshipSource::Scenario, syntax) { TopLevelCtes::Deferred => return Ok(None), TopLevelCtes::Failed(message) => ScenarioOutcome { - scan_error: Some(message), + error: Some(message), ..ScenarioOutcome::default() }, TopLevelCtes::Scanned(ctes) => { diff --git a/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs b/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs index bc61f1f4ac..b53085d4bd 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs @@ -1991,8 +1991,8 @@ pub(crate) fn rejected_outcome( } /// The authored-block reading of `sql`: its CTEs with body offsets, or its first error. -pub(crate) fn authored_ctes_response(sql: &str) -> Value { +pub(crate) fn authored_ctes_response(sql: &str, scenario: bool) -> Value { batch_response( - &json!({"tests": [{"sql": sql, "fileLabel": "tests/t.sql", "mode": "model", "authored": true}]}), + &json!({"tests": [{"sql": sql, "fileLabel": "tests/t.sql", "mode": "model", "authored": true, "scenario": scenario}]}), ) } diff --git a/crates/sqlbuild-analysis/src/compiler/tests/test_sql_scenario_extraction.rs b/crates/sqlbuild-analysis/src/compiler/tests/test_sql_scenario_extraction.rs index 1a3b083864..39606b5f89 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/test_sql_scenario_extraction.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_sql_scenario_extraction.rs @@ -64,16 +64,23 @@ fn given_scenarios_when_extracting_natively_then_python_outcomes_or_deferrals_ar ), }, ScenarioExtractionTestCase { - description: "a quoted CTE name is a scanner error Python may still accept with Polyglot", + description: "a quoted CTE name is Python's scanner error", sql: "WITH \"__source__raw\" AS (SELECT 1), __expected__orders AS (SELECT 1 AS id)", expected_json: Some( - r#"{"scanError":"SQL scenario 'tests/scenarios/orders.sql' expected a CTE name"}"#, + r#"{"error":"SQL scenario 'tests/scenarios/orders.sql' expected a CTE name"}"#, ), }, ScenarioExtractionTestCase { - description: "an unclosed body is a scanner error naming the scenario context", + description: "an unclosed body is Python's scanner error naming the scenario context", sql: "WITH __source__raw AS (SELECT 1", - expected_json: Some(r#"{"scanError":"SQL scenario contains an unclosed parenthesis"}"#), + expected_json: Some(r#"{"error":"SQL scenario contains an unclosed parenthesis"}"#), + }, + ScenarioExtractionTestCase { + description: "a materialization hint is Python's scanner error", + sql: "WITH __source__raw AS NOT MATERIALIZED (SELECT 1), __expected__orders AS (SELECT 1)", + expected_json: Some( + r#"{"error":"SQL scenario 'tests/scenarios/orders.sql' CTE '__source__raw' must not use AS NOT MATERIALIZED; materialization hints are not supported in SQL scenario CTEs"}"#, + ), }, ScenarioExtractionTestCase { description: "a keyword Python would match by case mapping defers", diff --git a/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_extraction_rules.rs b/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_extraction_rules.rs index bdcab5353f..a205367f8b 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_extraction_rules.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_sql_test_extraction_rules.rs @@ -385,6 +385,7 @@ fn given_authored_sql_tests_when_extracting_then_each_rule_holds() { fn given_authored_blocks_when_reading_ctes_then_offsets_and_name_errors_are_exact() { let test_cases = [ AuthoredCtesTestCase { + scenario: false, description: "body offsets count code points and skip leading whitespace", sql: "-- café\nWITH h AS ( SELECT 'é' AS a), __ref__orders AS (\n SELECT 1)", expected_response: json!({"tests": [{"kind": "authored", "ctes": [ @@ -393,6 +394,7 @@ fn given_authored_blocks_when_reading_ctes_then_offsets_and_name_errors_are_exac ]}]}), }, AuthoredCtesTestCase { + scenario: false, description: "a CTE name error keeps its code-point offset", sql: "-- é\nWITH h AS (SELECT 1), café AS (SELECT 2)", expected_response: json!({"error": { @@ -404,6 +406,55 @@ fn given_authored_blocks_when_reading_ctes_then_offsets_and_name_errors_are_exac }}), }, AuthoredCtesTestCase { + scenario: false, + description: "a materialization hint is located at the hint with help", + sql: "WITH h AS (SELECT 1),\n__ref__orders AS not\nmaterialized (SELECT 2)", + expected_response: json!({"error": { + "index": 0, + "message": "SQL test 'tests/t.sql' CTE '__ref__orders' must not use AS NOT MATERIALIZED; materialization hints are not supported in SQL test CTEs", + "help": "remove NOT MATERIALIZED and write __ref__orders AS (...)", + "token": "not\nmaterialized", + "tokenOffset": 39, + }}), + }, + AuthoredCtesTestCase { + scenario: true, + description: "a scenario's quoted CTE name names the scenario", + sql: "WITH \"__source__raw_orders\" AS (SELECT 1), __expected__orders AS (SELECT 1)", + expected_response: json!({"error": { + "index": 0, + "message": "SQL scenario 'tests/t.sql' CTE name \"__source__raw_orders\" must be an unquoted identifier of ASCII letters, digits and underscores", + "help": "rename the CTE, for example __source__raw_orders; quoted CTE names and names with $ or non-ASCII characters are not supported", + "token": "\"__source__raw_orders\"", + "tokenOffset": 5, + }}), + }, + AuthoredCtesTestCase { + scenario: true, + description: "a scenario's materialization hint names the scenario", + sql: "WITH __source__raw_orders AS MATERIALIZED (SELECT 1)", + expected_response: json!({"error": { + "index": 0, + "message": "SQL scenario 'tests/t.sql' CTE '__source__raw_orders' must not use AS MATERIALIZED; materialization hints are not supported in SQL scenario CTEs", + "help": "remove MATERIALIZED and write __source__raw_orders AS (...)", + "token": "MATERIALIZED", + "tokenOffset": 29, + }}), + }, + AuthoredCtesTestCase { + scenario: false, + description: "a CTE name starting with a digit is located with help", + sql: "WITH 2024_orders AS (SELECT 1)", + expected_response: json!({"error": { + "index": 0, + "message": "SQL test 'tests/t.sql' CTE name '2024_orders' must be an unquoted identifier of ASCII letters, digits and underscores", + "help": "rename the CTE, for example cte_2024_orders; quoted CTE names and names with $ or non-ASCII characters are not supported", + "token": "2024_orders", + "tokenOffset": 5, + }}), + }, + AuthoredCtesTestCase { + scenario: false, description: "other errors are left to extraction and read as no CTEs", sql: "WITH h AS (SELECT 1) SELECT * FROM h", expected_response: json!({"tests": [{"kind": "authored", "ctes": []}]}), @@ -412,7 +463,7 @@ fn given_authored_blocks_when_reading_ctes_then_offsets_and_name_errors_are_exac for test_case in test_cases { assert_eq!( - authored_ctes_response(test_case.sql), + authored_ctes_response(test_case.sql, test_case.scenario), test_case.expected_response, "{}", test_case.description diff --git a/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs b/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs index d83ddeb0f8..2f53e15443 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/test_types.rs @@ -162,6 +162,7 @@ pub(crate) struct ExtractionRuleTestCase { pub(crate) struct AuthoredCtesTestCase { pub(crate) description: &'static str, + pub(crate) scenario: bool, pub(crate) sql: &'static str, pub(crate) expected_response: serde_json::Value, } diff --git a/crates/sqlbuild-discovery/src/sql_tests/_helpers/scenario_parsing.rs b/crates/sqlbuild-discovery/src/sql_tests/_helpers/scenario_parsing.rs index f16696eaf6..7d55af6449 100644 --- a/crates/sqlbuild-discovery/src/sql_tests/_helpers/scenario_parsing.rs +++ b/crates/sqlbuild-discovery/src/sql_tests/_helpers/scenario_parsing.rs @@ -38,9 +38,15 @@ pub(crate) fn parse_scenario_file( contents[..header_start].matches('\n').count() + 1, )?; let sql_body: String = python_cleandoc(options.python, &contents[sql_start..]); + let span_start: usize = contents[..sql_start].chars().count(); + let body_span: (usize, usize) = ( + span_start, + span_start + contents[sql_start..].chars().count(), + ); Ok(DiscoveredScenarioFile { contents, header_values, sql_body, + body_span, }) } diff --git a/crates/sqlbuild-discovery/src/sql_tests/models.rs b/crates/sqlbuild-discovery/src/sql_tests/models.rs index 9f27db23ba..9b70acad65 100644 --- a/crates/sqlbuild-discovery/src/sql_tests/models.rs +++ b/crates/sqlbuild-discovery/src/sql_tests/models.rs @@ -36,4 +36,6 @@ pub struct DiscoveredScenarioFile { pub contents: String, pub header_values: Vec<(String, AuthoredValue)>, pub sql_body: String, + /// Code-point span of the authored SQL after the header, which `sql_body` cleans. + pub body_span: (usize, usize), } diff --git a/crates/sqlbuild-discovery/src/sql_tests/tests/helpers.rs b/crates/sqlbuild-discovery/src/sql_tests/tests/helpers.rs index 4a1676ae2e..fa307f994f 100644 --- a/crates/sqlbuild-discovery/src/sql_tests/tests/helpers.rs +++ b/crates/sqlbuild-discovery/src/sql_tests/tests/helpers.rs @@ -94,3 +94,10 @@ pub(super) fn statement_failures(contents: &str) -> [Option<(String, Option Option<(usize, usize)> { + parse_scenario_text(FILE_PATH, contents.to_owned(), &options()) + .ok() + .map(|file| file.body_span) +} diff --git a/crates/sqlbuild-discovery/src/sql_tests/tests/test_sql_test_files.rs b/crates/sqlbuild-discovery/src/sql_tests/tests/test_sql_test_files.rs index 3b3bd49d96..340d8cab57 100644 --- a/crates/sqlbuild-discovery/src/sql_tests/tests/test_sql_test_files.rs +++ b/crates/sqlbuild-discovery/src/sql_tests/tests/test_sql_test_files.rs @@ -1,7 +1,9 @@ use crate::sql_tests::tests::helpers::{ - FILE_PATH, expected_rows, scenario_rows, statement_failures, test_file_rows, + FILE_PATH, expected_rows, scenario_body_span, scenario_rows, statement_failures, test_file_rows, +}; +use crate::sql_tests::tests::test_types::{ + DeepStatementHeaderTestCase, ScenarioSpanTestCase, StatementFileTestCase, }; -use crate::sql_tests::tests::test_types::{DeepStatementHeaderTestCase, StatementFileTestCase}; #[test] fn given_sql_test_files_when_splitting_then_blocks_and_failures_match_python() { @@ -107,3 +109,32 @@ fn given_deeply_nested_test_and_scenario_headers_when_parsing_then_each_reports_ ); } } + +#[test] +fn given_scenario_file_when_parsing_then_body_span_counts_code_points_after_the_header() { + let test_cases = [ + ScenarioSpanTestCase { + description: "CRLF line breaks and non-ASCII header text before the body", + contents: "SCENARIO (description \"Commandes \u{e9}t\u{e9}\");\r\n\r\nWITH a AS (SELECT '\u{e9}')\r\n", + expected_body_span: Some((43, 67)), + }, + ScenarioSpanTestCase { + description: "an LF body directly after the header", + contents: "SCENARIO ();\nWITH a AS (SELECT 1)", + expected_body_span: Some((13, 33)), + }, + ScenarioSpanTestCase { + description: "a file without a header has no span", + contents: "WITH a AS (SELECT 1)", + expected_body_span: None, + }, + ]; + for test_case in test_cases { + assert_eq!( + scenario_body_span(test_case.contents), + test_case.expected_body_span, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-discovery/src/sql_tests/tests/test_types.rs b/crates/sqlbuild-discovery/src/sql_tests/tests/test_types.rs index db379eb3be..ca23de1d38 100644 --- a/crates/sqlbuild-discovery/src/sql_tests/tests/test_types.rs +++ b/crates/sqlbuild-discovery/src/sql_tests/tests/test_types.rs @@ -13,3 +13,10 @@ pub(super) struct DeepStatementHeaderTestCase { /// The test file's failure, then the scenario file's: each message and help. pub(super) expected_failures: [Option<(String, Option)>; 2], } + +/// Scenario contents and the code-point span of the SQL after its header. +pub(super) struct ScenarioSpanTestCase { + pub(super) description: &'static str, + pub(super) contents: &'static str, + pub(super) expected_body_span: Option<(usize, usize)>, +} diff --git a/crates/sqlbuild-python/src/bindings/_helpers/discovery/project_files.rs b/crates/sqlbuild-python/src/bindings/_helpers/discovery/project_files.rs index dcf64a4766..d851204dd3 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/discovery/project_files.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/discovery/project_files.rs @@ -426,6 +426,7 @@ fn scenario_object(py: Python<'_>, file: DiscoveredScenarioFile) -> PyResult, + syntax: &LexicalSyntax, +) -> Scan> { + if try_consume_keyword(sql, start, b"MATERIALIZED")?.is_some() { + return Ok(Some("MATERIALIZED")); + } + let Some(not_end) = try_consume_keyword(sql, start, b"NOT")? else { + return Ok(None); + }; + let index = skip_ignorable(sql, not_end, text, syntax)?; + Ok(try_consume_keyword(sql, index, b"MATERIALIZED")?.map(|_| "NOT MATERIALIZED")) +} + fn context_label(source: RelationshipSource) -> &'static str { match source { RelationshipSource::Test => "SQL test", diff --git a/crates/sqlbuild-scopes/src/relationship_names/tests/test_expected_model_names.rs b/crates/sqlbuild-scopes/src/relationship_names/tests/test_expected_model_names.rs index 94f30eecf4..215e703359 100644 --- a/crates/sqlbuild-scopes/src/relationship_names/tests/test_expected_model_names.rs +++ b/crates/sqlbuild-scopes/src/relationship_names/tests/test_expected_model_names.rs @@ -109,6 +109,14 @@ fn given_sql_test_when_scanning_expected_models_then_names_and_errors_match_pyth "SQL test 'tests/unit/test_orders.sql' CTE '__expected__a' must use AS (...)", ), }, + ExpectedNamesTestCase { + description: "a materialization hint raises Python's error", + sql: "WITH __expected__a AS not MATERIALIZED (SELECT 1) SELECT 1", + expected_outcome: failed( + "SQL test 'tests/unit/test_orders.sql' CTE '__expected__a' must not use AS NOT \ + MATERIALIZED; materialization hints are not supported in SQL test CTEs", + ), + }, ExpectedNamesTestCase { description: "an unclosed quote inside a body raises Python's error", sql: "WITH __expected__a AS (SELECT 'open) SELECT 1", diff --git a/scripts/compiler_differential/_helpers/corpus/attachment_failure_cases.py b/scripts/compiler_differential/_helpers/corpus/attachment_failure_cases.py index dd0e1ca45a..333921ece0 100644 --- a/scripts/compiler_differential/_helpers/corpus/attachment_failure_cases.py +++ b/scripts/compiler_differential/_helpers/corpus/attachment_failure_cases.py @@ -204,4 +204,54 @@ def attachment_failure_cases() -> tuple[FailureCase, ...]: f"{_EXPECTED_STAGING}\nSELECT 1\n" }, ), + failure_case( + name="scenario-quoted-cte-name", + expected_code="P001", + expected_message=( + f"SQL scenario '{_SCENARIO_PATH}' CTE name \"__source__raw_orders\" must be an " + "unquoted identifier of ASCII letters, digits and underscores" + ), + expected_help=( + "rename the CTE, for example __source__raw_orders; quoted CTE names and names " + "with $ or non-ASCII characters are not supported" + ), + files={ + _SCENARIO_PATH: _scenario( + '"__source__raw_orders" AS (\n SELECT 1 AS order_id\n)', _EXPECTED_STAGING + ) + }, + ), + failure_case( + name="scenario-materialized-cte", + expected_code="P001", + expected_message=( + f"SQL scenario '{_SCENARIO_PATH}' CTE '__source__raw_orders' must not use AS " + "MATERIALIZED; materialization hints are not supported in SQL scenario CTEs" + ), + expected_help="remove MATERIALIZED and write __source__raw_orders AS (...)", + files={ + _SCENARIO_PATH: _scenario( + "__source__raw_orders AS MATERIALIZED (\n SELECT 1 AS order_id\n)", + _EXPECTED_STAGING, + ) + }, + ), + failure_case( + name="macro-test-quoted-cte-name", + expected_code="P001", + expected_message=( + f"SQL test '{_TEST_PATH}' CTE name \"__macro_actual__\" must be an unquoted " + "identifier of ASCII letters, digits and underscores" + ), + expected_help=( + "rename the CTE, for example __macro_actual__; quoted CTE names and names with " + "$ or non-ASCII characters are not supported" + ), + files={ + "macros/amounts.py": "def doubled(value):\n return f'{value} * 2'\n", + _TEST_PATH: 'TEST (mode macro, name "doubled");\n\nWITH\n' + "\"__macro_actual__\" AS (\n SELECT @doubled('1') AS amount\n),\n" + "__macro_expected__ AS (\n SELECT 2 AS amount\n)\nSELECT 1\n", + }, + ), ) diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md index 27646b1115..9d8bd6a2da 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md @@ -111,6 +111,15 @@ false filter, as in [unit tests](testing.md). Every scenario must have at least one fixture CTE and at least one `__expected__` or `__assert__` CTE. The file ends after its last CTE; a trailing ceremonial `SELECT 1` is accepted but not required. +Scenario CTEs follow the same strict rules as [unit test CTEs](testing.md), on every +compiler engine, and other forms are compile errors that point at the offending text: + +- A CTE name is an unquoted identifier of ASCII letters, digits and underscores that starts with a + letter or underscore. Quoted names such as `"__source__raw_orders"` or `` `recent orders` ``, and + names with `$` or non-ASCII characters, are errors. +- A CTE body follows `AS` directly. Materialization hints such as `AS MATERIALIZED (...)` or + `AS NOT MATERIALIZED (...)` are not supported in scenario CTEs. + Scenario SQL uses macros, enums, and constants available from the scenario file's directory under `tests/scenarios/`. Public enums and constants available to a model are also available when the scenario defines `__expected__model_name` output for that model. Model-private values and macros diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md index 3cb54ae16e..3985301059 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/testing.md @@ -143,6 +143,8 @@ with a compile error that names the CTE: - A CTE name is an unquoted identifier of ASCII letters, digits and underscores that starts with a letter or underscore. Quoted names such as `"recent orders"` or `` `recent orders` ``, and names with `$` or non-ASCII characters, are errors that point at the name. +- A CTE body follows `AS` directly. Materialization hints such as `AS MATERIALIZED (...)` or + `AS NOT MATERIALIZED (...)` are not supported in test CTEs and are errors that point at the hint. - `WITH`, `AS`, `SELECT` and the other keywords match ASCII letters in any case. - Every column of an expected CTE (`__expected__`, `__macro_expected__`, `__udf_expected__` or `__table_fn_expected__`) needs a name. A bare or qualified column reference such as `order_id` diff --git a/src/sqlbuild/compiler/compile/_helpers/analysis/ctes.py b/src/sqlbuild/compiler/compile/_helpers/analysis/ctes.py deleted file mode 100644 index 9433ea8e4a..0000000000 --- a/src/sqlbuild/compiler/compile/_helpers/analysis/ctes.py +++ /dev/null @@ -1,110 +0,0 @@ -"""Polyglot-assisted top-level CTE extraction helpers.""" - -from __future__ import annotations - -import logging -import re -from typing import Any - -from sqlbuild.compiler.compile.exceptions import CompileInputError -from sqlbuild.compiler.sql_analysis.constants import ( - POLYGLOT_ANALYSIS_BODY_SQL as _POLYGLOT_ANALYSIS_BODY_SQL, -) -from sqlbuild.compiler.sql_analysis.constants import ( - POLYGLOT_ANALYSIS_CTE_FACTS as _POLYGLOT_ANALYSIS_CTE_FACTS, -) -from sqlbuild.compiler.sql_analysis.constants import ( - POLYGLOT_ANALYSIS_NAME as _POLYGLOT_ANALYSIS_NAME, -) -from sqlbuild.compiler.sql_analysis.constants import ( - POLYGLOT_ANALYSIS_PROJECTIONS as _POLYGLOT_ANALYSIS_PROJECTIONS, -) -from sqlbuild.compiler.sql_analysis.constants import ( - POLYGLOT_ANALYSIS_SHAPE as _POLYGLOT_ANALYSIS_SHAPE, -) -from sqlbuild.compiler.sql_analysis.constants import ( - POLYGLOT_ANALYSIS_SHAPE_SELECT as _POLYGLOT_ANALYSIS_SHAPE_SELECT, -) -from sqlbuild.compiler.sql_analysis.constants import ( - POLYGLOT_ANALYSIS_TRANSFORM_CONSTANT as _POLYGLOT_ANALYSIS_TRANSFORM_CONSTANT, -) -from sqlbuild.compiler.sql_analysis.constants import ( - POLYGLOT_ANALYSIS_TRANSFORM_KIND as _POLYGLOT_ANALYSIS_TRANSFORM_KIND, -) -from sqlbuild.compiler.sql_analysis.constants import ( - POLYGLOT_ANALYSIS_UPSTREAM as _POLYGLOT_ANALYSIS_UPSTREAM, -) -from sqlbuild.compiler.sql_analysis.main.import_polyglot_sql import import_polyglot_sql -from sqlbuild.diagnostics.main.log_debug_event import log_debug_event - -_DEBUG_LOGGER: logging.Logger = logging.getLogger("sqlbuild.compile") -_CEREMONIAL_SELECT_PATTERN: re.Pattern[str] = re.compile(r"\bSELECT\s+1\s*;?\s*$", re.IGNORECASE) - - -def extract_top_level_ctes_with_sql_analysis( - *, sql: str, file_label: str, context_label: str -) -> tuple[tuple[str, str], ...] | None: - """Extract top-level CTE aliases and rendered bodies with Polyglot when available.""" - - if _CEREMONIAL_SELECT_PATTERN.search(sql) is None: - return None - polyglot_module: Any = import_polyglot_sql() - try: - analysis: Any = polyglot_module.analyze_query(sql, {"dialect": "generic"}) - except polyglot_module.PolyglotError as error: - log_debug_event( - logger=_DEBUG_LOGGER, - message="top-level CTE compact extraction failed; falling back", - sqlbuild_file=file_label, - sqlbuild_error=str(error), - ) - return None - if not isinstance(analysis, dict): - return None - if not _is_ceremonial_select_analysis(analysis): - return None - cte_facts: Any = analysis.get(_POLYGLOT_ANALYSIS_CTE_FACTS) - if not isinstance(cte_facts, list) or not cte_facts: - return None - - ctes: list[tuple[str, str]] = [] - seen_cte_names: set[str] = set() - cte_fact: Any - for cte_fact in cte_facts: - if not isinstance(cte_fact, dict): - return None - cte_name: str | None = _cte_fact_name(cte_fact) - if cte_name is None: - return None - if cte_name in seen_cte_names: - raise CompileInputError( - f"{context_label} '{file_label}' defines duplicate CTE '{cte_name}'" - ) - seen_cte_names.add(cte_name) - body_sql: Any = cte_fact.get(_POLYGLOT_ANALYSIS_BODY_SQL) - if not isinstance(body_sql, str) or not body_sql: - return None - ctes.append((cte_name, body_sql)) - return tuple(ctes) - - -def _is_ceremonial_select_analysis(analysis: dict[str, Any]) -> bool: - if analysis.get(_POLYGLOT_ANALYSIS_SHAPE) != _POLYGLOT_ANALYSIS_SHAPE_SELECT: - return False - projections: Any = analysis.get(_POLYGLOT_ANALYSIS_PROJECTIONS) - if not isinstance(projections, list) or len(projections) != 1: - return False - projection: Any = projections[0] - if not isinstance(projection, dict): - return False - if projection.get(_POLYGLOT_ANALYSIS_NAME) is not None: - return False - if projection.get(_POLYGLOT_ANALYSIS_TRANSFORM_KIND) != _POLYGLOT_ANALYSIS_TRANSFORM_CONSTANT: - return False - upstream: Any = projection.get(_POLYGLOT_ANALYSIS_UPSTREAM) - return isinstance(upstream, list) and not upstream - - -def _cte_fact_name(cte_fact: dict[str, Any]) -> str | None: - name: Any = cte_fact.get(_POLYGLOT_ANALYSIS_NAME) - return str(name) if name is not None else None diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/declaration_scope.py b/src/sqlbuild/compiler/compile/_helpers/attachment/declaration_scope.py index 48c29ad860..ec401a01c1 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/declaration_scope.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/declaration_scope.py @@ -16,6 +16,7 @@ build_declaration_scope_resolver, ) from sqlbuild.compiler.compile._helpers.sql_tests.extraction_errors import ( + validate_authored_scenario_cte_names, validate_authored_test_cte_names, ) from sqlbuild.compiler.compile.classes.sql_test_scan_cache import SqlTestScanCache @@ -52,6 +53,9 @@ def build_declaration_scope( validate_authored_test_cte_names( test_files=discovered_inputs.test_files, syntax=sql_lexical_syntax ) + validate_authored_scenario_cte_names( + scenario_files=discovered_inputs.scenario_files, syntax=sql_lexical_syntax + ) if native_stage_enabled(NativeStage.DECLARATION_SCOPES): native_scope: DeclarationScopeBuild | None = _build_native_declaration_scope( discovered_inputs=discovered_inputs, diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/scope_relationships.py b/src/sqlbuild/compiler/compile/_helpers/attachment/scope_relationships.py index 0eef949471..a722180754 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/scope_relationships.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/scope_relationships.py @@ -8,9 +8,6 @@ import orjson -from sqlbuild.compiler.compile._helpers.analysis.ctes import ( - extract_top_level_ctes_with_sql_analysis, -) from sqlbuild.compiler.compile._helpers.render.macros import ( find_macro_call_names, find_nested_macro_call_names, @@ -63,8 +60,6 @@ type _SharedDeclarations = dict[tuple[str, str], tuple[DeclarationRecord, ...]] type _BlockNames = tuple[str, ...] | Exception -_SQL_TEST_CONTEXT: str = "SQL test" - def build_scope_relationship_grants( *, @@ -324,14 +319,7 @@ def _natively_tested_macro_names( if native_ctes is None: return _tested_macro_names(sql=sql, file_label=file_label, syntax=syntax) if isinstance(native_ctes, str): - polyglot_ctes: tuple[tuple[str, str], ...] | None = ( - extract_top_level_ctes_with_sql_analysis( - sql=sql, file_label=file_label, context_label=_SQL_TEST_CONTEXT - ) - ) - if polyglot_ctes is None: - raise CompileInputError(native_ctes, bridge_independent=True) - native_ctes = polyglot_ctes + raise CompileInputError(native_ctes, bridge_independent=True) actual_sql: str | None = next( (body for name, body in native_ctes if name == MACRO_ACTUAL_TEST_CTE_NAME), None ) diff --git a/src/sqlbuild/compiler/compile/_helpers/scenarios/core.py b/src/sqlbuild/compiler/compile/_helpers/scenarios/core.py index ea20aa0217..40e81b2b60 100644 --- a/src/sqlbuild/compiler/compile/_helpers/scenarios/core.py +++ b/src/sqlbuild/compiler/compile/_helpers/scenarios/core.py @@ -7,9 +7,6 @@ import orjson import sqlbuild._native as _native -from sqlbuild.compiler.compile._helpers.analysis.ctes import ( - extract_top_level_ctes_with_sql_analysis, -) from sqlbuild.compiler.compile._helpers.sql_tests.core import ( _require_prefixed_name, _skip_ignorable, @@ -54,36 +51,15 @@ def extract_sql_scenario_ctes( ) if native_ctes is not None: return native_ctes - try: - ctes: tuple[CompileSqlScenarioCte, ...] = extract_top_level_ctes_with_scanner( - sql=sql, - file_label=file_label, - context_label=_CONTEXT, - with_requirement=_WITH_REQUIREMENT, - cte_type=CompileSqlScenarioCte, - syntax=syntax, - ) - except CompileInputError as scanner_error: - polyglot_ctes: tuple[CompileSqlScenarioCte, ...] | None = _polyglot_scenario_ctes( - sql=sql, file_label=file_label - ) - if polyglot_ctes is None: - raise scanner_error from None - ctes = polyglot_ctes - return _classify_sql_scenario_ctes(ctes=ctes, file_label=file_label, syntax=syntax) - - -def _polyglot_scenario_ctes( - *, sql: str, file_label: str -) -> tuple[CompileSqlScenarioCte, ...] | None: - cte_values: tuple[tuple[str, str], ...] | None = extract_top_level_ctes_with_sql_analysis( + ctes: tuple[CompileSqlScenarioCte, ...] = extract_top_level_ctes_with_scanner( sql=sql, file_label=file_label, context_label=_CONTEXT, + with_requirement=_WITH_REQUIREMENT, + cte_type=CompileSqlScenarioCte, + syntax=syntax, ) - if cte_values is None: - return None - return tuple(CompileSqlScenarioCte(name=name, sql_body=body) for name, body in cte_values) + return _classify_sql_scenario_ctes(ctes=ctes, file_label=file_label, syntax=syntax) def _native_scenario_ctes( @@ -101,7 +77,7 @@ def _native_scenario_ctes( return None try: return _scenario_from_native_outcome( - outcome=orjson.loads(response), sql=sql, file_label=file_label, syntax=syntax + outcome=orjson.loads(response), file_label=file_label, syntax=syntax ) except CompileInputError as error: error.bridge_independent = True @@ -109,18 +85,10 @@ def _native_scenario_ctes( def _scenario_from_native_outcome( - *, outcome: dict[str, Any], sql: str, file_label: str, syntax: SqlLexicalSyntax + *, outcome: dict[str, Any], file_label: str, syntax: SqlLexicalSyntax ) -> CompileSqlScenarioCtes: - """Build the native outcome, running only Python's Polyglot steps the native scan leaves.""" + """Build the native outcome, running Python's independence check only where it parses.""" - scan_error: str | None = outcome.get("scanError") - if scan_error is not None: - polyglot_ctes: tuple[CompileSqlScenarioCte, ...] | None = _polyglot_scenario_ctes( - sql=sql, file_label=file_label - ) - if polyglot_ctes is None: - raise CompileInputError(scan_error) - return _classify_sql_scenario_ctes(ctes=polyglot_ctes, file_label=file_label, syntax=syntax) independence: list[list[str]] | None = outcome.get("independence") if independence is not None: validate_independent_expected_and_assertion_ctes( diff --git a/src/sqlbuild/compiler/compile/_helpers/sql_tests/core.py b/src/sqlbuild/compiler/compile/_helpers/sql_tests/core.py index fe99c6d313..47a41fee26 100644 --- a/src/sqlbuild/compiler/compile/_helpers/sql_tests/core.py +++ b/src/sqlbuild/compiler/compile/_helpers/sql_tests/core.py @@ -7,9 +7,6 @@ from typing import Any import sqlbuild._native as _native -from sqlbuild.compiler.compile._helpers.analysis.ctes import ( - extract_top_level_ctes_with_sql_analysis, -) from sqlbuild.compiler.compile._helpers.refs.references import extract_sql_references from sqlbuild.compiler.compile.constants import ( DEFAULT_SQL_TEST_MODE, @@ -47,6 +44,8 @@ _CONTEXT: str = "SQL test" _SQL_TEST_WITH_REQUIREMENT: str = "mock CTEs and one __expected__ CTE" _DIRECT_DEPENDENCY_PATH_LENGTH: int = 2 +_MATERIALIZED_KEYWORD: str = "MATERIALIZED" +_NOT_KEYWORD: str = "NOT" _SQL_IDENTIFIER_QUOTE_TOKENS: frozenset[str] = frozenset({'"', "`"}) @@ -55,25 +54,14 @@ def extract_unclassified_sql_test_ctes( ) -> tuple[CompileSqlTestCte, ...]: """Extract raw top-level CTEs before mode-specific classification.""" - try: - ctes: tuple[CompileSqlTestCte, ...] = extract_top_level_ctes_with_scanner( - sql=sql, - file_label=file_label, - context_label=_CONTEXT, - with_requirement=_SQL_TEST_WITH_REQUIREMENT, - cte_type=CompileSqlTestCte, - syntax=syntax, - ) - except CompileInputError as scanner_error: - cte_values: tuple[tuple[str, str], ...] | None = extract_top_level_ctes_with_sql_analysis( - sql=sql, - file_label=file_label, - context_label="SQL test", - ) - if cte_values is None: - raise scanner_error from None - ctes = tuple(CompileSqlTestCte(name=name, sql_body=body) for name, body in cte_values) - return ctes + return extract_top_level_ctes_with_scanner( + sql=sql, + file_label=file_label, + context_label=_CONTEXT, + with_requirement=_SQL_TEST_WITH_REQUIREMENT, + cte_type=CompileSqlTestCte, + syntax=syntax, + ) def extract_sql_test_expected_model_names( @@ -236,6 +224,14 @@ def _scan_top_level_ctes( context_label=context_label, ) index = _skip_ignorable(sql=sql, start=index, context_label=context_label, syntax=syntax) + hint: str | None = _materialization_hint( + sql=sql, start=index, context_label=context_label, syntax=syntax + ) + if hint is not None: + raise CompileInputError( + f"{context_label} '{file_label}' CTE '{cte_name}' must not use AS {hint}; " + f"materialization hints are not supported in {context_label} CTEs" + ) if index >= len(sql) or sql[index] != SQL_OPEN_PAREN_TOKEN: raise CompileInputError( f"{context_label} '{file_label}' CTE '{cte_name}' must use AS (...)" @@ -256,6 +252,20 @@ def _scan_top_level_ctes( return tuple(ctes), index +def _materialization_hint( + *, sql: str, start: int, context_label: str, syntax: SqlLexicalSyntax +) -> str | None: + if _try_consume_keyword(sql=sql, start=start, keyword=_MATERIALIZED_KEYWORD) is not None: + return _MATERIALIZED_KEYWORD + not_end: int | None = _try_consume_keyword(sql=sql, start=start, keyword=_NOT_KEYWORD) + if not_end is None: + return None + index: int = _skip_ignorable(sql=sql, start=not_end, context_label=context_label, syntax=syntax) + if _try_consume_keyword(sql=sql, start=index, keyword=_MATERIALIZED_KEYWORD) is None: + return None + return f"{_NOT_KEYWORD} {_MATERIALIZED_KEYWORD}" + + def _require_prefixed_name( *, cte_name: str, prefix: str, label: str, file_label: str, context_label: str = _CONTEXT ) -> str: diff --git a/src/sqlbuild/compiler/compile/_helpers/sql_tests/extraction_errors.py b/src/sqlbuild/compiler/compile/_helpers/sql_tests/extraction_errors.py index 52c5039e6b..b755dc283f 100644 --- a/src/sqlbuild/compiler/compile/_helpers/sql_tests/extraction_errors.py +++ b/src/sqlbuild/compiler/compile/_helpers/sql_tests/extraction_errors.py @@ -4,6 +4,7 @@ import sys from dataclasses import dataclass +from pathlib import Path from sqlbuild.compiler.compile._helpers.diagnostics.collector import report_compile_diagnostic from sqlbuild.compiler.compile._helpers.refs.references import ( @@ -19,7 +20,11 @@ from sqlbuild.compiler.compile.exceptions import CompileInputError, SqlTestExtractionError from sqlbuild.compiler.compile.models import CompilerDiagnostic, SqlReferenceScan from sqlbuild.compiler.compile.types import DiagnosticPhase, DiagnosticSeverity -from sqlbuild.compiler.discovery.models import DiscoveredSqlTestBlock, DiscoveredSqlTestFile +from sqlbuild.compiler.discovery.models import ( + DiscoveredSqlScenarioFile, + DiscoveredSqlTestBlock, + DiscoveredSqlTestFile, +) from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from sqlbuild.spec.contracts.models import SourceLocation @@ -62,6 +67,37 @@ def validate_authored_test_cte_names( ) from None +def validate_authored_scenario_cte_names( + *, scenario_files: tuple[DiscoveredSqlScenarioFile, ...], syntax: SqlLexicalSyntax +) -> None: + """Raise the located CTE-name or materialization error of the first scenario that has one.""" + + try: + _ = authored_sql_test_cte_batch( + tests=tuple( + (scenario_file.sql_body, str(scenario_file.relative_path)) + for scenario_file in scenario_files + ), + syntax=syntax, + scenarios=True, + ) + except SqlTestExtractionError as error: + failed: DiscoveredSqlScenarioFile = scenario_files[error.test_index] + offset: int | None = ( + _authored_offset( + contents=failed.contents, + authored=failed.sql_body, + span=failed.sql_body_span, + body_offset=error.token_offset, + ) + if error.token_offset is not None + else None + ) + raise _located_error( + error=error, contents=failed.contents, relative_path=failed.relative_path, offset=offset + ) from None + + def located_extraction_error( *, error: SqlTestExtractionError, @@ -78,15 +114,25 @@ def located_extraction_error( if error.token_offset is not None and sql == test_block.sql_body else None ) + return _located_error( + error=error, + contents=test_file.contents, + relative_path=test_file.relative_path, + offset=offset, + ) + + +def _located_error( + *, error: SqlTestExtractionError, contents: str, relative_path: Path, offset: int | None +) -> CompileInputError: if offset is None: return CompileInputError( error.message, code=error.code, help=error.help, bridge_independent=True ) - contents: str = test_file.contents line: int = contents.count(_LINE_BREAK, 0, offset) + 1 column: int = offset - (contents.rfind(_LINE_BREAK, 0, offset) + 1) + 1 return CompileInputError( - f"{error.message}\n --> {test_file.relative_path.as_posix()}:{line}:{column}", + f"{error.message}\n --> {relative_path.as_posix()}:{line}:{column}", code=error.code, help=error.help, bridge_independent=True, @@ -149,15 +195,25 @@ def authored_file_offset( ) -> int | None: """Map an offset in a block's cleaned SQL back to the file, or None if it cannot be exact.""" - span: tuple[int, int] | None = test_block.sql_body_span + return _authored_offset( + contents=test_file.contents, + authored=test_block.sql_body, + span=test_block.sql_body_span, + body_offset=body_offset, + ) + + +def _authored_offset( + *, contents: str, authored: str, span: tuple[int, int] | None, body_offset: int +) -> int | None: if span is None: return None lines: tuple[_CleanedLine, ...] | None = _cleaned_lines( - authored=test_block.sql_body, span=span, contents=test_file.contents + authored=authored, span=span, contents=contents ) if lines is None: return None - body: str = test_block.sql_body + body: str = authored line_index: int = body.count(_LINE_BREAK, 0, body_offset) column: int = body_offset - (body.rfind(_LINE_BREAK, 0, body_offset) + 1) cleaned: _CleanedLine = lines[line_index] diff --git a/src/sqlbuild/compiler/compile/_helpers/sql_tests/native.py b/src/sqlbuild/compiler/compile/_helpers/sql_tests/native.py index 427ba38bdf..76cabcd9b7 100644 --- a/src/sqlbuild/compiler/compile/_helpers/sql_tests/native.py +++ b/src/sqlbuild/compiler/compile/_helpers/sql_tests/native.py @@ -168,15 +168,21 @@ def authored_sql_test_ctes( def authored_sql_test_cte_batch( - *, tests: tuple[tuple[str, str], ...], syntax: SqlLexicalSyntax + *, tests: tuple[tuple[str, str], ...], syntax: SqlLexicalSyntax, scenarios: bool = False ) -> tuple[tuple[tuple[str, int, str], ...], ...]: - """Read many authored blocks at once; a CTE-name error names its block by `test_index`.""" + """Read many authored blocks or scenarios at once; a CTE error names one by `test_index`.""" if not tests: return () results: list[object] = _native_batch( tests=[ - {"sql": sql, "fileLabel": file_label, "mode": "model", "authored": True} + { + "sql": sql, + "fileLabel": file_label, + "mode": "model", + "authored": True, + "scenario": scenarios, + } for sql, file_label in tests ], syntax=syntax, diff --git a/src/sqlbuild/compiler/discovery/_helpers/native/sql_test_files.py b/src/sqlbuild/compiler/discovery/_helpers/native/sql_test_files.py index 9ea9fbe8be..da39899a51 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/native/sql_test_files.py +++ b/src/sqlbuild/compiler/discovery/_helpers/native/sql_test_files.py @@ -191,10 +191,11 @@ def _scenario_file_from_payload( error: Exception | None = native_payload_error(payload=payload, file_path=file_path) if error is not None: raise error - _tag, contents, values, sql_body = payload + _tag, contents, values, sql_body, sql_body_span = payload return build_sql_scenario_file( header_values=project_native_header_values(cast(dict[str, object], values)), sql_body=str(sql_body), + sql_body_span=cast(tuple[int, int], sql_body_span), contents=str(contents), file_path=file_path, relative_path=relative_path, diff --git a/src/sqlbuild/compiler/discovery/_helpers/sql/scenarios.py b/src/sqlbuild/compiler/discovery/_helpers/sql/scenarios.py index 4037b9909a..9ddb5b669c 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/sql/scenarios.py +++ b/src/sqlbuild/compiler/discovery/_helpers/sql/scenarios.py @@ -21,6 +21,7 @@ def build_sql_scenario_file( contents: str, file_path: Path, relative_path: Path, + sql_body_span: tuple[int, int] | None = None, ) -> DiscoveredSqlScenarioFile: """Validate a header-parsed SCENARIO file with only supported keys and build its record.""" @@ -43,6 +44,7 @@ def build_sql_scenario_file( contents=contents, header_values=header_values, sql_body=sql_body, + sql_body_span=sql_body_span, name=file_path.stem, ownership_root=Path(SQL_SCENARIOS_OWNERSHIP_ROOT), ) diff --git a/src/sqlbuild/compiler/discovery/models.py b/src/sqlbuild/compiler/discovery/models.py index ad2e2ad0c0..82cf620cea 100644 --- a/src/sqlbuild/compiler/discovery/models.py +++ b/src/sqlbuild/compiler/discovery/models.py @@ -376,6 +376,7 @@ class DiscoveredSqlScenarioFile: sql_body: str name: str ownership_root: Path = Path(SQL_SCENARIOS_OWNERSHIP_ROOT) + sql_body_span: tuple[int, int] | None = None @dataclass(frozen=True) diff --git a/src/sqlbuild/compiler/sql_analysis/constants.py b/src/sqlbuild/compiler/sql_analysis/constants.py index d40fe049bf..87156fe212 100644 --- a/src/sqlbuild/compiler/sql_analysis/constants.py +++ b/src/sqlbuild/compiler/sql_analysis/constants.py @@ -141,12 +141,10 @@ POLYGLOT_NULLIF_FUNCTION_NAME: str = "NULLIF" POLYGLOT_BINARY_OPERAND_COUNT: int = 2 POLYGLOT_ANALYSIS_BASE_TABLES: str = "baseTables" -POLYGLOT_ANALYSIS_BODY_SQL: str = "bodySql" POLYGLOT_ANALYSIS_CAST_TYPE: str = "castType" POLYGLOT_ANALYSIS_COLUMN_USES: str = "columnUses" POLYGLOT_ANALYSIS_CONTEXT: str = "context" POLYGLOT_ANALYSIS_CONTEXT_FILTER: str = "filter" -POLYGLOT_ANALYSIS_CTE_FACTS: str = "cteFacts" POLYGLOT_ANALYSIS_EXPRESSION_SQL: str = "expressionSql" POLYGLOT_ANALYSIS_IS_STAR: str = "isStar" POLYGLOT_ANALYSIS_NAME: str = "name" diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_test_errors/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_test_errors/_test_types.py index ad6a486147..1283ab599b 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_test_errors/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_test_errors/_test_types.py @@ -10,3 +10,4 @@ class NativeSqlTestErrorTestCase: description: str case_name: str expected_distinct_macro_call_counts: int + expected_location: str = "" diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_test_errors/test_native_sql_test_errors.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_test_errors/test_native_sql_test_errors.py index 3e227f83bd..411ed75228 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_test_errors/test_native_sql_test_errors.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/sql_test_errors/test_native_sql_test_errors.py @@ -80,6 +80,24 @@ case_name="scenario-first-of-two-errors", expected_distinct_macro_call_counts=1, ), + NativeSqlTestErrorTestCase( + description="a quoted scenario CTE name", + case_name="scenario-quoted-cte-name", + expected_distinct_macro_call_counts=1, + expected_location="--> tests/scenarios/orders.sql:6:1", + ), + NativeSqlTestErrorTestCase( + description="a materialized scenario CTE", + case_name="scenario-materialized-cte", + expected_distinct_macro_call_counts=1, + expected_location="--> tests/scenarios/orders.sql:6:25", + ), + NativeSqlTestErrorTestCase( + description="a quoted macro test CTE name", + case_name="macro-test-quoted-cte-name", + expected_distinct_macro_call_counts=1, + expected_location="--> tests/unit/test_stg_orders.sql:4:1", + ), NativeSqlTestErrorTestCase( description="a test mocking an unknown source", case_name="test-mocks-unknown-source", @@ -131,6 +149,8 @@ def test_given_sql_test_error_when_compiling_with_each_engine_then_errors_and_ty == {stderr_without_durations(stderr=runs[0].stderr)}, f"[{case.expected_code}]" in runs[0].report + runs[0].stderr, str(case.expected_message) in runs[0].report + runs[0].stderr, + (case.expected_help or "") in runs[0].report + runs[0].stderr, + test_case.expected_location in runs[0].report + runs[0].stderr, len(set(macro_calls)), tuple(error_types), ) == ( @@ -139,6 +159,8 @@ def test_given_sql_test_error_when_compiling_with_each_engine_then_errors_and_ty True, True, True, + True, + True, test_case.expected_distinct_macro_call_counts, ("CompileInputError", "CompileInputError", "CompileInputError"), ), ( diff --git a/tests/integration/src/sqlbuild/compiler/attachments/helpers.py b/tests/integration/src/sqlbuild/compiler/attachments/helpers.py index 92f2affef3..623ac34f95 100644 --- a/tests/integration/src/sqlbuild/compiler/attachments/helpers.py +++ b/tests/integration/src/sqlbuild/compiler/attachments/helpers.py @@ -862,21 +862,13 @@ def raw_test_compile_outcome( ) _SCENARIO_ERROR_WEIGHTS: tuple[int, int] = (3, 1) _DEFERRED_ANSWER: str = '{"deferred": true}' -_NATIVE_SCENARIO_ANSWERS: dict[tuple[frozenset[str], bool], str] = { - (frozenset({"deferred"}), False): "deferred", - (frozenset({"deferred"}), True): "deferred", - (frozenset({"scanError"}), False): "error", - (frozenset({"scanError"}), True): "polyglot", - (frozenset({"independence", "scenario"}), False): "independence", - (frozenset({"independence", "scenario"}), True): "independence", - (frozenset({"independence", "error"}), False): "independence", - (frozenset({"independence", "error"}), True): "independence", - (frozenset({"error"}), False): "error", - (frozenset({"error"}), True): "error", - (frozenset({"scenario"}), False): "extracted", - (frozenset({"scenario"}), True): "extracted", +_NATIVE_SCENARIO_ANSWERS: dict[frozenset[str], str] = { + frozenset({"deferred"}): "deferred", + frozenset({"independence", "scenario"}): "independence", + frozenset({"independence", "error"}): "independence", + frozenset({"error"}): "error", + frozenset({"scenario"}): "extracted", } -_CEREMONIAL_SELECT_TAIL: re.Pattern[str] = re.compile(r"\bSELECT\s+1\s*;?\s*$", re.IGNORECASE) _SCENARIO_TAILS: tuple[str, ...] = ("\nSELECT 1\n", "", ";", "\nSELECT 2", " -- end", ";\n") _SCENARIO_KEYWORDS: tuple[str, ...] = ( "WITH\n", @@ -923,9 +915,7 @@ def native_scenario_answer(*, sql: str, syntax: SqlLexicalSyntax) -> str: sql, "tests/scenarios/orders.sql", syntax.native_mapping ) outcome: dict[str, object] = orjson.loads(response or _DEFERRED_ANSWER) - return _NATIVE_SCENARIO_ANSWERS[ - (frozenset(outcome), _CEREMONIAL_SELECT_TAIL.search(sql) is not None) - ] + return _NATIVE_SCENARIO_ANSWERS[frozenset(outcome)] _TARGET_NAMES: tuple[str, ...] = ("orders", "customers", "channel_codes", "returns", "order_rows") diff --git a/tests/integration/src/sqlbuild/compiler/scopes/test_native_scope_parity.py b/tests/integration/src/sqlbuild/compiler/scopes/test_native_scope_parity.py index 4b520b593e..e2c35f1d45 100644 --- a/tests/integration/src/sqlbuild/compiler/scopes/test_native_scope_parity.py +++ b/tests/integration/src/sqlbuild/compiler/scopes/test_native_scope_parity.py @@ -88,13 +88,17 @@ expected_error="SQL test 'tests/unit/test_eu_label.sql' CTE '__macro_actual__' must", ), ScopeEngineParityTestCase( - description="a macro test only Polyglot reads keeps Python's fallback", + description="a macro test materialization hint fails with the located extraction error", files={ **SCOPED_PROJECT, "tests/unit/test_eu_label.sql": _MACRO_TEST_HEADER + "WITH\n__macro_actual__ AS MATERIALIZED (\n SELECT @cents('1') AS label\n),\n" "__macro_expected__ AS (\n SELECT 100 AS label\n)\nSELECT 1\n", }, + expected_error=( + "SQL test 'tests/unit/test_eu_label.sql' CTE '__macro_actual__' must not use " + "AS MATERIALIZED; materialization hints are not supported in SQL test CTEs" + ), ), ScopeEngineParityTestCase( description="non-ASCII folders keep Python's repr order of path keys", diff --git a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_scenarios.py b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_scenarios.py index 713af6c8e3..b1a145bfa0 100644 --- a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_scenarios.py +++ b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_scenarios.py @@ -147,21 +147,6 @@ expected_expected_model_names=("dim_countries",), expected_assertion_names=(), ), - ExtractSqlScenarioCtesTestCase( - description="extracts scenario ctes with sql_analysis fallback syntax", - sql=""" - WITH - "__source__raw__orders" AS MATERIALIZED (SELECT 1 AS order_id), - "__expected__daily_revenue" AS (SELECT order_id FROM "__source__raw__orders") - SELECT 1 - """.strip(), - expected_authored_cte_names=("__source__raw__orders",), - expected_source_fixture_names=("raw__orders",), - expected_ref_fixture_names=(), - expected_seed_fixture_names=(), - expected_expected_model_names=("daily_revenue",), - expected_assertion_names=(), - ), ], ids=lambda case: case.description, ) @@ -228,6 +213,29 @@ def test_given_sql_scenario_cte_variants_when_extracting_then_it_returns_expecte "'__assert__positive_order_id' through 'assertion_helper'" ), ), + ExtractSqlScenarioCtesErrorTestCase( + description="raises when a scenario cte name is quoted", + sql=""" + WITH + "__source__raw__orders" AS (SELECT 1 AS order_id), + __expected__daily_revenue AS (SELECT 1 AS order_id) + SELECT 1 + """.strip(), + expected_error_fragment="expected a CTE name", + ), + ExtractSqlScenarioCtesErrorTestCase( + description="raises when a scenario cte uses a materialization hint", + sql=""" + WITH + __source__raw__orders AS MATERIALIZED (SELECT 1 AS order_id), + __expected__daily_revenue AS (SELECT 1 AS order_id) + SELECT 1 + """.strip(), + expected_error_fragment=( + "CTE '__source__raw__orders' must not use AS MATERIALIZED; materialization " + "hints are not supported in SQL scenario CTEs" + ), + ), ExtractSqlScenarioCtesErrorTestCase( description="raises when scenario has no fixture ctes", sql=""" diff --git a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_tests.py b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_tests.py index 7fccaea116..130ed2df68 100644 --- a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_tests.py +++ b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_tests.py @@ -677,7 +677,10 @@ def test_given_direct_logic_sql_test_cte_variants_when_extracting_then_it_return __expected__orders AS (SELECT order_id FROM __source__raw_orders) SELECT 1 """.strip(), - expected_error_fragment="CTE '__source__raw_orders' must use AS \\(\\.\\.\\.\\)", + expected_error_fragment=( + "CTE '__source__raw_orders' must not use AS MATERIALIZED; materialization hints " + "are not supported in SQL test CTEs" + ), ), ExtractSqlTestCtesErrorTestCase( description="raises when expected cte omits target name", @@ -894,7 +897,10 @@ def test_given_invalid_sql_test_cte_variants_when_extracting_then_it_raises_clea CteScannerMessageTestCase( description="sql test with an invalid cte name keeps sql test wording", sql="WITH 1orders AS (SELECT 1) SELECT 1", - expected_message="SQL test 'tests/unit/orders.sql' expected a CTE name", + expected_message=( + "SQL test 'tests/unit/orders.sql' CTE name '1orders' must be an unquoted " + "identifier of ASCII letters, digits and underscores" + ), ), CteScannerMessageTestCase( description="sql test without a ceremonial select keeps sql test wording", diff --git a/tests/unit/src/sqlbuild/compiler/discovery/helpers.py b/tests/unit/src/sqlbuild/compiler/discovery/helpers.py index 40d07313b9..ff09c62576 100644 --- a/tests/unit/src/sqlbuild/compiler/discovery/helpers.py +++ b/tests/unit/src/sqlbuild/compiler/discovery/helpers.py @@ -120,6 +120,7 @@ def parse_sql_scenario_file( contents=str(payload[1]), file_path=file_path, relative_path=relative_path, + sql_body_span=cast(tuple[int, int], payload[4]), ) diff --git a/website/src/content/docs/docs/concepts/scenarios.mdx b/website/src/content/docs/docs/concepts/scenarios.mdx index 7c97a6756d..d0bde5599d 100644 --- a/website/src/content/docs/docs/concepts/scenarios.mdx +++ b/website/src/content/docs/docs/concepts/scenarios.mdx @@ -99,6 +99,15 @@ false filter, as in [unit tests](/docs/concepts/testing). Every scenario must have at least one fixture CTE and at least one `__expected__` or `__assert__` CTE. The file ends after its last CTE; a trailing ceremonial `SELECT 1` is accepted but not required. +Scenario CTEs follow the same strict rules as [unit test CTEs](/docs/concepts/testing), on every +compiler engine, and other forms are compile errors that point at the offending text: + +- A CTE name is an unquoted identifier of ASCII letters, digits and underscores that starts with a + letter or underscore. Quoted names such as `"__source__raw_orders"` or `` `recent orders` ``, and + names with `$` or non-ASCII characters, are errors. +- A CTE body follows `AS` directly. Materialization hints such as `AS MATERIALIZED (...)` or + `AS NOT MATERIALIZED (...)` are not supported in scenario CTEs. + Scenario SQL uses macros, enums, and constants available from the scenario file's directory under `tests/scenarios/`. Public enums and constants available to a model are also available when the scenario defines `__expected__model_name` output for that model. Model-private values and macros diff --git a/website/src/content/docs/docs/concepts/testing.mdx b/website/src/content/docs/docs/concepts/testing.mdx index 85c9691352..9bd4818c32 100644 --- a/website/src/content/docs/docs/concepts/testing.mdx +++ b/website/src/content/docs/docs/concepts/testing.mdx @@ -119,6 +119,8 @@ with a compile error that names the CTE: - A CTE name is an unquoted identifier of ASCII letters, digits and underscores that starts with a letter or underscore. Quoted names such as `"recent orders"` or `` `recent orders` ``, and names with `$` or non-ASCII characters, are errors that point at the name. +- A CTE body follows `AS` directly. Materialization hints such as `AS MATERIALIZED (...)` or + `AS NOT MATERIALIZED (...)` are not supported in test CTEs and are errors that point at the hint. - `WITH`, `AS`, `SELECT` and the other keywords match ASCII letters in any case. - Every column of an expected CTE (`__expected__`, `__macro_expected__`, `__udf_expected__` or `__table_fn_expected__`) needs a name. A bare or qualified column reference such as `order_id`