diff --git a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs index 508cd181ad..801b526570 100644 --- a/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs +++ b/crates/sqlbuild-analysis/src/compiler/_helpers/sql_tests/planning.rs @@ -1664,7 +1664,7 @@ fn unresolved_reference_warnings(request: UnresolvedReferenceRequest<'_>) -> Vec let mut protected = ProtectedRanges::new(&patterns.lexical, sql); for name in marker_names_in(&patterns.reference, &mut protected) { let message = format!( - "test '{test_name}': model '{model_name}' references __ref('{name}') which has no mock and is not in the expected chain" + "test '{test_name}': model '{model_name}' references __ref(\"{name}\") which has no mock and is not in the expected chain" ); warn(REF_FUNCTION, name, message); } @@ -1674,7 +1674,7 @@ fn unresolved_reference_warnings(request: UnresolvedReferenceRequest<'_>) -> Vec ] { for name in marker_names_in(pattern, &mut protected) { let message = format!( - "test '{test_name}': model '{model_name}' references {function_name}('{name}') which has no mock" + "test '{test_name}': model '{model_name}' references {function_name}(\"{name}\") which has no mock" ); warn(function_name, name, message); } diff --git a/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs b/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs index ace252923f..a006e32514 100644 --- a/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs +++ b/crates/sqlbuild-analysis/src/compiler/tests/helpers.rs @@ -337,7 +337,7 @@ pub(crate) fn unresolved_reference_fast_rejection_preserves_warning() -> bool { json!([{ "modelName": "orders", "severity": "error", - "message": "test 'orders_case': model 'orders' references __source('missing_orders') which has no mock" + "message": "test 'orders_case': model 'orders' references __source(\"missing_orders\") which has no mock" }]) ); true @@ -519,7 +519,7 @@ pub(crate) fn deep_shared_graph_reports_missing_mock_once() -> bool { json!([{ "modelName": "orders_00", "severity": "error", - "message": "test 'orders_totals': model 'orders_00' references __source('raw_orders') which has no mock" + "message": "test 'orders_totals': model 'orders_00' references __source(\"raw_orders\") which has no mock" }]), "sql_analysis_enabled={sql_analysis_enabled}" ); diff --git a/crates/sqlbuild-rules/src/engine/tests/test_sql_scanning.rs b/crates/sqlbuild-rules/src/engine/tests/test_sql_scanning.rs index 52a9a2f15f..755ea5799a 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_sql_scanning.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_sql_scanning.rs @@ -299,7 +299,7 @@ fn given_quoted_commented_and_malformed_fragments_when_scanning_then_every_scann test_case.description ); assert_eq!( - extract(&format!("SELECT {fragment} __ref('b')")).is_some(), + extract(&format!("SELECT {fragment} __ref(\"b\")")).is_some(), test_case.expected_reference_fast_path, "SQL reference fast path: {}", test_case.description diff --git a/crates/sqlbuild-sqltext/src/compiler/_helpers/sql_references/extraction.rs b/crates/sqlbuild-sqltext/src/compiler/_helpers/sql_references/extraction.rs index 1ab6ca4e89..e685f55c4a 100644 --- a/crates/sqlbuild-sqltext/src/compiler/_helpers/sql_references/extraction.rs +++ b/crates/sqlbuild-sqltext/src/compiler/_helpers/sql_references/extraction.rs @@ -3,7 +3,6 @@ use crate::compiler::types::StaticReference; use crate::constants::{DBT_REFERENCE_KIND, TABLE_FUNCTION_REFERENCE_KIND}; use crate::sql_scan::main::non_code_end::non_code_end; -use crate::sql_scan::main::quote_end::quote_end; use crate::sql_scan::models::QuotePolicy; const PREFIXES: [(&str, &str); 6] = [ @@ -39,13 +38,14 @@ pub(crate) fn extract(sql: &str) -> Option> { return None; } let (first_name, first_end) = reference_name(sql, index + prefix.len())?; - let mut cursor = ascii_whitespace_end(bytes, first_end); + let mut cursor = first_end; let mut package = None; let name; - if *kind == DBT_REFERENCE_KIND && bytes.get(cursor).copied() == Some(b',') { - cursor = ascii_whitespace_end(bytes, cursor + 1); - let (second_name, second_end) = reference_name(sql, cursor)?; - cursor = ascii_whitespace_end(bytes, second_end); + let separator = ascii_whitespace_end(bytes, cursor); + if *kind == DBT_REFERENCE_KIND && bytes.get(separator).copied() == Some(b',') { + let second_start = ascii_whitespace_end(bytes, separator + 1); + let (second_name, second_end) = reference_name(sql, second_start)?; + cursor = second_end; package = Some(first_name); name = second_name; } else { @@ -60,27 +60,17 @@ pub(crate) fn extract(sql: &str) -> Option> { Some(references) } +/// Read one bare `"name"` argument, the only form compile replacement accepts. fn reference_name(sql: &str, start: usize) -> Option<(String, usize)> { let bytes = sql.as_bytes(); - let start = ascii_whitespace_end(bytes, start); - let first = bytes.get(start).copied()?; - if matches!(first, b'\'' | b'"') { - let Ok(end) = quote_end(bytes, start, QuotePolicy::COMPILER) else { - return None; - }; - return Some((sql[start + 1..end - 1].to_owned(), end)); - } - if !is_identifier_start(first) { + if bytes.get(start).copied() != Some(b'"') { return None; } - let mut end = start + 1; - while bytes.get(end).copied().is_some_and(is_identifier_continue) { - end += 1; - } - if bytes.get(end).is_some_and(|value| !value.is_ascii()) { + let close = start + 1 + bytes[start + 1..].iter().position(|value| *value == b'"')?; + if close == start + 1 { return None; } - Some((sql[start..end].to_owned(), end)) + Some((sql[start + 1..close].to_owned(), close + 1)) } fn ascii_whitespace_end(bytes: &[u8], start: usize) -> usize { @@ -93,11 +83,3 @@ fn ascii_whitespace_end(bytes: &[u8], start: usize) -> usize { } end } - -fn is_identifier_start(value: u8) -> bool { - value.is_ascii_alphabetic() -} - -fn is_identifier_continue(value: u8) -> bool { - value.is_ascii_alphanumeric() || value == b'_' -} diff --git a/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs b/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs index 84ca3c6112..8f13e1b58e 100644 --- a/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs +++ b/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs @@ -43,7 +43,7 @@ pub(crate) fn dynamic_or_malformed_sql_requests_fallback() -> bool { pub(crate) fn simple_references_preserve_authored_order() -> bool { extract( - "SELECT * FROM __source('orders') UNION ALL SELECT * FROM __dbt_ref(\"shop\", \"customers\")", + "SELECT * FROM __source(\"orders\") UNION ALL SELECT * FROM __dbt_ref(\"shop\" , \"customers\")", ) == Some(vec![ ("source".to_owned(), "orders".to_owned(), None, None), ( @@ -56,7 +56,7 @@ pub(crate) fn simple_references_preserve_authored_order() -> bool { } pub(crate) fn comments_and_quoted_text_hide_references() -> bool { - extract("-- __ref(\"ignored\")\nSELECT '__seed(\"also_ignored\")' FROM __ref(orders)") + extract("-- __ref(\"ignored\")\nSELECT '__seed(\"also_ignored\")' FROM __ref(\"orders\")") == Some(vec![("ref".to_owned(), "orders".to_owned(), None, None)]) } @@ -64,7 +64,7 @@ pub(crate) fn dollar_quoted_text_hides_references() -> bool { extract(concat!( "SELECT $$Customer's order -- __ref(\"ignored\") $5$$ AS label, ", "$tag$ $$ __seed(\"also_ignored\") $tag$ AS note, price$1$ ", - "FROM __ref(orders)" + "FROM __ref(\"orders\")" )) == Some(vec![("ref".to_owned(), "orders".to_owned(), None, None)]) } @@ -77,6 +77,13 @@ pub(crate) fn complex_or_malformed_sql_requests_fallback() -> bool { "SELECT * FROM __ref(\"orders\") WHERE note = 'unterminated", "SELECT * FROM __ref(\"orders\") WHERE note = $$unterminated", "SELECT * FROM __ref(örders)", + "SELECT * FROM __ref(orders)", + "SELECT * FROM __ref('orders')", + "SELECT * FROM __ref( \"orders\" )", + "SELECT * FROM __ref(/* upstream */ \"orders\")", + "SELECT * FROM __ref(\"\")", + "SELECT * FROM __ref(\"ord\"\"ers\")", + "SELECT * FROM __dbt_ref(\"shop\", \"customers\" )", ] .into_iter() .all(|sql| extract(sql).is_none()) diff --git a/crates/sqlbuild-sqltext/src/sql_references/_helpers/scan.rs b/crates/sqlbuild-sqltext/src/sql_references/_helpers/scan.rs index b76bf9b2c1..9eac895239 100644 --- a/crates/sqlbuild-sqltext/src/sql_references/_helpers/scan.rs +++ b/crates/sqlbuild-sqltext/src/sql_references/_helpers/scan.rs @@ -1,9 +1,9 @@ //! The general reference scan of Python `extract_sql_references`, reproduced exactly. use crate::sql_references::constants::{ - DBT_REFERENCE_KIND, NAME_QUOTE_BYTES, NON_CODE_START_BYTES, PAIRED_QUOTE_LENGTH, - PYTHON_ASCII_WHITESPACE, QUOTE_BYTES, REFERENCE_CONTEXT, REFERENCE_PREFIXES, - TABLE_FUNCTION_CALL_CONTEXT, TABLE_FUNCTION_REFERENCE_KIND, + DBT_REFERENCE_KIND, NON_CODE_START_BYTES, PYTHON_ASCII_WHITESPACE, QUOTE_BYTES, + REFERENCE_CONTEXT, REFERENCE_PREFIXES, TABLE_FUNCTION_CALL_CONTEXT, + TABLE_FUNCTION_REFERENCE_KIND, }; use crate::sql_references::models::SqlReference; use crate::sql_references::types::ReferencePrefix; @@ -54,7 +54,7 @@ pub(crate) fn scan_references(sql: &[u8], syntax: &LexicalSyntax) -> Scan Scan Scan<(SqlReference, usize)> { - let (prefix, kind, call_name) = *call; + let (prefix, kind) = *call; let open = start + prefix.len() - 1; let close = matching_paren(sql, open, syntax, REFERENCE_CONTEXT)?; - let arguments = split_arguments(sql, open + 1, close, syntax)?; - if kind == DBT_REFERENCE_KIND { - if !matches!(arguments.len(), 1 | 2) { - return Err(failed(format!( - "{call_name} must contain one name argument or package/name arguments" - ))); - } - if let [package, name] = arguments.as_slice() { - let package = reference_name(package, kind, call_name)?; - let name = reference_name(name, kind, call_name)?; - return Ok(( - SqlReference { - kind, - name, - package: Some(package), - call_argument_count: None, - }, - close + 1, - )); - } - } else if arguments.len() != 1 { - return Err(failed(format!( - "{call_name} must contain exactly one name argument" - ))); - } + let arguments = &sql[open + 1..close]; + let (name, package) = if kind == DBT_REFERENCE_KIND { + dbt_reference_names(arguments)? + } else { + (bare_name(arguments)?, None) + }; let mut call_argument_count = None; if kind == TABLE_FUNCTION_REFERENCE_KIND { let suffix = python_whitespace_end(sql, close + 1)?; if sql.get(suffix) != Some(&b'(') { - return Err(failed(format!( - "{call_name} must be followed by an argument list" - ))); + return Err(Stop::Deferred); } let suffix_close = matching_paren(sql, suffix, syntax, TABLE_FUNCTION_CALL_CONTEXT)?; call_argument_count = Some(split_arguments(sql, suffix + 1, suffix_close, syntax)?.len()); @@ -115,14 +94,57 @@ fn parse_reference( Ok(( SqlReference { kind, - name: reference_name(&arguments[0], kind, call_name)?, - package: None, + name, + package, call_argument_count, }, close + 1, )) } +/// Python `"([^"]+)"` matched against the whole argument text. +fn bare_name(arguments: &[u8]) -> Scan { + match quoted_name_end(arguments, 0) { + Some(end) if end == arguments.len() => utf8(&arguments[1..end - 1]), + _ => Err(Stop::Deferred), + } +} + +/// Python `"([^"]+)"(?:\s*,\s*"([^"]+)")?` matched against the whole argument text. +fn dbt_reference_names(arguments: &[u8]) -> Scan<(String, Option)> { + let Some(first_end) = quoted_name_end(arguments, 0) else { + return Err(Stop::Deferred); + }; + if first_end == arguments.len() { + return Ok((utf8(&arguments[1..first_end - 1])?, None)); + } + let separator = python_whitespace_end(arguments, first_end)?; + if arguments.get(separator) != Some(&b',') { + return Err(Stop::Deferred); + } + let second_start = python_whitespace_end(arguments, separator + 1)?; + match quoted_name_end(arguments, second_start) { + Some(end) if end == arguments.len() => Ok(( + utf8(&arguments[second_start + 1..end - 1])?, + Some(utf8(&arguments[1..first_end - 1])?), + )), + _ => Err(Stop::Deferred), + } +} + +/// The offset after a `"name"` with at least one character starting at `start`. +fn quoted_name_end(arguments: &[u8], start: usize) -> Option { + if arguments.get(start) != Some(&b'"') { + return None; + } + let close = start + + 1 + + arguments[start + 1..] + .iter() + .position(|byte| *byte == b'"')?; + (close > start + 1).then_some(close + 1) +} + /// Python `_split_top_level_arguments` over `sql[start..end]`: comments read as one space. fn split_arguments( sql: &[u8], @@ -180,47 +202,6 @@ fn split_arguments( Ok(arguments) } -/// Python `_parse_reference_name` for one stripped argument. -fn reference_name(argument: &[u8], kind: &str, call_name: &str) -> Scan { - let paired_quote = |quotes: &[u8]| { - argument.len() >= PAIRED_QUOTE_LENGTH - && argument[0] == argument[argument.len() - 1] - && quotes.contains(&argument[0]) - }; - if kind == TABLE_FUNCTION_REFERENCE_KIND && !paired_quote(b"\"") { - return Err(failed(format!( - "{call_name} name argument must be double quoted" - ))); - } - if paired_quote(NAME_QUOTE_BYTES) { - return utf8(&argument[1..argument.len() - 1]); - } - if identifier_validity(argument)? { - return utf8(argument); - } - Err(failed(format!( - "{call_name} name argument must be a quoted string or identifier" - ))) -} - -/// Python `value.replace("_", "a").isalnum() and value[0].isalpha()` for a non-empty value. -fn identifier_validity(value: &[u8]) -> Scan { - if value - .iter() - .any(|byte| byte.is_ascii() && !(byte.is_ascii_alphanumeric() || *byte == b'_')) - { - return Ok(false); - } - if value[0].is_ascii() && !value[0].is_ascii_alphabetic() { - return Ok(false); - } - if value.is_ascii() { - Ok(true) - } else { - Err(Stop::Deferred) - } -} - /// Python `str.strip()`; a non-ASCII character at either edge may be Unicode whitespace. fn python_strip(value: &[u8]) -> Scan<&[u8]> { let start = value diff --git a/crates/sqlbuild-sqltext/src/sql_references/constants.rs b/crates/sqlbuild-sqltext/src/sql_references/constants.rs index eeb3d80be3..086b820dc9 100644 --- a/crates/sqlbuild-sqltext/src/sql_references/constants.rs +++ b/crates/sqlbuild-sqltext/src/sql_references/constants.rs @@ -2,14 +2,14 @@ use crate::sql_references::types::ReferencePrefix; -/// Call prefixes with their kinds and error names, in Python's matching order. +/// Call prefixes with their kinds, in Python's matching order. pub(crate) const REFERENCE_PREFIXES: [ReferencePrefix; 6] = [ - (b"__dbt_ref(", "dbt_ref", "__dbt_ref"), - (b"__table_fn(", "table_fn", "__table_fn"), - (b"__source(", "source", "__source"), - (b"__seed(", "seed", "__seed"), - (b"__udf(", "udf", "__udf"), - (b"__ref(", "ref", "__ref"), + (b"__dbt_ref(", "dbt_ref"), + (b"__table_fn(", "table_fn"), + (b"__source(", "source"), + (b"__seed(", "seed"), + (b"__udf(", "udf"), + (b"__ref(", "ref"), ]; pub(crate) const DBT_REFERENCE_KIND: &str = "dbt_ref"; pub(crate) const TABLE_FUNCTION_REFERENCE_KIND: &str = "table_fn"; @@ -21,9 +21,5 @@ pub(crate) const SUPPORTED_LINE_COMMENT_PREFIXES: [&str; 3] = ["--", "//", "#"]; pub(crate) const NON_CODE_START_BYTES: &[u8] = b"-/#'\"`$"; /// Non-code openers whose text an argument keeps; comments become one space. pub(crate) const QUOTE_BYTES: &[u8] = b"'\"`$"; -/// Shortest quoted reference name: the opening and closing quote. -pub(crate) const PAIRED_QUOTE_LENGTH: usize = 2; -/// Quotes that delimit a reference name. -pub(crate) const NAME_QUOTE_BYTES: &[u8] = b"'\""; /// Python `str.isspace()` for ASCII characters. pub(crate) const PYTHON_ASCII_WHITESPACE: &[u8] = b" \t\n\x0b\x0c\r\x1c\x1d\x1e\x1f"; diff --git a/crates/sqlbuild-sqltext/src/sql_references/tests/test_extract_sql_references.rs b/crates/sqlbuild-sqltext/src/sql_references/tests/test_extract_sql_references.rs index efe09d2111..a1c74c301f 100644 --- a/crates/sqlbuild-sqltext/src/sql_references/tests/test_extract_sql_references.rs +++ b/crates/sqlbuild-sqltext/src/sql_references/tests/test_extract_sql_references.rs @@ -11,8 +11,9 @@ fn given_sql_when_extracting_references_then_matches_python_scanner() { let test_cases = [ ExtractSqlReferencesTestCase { description: "every kind in authored order", - sql: "SELECT * FROM __ref(orders) JOIN __source('raw_orders') JOIN __seed(\"regions\") \ - JOIN __dbt_ref('shop', customers) WHERE __udf(clean)(x) > 0", + sql: "SELECT * FROM __ref(\"orders\") JOIN __source(\"raw_orders\") \ + JOIN __seed(\"regions\") JOIN __dbt_ref(\"shop\" ,\x1c\"customers\") \ + WHERE __udf(\"clean\")(x) > 0 OR __dbt_ref(\"orders\") IS NULL", syntax: generic(), expected_extraction: extracted(vec![ reference("ref", "orders"), @@ -20,6 +21,7 @@ fn given_sql_when_extracting_references_then_matches_python_scanner() { reference("seed", "regions"), dbt_reference("shop", "customers"), reference("udf", "clean"), + reference("dbt_ref", "orders"), ]), }, ExtractSqlReferencesTestCase { @@ -44,26 +46,26 @@ fn given_sql_when_extracting_references_then_matches_python_scanner() { ]), }, ExtractSqlReferencesTestCase { - description: "comments become spaces inside quoted names", + description: "comment inside a reference call defers to python", sql: "SELECT * FROM __ref(\"a\" /* note */ \"b\")", syntax: generic(), - expected_extraction: extracted(vec![reference("ref", "a\" \"b")]), + expected_extraction: ReferenceExtraction::Deferred, }, ExtractSqlReferencesTestCase { description: "comments, strings and dollar quotes hide references", - sql: "-- __ref(a)\nSELECT '__ref(b)', $$__ref(c)$$, /* __ref(d) */ 1 FROM __ref(e)", + sql: "-- __ref(a)\nSELECT '__ref(b)', $$__ref(c)$$, /* __ref(d) */ 1 FROM __ref(\"e\")", syntax: generic(), expected_extraction: extracted(vec![reference("ref", "e")]), }, ExtractSqlReferencesTestCase { description: "dialect hash comments hide references", - sql: "SELECT 1 # __ref(a)\nFROM __ref(b)", + sql: "SELECT 1 # __ref(a)\nFROM __ref(\"b\")", syntax: backslash_hash_comments(), expected_extraction: extracted(vec![reference("ref", "b")]), }, ExtractSqlReferencesTestCase { description: "dialect backslash escapes keep strings open", - sql: "SELECT 'it\\'s __ref(a)' FROM __ref(b)", + sql: "SELECT 'it\\'s __ref(a)' FROM __ref(\"b\")", syntax: backslash_hash_comments(), expected_extraction: extracted(vec![reference("ref", "b")]), }, @@ -74,89 +76,113 @@ fn given_sql_when_extracting_references_then_matches_python_scanner() { expected_extraction: extracted(vec![reference("ref", "commandes_é")]), }, ExtractSqlReferencesTestCase { - description: "wrong reference argument count", - sql: "SELECT * FROM __ref(a, b)", + description: "empty table function call argument", + sql: "SELECT * FROM __table_fn(\"f\")(1,,2)", syntax: generic(), - expected_extraction: failed("__ref must contain exactly one name argument"), + expected_extraction: failed("SQL reference contains an empty argument"), }, ExtractSqlReferencesTestCase { - description: "wrong dbt reference argument count", - sql: "SELECT * FROM __dbt_ref(a, b, c)", + description: "unclosed table function call", + sql: "SELECT * FROM __table_fn(\"f\")(1", syntax: generic(), - expected_extraction: failed( - "__dbt_ref must contain one name argument or package/name arguments", - ), + expected_extraction: failed("SQL table function call contains an unclosed parenthesis"), }, ExtractSqlReferencesTestCase { - description: "empty reference argument", - sql: "SELECT * FROM __source(a, )", + description: "unclosed reference", + sql: "SELECT * FROM __ref(orders", syntax: generic(), - expected_extraction: failed("SQL reference contains an empty argument"), + expected_extraction: failed("SQL reference contains an unclosed parenthesis"), }, ExtractSqlReferencesTestCase { - description: "empty table function call argument", - sql: "SELECT * FROM __table_fn(\"f\")(1,,2)", + description: "unclosed quoted text", + sql: "SELECT 'orders", syntax: generic(), - expected_extraction: failed("SQL reference contains an empty argument"), + expected_extraction: failed("SQL reference contains an unclosed quoted string"), }, ExtractSqlReferencesTestCase { - description: "table function without call suffix", - sql: "SELECT * FROM __table_fn(\"f\") /* x */ (1)", + description: "unquoted name defers to python", + sql: "SELECT * FROM __ref(orders)", syntax: generic(), - expected_extraction: failed("__table_fn must be followed by an argument list"), + expected_extraction: ReferenceExtraction::Deferred, }, ExtractSqlReferencesTestCase { - description: "table function call suffix checked before the name", - sql: "SELECT * FROM __table_fn(f)", + description: "single quoted name defers to python", + sql: "SELECT * FROM __source('raw_orders')", syntax: generic(), - expected_extraction: failed("__table_fn must be followed by an argument list"), + expected_extraction: ReferenceExtraction::Deferred, }, ExtractSqlReferencesTestCase { - description: "single quoted table function name", - sql: "SELECT * FROM __table_fn('f')(1)", + description: "spaces inside the call defer to python", + sql: "SELECT * FROM __seed( \"regions\" )", syntax: generic(), - expected_extraction: failed("__table_fn name argument must be double quoted"), + expected_extraction: ReferenceExtraction::Deferred, }, ExtractSqlReferencesTestCase { - description: "unclosed table function call", - sql: "SELECT * FROM __table_fn(\"f\")(1", + description: "empty quoted name defers to python", + sql: "SELECT * FROM __ref(\"\")", syntax: generic(), - expected_extraction: failed("SQL table function call contains an unclosed parenthesis"), + expected_extraction: ReferenceExtraction::Deferred, }, ExtractSqlReferencesTestCase { - description: "unclosed reference", - sql: "SELECT * FROM __ref(orders", + description: "doubled quote inside a name defers to python", + sql: "SELECT * FROM __ref(\"ord\"\"ers\")", syntax: generic(), - expected_extraction: failed("SQL reference contains an unclosed parenthesis"), + expected_extraction: ReferenceExtraction::Deferred, }, ExtractSqlReferencesTestCase { - description: "unclosed quoted text", - sql: "SELECT 'orders", + description: "second name argument defers to python", + sql: "SELECT * FROM __ref(\"a\", \"b\")", syntax: generic(), - expected_extraction: failed("SQL reference contains an unclosed quoted string"), + expected_extraction: ReferenceExtraction::Deferred, }, ExtractSqlReferencesTestCase { - description: "invalid identifier name", - sql: "SELECT * FROM __ref(1orders)", + description: "dbt reference trailing space defers to python", + sql: "SELECT * FROM __dbt_ref(\"shop\", \"customers\" )", syntax: generic(), - expected_extraction: failed( - "__ref name argument must be a quoted string or identifier", - ), + expected_extraction: ReferenceExtraction::Deferred, }, ExtractSqlReferencesTestCase { - description: "nested reference is not a name", - sql: "SELECT * FROM __ref(__ref(a))", + description: "dbt reference third name defers to python", + sql: "SELECT * FROM __dbt_ref(\"a\", \"b\", \"c\")", syntax: generic(), - expected_extraction: failed( - "__ref name argument must be a quoted string or identifier", - ), + expected_extraction: ReferenceExtraction::Deferred, }, ExtractSqlReferencesTestCase { - description: "non-ascii identifier defers", + description: "non-ascii space around a dbt comma defers to python", + sql: "SELECT * FROM __dbt_ref(\"shop\"\u{a0}, \"customers\")", + syntax: generic(), + expected_extraction: ReferenceExtraction::Deferred, + }, + ExtractSqlReferencesTestCase { + description: "table function without call suffix defers to python", + sql: "SELECT * FROM __table_fn(\"f\") /* x */ (1)", + syntax: generic(), + expected_extraction: ReferenceExtraction::Deferred, + }, + ExtractSqlReferencesTestCase { + description: "single quoted table function name defers to python", + sql: "SELECT * FROM __table_fn('f')(1)", + syntax: generic(), + expected_extraction: ReferenceExtraction::Deferred, + }, + ExtractSqlReferencesTestCase { + description: "nested reference name defers to python", + sql: "SELECT * FROM __ref(__ref(\"a\"))", + syntax: generic(), + expected_extraction: ReferenceExtraction::Deferred, + }, + ExtractSqlReferencesTestCase { + description: "non-ascii identifier defers to python", sql: "SELECT * FROM __ref(commandés)", syntax: generic(), expected_extraction: ReferenceExtraction::Deferred, }, + ExtractSqlReferencesTestCase { + description: "an earlier valid reference does not stop a later deferral", + sql: "SELECT * FROM __ref(\"a\") JOIN __ref(b)", + syntax: generic(), + expected_extraction: ReferenceExtraction::Deferred, + }, ExtractSqlReferencesTestCase { description: "non-ascii whitespace before a call suffix defers", sql: "SELECT * FROM __table_fn(\"f\")\u{a0}(1)", @@ -171,7 +197,7 @@ fn given_sql_when_extracting_references_then_matches_python_scanner() { }, ExtractSqlReferencesTestCase { description: "unsupported line comment prefix defers", - sql: "SELECT * FROM __ref(a)", + sql: "SELECT * FROM __ref(\"a\")", syntax: unsupported_comments(), expected_extraction: ReferenceExtraction::Deferred, }, diff --git a/crates/sqlbuild-sqltext/src/sql_references/types.rs b/crates/sqlbuild-sqltext/src/sql_references/types.rs index b45032984b..4ee75964a9 100644 --- a/crates/sqlbuild-sqltext/src/sql_references/types.rs +++ b/crates/sqlbuild-sqltext/src/sql_references/types.rs @@ -1,4 +1,4 @@ //! Shapes shared by the reference scan. -/// A call prefix such as `__ref(`, its Python reference kind and the name its errors use. -pub(crate) type ReferencePrefix = (&'static [u8], &'static str, &'static str); +/// A call prefix such as `__ref(` and its Python reference kind. +pub(crate) type ReferencePrefix = (&'static [u8], &'static str); diff --git a/scripts/compiler_differential/_helpers/corpus/failure_cases.py b/scripts/compiler_differential/_helpers/corpus/failure_cases.py index d0ec753932..4ff02ce71e 100644 --- a/scripts/compiler_differential/_helpers/corpus/failure_cases.py +++ b/scripts/compiler_differential/_helpers/corpus/failure_cases.py @@ -474,6 +474,65 @@ def _compile_failure_cases() -> tuple[FailureCase, ...]: ) }, ), + failure_case( + name="unquoted-ref-name", + expected_code="P012", + expected_message="__ref(stg_orders) is not a valid __ref() call", + files={ + FAILURE_MART_PATH: FAILURE_BASE_MART.replace( + '__ref("stg_orders")', "__ref(stg_orders)" + ) + }, + ), + failure_case( + name="commented-ref-argument", + expected_code="P012", + expected_message="is not a valid __ref() call", + files={ + FAILURE_MART_PATH: FAILURE_BASE_MART.replace( + '__ref("stg_orders")', "__ref( /* upstream */ 'stg_orders')" + ) + }, + ), + failure_case( + name="single-quoted-source-name", + expected_code="P012", + expected_message="__source('raw_orders') is not a valid __source() call", + files=staging_files( + FAILURE_BASE_STAGING.replace('__source("raw_orders")', "__source('raw_orders')") + ), + ), + failure_case( + name="invalid-reference-calls-in-two-files", + expected_code="P012", + expected_message="__ref(stg_orders) is not a valid __ref() call", + files={ + **staging_files( + FAILURE_BASE_STAGING.replace('__source("raw_orders")', "__source('raw_orders')") + ), + FAILURE_MART_PATH: FAILURE_BASE_MART.replace( + '__ref("stg_orders")', "__ref(stg_orders)" + ), + }, + ), + failure_case( + name="macro-returned-invalid-reference-call", + expected_code="P012", + expected_message=( + "__ref(stg_orders) is not a valid __ref() call, returned by macro staged_orders()" + ), + files={ + "models/marts/_sqlbuild/_macros/orders.py": ( + "def staged_orders() -> str:\n" + ' """Return the staged orders relation."""\n' + " return '__ref(stg_orders)'\n" + ), + **mart_body_files( + "SELECT customer_id, SUM(amount) AS total_amount\n" + "FROM @staged_orders()\nGROUP BY customer_id\n" + ), + }, + ), failure_case( name="invalid-declaration-name", expected_code="D016", diff --git a/scripts/compiler_differential/_helpers/corpus/reference_failure_cases.py b/scripts/compiler_differential/_helpers/corpus/reference_failure_cases.py index 9c9ebfd27a..e9ba5e96d3 100644 --- a/scripts/compiler_differential/_helpers/corpus/reference_failure_cases.py +++ b/scripts/compiler_differential/_helpers/corpus/reference_failure_cases.py @@ -9,10 +9,12 @@ _SELECT: str = "SELECT order_id AS customer_id, 1 AS total_amount\n" -def _mart_from(*, name: str, from_clause: str, expected_message: str) -> FailureCase: +def _mart_from( + *, name: str, from_clause: str, expected_code: str, expected_message: str +) -> FailureCase: return failure_case( name=name, - expected_code="P001", + expected_code=expected_code, expected_message=expected_message, files=mart_body_files(f"{_SELECT}FROM {from_clause}\n"), ) @@ -25,46 +27,55 @@ def reference_failure_cases() -> tuple[FailureCase, ...]: _mart_from( name="reference-two-name-arguments", from_clause='__ref("stg_orders", "orders")', - expected_message="__ref must contain exactly one name argument", + expected_code="P012", + expected_message='__ref("stg_orders", "orders") is not a valid __ref() call', ), _mart_from( name="reference-dbt-three-arguments", from_clause='__dbt_ref("shop", "orders", "extra")', - expected_message="__dbt_ref must contain one name argument or package/name arguments", + expected_code="P012", + expected_message='__dbt_ref("shop", "orders", "extra") is not a valid __dbt_ref() call', ), _mart_from( name="reference-empty-argument", from_clause='__ref("stg_orders", )', - expected_message="SQL reference contains an empty argument", + expected_code="P012", + expected_message='__ref("stg_orders", ) is not a valid __ref() call', ), _mart_from( name="reference-expression-name", from_clause="__ref(concat('stg_', 'orders'))", - expected_message="__ref name argument must be a quoted string or identifier", + expected_code="P012", + expected_message="__ref(concat('stg_', 'orders')) is not a valid __ref() call", ), _mart_from( name="reference-unclosed-call", from_clause='__ref("stg_orders"', + expected_code="P001", expected_message="SQL reference contains an unclosed parenthesis", ), _mart_from( name="table-function-without-arguments", from_clause='__table_fn("table_fn__orders_for") /* no call */', + expected_code="P012", expected_message="__table_fn must be followed by an argument list", ), _mart_from( name="table-function-single-quoted-name", from_clause="__table_fn('table_fn__orders_for')(1)", - expected_message="__table_fn name argument must be double quoted", + expected_code="P012", + expected_message="__table_fn('table_fn__orders_for') is not a valid __table_fn() call", ), _mart_from( name="table-function-empty-call-argument", from_clause='__table_fn("table_fn__orders_for")(1, , 2)', + expected_code="P001", expected_message="SQL reference contains an empty argument", ), _mart_from( name="table-function-unclosed-call", from_clause='__table_fn("table_fn__orders_for")(1 -- )', + expected_code="P001", expected_message="SQL table function call contains an unclosed parenthesis", ), ) diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/models.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/models.md index 3154c504b8..901bb6355a 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/models.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/models.md @@ -41,6 +41,20 @@ The header controls how SQLBuild builds, validates, and documents the model. The | Source | `__source("name")` | An external source | | Scalar UDF | `__udf("name")` | A user-defined function | +Write the name as exactly one double-quoted string, with no spaces or comments inside the parentheses. `__dbt_ref("package", "name")` is the only call that takes two names. Compile rejects any other form, such as `__ref(stg_orders)`, `__ref('stg_orders')` or `__ref( /* upstream */ "stg_orders")`, with a `P012` error at the call that shows the corrected call: + +```text +error[P012]: __ref(stg_orders) is not a valid __ref() call + model: fact_orders + --> models/marts/fact_orders.sql:24:6 + | + 24 | FROM __ref(stg_orders) o + | ^^^^^^^^^^^^^^^^^ + = help: __ref() takes exactly one double-quoted name, with no comments or extra spaces inside the parentheses: __ref("stg_orders") +``` + +One compile reports every rejected call. When a macro returns the rejected call, the error points at the macro and shows how to pass the reference in instead, for example `@staged_orders(__ref("stg_orders"))`. The same rule applies inside test and scenario files, including `__assert__` CTEs. + SQLBuild discovers the dependency graph from these calls and orders selected work topologically. Among selected models, upstream models run before downstream dependents. An unselected upstream is read from its existing warehouse relation; use an upstream-expanding selector such as `+fact_orders` when it should also be built. Seeds use `__seed()`, not `__ref()`. See [Functions](functions.md) for scalar UDF and table-function references. diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/audits.py b/src/sqlbuild/compiler/compile/_helpers/attachment/audits.py index 4234550ba8..05db949a19 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/audits.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/audits.py @@ -60,6 +60,7 @@ LoadedMacro, MacroContext, ModelInputBuildContext, + SqlReferenceOrigin, ) from sqlbuild.compiler.compile.types import ( AttachedAuditTargetKind, @@ -300,7 +301,10 @@ def _build_singular_audit_inputs( ) references: tuple[CompileSqlReference, ...] = merge_call_site_references( references=_combined_references( - expanded_sql_body, expanded_evidence_sql, syntax=sql_lexical_syntax + expanded_sql_body, + expanded_evidence_sql, + syntax=sql_lexical_syntax, + origin=_audit_reference_origin(audit_file), ), argument_references=_argument_references(expansion, evidence_expansion), ) @@ -727,7 +731,10 @@ def build_attached_audit_input( ) references: tuple[CompileSqlReference, ...] = merge_call_site_references( references=_combined_references( - expanded_sql_body, expanded_evidence_sql, syntax=context.sql_lexical_syntax + expanded_sql_body, + expanded_evidence_sql, + syntax=context.sql_lexical_syntax, + origin=_audit_reference_origin(definition[0]), ), argument_references=_argument_references(expansion, evidence_expansion), ) @@ -1050,13 +1057,21 @@ def _argument_references( return tuple(references) +def _audit_reference_origin(audit_file: DiscoveredAuditFile) -> SqlReferenceOrigin: + return SqlReferenceOrigin( + file_path=audit_file.file_path, + relative_path=audit_file.relative_path, + contents=audit_file.contents, + ) + + def _combined_references( - *sql_values: str | None, syntax: SqlLexicalSyntax + *sql_values: str | None, syntax: SqlLexicalSyntax, origin: SqlReferenceOrigin ) -> tuple[CompileSqlReference, ...]: """Extract references from independently compiled measurement/evidence queries.""" references: list[CompileSqlReference] = [] for sql in sql_values: if sql is not None: - references.extend(extract_sql_references(sql=sql, syntax=syntax)) + references.extend(extract_sql_references(sql=sql, syntax=syntax, origin=origin)) return tuple(dict.fromkeys(references)) diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/core.py b/src/sqlbuild/compiler/compile/_helpers/attachment/core.py index 9a64b6cca5..f16faa7ce7 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/core.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/core.py @@ -123,7 +123,10 @@ ModelHeaderColumnCache, ModelInputBuildContext, SqlAnalysisOptOutRequest, + SqlReferenceOrigin, + SqlReferenceScan, ) +from sqlbuild.compiler.compile.types import CompiledResourceType, SqlReferenceExtractor from sqlbuild.compiler.discovery.constants import PROJECT_CONFIG_FILENAME from sqlbuild.compiler.discovery.models import ( ConstantDeclaration, @@ -254,7 +257,7 @@ class _ModelValidationContext: project_config_path: Path defer_model_sql_validation: bool external_sql_reference_resolver: ExternalSqlReferenceResolver | None - extract_references: Callable[[str], tuple[CompileSqlReference, ...]] + extract_references: SqlReferenceExtractor known_model_names: set[str] known_seed_names: set[str] known_source_names: set[str] @@ -289,6 +292,7 @@ class _HookExpansionContext: consumer: ResourceIdentity | DeclarationIdentity facts: _HookExpansionFacts sql_lexical_syntax: SqlLexicalSyntax + reference_origin: SqlReferenceOrigin | None = None @dataclass @@ -465,7 +469,7 @@ def _build_model_inputs( no_sql_validation: bool, defer_model_sql_validation: bool, external_sql_reference_resolver: ExternalSqlReferenceResolver | None, - extract_references: Callable[[str], tuple[CompileSqlReference, ...]], + extract_references: SqlReferenceExtractor, legacy_schema_files: tuple[DiscoveredSchemaFile, ...], render_reuse: CompileRenderReuseSession | None, ) -> tuple[CompileModelInput, ...]: @@ -713,7 +717,7 @@ def _build_model_input( ) hook_expansion: HookExpansionResult = expand_model_hook_macros_result( values=effective_config.values, - file_path=model_file.file_path, + model_file=_model_reference_origin(model_file), effective_vars=effective_vars, context_values=build_model_context_values( values=effective_config.values, @@ -881,6 +885,16 @@ def _hook_expanded_config( ) +def _model_reference_origin(model_file: DiscoveredSqlModelFile) -> SqlReferenceOrigin: + return SqlReferenceOrigin( + file_path=model_file.file_path, + relative_path=model_file.relative_path, + contents=model_file.contents, + resource_type=CompiledResourceType.MODEL, + resource_name=model_file.file_path.stem, + ) + + def _validate_model_input( *, context: _ModelValidationContext, @@ -925,8 +939,11 @@ def _validate_model_input( file_path=model_file.file_path, placeholders=sql_validation_placeholders, ) + reference_scan: SqlReferenceScan = context.extract_references( + sql=expanded_query_sql, origin=_model_reference_origin(model_file) + ) references: tuple[CompileSqlReference, ...] = merge_call_site_references( - references=context.extract_references(expanded_query_sql), + references=reference_scan.references, argument_references=argument_references, ) validate_model_references( @@ -971,7 +988,11 @@ def _validate_model_input( query_sql=expanded_query_sql, custom_materialization_names=context.custom_materialization_names, ) - return sql_validation_enabled, references, rejected_opt_out + return ( + sql_validation_enabled and not reference_scan.invalid_calls, + references, + rejected_opt_out, + ) def _build_visible_declaration_indexes( @@ -1325,7 +1346,7 @@ def build_model_config(*, request: ModelConfigBuildRequest) -> CompileModelConfi def expand_model_hook_macros_result( *, values: dict[str, object], - file_path: Path, + model_file: SqlReferenceOrigin, effective_vars: dict[str, object], context_values: dict[str, str | None], loaded_macros: dict[str, LoadedMacro], @@ -1347,16 +1368,18 @@ def expand_model_hook_macros_result( expanded_values[hook_key] = expand_sql_macros_in_value( value=raw_hook_value, context=_HookExpansionContext( - file_path=file_path, + file_path=model_file.file_path, effective_vars=effective_vars, context_values=context_values, loaded_macros=loaded_macros, macro_context=macro_context, declaration_expansion=declaration_expansion, sql_hook_definitions=sql_hook_definitions or {}, - consumer=consumer or ResourceIdentity(ResourceKind.MODEL, file_path.stem), + consumer=consumer + or ResourceIdentity(ResourceKind.MODEL, model_file.file_path.stem), facts=facts, sql_lexical_syntax=sql_lexical_syntax, + reference_origin=model_file, ), hook_key=hook_key, ) @@ -1595,7 +1618,9 @@ def expand_sql_macros_in_value( facts.add_references( merge_call_site_references( references=extract_sql_references( - sql=expansion.sql, syntax=context.sql_lexical_syntax + sql=expansion.sql, + syntax=context.sql_lexical_syntax, + origin=context.reference_origin, ), argument_references=expansion.argument_references, ) diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/functions.py b/src/sqlbuild/compiler/compile/_helpers/attachment/functions.py index 03fc4cbf80..08c14d5418 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/functions.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/functions.py @@ -46,6 +46,7 @@ InferredColumn, LoadedMacro, MacroContext, + SqlReferenceOrigin, ) from sqlbuild.compiler.compile.types import ( FunctionLanguage, @@ -228,7 +229,15 @@ def build_sql_function_inputs( function_file=function_file, ) references: tuple[CompileSqlReference, ...] = merge_call_site_references( - references=extract_sql_references(sql=expanded_body_sql, syntax=sql_lexical_syntax), + references=extract_sql_references( + sql=expanded_body_sql, + syntax=sql_lexical_syntax, + origin=SqlReferenceOrigin( + file_path=function_file.file_path, + relative_path=function_file.relative_path, + contents=function_file.contents, + ), + ), argument_references=expansion.argument_references, ) validate_function_references( diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/references.py b/src/sqlbuild/compiler/compile/_helpers/attachment/references.py index 99b05079d7..8196bbcaf5 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/references.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/references.py @@ -41,7 +41,7 @@ def validate_model_references( raise CompileInputError( f"Model file {model_file.relative_path} references seed '{reference.ref_name}' " f"with {SqlReferenceKind.REF.placeholder_call('...')}. Use " - f"{SqlReferenceKind.SEED.example_call(reference.ref_name)} for seed " + f"{SqlReferenceKind.SEED.example_call(reference.ref_name, quote='"')} for seed " f"references; {SqlReferenceKind.REF.function_name} only resolves models." ) raise CompileInputError( @@ -57,7 +57,7 @@ def validate_model_references( f"Model file {model_file.relative_path} references model " f"'{reference.ref_name}' " f"with {SqlReferenceKind.SEED.placeholder_call('...')}. Use " - f"{SqlReferenceKind.REF.example_call(reference.ref_name)} for model " + f"{SqlReferenceKind.REF.example_call(reference.ref_name, quote='"')} for model " "references." ) raise CompileInputError( @@ -76,7 +76,7 @@ def validate_model_references( if external_sql_reference_resolver is None: raise CompileInputError( f"Model file {model_file.relative_path} uses " - f"{SqlReferenceKind.DBT_REF.example_call(reference.ref_name)} " + f"{SqlReferenceKind.DBT_REF.example_call(reference.ref_name, quote='"')} " "but no dbt manifest was found", code="C214", help=( @@ -198,7 +198,7 @@ def validate_function_references( raise CompileInputError( f"SQL function file {function_file.relative_path} references seed " f"'{reference.ref_name}' with {SqlReferenceKind.REF.placeholder_call('...')}. " - f"Use {SqlReferenceKind.SEED.example_call(reference.ref_name)} " + f"Use {SqlReferenceKind.SEED.example_call(reference.ref_name, quote='"')} " f"for seed references; {SqlReferenceKind.REF.function_name} only " "resolves models." ) @@ -225,8 +225,8 @@ def validate_function_references( if reference.ref_kind == SqlReferenceKind.DBT_REF: raise CompileInputError( f"SQL function file {function_file.relative_path} uses " - f"{SqlReferenceKind.DBT_REF.example_call(reference.ref_name)} but dbt refs " - "are not supported yet; " + f"{SqlReferenceKind.DBT_REF.example_call(reference.ref_name, quote='"')} " + "but dbt refs are not supported yet; " "support may be added in a future release" ) if ( @@ -273,8 +273,8 @@ def validate_audit_references( if reference.ref_kind == SqlReferenceKind.DBT_REF: raise CompileInputError( f"Audit file {audit_file.relative_path} may not use " - f"{SqlReferenceKind.DBT_REF.example_call(reference.ref_name)}; audit dbt " - "model checks belong in dbt" + f"{SqlReferenceKind.DBT_REF.example_call(reference.ref_name, quote='"')}; " + "audit dbt model checks belong in dbt" ) if ( reference.ref_kind == SqlReferenceKind.REF @@ -284,7 +284,7 @@ def validate_audit_references( raise CompileInputError( f"Audit file {audit_file.relative_path} references seed '{reference.ref_name}' " f"with {SqlReferenceKind.REF.placeholder_call('...')}. Use " - f"{SqlReferenceKind.SEED.example_call(reference.ref_name)} for seed " + f"{SqlReferenceKind.SEED.example_call(reference.ref_name, quote='"')} for seed " f"references; {SqlReferenceKind.REF.function_name} only resolves models." ) raise CompileInputError( diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/sql_tests.py b/src/sqlbuild/compiler/compile/_helpers/attachment/sql_tests.py index 8b29ceb4dd..8c62622748 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/sql_tests.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/sql_tests.py @@ -55,6 +55,7 @@ DeclarationScopeResolver, LoadedMacro, MacroContext, + SqlReferenceOrigin, ) from sqlbuild.compiler.compile.types import ( SqlTestMode, @@ -301,6 +302,7 @@ def build_test_inputs( extract_assertion_target_model_names( assertion_sql=tuple(cte.sql_body for cte in test_ctes.payload.assertion_ctes), syntax=sql_lexical_syntax, + origin=_sql_file_reference_origin(test.test_file), ) if isinstance(test_ctes.payload, CompileModelSqlTestCtes) else () @@ -494,7 +496,9 @@ def _infer_tested_udf_names( "__udf_actual__ CTE and exactly one __udf_expected__ CTE" ) references: tuple[CompileSqlReference, ...] = extract_sql_references( - sql=raw_test_ctes.payload.actual_cte.sql_body, syntax=syntax + sql=raw_test_ctes.payload.actual_cte.sql_body, + syntax=syntax, + origin=_sql_file_reference_origin(test_file), ) tested_udf_names: tuple[str, ...] = tuple( dict.fromkeys( @@ -539,7 +543,9 @@ def _infer_tested_table_function_names( "__table_fn_actual__ CTE and exactly one __table_fn_expected__ CTE" ) references: tuple[CompileSqlReference, ...] = extract_sql_references( - sql=raw_test_ctes.payload.actual_cte.sql_body, syntax=syntax + sql=raw_test_ctes.payload.actual_cte.sql_body, + syntax=syntax, + origin=_sql_file_reference_origin(test_file), ) validate_table_function_reference_arities( references=references, @@ -637,6 +643,7 @@ def build_scenario_inputs( assertion_target_model_names: tuple[str, ...] = extract_assertion_target_model_names( assertion_sql=tuple(cte.sql_body for cte in scenario_ctes.assertion_ctes), syntax=sql_lexical_syntax, + origin=_sql_file_reference_origin(scenario_file), ) scenario_inputs.append( CompileSqlScenarioInput( @@ -663,6 +670,16 @@ def build_scenario_inputs( return tuple(scenario_inputs) +def _sql_file_reference_origin( + sql_file: DiscoveredSqlTestFile | DiscoveredSqlScenarioFile, +) -> SqlReferenceOrigin: + return SqlReferenceOrigin( + file_path=sql_file.file_path, + relative_path=sql_file.relative_path, + contents=sql_file.contents, + ) + + def _validate_scenario_source_references( *, scenario_ctes: CompileSqlScenarioCtes, @@ -673,7 +690,7 @@ def _validate_scenario_source_references( cte: CompileSqlScenarioCte for cte in (*scenario_ctes.expected_ctes, *scenario_ctes.assertion_ctes): references: tuple[CompileSqlReference, ...] = extract_sql_references( - sql=cte.sql_body, syntax=syntax + sql=cte.sql_body, syntax=syntax, origin=_sql_file_reference_origin(scenario_file) ) reference: CompileSqlReference for reference in references: @@ -687,7 +704,9 @@ def _validate_scenario_source_references( ) for cte in scenario_ctes.authored_ctes: - references = extract_sql_references(sql=cte.sql_body, syntax=syntax) + references = extract_sql_references( + sql=cte.sql_body, syntax=syntax, origin=_sql_file_reference_origin(scenario_file) + ) for reference in references: if reference.ref_kind != SqlReferenceKind.SOURCE: continue diff --git a/src/sqlbuild/compiler/compile/_helpers/explicit_references/macro_arguments.py b/src/sqlbuild/compiler/compile/_helpers/explicit_references/macro_arguments.py index 6965afd73c..e16700e9b0 100644 --- a/src/sqlbuild/compiler/compile/_helpers/explicit_references/macro_arguments.py +++ b/src/sqlbuild/compiler/compile/_helpers/explicit_references/macro_arguments.py @@ -9,10 +9,23 @@ from sqlbuild.compiler.compile._helpers.diagnostics.collector import ( report_compile_diagnostic, ) -from sqlbuild.compiler.compile._helpers.refs.references import extract_sql_references -from sqlbuild.compiler.compile.constants import MACRO_GENERATED_REFERENCE_CODE +from sqlbuild.compiler.compile._helpers.refs.references import ( + extract_sql_references, + reference_call_location, + reference_call_syntax_key, + scan_sql_reference_calls, +) +from sqlbuild.compiler.compile.constants import ( + MACRO_GENERATED_REFERENCE_CODE, + REFERENCE_CALL_SYNTAX_CODE, +) from sqlbuild.compiler.compile.exceptions import CompileInputError -from sqlbuild.compiler.compile.models import CompilerDiagnostic, CompileSqlReference, LoadedMacro +from sqlbuild.compiler.compile.models import ( + CompilerDiagnostic, + CompileSqlReference, + InvalidSqlReferenceCall, + LoadedMacro, +) from sqlbuild.compiler.compile.types import ( CompiledResourceType, DiagnosticPhase, @@ -25,6 +38,7 @@ from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from sqlbuild.python_nodes.models import SqlResourceRef from sqlbuild.python_nodes.types import SqlResourceRefKind +from sqlbuild.spec.contracts.models import SourceLocation _TYPED_REFERENCE_KINDS: dict[str, SqlResourceRefKind] = { "__ref": SqlResourceRefKind.MODEL, @@ -45,6 +59,12 @@ ResourceKind.SEED: CompiledResourceType.SEED, } _REFERENCE_CALL_MARKERS: tuple[str, ...] = tuple(f"{name}(" for name in _TYPED_REFERENCE_KINDS) +_ALL_REFERENCE_CALL_MARKERS: tuple[str, ...] = tuple( + f"{kind.function_name}(" for kind in SqlReferenceKind +) +_PASSABLE_REFERENCE_KINDS: frozenset[SqlReferenceKind] = frozenset( + {SqlReferenceKind.REF, SqlReferenceKind.SOURCE, SqlReferenceKind.SEED} +) _RELATION_PLACEHOLDER_PREFIX: str = "__sqlbuild_relation_" _RELATION_PLACEHOLDER_PATTERN: re.Pattern[str] = re.compile( rf"{_RELATION_PLACEHOLDER_PREFIX}(\d+)__" @@ -168,11 +188,73 @@ def reject_macro_generated_references( ) +def report_macro_reference_call_syntax( + *, + loaded_macro: LoadedMacro, + macro_result: str, + file_path: Path, + consumer: ResourceIdentity | DeclarationIdentity | None, +) -> None: + """Report each reference call a macro returned that compile could not replace.""" + + if not any(marker in macro_result for marker in _ALL_REFERENCE_CALL_MARKERS): + return + try: + invalid_calls: tuple[InvalidSqlReferenceCall, ...] = scan_sql_reference_calls( + sql=macro_result, syntax=_GENERIC_SQL_SYNTAX + ).invalid_calls + except CompileInputError: + return + resource: ResourceIdentity | None = consumer if isinstance(consumer, ResourceIdentity) else None + resource_type: CompiledResourceType | None = ( + _RESOURCE_TYPE_BY_CONSUMER_KIND.get(resource.kind) if resource is not None else None + ) + location: str = ( + "the model" + if isinstance(consumer, ResourceIdentity) and consumer.kind is ResourceKind.MODEL + else "the calling SQL" + ) + for invalid_call in invalid_calls: + pass_in_help: str = ( + f"write {invalid_call.corrected_call} in {location} and pass it in: " + f"@{loaded_macro.name}({invalid_call.corrected_call})\n = help: " + if invalid_call.ref_kind in _PASSABLE_REFERENCE_KINDS + else "" + ) + report_compile_diagnostic( + key=reference_call_syntax_key( + file_path=file_path, call=invalid_call.call, location=None + ), + diagnostic=CompilerDiagnostic( + phase=DiagnosticPhase.COMPILE, + severity=DiagnosticSeverity.ERROR, + code=REFERENCE_CALL_SYNTAX_CODE, + message=f"{invalid_call.message}, returned by macro {loaded_macro.name}()", + resource_type=resource_type, + resource_name=( + resource.name if resource is not None and resource_type is not None else None + ), + path=loaded_macro.relative_path, + location=_macro_source_location(loaded_macro=loaded_macro, call=invalid_call.call), + help=f"{pass_in_help}{invalid_call.help}", + ), + ) + + +def _macro_source_location(*, loaded_macro: LoadedMacro, call: str) -> SourceLocation | None: + start: int = loaded_macro.raw_source.find(call) + if start == -1: + return None + return reference_call_location( + path=loaded_macro.relative_path, text=loaded_macro.raw_source, start=start, call=call + ) + + def _generated_references(sql: str) -> tuple[SqlResourceRef, ...]: try: - references: tuple[CompileSqlReference, ...] = extract_sql_references( + references: tuple[CompileSqlReference, ...] = scan_sql_reference_calls( sql=sql, syntax=_GENERIC_SQL_SYNTAX - ) + ).references except CompileInputError: return () generated: dict[SqlResourceRef, None] = {} diff --git a/src/sqlbuild/compiler/compile/_helpers/refs/cache.py b/src/sqlbuild/compiler/compile/_helpers/refs/cache.py index c6241d5e72..1abcdf65f4 100644 --- a/src/sqlbuild/compiler/compile/_helpers/refs/cache.py +++ b/src/sqlbuild/compiler/compile/_helpers/refs/cache.py @@ -6,19 +6,27 @@ import hmac import json import sqlite3 -from collections.abc import Callable, Iterator +from collections.abc import Iterator from contextlib import contextmanager from pathlib import Path from types import TracebackType from typing import Any, cast -from sqlbuild.compiler.compile._helpers.refs.references import extract_sql_references +from sqlbuild.compiler.compile._helpers.refs.references import ( + report_invalid_reference_calls, + scan_sql_reference_calls, +) from sqlbuild.compiler.compile.exceptions import AnalysisCacheEntryError -from sqlbuild.compiler.compile.models import CompileSqlReference +from sqlbuild.compiler.compile.models import ( + CompileSqlReference, + SqlReferenceOrigin, + SqlReferenceScan, +) +from sqlbuild.compiler.compile.types import SqlReferenceExtractor from sqlbuild.compiler.references.types import SqlReferenceKind from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax -_REFERENCE_CACHE_VERSION: int = 2 +_REFERENCE_CACHE_VERSION: int = 3 _CACHE_DATABASE_NAME: str = "sql-references.sqlite3" _CACHE_ENTRY_SEPARATOR: str = "\n" _MAX_CACHE_ENTRY_BYTES: int = 1_000_000 @@ -83,8 +91,8 @@ def __exit__( except sqlite3.DatabaseError: pass - def references(self, sql: str) -> tuple[CompileSqlReference, ...]: - """Return cached references or scan and record this exact expanded SQL.""" + def references(self, *, sql: str, origin: SqlReferenceOrigin | None) -> SqlReferenceScan: + """Return cached references, or scan, report rejected calls, and cache clean SQL.""" cache_key: str = _reference_cache_key(sql=sql, syntax_key=self._syntax_key) pending_contents: str | None = self._pending_contents_by_key.get(cache_key) @@ -94,7 +102,7 @@ def references(self, sql: str) -> tuple[CompileSqlReference, ...]: expected_cache_key=cache_key, ) if pending_references is not None: - return pending_references + return SqlReferenceScan(references=pending_references) connection: sqlite3.Connection | None = self._connection if connection is not None: try: @@ -107,25 +115,26 @@ def references(self, sql: str) -> tuple[CompileSqlReference, ...]: _references_from_contents(contents=row[0], expected_cache_key=cache_key) ) if cached_references is not None: - return cached_references + return SqlReferenceScan(references=cached_references) except sqlite3.DatabaseError: self._disable() connection = None - references: tuple[CompileSqlReference, ...] = extract_sql_references( - sql=sql, syntax=self._syntax + scan: SqlReferenceScan = scan_sql_reference_calls(sql=sql, syntax=self._syntax) + report_invalid_reference_calls( + invalid_calls=scan.invalid_calls, origin=origin, syntax=self._syntax ) - if self._database_path is not None: + if self._database_path is not None and not scan.invalid_calls: try: contents: str = _reference_contents( cache_key=cache_key, - references=references, + references=scan.references, ) if len(contents.encode()) <= _MAX_CACHE_ENTRY_BYTES: self._pending_contents_by_key[cache_key] = contents except (TypeError, ValueError): pass - return references + return scan def _write_pending(self, *, connection: sqlite3.Connection | None) -> sqlite3.Connection | None: if not self._pending_contents_by_key or self._database_path is None: @@ -256,7 +265,7 @@ def _cache_entry_digest(*, cache_key: str, serialized_payload: str) -> str: @contextmanager def cached_sql_reference_extractor( *, root: Path | None, syntax: SqlLexicalSyntax -) -> Iterator[Callable[[str], tuple[CompileSqlReference, ...]]]: +) -> Iterator[SqlReferenceExtractor]: """Yield an exact cached reference extractor for one compile invocation and dialect.""" with _SqlReferenceCache(root=root, syntax=syntax) as cache: diff --git a/src/sqlbuild/compiler/compile/_helpers/refs/references.py b/src/sqlbuild/compiler/compile/_helpers/refs/references.py index 782d015192..cf397055bf 100644 --- a/src/sqlbuild/compiler/compile/_helpers/refs/references.py +++ b/src/sqlbuild/compiler/compile/_helpers/refs/references.py @@ -3,10 +3,13 @@ from __future__ import annotations import re +from pathlib import Path import sqlbuild._native as _native +from sqlbuild.compiler.compile._helpers.diagnostics.collector import report_compile_diagnostic from sqlbuild.compiler.compile._helpers.refs.native import extract_native_sql_references from sqlbuild.compiler.compile.constants import ( + REFERENCE_CALL_SYNTAX_CODE, SQL_ARGUMENT_SEPARATOR_TOKEN, SQL_CLOSE_PAREN_TOKEN, SQL_OPEN_PAREN_TOKEN, @@ -14,7 +17,14 @@ SQL_REFERENCE_NAME_QUOTE_TOKENS, ) from sqlbuild.compiler.compile.exceptions import CompileInputError -from sqlbuild.compiler.compile.models import CompileSqlReference +from sqlbuild.compiler.compile.models import ( + CompilerDiagnostic, + CompileSqlReference, + InvalidSqlReferenceCall, + SqlReferenceOrigin, + SqlReferenceScan, +) +from sqlbuild.compiler.compile.types import DiagnosticPhase, DiagnosticSeverity from sqlbuild.compiler.frontier.constants import COMPILER_ENGINE_ENV_VAR from sqlbuild.compiler.frontier.exceptions import NativeStageMismatchError from sqlbuild.compiler.frontier.main.native_stage_enabled import native_stage_enabled @@ -23,10 +33,22 @@ from sqlbuild.compiler.sql_analysis.main._find_matching_paren import find_matching_paren from sqlbuild.compiler.sql_analysis.main._skip_dialect_non_code import dialect_non_code_end from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax +from sqlbuild.spec.contracts.models import SourceLocation _CONTEXT: str = "SQL reference" -_DOUBLE_QUOTE_TOKEN: str = '"' _PAIRED_QUOTE_CHARACTER_COUNT: int = 2 +_DBT_REF_PACKAGE_ARGUMENT_COUNT: int = 2 +_NAME_ARGUMENT_PATTERN: re.Pattern[str] = re.compile(r'"([^"]+)"') +_DBT_REF_ARGUMENTS_PATTERN: re.Pattern[str] = re.compile(r'"([^"]+)"(?:\s*,\s*"([^"]+)")?') +_AUTHORED_NAME_PATTERN: re.Pattern[str] = re.compile(r"[A-Za-z_][A-Za-z0-9_.]*") +_PLACEHOLDER_NAMES: dict[SqlReferenceKind, tuple[str, ...]] = { + SqlReferenceKind.REF: ("model_name",), + SqlReferenceKind.SOURCE: ("source_name",), + SqlReferenceKind.SEED: ("seed_name",), + SqlReferenceKind.UDF: ("function_name",), + SqlReferenceKind.TABLE_FUNCTION: ("function_name",), + SqlReferenceKind.DBT_REF: ("package_name", "model_name"), +} _REFERENCE_PREFIXES: tuple[tuple[str, SqlReferenceKind], ...] = ( ("__dbt_ref(", SqlReferenceKind.DBT_REF), ("__table_fn(", SqlReferenceKind.TABLE_FUNCTION), @@ -42,46 +64,52 @@ def extract_sql_references( - *, sql: str, syntax: SqlLexicalSyntax + *, sql: str, syntax: SqlLexicalSyntax, origin: SqlReferenceOrigin | None = None ) -> tuple[CompileSqlReference, ...]: - """Return logical SQL refs found outside comments and quoted text under the dialect's rules.""" + """Return logical SQL refs outside comments and quotes, reporting rejected calls as P012.""" + + scan: SqlReferenceScan = scan_sql_reference_calls(sql=sql, syntax=syntax) + report_invalid_reference_calls(invalid_calls=scan.invalid_calls, origin=origin, syntax=syntax) + return scan.references + + +def scan_sql_reference_calls(*, sql: str, syntax: SqlLexicalSyntax) -> SqlReferenceScan: + """Return valid references and rejected reference calls without reporting either.""" if native_stage_enabled(NativeStage.REFERENCE_EXTRACTION): - return _extract_sql_references_natively(sql=sql, syntax=syntax) + return _scan_sql_references_natively(sql=sql, syntax=syntax) if syntax.reads_differently_from_generic(sql): - return _extract_sql_references_with_python(sql=sql, syntax=syntax) + return _scan_sql_references_with_python(sql=sql, syntax=syntax) native_references: list[tuple[str, str, str | None, int | None]] | None = ( _native.extract_static_sql_references(sql) ) if native_references is not None: - return tuple( - CompileSqlReference( - ref_kind=SqlReferenceKind(kind), - ref_name=name, - ref_package=package, - call_argument_count=call_argument_count, + return SqlReferenceScan( + references=tuple( + CompileSqlReference( + ref_kind=SqlReferenceKind(kind), + ref_name=name, + ref_package=package, + call_argument_count=call_argument_count, + ) + for kind, name, package, call_argument_count in native_references ) - for kind, name, package, call_argument_count in native_references ) - return _extract_sql_references_with_python(sql=sql, syntax=syntax) + return _scan_sql_references_with_python(sql=sql, syntax=syntax) -def _extract_sql_references_natively( - *, sql: str, syntax: SqlLexicalSyntax -) -> tuple[CompileSqlReference, ...]: +def _scan_sql_references_natively(*, sql: str, syntax: SqlLexicalSyntax) -> SqlReferenceScan: """Return the native refs; a native error stands only when Python raises the same error.""" native_references: tuple[CompileSqlReference, ...] | str | None = extract_native_sql_references( sql=sql, syntax=syntax ) if native_references is None: - return _extract_sql_references_with_python(sql=sql, syntax=syntax) + return _scan_sql_references_with_python(sql=sql, syntax=syntax) if not isinstance(native_references, str): - return native_references + return SqlReferenceScan(references=native_references) try: - python_references: tuple[CompileSqlReference, ...] = _extract_sql_references_with_python( - sql=sql, syntax=syntax - ) + python_scan: SqlReferenceScan = _scan_sql_references_with_python(sql=sql, syntax=syntax) except CompileInputError as error: if str(error) == native_references: raise @@ -90,7 +118,10 @@ def _extract_sql_references_natively( ) from error raise _reference_mismatch( native_message=native_references, - python_outcome=f"found {len(python_references)} reference(s)", + python_outcome=( + f"found {len(python_scan.references)} reference(s) and " + f"{len(python_scan.invalid_calls)} rejected call(s)" + ), ) @@ -102,12 +133,105 @@ def _reference_mismatch(*, native_message: str, python_outcome: str) -> NativeSt ) -def _extract_sql_references_with_python( - *, sql: str, syntax: SqlLexicalSyntax -) -> tuple[CompileSqlReference, ...]: +def report_invalid_reference_calls( + *, + invalid_calls: tuple[InvalidSqlReferenceCall, ...], + origin: SqlReferenceOrigin | None, + syntax: SqlLexicalSyntax, +) -> None: + """Report each rejected reference call at its authored location when one is known.""" + + if not invalid_calls: + return + authored_starts: dict[str, list[int]] = _authored_invalid_call_starts( + origin=origin, syntax=syntax + ) + seen: dict[str, int] = {} + for invalid_call in invalid_calls: + occurrence: int = seen.get(invalid_call.call, 0) + seen[invalid_call.call] = occurrence + 1 + starts: list[int] = authored_starts.get(invalid_call.call, []) + location: SourceLocation | None = ( + reference_call_location( + path=origin.relative_path, + text=origin.contents, + start=starts[occurrence], + call=invalid_call.call, + ) + if origin is not None and occurrence < len(starts) + else None + ) + report_compile_diagnostic( + key=reference_call_syntax_key( + file_path=origin.file_path if origin is not None else None, + call=invalid_call.call, + location=location, + ), + diagnostic=CompilerDiagnostic( + phase=DiagnosticPhase.COMPILE, + severity=DiagnosticSeverity.ERROR, + code=REFERENCE_CALL_SYNTAX_CODE, + message=invalid_call.message, + resource_type=origin.resource_type if origin is not None else None, + resource_name=origin.resource_name if origin is not None else None, + path=origin.relative_path if origin is not None and location is None else None, + location=location, + help=invalid_call.help, + ), + ) + + +def reference_call_syntax_key( + *, file_path: Path | None, call: str, location: SourceLocation | None +) -> tuple[str, ...]: + """Return the collector key shared by every report of one rejected reference call.""" + + owner: str = file_path.as_posix() if file_path is not None else "" + if location is None: + return (REFERENCE_CALL_SYNTAX_CODE, owner, call) + return ( + REFERENCE_CALL_SYNTAX_CODE, + owner, + f"{location.line:09d}:{location.column:09d}", + call, + ) + + +def _authored_invalid_call_starts( + *, origin: SqlReferenceOrigin | None, syntax: SqlLexicalSyntax +) -> dict[str, list[int]]: + if origin is None: + return {} + try: + authored: SqlReferenceScan = _scan_sql_references_with_python( + sql=origin.contents, syntax=syntax + ) + except CompileInputError: + return {} + starts: dict[str, list[int]] = {} + for invalid_call in authored.invalid_calls: + starts.setdefault(invalid_call.call, []).append(invalid_call.start) + return starts + + +def reference_call_location(*, path: Path, text: str, start: int, call: str) -> SourceLocation: + """Return the span of ``call`` starting at ``start`` in ``text`` as a source location.""" + + end: int = start + len(call) + return SourceLocation( + path=path, + line=text.count("\n", 0, start) + 1, + column=start - (text.rfind("\n", 0, start) + 1) + 1, + end_line=text.count("\n", 0, end) + 1, + end_column=end - (text.rfind("\n", 0, end) + 1) + 1, + ) + + +def _scan_sql_references_with_python(*, sql: str, syntax: SqlLexicalSyntax) -> SqlReferenceScan: """Return logical SQL refs through the authoritative general scanner.""" references: list[CompileSqlReference] = [] + invalid_calls: list[InvalidSqlReferenceCall] = [] index: int = 0 length: int = len(sql) while index < length: @@ -121,17 +245,18 @@ def _extract_sql_references_with_python( index = non_code_end continue - parsed_reference: tuple[CompileSqlReference, int] | None = _parse_reference_at( - sql=sql, - start=index, - syntax=syntax, + parsed: tuple[CompileSqlReference | InvalidSqlReferenceCall, int] | None = ( + _parse_reference_at(sql=sql, start=index, syntax=syntax) ) - if parsed_reference is None: + if parsed is None: index += 1 continue - references.append(parsed_reference[0]) - index = parsed_reference[1] - return tuple(references) + if isinstance(parsed[0], InvalidSqlReferenceCall): + invalid_calls.append(parsed[0]) + else: + references.append(parsed[0]) + index = parsed[1] + return SqlReferenceScan(references=tuple(references), invalid_calls=tuple(invalid_calls)) def _next_reference_scan_position(*, sql: str, start: int) -> int: @@ -141,7 +266,7 @@ def _next_reference_scan_position(*, sql: str, start: int) -> int: def _parse_reference_at( *, sql: str, start: int, syntax: SqlLexicalSyntax -) -> tuple[CompileSqlReference, int] | None: +) -> tuple[CompileSqlReference | InvalidSqlReferenceCall, int] | None: ref_kind: SqlReferenceKind | None = None prefix: str for prefix, candidate_kind in _REFERENCE_PREFIXES: @@ -156,33 +281,51 @@ def _parse_reference_at( sql=sql, open_paren_index=open_paren_index, context=_CONTEXT, syntax=syntax ) raw_arguments: str = sql[open_paren_index + 1 : closing_paren_index] - argument_values: tuple[str, ...] = _split_top_level_arguments( - raw_arguments=raw_arguments, syntax=syntax - ) - two_argument_count: int = 2 - if ref_kind == SqlReferenceKind.DBT_REF: - if len(argument_values) not in {1, two_argument_count}: - raise CompileInputError( - f"{ref_prefix(ref_kind)} must contain one name argument or package/name arguments" - ) - if len(argument_values) == two_argument_count: + call: str = sql[start : closing_paren_index + 1] + argument_match: re.Match[str] | None = ( + _DBT_REF_ARGUMENTS_PATTERN + if ref_kind == SqlReferenceKind.DBT_REF + else _NAME_ARGUMENT_PATTERN + ).fullmatch(raw_arguments) + if argument_match is None: + return ( + _invalid_reference_arguments( + ref_kind=ref_kind, + call=call, + start=start, + raw_arguments=raw_arguments, + syntax=syntax, + ), + closing_paren_index + 1, + ) + if ref_kind == SqlReferenceKind.DBT_REF and argument_match.group(2) is not None: + return ( + CompileSqlReference( + ref_kind=ref_kind, + ref_package=argument_match.group(1), + ref_name=argument_match.group(2), + ), + closing_paren_index + 1, + ) + call_argument_count: int | None = None + if ref_kind == SqlReferenceKind.TABLE_FUNCTION: + call_suffix_start: int = _skip_whitespace(sql=sql, start=closing_paren_index + 1) + if call_suffix_start >= len(sql) or sql[call_suffix_start] != SQL_OPEN_PAREN_TOKEN: + corrected_call: str = ref_kind.example_call(argument_match.group(1), quote='"') return ( - CompileSqlReference( + InvalidSqlReferenceCall( ref_kind=ref_kind, - ref_package=_parse_reference_name( - raw_value=argument_values[0], ref_kind=ref_kind + call=call, + start=start, + message=f"{ref_prefix(ref_kind)} must be followed by an argument list", + help=( + "pass the function arguments in a second set of parentheses, using () " + f"for no arguments: {corrected_call}()" ), - ref_name=_parse_reference_name(raw_value=argument_values[1], ref_kind=ref_kind), + corrected_call=f"{corrected_call}()", ), closing_paren_index + 1, ) - elif len(argument_values) != 1: - raise CompileInputError(f"{ref_prefix(ref_kind)} must contain exactly one name argument") - call_argument_count: int | None = None - if ref_kind == SqlReferenceKind.TABLE_FUNCTION: - call_suffix_start: int = _skip_whitespace(sql=sql, start=closing_paren_index + 1) - if call_suffix_start >= len(sql) or sql[call_suffix_start] != SQL_OPEN_PAREN_TOKEN: - raise CompileInputError(f"{ref_prefix(ref_kind)} must be followed by an argument list") call_suffix_end: int = find_matching_paren( sql=sql, open_paren_index=call_suffix_start, @@ -197,13 +340,81 @@ def _parse_reference_at( return ( CompileSqlReference( ref_kind=ref_kind, - ref_name=_parse_reference_name(raw_value=argument_values[0], ref_kind=ref_kind), + ref_name=argument_match.group(1), call_argument_count=call_argument_count, ), closing_paren_index + 1, ) +def _invalid_reference_arguments( + *, + ref_kind: SqlReferenceKind, + call: str, + start: int, + raw_arguments: str, + syntax: SqlLexicalSyntax, +) -> InvalidSqlReferenceCall: + """Describe a reference call that later compile stages could not replace.""" + + names: tuple[str, ...] | None = _authored_reference_names( + raw_arguments=raw_arguments, syntax=syntax + ) + allowed_counts: frozenset[int] = ( + frozenset({1, _DBT_REF_PACKAGE_ARGUMENT_COUNT}) + if ref_kind == SqlReferenceKind.DBT_REF + else frozenset({1}) + ) + if names is None or len(names) not in allowed_counts: + names = _PLACEHOLDER_NAMES[ref_kind] + corrected_call: str = ref_kind.example_call(*names, quote='"') + if ref_kind == SqlReferenceKind.TABLE_FUNCTION: + corrected_call += "(...)" + accepted: str = ( + "one double-quoted model name, or a double-quoted package name and model name " + "separated by a comma" + if ref_kind == SqlReferenceKind.DBT_REF + else "exactly one double-quoted name" + ) + return InvalidSqlReferenceCall( + ref_kind=ref_kind, + call=call, + start=start, + message=f"{' '.join(call.split())} is not a valid {ref_prefix(ref_kind)}() call", + help=( + f"{ref_prefix(ref_kind)}() takes {accepted}, with no comments or extra spaces " + f"inside the parentheses: {corrected_call}" + ), + corrected_call=corrected_call, + ) + + +def _authored_reference_names( + *, raw_arguments: str, syntax: SqlLexicalSyntax +) -> tuple[str, ...] | None: + """Return the names the author meant, when each argument is a plain name or string.""" + + try: + arguments: tuple[str, ...] = _split_top_level_arguments( + raw_arguments=raw_arguments, syntax=syntax + ) + except CompileInputError: + return None + names: list[str] = [] + for argument in arguments: + name: str = argument + if ( + len(argument) >= _PAIRED_QUOTE_CHARACTER_COUNT + and argument[0] == argument[-1] + and argument[0] in SQL_REFERENCE_NAME_QUOTE_TOKENS + ): + name = argument[1:-1] + if _AUTHORED_NAME_PATTERN.fullmatch(name) is None: + return None + names.append(name) + return tuple(names) + + def ref_prefix(ref_kind: SqlReferenceKind | str) -> str: normalized_ref_kind: SqlReferenceKind = SqlReferenceKind(ref_kind) return _REFERENCE_PREFIX_BY_KIND[normalized_ref_kind] @@ -257,27 +468,3 @@ def _split_top_level_arguments(*, raw_arguments: str, syntax: SqlLexicalSyntax) elif saw_separator: raise CompileInputError(f"{_CONTEXT} contains an empty argument") return tuple(arguments) - - -def _parse_reference_name(*, raw_value: str, ref_kind: SqlReferenceKind) -> str: - stripped_value: str = raw_value.strip() - if ref_kind == SqlReferenceKind.TABLE_FUNCTION and not ( - len(stripped_value) >= _PAIRED_QUOTE_CHARACTER_COUNT - and stripped_value[0] == stripped_value[-1] == _DOUBLE_QUOTE_TOKEN - ): - raise CompileInputError(f"{ref_prefix(ref_kind)} name argument must be double quoted") - if ( - len(stripped_value) >= _PAIRED_QUOTE_CHARACTER_COUNT - and stripped_value[0] == stripped_value[-1] - and stripped_value[0] in SQL_REFERENCE_NAME_QUOTE_TOKENS - ): - return stripped_value[1:-1] - if ( - stripped_value - and stripped_value.replace("_", "a").isalnum() - and stripped_value[0].isalpha() - ): - return stripped_value - raise CompileInputError( - f"{ref_prefix(ref_kind)} name argument must be a quoted string or identifier" - ) diff --git a/src/sqlbuild/compiler/compile/_helpers/render/macros.py b/src/sqlbuild/compiler/compile/_helpers/render/macros.py index 9228c39ac3..b71c5ef5a6 100644 --- a/src/sqlbuild/compiler/compile/_helpers/render/macros.py +++ b/src/sqlbuild/compiler/compile/_helpers/render/macros.py @@ -23,6 +23,7 @@ reject_macro_generated_references, relation_placeholder_text, render_relation_placeholders, + report_macro_reference_call_syntax, ) from sqlbuild.compiler.compile.constants import ( DECLARATION_REFERENCE_NAMES, @@ -1527,6 +1528,12 @@ def _evaluate_macro_call( _validate_final_macro_sql( macro_name=macro_name, file_path=file_path, macro_result=macro_result ) + report_macro_reference_call_syntax( + loaded_macro=loaded_macro, + macro_result=macro_result, + file_path=file_path, + consumer=state.consumer, + ) if state.macro_context._enforce_explicit_references: reject_macro_generated_references( loaded_macro=loaded_macro, diff --git a/src/sqlbuild/compiler/compile/_helpers/sql_tests/core.py b/src/sqlbuild/compiler/compile/_helpers/sql_tests/core.py index df50e613eb..3673fdaace 100644 --- a/src/sqlbuild/compiler/compile/_helpers/sql_tests/core.py +++ b/src/sqlbuild/compiler/compile/_helpers/sql_tests/core.py @@ -5,6 +5,7 @@ from collections.abc import Callable from dataclasses import dataclass from functools import lru_cache +from pathlib import Path from typing import Any from sqlbuild.compiler.compile._helpers.analysis.ctes import ( @@ -51,6 +52,7 @@ CompileSqlScenarioCte, CompileSqlTestCte, CompileSqlTestCtes, + SqlReferenceOrigin, ) from sqlbuild.compiler.compile.types import SqlTestMode from sqlbuild.compiler.discovery.main.omitted_ceremonial_select import ( @@ -209,7 +211,10 @@ def extract_sql_test_expected_model_names( def extract_assertion_target_model_names( - *, assertion_sql: tuple[str, ...], syntax: SqlLexicalSyntax + *, + assertion_sql: tuple[str, ...], + syntax: SqlLexicalSyntax, + origin: SqlReferenceOrigin | None = None, ) -> tuple[str, ...]: """Extract assertion model targets in authored order using canonical references.""" @@ -217,7 +222,7 @@ def extract_assertion_target_model_names( for sql in assertion_sql: targets.extend( reference.ref_name - for reference in extract_sql_references(sql=sql, syntax=syntax) + for reference in extract_sql_references(sql=sql, syntax=syntax, origin=origin) if reference.ref_kind == SqlReferenceKind.REF ) return tuple(dict.fromkeys(targets)) @@ -700,7 +705,13 @@ def _validate_no_direct_logic_calls( raise CompileInputError( f"SQL test '{file_label}' mode '{mode.value}' {cte_label} must not call macros" ) - references: tuple[CompileSqlReference, ...] = extract_sql_references(sql=sql, syntax=syntax) + references: tuple[CompileSqlReference, ...] = extract_sql_references( + sql=sql, + syntax=syntax, + origin=SqlReferenceOrigin( + file_path=Path(file_label), relative_path=Path(file_label), contents="" + ), + ) reference: CompileSqlReference | None = next( ( item diff --git a/src/sqlbuild/compiler/compile/constants.py b/src/sqlbuild/compiler/compile/constants.py index 0c61e28222..e4b9e90ee8 100644 --- a/src/sqlbuild/compiler/compile/constants.py +++ b/src/sqlbuild/compiler/compile/constants.py @@ -21,6 +21,7 @@ UNNEEDED_SQL_ANALYSIS_OPT_OUT_CODE: str = "P009" MISSING_DESCRIPTION_CODE: str = "P010" CURSOR_MODEL_WITHOUT_INPUTS_CODE: str = "P011" +REFERENCE_CALL_SYNTAX_CODE: str = "P012" DESCRIPTION_REQUIRED_INPUT_KINDS: dict[str, str] = { "model_files": "model", "scenario_files": "scenario", diff --git a/src/sqlbuild/compiler/compile/models.py b/src/sqlbuild/compiler/compile/models.py index 88b2dd7b71..db17965a78 100644 --- a/src/sqlbuild/compiler/compile/models.py +++ b/src/sqlbuild/compiler/compile/models.py @@ -516,6 +516,37 @@ def __post_init__(self) -> None: object.__setattr__(self, "ref_kind", SqlReferenceKind(self.ref_kind)) +@dataclass(frozen=True) +class InvalidSqlReferenceCall: + """One reference call whose arguments compile could not replace with a relation.""" + + ref_kind: SqlReferenceKind + call: str + start: int + message: str + help: str + corrected_call: str + + +@dataclass(frozen=True) +class SqlReferenceScan: + """Valid references and rejected reference calls found in one SQL text.""" + + references: tuple[CompileSqlReference, ...] + invalid_calls: tuple[InvalidSqlReferenceCall, ...] = () + + +@dataclass(frozen=True) +class SqlReferenceOrigin: + """The authored file that SQL handed to reference extraction came from.""" + + file_path: Path + relative_path: Path + contents: str + resource_type: CompiledResourceType | None = None + resource_name: str | None = None + + @dataclass(frozen=True) class CompiledLineageSourceFact: """Compact upstream column fact extracted during SQL analysis.""" diff --git a/src/sqlbuild/compiler/compile/types.py b/src/sqlbuild/compiler/compile/types.py index e049aee727..d3faf5c129 100644 --- a/src/sqlbuild/compiler/compile/types.py +++ b/src/sqlbuild/compiler/compile/types.py @@ -11,6 +11,8 @@ from sqlbuild.compiler.compile.models import ( CompactBatchPreparation, CompileProjectInputs, + SqlReferenceOrigin, + SqlReferenceScan, StoredModelAnalysis, ) from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax @@ -37,6 +39,12 @@ class CompactBatchResponseCallback(Protocol): def __call__(self, *, preparation: CompactBatchPreparation, response: object) -> None: ... +class SqlReferenceExtractor(Protocol): + """Keyword-only reference extraction for one expanded SQL text.""" + + def __call__(self, *, sql: str, origin: SqlReferenceOrigin | None) -> SqlReferenceScan: ... + + class CompileInputsReadyCallback(Protocol): """Invocation-local observer for the completed pre-semantic input snapshot.""" diff --git a/src/sqlbuild/compiler/planner/_helpers/fixtures/completion.py b/src/sqlbuild/compiler/planner/_helpers/fixtures/completion.py index bdd11253a8..3d859ba3a7 100644 --- a/src/sqlbuild/compiler/planner/_helpers/fixtures/completion.py +++ b/src/sqlbuild/compiler/planner/_helpers/fixtures/completion.py @@ -42,7 +42,7 @@ from sqlbuild.compiler.sql_analysis.main.import_polyglot_sql import import_polyglot_sql _PARTIAL_FIXTURE_ALIAS: str = "__sqlbuild_partial_fixture" -_DBT_REF_PATTERN: re.Pattern[str] = re.compile(r'__dbt_ref\("([^"]+)",\s*"([^"]+)"\)') +_DBT_REF_PATTERN: re.Pattern[str] = re.compile(r'__dbt_ref\("([^"]+)"\s*,\s*"([^"]+)"\)') def build_relation_fixture_completion( diff --git a/src/sqlbuild/compiler/planner/_helpers/scenario/relations.py b/src/sqlbuild/compiler/planner/_helpers/scenario/relations.py index 2961e816cd..818139b9a6 100644 --- a/src/sqlbuild/compiler/planner/_helpers/scenario/relations.py +++ b/src/sqlbuild/compiler/planner/_helpers/scenario/relations.py @@ -81,22 +81,19 @@ from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from sqlbuild.spec.contracts.models import SourceEntry +_NAME_ARGUMENT_PATTERN_TEXT: str = r'"(?P[^"]+)"\)' _REF_PATTERN: re.Pattern[str] = re.compile( - rf"{reference_call_prefix_pattern_text(SqlReferenceKind.REF)}\s*" - r"[\"']?(?P[A-Za-z_][A-Za-z0-9_.]*)[\"']?\s*\)" + reference_call_prefix_pattern_text(SqlReferenceKind.REF) + _NAME_ARGUMENT_PATTERN_TEXT ) _SEED_PATTERN: re.Pattern[str] = re.compile( - rf"{reference_call_prefix_pattern_text(SqlReferenceKind.SEED)}\s*" - r"[\"']?(?P[A-Za-z_][A-Za-z0-9_.]*)[\"']?\s*\)" + reference_call_prefix_pattern_text(SqlReferenceKind.SEED) + _NAME_ARGUMENT_PATTERN_TEXT ) _SOURCE_PATTERN: re.Pattern[str] = re.compile( - rf"{reference_call_prefix_pattern_text(SqlReferenceKind.SOURCE)}\s*" - r"[\"']?(?P[A-Za-z_][A-Za-z0-9_.]*)[\"']?\s*\)" + reference_call_prefix_pattern_text(SqlReferenceKind.SOURCE) + _NAME_ARGUMENT_PATTERN_TEXT ) _DBT_REF_PATTERN: re.Pattern[str] = re.compile( - rf'{reference_call_prefix_pattern_text(SqlReferenceKind.DBT_REF)}\s*["\']' - r'(?P[A-Za-z_][A-Za-z0-9_]*)["\']\s*' - r'(?:,\s*["\'](?P[A-Za-z_][A-Za-z0-9_]*)["\']\s*)?\)' + reference_call_prefix_pattern_text(SqlReferenceKind.DBT_REF) + + r'"(?P[^"]+)"(?:\s*,\s*"(?P[^"]+)")?\)' ) diff --git a/src/sqlbuild/integrations/dbt/_helpers/manifest/compile_refs.py b/src/sqlbuild/integrations/dbt/_helpers/manifest/compile_refs.py index f01974bac9..0e13550edd 100644 --- a/src/sqlbuild/integrations/dbt/_helpers/manifest/compile_refs.py +++ b/src/sqlbuild/integrations/dbt/_helpers/manifest/compile_refs.py @@ -73,7 +73,7 @@ def validate_compile_dbt_model_reference( if dbt_manifest is None: raise CompileInputError( f"Model file {model_relative_path} uses " - f"{SqlReferenceKind.DBT_REF.example_call(reference.ref_name)} but no dbt " + f"{SqlReferenceKind.DBT_REF.example_call(reference.ref_name, quote='"')} but no dbt " "manifest was found", code="C214", help=( diff --git a/src/sqlbuild/integrations/dbt/_helpers/manifest/core.py b/src/sqlbuild/integrations/dbt/_helpers/manifest/core.py index 2ebc9f03e6..e0bde28e89 100644 --- a/src/sqlbuild/integrations/dbt/_helpers/manifest/core.py +++ b/src/sqlbuild/integrations/dbt/_helpers/manifest/core.py @@ -160,7 +160,7 @@ def resolve_dbt_manifest_model( f"dbt model '{name}' is ambiguous across packages: {packages}", code="C213", help=( - f"Use {SqlReferenceKind.DBT_REF.example_call('package_name', name)} to " + f"Use {SqlReferenceKind.DBT_REF.example_call('package_name', name, quote='"')} to " "choose one dbt model." ), ) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py index 5d40696aa0..98d12b46fa 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/_test_types.py @@ -853,3 +853,23 @@ class NativeModelConfigParityTestCase: engines: tuple[str, str] expected_exit_codes: tuple[int, int] expected_report_text: str + + +@dataclass(frozen=True) +class ReplaceableReferenceCallTestCase: + """A compiler engine that must build double-quoted reference calls end to end.""" + + description: str + engine: str + expected_total_cents: str + + +@dataclass(frozen=True) +class RejectedReferenceCallTestCase: + """A reference call form compile cannot replace and the error it must report first.""" + + description: str + engine: str + staging_from: str + mart_from: str + expected_diagnostics: tuple[tuple[str, str, str, int, int, str], ...] diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/helpers.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/helpers.py index a784a02d06..7fb69439ac 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/helpers.py @@ -4215,3 +4215,51 @@ def store_digests(*, project_dir: Path, stores: tuple[str, ...]) -> dict[str, st path.relative_to(project_dir).as_posix(): hashlib.sha256(path.read_bytes()).hexdigest() for path in files } + + +_REFERENCE_CALL_PROJECT_TOML: str = ( + 'name = "orders"\nadapter = "duckdb"\n\n[connection]\ndatabase = "warehouse.duckdb"\n' +) +_REFERENCE_CALL_SOURCES_YML: str = ( + "sources:\n" + " - name: raw_orders\n" + " description: Raw orders.\n" + " schema: main\n" + " table: raw_orders\n" + " columns:\n" + " - name: order_id\n" + " type: INTEGER\n" + " - name: amount_cents\n" + " type: INTEGER\n" +) +_REFERENCE_CALL_RAW_ORDERS_SQL: str = ( + "CREATE TABLE main.raw_orders AS " + "SELECT * FROM (VALUES (1, 1200), (2, 1650)) AS t(order_id, amount_cents)" +) + + +def prepare_reference_call_project(*, tmp_path: Path, staging_from: str, mart_from: str) -> Path: + """Write an orders project whose models use the given reference calls, with raw data.""" + + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="orders", + repo_files={ + "sqlbuild_project.toml": _REFERENCE_CALL_PROJECT_TOML, + "sources/raw.yml": _REFERENCE_CALL_SOURCES_YML, + "models/staging/stg_orders.sql": ( + 'MODEL (description "Staged orders.", materialized view);\n\n' + f"SELECT order_id, amount_cents FROM {staging_from}\n" + ), + "models/marts/order_totals.sql": ( + 'MODEL (description "Order totals.", materialized table);\n\n' + "SELECT COUNT(*) AS order_count, SUM(amount_cents) AS total_cents\n" + f"FROM {mart_from} AS orders\n" + ), + }, + ) + seeded: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("query", _REFERENCE_CALL_RAW_ORDERS_SQL) + ) + assert seeded.returncode == 0, seeded.stdout + seeded.stderr + return project_dir diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_rule_gate.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_rule_gate.py index 969fdb1ae3..9880520e71 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_rule_gate.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_compile_rule_gate.py @@ -18,7 +18,7 @@ from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project, run_sqb _MISSING_MOCK_MESSAGE: str = ( - "test 'orders_case': model 'orders' references __source('raw_refunds') which has no mock" + "test 'orders_case': model 'orders' references __source(\"raw_refunds\") which has no mock" ) _EXPECTED_DIAGNOSTICS: tuple[tuple[str, str, str], ...] = ( ("rule", "SQBRMODEL102", "SELECT * is allowed only inside dependency import CTEs"), diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_reference_call_syntax.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_reference_call_syntax.py new file mode 100644 index 0000000000..d61d49961a --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_reference_call_syntax.py @@ -0,0 +1,247 @@ +"""E2E tests for reference calls that compile could never replace with a relation.""" + +from __future__ import annotations + +import json +import subprocess +from pathlib import Path +from typing import cast + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.compile._test_types import ( + RejectedReferenceCallTestCase, + ReplaceableReferenceCallTestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.compile.helpers import ( + prepare_reference_call_project, +) +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import run_sqb + +_ENGINE_ENV_VAR: str = "SQLBUILD_COMPILER_ENGINE" +_CANONICAL_SOURCE: str = '__source("raw_orders")' +_CANONICAL_REF: str = '__ref("stg_orders")' + + +@pytest.mark.parametrize( + "test_case", + [ + ReplaceableReferenceCallTestCase( + description="python engine builds double quoted references", + engine="python", + expected_total_cents="2850", + ), + ReplaceableReferenceCallTestCase( + description="native engine builds double quoted references", + engine="native", + expected_total_cents="2850", + ), + ], + ids=lambda case: case.description, +) +def test_given_double_quoted_reference_calls_when_building_then_relations_resolve( + test_case: ReplaceableReferenceCallTestCase, tmp_path: Path +) -> None: + project_dir: Path = prepare_reference_call_project( + tmp_path=tmp_path, staging_from=_CANONICAL_SOURCE, mart_from=_CANONICAL_REF + ) + + built: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("build",), env={_ENGINE_ENV_VAR: test_case.engine} + ) + totals: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, + command=("query", "SELECT total_cents FROM main.order_totals", "--json"), + ) + + assert built.returncode == 0, built.stdout + built.stderr + assert totals.returncode == 0, totals.stdout + totals.stderr + assert test_case.expected_total_cents in totals.stdout + + +@pytest.mark.parametrize( + "test_case", + [ + RejectedReferenceCallTestCase( + description="python engine rejects unquoted ref name", + engine="python", + staging_from=_CANONICAL_SOURCE, + mart_from="__ref(stg_orders)", + expected_diagnostics=( + ( + "P012", + "__ref(stg_orders) is not a valid __ref() call", + "models/marts/order_totals.sql", + 4, + 6, + '__ref() takes exactly one double-quoted name, with no comments or extra spaces inside the parentheses: __ref("stg_orders")', + ), + ), + ), + RejectedReferenceCallTestCase( + description="python engine rejects comment inside ref call", + engine="python", + staging_from=_CANONICAL_SOURCE, + mart_from="__ref( /* upstream */ 'stg_orders')", + expected_diagnostics=( + ( + "P012", + "__ref( /* upstream */ 'stg_orders') is not a valid __ref() call", + "models/marts/order_totals.sql", + 4, + 6, + '__ref() takes exactly one double-quoted name, with no comments or extra spaces inside the parentheses: __ref("stg_orders")', + ), + ), + ), + RejectedReferenceCallTestCase( + description="python engine rejects single quoted source name", + engine="python", + staging_from="__source('raw_orders')", + mart_from=_CANONICAL_REF, + expected_diagnostics=( + ( + "P012", + "__source('raw_orders') is not a valid __source() call", + "models/staging/stg_orders.sql", + 3, + 36, + '__source() takes exactly one double-quoted name, with no comments or extra spaces inside the parentheses: __source("raw_orders")', + ), + ), + ), + RejectedReferenceCallTestCase( + description="python engine rejects invalid calls in two files", + engine="python", + staging_from="__source('raw_orders')", + mart_from="__ref(stg_orders)", + expected_diagnostics=( + ( + "P012", + "__ref(stg_orders) is not a valid __ref() call", + "models/marts/order_totals.sql", + 4, + 6, + '__ref() takes exactly one double-quoted name, with no comments or extra spaces inside the parentheses: __ref("stg_orders")', + ), + ( + "P012", + "__source('raw_orders') is not a valid __source() call", + "models/staging/stg_orders.sql", + 3, + 36, + '__source() takes exactly one double-quoted name, with no comments or extra spaces inside the parentheses: __source("raw_orders")', + ), + ), + ), + RejectedReferenceCallTestCase( + description="native engine rejects unquoted ref name", + engine="native", + staging_from=_CANONICAL_SOURCE, + mart_from="__ref(stg_orders)", + expected_diagnostics=( + ( + "P012", + "__ref(stg_orders) is not a valid __ref() call", + "models/marts/order_totals.sql", + 4, + 6, + '__ref() takes exactly one double-quoted name, with no comments or extra spaces inside the parentheses: __ref("stg_orders")', + ), + ), + ), + RejectedReferenceCallTestCase( + description="native engine rejects comment inside ref call", + engine="native", + staging_from=_CANONICAL_SOURCE, + mart_from="__ref( /* upstream */ 'stg_orders')", + expected_diagnostics=( + ( + "P012", + "__ref( /* upstream */ 'stg_orders') is not a valid __ref() call", + "models/marts/order_totals.sql", + 4, + 6, + '__ref() takes exactly one double-quoted name, with no comments or extra spaces inside the parentheses: __ref("stg_orders")', + ), + ), + ), + RejectedReferenceCallTestCase( + description="native engine rejects single quoted source name", + engine="native", + staging_from="__source('raw_orders')", + mart_from=_CANONICAL_REF, + expected_diagnostics=( + ( + "P012", + "__source('raw_orders') is not a valid __source() call", + "models/staging/stg_orders.sql", + 3, + 36, + '__source() takes exactly one double-quoted name, with no comments or extra spaces inside the parentheses: __source("raw_orders")', + ), + ), + ), + RejectedReferenceCallTestCase( + description="native engine rejects invalid calls in two files", + engine="native", + staging_from="__source('raw_orders')", + mart_from="__ref(stg_orders)", + expected_diagnostics=( + ( + "P012", + "__ref(stg_orders) is not a valid __ref() call", + "models/marts/order_totals.sql", + 4, + 6, + '__ref() takes exactly one double-quoted name, with no comments or extra spaces inside the parentheses: __ref("stg_orders")', + ), + ( + "P012", + "__source('raw_orders') is not a valid __source() call", + "models/staging/stg_orders.sql", + 3, + 36, + '__source() takes exactly one double-quoted name, with no comments or extra spaces inside the parentheses: __source("raw_orders")', + ), + ), + ), + ], + ids=lambda case: case.description, +) +def test_given_reference_call_compile_cannot_replace_when_building_then_fails_with_corrected_call( + test_case: RejectedReferenceCallTestCase, tmp_path: Path +) -> None: + project_dir: Path = prepare_reference_call_project( + tmp_path=tmp_path, staging_from=test_case.staging_from, mart_from=test_case.mart_from + ) + + compiled: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, + command=("compile", "--json"), + env={_ENGINE_ENV_VAR: test_case.engine}, + ) + built: subprocess.CompletedProcess[str] = run_sqb( + project_dir=project_dir, command=("build",), env={_ENGINE_ENV_VAR: test_case.engine} + ) + + diagnostics: list[dict[str, object]] = cast( + list[dict[str, object]], json.loads(compiled.stdout)["diagnostics"] + ) + assert compiled.returncode == 1, compiled.stdout + compiled.stderr + assert built.returncode == 1, built.stdout + built.stderr + assert ( + tuple( + ( + item["code"], + item["message"], + item["path"], + item["line"], + item["column"], + item["help"], + ) + for item in diagnostics + ) + == test_case.expected_diagnostics + ) + assert "error[P012]" in built.stdout + built.stderr diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_test.py b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_test.py index 4b394c821e..e03de5d349 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/test/test_test.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/test/test_test.py @@ -1055,7 +1055,7 @@ def test_given_shared_upstream_graph_when_running_test_then_outcomes_and_sql_mat layers=12, expected_error=( "test 'deep_orders_missing_mock': model 'orders_00_left' references " - "__source('raw_orders') which has no mock" + '__source("raw_orders") which has no mock' ), expected_stdout_fragments=(), ), @@ -1165,10 +1165,10 @@ def test_given_expected_cte_column_subset_when_testing_then_only_listed_columns_ MissingMockCliE2ETestCase( description="missing source mock fails test and compile until mocked", expected_error=( - "test 'test_chain': model 'stg_orders' references __source('raw') which has no mock" + "test 'test_chain': model 'stg_orders' references __source(\"raw\") which has no mock" ), compile_error_line=( - "error[S000]: test 'test_chain': model 'stg_orders' references __source('raw') " + "error[S000]: test 'test_chain': model 'stg_orders' references __source(\"raw\") " "which has no mock" ), compile_summary_fragment="1 error, 0 warnings", diff --git a/tests/integration/src/sqlbuild/compiler/compile/_helpers/refs/_test_types.py b/tests/integration/src/sqlbuild/compiler/compile/_helpers/refs/_test_types.py index 5c07c1fd34..c0c70c7230 100644 --- a/tests/integration/src/sqlbuild/compiler/compile/_helpers/refs/_test_types.py +++ b/tests/integration/src/sqlbuild/compiler/compile/_helpers/refs/_test_types.py @@ -25,6 +25,7 @@ class GeneratedReferenceParityTestCase: expected_minimum_extracted: int expected_minimum_failed: int expected_minimum_table_functions: int + expected_minimum_rejected: int expected_maximum_deferred: int diff --git a/tests/integration/src/sqlbuild/compiler/compile/_helpers/refs/helpers.py b/tests/integration/src/sqlbuild/compiler/compile/_helpers/refs/helpers.py index b17ec2488a..15a36f2bee 100644 --- a/tests/integration/src/sqlbuild/compiler/compile/_helpers/refs/helpers.py +++ b/tests/integration/src/sqlbuild/compiler/compile/_helpers/refs/helpers.py @@ -15,10 +15,10 @@ from sqlbuild.adapters.sqlserver.classes.sqlserver_adapter import SqlServerAdapter from sqlbuild.compiler.compile._helpers.refs.native import extract_native_sql_references from sqlbuild.compiler.compile._helpers.refs.references import ( - _extract_sql_references_with_python, + _scan_sql_references_with_python, ) from sqlbuild.compiler.compile.exceptions import CompileInputError -from sqlbuild.compiler.compile.models import CompileSqlReference +from sqlbuild.compiler.compile.models import CompileSqlReference, SqlReferenceScan from sqlbuild.compiler.references.types import SqlReferenceKind from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from tests.integration.src.sqlbuild.compiler.helpers import mismatches @@ -33,6 +33,12 @@ "sqlserver": SqlServerAdapter.sql_lexical_syntax, } _NAMES: tuple[str, ...] = ( + '"orders"', + '"customers"', + '"order items"', + '"größe"', +) +_REJECTED_NAMES: tuple[str, ...] = ( "'customers'", "products", " inventory\t", @@ -40,13 +46,10 @@ "orders\x1c", "orders /* note */", "-- note\norders", -) -_QUOTED_NAMES: tuple[str, ...] = ( - '"orders"', + ' "orders" ', ' "order""s" ', '"a" "b"', '""', - '"größe"', '/* note */ "orders"', ) _MALFORMED_NAMES: tuple[str, ...] = ( @@ -65,7 +68,7 @@ "orders", ) _NO_SECOND_ARGUMENT: tuple[str, ...] = ("",) -_DBT_SECOND_ARGUMENTS: tuple[str, ...] = ("", ", products", ' , "orders"') +_DBT_SECOND_ARGUMENTS: tuple[str, ...] = ("", ', "products"', ' ,\x1c"orders"', ", products") _MALFORMED_SECOND_ARGUMENTS: tuple[str, ...] = (",", ", ", ",, products", ", products", ' ,"a"') _NO_CALL_SUFFIX: tuple[str, ...] = ("",) _CALL_SUFFIXES: tuple[str, ...] = ( @@ -101,6 +104,8 @@ "'it''s'", "a$$b", "é", + "/* unclosed", + "'unclosed", ) _RISKY_FILLERS: tuple[str, ...] = ( "# __ref(hidden)\n", @@ -127,15 +132,15 @@ class _CallShape: call_suffixes: tuple[str, ...] -_ANY_NAMES: tuple[str, ...] = (*_NAMES, *_QUOTED_NAMES) +_ANY_NAMES: tuple[str, ...] = (*(_NAMES * 9), *_REJECTED_NAMES) _CALL_SHAPES: tuple[_CallShape, ...] = ( _CallShape("__ref(", _ANY_NAMES, _NO_SECOND_ARGUMENT, _NO_CALL_SUFFIX), _CallShape("__source(", _ANY_NAMES, _NO_SECOND_ARGUMENT, _NO_CALL_SUFFIX), _CallShape("__seed(", _ANY_NAMES, _NO_SECOND_ARGUMENT, _NO_CALL_SUFFIX), _CallShape("__udf(", _ANY_NAMES, _NO_SECOND_ARGUMENT, ("(x)",)), _CallShape("__dbt_ref(", _ANY_NAMES, _DBT_SECOND_ARGUMENTS, _NO_CALL_SUFFIX), - _CallShape("__table_fn(", _QUOTED_NAMES, _NO_SECOND_ARGUMENT, _CALL_SUFFIXES), - _CallShape("__table_fn(", _QUOTED_NAMES, _NO_SECOND_ARGUMENT, _CALL_SUFFIXES), + _CallShape("__table_fn(", _ANY_NAMES, _NO_SECOND_ARGUMENT, _CALL_SUFFIXES), + _CallShape("__table_fn(", _NAMES, _NO_SECOND_ARGUMENT, _CALL_SUFFIXES), _CallShape("___ref(", _ANY_NAMES, _NO_SECOND_ARGUMENT, _NO_CALL_SUFFIX), _CallShape("x__ref(", _ANY_NAMES, _NO_SECOND_ARGUMENT, _NO_CALL_SUFFIX), ) @@ -153,6 +158,7 @@ class ReferenceParity: extracted: int failed: int deferred: int + rejected: int table_functions: int @@ -180,25 +186,31 @@ def _generated_reference_call(*, rng: random.Random) -> str: ) -def python_outcome(*, sql: str, syntax: SqlLexicalSyntax) -> tuple[CompileSqlReference, ...] | str: - """Return the oracle scanner's references or the message of the error it raises.""" +def python_outcome( + *, sql: str, syntax: SqlLexicalSyntax +) -> tuple[CompileSqlReference, ...] | str | None: + """Return the oracle's references, its error message, or None when it rejects a call.""" try: - return _extract_sql_references_with_python(sql=sql, syntax=syntax) + scan: SqlReferenceScan = _scan_sql_references_with_python(sql=sql, syntax=syntax) except CompileInputError as error: return str(error) + return (scan.references, None)[bool(scan.invalid_calls)] def reference_parity(*, sqls: list[str], syntax: SqlLexicalSyntax) -> ReferenceParity: - """Compare native extraction with Python wherever the native scan does not defer.""" + """Compare native extraction with Python; native must defer every SQL Python rejects.""" native: list[tuple[CompileSqlReference, ...] | str | None] = [ extract_native_sql_references(sql=sql, syntax=syntax) for sql in sqls ] - python: list[tuple[CompileSqlReference, ...] | str] = [ + python: list[tuple[CompileSqlReference, ...] | str | None] = [ python_outcome(sql=sql, syntax=syntax) for sql in sqls ] - scanned: list[bool] = [outcome is not None for outcome in native] + scanned: list[bool] = [ + native_outcome is not None or python_result is None + for native_outcome, python_result in zip(native, python, strict=True) + ] extracted: list[tuple[CompileSqlReference, ...]] = cast( list[tuple[CompileSqlReference, ...]], list(compress(native, [isinstance(outcome, tuple) for outcome in native])), @@ -211,7 +223,8 @@ def reference_parity(*, sqls: list[str], syntax: SqlLexicalSyntax) -> ReferenceP ), extracted=len(extracted), failed=sum(isinstance(outcome, str) for outcome in native), - deferred=scanned.count(False), + deferred=native.count(None) - python.count(None), + rejected=python.count(None), table_functions=sum( reference.ref_kind is SqlReferenceKind.TABLE_FUNCTION for reference in chain.from_iterable(extracted) diff --git a/tests/integration/src/sqlbuild/compiler/compile/_helpers/refs/test_native_references.py b/tests/integration/src/sqlbuild/compiler/compile/_helpers/refs/test_native_references.py index 76ae127c5e..e2940592d7 100644 --- a/tests/integration/src/sqlbuild/compiler/compile/_helpers/refs/test_native_references.py +++ b/tests/integration/src/sqlbuild/compiler/compile/_helpers/refs/test_native_references.py @@ -39,32 +39,42 @@ ] = ( ( "table_function", - 'SELECT * FROM __table_fn("orders_for")(1, 2) JOIN __ref(customers)', + 'SELECT * FROM __table_fn("orders_for")(1, 2) JOIN __ref("customers")', (("table_fn", "orders_for", None, 2), ("ref", "customers", None, None)), "", ), ( - "argument_count_error", + "rejected_call", "SELECT * FROM __ref(orders, customers)", (), - "__ref must contain exactly one name argument", + "__ref(orders, customers) is not a valid __ref() call", + ), + ( + "unclosed_call", + 'SELECT * FROM __ref("orders"', + (), + "SQL reference contains an unclosed parenthesis", ), ) _MANY_CALLS: str = "SELECT 1 FROM " + " JOIN ".join( - f"__table_fn(\"orders_for\")({index}, 'a,b', (1, 2)) JOIN __ref(orders_{index})" + f'__table_fn("orders_for")({index}, \'a,b\', (1, 2)) JOIN __ref("orders_{index}")' for index in range(5_000) ) _WIDE_CALL: str = ( '__table_fn("orders_for")(' + ", ".join(f"(c{index} /* , */)" for index in range(100_000)) + ")" ) _NESTED_CALLS: str = '__table_fn("orders_for")(' * 1_000 + "1" + ")" * 1_000 -_OUTCOME_SUMMARIES: dict[type, Callable[[Any], int | str]] = {tuple: len, str: str} -_ERROR_AFTER_MANY: str = ( - "SELECT 1 FROM " - + " JOIN ".join(f"__ref(orders_{index})" for index in range(50_000)) - + " JOIN __ref(orders, customers)" +_OUTCOME_SUMMARIES: dict[type, Callable[[Any], int | str]] = { + tuple: len, + str: str, + type(None): repr, +} +_MANY_REFERENCES: str = "SELECT 1 FROM " + " JOIN ".join( + f'__ref("orders_{index}")' for index in range(50_000) ) +_ERROR_AFTER_MANY: str = _MANY_REFERENCES + ' JOIN __ref("orders"' +_REJECTED_AFTER_MANY: str = _MANY_REFERENCES + " JOIN __ref(orders, customers)" @pytest.mark.parametrize( @@ -73,9 +83,9 @@ CraftedReferenceParityTestCase( description="every_kind_in_authored_order", sql=( - "SELECT * FROM __ref(orders) JOIN __source('raw_orders') " - "JOIN __seed(\"regions\") JOIN __dbt_ref('shop', customers) " - "WHERE __udf(clean)(x) > 0" + 'SELECT * FROM __ref("orders") JOIN __source("raw_orders") ' + 'JOIN __seed("regions") JOIN __dbt_ref("shop" ,\x1c"customers") ' + 'WHERE __udf("clean")(x) > 0 OR __dbt_ref("orders") IS NULL' ), ), CraftedReferenceParityTestCase( @@ -93,51 +103,77 @@ description="hidden_in_comments_strings_and_dollar_quotes", sql=( "-- __ref(a)\nSELECT '__ref(b)', $$__table_fn(\"c\")$$, $t$ __ref(d) $t$, " - '/* __ref(e) */ "__ref(f)", `__ref(g)` FROM __ref(h)' + '/* __ref(e) */ "__ref(f)", `__ref(g)` FROM __ref("h")' ), ), CraftedReferenceParityTestCase( description="dialect_sensitive_text", sql=( "SELECT 'it\\'s __ref(a)', E'x\\'__ref(b)', r'C:\\' AS p, '''__ref(c)''', " - "/* x /* y */ __ref(d) */ 1 # __ref(e)\n// __ref(f)\nFROM __ref(g)" + '/* x /* y */ __ref(d) */ 1 # __ref(e)\n// __ref(f)\nFROM __ref("g")' ), + expected_deferred=True, ), CraftedReferenceParityTestCase( - description="comments_inside_quoted_names", + description="comments_inside_calls_are_rejected", sql='SELECT * FROM __ref("a" /* note */ "b") JOIN __ref(-- c\n"orders")', + expected_deferred=True, ), CraftedReferenceParityTestCase( description="non_ascii_quoted_names_and_text", sql="SELECT 'é' FROM __ref(\"commandés\") JOIN __table_fn(\"größe\")('ü')", ), CraftedReferenceParityTestCase( - description="wrong_reference_argument_count", + description="wrong_reference_argument_count_is_rejected", sql="SELECT * FROM __ref(orders, customers)", + expected_deferred=True, + ), + CraftedReferenceParityTestCase( + description="unquoted_and_single_quoted_names_are_rejected", + sql="SELECT * FROM __ref(orders) JOIN __source('raw_orders')", + expected_deferred=True, + ), + CraftedReferenceParityTestCase( + description="spaces_inside_calls_are_rejected", + sql='SELECT * FROM __seed( "regions" ) JOIN __dbt_ref("shop", "orders" )', + expected_deferred=True, + ), + CraftedReferenceParityTestCase( + description="unclosed_text_after_a_rejected_call_is_rejected", + sql="SELECT * FROM __ref(orders) WHERE note = 'unclosed", + expected_deferred=True, ), CraftedReferenceParityTestCase( - description="wrong_dbt_reference_argument_count", + description="unclosed_text_before_a_rejected_call_fails", + sql='SELECT * FROM __ref("orders" WHERE __ref(customers)', + ), + CraftedReferenceParityTestCase( + description="wrong_dbt_reference_argument_count_is_rejected", sql="SELECT * FROM __dbt_ref(shop, orders, extra)", + expected_deferred=True, ), CraftedReferenceParityTestCase( description="empty_table_function_call_argument", sql='SELECT * FROM __table_fn("orders_for")(1,,2)', ), CraftedReferenceParityTestCase( - description="table_function_without_call", + description="table_function_without_call_is_rejected", sql='SELECT * FROM __table_fn("orders_for") /* gap */ (1)', + expected_deferred=True, ), CraftedReferenceParityTestCase( - description="table_function_name_checked_after_call", + description="unquoted_table_function_name_is_rejected", sql="SELECT * FROM __table_fn(orders_for)(1)", + expected_deferred=True, ), CraftedReferenceParityTestCase( description="unclosed_table_function_call", sql='SELECT * FROM __table_fn("orders_for")(1', ), CraftedReferenceParityTestCase( - description="nested_reference_as_name", + description="nested_reference_as_name_is_rejected", sql="SELECT * FROM __ref(__ref(orders))", + expected_deferred=True, ), CraftedReferenceParityTestCase( description="non_ascii_identifier_defers", @@ -162,7 +198,7 @@ def test_given_crafted_reference_sql_when_extracting_natively_then_matches_pytho ] assert dialect_mismatches == [[]] * len(LEXICAL_SYNTAXES) - assert parity.deferred == int(test_case.expected_deferred) + assert parity.deferred + parity.rejected == int(test_case.expected_deferred) @pytest.mark.parametrize( @@ -173,10 +209,11 @@ def test_given_crafted_reference_sql_when_extracting_natively_then_matches_pytho syntax=syntax, seed=20261007 + offset, count=3000, - expected_minimum_extracted=1500, + expected_minimum_extracted=700, expected_minimum_failed=500, - expected_minimum_table_functions=500, - expected_maximum_deferred=200, + expected_minimum_table_functions=300, + expected_minimum_rejected=1000, + expected_maximum_deferred=400, ) for offset, syntax in enumerate(LEXICAL_SYNTAXES) ], @@ -196,8 +233,9 @@ def test_given_generated_reference_sql_when_extracting_natively_then_matches_pyt parity.extracted >= test_case.expected_minimum_extracted, parity.failed >= test_case.expected_minimum_failed, parity.table_functions >= test_case.expected_minimum_table_functions, + parity.rejected >= test_case.expected_minimum_rejected, parity.deferred <= test_case.expected_maximum_deferred, - ) == ([], True, True, True, True), (parity.extracted, parity.failed, parity.deferred) + ) == ([], True, True, True, True, True), parity @pytest.mark.parametrize( @@ -256,15 +294,21 @@ def counting_extract( [ ReferenceMismatchTestCase( description="python_extracts_references", + sql='SELECT * FROM __ref("orders")', + native_message="SQL reference contains an empty argument", + expected_python_outcome="found 1 reference(s) and 0 rejected call(s)", + ), + ReferenceMismatchTestCase( + description="python_rejects_a_call", sql="SELECT * FROM __ref(orders)", - native_message="__ref must contain exactly one name argument", - expected_python_outcome="found 1 reference(s)", + native_message="SQL reference contains an empty argument", + expected_python_outcome="found 0 reference(s) and 1 rejected call(s)", ), ReferenceMismatchTestCase( description="python_raises_a_different_error", - sql="SELECT * FROM __ref(orders, customers)", + sql='SELECT * FROM __ref("orders"', native_message="SQL reference contains an empty argument", - expected_python_outcome="raised '__ref must contain exactly one name argument'", + expected_python_outcome="raised 'SQL reference contains an unclosed parenthesis'", ), ], ids=lambda case: case.description, @@ -311,7 +355,13 @@ def failing_extract(*, sql: str, syntax: SqlLexicalSyntax) -> str: ReferenceScanBoundTestCase( description="error_after_fifty_thousand_references", sql=_ERROR_AFTER_MANY, - expected_outcome="__ref must contain exactly one name argument", + expected_outcome="SQL reference contains an unclosed parenthesis", + expected_maximum_seconds=0.5, + ), + ReferenceScanBoundTestCase( + description="rejected_call_after_fifty_thousand_references", + sql=_REJECTED_AFTER_MANY, + expected_outcome="None", expected_maximum_seconds=0.5, ), ], diff --git a/tests/unit/src/sqlbuild/cli/commands/main/compile/test_compile_command.py b/tests/unit/src/sqlbuild/cli/commands/main/compile/test_compile_command.py index 394d401383..d2cf82f76f 100644 --- a/tests/unit/src/sqlbuild/cli/commands/main/compile/test_compile_command.py +++ b/tests/unit/src/sqlbuild/cli/commands/main/compile/test_compile_command.py @@ -553,7 +553,7 @@ def test_given_lineage_disabled_when_building_compile_lineage_then_skips_analyze min_prepared_models=128, test_header='name "orders_case"', expected_codes=("SQBRMODEL102", "S000"), - expected_message_fragment="references __source('raw_refunds') which has no mock", + expected_message_fragment='references __source("raw_refunds") which has no mock', expected_inline_planning_calls=0, ), RuleGatedTestPlanningTestCase( @@ -561,7 +561,7 @@ def test_given_lineage_disabled_when_building_compile_lineage_then_skips_analyze min_prepared_models=1, test_header='name "orders_case"', expected_codes=("SQBRMODEL102", "S000"), - expected_message_fragment="references __source('raw_refunds') which has no mock", + expected_message_fragment='references __source("raw_refunds") which has no mock', expected_inline_planning_calls=0, ), RuleGatedTestPlanningTestCase( diff --git a/tests/unit/src/sqlbuild/cli/commands/main/compile/test_compile_target_writer.py b/tests/unit/src/sqlbuild/cli/commands/main/compile/test_compile_target_writer.py index 3c75541eb4..5ce8ab56c7 100644 --- a/tests/unit/src/sqlbuild/cli/commands/main/compile/test_compile_target_writer.py +++ b/tests/unit/src/sqlbuild/cli/commands/main/compile/test_compile_target_writer.py @@ -347,7 +347,9 @@ def test_given_unchanged_test_artifact_when_writing_again_then_skips_test_plan_r TargetWriterPlanningErrorTestCase( description="missing mock is reported on every write", expected_builder_calls=1, - expected_message=("model 'orders' references __source('raw_orders') which has no mock"), + expected_message=( + "model 'orders' references __source(\"raw_orders\") which has no mock" + ), ), ), ids=lambda case: case.description, diff --git a/tests/unit/src/sqlbuild/compiler/compile/_helpers/_test_types.py b/tests/unit/src/sqlbuild/compiler/compile/_helpers/_test_types.py index 5bfd49aedc..3416f6b2c9 100644 --- a/tests/unit/src/sqlbuild/compiler/compile/_helpers/_test_types.py +++ b/tests/unit/src/sqlbuild/compiler/compile/_helpers/_test_types.py @@ -1,5 +1,6 @@ from collections.abc import Callable from dataclasses import dataclass, field +from pathlib import Path from sqlbuild.adapter.contract.models import ExpressionInferenceProfile from sqlbuild.compiler.compile.models import ( @@ -19,7 +20,7 @@ from sqlbuild.compiler.references.types import SqlReferenceKind from sqlbuild.compiler.scopes.models import UsageRecord from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax -from sqlbuild.spec.contracts.models import ResolvedTimeTravelRetention +from sqlbuild.spec.contracts.models import ResolvedTimeTravelRetention, SourceLocation from sqlbuild.sql_values.models import SqlValue from tests.unit.src.sqlbuild.compiler.compile._helpers.helpers import RequiredDescriptionInputs @@ -59,6 +60,8 @@ class SqlReferenceExtractionErrorTestCase: description: str sql: str expected_error: str + expected_code: str = "P001" + expected_help: str | None = None @dataclass(frozen=True) @@ -1155,3 +1158,11 @@ class CursorModelWithoutInputsCase: reference_kinds: tuple[str, ...] expected_codes: tuple[str, ...] expected_locations: tuple[tuple[str, int, int, int], ...] = () + + +@dataclass(frozen=True) +class CollectedReferenceCallSyntaxTestCase: + description: str + contents: str + sql: str + expected_diagnostics: tuple[tuple[str, str, Path, SourceLocation | None], ...] diff --git a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_analysis_cache.py b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_analysis_cache.py index ea1e0d943c..5c0cec1641 100644 --- a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_analysis_cache.py +++ b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_analysis_cache.py @@ -137,7 +137,7 @@ def test_given_successful_analysis_when_compiling_again_then_reuses_identical_ca reference_scanner: Mock = Mock( side_effect=AssertionError("SQL references must not be scanned on a cache hit") ) - monkeypatch.setattr(reference_cache, "extract_sql_references", reference_scanner) + monkeypatch.setattr(reference_cache, "scan_sql_reference_calls", reference_scanner) warm_project: CompiledProject = compile_project_with_cache(project_dir=tmp_path) @@ -385,7 +385,7 @@ def test_given_corrupt_reference_cache_when_compiling_then_rescans_and_repairs_t write_repo_files(tmp_path, _CACHE_REPO_FILES) cold_project: CompiledProject = compile_project_with_cache(project_dir=tmp_path) cache_path: Path = next( - (compiler_cache_directory(tmp_path) / "references-v2").glob("*.sqlite3") + (compiler_cache_directory(tmp_path) / "references-v3").glob("*.sqlite3") ) with sqlite3.connect(cache_path) as connection: persisted_contents: str = connection.execute( @@ -397,8 +397,8 @@ def test_given_corrupt_reference_cache_when_compiling_then_rescans_and_repairs_t "UPDATE sql_reference SET payload = ?", (corrupt_contents,), ) - scanner: Mock = Mock(wraps=reference_cache.extract_sql_references) - monkeypatch.setattr(reference_cache, "extract_sql_references", scanner) + scanner: Mock = Mock(wraps=reference_cache.scan_sql_reference_calls) + monkeypatch.setattr(reference_cache, "scan_sql_reference_calls", scanner) repaired_project: CompiledProject = compile_project_with_cache(project_dir=tmp_path) @@ -425,15 +425,15 @@ def test_given_non_text_reference_cache_when_compiling_then_rescans_safely( write_repo_files(tmp_path, _CACHE_REPO_FILES) _ = compile_project_with_cache(project_dir=tmp_path) cache_path: Path = next( - (compiler_cache_directory(tmp_path) / "references-v2").glob("*.sqlite3") + (compiler_cache_directory(tmp_path) / "references-v3").glob("*.sqlite3") ) with sqlite3.connect(cache_path) as connection: _ = connection.execute( "UPDATE sql_reference SET payload = ?", (sqlite3.Binary(b"broken"),), ) - scanner: Mock = Mock(wraps=reference_cache.extract_sql_references) - monkeypatch.setattr(reference_cache, "extract_sql_references", scanner) + scanner: Mock = Mock(wraps=reference_cache.scan_sql_reference_calls) + monkeypatch.setattr(reference_cache, "scan_sql_reference_calls", scanner) _ = compile_project_with_cache(project_dir=tmp_path) diff --git a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_assembly.py b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_assembly.py index d0e4494d48..f3e0e0a350 100644 --- a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_assembly.py +++ b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_assembly.py @@ -65,7 +65,7 @@ audits [not_null (column order_id, always_run true)], ); -select * from __source('raw_orders') +select * from __source("raw_orders") """.strip() + "\n", "sources/raw.yml": """ @@ -87,7 +87,7 @@ + "\n", "seeds/country_codes.csv": "code\nUS\n", "models/staging/_sqlbuild/_audits/generic/not_null.sql": "AUDIT ();\n\n" - "SELECT @column FROM __ref('@model') WHERE @column IS NULL\n", + 'SELECT @column FROM __ref("@model") WHERE @column IS NULL\n', "tests/unit/orders_test.sql": """ TEST (); 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 a61c8f3a6d..713af6c8e3 100644 --- a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_scenarios.py +++ b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_scenarios.py @@ -86,7 +86,7 @@ __seed__waffle_types AS (SELECT 1 AS waffle_type_id), __expected__daily_revenue AS (SELECT 1 AS order_id), __assert__no_negative_revenue AS ( - SELECT * FROM __ref(daily_revenue) WHERE revenue < 0 + SELECT * FROM __ref("daily_revenue") WHERE revenue < 0 ) SELECT 1 """.strip(), @@ -108,7 +108,7 @@ WITH __source__raw__orders AS (SELECT 1 AS order_id), __assert__has_no_null_orders AS ( - SELECT * FROM __ref(fact_orders) WHERE order_id IS NULL + SELECT * FROM __ref("fact_orders") WHERE order_id IS NULL ) SELECT 1 """.strip(), @@ -265,7 +265,7 @@ def test_given_sql_scenario_cte_variants_when_extracting_then_it_returns_expecte sql=""" WITH __source__raw__orders AS (SELECT 1 AS order_id), - __assert__ AS (SELECT * FROM __ref(fact_orders)) + __assert__ AS (SELECT * FROM __ref("fact_orders")) SELECT 1 """.strip(), expected_error_fragment="__assert__", diff --git a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_references.py b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_references.py index 4148fbc475..8dd6bb50a1 100644 --- a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_references.py +++ b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_references.py @@ -2,6 +2,9 @@ from __future__ import annotations +import re +from pathlib import Path + import pytest from sqlbuild.adapters.bigquery.classes.bigquery_adapter import BigQueryAdapter @@ -9,18 +12,23 @@ from sqlbuild.adapters.duckdb.classes.duckdb_adapter import DuckDbAdapter from sqlbuild.adapters.postgres.classes.postgres_adapter import PostgresAdapter from sqlbuild.adapters.snowflake.classes.snowflake_adapter import SnowflakeAdapter +from sqlbuild.compiler.compile._helpers.diagnostics.collector import collect_compile_diagnostics from sqlbuild.compiler.compile._helpers.refs.references import extract_sql_references from sqlbuild.compiler.compile.exceptions import CompileInputError -from sqlbuild.compiler.compile.models import CompileSqlReference +from sqlbuild.compiler.compile.models import CompileSqlReference, SqlReferenceOrigin +from sqlbuild.compiler.compile.types import CompiledResourceType from sqlbuild.compiler.references.types import SqlReferenceKind from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax +from sqlbuild.spec.contracts.models import SourceLocation from tests.unit.src.sqlbuild.compiler.compile._helpers._test_types import ( + CollectedReferenceCallSyntaxTestCase, DialectSqlScanTestCase, SqlReferenceExtractionErrorTestCase, SqlReferenceExtractionTestCase, ) _GENERIC_SQL_SYNTAX: SqlLexicalSyntax = SqlLexicalSyntax() +_ORIGIN_PATH: Path = Path("models/marts/order_totals.sql") @pytest.mark.parametrize( @@ -29,8 +37,8 @@ SqlReferenceExtractionTestCase( description="simple references preserve authored order", sql=( - "SELECT * FROM __source('orders') " - 'UNION ALL SELECT * FROM __dbt_ref("shop", "customers")' + 'SELECT * FROM __source("orders") ' + 'UNION ALL SELECT * FROM __dbt_ref("shop" , "customers")' ), expected_references=( (SqlReferenceKind.SOURCE, "orders", None), @@ -81,11 +89,6 @@ def test_given_simple_references_when_extracting_then_returns_authored_order( sql='SELECT * FROM __ref("orders"', expected_error="unclosed parenthesis", ), - SqlReferenceExtractionErrorTestCase( - description="expression reference name preserves diagnostic", - sql="SELECT * FROM __ref(concat('ord', 'ers'))", - expected_error="name argument must be a quoted string or identifier", - ), SqlReferenceExtractionErrorTestCase( description="unclosed block comment preserves diagnostic", sql='SELECT * FROM __ref("orders") /* unterminated', @@ -111,6 +114,181 @@ def test_given_unsupported_reference_sql_when_extracting_then_preserves_python_d extract_sql_references(sql=test_case.sql, syntax=_GENERIC_SQL_SYNTAX) +@pytest.mark.parametrize( + "test_case", + [ + SqlReferenceExtractionErrorTestCase( + description="unquoted ref name", + sql="SELECT * FROM __ref(stg_orders)", + expected_error=re.escape("__ref(stg_orders) is not a valid __ref() call"), + expected_code="P012", + expected_help='__ref("stg_orders")', + ), + SqlReferenceExtractionErrorTestCase( + description="comment inside ref call", + sql="SELECT * FROM __ref( /* upstream */ 'stg_orders')", + expected_error=re.escape("is not a valid __ref() call"), + expected_code="P012", + expected_help='__ref("stg_orders")', + ), + SqlReferenceExtractionErrorTestCase( + description="single quoted source name", + sql="SELECT * FROM __source('raw_orders')", + expected_error=re.escape("__source('raw_orders') is not a valid __source() call"), + expected_code="P012", + expected_help='__source("raw_orders")', + ), + SqlReferenceExtractionErrorTestCase( + description="whitespace inside seed call", + sql='SELECT * FROM __seed( "country_codes" )', + expected_error=re.escape("is not a valid __seed() call"), + expected_code="P012", + expected_help='__seed("country_codes")', + ), + SqlReferenceExtractionErrorTestCase( + description="unquoted udf name", + sql="SELECT __udf(is_completed)(status) FROM orders", + expected_error=re.escape("__udf(is_completed) is not a valid __udf() call"), + expected_code="P012", + expected_help='__udf("is_completed")', + ), + SqlReferenceExtractionErrorTestCase( + description="single quoted table function name", + sql="SELECT * FROM __table_fn('customer_orders')(1)", + expected_error=re.escape("is not a valid __table_fn() call"), + expected_code="P012", + expected_help='__table_fn("customer_orders")(...)', + ), + SqlReferenceExtractionErrorTestCase( + description="unquoted dbt ref package and name", + sql="SELECT * FROM __dbt_ref(shop, customers)", + expected_error=re.escape("is not a valid __dbt_ref() call"), + expected_code="P012", + expected_help='__dbt_ref("shop", "customers")', + ), + SqlReferenceExtractionErrorTestCase( + description="expression name falls back to a placeholder call", + sql="SELECT * FROM __ref(concat('ord', 'ers'))", + expected_error=re.escape("is not a valid __ref() call"), + expected_code="P012", + expected_help='__ref("model_name")', + ), + SqlReferenceExtractionErrorTestCase( + description="second ref name argument", + sql='SELECT * FROM __ref("orders", "customers")', + expected_error=re.escape("is not a valid __ref() call"), + expected_code="P012", + expected_help='__ref("model_name")', + ), + SqlReferenceExtractionErrorTestCase( + description="empty quoted name", + sql='SELECT * FROM __ref("")', + expected_error=re.escape("is not a valid __ref() call"), + expected_code="P012", + expected_help='__ref("model_name")', + ), + SqlReferenceExtractionErrorTestCase( + description="table function without argument list", + sql='SELECT * FROM __table_fn("customer_orders")', + expected_error="must be followed by an argument list", + expected_code="P012", + expected_help='__table_fn("customer_orders")()', + ), + ], + ids=lambda case: case.description, +) +def test_given_reference_call_compile_cannot_replace_when_extracting_then_raises_syntax_error( + test_case: SqlReferenceExtractionErrorTestCase, +) -> None: + with pytest.raises(CompileInputError, match=test_case.expected_error) as raised: + extract_sql_references(sql=test_case.sql, syntax=_GENERIC_SQL_SYNTAX) + + assert raised.value.code == test_case.expected_code + assert raised.value.help is not None + assert test_case.expected_help is not None + assert raised.value.help.endswith(test_case.expected_help) + + +@pytest.mark.parametrize( + "test_case", + [ + CollectedReferenceCallSyntaxTestCase( + description="every rejected call is reported at its authored location", + contents=( + 'MODEL (description "Order totals.");\n\n' + "SELECT * FROM __ref(stg_orders) o\n" + "JOIN __source('raw_payments') p USING (order_id)\n" + ), + sql=( + "SELECT * FROM __ref(stg_orders) o\n" + "JOIN __source('raw_payments') p USING (order_id)\n" + ), + expected_diagnostics=( + ( + "P012", + "__ref(stg_orders) is not a valid __ref() call", + _ORIGIN_PATH, + SourceLocation(path=_ORIGIN_PATH, line=3, column=15, end_line=3, end_column=32), + ), + ( + "P012", + "__source('raw_payments') is not a valid __source() call", + _ORIGIN_PATH, + SourceLocation(path=_ORIGIN_PATH, line=4, column=6, end_line=4, end_column=30), + ), + ), + ), + CollectedReferenceCallSyntaxTestCase( + description="a commented-out rejected call does not take the authored location", + contents=("-- was __ref(stg_orders)\nSELECT * FROM __ref(stg_orders)\n"), + sql="SELECT * FROM __ref(stg_orders)\n", + expected_diagnostics=( + ( + "P012", + "__ref(stg_orders) is not a valid __ref() call", + _ORIGIN_PATH, + SourceLocation(path=_ORIGIN_PATH, line=2, column=15, end_line=2, end_column=32), + ), + ), + ), + CollectedReferenceCallSyntaxTestCase( + description="a call missing from the authored text keeps the file path", + contents="SELECT * FROM @staged_orders()\n", + sql="SELECT * FROM __ref(stg_orders)\n", + expected_diagnostics=( + ("P012", "__ref(stg_orders) is not a valid __ref() call", _ORIGIN_PATH, None), + ), + ), + ], + ids=lambda case: case.description, +) +def test_given_rejected_calls_when_collecting_diagnostics_then_reports_each_with_location( + test_case: CollectedReferenceCallSyntaxTestCase, +) -> None: + origin: SqlReferenceOrigin = SqlReferenceOrigin( + file_path=Path("/project/models/marts/order_totals.sql"), + relative_path=_ORIGIN_PATH, + contents=test_case.contents, + resource_type=CompiledResourceType.MODEL, + resource_name="order_totals", + ) + + with collect_compile_diagnostics() as collected: + references: tuple[CompileSqlReference, ...] = extract_sql_references( + sql=test_case.sql, syntax=_GENERIC_SQL_SYNTAX, origin=origin + ) + + assert references == () + assert ( + tuple( + (diagnostic.code, diagnostic.message, diagnostic.path, diagnostic.location) + for diagnostic in collected.diagnostics + ) + == test_case.expected_diagnostics + ) + assert {diagnostic.resource_name for diagnostic in collected.diagnostics} == {"order_totals"} + + @pytest.mark.parametrize( "test_case", [ diff --git a/tests/unit/src/sqlbuild/compiler/compile/_test_types.py b/tests/unit/src/sqlbuild/compiler/compile/_test_types.py index c527c52cd5..f1952b41bd 100644 --- a/tests/unit/src/sqlbuild/compiler/compile/_test_types.py +++ b/tests/unit/src/sqlbuild/compiler/compile/_test_types.py @@ -113,6 +113,13 @@ class BuildCompileInputsErrorTestCase: environment_variables: dict[str, str] = field(default_factory=dict) +@dataclass(frozen=True) +class BuildCompileInputsDiagnosticTestCase: + description: str + repo_files: dict[str, str] + expected_diagnostics: tuple[tuple[str, str, int | None, int | None], ...] + + @dataclass(frozen=True) class BuildCompileInputsPythonHookValidationTestCase: description: str diff --git a/tests/unit/src/sqlbuild/compiler/compile/test_main.py b/tests/unit/src/sqlbuild/compiler/compile/test_main.py index b78a2202ab..99ebb91d9f 100644 --- a/tests/unit/src/sqlbuild/compiler/compile/test_main.py +++ b/tests/unit/src/sqlbuild/compiler/compile/test_main.py @@ -53,6 +53,7 @@ ) from tests.unit.src.sqlbuild.compiler.compile._test_types import ( AuthoredRetentionCompileTestCase, + BuildCompileInputsDiagnosticTestCase, BuildCompileInputsErrorTestCase, BuildCompileInputsPythonHookValidationTestCase, BuildCompileInputsTestCase, @@ -3243,6 +3244,77 @@ def test_given_project_and_local_environment_when_resolving_then_local_values_ov ) +@pytest.mark.parametrize( + "test_case", + [ + BuildCompileInputsDiagnosticTestCase( + description="reports a table function call without its argument list", + repo_files=base_repo_files() + | { + "sqlbuild_project.toml": ( + 'name = "demo"\nadapter = "duckdb"\n\n[settings]\nsql_validation = false\n' + ), + "models/orders.sql": ( + 'MODEL (description "Test model orders.");\n\n' + 'SELECT * FROM __table_fn("customer_orders")\n' + ), + "functions/sql/customer_orders.sql": ( + 'FUNCTION (description "Test function customer_orders.",\n' + " arguments (customer_id INTEGER),\n" + " returns table (order_id INTEGER)\n" + ");\n\n" + "SELECT customer_id AS order_id\n" + ), + }, + expected_diagnostics=( + ("P012", "__table_fn must be followed by an argument list", 3, 15), + ), + ), + BuildCompileInputsDiagnosticTestCase( + description="reports a table function name that is not double quoted", + repo_files=base_repo_files() + | { + "sqlbuild_project.toml": ( + 'name = "demo"\nadapter = "duckdb"\n\n[settings]\nsql_validation = false\n' + ), + "models/orders.sql": ( + "MODEL (description 'Test model orders.');\n\n" + "SELECT * FROM __table_fn('customer_orders')(1)\n" + ), + }, + expected_diagnostics=( + ( + "P012", + "__table_fn('customer_orders') is not a valid __table_fn() call", + 3, + 15, + ), + ), + ), + ], + ids=lambda case: case.description, +) +def test_given_rejected_reference_calls_when_building_compile_inputs_then_reports_located_diagnostics( + test_case: BuildCompileInputsDiagnosticTestCase, + tmp_path: Path, + write_repo_files: Callable[[Path, dict[str, str]], None], +) -> None: + write_repo_files(tmp_path, test_case.repo_files) + + compile_inputs: CompileProjectInputs = build_compile_inputs( + discovered_inputs=discover_project_inputs(project_dir=tmp_path), + adapter_context=DUCKDB_COMPILE_ADAPTER_CONTEXT, + ) + + assert ( + tuple( + (diagnostic.code, diagnostic.message, diagnostic.line, diagnostic.column) + for diagnostic in compile_inputs.diagnostics + ) + == test_case.expected_diagnostics + ) + + @pytest.mark.parametrize( "test_case", [ @@ -3310,45 +3382,6 @@ def test_given_project_and_local_environment_when_resolving_then_local_values_ov run_id=None, expected_error_fragment="references scalar function 'order_total' with __table_fn(...)", ), - BuildCompileInputsErrorTestCase( - description="raises when a model omits table function call arguments", - repo_files=base_repo_files() - | { - "sqlbuild_project.toml": ( - 'name = "demo"\nadapter = "duckdb"\n\n[settings]\nsql_validation = false\n' - ), - "models/orders.sql": ( - 'MODEL (description "Test model orders.");\n\nSELECT * FROM __table_fn("customer_orders")\n' - ), - "functions/sql/customer_orders.sql": """ -FUNCTION (description "Test function customer_orders.", - arguments (customer_id INTEGER), - returns table (order_id INTEGER) -); - -SELECT customer_id AS order_id -""".strip() - + "\n", - }, - selected_target=None, - run_id=None, - expected_error_fragment="must be followed by an argument list", - ), - BuildCompileInputsErrorTestCase( - description="raises when a table function name is not double quoted", - repo_files=base_repo_files() - | { - "sqlbuild_project.toml": ( - 'name = "demo"\nadapter = "duckdb"\n\n[settings]\nsql_validation = false\n' - ), - "models/orders.sql": ( - "MODEL (description 'Test model orders.');\n\nSELECT * FROM __table_fn('customer_orders')(1)\n" - ), - }, - selected_target=None, - run_id=None, - expected_error_fragment="name argument must be double quoted", - ), BuildCompileInputsErrorTestCase( description="raises when table function argument count does not match", repo_files=base_repo_files() @@ -3538,7 +3571,7 @@ def test_given_project_and_local_environment_when_resolving_then_local_values_ov selected_target=None, run_id=None, expected_error_fragment=( - r"Model file models/staging/orders\.sql uses __dbt_ref\('stg_orders'\) " + r'Model file models/staging/orders\.sql uses __dbt_ref\("stg_orders"\) ' "but no dbt manifest was found" ), ), @@ -3558,7 +3591,7 @@ def test_given_project_and_local_environment_when_resolving_then_local_values_ov selected_target=None, run_id=None, expected_error_fragment=( - r"SQL function file functions/sql/orders\.sql uses __dbt_ref\('stg_orders'\) " + r'SQL function file functions/sql/orders\.sql uses __dbt_ref\("stg_orders"\) ' "but dbt refs are not supported yet" ), ), @@ -3576,7 +3609,7 @@ def test_given_project_and_local_environment_when_resolving_then_local_values_ov selected_target=None, run_id=None, expected_error_fragment=( - r"Audit file audits/singular/orders\.sql may not use __dbt_ref\('stg_orders'\); " + r'Audit file audits/singular/orders\.sql may not use __dbt_ref\("stg_orders"\); ' "audit dbt model checks belong in dbt" ), ), diff --git a/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_sql_scenarios.py b/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_sql_scenarios.py index 4eb3533da6..886f0e293c 100644 --- a/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_sql_scenarios.py +++ b/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_sql_scenarios.py @@ -53,7 +53,7 @@ WITH __assert__daily_revenue_has_rows AS ( - SELECT * FROM __ref(daily_revenue) WHERE order_id IS NULL + SELECT * FROM __ref("daily_revenue") WHERE order_id IS NULL ) SELECT 1 """, @@ -62,7 +62,7 @@ expected_sql_body=( "WITH\n" "__assert__daily_revenue_has_rows AS (\n" - " SELECT * FROM __ref(daily_revenue) WHERE order_id IS NULL\n" + ' SELECT * FROM __ref("daily_revenue") WHERE order_id IS NULL\n' ")\n" "SELECT 1" ), diff --git a/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_graph.py b/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_graph.py index ef6c09daf3..b66b33bf92 100644 --- a/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_graph.py +++ b/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_graph.py @@ -53,7 +53,7 @@ model_deps={"daily_revenue": ("raw__orders",)}, source_names=("raw__orders",), seed_names=(), - assertion_sql_bodies=("SELECT * FROM __ref(daily_revenue) WHERE revenue < 0",), + assertion_sql_bodies=('SELECT * FROM __ref("daily_revenue") WHERE revenue < 0',), source_fixture_names=("raw__orders",), expected_plan=ScenarioGraphPlan( key=SCENARIO_KEY, @@ -71,7 +71,7 @@ seed_names=(), assertion_sql_bodies=( "SELECT '__ref(not_a_model)' AS marker_text " - "FROM __ref(daily_revenue) -- __ref(commented_model)", + 'FROM __ref("daily_revenue") -- __ref(commented_model)', ), source_fixture_names=("raw__orders",), expected_plan=ScenarioGraphPlan( @@ -92,7 +92,7 @@ source_names=("raw__orders",), seed_names=(), expected_model_names=("daily_revenue",), - assertion_sql_bodies=("SELECT * FROM __ref(customer_revenue)",), + assertion_sql_bodies=('SELECT * FROM __ref("customer_revenue")',), source_fixture_names=("raw__orders",), expected_plan=ScenarioGraphPlan( key=SCENARIO_KEY, @@ -212,7 +212,7 @@ def test_given_scenario_when_planning_graph_then_infers_expected_slice( model_deps={}, source_names=(), seed_names=(), - assertion_sql_bodies=("SELECT * FROM __ref(daily_revenue)",), + assertion_sql_bodies=('SELECT * FROM __ref("daily_revenue")',), expected_error_fragment="assertion references unknown model 'daily_revenue'", ), PlanScenarioGraphErrorTestCase( diff --git a/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_relations.py b/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_relations.py index b087bcff96..37192101cb 100644 --- a/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_relations.py +++ b/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_relations.py @@ -546,8 +546,8 @@ def test_given_required_unmocked_seed_when_building_execution_plan_then_loads_pr ScenarioCheckSqlResolutionTestCase( description="resolves assertion refs to scenario model and ref fixture relations", sql=( - "SELECT * FROM __ref(daily_revenue) dr " - "JOIN __ref(stg_customers) sc ON dr.customer_id = sc.customer_id" + 'SELECT * FROM __ref("daily_revenue") dr ' + 'JOIN __ref("stg_customers") sc ON dr.customer_id = sc.customer_id' ), expected_sql=( "SELECT * FROM scenario_schema.__sqb_51b385aebe20__model__daily_revenue dr " @@ -558,8 +558,8 @@ def test_given_required_unmocked_seed_when_building_execution_plan_then_loads_pr ScenarioCheckSqlResolutionTestCase( description="resolves seed and source markers to scenario fixture relations", sql=( - "SELECT * FROM __seed(country_codes) c " - "JOIN __source(raw__orders) o ON c.country_code = o.country_code" + 'SELECT * FROM __seed("country_codes") c ' + 'JOIN __source("raw__orders") o ON c.country_code = o.country_code' ), expected_sql=( "SELECT * FROM scenario_schema.__sqb_51b385aebe20__seed__country_codes c " @@ -577,20 +577,20 @@ def test_given_required_unmocked_seed_when_building_execution_plan_then_loads_pr ScenarioCheckSqlResolutionTestCase( description="check sql resolution keeps strings and comments as authored", sql=( - "SELECT '__ref(daily_revenue)' AS marker_text " - "FROM __ref(daily_revenue) dr -- __source(raw__orders)" + "SELECT '__ref(\"daily_revenue\")' AS marker_text " + 'FROM __ref("daily_revenue") dr -- __source("raw__orders")' ), expected_sql=( - "SELECT '__ref(daily_revenue)' AS marker_text " + "SELECT '__ref(\"daily_revenue\")' AS marker_text " "FROM scenario_schema.__sqb_51b385aebe20__model__daily_revenue dr " - "-- __source(raw__orders)" + '-- __source("raw__orders")' ), ), ScenarioCheckSqlResolutionTestCase( description="authored function spellings and keyword case are not regenerated", sql=( "select STARTSWITH(name, 'a') as flagged, IFNULL(total, 0) as total " - "from __ref(daily_revenue) where total != 0" + 'from __ref("daily_revenue") where total != 0' ), expected_sql=( "select STARTSWITH(name, 'a') as flagged, IFNULL(total, 0) as total " @@ -670,8 +670,8 @@ def test_given_scenario_check_sql_when_resolving_then_uses_scenario_relations( description="even backslash-escaped quotes around a marker", lexical_syntaxes=BACKSLASH_LEXICAL_SYNTAXES, sql=( - "select * from __ref(daily_revenue) where note = 'O\\'Brien' " - "union all select * from __ref(stg_customers) where note = 'D\\'Arcy'" + "select * from __ref(\"daily_revenue\") where note = 'O\\'Brien' " + "union all select * from __ref(\"stg_customers\") where note = 'D\\'Arcy'" ), expected_sql=( f"select * from {DAILY_REVENUE_TARGET} where note = 'O\\'Brien' " @@ -682,8 +682,8 @@ def test_given_scenario_check_sql_when_resolving_then_uses_scenario_relations( description="escape strings around a marker", lexical_syntaxes=ESCAPE_LEXICAL_SYNTAXES, sql=( - "select * from __ref(daily_revenue) where note = E'O\\'Brien' " - "union all select * from __ref(stg_customers) where note = e'D\\'Arcy'" + "select * from __ref(\"daily_revenue\") where note = E'O\\'Brien' " + "union all select * from __ref(\"stg_customers\") where note = e'D\\'Arcy'" ), expected_sql=( f"select * from {DAILY_REVENUE_TARGET} where note = E'O\\'Brien' " @@ -693,27 +693,27 @@ def test_given_scenario_check_sql_when_resolving_then_uses_scenario_relations( ScenarioDialectCheckSqlResolutionTestCase( description="dollar-quoted text hides quotes and markers", lexical_syntaxes=(SnowflakeAdapter.sql_lexical_syntax, *ESCAPE_LEXICAL_SYNTAXES), - sql="select $$O'Brien __ref(stg_customers)$$ as note from __ref(daily_revenue)", + sql='select $$O\'Brien __ref("stg_customers")$$ as note from __ref("daily_revenue")', expected_sql=( - f"select $$O'Brien __ref(stg_customers)$$ as note from {DAILY_REVENUE_TARGET}" + f'select $$O\'Brien __ref("stg_customers")$$ as note from {DAILY_REVENUE_TARGET}' ), ), ScenarioDialectCheckSqlResolutionTestCase( description="snowflake quoted identifier ending in a backslash", lexical_syntaxes=(SnowflakeAdapter.sql_lexical_syntax,), - sql='select "note\\", "it""s" from __ref(daily_revenue)', + sql='select "note\\", "it""s" from __ref("daily_revenue")', expected_sql=f'select "note\\", "it""s" from {DAILY_REVENUE_TARGET}', ), ScenarioDialectCheckSqlResolutionTestCase( description="bigquery backtick identifier with an escaped backtick", lexical_syntaxes=(BigQueryAdapter.sql_lexical_syntax,), - sql="select `note\\`s` from __ref(daily_revenue)", + sql='select `note\\`s` from __ref("daily_revenue")', expected_sql=f"select `note\\`s` from {DAILY_REVENUE_TARGET}", ), ScenarioDialectCheckSqlResolutionTestCase( description="bigquery triple-quoted string with an apostrophe", lexical_syntaxes=(BigQueryAdapter.sql_lexical_syntax,), - sql="select '''it's''' as note from __ref(daily_revenue)", + sql="select '''it's''' as note from __ref(\"daily_revenue\")", expected_sql=f"select '''it's''' as note from {DAILY_REVENUE_TARGET}", ), ScenarioDialectCheckSqlResolutionTestCase( @@ -724,27 +724,27 @@ def test_given_scenario_check_sql_when_resolving_then_uses_scenario_relations( DatabricksAdapter.sql_lexical_syntax, SqlServerAdapter.sql_lexical_syntax, ), - sql="select * /* a /* b */ it's __ref(stg_customers) */ from __ref(daily_revenue)", + sql='select * /* a /* b */ it\'s __ref("stg_customers") */ from __ref("daily_revenue")', expected_sql=( - f"select * /* a /* b */ it's __ref(stg_customers) */ from {DAILY_REVENUE_TARGET}" + f'select * /* a /* b */ it\'s __ref("stg_customers") */ from {DAILY_REVENUE_TARGET}' ), ), ScenarioDialectCheckSqlResolutionTestCase( description="bigquery hash line comment with an apostrophe", lexical_syntaxes=(BigQueryAdapter.sql_lexical_syntax,), - sql="select * # it's __ref(stg_customers)\nfrom __ref(daily_revenue)", - expected_sql=f"select * # it's __ref(stg_customers)\nfrom {DAILY_REVENUE_TARGET}", + sql='select * # it\'s __ref("stg_customers")\nfrom __ref("daily_revenue")', + expected_sql=f'select * # it\'s __ref("stg_customers")\nfrom {DAILY_REVENUE_TARGET}', ), ScenarioDialectCheckSqlResolutionTestCase( description="snowflake double slash line comment with an apostrophe", lexical_syntaxes=(SnowflakeAdapter.sql_lexical_syntax,), - sql="select * // it's __ref(stg_customers)\nfrom __ref(daily_revenue)", - expected_sql=f"select * // it's __ref(stg_customers)\nfrom {DAILY_REVENUE_TARGET}", + sql='select * // it\'s __ref("stg_customers")\nfrom __ref("daily_revenue")', + expected_sql=f'select * // it\'s __ref("stg_customers")\nfrom {DAILY_REVENUE_TARGET}', ), ScenarioDialectCheckSqlResolutionTestCase( description="databricks raw string ending in a backslash", lexical_syntaxes=(DatabricksAdapter.sql_lexical_syntax,), - sql="select r'C:\\' as path from __ref(daily_revenue)", + sql="select r'C:\\' as path from __ref(\"daily_revenue\")", expected_sql=f"select r'C:\\' as path from {DAILY_REVENUE_TARGET}", ), ], @@ -782,8 +782,8 @@ def test_given_dialect_lexical_syntax_when_resolving_check_sql_then_replaces_eve description="backslash before a quote is not an escape outside escape strings", lexical_syntaxes=ESCAPE_LEXICAL_SYNTAXES, sql=( - "select * from __ref(daily_revenue) where note = 'O\\'Brien' " - "union all select * from __ref(stg_customers)" + "select * from __ref(\"daily_revenue\") where note = 'O\\'Brien' " + 'union all select * from __ref("stg_customers")' ), expected_error_code="S511", expected_error_fragment="unclosed quoted string", @@ -791,7 +791,7 @@ def test_given_dialect_lexical_syntax_when_resolving_check_sql_then_replaces_eve ScenarioCheckSqlResolutionErrorTestCase( description="escaped closing quote leaves the string unclosed", lexical_syntaxes=BACKSLASH_LEXICAL_SYNTAXES, - sql="select * from __ref(daily_revenue) where note = 'O\\'", + sql="select * from __ref(\"daily_revenue\") where note = 'O\\'", expected_error_code="S511", expected_error_fragment="unclosed quoted string", ), @@ -801,16 +801,16 @@ def test_given_dialect_lexical_syntax_when_resolving_check_sql_then_replaces_eve SnowflakeAdapter.sql_lexical_syntax, BigQueryAdapter.sql_lexical_syntax, ), - sql="select * /* a /* b */ it's */ from __ref(daily_revenue)", + sql='select * /* a /* b */ it\'s */ from __ref("daily_revenue")', expected_error_code="S511", expected_error_fragment="unclosed quoted string", ), ScenarioCheckSqlResolutionErrorTestCase( description="marker naming a relation outside the scenario", lexical_syntaxes=(SnowflakeAdapter.sql_lexical_syntax,), - sql="select * from __ref(daily_revenue) join __ref(unknown_orders) using (id)", + sql='select * from __ref("daily_revenue") join __ref("unknown_orders") using (id)', expected_error_code="S511", - expected_error_fragment="references '__ref(unknown_orders)'", + expected_error_fragment="references '__ref(\"unknown_orders\")'", ), ], ids=lambda case: case.description, @@ -847,16 +847,16 @@ def test_given_unresolvable_check_sql_when_resolving_then_raises_scenario_error( ScenarioFixtureSqlResolutionErrorTestCase( description="unclosed string in duckdb fixture sql", lexical_syntax=DuckDbAdapter.sql_lexical_syntax, - fixture_sql="SELECT 'O\\'Brien' AS note FROM __source(raw__orders)", + fixture_sql="SELECT 'O\\'Brien' AS note FROM __source(\"raw__orders\")", expected_error_code="S511", expected_error_fragment="unclosed quoted string", ), ScenarioFixtureSqlResolutionErrorTestCase( description="fixture sql naming an unknown project source", lexical_syntax=SnowflakeAdapter.sql_lexical_syntax, - fixture_sql="SELECT * FROM __source(raw__returns)", + fixture_sql='SELECT * FROM __source("raw__returns")', expected_error_code="S511", - expected_error_fragment="references '__source(raw__returns)'", + expected_error_fragment="references '__source(\"raw__returns\")'", ), ], ids=lambda case: case.description, diff --git a/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_source_load_nodes.py b/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_source_load_nodes.py index 65b2c02a51..11ac5cbf33 100644 --- a/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_source_load_nodes.py +++ b/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_source_load_nodes.py @@ -176,7 +176,7 @@ def test_given_unselected_upstream_in_closure_when_filtering_metadata_then_uses_ deps=(upstream_key,), name="fact_orders", relative_path=Path("models/fact_orders.sql"), - query_sql="SELECT * FROM __ref('stg_orders')", + query_sql='SELECT * FROM __ref("stg_orders")', config=CompileModelConfig(), destination=CompiledRelationLocation( database="ANALYTICS_DB", diff --git a/tests/unit/src/sqlbuild/compiler/python_nodes/_helpers/helpers.py b/tests/unit/src/sqlbuild/compiler/python_nodes/_helpers/helpers.py index c15e5e59c6..7fc548ddaa 100644 --- a/tests/unit/src/sqlbuild/compiler/python_nodes/_helpers/helpers.py +++ b/tests/unit/src/sqlbuild/compiler/python_nodes/_helpers/helpers.py @@ -513,7 +513,7 @@ def build_orders_project_graph() -> ProjectGraph: deps=(raw_orders_key,), name="orders", relative_path=Path("models/orders.sql"), - query_sql="select * from __source('raw_orders')", + query_sql='select * from __source("raw_orders")', config=CompileModelConfig(values={"tags": ["daily"]}), destination=CompiledRelationLocation( database=None, diff --git a/website/src/content/docs/docs/concepts/models.mdx b/website/src/content/docs/docs/concepts/models.mdx index 4e8c952c2c..d8757af255 100644 --- a/website/src/content/docs/docs/concepts/models.mdx +++ b/website/src/content/docs/docs/concepts/models.mdx @@ -38,6 +38,20 @@ The header controls how SQLBuild builds, validates, and documents the model. The | Source | `__source("name")` | An external source | | Scalar UDF | `__udf("name")` | A user-defined function | +Write the name as exactly one double-quoted string, with no spaces or comments inside the parentheses. `__dbt_ref("package", "name")` is the only call that takes two names. Compile rejects any other form, such as `__ref(stg_orders)`, `__ref('stg_orders')` or `__ref( /* upstream */ "stg_orders")`, with a `P012` error at the call that shows the corrected call: + +```text +error[P012]: __ref(stg_orders) is not a valid __ref() call + model: fact_orders + --> models/marts/fact_orders.sql:24:6 + | + 24 | FROM __ref(stg_orders) o + | ^^^^^^^^^^^^^^^^^ + = help: __ref() takes exactly one double-quoted name, with no comments or extra spaces inside the parentheses: __ref("stg_orders") +``` + +One compile reports every rejected call. When a macro returns the rejected call, the error points at the macro and shows how to pass the reference in instead, for example `@staged_orders(__ref("stg_orders"))`. The same rule applies inside test and scenario files, including `__assert__` CTEs. + SQLBuild discovers the dependency graph from these calls and orders selected work topologically. Among selected models, upstream models run before downstream dependents. An unselected upstream is read from its existing warehouse relation; use an upstream-expanding selector such as `+fact_orders` when it should also be built. Seeds use `__seed()`, not `__ref()`. See [Functions](/docs/concepts/functions) for scalar UDF and table-function references.