Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ const DIRECT_NAMES: [(&str, &str, &str); 3] = [
"__table_fn_expected__",
),
];
const TEST_CONTEXT: &str = "SQL test";
const SCENARIO_CONTEXT: &str = "SQL scenario";
const DECLARATION_CALLS: [&str; 4] = ["enum", "const", "var", "param"];
/// Longest projection the alias help repeats verbatim.
const ALIAS_HELP_EXPRESSION_LIMIT: usize = 80;
Expand Down Expand Up @@ -67,6 +69,9 @@ struct TestRequest {
/// Read only the CTE names and body offsets of the authored block, before any expansion.
#[serde(default)]
authored: bool,
/// The authored text is a scenario rather than a SQL test block.
#[serde(default)]
scenario: bool,
}

/// One extraction error, with help and the offending text where the author can act on it.
Expand Down Expand Up @@ -189,7 +194,7 @@ fn extract_test(test: &TestRequest, syntax: &LexicalSyntax) -> Result<Classified
if test.authored {
return authored_ctes(test, syntax);
}
let ctes: Vec<Cte> = scan_ctes(&test.sql, &test.file_label, syntax)?.ctes;
let ctes: Vec<Cte> = scan_ctes(&test.sql, &test.file_label, TEST_CONTEXT, syntax)?.ctes;
let scope = TestScope {
file: &test.file_label,
syntax,
Expand All @@ -204,7 +209,12 @@ fn extract_test(test: &TestRequest, syntax: &LexicalSyntax) -> Result<Classified

/// The authored block's CTEs, or the located error of a CTE name; other errors read as no CTEs.
fn authored_ctes(test: &TestRequest, syntax: &LexicalSyntax) -> Result<Classified, Failure> {
match scan_ctes(&test.sql, &test.file_label, syntax) {
let context: &str = if test.scenario {
SCENARIO_CONTEXT
} else {
TEST_CONTEXT
};
match scan_ctes(&test.sql, &test.file_label, context, syntax) {
Ok(scanned) => Ok(Classified::Authored {
ctes: scanned
.ctes
Expand All @@ -218,13 +228,30 @@ fn authored_ctes(test: &TestRequest, syntax: &LexicalSyntax) -> Result<Classifie
}
}

/// Who owns the CTEs being read, which names them in errors, and the adapter's lexical rules.
struct CteOwner<'a> {
file: &'a str,
context: &'a str,
syntax: &'a LexicalSyntax,
}

/// The top-level CTEs of one test, with the code-point offset of each stripped body.
struct ScannedCtes {
ctes: Vec<Cte>,
body_starts: Vec<usize>,
}

fn scan_ctes(sql: &str, file: &str, syntax: &LexicalSyntax) -> Result<ScannedCtes, Failure> {
fn scan_ctes(
sql: &str,
file: &str,
context: &str,
syntax: &LexicalSyntax,
) -> Result<ScannedCtes, Failure> {
let owner = CteOwner {
file,
context,
syntax,
};
let mut body_starts: Vec<usize> = Vec::new();
let mut index = skip_ignorable(sql, 0, syntax)?;
index = consume_keyword(sql, index, "WITH").ok_or_else(|| {
Expand All @@ -237,7 +264,7 @@ fn scan_ctes(sql: &str, file: &str, syntax: &LexicalSyntax) -> Result<ScannedCte
let mut ctes: Vec<Cte> = Vec::new();
let mut seen: HashSet<String> = HashSet::new();
loop {
let (name, end) = read_cte_name(sql, index, file, syntax)?;
let (name, end) = read_cte_name(sql, index, &owner)?;
if !seen.insert(name.clone()) {
return Err(format!("SQL test '{file}' defines duplicate CTE '{name}'").into());
}
Expand All @@ -252,8 +279,11 @@ fn scan_ctes(sql: &str, file: &str, syntax: &LexicalSyntax) -> Result<ScannedCte
index = consume_keyword(sql, index, "AS")
.ok_or_else(|| format!("SQL test '{file}' expected keyword AS"))?;
index = skip_ignorable(sql, index, syntax)?;
if let Some(failure) = materialization_failure(sql, index, &owner, &name)? {
return Err(failure);
}
if byte_at(sql, index) != Some(b'(') {
return Err(format!("SQL test '{file}' CTE '{name}' must use AS (...)").into());
return Err(format!("{context} '{file}' CTE '{name}' must use AS (...)").into());
}
let close = matching_paren(sql, index, "SQL test", syntax)?;
let body = &sql[index + 1..close];
Expand All @@ -279,9 +309,9 @@ fn scan_ctes(sql: &str, file: &str, syntax: &LexicalSyntax) -> Result<ScannedCte
fn read_cte_name(
sql: &str,
start: usize,
file: &str,
syntax: &LexicalSyntax,
owner: &CteOwner<'_>,
) -> Result<(String, usize), Failure> {
let (file, context, syntax) = (owner.file, owner.context, owner.syntax);
if let Some(quote) = byte_at(sql, start).filter(|byte| matches!(byte, b'"' | b'`' | b'[')) {
let end = if quote == b'[' {
sql[start..]
Expand All @@ -290,28 +320,59 @@ fn read_cte_name(
} else {
skip_non_code(sql, start, syntax)?
};
return Err(cte_name_failure(sql, start, end, file));
return Err(cte_name_failure(sql, start, end, owner));
}
let Some(first) = char_at(sql, start).filter(|first| *first == '_' || first.is_alphabetic())
else {
return Err(format!("SQL test '{file}' expected a CTE name").into());
let Some(first) = char_at(sql, start).filter(|first| is_identifier_continue(*first)) else {
return Err(format!("{context} '{file}' expected a CTE name").into());
};
let mut end = start + first.len_utf8();
while let Some(character) =
char_at(sql, end).filter(|character| is_identifier_continue(*character))
{
end += character.len_utf8();
}
if sql[start..end]
.bytes()
.all(|byte| byte.is_ascii_alphanumeric() || byte == b'_')
if (first == '_' || first.is_ascii_alphabetic())
&& sql[start..end]
.bytes()
.all(|byte| byte.is_ascii_alphanumeric() || byte == b'_')
{
return Ok((sql[start..end].to_owned(), end));
}
Err(cte_name_failure(sql, start, end, file))
Err(cte_name_failure(sql, start, end, owner))
}

/// Python's error for `AS MATERIALIZED` or `AS NOT MATERIALIZED`, located at the hint.
fn materialization_failure(
sql: &str,
start: usize,
owner: &CteOwner<'_>,
name: &str,
) -> Result<Option<Failure>, Failure> {
let (hint, end) = match consume_keyword(sql, start, "NOT") {
Some(not_end) => {
let index = skip_ignorable(sql, not_end, owner.syntax)?;
match consume_keyword(sql, index, "MATERIALIZED") {
Some(end) => ("NOT MATERIALIZED", end),
None => return Ok(None),
}
}
None => match consume_keyword(sql, start, "MATERIALIZED") {
Some(end) => ("MATERIALIZED", end),
None => return Ok(None),
},
};
Ok(Some(Failure {
message: format!(
"{} '{}' CTE '{name}' must not use AS {hint}; materialization hints are not supported in {} CTEs",
owner.context, owner.file, owner.context
),
help: Some(format!("remove {hint} and write {name} AS (...)")),
token: Some((sql[start..end].to_owned(), sql[..start].chars().count())),
}))
}

fn cte_name_failure(sql: &str, start: usize, end: usize, file: &str) -> Failure {
fn cte_name_failure(sql: &str, start: usize, end: usize, owner: &CteOwner<'_>) -> Failure {
let (file, context) = (owner.file, owner.context);
let token = &sql[start..end];
let shown = if matches!(byte_at(sql, start), Some(b'"' | b'`' | b'[')) {
token.to_owned()
Expand All @@ -338,7 +399,7 @@ fn cte_name_failure(sql: &str, start: usize, end: usize, file: &str) -> Failure
}
Failure {
message: format!(
"SQL test '{file}' CTE name {shown} must be an unquoted identifier of ASCII letters, digits and underscores"
"{context} '{file}' CTE name {shown} must be an unquoted identifier of ASCII letters, digits and underscores"
),
help: Some(format!(
"rename the CTE, for example {suggestion}; quoted CTE names and names with $ or non-ASCII characters are not supported"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,10 @@ struct ClassifiedScenario {
assertion_names: Vec<String>,
}

/// One scenario's scanner error, or its classification after any Polyglot independence check.
/// One scenario's error, or its classification after any Polyglot independence check.
#[derive(Debug, Default, Serialize)]
#[serde(rename_all = "camelCase")]
struct ScenarioOutcome {
#[serde(skip_serializing_if = "Option::is_none")]
scan_error: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
independence: Option<Vec<Cte>>,
#[serde(skip_serializing_if = "Option::is_none")]
Expand All @@ -54,7 +52,7 @@ pub(crate) fn extract_scenario_json(
match scan_top_level_ctes(sql, file, RelationshipSource::Scenario, syntax) {
TopLevelCtes::Deferred => return Ok(None),
TopLevelCtes::Failed(message) => ScenarioOutcome {
scan_error: Some(message),
error: Some(message),
..ScenarioOutcome::default()
},
TopLevelCtes::Scanned(ctes) => {
Expand Down
4 changes: 2 additions & 2 deletions crates/sqlbuild-analysis/src/compiler/tests/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1991,8 +1991,8 @@ pub(crate) fn rejected_outcome(
}

/// The authored-block reading of `sql`: its CTEs with body offsets, or its first error.
pub(crate) fn authored_ctes_response(sql: &str) -> Value {
pub(crate) fn authored_ctes_response(sql: &str, scenario: bool) -> Value {
batch_response(
&json!({"tests": [{"sql": sql, "fileLabel": "tests/t.sql", "mode": "model", "authored": true}]}),
&json!({"tests": [{"sql": sql, "fileLabel": "tests/t.sql", "mode": "model", "authored": true, "scenario": scenario}]}),
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,23 @@ fn given_scenarios_when_extracting_natively_then_python_outcomes_or_deferrals_ar
),
},
ScenarioExtractionTestCase {
description: "a quoted CTE name is a scanner error Python may still accept with Polyglot",
description: "a quoted CTE name is Python's scanner error",
sql: "WITH \"__source__raw\" AS (SELECT 1), __expected__orders AS (SELECT 1 AS id)",
expected_json: Some(
r#"{"scanError":"SQL scenario 'tests/scenarios/orders.sql' expected a CTE name"}"#,
r#"{"error":"SQL scenario 'tests/scenarios/orders.sql' expected a CTE name"}"#,
),
},
ScenarioExtractionTestCase {
description: "an unclosed body is a scanner error naming the scenario context",
description: "an unclosed body is Python's scanner error naming the scenario context",
sql: "WITH __source__raw AS (SELECT 1",
expected_json: Some(r#"{"scanError":"SQL scenario contains an unclosed parenthesis"}"#),
expected_json: Some(r#"{"error":"SQL scenario contains an unclosed parenthesis"}"#),
},
ScenarioExtractionTestCase {
description: "a materialization hint is Python's scanner error",
sql: "WITH __source__raw AS NOT MATERIALIZED (SELECT 1), __expected__orders AS (SELECT 1)",
expected_json: Some(
r#"{"error":"SQL scenario 'tests/scenarios/orders.sql' CTE '__source__raw' must not use AS NOT MATERIALIZED; materialization hints are not supported in SQL scenario CTEs"}"#,
),
},
ScenarioExtractionTestCase {
description: "a keyword Python would match by case mapping defers",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ fn given_authored_sql_tests_when_extracting_then_each_rule_holds() {
fn given_authored_blocks_when_reading_ctes_then_offsets_and_name_errors_are_exact() {
let test_cases = [
AuthoredCtesTestCase {
scenario: false,
description: "body offsets count code points and skip leading whitespace",
sql: "-- café\nWITH h AS ( SELECT 'é' AS a), __ref__orders AS (\n SELECT 1)",
expected_response: json!({"tests": [{"kind": "authored", "ctes": [
Expand All @@ -393,6 +394,7 @@ fn given_authored_blocks_when_reading_ctes_then_offsets_and_name_errors_are_exac
]}]}),
},
AuthoredCtesTestCase {
scenario: false,
description: "a CTE name error keeps its code-point offset",
sql: "-- é\nWITH h AS (SELECT 1), café AS (SELECT 2)",
expected_response: json!({"error": {
Expand All @@ -404,6 +406,55 @@ fn given_authored_blocks_when_reading_ctes_then_offsets_and_name_errors_are_exac
}}),
},
AuthoredCtesTestCase {
scenario: false,
description: "a materialization hint is located at the hint with help",
sql: "WITH h AS (SELECT 1),\n__ref__orders AS not\nmaterialized (SELECT 2)",
expected_response: json!({"error": {
"index": 0,
"message": "SQL test 'tests/t.sql' CTE '__ref__orders' must not use AS NOT MATERIALIZED; materialization hints are not supported in SQL test CTEs",
"help": "remove NOT MATERIALIZED and write __ref__orders AS (...)",
"token": "not\nmaterialized",
"tokenOffset": 39,
}}),
},
AuthoredCtesTestCase {
scenario: true,
description: "a scenario's quoted CTE name names the scenario",
sql: "WITH \"__source__raw_orders\" AS (SELECT 1), __expected__orders AS (SELECT 1)",
expected_response: json!({"error": {
"index": 0,
"message": "SQL scenario 'tests/t.sql' CTE name \"__source__raw_orders\" must be an unquoted identifier of ASCII letters, digits and underscores",
"help": "rename the CTE, for example __source__raw_orders; quoted CTE names and names with $ or non-ASCII characters are not supported",
"token": "\"__source__raw_orders\"",
"tokenOffset": 5,
}}),
},
AuthoredCtesTestCase {
scenario: true,
description: "a scenario's materialization hint names the scenario",
sql: "WITH __source__raw_orders AS MATERIALIZED (SELECT 1)",
expected_response: json!({"error": {
"index": 0,
"message": "SQL scenario 'tests/t.sql' CTE '__source__raw_orders' must not use AS MATERIALIZED; materialization hints are not supported in SQL scenario CTEs",
"help": "remove MATERIALIZED and write __source__raw_orders AS (...)",
"token": "MATERIALIZED",
"tokenOffset": 29,
}}),
},
AuthoredCtesTestCase {
scenario: false,
description: "a CTE name starting with a digit is located with help",
sql: "WITH 2024_orders AS (SELECT 1)",
expected_response: json!({"error": {
"index": 0,
"message": "SQL test 'tests/t.sql' CTE name '2024_orders' must be an unquoted identifier of ASCII letters, digits and underscores",
"help": "rename the CTE, for example cte_2024_orders; quoted CTE names and names with $ or non-ASCII characters are not supported",
"token": "2024_orders",
"tokenOffset": 5,
}}),
},
AuthoredCtesTestCase {
scenario: false,
description: "other errors are left to extraction and read as no CTEs",
sql: "WITH h AS (SELECT 1) SELECT * FROM h",
expected_response: json!({"tests": [{"kind": "authored", "ctes": []}]}),
Expand All @@ -412,7 +463,7 @@ fn given_authored_blocks_when_reading_ctes_then_offsets_and_name_errors_are_exac

for test_case in test_cases {
assert_eq!(
authored_ctes_response(test_case.sql),
authored_ctes_response(test_case.sql, test_case.scenario),
test_case.expected_response,
"{}",
test_case.description
Expand Down
1 change: 1 addition & 0 deletions crates/sqlbuild-analysis/src/compiler/tests/test_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ pub(crate) struct ExtractionRuleTestCase {

pub(crate) struct AuthoredCtesTestCase {
pub(crate) description: &'static str,
pub(crate) scenario: bool,
pub(crate) sql: &'static str,
pub(crate) expected_response: serde_json::Value,
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,15 @@ pub(crate) fn parse_scenario_file(
contents[..header_start].matches('\n').count() + 1,
)?;
let sql_body: String = python_cleandoc(options.python, &contents[sql_start..]);
let span_start: usize = contents[..sql_start].chars().count();
let body_span: (usize, usize) = (
span_start,
span_start + contents[sql_start..].chars().count(),
);
Ok(DiscoveredScenarioFile {
contents,
header_values,
sql_body,
body_span,
})
}
2 changes: 2 additions & 0 deletions crates/sqlbuild-discovery/src/sql_tests/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,6 @@ pub struct DiscoveredScenarioFile {
pub contents: String,
pub header_values: Vec<(String, AuthoredValue)>,
pub sql_body: String,
/// Code-point span of the authored SQL after the header, which `sql_body` cleans.
pub body_span: (usize, usize),
}
7 changes: 7 additions & 0 deletions crates/sqlbuild-discovery/src/sql_tests/tests/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,10 @@ pub(super) fn statement_failures(contents: &str) -> [Option<(String, Option<Stri
.map(failure),
]
}

/// The code-point span of a parsed scenario's SQL after its header, or `None` if parsing failed.
pub(super) fn scenario_body_span(contents: &str) -> Option<(usize, usize)> {
parse_scenario_text(FILE_PATH, contents.to_owned(), &options())
.ok()
.map(|file| file.body_span)
}
Loading
Loading