From 5ac188742597d53b997b3300e0e22d55545c488a Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 8 Oct 2026 01:30:51 +0100 Subject: [PATCH 1/2] fix(native): report deeply nested header values instead of crashing The native header value parser recursed once per nesting level, so a header value nested about 20,000 levels deep overflowed the stack and crashed discovery under every compiler engine. The parser now refuses to open more than 256 nested containers, the same limit the YAML loader uses. Discovery reports it under each statement's existing header syntax code, naming the file line of the container that is too deep, with help on how to fix it. The in-memory parse entry points run on the discovery worker pool. --- .../src/_helpers/header_syntax.rs | 36 ++++ crates/sqlbuild-discovery/src/_helpers/mod.rs | 1 + .../sqlbuild-discovery/src/_helpers/pool.rs | 10 +- .../src/_helpers/statement_headers.rs | 27 ++- .../_helpers/parsing/declaration_headers.rs | 29 ++- .../main/parse_declaration_text.rs | 12 +- .../src/declaration_files/tests/helpers.rs | 15 ++ .../tests/test_parse_declaration_text.rs | 59 +++++- .../src/declaration_files/tests/test_types.rs | 12 ++ .../src/model_files/_helpers/parsing.rs | 23 ++- .../src/model_files/main/parse_model_text.rs | 5 +- .../tests/test_discover_model_files.rs | 45 ++++- .../src/model_files/tests/test_types.rs | 7 + .../src/sql_tests/main/parse_scenario_text.rs | 5 +- .../src/sql_tests/main/parse_sql_test_text.rs | 5 +- .../src/sql_tests/tests/helpers.rs | 12 ++ .../sql_tests/tests/test_sql_test_files.rs | 39 +++- .../src/sql_tests/tests/test_types.rs | 8 + .../_helpers/model_headers/tokenization.rs | 154 ++++++++++---- .../sqlbuild-sqltext/src/compiler/main/mod.rs | 1 + .../src/compiler/main/model_header_errors.rs | 11 + .../main/model_header_single_parsing.rs | 2 +- .../sqlbuild-sqltext/src/compiler/models.rs | 10 + crates/sqlbuild-sqltext/src/compiler/tests.rs | 2 + .../src/compiler/tests/helpers.rs | 32 +++ .../tests/test_model_header_nesting.rs | 191 ++++++++++++++++++ .../src/compiler/tests/test_types.rs | 22 ++ crates/sqlbuild-sqltext/src/constants.rs | 2 + .../corpus/discovery_failure_cases.py | 23 +++ .../compiler/discovery/_helpers/sql/audits.py | 1 + .../discovery/_helpers/sql/declarations.py | 3 +- .../discovery/_helpers/sql/functions.py | 4 +- .../compiler/discovery/_helpers/sql/hooks.py | 1 + .../discovery/_helpers/sql/model_files.py | 19 +- src/sqlbuild/lint/_helpers/native.py | 1 + .../cli/commands/main/compile/_test_types.py | 11 + .../main/compile/test_deep_header_nesting.py | 113 +++++++++++ .../compiler/discovery/_test_types.py | 15 ++ .../test_native_declaration_files.py | 74 +++++++ 39 files changed, 952 insertions(+), 90 deletions(-) create mode 100644 crates/sqlbuild-discovery/src/_helpers/header_syntax.rs create mode 100644 crates/sqlbuild-sqltext/src/compiler/main/model_header_errors.rs create mode 100644 crates/sqlbuild-sqltext/src/compiler/tests/test_model_header_nesting.rs create mode 100644 tests/e2e/src/sqlbuild/cli/commands/main/compile/test_deep_header_nesting.py 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..7a978510db 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,79 @@ 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.1, 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 `name(`, which has been consumed. + fn parse_call( + &mut self, + name: String, + 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); + } + if name == CONSTANT_CALL { + return Ok(AuthoredValue::TypedConstant(self.parse_map( + ParseMapOptions { + end: Some(")"), + ..ParseMapOptions::default() + }, + )?)); + } + Ok(AuthoredValue::Map(vec![( + name, + AuthoredValue::Map(self.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() + })?), + )])) + } + + /// 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 +290,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 +334,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)) } fn parse_relation(&mut self, name: &str) -> Result { @@ -416,6 +454,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..99d919ff61 --- /dev/null +++ b/crates/sqlbuild-sqltext/src/compiler/tests/test_model_header_nesting.rs @@ -0,0 +1,191 @@ +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 20k deep", + prefix: "meta ", + open: "call(key ", + close: ")", + suffix: "", + depth: 20_000, + expected_error_position: Some(2_309), + }, + 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 and each hook", + prefix: "post_hooks [", + open: "sql(\"refresh\", nested: ", + close: ")", + suffix: "]", + depth: 100_000, + expected_error_position: Some(5_877), + }, + ]; + + 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..67bd4a8904 100644 --- a/scripts/compiler_differential/_helpers/corpus/discovery_failure_cases.py +++ b/scripts/compiler_differential/_helpers/corpus/discovery_failure_cases.py @@ -32,6 +32,9 @@ " 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_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 +106,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 +160,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_VALUE});\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..be070e3d68 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/sql/model_files.py +++ b/src/sqlbuild/compiler/discovery/_helpers/sql/model_files.py @@ -52,6 +52,9 @@ _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 +_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 @@ -556,9 +559,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 +574,18 @@ def parse_header_values( except ModelSqlParseError: raise except ModelHeaderSyntaxError as error: + nesting: re.Match[str] | None = _MODEL_HEADER_NESTING_ERROR_PATTERN.fullmatch(str(error)) + 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}" + ) from error + limit: str = nesting.group("limit") + 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 {limit} levels", + help=f"flatten the value so it nests at most {limit} levels deep", ) from error diff --git a/src/sqlbuild/lint/_helpers/native.py b/src/sqlbuild/lint/_helpers/native.py index 0187249572..ce1975372d 100644 --- a/src/sqlbuild/lint/_helpers/native.py +++ b/src/sqlbuild/lint/_helpers/native.py @@ -310,6 +310,7 @@ def _parse_header_values(*, kind: str, header_text: str) -> dict[str, object]: 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..30723cd8a8 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,14 @@ 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, ...] 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..ef9eb0ba0d --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_deep_header_nesting.py @@ -0,0 +1,113 @@ +"""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 ( + 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" + + +@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 + + +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..5c9e0ab566 100644 --- a/tests/integration/src/sqlbuild/compiler/discovery/_test_types.py +++ b/tests/integration/src/sqlbuild/compiler/discovery/_test_types.py @@ -248,3 +248,18 @@ 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" 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", From ba15cf7aef50c847a711fd64eb9fbe504bf85066 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 8 Oct 2026 02:20:47 +0100 Subject: [PATCH 2/2] fix(native): project nested header values without Python recursion Python projected native header values recursively, spending about four frames per call level, so call syntax nested 231 to 256 levels deep raised RecursionError in every engine. The projection now walks values with an explicit stack, and a plain call counts as the two levels it builds (a hook's arguments count as one more), so the 256-level limit measures the values discovery actually produces. Lint suggests the same flatten help for a header nested too deeply. --- .../_helpers/model_headers/tokenization.rs | 32 ++--- .../tests/test_model_header_nesting.rs | 24 +++- .../corpus/discovery_failure_cases.py | 3 +- .../discovery/_helpers/sql/model_files.py | 111 ++++++++++++++---- src/sqlbuild/lint/_helpers/native.py | 9 +- .../cli/commands/main/compile/_test_types.py | 8 ++ .../main/compile/test_deep_header_nesting.py | 76 ++++++++++++ .../compiler/discovery/_test_types.py | 11 ++ .../sqlbuild/compiler/discovery/helpers.py | 60 ++++++++++ .../discovery/test_deep_header_values.py | 78 ++++++++++++ .../src/sqlbuild/lint/_helpers/_test_types.py | 7 ++ .../src/sqlbuild/lint/_helpers/test_native.py | 32 +++++ 12 files changed, 409 insertions(+), 42 deletions(-) create mode 100644 tests/integration/src/sqlbuild/compiler/discovery/test_deep_header_values.py 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 7a978510db..0afb9098a4 100644 --- a/crates/sqlbuild-sqltext/src/compiler/_helpers/model_headers/tokenization.rs +++ b/crates/sqlbuild-sqltext/src/compiler/_helpers/model_headers/tokenization.rs @@ -201,7 +201,7 @@ impl HeaderParser { self.advance(); if self.peek().0 == SYMBOL_TOKEN && self.peek().1 == OPEN_PAREN { self.advance(); - return self.nested(token.2, |parser| parser.parse_call(token.1, options)); + return self.nested(token.2, |parser| parser.parse_call(token, options)); } return Ok(parse_word(token.1)); } @@ -230,17 +230,17 @@ impl HeaderParser { Err(format!("expected value at position {}", token.2)) } - /// The value after `name(`, which has been consumed. + /// The value after a consumed `name(`; a plain call nests a map in a map, two levels. fn parse_call( &mut self, - name: String, + (_, 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); + return self.parse_hook(&name, position); } if name == CONSTANT_CALL { return Ok(AuthoredValue::TypedConstant(self.parse_map( @@ -250,23 +250,26 @@ impl HeaderParser { }, )?)); } - Ok(AuthoredValue::Map(vec![( - name, - AuthoredValue::Map(self.parse_map(ParseMapOptions { + 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, ..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( + fn nested( &mut self, position: usize, - parse: impl FnOnce(&mut Self) -> Result, - ) -> Result { + parse: impl FnOnce(&mut Self) -> Result, + ) -> Result { if self.depth >= MAX_HEADER_NESTING_DEPTH { return Err(nesting_error(position)); } @@ -334,7 +337,7 @@ impl HeaderParser { if !matches!(token.1.as_str(), "inline_sql" | "sql" | "python") { return Err(error()); } - self.nested(token.2, |parser| parser.parse_hook(&token.1)) + self.nested(token.2, |parser| parser.parse_hook(&token.1, token.2)) } fn parse_relation(&mut self, name: &str) -> Result { @@ -352,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 { @@ -373,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 { 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 index 99d919ff61..05211581ba 100644 --- a/crates/sqlbuild-sqltext/src/compiler/tests/test_model_header_nesting.rs +++ b/crates/sqlbuild-sqltext/src/compiler/tests/test_model_header_nesting.rs @@ -84,6 +84,24 @@ fn given_nested_header_values_when_parsing_on_an_ordinary_thread_then_depth_is_c 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 ", @@ -91,7 +109,7 @@ fn given_nested_header_values_when_parsing_on_an_ordinary_thread_then_depth_is_c close: ")", suffix: "", depth: 20_000, - expected_error_position: Some(2_309), + expected_error_position: Some(1_157), }, HeaderNestingTestCase { description: "typed constants 1k deep", @@ -103,13 +121,13 @@ fn given_nested_header_values_when_parsing_on_an_ordinary_thread_then_depth_is_c expected_error_position: Some(3_845), }, HeaderNestingTestCase { - description: "hook arguments 100k deep count the hook list and each hook", + 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(5_877), + expected_error_position: Some(2_933), }, ]; diff --git a/scripts/compiler_differential/_helpers/corpus/discovery_failure_cases.py b/scripts/compiler_differential/_helpers/corpus/discovery_failure_cases.py index 67bd4a8904..d3fed8b35e 100644 --- a/scripts/compiler_differential/_helpers/corpus/discovery_failure_cases.py +++ b/scripts/compiler_differential/_helpers/corpus/discovery_failure_cases.py @@ -34,6 +34,7 @@ _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" @@ -165,7 +166,7 @@ def _sql_file_cases() -> tuple[FailureCase, ...]: expected_code="D013", files={ "models/staging/_sqlbuild/_constants/statuses.sql": ( - f"CONSTANT (name kept_statuses, value {_DEEP_HEADER_VALUE});\n" + f"CONSTANT (name kept_statuses, value {_DEEP_HEADER_CALLS});\n" ) }, expected_message=_DEEP_HEADER_MESSAGE, diff --git a/src/sqlbuild/compiler/discovery/_helpers/sql/model_files.py b/src/sqlbuild/compiler/discovery/_helpers/sql/model_files.py index be070e3d68..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,7 @@ _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+)" ) @@ -79,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 @@ -575,17 +586,22 @@ def parse_header_values( 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}" + f"{error}", + help=nesting_help, ) from error - limit: str = nesting.group("limit") line: int = header_line + header.count("\n", 0, int(nesting.group("position"))) raise error_class( f"{statement_name}(...) in '{file_path}:{line}' contains invalid SQLBuild header " - f"syntax: values nest deeper than {limit} levels", - help=f"flatten the value so it nests at most {limit} levels deep", + f"syntax: values nest deeper than {nesting.group('limit')} levels", + help=nesting_help, ) from error @@ -653,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) @@ -688,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 ce1975372d..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,6 +304,11 @@ 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( 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 30723cd8a8..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 @@ -985,3 +985,11 @@ class DeepHeaderNestingCompileCase: 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 index ef9eb0ba0d..62a291291b 100644 --- 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 @@ -9,6 +9,7 @@ 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 @@ -17,6 +18,13 @@ _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( @@ -109,5 +117,73 @@ def test_given_deeply_nested_header_value_when_compiling_then_fails_with_coded_d 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 5c9e0ab566..43308c43f7 100644 --- a/tests/integration/src/sqlbuild/compiler/discovery/_test_types.py +++ b/tests/integration/src/sqlbuild/compiler/discovery/_test_types.py @@ -263,3 +263,14 @@ class DeepDeclarationHeaderTestCase: 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/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", [