diff --git a/crates/sqlbuild-discovery/src/_helpers/header_syntax.rs b/crates/sqlbuild-discovery/src/_helpers/header_syntax.rs new file mode 100644 index 0000000000..608f499c4e --- /dev/null +++ b/crates/sqlbuild-discovery/src/_helpers/header_syntax.rs @@ -0,0 +1,36 @@ +//! The invalid-header-syntax failure every statement header reports, as Python's discovery does. + +use crate::models::{DiscoveryFailure, FailureKind}; +use sqlbuild_sqltext::compiler::main::model_header_errors::header_nesting_failure; + +/// One header that failed to parse, with the one-based file line its text starts on. +pub(crate) struct FailedHeader<'a> { + pub(crate) kind: FailureKind, + /// The name in syntax errors, such as `MODEL`. + pub(crate) statement_name: &'a str, + pub(crate) file_path: &'a str, + pub(crate) text: &'a str, + pub(crate) line: usize, +} + +/// The invalid-syntax failure for `error`; a nesting error names its file line and a fix. +pub(crate) fn header_syntax_failure(header: &FailedHeader<'_>, error: &str) -> DiscoveryFailure { + let (statement_name, file_path) = (header.statement_name, header.file_path); + let Some(nesting) = header_nesting_failure(error, header.text, header.line) else { + return DiscoveryFailure::new( + header.kind, + format!( + "{statement_name}(...) in '{file_path}' contains invalid SQLBuild header syntax: \ + {error}" + ), + ); + }; + DiscoveryFailure { + kind: header.kind, + message: format!( + "{statement_name}(...) in '{file_path}:{}' contains invalid SQLBuild header syntax: {}", + nesting.line, nesting.message + ), + help: Some(nesting.help), + } +} diff --git a/crates/sqlbuild-discovery/src/_helpers/mod.rs b/crates/sqlbuild-discovery/src/_helpers/mod.rs index e331bbba87..7c99418bd0 100644 --- a/crates/sqlbuild-discovery/src/_helpers/mod.rs +++ b/crates/sqlbuild-discovery/src/_helpers/mod.rs @@ -1,4 +1,5 @@ pub(crate) mod header_keys; +pub(crate) mod header_syntax; pub(crate) mod locations; pub(crate) mod pool; pub(crate) mod reading; diff --git a/crates/sqlbuild-discovery/src/_helpers/pool.rs b/crates/sqlbuild-discovery/src/_helpers/pool.rs index 8748adcf96..b5f6c244dd 100644 --- a/crates/sqlbuild-discovery/src/_helpers/pool.rs +++ b/crates/sqlbuild-discovery/src/_helpers/pool.rs @@ -1,4 +1,4 @@ -//! The worker pool native discovery runs on; header parsing recurses, so stacks are large. +//! The worker pool native discovery runs on; parsing recurses, so stacks are large. use rayon::{ThreadPool, ThreadPoolBuilder}; use std::sync::OnceLock; @@ -18,3 +18,11 @@ pub(crate) fn discovery_pool() -> Result<&'static ThreadPool, String> { .as_ref() .map_err(Clone::clone) } + +/// Run `work` on the discovery pool, or on the calling thread (safe: nesting is bounded). +pub(crate) fn on_discovery_pool(work: impl FnOnce() -> T + Send) -> T { + match discovery_pool() { + Ok(pool) => pool.install(work), + Err(_) => work(), + } +} diff --git a/crates/sqlbuild-discovery/src/_helpers/statement_headers.rs b/crates/sqlbuild-discovery/src/_helpers/statement_headers.rs index 4f80974682..f116cb3e12 100644 --- a/crates/sqlbuild-discovery/src/_helpers/statement_headers.rs +++ b/crates/sqlbuild-discovery/src/_helpers/statement_headers.rs @@ -1,6 +1,7 @@ //! Parse a statement header and reject its unsupported keys, as Python's discovery does. use crate::_helpers::header_keys::{UnsupportedKeys, unsupported_keys_failure}; +use crate::_helpers::header_syntax::{FailedHeader, header_syntax_failure}; use crate::models::{DiscoveryFailure, FailureKind}; use sqlbuild_core::text::models::PythonText; use sqlbuild_sqltext::compiler::main::model_header_single_parsing::parse_one; @@ -25,11 +26,13 @@ pub(crate) fn parse_statement_header( header_line: usize, ) -> Result, DiscoveryFailure> { let values: Vec<(String, AuthoredValue)> = match parse_one(header) { - (_, _, Some(error)) => return Err(syntax_failure(contract, &error)), + (_, _, Some(error)) => return Err(syntax_failure(contract, header, header_line, &error)), (Some(AuthoredValue::Map(values)), _, None) => values, _ => { return Err(syntax_failure( contract, + header, + header_line, "Native MODEL header parser returned neither values nor an error", )); } @@ -54,13 +57,21 @@ pub(crate) fn parse_statement_header( })) } -fn syntax_failure(contract: &StatementHeader<'_>, error: &str) -> DiscoveryFailure { - DiscoveryFailure::new( - contract.kind, - format!( - "{}(...) in '{}' contains invalid SQLBuild header syntax: {error}", - contract.statement_name, contract.file_path - ), +fn syntax_failure( + contract: &StatementHeader<'_>, + header: &str, + header_line: usize, + error: &str, +) -> DiscoveryFailure { + header_syntax_failure( + &FailedHeader { + kind: contract.kind, + statement_name: contract.statement_name, + file_path: contract.file_path, + text: header, + line: header_line, + }, + error, ) } diff --git a/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/declaration_headers.rs b/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/declaration_headers.rs index 902475055d..e67d6e3e05 100644 --- a/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/declaration_headers.rs +++ b/crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/declaration_headers.rs @@ -1,5 +1,6 @@ //! Python's `_parse_declaration_headers`: split a file into `KIND(...);` headers and parse each. +use crate::_helpers::header_syntax::{FailedHeader, header_syntax_failure}; use crate::declaration_files::_helpers::checks::python_values::failure; use crate::declaration_files::_helpers::checks::stops::ParseStop; use crate::models::FailureKind; @@ -55,21 +56,21 @@ pub(crate) fn declaration_headers( ))); } let header_start: usize = open_index + 1; - let (values, column_offsets) = match parse_one(&contents[header_start..close_index]) { - (_, _, Some(error)) => { - return Err(declaration(syntax_message( - expected_kind, - file_path, - &error, - ))); - } + let header = FailedHeader { + kind: FailureKind::Declaration, + statement_name: expected_kind, + file_path, + text: &contents[header_start..close_index], + line: contents[..header_start].matches('\n').count() + 1, + }; + let syntax_failure = |error: &str| ParseStop::Failed(header_syntax_failure(&header, error)); + let (values, column_offsets) = match parse_one(header.text) { + (_, _, Some(error)) => return Err(syntax_failure(&error)), (Some(AuthoredValue::Map(values)), Some(offsets), None) => (values, offsets), _ => { - return Err(declaration(syntax_message( - expected_kind, - file_path, + return Err(syntax_failure( "Native MODEL header parser returned neither values nor an error", - ))); + )); } }; headers.push(DeclarationHeader { @@ -88,10 +89,6 @@ pub(crate) fn declaration_headers( Ok(headers) } -fn syntax_message(kind: &str, file_path: &str, error: &str) -> String { - format!("{kind}(...) in '{file_path}' contains invalid SQLBuild header syntax: {error}") -} - /// `(ENUM|CONSTANT|SCHEMA)\s*\(` at `start`: the kind and the byte offset of the parenthesis. fn declaration_start(contents: &str, start: usize) -> Option<(&'static str, usize)> { let rest: &str = &contents[start..]; diff --git a/crates/sqlbuild-discovery/src/declaration_files/main/parse_declaration_text.rs b/crates/sqlbuild-discovery/src/declaration_files/main/parse_declaration_text.rs index b467ac5d1f..749733c062 100644 --- a/crates/sqlbuild-discovery/src/declaration_files/main/parse_declaration_text.rs +++ b/crates/sqlbuild-discovery/src/declaration_files/main/parse_declaration_text.rs @@ -1,5 +1,6 @@ //! Parse in-memory declaration file contents as discovery parses a file of one kind. +use crate::_helpers::pool::on_discovery_pool; use crate::declaration_files::_helpers::checks::stops::ParseStop; use crate::declaration_files::_helpers::parsing::audit_files::parse_audit_file; use crate::declaration_files::_helpers::parsing::constant_files::parse_constant_file; @@ -12,12 +13,21 @@ use crate::declaration_files::models::{ }; use crate::models::FileOutcome; -/// `contents` parsed as a `kind` file named `file_path`; `None` for seeds and macros, which have none. +/// `contents` parsed on the discovery pool as a `kind` file; `None` for seeds and macros. pub fn parse_declaration_text( kind: CollectionKind, (file_path, hook_name): (&str, &str), contents: String, options: &DeclarationFileOptions, +) -> Option> { + on_discovery_pool(|| parse_text(kind, (file_path, hook_name), contents, options)) +} + +fn parse_text( + kind: CollectionKind, + (file_path, hook_name): (&str, &str), + contents: String, + options: &DeclarationFileOptions, ) -> Option> { let parsed: Result = match kind { CollectionKind::Enums => { diff --git a/crates/sqlbuild-discovery/src/declaration_files/tests/helpers.rs b/crates/sqlbuild-discovery/src/declaration_files/tests/helpers.rs index 3521bc8517..ab6519c6f8 100644 --- a/crates/sqlbuild-discovery/src/declaration_files/tests/helpers.rs +++ b/crates/sqlbuild-discovery/src/declaration_files/tests/helpers.rs @@ -82,3 +82,18 @@ pub(super) fn session_reads( fn collection_len(collection: &DeclarationCollection) -> usize { format!("{collection:?}").matches("ScopedFile {").count() } + +/// The debug text of the failure a header nested too deeply reports at `(statement, line)`. +pub(super) fn nesting_failure_debug((statement, line): (&str, usize)) -> String { + format!( + "Some(Failed(DiscoveryFailure {{ kind: {}, message: \"{statement}(...) in '{FILE_PATH}:{line}' \ + contains invalid SQLBuild header syntax: values nest deeper than 256 levels\", help: \ + Some(\"flatten the value so it nests at most 256 levels deep\") }}))", + match statement { + "CONSTANT" => "Declaration", + "HOOK" => "SqlHook", + "AUDIT" => "SqlAudit", + _ => "ModelSql", + } + ) +} diff --git a/crates/sqlbuild-discovery/src/declaration_files/tests/test_parse_declaration_text.rs b/crates/sqlbuild-discovery/src/declaration_files/tests/test_parse_declaration_text.rs index 0aab6e90ac..5f38e8839b 100644 --- a/crates/sqlbuild-discovery/src/declaration_files/tests/test_parse_declaration_text.rs +++ b/crates/sqlbuild-discovery/src/declaration_files/tests/test_parse_declaration_text.rs @@ -1,6 +1,8 @@ use crate::declaration_files::models::CollectionKind; -use crate::declaration_files::tests::helpers::parsed_debug; -use crate::declaration_files::tests::test_types::DeclarationTextTestCase; +use crate::declaration_files::tests::helpers::{nesting_failure_debug, parsed_debug}; +use crate::declaration_files::tests::test_types::{ + DeclarationTextTestCase, DeepDeclarationTestCase, +}; #[test] fn given_declaration_files_when_parsing_then_values_and_failures_match_python() { @@ -213,3 +215,56 @@ fn given_declaration_files_when_parsing_then_values_and_failures_match_python() ); } } + +#[test] +fn given_deeply_nested_declaration_headers_when_parsing_then_each_reports_its_file_line() { + let test_cases = [ + DeepDeclarationTestCase { + description: "a constant value", + kind: CollectionKind::Constants, + prefix: "CONSTANT (\n name limits,\n value ", + suffix: ",\n);\n", + expected_location: ("CONSTANT", 3), + }, + DeepDeclarationTestCase { + description: "a hook description", + kind: CollectionKind::SqlHooks, + prefix: "HOOK (\n description ", + suffix: ",\n);\n\nSELECT 1\n", + expected_location: ("HOOK", 2), + }, + DeepDeclarationTestCase { + description: "a function return type", + kind: CollectionKind::SqlFunctions, + prefix: "FUNCTION (\n description \"Whether placed\",\n returns ", + suffix: ",\n);\n\norder_status = 'placed'\n", + expected_location: ("FUNCTION", 3), + }, + DeepDeclarationTestCase { + description: "an audit value on a later block", + kind: CollectionKind::Audits, + prefix: "AUDIT (name a);\nSELECT 1\nAUDIT (\n name b,\n value ", + suffix: ",\n);\nSELECT 2\n", + expected_location: ("AUDIT", 5), + }, + ]; + + for test_case in test_cases { + let contents: String = format!( + "{}{}1{}{}", + test_case.prefix, + "[".repeat(20_000), + "]".repeat(20_000), + test_case.suffix + ); + + let debug: String = parsed_debug(test_case.kind, &contents); + + assert_eq!( + debug, + nesting_failure_debug(test_case.expected_location), + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-discovery/src/declaration_files/tests/test_types.rs b/crates/sqlbuild-discovery/src/declaration_files/tests/test_types.rs index f0cc45c79b..11afe32f54 100644 --- a/crates/sqlbuild-discovery/src/declaration_files/tests/test_types.rs +++ b/crates/sqlbuild-discovery/src/declaration_files/tests/test_types.rs @@ -18,3 +18,15 @@ pub(super) struct SessionReadTestCase { /// The layouts walked: one shared by every kind, plus one per isolated kind. pub(super) expected_layouts: usize, } + +/// A declaration file whose header nests one value far too deeply, on header line `line`. +pub(super) struct DeepDeclarationTestCase { + pub(super) description: &'static str, + pub(super) kind: CollectionKind, + /// The file text before the nested value. + pub(super) prefix: &'static str, + /// The file text after the nested value. + pub(super) suffix: &'static str, + /// The statement name and file line the failure reports. + pub(super) expected_location: (&'static str, usize), +} diff --git a/crates/sqlbuild-discovery/src/model_files/_helpers/parsing.rs b/crates/sqlbuild-discovery/src/model_files/_helpers/parsing.rs index 7ba71042ba..abfed5e535 100644 --- a/crates/sqlbuild-discovery/src/model_files/_helpers/parsing.rs +++ b/crates/sqlbuild-discovery/src/model_files/_helpers/parsing.rs @@ -1,6 +1,7 @@ //! Parse one model file: header checks, model-local declaration checks and their messages. use crate::_helpers::header_keys::{UnsupportedKeys, unsupported_keys_failure}; +use crate::_helpers::header_syntax::{FailedHeader, header_syntax_failure}; use crate::_helpers::locations::header_column_locations; use crate::model_files::_helpers::output_columns::output_column_locations; use crate::model_files::models::{DiscoveredModelFile, ModelFileOptions}; @@ -28,12 +29,20 @@ pub(crate) fn parse_model_file( ))); }; let header = &contents[header_start..header_end]; + let header_line: usize = contents[..header_start].matches('\n').count() + 1; + let failed_header = FailedHeader { + kind: FailureKind::ModelSql, + statement_name: "MODEL", + file_path, + text: header, + line: header_line, + }; let (header_values, column_offsets) = match parse_one(header) { - (_, _, Some(error)) => return Err(syntax_failure(file_path, &error)), + (_, _, Some(error)) => return Err(header_syntax_failure(&failed_header, &error)), (Some(AuthoredValue::Map(values)), Some(offsets), None) => (values, offsets), _ => { - return Err(syntax_failure( - file_path, + return Err(header_syntax_failure( + &failed_header, "Native MODEL header parser returned neither values nor an error", )); } @@ -58,7 +67,7 @@ pub(crate) fn parse_model_file( statement: MODEL_STATEMENT, file_path, header, - header_line: contents[..header_start].matches('\n').count() + 1, + header_line, keys: &unsupported, supported_keys: &options.supported_keys, python: options.python, @@ -91,12 +100,6 @@ pub(crate) fn parse_model_file( }) } -fn syntax_failure(file_path: &str, error: &str) -> DiscoveryFailure { - failure(format!( - "MODEL(...) in '{file_path}' contains invalid SQLBuild header syntax: {error}" - )) -} - fn header_keys(values: &[(String, AuthoredValue)]) -> impl Iterator { values.iter().map(|(key, _)| key.as_str()) } diff --git a/crates/sqlbuild-discovery/src/model_files/main/parse_model_text.rs b/crates/sqlbuild-discovery/src/model_files/main/parse_model_text.rs index 4084878b35..f6b62a408f 100644 --- a/crates/sqlbuild-discovery/src/model_files/main/parse_model_text.rs +++ b/crates/sqlbuild-discovery/src/model_files/main/parse_model_text.rs @@ -1,14 +1,15 @@ //! Parse SQL model contents that are already in memory. +use crate::_helpers::pool::on_discovery_pool; use crate::model_files::_helpers::parsing::parse_model_file; use crate::model_files::models::{DiscoveredModelFile, ModelFileOptions}; use crate::models::DiscoveryFailure; -/// One model file's parsed contents, named `file_path` in messages. +/// One model file's parsed contents, named `file_path` in messages, parsed on the discovery pool. pub fn parse_model_text( file_path: &str, contents: String, options: &ModelFileOptions, ) -> Result { - parse_model_file(file_path, contents, options) + on_discovery_pool(|| parse_model_file(file_path, contents, options)) } diff --git a/crates/sqlbuild-discovery/src/model_files/tests/test_discover_model_files.rs b/crates/sqlbuild-discovery/src/model_files/tests/test_discover_model_files.rs index 56e30c12ed..a37923fa0a 100644 --- a/crates/sqlbuild-discovery/src/model_files/tests/test_discover_model_files.rs +++ b/crates/sqlbuild-discovery/src/model_files/tests/test_discover_model_files.rs @@ -1,5 +1,5 @@ use crate::model_files::tests::helpers::{failed, parsed, parsed_summary}; -use crate::model_files::tests::test_types::ModelFileTestCase; +use crate::model_files::tests::test_types::{DeepModelHeaderTestCase, ModelFileTestCase}; const SUPPORTED_KEYS_HELP: &str = "supported keys: audits, columns, constants, description, enums, materialized, tags, unique_key"; @@ -117,3 +117,46 @@ fn given_model_files_when_parsing_then_values_locations_and_failures_match_pytho ); } } + +#[test] +fn given_deeply_nested_model_header_when_parsing_then_fails_with_its_file_line_and_help() { + let expected_summary = || { + failed( + "MODEL(...) in '/project/models/orders.sql:3' contains invalid SQLBuild header \ + syntax: values nest deeper than 256 levels", + Some("flatten the value so it nests at most 256 levels deep"), + ) + }; + let test_cases = [ + DeepModelHeaderTestCase { + description: "1k nested lists", + depth: 1_000, + expected_summary: expected_summary(), + }, + DeepModelHeaderTestCase { + description: "20k nested lists", + depth: 20_000, + expected_summary: expected_summary(), + }, + DeepModelHeaderTestCase { + description: "100k nested lists", + depth: 100_000, + expected_summary: expected_summary(), + }, + ]; + + for test_case in test_cases { + let contents: String = format!( + "MODEL (\n description \"Orders\",\n tags {}1{},\n);\nSELECT 1", + "[".repeat(test_case.depth), + "]".repeat(test_case.depth) + ); + + assert_eq!( + parsed_summary(&contents), + test_case.expected_summary, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-discovery/src/model_files/tests/test_types.rs b/crates/sqlbuild-discovery/src/model_files/tests/test_types.rs index ab03edff85..45db919f61 100644 --- a/crates/sqlbuild-discovery/src/model_files/tests/test_types.rs +++ b/crates/sqlbuild-discovery/src/model_files/tests/test_types.rs @@ -13,3 +13,10 @@ pub(super) struct ModelFileTestCase { pub(super) contents: &'static str, pub(super) expected_summary: ModelSummary, } + +/// A model whose `tags` header value nests `depth` lists deep. +pub(super) struct DeepModelHeaderTestCase { + pub(super) description: &'static str, + pub(super) depth: usize, + pub(super) expected_summary: ModelSummary, +} diff --git a/crates/sqlbuild-discovery/src/sql_tests/main/parse_scenario_text.rs b/crates/sqlbuild-discovery/src/sql_tests/main/parse_scenario_text.rs index e766ca35ff..2cc3c73a45 100644 --- a/crates/sqlbuild-discovery/src/sql_tests/main/parse_scenario_text.rs +++ b/crates/sqlbuild-discovery/src/sql_tests/main/parse_scenario_text.rs @@ -1,14 +1,15 @@ //! Header-parse SQL scenario contents that are already in memory. +use crate::_helpers::pool::on_discovery_pool; use crate::models::DiscoveryFailure; use crate::sql_tests::_helpers::scenario_parsing::parse_scenario_file; use crate::sql_tests::models::{DiscoveredScenarioFile, SqlTestFileOptions}; -/// The parsed header and body of one scenario file's contents, named `file_path` in messages. +/// The parsed header and body of one scenario file's contents, named `file_path` in messages, parsed on the discovery pool. pub fn parse_scenario_text( file_path: &str, contents: String, options: &SqlTestFileOptions, ) -> Result { - parse_scenario_file(file_path, contents, options) + on_discovery_pool(|| parse_scenario_file(file_path, contents, options)) } diff --git a/crates/sqlbuild-discovery/src/sql_tests/main/parse_sql_test_text.rs b/crates/sqlbuild-discovery/src/sql_tests/main/parse_sql_test_text.rs index 3010f15752..1b263a7683 100644 --- a/crates/sqlbuild-discovery/src/sql_tests/main/parse_sql_test_text.rs +++ b/crates/sqlbuild-discovery/src/sql_tests/main/parse_sql_test_text.rs @@ -1,14 +1,15 @@ //! Split SQL unit-test contents that are already in memory. +use crate::_helpers::pool::on_discovery_pool; use crate::models::DiscoveryFailure; use crate::sql_tests::_helpers::test_blocks::parse_sql_test_file; use crate::sql_tests::models::{DiscoveredSqlTestFile, SqlTestFileOptions}; -/// The header-parsed blocks of one SQL test file's contents, named `file_path` in messages. +/// The header-parsed blocks of one SQL test file's contents, named `file_path` in messages, parsed on the discovery pool. pub fn parse_sql_test_text( file_path: &str, contents: String, options: &SqlTestFileOptions, ) -> Result { - parse_sql_test_file(file_path, contents, options) + on_discovery_pool(|| parse_sql_test_file(file_path, contents, options)) } diff --git a/crates/sqlbuild-discovery/src/sql_tests/tests/helpers.rs b/crates/sqlbuild-discovery/src/sql_tests/tests/helpers.rs index 3abaa38ac5..3bbafd4871 100644 --- a/crates/sqlbuild-discovery/src/sql_tests/tests/helpers.rs +++ b/crates/sqlbuild-discovery/src/sql_tests/tests/helpers.rs @@ -81,3 +81,15 @@ pub(super) fn expected_rows( expected_failure.map(str::to_owned), ) } + +/// The failure that stops `contents` as a test file and as a scenario file, with their help. +pub(super) fn statement_failures(contents: &str) -> [Option<(String, Option)>; 2] { + let failure = |failure: DiscoveryFailure| (failure.message, failure.help); + [ + parse_sql_test_text(FILE_PATH, contents.to_owned(), &options()) + .map_or_else(|stop| Some(failure(stop)), |file| file.failure.map(failure)), + parse_scenario_text(FILE_PATH, contents.replace("TEST", "SCENARIO"), &options()) + .err() + .map(failure), + ] +} diff --git a/crates/sqlbuild-discovery/src/sql_tests/tests/test_sql_test_files.rs b/crates/sqlbuild-discovery/src/sql_tests/tests/test_sql_test_files.rs index 96e92835bd..3b3bd49d96 100644 --- a/crates/sqlbuild-discovery/src/sql_tests/tests/test_sql_test_files.rs +++ b/crates/sqlbuild-discovery/src/sql_tests/tests/test_sql_test_files.rs @@ -1,5 +1,7 @@ -use crate::sql_tests::tests::helpers::{expected_rows, scenario_rows, test_file_rows}; -use crate::sql_tests::tests::test_types::StatementFileTestCase; +use crate::sql_tests::tests::helpers::{ + FILE_PATH, expected_rows, scenario_rows, statement_failures, test_file_rows, +}; +use crate::sql_tests::tests::test_types::{DeepStatementHeaderTestCase, StatementFileTestCase}; #[test] fn given_sql_test_files_when_splitting_then_blocks_and_failures_match_python() { @@ -72,3 +74,36 @@ fn given_scenario_files_when_parsing_then_headers_and_failures_match_python() { ); } } + +#[test] +fn given_deeply_nested_test_and_scenario_headers_when_parsing_then_each_reports_its_file_line() { + let expected = |statement: &str| { + Some(( + format!( + "{statement}(...) in '{FILE_PATH}:3' contains invalid SQLBuild header syntax: \ + values nest deeper than 256 levels" + ), + Some("flatten the value so it nests at most 256 levels deep".to_owned()), + )) + }; + let test_cases = [DeepStatementHeaderTestCase { + description: "20k nested case maps", + depth: 20_000, + expected_failures: [expected("TEST"), expected("SCENARIO")], + }]; + + for test_case in test_cases { + let contents: String = format!( + "TEST (\n name \"keeps_status\",\n cases {}1{},\n);\n\nSELECT 1\n", + "(case ".repeat(test_case.depth), + ")".repeat(test_case.depth) + ); + + assert_eq!( + statement_failures(&contents), + test_case.expected_failures, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-discovery/src/sql_tests/tests/test_types.rs b/crates/sqlbuild-discovery/src/sql_tests/tests/test_types.rs index f45e83d054..db379eb3be 100644 --- a/crates/sqlbuild-discovery/src/sql_tests/tests/test_types.rs +++ b/crates/sqlbuild-discovery/src/sql_tests/tests/test_types.rs @@ -5,3 +5,11 @@ pub(super) struct StatementFileTestCase { pub(super) expected_blocks: &'static [(&'static [&'static str], &'static str)], pub(super) expected_failure: Option<&'static str>, } + +/// A TEST header, and the same header as a SCENARIO, whose `cases` value nests `depth` maps deep. +pub(super) struct DeepStatementHeaderTestCase { + pub(super) description: &'static str, + pub(super) depth: usize, + /// The test file's failure, then the scenario file's: each message and help. + pub(super) expected_failures: [Option<(String, Option)>; 2], +} diff --git a/crates/sqlbuild-sqltext/src/compiler/_helpers/model_headers/tokenization.rs b/crates/sqlbuild-sqltext/src/compiler/_helpers/model_headers/tokenization.rs index 06224c0bc9..0afb9098a4 100644 --- a/crates/sqlbuild-sqltext/src/compiler/_helpers/model_headers/tokenization.rs +++ b/crates/sqlbuild-sqltext/src/compiler/_helpers/model_headers/tokenization.rs @@ -3,9 +3,10 @@ use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; use rayon::{ThreadPool, ThreadPoolBuilder}; -use crate::compiler::models::AuthoredValue; +use crate::compiler::models::{AuthoredValue, NestingFailure}; use crate::constants::{ - COLUMNS_KEY, CONSTANT_CALL, INLINE_SQL_HOOK, OUTSIDE_KEY, SQL_HOOK, THRESHOLDS_KEY, TYPE_KEY, + COLUMNS_KEY, CONSTANT_CALL, INLINE_SQL_HOOK, MAX_HEADER_NESTING_DEPTH, OUTSIDE_KEY, SQL_HOOK, + THRESHOLDS_KEY, TYPE_KEY, }; use sqlbuild_core::constants::{CLOSE_PAREN, OPEN_PAREN}; @@ -53,6 +54,8 @@ struct HeaderParser { tokens: Vec, index: usize, column_offsets: Vec, + /// The open containers, at most `MAX_HEADER_NESTING_DEPTH`, which bounds the recursion. + depth: usize, } #[derive(Clone, Copy, Default)] @@ -80,6 +83,7 @@ impl HeaderParser { tokens, index: 0, column_offsets: Vec::new(), + depth: 0, }; let value = if parser.peek().0 == END_TOKEN { AuthoredValue::Map(Vec::new()) @@ -197,49 +201,82 @@ impl HeaderParser { self.advance(); if self.peek().0 == SYMBOL_TOKEN && self.peek().1 == OPEN_PAREN { self.advance(); - if matches!(token.1.as_str(), "__ref" | "__seed" | "__source") { - return self.parse_relation(&token.1); - } - if matches!(token.1.as_str(), "inline_sql" | "sql" | "python") { - return self.parse_hook(&token.1); - } - if token.1 == CONSTANT_CALL { - return Ok(AuthoredValue::TypedConstant(self.parse_map( - ParseMapOptions { - end: Some(")"), - ..ParseMapOptions::default() - }, - )?)); - } - return Ok(AuthoredValue::Map(vec![( - token.1, - AuthoredValue::Map(self.parse_map(ParseMapOptions { - end: Some(")"), - threshold_policy: options.threshold_policy, - allow_outside: options.allow_outside, - ..ParseMapOptions::default() - })?), - )])); + return self.nested(token.2, |parser| parser.parse_call(token, options)); } return Ok(parse_word(token.1)); } if self.match_symbol("[") { - return Ok(AuthoredValue::List(self.parse_sequence("]")?)); + return self.nested(token.2, |parser| { + Ok(AuthoredValue::List(parser.parse_sequence("]")?)) + }); } if self.match_symbol("{") { - return Ok(AuthoredValue::Set(self.parse_sequence("}")?)); + return self.nested(token.2, |parser| { + Ok(AuthoredValue::Set(parser.parse_sequence("}")?)) + }); } if self.match_symbol("(") { - return Ok(AuthoredValue::Map(self.parse_map(ParseMapOptions { + return self.nested(token.2, |parser| { + Ok(AuthoredValue::Map(parser.parse_map(ParseMapOptions { + end: Some(")"), + threshold_policy: options.threshold_policy, + allow_outside: options.allow_outside, + column_entries: options.nested_columns, + column_metadata: options.nested_metadata, + record_column_entries: options.record_nested_columns, + })?)) + }); + } + Err(format!("expected value at position {}", token.2)) + } + + /// The value after a consumed `name(`; a plain call nests a map in a map, two levels. + fn parse_call( + &mut self, + (_, name, position): HeaderToken, + options: ParseValueOptions, + ) -> Result { + if matches!(name.as_str(), "__ref" | "__seed" | "__source") { + return self.parse_relation(&name); + } + if matches!(name.as_str(), "inline_sql" | "sql" | "python") { + return self.parse_hook(&name, position); + } + if name == CONSTANT_CALL { + return Ok(AuthoredValue::TypedConstant(self.parse_map( + ParseMapOptions { + end: Some(")"), + ..ParseMapOptions::default() + }, + )?)); + } + let arguments: Vec<(String, AuthoredValue)> = self.nested(position, |parser| { + parser.parse_map(ParseMapOptions { end: Some(")"), threshold_policy: options.threshold_policy, allow_outside: options.allow_outside, - column_entries: options.nested_columns, - column_metadata: options.nested_metadata, - record_column_entries: options.record_nested_columns, - })?)); - } - Err(format!("expected value at position {}", token.2)) + ..ParseMapOptions::default() + }) + })?; + Ok(AuthoredValue::Map(vec![( + name, + AuthoredValue::Map(arguments), + )])) + } + + /// Parse one container opened at `position`, refusing to nest deeper than the limit. + fn nested( + &mut self, + position: usize, + parse: impl FnOnce(&mut Self) -> Result, + ) -> Result { + if self.depth >= MAX_HEADER_NESTING_DEPTH { + return Err(nesting_error(position)); + } + self.depth += 1; + let value = parse(self); + self.depth -= 1; + value } fn parse_sequence(&mut self, end: &str) -> Result, String> { @@ -256,12 +293,16 @@ impl HeaderParser { } fn parse_hook_field(&mut self, field: &str) -> Result { + let position: usize = self.peek().2; if !self.match_symbol("[") { return Err(format!( - "{field} must be a list of typed inline_sql(...), sql(...), or python(...) hook entries at position {}", - self.peek().2 + "{field} must be a list of typed inline_sql(...), sql(...), or python(...) hook entries at position {position}" )); } + self.nested(position, |parser| parser.parse_hook_entries(field)) + } + + fn parse_hook_entries(&mut self, field: &str) -> Result { let mut values: Vec = Vec::new(); while !self.at_end_symbol(Some("]")) { if self.match_symbol(",") { @@ -296,7 +337,7 @@ impl HeaderParser { if !matches!(token.1.as_str(), "inline_sql" | "sql" | "python") { return Err(error()); } - self.parse_hook(&token.1) + self.nested(token.2, |parser| parser.parse_hook(&token.1, token.2)) } fn parse_relation(&mut self, name: &str) -> Result { @@ -314,7 +355,8 @@ impl HeaderParser { ))) } - fn parse_hook(&mut self, name: &str) -> Result { + /// The hook after `name(` at `position`; its keyword arguments are one more level. + fn parse_hook(&mut self, name: &str, position: usize) -> Result { if name == INLINE_SQL_HOOK { let token = self.peek().clone(); if token.0 != STRING_TOKEN { @@ -335,7 +377,7 @@ impl HeaderParser { if token.1.chars().all(is_python_whitespace) { return Err(format!("{name}(...) requires a non-empty hook name")); } - let kwargs = self.parse_hook_kwargs()?; + let kwargs = self.nested(position, Self::parse_hook_kwargs)?; if name == SQL_HOOK { Ok(AuthoredValue::NamedSqlHook(token.1, kwargs)) } else { @@ -416,6 +458,48 @@ impl HeaderParser { } } +fn nesting_error(position: usize) -> String { + format!("{}{position}", nesting_error_prefix()) +} + +fn nesting_error_prefix() -> String { + format!("values nest deeper than {MAX_HEADER_NESTING_DEPTH} levels at position ") +} + +/// The located nesting failure for `error` of `header` starting on `header_line`, if it is one. +pub(crate) fn nesting_failure( + error: &str, + header: &str, + header_line: usize, +) -> Option { + let digits: &str = error.strip_prefix(nesting_error_prefix().as_str())?; + if digits.is_empty() { + return None; + } + let position: usize = digits.bytes().try_fold(0_usize, |position, digit| { + digit + .is_ascii_digit() + .then(|| { + position + .checked_mul(10)? + .checked_add(usize::from(digit - b'0')) + }) + .flatten() + })?; + Some(NestingFailure { + line: header_line + + header + .chars() + .take(position) + .filter(|character| *character == '\n') + .count(), + message: format!("values nest deeper than {MAX_HEADER_NESTING_DEPTH} levels"), + help: format!( + "flatten the value so it nests at most {MAX_HEADER_NESTING_DEPTH} levels deep" + ), + }) +} + fn parse_word(value: String) -> AuthoredValue { match value.as_str() { "true" => AuthoredValue::Boolean(true), diff --git a/crates/sqlbuild-sqltext/src/compiler/main/mod.rs b/crates/sqlbuild-sqltext/src/compiler/main/mod.rs index a9be565a74..f5e0b71fd8 100644 --- a/crates/sqlbuild-sqltext/src/compiler/main/mod.rs +++ b/crates/sqlbuild-sqltext/src/compiler/main/mod.rs @@ -1,5 +1,6 @@ pub mod declaration_references; pub mod model_header_byte_matching; +pub mod model_header_errors; pub mod model_header_matching; pub mod model_header_parsing; pub mod model_header_single_parsing; diff --git a/crates/sqlbuild-sqltext/src/compiler/main/model_header_errors.rs b/crates/sqlbuild-sqltext/src/compiler/main/model_header_errors.rs new file mode 100644 index 0000000000..623a057587 --- /dev/null +++ b/crates/sqlbuild-sqltext/src/compiler/main/model_header_errors.rs @@ -0,0 +1,11 @@ +use crate::compiler::_helpers::model_headers::tokenization; +use crate::compiler::models::NestingFailure; + +/// The nesting failure `error` reports for `header`, starting on one-based `header_line`, if any. +pub fn header_nesting_failure( + error: &str, + header: &str, + header_line: usize, +) -> Option { + tokenization::nesting_failure(error, header, header_line) +} diff --git a/crates/sqlbuild-sqltext/src/compiler/main/model_header_single_parsing.rs b/crates/sqlbuild-sqltext/src/compiler/main/model_header_single_parsing.rs index c3af4f4bac..60d9428278 100644 --- a/crates/sqlbuild-sqltext/src/compiler/main/model_header_single_parsing.rs +++ b/crates/sqlbuild-sqltext/src/compiler/main/model_header_single_parsing.rs @@ -1,6 +1,6 @@ use crate::compiler::_helpers::model_headers::tokenization::{self, HeaderParseResult}; -/// Parse one header on the calling thread; deep headers need a large worker stack. +/// Parse one header on the calling thread; nesting is bounded, so any thread stack suffices. pub fn parse_one(header: &str) -> HeaderParseResult { tokenization::parse_one(header) } diff --git a/crates/sqlbuild-sqltext/src/compiler/models.rs b/crates/sqlbuild-sqltext/src/compiler/models.rs index 4a6f0fd7ad..fa15867ec1 100644 --- a/crates/sqlbuild-sqltext/src/compiler/models.rs +++ b/crates/sqlbuild-sqltext/src/compiler/models.rs @@ -41,3 +41,13 @@ pub struct DeclarationReference { pub start: usize, pub end: usize, } + +/// A header whose values nest deeper than the parser allows, located in its file. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct NestingFailure { + /// The one-based file line of the container that exceeds the limit. + pub line: usize, + /// The syntax error, without a position. + pub message: String, + pub help: String, +} diff --git a/crates/sqlbuild-sqltext/src/compiler/tests.rs b/crates/sqlbuild-sqltext/src/compiler/tests.rs index 97a4fcc4b8..759eb2c611 100644 --- a/crates/sqlbuild-sqltext/src/compiler/tests.rs +++ b/crates/sqlbuild-sqltext/src/compiler/tests.rs @@ -4,6 +4,8 @@ mod declaration_references; mod helpers; #[path = "tests/test_model_header_matching.rs"] mod model_header_matching; +#[path = "tests/test_model_header_nesting.rs"] +mod model_header_nesting; #[path = "tests/test_model_header_tokenization.rs"] mod model_header_tokenization; #[path = "tests/test_sql_interpolation.rs"] diff --git a/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs b/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs index f0abc3e0dc..9a177333e3 100644 --- a/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs +++ b/crates/sqlbuild-sqltext/src/compiler/tests/helpers.rs @@ -6,7 +6,10 @@ use crate::compiler::_helpers::sql_interpolation::substitution::{ }; use crate::compiler::_helpers::sql_references::extraction::extract; use crate::compiler::main::declaration_references::scan_declaration_references; +use crate::compiler::main::model_header_single_parsing::parse_one; use crate::compiler::models::{AuthoredValue, DeclarationReference}; +use crate::compiler::tests::test_types::HeaderNestingTestCase; +use std::thread; pub(crate) fn scalar_variables_preserve_lexical_boundaries() -> bool { let sqls = vec![ @@ -239,3 +242,32 @@ pub(crate) fn scanned_references(sql: &str) -> Option { .join(", "), ) } + +/// Rust's default spawned-thread stack: bounded nesting must parse on any ordinary thread. +const ORDINARY_THREAD_STACK_BYTES: usize = 2 * 1024 * 1024; + +/// The nesting test case's header. +pub(crate) fn nested_header(test_case: &HeaderNestingTestCase) -> String { + format!( + "{}{}1{}{}", + test_case.prefix, + test_case.open.repeat(test_case.depth), + test_case.close.repeat(test_case.depth), + test_case.suffix + ) +} + +/// The parse error of `header`, parsed on a thread with an ordinary stack. +pub(crate) fn parse_error_on_ordinary_thread(header: String) -> Option { + thread::Builder::new() + .stack_size(ORDINARY_THREAD_STACK_BYTES) + .spawn(move || parse_one(&header).2) + .expect("parser thread starts") + .join() + .expect("parser thread does not overflow its stack") +} + +/// The error the parser reports for a container past the nesting limit at `position`. +pub(crate) fn nesting_error_at(position: usize) -> String { + format!("values nest deeper than 256 levels at position {position}") +} diff --git a/crates/sqlbuild-sqltext/src/compiler/tests/test_model_header_nesting.rs b/crates/sqlbuild-sqltext/src/compiler/tests/test_model_header_nesting.rs new file mode 100644 index 0000000000..05211581ba --- /dev/null +++ b/crates/sqlbuild-sqltext/src/compiler/tests/test_model_header_nesting.rs @@ -0,0 +1,209 @@ +use std::time::{Duration, Instant}; + +use crate::compiler::main::model_header_errors::header_nesting_failure; +use crate::compiler::models::NestingFailure; +use crate::compiler::tests::helpers::{ + nested_header, nesting_error_at, parse_error_on_ordinary_thread, +}; +use crate::compiler::tests::test_types::{HeaderNestingLocationTestCase, HeaderNestingTestCase}; + +const DEEP_HEADER_TIME_BOUND: Duration = Duration::from_secs(10); + +#[test] +fn given_nested_header_values_when_parsing_on_an_ordinary_thread_then_depth_is_capped() { + let test_cases = [ + HeaderNestingTestCase { + description: "lists at the limit parse", + prefix: "meta ", + open: "[", + close: "]", + suffix: "", + depth: 256, + expected_error_position: None, + }, + HeaderNestingTestCase { + description: "lists one past the limit fail at the outermost extra list", + prefix: "meta ", + open: "[", + close: "]", + suffix: "", + depth: 257, + expected_error_position: Some(261), + }, + HeaderNestingTestCase { + description: "lists 1k deep", + prefix: "meta ", + open: "[", + close: "]", + suffix: "", + depth: 1_000, + expected_error_position: Some(261), + }, + HeaderNestingTestCase { + description: "lists 20k deep", + prefix: "meta ", + open: "[", + close: "]", + suffix: "", + depth: 20_000, + expected_error_position: Some(261), + }, + HeaderNestingTestCase { + description: "lists 100k deep", + prefix: "meta ", + open: "[", + close: "]", + suffix: "", + depth: 100_000, + expected_error_position: Some(261), + }, + HeaderNestingTestCase { + description: "sets 100k deep", + prefix: "meta ", + open: "{", + close: "}", + suffix: "", + depth: 100_000, + expected_error_position: Some(261), + }, + HeaderNestingTestCase { + description: "maps at the limit parse", + prefix: "meta ", + open: "(key ", + close: ")", + suffix: "", + depth: 256, + expected_error_position: None, + }, + HeaderNestingTestCase { + description: "maps 100k deep", + prefix: "meta ", + open: "(key ", + close: ")", + suffix: "", + depth: 100_000, + expected_error_position: Some(1_285), + }, + HeaderNestingTestCase { + description: "calls count their map and arguments, so 128 reach the limit", + prefix: "meta ", + open: "call(key ", + close: ")", + suffix: "", + depth: 128, + expected_error_position: None, + }, + HeaderNestingTestCase { + description: "calls one past the limit fail at the outermost extra call", + prefix: "meta ", + open: "call(key ", + close: ")", + suffix: "", + depth: 129, + expected_error_position: Some(1_157), + }, + HeaderNestingTestCase { + description: "calls 20k deep", + prefix: "meta ", + open: "call(key ", + close: ")", + suffix: "", + depth: 20_000, + expected_error_position: Some(1_157), + }, + HeaderNestingTestCase { + description: "typed constants 1k deep", + prefix: "meta ", + open: "constant(value ", + close: ")", + suffix: "", + depth: 1_000, + expected_error_position: Some(3_845), + }, + HeaderNestingTestCase { + description: "hook arguments 100k deep count the hook list, each hook and its arguments", + prefix: "post_hooks [", + open: "sql(\"refresh\", nested: ", + close: ")", + suffix: "]", + depth: 100_000, + expected_error_position: Some(2_933), + }, + ]; + + for test_case in test_cases { + let started: Instant = Instant::now(); + + let error: Option = parse_error_on_ordinary_thread(nested_header(&test_case)); + + assert_eq!( + error, + test_case.expected_error_position.map(nesting_error_at), + "{}", + test_case.description + ); + assert!( + started.elapsed() < DEEP_HEADER_TIME_BOUND, + "{}", + test_case.description + ); + } +} + +#[test] +fn given_parse_errors_when_locating_nesting_failures_then_only_nesting_names_its_line() { + let test_cases = [ + HeaderNestingLocationTestCase { + description: "nesting error on the first header line", + error: "values nest deeper than 256 levels at position 3", + header: "a [[", + header_line: 4, + expected_failure: Some(NestingFailure { + line: 4, + message: "values nest deeper than 256 levels".to_owned(), + help: "flatten the value so it nests at most 256 levels deep".to_owned(), + }), + }, + HeaderNestingLocationTestCase { + description: "nesting error after non-ASCII text on later lines", + error: "values nest deeper than 256 levels at position 12", + header: "\n é \"ü\",\n [[", + header_line: 2, + expected_failure: Some(NestingFailure { + line: 4, + message: "values nest deeper than 256 levels".to_owned(), + help: "flatten the value so it nests at most 256 levels deep".to_owned(), + }), + }, + HeaderNestingLocationTestCase { + description: "other syntax errors are not nesting failures", + error: "expected value at position 3", + header: "a ]", + header_line: 1, + expected_failure: None, + }, + HeaderNestingLocationTestCase { + description: "a nesting prefix with an empty position is not a nesting failure", + error: "values nest deeper than 256 levels at position ", + header: "a [[", + header_line: 1, + expected_failure: None, + }, + HeaderNestingLocationTestCase { + description: "a nesting prefix without a position is not a nesting failure", + error: "values nest deeper than 256 levels at position x", + header: "a [[", + header_line: 1, + expected_failure: None, + }, + ]; + + for test_case in test_cases { + assert_eq!( + header_nesting_failure(test_case.error, test_case.header, test_case.header_line), + test_case.expected_failure, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-sqltext/src/compiler/tests/test_types.rs b/crates/sqlbuild-sqltext/src/compiler/tests/test_types.rs index 34c31f6162..ecff676596 100644 --- a/crates/sqlbuild-sqltext/src/compiler/tests/test_types.rs +++ b/crates/sqlbuild-sqltext/src/compiler/tests/test_types.rs @@ -1,3 +1,5 @@ +use crate::compiler::models::NestingFailure; + pub(crate) struct ModelHeaderTokenizationTestCase { pub(crate) description: &'static str, pub(crate) run: fn() -> bool, @@ -22,3 +24,23 @@ pub(crate) struct DeclarationReferenceScanTestCase { /// References spelled `kind:name[.member]@start..end, ...` in code points. pub(crate) expected_references: Option<&'static str>, } + +pub(crate) struct HeaderNestingTestCase { + pub(crate) description: &'static str, + /// The header is `prefix`, `open` repeated `depth` times, `1`, `close` as often, then `suffix`. + pub(crate) prefix: &'static str, + pub(crate) open: &'static str, + pub(crate) close: &'static str, + pub(crate) suffix: &'static str, + pub(crate) depth: usize, + /// The header position of the container past the limit; `None` when the header parses. + pub(crate) expected_error_position: Option, +} + +pub(crate) struct HeaderNestingLocationTestCase { + pub(crate) description: &'static str, + pub(crate) error: &'static str, + pub(crate) header: &'static str, + pub(crate) header_line: usize, + pub(crate) expected_failure: Option, +} diff --git a/crates/sqlbuild-sqltext/src/constants.rs b/crates/sqlbuild-sqltext/src/constants.rs index 0025aeadcf..76a1c686d9 100644 --- a/crates/sqlbuild-sqltext/src/constants.rs +++ b/crates/sqlbuild-sqltext/src/constants.rs @@ -2,6 +2,8 @@ pub(crate) const COLUMNS_KEY: &str = "columns"; pub(crate) const CONSTANT_CALL: &str = "constant"; pub(crate) const DBT_REFERENCE_KIND: &str = "dbt_ref"; pub(crate) const INLINE_SQL_HOOK: &str = "inline_sql"; +/// The deepest authored header value nesting; YAML and JSON readers bound theirs the same way. +pub(crate) const MAX_HEADER_NESTING_DEPTH: usize = 256; pub(crate) const OUTSIDE_KEY: &str = "outside"; pub(crate) const SQL_HOOK: &str = "sql"; pub(crate) const TABLE_FUNCTION_REFERENCE_KIND: &str = "table_function"; diff --git a/scripts/compiler_differential/_helpers/corpus/discovery_failure_cases.py b/scripts/compiler_differential/_helpers/corpus/discovery_failure_cases.py index 6c9d0b5217..d3fed8b35e 100644 --- a/scripts/compiler_differential/_helpers/corpus/discovery_failure_cases.py +++ b/scripts/compiler_differential/_helpers/corpus/discovery_failure_cases.py @@ -32,6 +32,10 @@ " return None\n" ) _MACRO_MODULE: str = "def cents(value):\n return f'{value} * 100'\n" +_DEEP_HEADER_DEPTH: int = 20_000 +_DEEP_HEADER_VALUE: str = "[" * _DEEP_HEADER_DEPTH + '"placed"' + "]" * _DEEP_HEADER_DEPTH +_DEEP_HEADER_CALLS: str = "limit(level " * _DEEP_HEADER_DEPTH + "1" + ")" * _DEEP_HEADER_DEPTH +_DEEP_HEADER_MESSAGE: str = "contains invalid SQLBuild header syntax: values nest deeper than 256" _SEED_DECLARATION: str = ( "seeds:\n - name: order_channels\n description: Order channels.\n" " columns:\n - name: id\n type: INTEGER\n" @@ -103,6 +107,16 @@ def _sql_file_cases() -> tuple[FailureCase, ...]: ) }, ), + failure_case( + name="model-header-nested-too-deep", + expected_code="D002", + files={ + FAILURE_STAGING_PATH: FAILURE_BASE_STAGING.replace( + '"Staged orders",', f'"Staged orders",\n tags {_DEEP_HEADER_VALUE},' + ) + }, + expected_message=_DEEP_HEADER_MESSAGE, + ), failure_case( name="model-path-not-utf8", expected_code="D016", @@ -147,6 +161,16 @@ def _sql_file_cases() -> tuple[FailureCase, ...]: ) }, ), + failure_case( + name="constant-header-nested-too-deep", + expected_code="D013", + files={ + "models/staging/_sqlbuild/_constants/statuses.sql": ( + f"CONSTANT (name kept_statuses, value {_DEEP_HEADER_CALLS});\n" + ) + }, + expected_message=_DEEP_HEADER_MESSAGE, + ), failure_case( name="enum-duplicate-member", expected_code="D013", diff --git a/src/sqlbuild/compiler/discovery/_helpers/sql/audits.py b/src/sqlbuild/compiler/discovery/_helpers/sql/audits.py index 138f3616b8..e7ff307bbc 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/sql/audits.py +++ b/src/sqlbuild/compiler/discovery/_helpers/sql/audits.py @@ -126,6 +126,7 @@ def _parse_audit_header(*, header: str, header_line: int, file_path: Path) -> di header=header, file_path=file_path, statement_name="AUDIT", + header_line=header_line, error_class=SqlAuditParseError, ) reject_unsupported_header_keys( diff --git a/src/sqlbuild/compiler/discovery/_helpers/sql/declarations.py b/src/sqlbuild/compiler/discovery/_helpers/sql/declarations.py index 45e31c7c9d..e31455d249 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/sql/declarations.py +++ b/src/sqlbuild/compiler/discovery/_helpers/sql/declarations.py @@ -260,13 +260,14 @@ def _parse_declaration_headers( header=header, file_path=file_path, statement_name=expected_kind, + header_line=contents.count("\n", 0, open_index + 1) + 1, ), header=header, header_start=open_index + 1, ) ) except ModelSqlParseError as error: - raise DeclarationParseError(str(error)) from error + raise DeclarationParseError(str(error), help=error.help) from error cursor += 1 if not headers: raise DeclarationParseError(f"{file_path} contains no {expected_kind}(...) declarations") diff --git a/src/sqlbuild/compiler/discovery/_helpers/sql/functions.py b/src/sqlbuild/compiler/discovery/_helpers/sql/functions.py index 38bbdf67fb..06cab4dd39 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/sql/functions.py +++ b/src/sqlbuild/compiler/discovery/_helpers/sql/functions.py @@ -29,17 +29,19 @@ def parse_function_sql(*, contents: str, file_path: Path) -> tuple[dict[str, obj "non-whitespace content" ) + header_line: int = contents.count("\n", 0, header_match.start("header")) + 1 header_values: dict[str, object] = parse_header_values( header=header_match.group("header"), file_path=file_path, statement_name="FUNCTION", + header_line=header_line, ) reject_unsupported_header_keys( header_values=header_values, supported_keys=SQL_FUNCTION_HEADER_KEYS, statement="FUNCTION()", header=header_match.group("header"), - header_line=contents.count("\n", 0, header_match.start("header")) + 1, + header_line=header_line, file_path=file_path, error_class=ModelSqlParseError, ) diff --git a/src/sqlbuild/compiler/discovery/_helpers/sql/hooks.py b/src/sqlbuild/compiler/discovery/_helpers/sql/hooks.py index 7802abec2c..cd2c7e473d 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/sql/hooks.py +++ b/src/sqlbuild/compiler/discovery/_helpers/sql/hooks.py @@ -107,6 +107,7 @@ def _parse_hook_header(*, header: str, header_line: int, file_path: Path) -> dic header=header, file_path=file_path, statement_name="HOOK", + header_line=header_line, error_class=SqlHookParseError, ) reject_unsupported_header_keys( diff --git a/src/sqlbuild/compiler/discovery/_helpers/sql/model_files.py b/src/sqlbuild/compiler/discovery/_helpers/sql/model_files.py index c69a97c45b..9da71444c9 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/sql/model_files.py +++ b/src/sqlbuild/compiler/discovery/_helpers/sql/model_files.py @@ -5,7 +5,8 @@ import re from bisect import bisect_right from collections import OrderedDict -from dataclasses import dataclass +from collections.abc import Callable +from dataclasses import dataclass, field from pathlib import Path from typing import cast @@ -52,6 +53,10 @@ _MODEL_HEADER_INTEGER_PATTERN: re.Pattern[str] = re.compile(r"^[+-]?\d+$") _MODEL_HEADER_FLOAT_PATTERN: re.Pattern[str] = re.compile(r"^[+-]?(?:\d+\.\d*|\d*\.\d+)$") _MODEL_HEADER_TOKEN_CACHE_SIZE: int = 4096 +_PROJECTION_PENDING: object = object() +_MODEL_HEADER_NESTING_ERROR_PATTERN: re.Pattern[str] = re.compile( + r"values nest deeper than (?P\d+) levels at position (?P\d+)" +) _NATIVE_MODEL_HEADER_END_TOKEN: int = 0 _NATIVE_MODEL_HEADER_WORD_TOKEN: int = 1 _NATIVE_MODEL_HEADER_STRING_TOKEN: int = 2 @@ -76,6 +81,15 @@ _SELECT_SCAN_SPECIAL: re.Pattern[str] = re.compile(r"['\"`()sSfFuU]") +@dataclass +class _ProjectionFrame: + """One container being projected: its native children, projected so far, and its builder.""" + + children: tuple[object, ...] + build: Callable[[list[object]], object] + projected: list[object] = field(default_factory=list) + + @dataclass(frozen=True) class _ModelHeaderToken: kind: str @@ -556,9 +570,10 @@ def parse_header_values( header: str, file_path: Path, statement_name: str, + header_line: int | None, error_class: type[DiscoveryError] = ModelSqlParseError, ) -> dict[str, object]: - """Parse one SQLBuild parenthesized header into nested Python values.""" + """Parse one header; a nesting error names its file line when `header_line` is known.""" try: parsed: _ModelHeaderTokenization = _model_header_parse(header) @@ -570,9 +585,23 @@ def parse_header_values( except ModelSqlParseError: raise except ModelHeaderSyntaxError as error: + nesting: re.Match[str] | None = _MODEL_HEADER_NESTING_ERROR_PATTERN.fullmatch(str(error)) + nesting_help: str | None = ( + None + if nesting is None + else f"flatten the value so it nests at most {nesting.group('limit')} levels deep" + ) + if nesting is None or header_line is None: + raise error_class( + f"{statement_name}(...) in '{file_path}' contains invalid SQLBuild header syntax: " + f"{error}", + help=nesting_help, + ) from error + line: int = header_line + header.count("\n", 0, int(nesting.group("position"))) raise error_class( - f"{statement_name}(...) in '{file_path}' contains invalid SQLBuild header syntax: " - f"{error}" + f"{statement_name}(...) in '{file_path}:{line}' contains invalid SQLBuild header " + f"syntax: values nest deeper than {nesting.group('limit')} levels", + help=nesting_help, ) from error @@ -640,34 +669,58 @@ def project_native_header_values(values: dict[str, object]) -> dict[str, object] def _project_native_header_map(values: dict[str, object]) -> dict[str, object]: - return {key: _project_native_header_value(value) for key, value in values.items()} + return cast(dict[str, object], _project_native_header_value(values)) def _project_native_header_value(value: object) -> object: + """Project one native value depth-first with an explicit stack, never Python recursion.""" + + frames: list[_ProjectionFrame] = [] + node: object = value + while True: + expansion: _ProjectionFrame | None = _projection_frame(node) + if expansion is None: + projected: object = _project_native_leaf(node) + else: + frames.append(expansion) + projected = _PROJECTION_PENDING + while True: + if projected is not _PROJECTION_PENDING: + if not frames: + return projected + frames[-1].projected.append(projected) + frame: _ProjectionFrame = frames[-1] + if len(frame.projected) < len(frame.children): + node = frame.children[len(frame.projected)] + break + frames.pop() + projected = frame.build(frame.projected) + + +def _projection_frame(value: object) -> _ProjectionFrame | None: if isinstance(value, dict): - return _project_native_header_map(cast(dict[str, object], value)) + return _map_frame(values=cast(dict[str, object], value), build=dict) if isinstance(value, list): - return [_project_native_header_value(item) for item in value] + return _ProjectionFrame(children=tuple(value), build=list) if ( not isinstance(value, tuple) or len(value) != _NATIVE_MARKER_LENGTH or not isinstance(value[0], str) ): - return value + return None kind: str = value[0] payload: object = value[1] - if kind == _MODEL_HEADER_WORD_TOKEN and isinstance(payload, str): - return _parse_word_value(payload) if kind == _NATIVE_SET_MARKER and isinstance(payload, list): - return AuthoredSqlSet(tuple(_project_native_header_value(item) for item in payload)) + return _ProjectionFrame( + children=tuple(payload), build=lambda items: AuthoredSqlSet(tuple(items)) + ) if kind == _NATIVE_TUPLE_MARKER and isinstance(payload, list): - return tuple(_project_native_header_value(item) for item in payload) + return _ProjectionFrame(children=tuple(payload), build=tuple) if kind == _NATIVE_CONSTANT_MARKER and isinstance(payload, dict): - return AuthoredSqlValueCall( - arguments=tuple(_project_native_header_map(cast(dict[str, object], payload)).items()) + return _map_frame( + values=cast(dict[str, object], payload), + build=lambda items: AuthoredSqlValueCall(arguments=tuple(items)), ) - if kind == _NATIVE_INLINE_SQL_MARKER and isinstance(payload, str): - return SqlHookEntry(statement=payload) if ( kind in {_NATIVE_SQL_MARKER, _NATIVE_PYTHON_MARKER} and isinstance(payload, tuple) @@ -675,12 +728,39 @@ def _project_native_header_value(value: object) -> object: ): name, kwargs = payload if isinstance(name, str) and isinstance(kwargs, dict): - projected_kwargs: dict[str, object] = _project_native_header_map( - cast(dict[str, object], kwargs) + entry_type: type[NamedSqlHookEntry] | type[PythonHookEntry] = ( + NamedSqlHookEntry if kind == _NATIVE_SQL_MARKER else PythonHookEntry ) - if kind == _NATIVE_SQL_MARKER: - return NamedSqlHookEntry(name=name, kwargs=projected_kwargs) - return PythonHookEntry(name=name, kwargs=projected_kwargs) + return _map_frame( + values=cast(dict[str, object], kwargs), + build=lambda items: entry_type(name=name, kwargs=dict(items)), + ) + return None + + +def _map_frame( + *, values: dict[str, object], build: Callable[[list[tuple[str, object]]], object] +) -> _ProjectionFrame: + keys: tuple[str, ...] = tuple(values) + return _ProjectionFrame( + children=tuple(values.values()), + build=lambda projected: build(list(zip(keys, projected, strict=True))), + ) + + +def _project_native_leaf(value: object) -> object: + if ( + not isinstance(value, tuple) + or len(value) != _NATIVE_MARKER_LENGTH + or not isinstance(value[0], str) + ): + return value + kind: str = value[0] + payload: object = value[1] + if kind == _MODEL_HEADER_WORD_TOKEN and isinstance(payload, str): + return _parse_word_value(payload) + if kind == _NATIVE_INLINE_SQL_MARKER and isinstance(payload, str): + return SqlHookEntry(statement=payload) raise ModelHeaderSyntaxError(f"Native MODEL header parser returned invalid '{kind}' marker") diff --git a/src/sqlbuild/lint/_helpers/native.py b/src/sqlbuild/lint/_helpers/native.py index 0187249572..3cdf968b2a 100644 --- a/src/sqlbuild/lint/_helpers/native.py +++ b/src/sqlbuild/lint/_helpers/native.py @@ -16,7 +16,7 @@ source_line_starts, source_position, ) -from sqlbuild.compiler.discovery.exceptions import ModelSqlParseError +from sqlbuild.compiler.discovery.exceptions import DiscoveryError, ModelSqlParseError from sqlbuild.lint.constants import ( CARRIAGE_RETURN_LINE_FEED, CLOSING_PAREN_CHARACTER, @@ -245,7 +245,7 @@ def _lint_header_values( message=f"{header.kind}() header could not be parsed: {error}", severity=VIOLATION_SEVERITY_FAULT, engine=LINT_ENGINE_SQLBUILD, - remediation=f"Correct the {header.kind}() header syntax.", + remediation=_header_parse_remediation(kind=header.kind, error=error), ), ) @@ -304,12 +304,18 @@ def _lint_header_values( return tuple(violations) +def _header_parse_remediation(*, kind: str, error: Exception) -> str: + help_text: str | None = error.help if isinstance(error, DiscoveryError) else None + return f"Correct the {kind}() header syntax." if help_text is None else help_text + + def _parse_header_values(*, kind: str, header_text: str) -> dict[str, object]: inner: str = _inner_header_text(header_text=header_text) return parse_header_values( header=inner, file_path=Path(_DESCRIPTION_SENTINEL_PATH), statement_name=kind, + header_line=None, ) 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 90da17fbf2..2e82b7c868 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 @@ -974,3 +974,22 @@ class SecondCompileStoreTestCase: description: str expected_second_matches_first: bool + + +@dataclass(frozen=True) +class DeepHeaderNestingCompileCase: + description: str + engine: str + path: str + prefix: str + suffix: str + depth: int + expected_fragments: tuple[str, ...] + + +@dataclass(frozen=True) +class DeepCallNestingCompileCase: + description: str + depth: int + engines: tuple[str, ...] + expected_fragments: tuple[str, ...] diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_deep_header_nesting.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_deep_header_nesting.py new file mode 100644 index 0000000000..62a291291b --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_deep_header_nesting.py @@ -0,0 +1,189 @@ +"""E2E coverage for deeply nested header values failing discovery instead of crashing it.""" + +from __future__ import annotations + +import subprocess +import time +from pathlib import Path + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.compile._test_types import ( + DeepCallNestingCompileCase, + DeepHeaderNestingCompileCase, +) +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project, run_sqb + +_PROJECT_CONFIG: str = 'name = "orders"\nadapter = "duckdb"\n' +_ORDERS_MODEL: str = "MODEL (description 'Orders.');\n\nSELECT 1 AS order_id\n" +_COMPILE_TIME_BOUND_SECONDS: float = 120.0 +_NESTING_HELP: str = "help: flatten the value so it nests at most 256 levels deep" +_ENGINES: tuple[str, ...] = ("python", "native", "native-preview") +_CONSTANT_CALL_NESTING: tuple[str, ...] = ( + "error[D013]", + "constants/limits.sql:3' contains invalid SQLBuild header syntax: " + "values nest deeper than 256 levels", + _NESTING_HELP, +) + + +@pytest.mark.parametrize( + "test_case", + [ + DeepHeaderNestingCompileCase( + description="python_model_1k", + engine="python", + path="models/orders.sql", + prefix="MODEL (\n description 'Orders.',\n tags ", + suffix=",\n);\n\nSELECT 1 AS order_id\n", + depth=1_000, + expected_fragments=( + "error[D002]", + "models/orders.sql:3' contains invalid SQLBuild header syntax: " + "values nest deeper than 256 levels", + _NESTING_HELP, + ), + ), + DeepHeaderNestingCompileCase( + description="native_constant_20k", + engine="native", + path="constants/limits.sql", + prefix="\nCONSTANT (\n name order_limits,\n value ", + suffix=",\n);\n", + depth=20_000, + expected_fragments=( + "error[D013]", + "constants/limits.sql:4' contains invalid SQLBuild header syntax: " + "values nest deeper than 256 levels", + _NESTING_HELP, + ), + ), + DeepHeaderNestingCompileCase( + description="native_preview_hook_100k", + engine="native-preview", + path="hooks/sql/record_refresh.sql", + prefix="HOOK (\n description ", + suffix=",\n);\n\nSELECT 1\n", + depth=100_000, + expected_fragments=( + "error[D014]", + "hooks/sql/record_refresh.sql:2' contains invalid SQLBuild header syntax: " + "values nest deeper than 256 levels", + _NESTING_HELP, + ), + ), + DeepHeaderNestingCompileCase( + description="python_hook_100k", + engine="python", + path="hooks/sql/record_refresh.sql", + prefix="HOOK (\n description ", + suffix=",\n);\n\nSELECT 1\n", + depth=100_000, + expected_fragments=( + "error[D014]", + "hooks/sql/record_refresh.sql:2' contains invalid SQLBuild header syntax: " + "values nest deeper than 256 levels", + _NESTING_HELP, + ), + ), + ], + ids=lambda case: case.description, +) +def test_given_deeply_nested_header_value_when_compiling_then_fails_with_coded_discovery_error( + test_case: DeepHeaderNestingCompileCase, tmp_path: Path +) -> None: + nested_value: str = "[" * test_case.depth + "1" + "]" * test_case.depth + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="orders", + repo_files={ + "sqlbuild_project.toml": _PROJECT_CONFIG, + "models/orders.sql": _ORDERS_MODEL, + test_case.path: test_case.prefix + nested_value + test_case.suffix, + }, + ) + + started: float = time.monotonic() + result: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", "compile"), + project_dir=project_dir, + env={"SQLBUILD_COMPILER_ENGINE": test_case.engine}, + ) + elapsed: float = time.monotonic() - started + + output: str = result.stdout + result.stderr + assert result.returncode == 1, output + assert all(fragment in output for fragment in test_case.expected_fragments), output + assert elapsed < _COMPILE_TIME_BOUND_SECONDS + + +@pytest.mark.parametrize( + "test_case", + [ + DeepCallNestingCompileCase( + description="calls_240", + depth=240, + engines=_ENGINES, + expected_fragments=_CONSTANT_CALL_NESTING, + ), + DeepCallNestingCompileCase( + description="calls_256", + depth=256, + engines=_ENGINES, + expected_fragments=_CONSTANT_CALL_NESTING, + ), + DeepCallNestingCompileCase( + description="calls_257", + depth=257, + engines=_ENGINES, + expected_fragments=_CONSTANT_CALL_NESTING, + ), + DeepCallNestingCompileCase( + description="calls_20k", + depth=20_000, + engines=_ENGINES, + expected_fragments=_CONSTANT_CALL_NESTING, + ), + ], + ids=lambda case: case.description, +) +def test_given_deeply_nested_constant_calls_when_compiling_with_each_engine_then_fails_coded( + test_case: DeepCallNestingCompileCase, tmp_path: Path +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="orders", + repo_files={ + "sqlbuild_project.toml": _PROJECT_CONFIG, + "models/orders.sql": _ORDERS_MODEL, + "constants/limits.sql": ( + "CONSTANT (\n name order_limits,\n value " + + "limit(level " * test_case.depth + + "1" + + ")" * test_case.depth + + ",\n);\n" + ), + }, + ) + + started: float = time.monotonic() + results: list[subprocess.CompletedProcess[str]] = [ + run_sqb( + command=("--no-color", "compile"), + project_dir=project_dir, + env={"SQLBUILD_COMPILER_ENGINE": engine}, + ) + for engine in test_case.engines + ] + elapsed: float = time.monotonic() - started + + outputs: list[str] = [result.stdout + result.stderr for result in results] + errors: list[str] = [output[output.find("error[") :] for output in outputs] + assert [result.returncode for result in results] == [1] * len(test_case.engines), outputs + assert errors == [errors[0]] * len(test_case.engines), outputs + assert all(fragment in errors[0] for fragment in test_case.expected_fragments), outputs + assert elapsed < _COMPILE_TIME_BOUND_SECONDS * len(test_case.engines) + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/integration/src/sqlbuild/compiler/discovery/_test_types.py b/tests/integration/src/sqlbuild/compiler/discovery/_test_types.py index 03b2960fe3..43308c43f7 100644 --- a/tests/integration/src/sqlbuild/compiler/discovery/_test_types.py +++ b/tests/integration/src/sqlbuild/compiler/discovery/_test_types.py @@ -248,3 +248,29 @@ class TolerantFailureTextTestCase: description: str files: tuple[tuple[str, bytes], ...] expected_fault_keys: tuple[str, ...] + + +@dataclass(frozen=True) +class DeepDeclarationHeaderTestCase: + """A declaration file whose header nests one value `depth` levels deep.""" + + description: str + kind: str + relative_path: str + prefix: str + suffix: str + depth: int + expected_failure_type: str + expected_message_suffix: str + expected_help: str = "flatten the value so it nests at most 256 levels deep" + + +@dataclass(frozen=True) +class DeepHeaderValuesTestCase: + """A model header whose values reach the nesting limit, discovered on a deep Python stack.""" + + description: str + header: str + stack_frames: int + expected_engines: tuple[str, ...] + expected_max_depth: int diff --git a/tests/integration/src/sqlbuild/compiler/discovery/helpers.py b/tests/integration/src/sqlbuild/compiler/discovery/helpers.py index f3d8f9c92a..f9b725c006 100644 --- a/tests/integration/src/sqlbuild/compiler/discovery/helpers.py +++ b/tests/integration/src/sqlbuild/compiler/discovery/helpers.py @@ -2,7 +2,10 @@ from __future__ import annotations +import dataclasses import datetime +import functools +import itertools import json import math import os @@ -53,11 +56,15 @@ DiscoveredEnumFile, DiscoveredProjectInputs, DiscoveryFileFault, + NamedSqlHookEntry, + PythonHookEntry, + SqlHookEntry, TolerantScopeDiscovery, ) from sqlbuild.compiler.frontier._helpers.stage_capture import render_stage_capture from sqlbuild.compiler.frontier.constants import COMPILER_ENGINE_ENV_VAR from sqlbuild.spec.contracts.models import LocalConfig, ProjectConfig +from sqlbuild.sql_values.models import AuthoredSqlSet, AuthoredSqlValueCall from tests.integration.src.sqlbuild.compiler.discovery._test_types import ( GeneratedDeclarationFileTestCase, ) @@ -1284,3 +1291,56 @@ def write_undecodable_hook(*, project_dir: Path, contents: bytes) -> None: hooks: Path = project_dir / "hooks" / "sql" hooks.mkdir(parents=True) _ = Path(os.fsdecode(bytes(hooks) + b"/refresh_\xff.sql")).write_bytes(contents) + + +def on_deep_stack(*, frames: int, call: Callable[[], object]) -> object: + """Return `call()` invoked beneath `frames` extra Python frames.""" + + nested: Callable[[], object] = functools.reduce( + lambda inner, _: lambda: inner(), range(frames), call + ) + return nested() + + +def _dataclass_children(node: object) -> list[object]: + return [getattr(node, field.name) for field in dataclasses.fields(cast(Any, node))] + + +_SHAPE_CHILDREN: dict[type, Callable[[Any], list[object]]] = { + dict: lambda node: list(itertools.chain.from_iterable(node.items())), + list: list, + tuple: list, + AuthoredSqlSet: _dataclass_children, + AuthoredSqlValueCall: _dataclass_children, + NamedSqlHookEntry: _dataclass_children, + PythonHookEntry: _dataclass_children, + SqlHookEntry: _dataclass_children, +} +_SHAPE_LABELS: dict[type, Callable[[Any], str]] = dict.fromkeys(_SHAPE_CHILDREN, lambda _: "") + + +def value_shape(value: object) -> tuple[tuple[int, str], ...]: + """Each node of `value` as its depth and type or scalar text, walked without recursion.""" + + shape: list[tuple[int, str]] = [] + pending: list[tuple[int, object]] = [(0, value)] + while pending: + depth, node = pending.pop() + children: list[object] = _SHAPE_CHILDREN.get(type(node), lambda _: [])(node) + label: str = _SHAPE_LABELS.get(type(node), repr)(node) + shape.append((depth, f"{type(node).__name__}:{label}")) + pending.extend((depth + 1, child) for child in reversed(children)) + return tuple(shape) + + +def deep_model_header_values( + *, project_dir: Path, engine: str, monkeypatch: pytest.MonkeyPatch, frames: int +) -> dict[str, object]: + """Discover the project's only model under `engine` beneath `frames` extra Python frames.""" + + monkeypatch.setenv(COMPILER_ENGINE_ENV_VAR, engine) + inputs: DiscoveredProjectInputs = cast( + DiscoveredProjectInputs, + on_deep_stack(frames=frames, call=lambda: discover_project_inputs(project_dir=project_dir)), + ) + return inputs.model_files[0].header_values diff --git a/tests/integration/src/sqlbuild/compiler/discovery/test_deep_header_values.py b/tests/integration/src/sqlbuild/compiler/discovery/test_deep_header_values.py new file mode 100644 index 0000000000..669e057847 --- /dev/null +++ b/tests/integration/src/sqlbuild/compiler/discovery/test_deep_header_values.py @@ -0,0 +1,78 @@ +"""Header values at the nesting limit project identically on a deep Python stack.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from sqlbuild.compiler.discovery._helpers.sql.model_files import parse_header_values +from tests.integration.src.sqlbuild.compiler.discovery._test_types import ( + DeepHeaderValuesTestCase, +) +from tests.integration.src.sqlbuild.compiler.discovery.helpers import ( + deep_model_header_values, + on_deep_stack, + value_shape, + write_project, +) + +_AT_LIMIT_HEADER: str = ( + "\n description 'Orders.',\n" + f" tags {'[' * 256}'placed'{']' * 256},\n" + f" config {'(level ' * 256}1{')' * 256},\n" + f" placeholders {'call(level ' * 128}1{')' * 128},\n" + f" audits {'{' * 256}1{'}' * 256},\n" + f" row_diff_tolerances {'constant(value ' * 256}1{')' * 256},\n" + f" post_hooks [{'sql("refresh", level: ' * 127}1{')' * 127}],\n" +) + + +@pytest.mark.parametrize( + "test_case", + [ + DeepHeaderValuesTestCase( + description="every container kind at the limit beneath 300 extra frames", + header=_AT_LIMIT_HEADER, + stack_frames=300, + expected_engines=("python", "native", "native-preview"), + expected_max_depth=769, + ) + ], + ids=lambda case: case.description, +) +def test_given_header_values_at_the_limit_when_discovering_on_a_deep_stack_then_engines_agree( + test_case: DeepHeaderValuesTestCase, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + write_project( + project_dir=tmp_path, + files=(("models/orders.sql", f"MODEL ({test_case.header});\nSELECT 1 AS id\n".encode()),), + ) + parsed: object = on_deep_stack( + frames=test_case.stack_frames, + call=lambda: parse_header_values( + header=test_case.header, + file_path=tmp_path / "models/orders.sql", + statement_name="MODEL", + header_line=1, + ), + ) + + shapes: list[tuple[tuple[int, str], ...]] = [ + value_shape( + deep_model_header_values( + project_dir=tmp_path, + engine=engine, + monkeypatch=monkeypatch, + frames=test_case.stack_frames, + ) + ) + for engine in test_case.expected_engines + ] + + assert shapes == [value_shape(parsed)] * len(test_case.expected_engines) + assert max(depth for depth, _ in shapes[0]) == test_case.expected_max_depth + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/integration/src/sqlbuild/compiler/discovery/test_native_declaration_files.py b/tests/integration/src/sqlbuild/compiler/discovery/test_native_declaration_files.py index a6e57f3960..ac76077d7e 100644 --- a/tests/integration/src/sqlbuild/compiler/discovery/test_native_declaration_files.py +++ b/tests/integration/src/sqlbuild/compiler/discovery/test_native_declaration_files.py @@ -21,13 +21,16 @@ DeclarationFilesParityTestCase, DeclarationMismatchTestCase, DeclarationReuseTestCase, + DeepDeclarationHeaderTestCase, GeneratedDeclarationFileTestCase, NativeSessionTestCase, TolerantDeclarationFilesTestCase, ) from tests.integration.src.sqlbuild.compiler.discovery.helpers import ( accept_any_declarations, + declaration_files_outcome, generated_declaration_outcomes, + native_declaration_tag, reject_any_contents, stage_outcome, tolerant_declaration_outcome, @@ -36,6 +39,7 @@ ) _PREVIEW: str = "native-preview" +_NESTING_SUFFIX: str = "contains invalid SQLBuild header syntax: values nest deeper than 256 levels" _MODEL: bytes = b'MODEL (description "Orders");\nSELECT 1 AS order_id' _MACRO: bytes = b"def cents(value):\n return f'{value} * 100'\n" _ENUMS: bytes = ( @@ -289,6 +293,76 @@ def test_given_declaration_files_when_discovering_through_the_engine_switch_then assert test_case.expected_message_fragment in str(native[2]) +@pytest.mark.parametrize( + "test_case", + [ + DeepDeclarationHeaderTestCase( + description="a constant value nested 1k levels", + kind="constant", + relative_path="constants/limits.sql", + prefix="CONSTANT (name cap, value 3);\nCONSTANT (\n name regions,\n value ", + suffix=",\n);\n", + depth=1_000, + expected_failure_type="DeclarationParseError", + expected_message_suffix=f"limits.sql:4' {_NESTING_SUFFIX}", + ), + DeepDeclarationHeaderTestCase( + description="a hook description nested 20k levels", + kind="sql_hook", + relative_path="hooks/sql/refresh.sql", + prefix="HOOK (\n description ", + suffix=",\n);\n\nSELECT 1\n", + depth=20_000, + expected_failure_type="SqlHookParseError", + expected_message_suffix=f"refresh.sql:2' {_NESTING_SUFFIX}", + ), + DeepDeclarationHeaderTestCase( + description="a function return type nested 100k levels", + kind="sql_function", + relative_path="functions/sql/doubled.sql", + prefix='FUNCTION (\n description "Doubles",\n returns ', + suffix=",\n);\nSELECT 2\n", + depth=100_000, + expected_failure_type="ModelSqlParseError", + expected_message_suffix=f"doubled.sql:3' {_NESTING_SUFFIX}", + ), + DeepDeclarationHeaderTestCase( + description="a later audit block's value nested 100k levels", + kind="audit", + relative_path="audits/generic/is_true.sql", + prefix="AUDIT (name first);\nSELECT 1\nAUDIT (\n name second,\n value ", + suffix=",\n);\nSELECT 2\n", + depth=100_000, + expected_failure_type="SqlAuditParseError", + expected_message_suffix=f"is_true.sql:5' {_NESTING_SUFFIX}", + ), + ], + ids=lambda case: case.description, +) +def test_given_deeply_nested_declaration_header_when_discovering_then_engines_report_its_line( + test_case: DeepDeclarationHeaderTestCase, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + nested: str = "[" * test_case.depth + "1" + "]" * test_case.depth + contents: str = test_case.prefix + nested + test_case.suffix + write_project(project_dir=tmp_path, files=((test_case.relative_path, contents.encode()),)) + python: tuple[object, ...] = declaration_files_outcome( + project_dir=tmp_path, kind=test_case.kind, engine="python", monkeypatch=monkeypatch + ) + + native: tuple[object, ...] = declaration_files_outcome( + project_dir=tmp_path, kind=test_case.kind, engine=_PREVIEW, monkeypatch=monkeypatch + ) + + assert native == python + assert native[0] == test_case.expected_failure_type + assert str(native[1]).endswith(test_case.expected_message_suffix), native[1] + assert native[2] == test_case.expected_help + assert ( + native_declaration_tag(file_path=tmp_path / test_case.relative_path, kind=test_case.kind) + == "error" + ) + + @pytest.mark.skipif(sys.platform != "linux", reason="needs file names that are not UTF-8") @pytest.mark.parametrize( "test_case", diff --git a/tests/unit/src/sqlbuild/lint/_helpers/_test_types.py b/tests/unit/src/sqlbuild/lint/_helpers/_test_types.py index d4385d6164..14309ea470 100644 --- a/tests/unit/src/sqlbuild/lint/_helpers/_test_types.py +++ b/tests/unit/src/sqlbuild/lint/_helpers/_test_types.py @@ -252,3 +252,10 @@ class SelectedProjectFilesTestCase: description: str selected: str expected_present: tuple[str, ...] + + +@dataclass(frozen=True) +class LintRemediationTestCase: + description: str + contents: str + expected_remediations: tuple[str, ...] diff --git a/tests/unit/src/sqlbuild/lint/_helpers/test_native.py b/tests/unit/src/sqlbuild/lint/_helpers/test_native.py index 8a9cdd162b..3c66ffd29c 100644 --- a/tests/unit/src/sqlbuild/lint/_helpers/test_native.py +++ b/tests/unit/src/sqlbuild/lint/_helpers/test_native.py @@ -18,6 +18,7 @@ FormatDescriptionTestCase, FormatNativeTestCase, LintNativeTestCase, + LintRemediationTestCase, ) FILE_PATH: Path = Path("models/example.sql") @@ -115,6 +116,37 @@ def test_given_contents_when_linting_then_codes_match_expected( assert tuple(violation.code for violation in violations) == test_case.expected_codes +@pytest.mark.parametrize( + "test_case", + [ + LintRemediationTestCase( + description="header nested past the limit suggests flattening it", + contents="MODEL (\n tags " + "[" * 20_000 + "1" + "]" * 20_000 + "\n);\nSELECT 1\n", + expected_remediations=("flatten the value so it nests at most 256 levels deep",), + ), + LintRemediationTestCase( + description="other header syntax errors suggest correcting the header", + contents="MODEL (\n materialized table,\n description\n);\nSELECT 1\n", + expected_remediations=("Correct the MODEL() header syntax.",), + ), + ], + ids=lambda case: case.description, +) +def test_given_unparseable_header_when_linting_then_remediation_matches_the_error( + test_case: LintRemediationTestCase, +) -> None: + violations: tuple = lint_native_headers( + contents=test_case.contents, + file_path=FILE_PATH, + headers=scan_headers(contents=test_case.contents), + config=DEFAULT_CONFIG, + ) + + assert tuple(violation.remediation for violation in violations) == ( + test_case.expected_remediations + ) + + @pytest.mark.parametrize( "test_case", [