From c2130801a8b39fdecb9b43f1baf2a9d89d56c5a7 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 1 Oct 2026 19:30:27 +0100 Subject: [PATCH 1/5] fix(compiler)!: reject unknown MODEL, model-default and source keys MODEL() headers, [defaults], [path_defaults.*] and sources/*.yml silently ignored unknown keys, so a typo or a removed setting had no effect. Every statement header (MODEL, FUNCTION, AUDIT, TEST, HOOK, SCENARIO, @udf) now shares one unsupported-keys error that names the key with its file and line and suggests the nearest supported key. Custom materialization values stay inside the free-form config block. BREAKING CHANGE: unknown keys in MODEL() headers, project defaults, path defaults and sources files now fail compilation. --- .../_helpers/model_headers/tokenization.rs | 31 ++- .../src/compiler/tests/helpers.rs | 41 ++++ .../tests/test_model_header_tokenization.rs | 8 +- .../docs/concepts/models/configuration.md | 24 +++ .../docs/concepts/project-configuration.md | 19 +- .../references/docs/concepts/sources.md | 11 +- .../compile/_helpers/attachment/core.py | 11 -- .../_helpers/attachment/model_config.py | 10 +- .../_helpers/integrations/loaders.py | 22 +++ .../discovery/_helpers/python/functions.py | 10 +- .../compiler/discovery/_helpers/sql/audits.py | 24 ++- .../discovery/_helpers/sql/functions.py | 27 +-- .../discovery/_helpers/sql/header_keys.py | 63 +++++++ .../compiler/discovery/_helpers/sql/hooks.py | 30 +-- .../discovery/_helpers/sql/model_files.py | 22 ++- .../discovery/_helpers/sql/scenarios.py | 26 +-- .../compiler/discovery/_helpers/sql/tests.py | 55 +++--- .../_helpers/validation/supported_keys.py | 49 +++++ .../discovery/_helpers/yml/project.py | 50 ++++- .../compiler/discovery/_helpers/yml/schema.py | 16 +- .../discovery/_helpers/yml/sources.py | 66 +++++++ src/sqlbuild/compiler/discovery/constants.py | 72 +++++++ .../cli/commands/main/compile/_test_types.py | 7 + .../main/compile/test_unknown_config_keys.py | 129 +++++++++++++ .../commands/main/test_contract_command.py | 41 ++++ .../sqlbuild/compiler/compile/test_main.py | 22 --- .../discovery/_helpers/_test_types.py | 9 + .../compiler/discovery/_helpers/helpers.py | 24 +++ .../discovery/_helpers/test_sql_models.py | 28 ++- .../discovery/_helpers/test_sql_scenarios.py | 2 +- .../_helpers/test_statement_header_keys.py | 177 ++++++++++++++++++ .../discovery/_helpers/test_yml_sources.py | 66 +++++++ .../docs/concepts/models/configuration.mdx | 24 +++ .../docs/concepts/project-configuration.mdx | 19 +- .../content/docs/docs/concepts/sources.mdx | 11 +- 35 files changed, 1105 insertions(+), 141 deletions(-) create mode 100644 src/sqlbuild/compiler/discovery/_helpers/sql/header_keys.py create mode 100644 src/sqlbuild/compiler/discovery/_helpers/validation/supported_keys.py create mode 100644 tests/e2e/src/sqlbuild/cli/commands/main/compile/test_unknown_config_keys.py create mode 100644 tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_statement_header_keys.py diff --git a/crates/sqlbuild-rules-native/src/compiler/_helpers/model_headers/tokenization.rs b/crates/sqlbuild-rules-native/src/compiler/_helpers/model_headers/tokenization.rs index 2618202e88..7f0aa041c1 100644 --- a/crates/sqlbuild-rules-native/src/compiler/_helpers/model_headers/tokenization.rs +++ b/crates/sqlbuild-rules-native/src/compiler/_helpers/model_headers/tokenization.rs @@ -438,6 +438,10 @@ fn tokenize(header: &str) -> Result, String> { index += 1; continue; } + if starts_comment(&characters, index) { + index = comment_end(&characters, index)?; + continue; + } if starts_template(&characters, index) { let template_end = find_character(&characters, '}', index + 2) .ok_or_else(|| format!("unterminated template value at position {index}"))?; @@ -470,7 +474,11 @@ fn tokenize(header: &str) -> Result, String> { continue; } let next = characters[index]; - if is_python_whitespace(next) || is_symbol(next) || next == ':' { + if is_python_whitespace(next) + || is_symbol(next) + || next == ':' + || starts_comment(&characters, index) + { break; } if matches!(next, '\'' | '"') { @@ -518,6 +526,27 @@ fn read_quoted_string(characters: &[char], start: usize) -> Result<(String, usiz )) } +fn starts_comment(characters: &[char], index: usize) -> bool { + matches!( + (characters.get(index), characters.get(index + 1)), + (Some('-'), Some('-')) | (Some('/'), Some('*')) + ) +} + +fn comment_end(characters: &[char], start: usize) -> Result { + if characters[start] == '-' { + return Ok(find_character(characters, '\n', start).map_or(characters.len(), |end| end + 1)); + } + let mut index = start + 2; + while index + 1 < characters.len() { + if characters[index] == '*' && characters[index + 1] == '/' { + return Ok(index + 2); + } + index += 1; + } + Err(format!("unterminated block comment at position {start}")) +} + fn starts_template(characters: &[char], index: usize) -> bool { characters.get(index) == Some(&'$') && characters.get(index + 1) == Some(&'{') } diff --git a/crates/sqlbuild-rules-native/src/compiler/tests/helpers.rs b/crates/sqlbuild-rules-native/src/compiler/tests/helpers.rs index 9c187d1549..e2dc0549c6 100644 --- a/crates/sqlbuild-rules-native/src/compiler/tests/helpers.rs +++ b/crates/sqlbuild-rules-native/src/compiler/tests/helpers.rs @@ -144,6 +144,47 @@ pub(crate) fn invalid_headers_return_each_exact_error_in_order() -> bool { true } +pub(crate) fn header_comments_are_skipped_without_shifting_offsets() -> bool { + let headers = vec![ + "materialized table -- keep model metadata\n columns (id ()) /* it's, (fine) */" + .to_owned(), + "materialized table--trailing note\n".to_owned(), + "materialized table /* unclosed".to_owned(), + ]; + + let results = parse_batch(&headers).expect("worker pool builds"); + + let AuthoredValue::Map(values) = results[0].0.as_ref().expect("header values") else { + panic!("header values must be a map"); + }; + assert_eq!( + values + .iter() + .map(|(key, _)| key.as_str()) + .collect::>(), + vec!["materialized", "columns"] + ); + assert_eq!( + results[0].1.as_ref().expect("column offsets"), + &vec![("id".to_owned(), 53, 2)] + ); + let AuthoredValue::Map(values) = results[1].0.as_ref().expect("header values") else { + panic!("header values must be a map"); + }; + assert_eq!( + values + .iter() + .map(|(key, _)| key.as_str()) + .collect::>(), + vec!["materialized"] + ); + assert_eq!( + results[2].2.as_deref(), + Some("unterminated block comment at position 19") + ); + true +} + pub(crate) fn nested_and_root_columns_return_only_root_offsets() -> bool { let headers = vec![ "config (columns (nested (type INTEGER))), columns (top (type INTEGER))".to_owned(), diff --git a/crates/sqlbuild-rules-native/src/compiler/tests/test_model_header_tokenization.rs b/crates/sqlbuild-rules-native/src/compiler/tests/test_model_header_tokenization.rs index 577bf1dd46..0f1ab9bf9e 100644 --- a/crates/sqlbuild-rules-native/src/compiler/tests/test_model_header_tokenization.rs +++ b/crates/sqlbuild-rules-native/src/compiler/tests/test_model_header_tokenization.rs @@ -1,5 +1,6 @@ use crate::compiler::tests::helpers::{ - batch_sizes_bound_workers_by_contract, invalid_headers_return_each_exact_error_in_order, + batch_sizes_bound_workers_by_contract, header_comments_are_skipped_without_shifting_offsets, + invalid_headers_return_each_exact_error_in_order, nested_and_root_columns_return_only_root_offsets, nested_authored_headers_preserve_values_and_offsets, }; @@ -18,6 +19,11 @@ fn given_model_header_cases_when_exercising_native_parser_then_expected_behavior run: invalid_headers_return_each_exact_error_in_order, expected_success: true, }, + ModelHeaderTokenizationTestCase { + description: "line and block comments are skipped without shifting offsets", + run: header_comments_are_skipped_without_shifting_offsets, + expected_success: true, + }, ModelHeaderTokenizationTestCase { description: "nested columns do not leak into root offsets", run: nested_and_root_columns_return_only_root_offsets, diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/models/configuration.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/models/configuration.md index 661019e3aa..66c5d81e63 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/models/configuration.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/models/configuration.md @@ -16,6 +16,19 @@ Online: https://sqlbuild.com/docs/concepts/models/configuration/ - Custom materialization fields - Diff fields +Every `MODEL()` key must be one of the fields on this page. Any other key fails compilation with +`D002`, naming the key and its file and line and suggesting the nearest supported key. SQL comments +(`-- ...` and `/* ... */`) inside a header are ignored: + +```text +error[D002]: MODEL() in 'models/fct_orders.sql:3' has unsupported keys: descripton + = help: did you mean 'description'? +``` + +`[defaults]` and `[path_defaults.*]` in `sqlbuild_project.toml` reject unknown keys in the same way +with `D001`. Values that a custom materialization reads belong inside its `config (...)` block, +which accepts any keys. + ## Common fields | Field | Description | @@ -36,6 +49,8 @@ Online: https://sqlbuild.com/docs/concepts/models/configuration/ | `enabled` | Set to `false` to disable the model | | `contract` | `none` for an open statically checked declaration, or `enforced` for an exact declaration | | `sql_analysis` | Per-model SQL-analysis override | +| `dynamic_columns` | Named pivot column families with `pivot_column`, `value_column`, `aggregate`, `type`, and `name_pattern` | +| `audit_factories` | Audit factories that generate audit cases for this model; see [Audits](../audits.md) | ## SQL analysis @@ -54,6 +69,8 @@ configuration. | Field | Description | |-------|-------------| +| `table_type` | Snowflake table type: `permanent`, `transient`, or `inherit` | +| `time_travel_retention` | Managed time travel retention such as `7d`, `inherit`, or `disabled` | | `run_despite_unchanged` | Removed with virtual environments. It is rejected in MODEL headers, project defaults, and path defaults. | Table promotion mode is a project setting rather than a `MODEL()` field. Staged promotion is the default. Immediate promotion is incompatible with model type enforcement and exact contracts; see [Materializations](materializations.md#table). @@ -67,6 +84,11 @@ Table promotion mode is a project setting rather than a `MODEL()` field. Staged | `cursor_type` | `timestamp` or `integer` | | `cursor_grain` | Timestamp grain such as `second`, `hour`, or `day` | | `cursor_start` | Lower cursor bound | +| `cursor_end` | Upper cursor bound | +| `cursor_start_max_ahead` | Largest distance a discovered automatic start may sit ahead of the invocation time, or `disabled` | +| `cursor_start_max_action` | `cap` or `error` when the start exceeds `cursor_start_max_ahead` | +| `cursor_future_max_distance` | Largest distance a discovered cursor watermark may sit in the future, or `disabled` | +| `cursor_future_action` | `cap` or `error` when a watermark exceeds `cursor_future_max_distance` | | `cursor_inputs` | Upstream names mapped to cursor columns | | `unique_key` | Merge or delete/insert matching columns | | `incremental_mode` | Set to `microbatch` for batched execution | @@ -75,6 +97,8 @@ Table promotion mode is a project setting rather than a `MODEL()` field. Staged | `batch_size` | Timestamp duration string such as `1d` or `1h`; use a numeric string such as `"1000"` for an integer cursor | | `batch_concurrency` | Concurrent batch workers; values above `1` require `delete_insert` and the project concurrency gate | | `microbatch_limit` | Nested `max_batches` and `action` policy for watermark execution | +| `max_microbatches` | Legacy scalar batch-count guard; use `microbatch_limit` for new models | +| `unaccounted_partition_policy` | Handling for microbatch partitions with no recorded completion: `synthesize`, `recover_empty`, or `recover_all` | | `lookback` | Backward replay extension | | `append_cursor_inclusive` | Include (`true`, default) or exclude (`false`) the current append-cursor boundary | | `merge_exclude_columns` | Columns left unchanged by matched-row merge updates | diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/project-configuration.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/project-configuration.md index f73dc774ed..2b0e640a03 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/project-configuration.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/project-configuration.md @@ -330,7 +330,7 @@ to inherit their project values. ## Defaults -Project-wide model defaults. Any field you can set in a `MODEL()` header can be set here as a default: +Project-wide defaults for models, seeds, and functions: ```toml [defaults] @@ -342,6 +342,21 @@ tags = ["managed"] These apply to all models unless overridden by path defaults or the model's own `MODEL()` header. +`[defaults]` accepts only the following keys. Any other key, including other `MODEL()` header +fields, fails compilation with `D001`; set those in `[path_defaults]` or the `MODEL()` header +instead. + +- Placement: `database`, `schema`, `seed_database`, `seed_schema`, `function_database`, + `function_schema` +- Materialization: `materialized`, `contract`, `full_refresh`, `replay_on_change`, `tags` +- Incremental: `incremental_strategy`, `incremental_mode`, `merge_exclude_columns`, + `append_cursor_inclusive`, `lookback`, `unaccounted_partition_policy` +- Cursors: `cursor_start`, `cursor_end`, `cursor_watermark_mode`, `cursor_future_action`, + `cursor_future_max_distance`, `cursor_start_max_action`, `cursor_start_max_ahead` +- Microbatch: `microbatch_strategy`, `batch_size`, `batch_concurrency`, `max_microbatches` +- Row diff: `row_diff_exclude_columns`, `row_diff_tolerances`, `row_diff_sample_rows`, + `row_diff_sample_seed` + ## Constants Collection constants default to parenthesized SQL value lists. Set a project-wide default when lists and sets should instead compile to first-class adapter-native arrays: @@ -357,7 +372,7 @@ This setting does not make unsupported adapter features portable. In particular, ## Path defaults -Per-directory model defaults. Useful for applying different config to different parts of your project: +Per-directory model defaults. Useful for applying different config to different parts of your project. Each path default accepts any `MODEL()` header field; an unknown key fails compilation with `D001`: ```toml [path_defaults."models/staging"] diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/sources.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/sources.md index 76247d1626..32e33b0b15 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/sources.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/sources.md @@ -228,6 +228,8 @@ Use [`sqb freshness`](../cli/freshness.md) to observe source freshness on demand | `column` | Column name for `column` strategy | | `query` | SQL query for `sql` strategy | | `lag_tolerance` | Duration tolerance for timestamp comparisons (e.g. `15m`, `2h`, `1d`). Only valid with `type: timestamp`. | +| `filter` | Row filter for the `column` strategy | +| `age_policy` | `warn_after` and `error_after` durations | ## Config reference @@ -246,5 +248,12 @@ Use [`sqb freshness`](../cli/freshness.md) to observe source freshness on demand | `description` | Human-readable description | | `type_enforcement` | Override implicit type enforcement (`true`/`false`). Defaults to `true` when any column declares a type. | | `contract` | `enforced` or `none`. When enforced, downstream models validate configured column references against source columns. | -| `columns` | Column declarations with optional types and audits | +| `columns` | Column declarations with `name`, `type`, `nullable`, `description`, `meta`, and `audits` | | `audits` | Source-level audits | +| `meta` | Free-form metadata mapping | +| `load_batch_size` | Positive row batch size for managed loaders | +| `ingestr` | Declarative ingestr loader config (see [ingestr](../integrations/ingestr.md)) | + +A sources file contains only `sources` and `dlt_sources`. Unknown keys in a sources file, a source, +a source column, `freshness`, `freshness.age_policy`, or `ingestr` fail discovery with `D006`, +naming the key and suggesting the nearest supported key. diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/core.py b/src/sqlbuild/compiler/compile/_helpers/attachment/core.py index e0cc41a49f..f1961fc5b0 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/core.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/core.py @@ -162,7 +162,6 @@ ) _HOOK_TEMPLATE_PATTERN: re.Pattern[str] = re.compile(r"\$\{[^}]+\}") -_LEGACY_MODEL_HOOK_KEYS: frozenset[str] = frozenset({"pre_hook", "post_hook"}) _MODEL_HOOK_KEYS: frozenset[str] = frozenset({"pre_hooks", "post_hooks"}) _REUSABLE_MODEL_HEADER_KEYS: frozenset[str] = frozenset({"columns"}) _HOOK_CONTEXT_PARAMETER_NAMES: frozenset[str] = frozenset( @@ -1241,16 +1240,6 @@ def expand_model_hook_macros_result( def validate_model_hook_config(*, values: dict[str, object], model_name: str) -> None: - legacy_key: str - for legacy_key in sorted(_LEGACY_MODEL_HOOK_KEYS): - if legacy_key in values: - plural_key: str = f"{legacy_key}s" - raise CompileInputError( - f"model '{model_name}' uses legacy '{legacy_key}'; use typed '{plural_key}' " - 'entries like inline_sql("..."), sql("hook_name"), or ' - 'python("hook_name")' - ) - hook_key: str for hook_key in sorted(_MODEL_HOOK_KEYS): if hook_key not in values: diff --git a/src/sqlbuild/compiler/compile/_helpers/attachment/model_config.py b/src/sqlbuild/compiler/compile/_helpers/attachment/model_config.py index 343d3afd9c..499df98900 100644 --- a/src/sqlbuild/compiler/compile/_helpers/attachment/model_config.py +++ b/src/sqlbuild/compiler/compile/_helpers/attachment/model_config.py @@ -53,7 +53,6 @@ ) _MODEL_HOOK_KEYS: frozenset[str] = frozenset({"pre_hooks", "post_hooks"}) -_REMOVED_MODEL_KEYS: frozenset[str] = frozenset({"run_despite_unchanged"}) def _contains_template_data_cached(*, value: object, cache: IdentityPresenceCache | None) -> bool: @@ -137,14 +136,7 @@ def build_layered_model_values( values: dict[str, object] = project_defaults_to_mapping(defaults) if matched_path_default is not None: values = _merged_with_tag_union(base=values, overlay=path_defaults[matched_path_default]) - values = _merged_with_tag_union(base=values, overlay=model_header_values) - removed: list[str] = sorted(_REMOVED_MODEL_KEYS.intersection(values)) - if removed: - raise CompileInputError( - f"MODEL option(s) {', '.join(removed)} were removed with virtual environments; " - "projects run in direct mode" - ) - return values + return _merged_with_tag_union(base=values, overlay=model_header_values) def _merged_with_tag_union( diff --git a/src/sqlbuild/compiler/discovery/_helpers/integrations/loaders.py b/src/sqlbuild/compiler/discovery/_helpers/integrations/loaders.py index 81ced7e510..57ff142659 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/integrations/loaders.py +++ b/src/sqlbuild/compiler/discovery/_helpers/integrations/loaders.py @@ -12,6 +12,9 @@ from sqlbuild.compiler.authored_values.main._require_non_empty_string import ( require_non_empty_string, ) +from sqlbuild.compiler.discovery._helpers.validation.supported_keys import ( + reject_unknown_mapping_keys, +) from sqlbuild.compiler.discovery.constants import ( DLT_LOADER_KIND, DLT_RESOURCE_WRITE_STRATEGY_KEY, @@ -31,6 +34,18 @@ _ingestr_strategies: frozenset[str] = frozenset( {"replace", "append", "merge", "delete+insert", "truncate+insert"} ) +_INGESTR_SOURCE_KEYS: frozenset[str] = frozenset( + { + "columns", + "extra_args", + "incremental_key", + "primary_key", + "source_table", + "source_uri", + "strategy", + "unique_key", + } +) def integration_loader_name(*, kind: str, source_name: str) -> str: @@ -91,6 +106,13 @@ def _parse_ingestr_source_config(*, raw_config: object, file_path: Path) -> Inge if not isinstance(raw_config, dict): raise SourceParseError(f"{file_path} source 'ingestr' must be a mapping") config: dict[str, object] = cast(dict[str, object], raw_config) + reject_unknown_mapping_keys( + mapping=config, + allowed=_INGESTR_SOURCE_KEYS, + file_path=file_path, + label="source ingestr", + error_class=SourceParseError, + ) parsed: IngestrSourceConfig = IngestrSourceConfig( source_uri=require_non_empty_string( entry=config, diff --git a/src/sqlbuild/compiler/discovery/_helpers/python/functions.py b/src/sqlbuild/compiler/discovery/_helpers/python/functions.py index 7de63870b9..b4814b41bf 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/python/functions.py +++ b/src/sqlbuild/compiler/discovery/_helpers/python/functions.py @@ -5,6 +5,7 @@ import ast from pathlib import Path +from sqlbuild.compiler.discovery._helpers.sql.header_keys import unsupported_keys_error from sqlbuild.compiler.discovery.constants import ( PYTHON_UDF_DECORATOR_NAME, PYTHON_UDF_IMPORT_MODULES, @@ -66,9 +67,12 @@ def parse_python_function(*, contents: str, file_path: Path) -> tuple[dict[str, keyword for keyword in decorator_call.keywords if keyword.arg not in PYTHON_UDF_KEYS ) if unsupported: - raise ModelSqlParseError( - f"@udf(...) in '{file_path}:{unsupported[0].lineno}' has unsupported keys: " - f"{', '.join(str(keyword.arg) for keyword in unsupported)}" + raise unsupported_keys_error( + statement="@udf(...)", + location=f"{file_path}:{unsupported[0].lineno}", + keys=(str(keyword.arg) for keyword in unsupported), + supported_keys=PYTHON_UDF_KEYS, + error_class=ModelSqlParseError, ) entry_point: object | None = values.get("entry_point") diff --git a/src/sqlbuild/compiler/discovery/_helpers/sql/audits.py b/src/sqlbuild/compiler/discovery/_helpers/sql/audits.py index d7a7363172..ef5b723390 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/sql/audits.py +++ b/src/sqlbuild/compiler/discovery/_helpers/sql/audits.py @@ -9,6 +9,7 @@ from sqlbuild.compiler.auditing.models import MeasurementContract from sqlbuild.compiler.auditing.types import AuditEvaluationMode +from sqlbuild.compiler.discovery._helpers.sql.header_keys import reject_unsupported_header_keys from sqlbuild.compiler.discovery._helpers.sql.model_files import parse_header_values from sqlbuild.compiler.discovery.exceptions import SqlAuditParseError from sqlbuild.compiler.discovery.models import DiscoveredAuditBlock @@ -61,6 +62,7 @@ def parse_sql_audit_file(*, contents: str, file_path: Path) -> tuple[DiscoveredA _parse_single_sql_audit_block( file_path=file_path, raw_audit_block=contents[start:end].strip(), + block_line=contents.count("\n", 0, start) + 1, audit_index=index, ) ) @@ -70,7 +72,7 @@ def parse_sql_audit_file(*, contents: str, file_path: Path) -> tuple[DiscoveredA def _parse_single_sql_audit_block( - *, file_path: Path, raw_audit_block: str, audit_index: int + *, file_path: Path, raw_audit_block: str, block_line: int, audit_index: int ) -> DiscoveredAuditBlock: open_index: int = _keyword_open_paren( text=raw_audit_block, keyword="AUDIT", file_path=file_path @@ -88,7 +90,9 @@ def _parse_single_sql_audit_block( label="AUDIT(...) header", ) header_values: dict[str, object] = _parse_audit_header( - header=raw_audit_block[open_index + 1 : close_index], file_path=file_path + header=raw_audit_block[open_index + 1 : close_index], + header_line=block_line + raw_audit_block.count("\n", 0, open_index + 1), + file_path=file_path, ) evaluation_mode: AuditEvaluationMode = _parse_evaluation_mode( header_values=header_values, file_path=file_path @@ -130,20 +134,22 @@ def _parse_single_sql_audit_block( ) -def _parse_audit_header(*, header: str, file_path: Path) -> dict[str, object]: +def _parse_audit_header(*, header: str, header_line: int, file_path: Path) -> dict[str, object]: parsed_header: dict[str, object] = parse_header_values( header=header, file_path=file_path, statement_name="AUDIT", error_class=SqlAuditParseError, ) - unsupported_keys: tuple[str, ...] = tuple( - str(key) for key in parsed_header if key not in _SUPPORTED_AUDIT_HEADER_KEYS + reject_unsupported_header_keys( + header_values=parsed_header, + supported_keys=_SUPPORTED_AUDIT_HEADER_KEYS, + statement="AUDIT()", + header=header, + header_line=header_line, + file_path=file_path, + error_class=SqlAuditParseError, ) - if unsupported_keys: - raise SqlAuditParseError( - f"AUDIT() in '{file_path}' has unsupported keys: {', '.join(unsupported_keys)}" - ) for key in ( _AUDIT_NAME_HEADER_KEY, _AUDIT_SEVERITY_HEADER_KEY, diff --git a/src/sqlbuild/compiler/discovery/_helpers/sql/functions.py b/src/sqlbuild/compiler/discovery/_helpers/sql/functions.py index 243fcb99b9..38bbdf67fb 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/sql/functions.py +++ b/src/sqlbuild/compiler/discovery/_helpers/sql/functions.py @@ -5,6 +5,7 @@ import re from pathlib import Path +from sqlbuild.compiler.discovery._helpers.sql.header_keys import reject_unsupported_header_keys from sqlbuild.compiler.discovery._helpers.sql.model_files import parse_header_values from sqlbuild.compiler.discovery.constants import ( SQL_FUNCTION_HEADER_KEYS, @@ -33,26 +34,16 @@ def parse_function_sql(*, contents: str, file_path: Path) -> tuple[dict[str, obj file_path=file_path, statement_name="FUNCTION", ) - unsupported_keys: tuple[str, ...] = tuple( - str(key) for key in header_values if key not in SQL_FUNCTION_HEADER_KEYS + 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, + file_path=file_path, + error_class=ModelSqlParseError, ) - if unsupported_keys: - line: int = contents.count( - "\n", - 0, - header_match.start("header") - + _header_key_offset(header=header_match.group("header"), key=unsupported_keys[0]), - ) - raise ModelSqlParseError( - f"FUNCTION() in '{file_path}:{line + 1}' has unsupported keys: " - f"{', '.join(unsupported_keys)}" - ) body_sql: str = header_match.group("sql").strip() if not body_sql: raise ModelSqlParseError(f"SQL function '{file_path}' must contain SQL after FUNCTION(...)") return header_values, body_sql - - -def _header_key_offset(*, header: str, key: str) -> int: - match: re.Match[str] | None = re.search(rf"(?:^|[(,])\s*({re.escape(key)})\b", header) - return match.start(1) if match is not None else 0 diff --git a/src/sqlbuild/compiler/discovery/_helpers/sql/header_keys.py b/src/sqlbuild/compiler/discovery/_helpers/sql/header_keys.py new file mode 100644 index 0000000000..e99ca9bdf4 --- /dev/null +++ b/src/sqlbuild/compiler/discovery/_helpers/sql/header_keys.py @@ -0,0 +1,63 @@ +"""Rejection of unsupported keys in SQLBuild statement headers.""" + +from __future__ import annotations + +import re +from collections.abc import Iterable +from pathlib import Path + +from sqlbuild.compiler.discovery._helpers.validation.supported_keys import unsupported_keys_help +from sqlbuild.compiler.discovery.exceptions import DiscoveryError + + +def reject_unsupported_header_keys( + *, + header_values: dict[str, object], + supported_keys: frozenset[str], + statement: str, + header: str, + header_line: int, + file_path: Path, + error_class: type[DiscoveryError], +) -> None: + """Raise when a header declares keys outside its set; `header_line` is 1-based.""" + + unsupported: tuple[str, ...] = tuple( + str(key) for key in header_values if key not in supported_keys + ) + if not unsupported: + return + line: int = header_line + header.count( + "\n", 0, _header_key_offset(header=header, key=unsupported[0]) + ) + raise unsupported_keys_error( + statement=statement, + location=f"{file_path}:{line}", + keys=unsupported, + supported_keys=supported_keys, + error_class=error_class, + ) + + +def unsupported_keys_error( + *, + statement: str, + location: str, + keys: Iterable[str], + supported_keys: frozenset[str], + error_class: type[DiscoveryError], +) -> DiscoveryError: + """Build the shared unsupported-keys error with a nearest-key suggestion.""" + + unsupported: tuple[str, ...] = tuple(keys) + return error_class( + f"{statement} in '{location}' has unsupported keys: {', '.join(unsupported)}", + help=unsupported_keys_help(keys=unsupported, supported_keys=supported_keys), + ) + + +def _header_key_offset(*, header: str, key: str) -> int: + match: re.Match[str] | None = re.search( + rf"(?:^|[(,])(?:\s|--[^\n]*(?:\n|$)|/\*.*?\*/)*({re.escape(key)})\b", header, re.S + ) + return match.start(1) if match is not None else 0 diff --git a/src/sqlbuild/compiler/discovery/_helpers/sql/hooks.py b/src/sqlbuild/compiler/discovery/_helpers/sql/hooks.py index e642a4458b..c0f2e614ba 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/sql/hooks.py +++ b/src/sqlbuild/compiler/discovery/_helpers/sql/hooks.py @@ -6,6 +6,7 @@ from pathlib import Path from typing import cast +from sqlbuild.compiler.discovery._helpers.sql.header_keys import reject_unsupported_header_keys from sqlbuild.compiler.discovery._helpers.sql.model_files import parse_header_values from sqlbuild.compiler.discovery.exceptions import SqlHookParseError from sqlbuild.compiler.discovery.models import DiscoveredSqlHookFile @@ -26,8 +27,13 @@ def parse_sql_hook_file( header: str raw_sql_body: str - header, raw_sql_body = _split_hook_file(contents=contents, file_path=file_path) - header_values: dict[str, object] = _parse_hook_header(header=header, file_path=file_path) + header_start: int + header, header_start, raw_sql_body = _split_hook_file(contents=contents, file_path=file_path) + header_values: dict[str, object] = _parse_hook_header( + header=header, + header_line=contents.count("\n", 0, header_start) + 1, + file_path=file_path, + ) sql_body: str = cleandoc(raw_sql_body) if not sql_body: raise SqlHookParseError(f"SQL hook '{file_path}' must define SQL after HOOK(...)") @@ -43,7 +49,7 @@ def parse_sql_hook_file( ) -def _split_hook_file(*, contents: str, file_path: Path) -> tuple[str, str]: +def _split_hook_file(*, contents: str, file_path: Path) -> tuple[str, int, str]: index: int = 0 while index < len(contents) and contents[index].isspace(): index += 1 @@ -93,23 +99,25 @@ def _split_hook_file(*, contents: str, file_path: Path) -> tuple[str, str]: index += 1 if index >= len(contents) or contents[index] != _SQL_STATEMENT_TERMINATOR: raise SqlHookParseError(f"SQL hook '{file_path}' HOOK(...) header must end with ';'") - return contents[header_start:header_end], contents[index + 1 :] + return contents[header_start:header_end], header_start, contents[index + 1 :] -def _parse_hook_header(*, header: str, file_path: Path) -> dict[str, object]: +def _parse_hook_header(*, header: str, header_line: int, file_path: Path) -> dict[str, object]: parsed_header: dict[str, object] = parse_header_values( header=header, file_path=file_path, statement_name="HOOK", error_class=SqlHookParseError, ) - unsupported_keys: tuple[str, ...] = tuple( - str(key) for key in parsed_header if key not in _SUPPORTED_HOOK_HEADER_KEYS + reject_unsupported_header_keys( + header_values=parsed_header, + supported_keys=_SUPPORTED_HOOK_HEADER_KEYS, + statement="HOOK()", + header=header, + header_line=header_line, + file_path=file_path, + error_class=SqlHookParseError, ) - if unsupported_keys: - raise SqlHookParseError( - f"HOOK() in '{file_path}' has unsupported keys: {', '.join(unsupported_keys)}" - ) description: object | None = parsed_header.get(_HOOK_DESCRIPTION_HEADER_KEY) if _HOOK_DESCRIPTION_HEADER_KEY in parsed_header and ( not isinstance(description, str) or not description.strip() diff --git a/src/sqlbuild/compiler/discovery/_helpers/sql/model_files.py b/src/sqlbuild/compiler/discovery/_helpers/sql/model_files.py index 4340aa967c..c429eef188 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/sql/model_files.py +++ b/src/sqlbuild/compiler/discovery/_helpers/sql/model_files.py @@ -10,7 +10,12 @@ from typing import cast import sqlbuild._native as _native -from sqlbuild.compiler.discovery.constants import STATEMENT_HEADER_BODY_PATTERN +from sqlbuild.compiler.discovery._helpers.sql.header_keys import reject_unsupported_header_keys +from sqlbuild.compiler.discovery.constants import ( + REMOVED_SQL_MODEL_HEADER_KEYS, + SQL_MODEL_HEADER_KEYS, + STATEMENT_HEADER_BODY_PATTERN, +) from sqlbuild.compiler.discovery.exceptions import ( DiscoveryError, ModelHeaderSyntaxError, @@ -138,6 +143,21 @@ def parse_matched_model_sql( file_path=file_path, statement_name="MODEL", ) + removed: list[str] = sorted(REMOVED_SQL_MODEL_HEADER_KEYS.intersection(header_values)) + if removed: + raise ModelSqlParseError( + f"MODEL() option(s) {', '.join(removed)} in '{file_path}' were removed with virtual " + "environments; projects run in direct mode" + ) + reject_unsupported_header_keys( + header_values=header_values, + supported_keys=SQL_MODEL_HEADER_KEYS, + statement="MODEL()", + header=header_match.group("header"), + header_line=header_match.string.count("\n", 0, header_match.start("header")) + 1, + file_path=file_path, + error_class=ModelSqlParseError, + ) query: str = header_match.group("sql").strip() if not query: raise ModelSqlParseError(f"SQL model '{file_path}' must contain SQL after MODEL(...)") diff --git a/src/sqlbuild/compiler/discovery/_helpers/sql/scenarios.py b/src/sqlbuild/compiler/discovery/_helpers/sql/scenarios.py index f6ed07d547..431ad0b816 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/sql/scenarios.py +++ b/src/sqlbuild/compiler/discovery/_helpers/sql/scenarios.py @@ -6,6 +6,7 @@ from inspect import cleandoc from pathlib import Path +from sqlbuild.compiler.discovery._helpers.sql.header_keys import reject_unsupported_header_keys from sqlbuild.compiler.discovery._helpers.sql.model_files import parse_header_values from sqlbuild.compiler.discovery.constants import ( SQL_SCENARIOS_OWNERSHIP_ROOT, @@ -20,6 +21,9 @@ ) _SCENARIO_DESCRIPTION_HEADER_KEY: str = "description" _SCENARIO_TAGS_HEADER_KEY: str = "tags" +_SCENARIO_HEADER_KEYS: frozenset[str] = frozenset( + {_SCENARIO_DESCRIPTION_HEADER_KEY, _SCENARIO_TAGS_HEADER_KEY} +) def parse_sql_scenario_file( @@ -36,6 +40,7 @@ def parse_sql_scenario_file( header_values: dict[str, object] = _parse_scenario_header( header=header_match.group("header"), + header_line=contents.count("\n", 0, header_match.start("header")) + 1, file_path=file_path, ) sql_body: str = cleandoc(header_match.group("sql")) @@ -55,7 +60,7 @@ def parse_sql_scenario_file( ) -def _parse_scenario_header(*, header: str, file_path: Path) -> dict[str, object]: +def _parse_scenario_header(*, header: str, header_line: int, file_path: Path) -> dict[str, object]: parsed_header: dict[str, object] = parse_header_values( header=header, file_path=file_path, @@ -63,18 +68,15 @@ def _parse_scenario_header(*, header: str, file_path: Path) -> dict[str, object] error_class=SqlScenarioParseError, ) - supported_keys: set[str] = { - _SCENARIO_DESCRIPTION_HEADER_KEY, - _SCENARIO_TAGS_HEADER_KEY, - } - unsupported_keys: tuple[str, ...] = tuple( - str(key) for key in parsed_header if key not in supported_keys + reject_unsupported_header_keys( + header_values=parsed_header, + supported_keys=_SCENARIO_HEADER_KEYS, + statement="SCENARIO()", + header=header, + header_line=header_line, + file_path=file_path, + error_class=SqlScenarioParseError, ) - if unsupported_keys: - raise SqlScenarioParseError( - f"SCENARIO() in '{file_path}' only supports `description` and `tags` right now; " - f"unsupported keys: {', '.join(unsupported_keys)}" - ) description_value: object | None = parsed_header.get(_SCENARIO_DESCRIPTION_HEADER_KEY) if _SCENARIO_DESCRIPTION_HEADER_KEY in parsed_header and not isinstance(description_value, str): diff --git a/src/sqlbuild/compiler/discovery/_helpers/sql/tests.py b/src/sqlbuild/compiler/discovery/_helpers/sql/tests.py index 15654b9358..32aff23f68 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/sql/tests.py +++ b/src/sqlbuild/compiler/discovery/_helpers/sql/tests.py @@ -10,6 +10,7 @@ from sqlbuild.compiler.compile.constants import DEFAULT_SQL_TEST_MODE from sqlbuild.compiler.compile.types import SqlTestMode +from sqlbuild.compiler.discovery._helpers.sql.header_keys import reject_unsupported_header_keys from sqlbuild.compiler.discovery._helpers.sql.model_files import ( parse_header_values, prepare_model_header_tokens, @@ -40,6 +41,16 @@ _TEST_CASES_HEADER_KEY: str = "cases" _TEST_CURSOR_START_HEADER_KEY: str = "cursor_start" _TEST_CURSOR_END_HEADER_KEY: str = "cursor_end" +_TEST_HEADER_KEYS: frozenset[str] = frozenset( + { + _TEST_NAME_HEADER_KEY, + _TEST_MODE_HEADER_KEY, + _TEST_PARAMETERS_HEADER_KEY, + _TEST_CASES_HEADER_KEY, + _TEST_CURSOR_START_HEADER_KEY, + _TEST_CURSOR_END_HEADER_KEY, + } +) _PARAMETER_TYPES: tuple[SqlValueKind, ...] = ( SqlValueKind.STRING, SqlValueKind.INTEGER, @@ -64,7 +75,7 @@ def prepare_sql_test_file_headers(contents_batch: list[str]) -> None: def parse_sql_test_file(*, contents: str, file_path: Path) -> tuple[DiscoveredSqlTestBlock, ...]: """Parse one SQL-native test file into one or more raw TEST(...) blocks.""" - raw_test_blocks: tuple[str, ...] = _split_sql_test_blocks( + raw_test_blocks: tuple[tuple[str, int], ...] = _split_sql_test_blocks( file_path=file_path, contents=contents ) if not raw_test_blocks: @@ -76,11 +87,13 @@ def parse_sql_test_file(*, contents: str, file_path: Path) -> tuple[DiscoveredSq discovered_blocks: list[DiscoveredSqlTestBlock] = [] test_index: int raw_test_block: str - for test_index, raw_test_block in enumerate(raw_test_blocks, start=1): + block_line: int + for test_index, (raw_test_block, block_line) in enumerate(raw_test_blocks, start=1): discovered_blocks.append( _parse_single_sql_test_block( file_path=file_path, raw_test_block=raw_test_block, + block_line=block_line, test_index=test_index, ) ) @@ -89,7 +102,7 @@ def parse_sql_test_file(*, contents: str, file_path: Path) -> tuple[DiscoveredSq return tuple(discovered_blocks) -def _split_sql_test_blocks(*, file_path: Path, contents: str) -> tuple[str, ...]: +def _split_sql_test_blocks(*, file_path: Path, contents: str) -> tuple[tuple[str, int], ...]: matches: tuple[re.Match[str], ...] = tuple(_TEST_HEADER_ONLY_PATTERN.finditer(contents)) if not matches: return () @@ -99,14 +112,16 @@ def _split_sql_test_blocks(*, file_path: Path, contents: str) -> tuple[str, ...] "non-whitespace content" ) - raw_blocks: list[str] = [] + raw_blocks: list[tuple[str, int]] = [] match_index: int match: re.Match[str] for match_index, match in enumerate(matches): next_start: int = ( matches[match_index + 1].start() if match_index + 1 < len(matches) else len(contents) ) - raw_blocks.append(contents[match.start() : next_start].strip()) + raw_block: str = contents[match.start() : next_start] + block_start: int = match.start() + len(raw_block) - len(raw_block.lstrip()) + raw_blocks.append((raw_block.strip(), contents.count("\n", 0, block_start) + 1)) return tuple(raw_blocks) @@ -114,6 +129,7 @@ def _parse_single_sql_test_block( *, file_path: Path, raw_test_block: str, + block_line: int, test_index: int, ) -> DiscoveredSqlTestBlock: header_match: re.Match[str] | None = _TEST_HEADER_PATTERN.match(raw_test_block) @@ -125,6 +141,7 @@ def _parse_single_sql_test_block( header_values: dict[str, object] = _parse_test_header( header=header_match.group("header"), + header_line=block_line + raw_test_block.count("\n", 0, header_match.start("header")), file_path=file_path, ) sql_body: str = cleandoc(header_match.group("sql")) @@ -180,7 +197,7 @@ def _parse_test_cursor_bound( ) -def _parse_test_header(*, header: str, file_path: Path) -> dict[str, object]: +def _parse_test_header(*, header: str, header_line: int, file_path: Path) -> dict[str, object]: parsed_header: dict[str, object] = parse_header_values( header=header, file_path=file_path, @@ -188,25 +205,15 @@ def _parse_test_header(*, header: str, file_path: Path) -> dict[str, object]: error_class=SqlTestParseError, ) - supported_keys: frozenset[str] = frozenset( - { - _TEST_NAME_HEADER_KEY, - _TEST_MODE_HEADER_KEY, - _TEST_PARAMETERS_HEADER_KEY, - _TEST_CASES_HEADER_KEY, - _TEST_CURSOR_START_HEADER_KEY, - _TEST_CURSOR_END_HEADER_KEY, - } - ) - unsupported_keys: tuple[str, ...] = tuple( - str(key) for key in parsed_header if key not in supported_keys + reject_unsupported_header_keys( + header_values=parsed_header, + supported_keys=_TEST_HEADER_KEYS, + statement="TEST()", + header=header, + header_line=header_line, + file_path=file_path, + error_class=SqlTestParseError, ) - if unsupported_keys: - raise SqlTestParseError( - f"TEST() in '{file_path}' only supports `name`, `mode`, `parameters`, `cases`, " - "`cursor_start`, and `cursor_end`; unsupported keys: " - f"{', '.join(unsupported_keys)}" - ) if _TEST_NAME_HEADER_KEY in parsed_header: _validate_test_name(name_value=parsed_header[_TEST_NAME_HEADER_KEY], file_path=file_path) diff --git a/src/sqlbuild/compiler/discovery/_helpers/validation/supported_keys.py b/src/sqlbuild/compiler/discovery/_helpers/validation/supported_keys.py new file mode 100644 index 0000000000..fc8dc71052 --- /dev/null +++ b/src/sqlbuild/compiler/discovery/_helpers/validation/supported_keys.py @@ -0,0 +1,49 @@ +"""Shared rejection of unsupported keys in authored configuration mappings.""" + +from __future__ import annotations + +from collections.abc import Mapping +from difflib import get_close_matches +from pathlib import Path + +from sqlbuild.compiler.discovery.exceptions import DiscoveryError + +_SUGGESTION_CUTOFF: float = 0.6 +_LISTED_SUPPORTED_KEYS_LIMIT: int = 12 + + +def unsupported_keys_help(*, keys: tuple[str, ...], supported_keys: frozenset[str]) -> str: + """Suggest the nearest supported key for each unsupported key.""" + + candidates: list[str] = sorted(supported_keys) + suggestions: list[str] = [] + for key in keys: + matches: list[str] = get_close_matches(key, candidates, n=1, cutoff=_SUGGESTION_CUTOFF) + if matches: + suggestions.append( + f"'{matches[0]}'" if len(keys) == 1 else f"'{matches[0]}' for '{key}'" + ) + if suggestions: + return f"did you mean {', '.join(suggestions)}?" + if len(candidates) <= _LISTED_SUPPORTED_KEYS_LIMIT: + return f"supported keys: {', '.join(candidates)}" + return "remove the key; see the reference for supported keys" + + +def reject_unknown_mapping_keys( + *, + mapping: Mapping[str, object], + allowed: frozenset[str], + file_path: Path, + label: str, + error_class: type[DiscoveryError], +) -> None: + """Raise when a YAML mapping declares keys outside its allowed set.""" + + unknown: tuple[str, ...] = tuple(sorted(str(key) for key in mapping if key not in allowed)) + if unknown: + raise error_class( + f"{file_path} {label} has unknown keys: {', '.join(unknown)}; allowed keys: " + f"{', '.join(sorted(allowed))}", + help=unsupported_keys_help(keys=unknown, supported_keys=allowed), + ) diff --git a/src/sqlbuild/compiler/discovery/_helpers/yml/project.py b/src/sqlbuild/compiler/discovery/_helpers/yml/project.py index 415ce91d6e..f7446c12a9 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/yml/project.py +++ b/src/sqlbuild/compiler/discovery/_helpers/yml/project.py @@ -15,6 +15,7 @@ from sqlbuild.compiler.auditing.types import AuditSeverity from sqlbuild.compiler.compile.constants import MAX_MICROBATCHES_CONFIG_KEY +from sqlbuild.compiler.discovery._helpers.validation.supported_keys import unsupported_keys_help from sqlbuild.compiler.discovery.constants import ( CONFIG_CONCURRENCY_KEY, DBT_DEFER_CLONE_CONFIG_KEY, @@ -29,6 +30,7 @@ MODELS_DIRECTORY_NAME, PROJECT_CONFIG_FILENAME, SQL_ANALYSIS_CONFIG_KEY, + SQL_MODEL_HEADER_KEYS, TOML_FILE_SUFFIX, ) from sqlbuild.compiler.discovery.exceptions import ProjectConfigError @@ -105,6 +107,42 @@ _BATCH_CONCURRENCY_CONFIG_KEY: str = "batch_concurrency" _MAX_BATCHES_CONFIG_KEY: str = "max_batches" _EVENT_EXPORTER_FILTER_KEYS: frozenset[str] = frozenset({"event_kinds", "min_severity"}) +_DEFAULTS_KEYS: frozenset[str] = frozenset( + { + "append_cursor_inclusive", + "batch_concurrency", + "batch_size", + "contract", + "cursor_end", + "cursor_future_action", + "cursor_future_max_distance", + "cursor_start", + "cursor_start_max_action", + "cursor_start_max_ahead", + "cursor_watermark_mode", + "database", + "full_refresh", + "function_database", + "function_schema", + "incremental_mode", + "incremental_strategy", + "lookback", + "materialized", + "max_microbatches", + "merge_exclude_columns", + "microbatch_strategy", + "replay_on_change", + "row_diff_exclude_columns", + "row_diff_sample_rows", + "row_diff_sample_seed", + "row_diff_tolerances", + "schema", + "seed_database", + "seed_schema", + "tags", + "unaccounted_partition_policy", + } +) _LIFECYCLE_SHUTDOWN_TIMEOUT_CONFIG_KEY: str = "shutdown_timeout" _LIFECYCLE_SHUTDOWN_TIMEOUT_MAX_SECONDS: int = 600 _ZERO_FIXED_DURATION: str = "0s" @@ -818,7 +856,8 @@ def _validate_allowed_keys( allowed: str = ", ".join(sorted(allowed_keys)) unknown: str = ", ".join(unknown_keys) raise ProjectConfigError( - f"{file_path} {label} contains unknown key(s): {unknown}. Allowed keys: {allowed}" + f"{file_path} {label} contains unknown key(s): {unknown}. Allowed keys: {allowed}", + help=unsupported_keys_help(keys=unknown_keys, supported_keys=allowed_keys), ) @@ -873,6 +912,9 @@ def _load_defaults(*, payload: object, file_path: Path) -> DefaultsConfig: label="defaults", file_path=file_path, ) + _validate_allowed_keys( + mapping=mapping, allowed_keys=_DEFAULTS_KEYS, label="[defaults]", file_path=file_path + ) row_diff_exclude_columns: tuple[str, ...] = tuple( _load_string_sequence( payload=mapping.get("row_diff_exclude_columns"), @@ -966,6 +1008,12 @@ def _load_path_defaults(*, payload: object, file_path: Path) -> dict[str, dict[s label="path_defaults", file_path=file_path, ) + _validate_allowed_keys( + mapping=path_dict, + allowed_keys=SQL_MODEL_HEADER_KEYS, + label=f"path_defaults['{path_key}']", + file_path=file_path, + ) _validate_path_default_tags(path_dict=path_dict, path_key=path_key, file_path=file_path) normalized_path_key: str = _normalize_path_default_key( path_key=path_key, file_path=file_path diff --git a/src/sqlbuild/compiler/discovery/_helpers/yml/schema.py b/src/sqlbuild/compiler/discovery/_helpers/yml/schema.py index ef1882769f..5feeaf44ff 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/yml/schema.py +++ b/src/sqlbuild/compiler/discovery/_helpers/yml/schema.py @@ -18,6 +18,9 @@ from sqlbuild.compiler.authored_values.main._require_non_empty_string import ( require_non_empty_string, ) +from sqlbuild.compiler.discovery._helpers.validation.supported_keys import ( + reject_unknown_mapping_keys, +) from sqlbuild.compiler.discovery.constants import NOT_NULL_AUDIT_NAME, SEEDS_DIRECTORY_NAME from sqlbuild.compiler.discovery.exceptions import SchemaParseError from sqlbuild.spec.contracts.models import ( @@ -200,12 +203,13 @@ def _parse_model_entry(*, entry: dict[str, object], file_path: Path) -> SchemaMo def _reject_unknown_keys( *, mapping: dict[str, object], allowed: frozenset[str], file_path: Path, label: str ) -> None: - unknown: tuple[str, ...] = tuple(sorted(str(key) for key in mapping if key not in allowed)) - if unknown: - raise SchemaParseError( - f"{file_path} {label} has unknown keys: {', '.join(unknown)}; allowed keys: " - f"{', '.join(sorted(allowed))}" - ) + reject_unknown_mapping_keys( + mapping=mapping, + allowed=allowed, + file_path=file_path, + label=label, + error_class=SchemaParseError, + ) def _parse_seed_entry(*, entry: dict[str, object], file_path: Path) -> SchemaSeedEntry: diff --git a/src/sqlbuild/compiler/discovery/_helpers/yml/sources.py b/src/sqlbuild/compiler/discovery/_helpers/yml/sources.py index 35ac84ddbc..0813d61a05 100644 --- a/src/sqlbuild/compiler/discovery/_helpers/yml/sources.py +++ b/src/sqlbuild/compiler/discovery/_helpers/yml/sources.py @@ -22,6 +22,9 @@ parse_dlt_sources, parse_source_integration_loader, ) +from sqlbuild.compiler.discovery._helpers.validation.supported_keys import ( + reject_unknown_mapping_keys, +) from sqlbuild.compiler.discovery.constants import ( NOT_NULL_AUDIT_NAME, SOURCE_AGE_POLICY_CONFIG_KEY, @@ -45,6 +48,36 @@ SourceWriteStrategy, ) +_SOURCE_FILE_KEYS: frozenset[str] = frozenset({"sources", "dlt_sources"}) +_SOURCE_ENTRY_KEYS: frozenset[str] = frozenset( + { + "audits", + "columns", + "contract", + "cursor_column", + "database", + "description", + "expression", + "freshness", + "ingestr", + "load_batch_size", + "managed", + "meta", + "name", + "schema", + "table", + "type_enforcement", + "unique_key", + "write_strategy", + } +) +_SOURCE_COLUMN_KEYS: frozenset[str] = frozenset( + {"audits", "description", "meta", "name", "nullable", "type"} +) +_SOURCE_FRESHNESS_KEYS: frozenset[str] = frozenset( + {"age_policy", "column", "filter", "lag_tolerance", "query", "strategy", "type"} +) +_SOURCE_FRESHNESS_AGE_POLICY_KEYS: frozenset[str] = frozenset({"error_after", "warn_after"}) _SOURCE_WRITE_STRATEGIES: frozenset[str] = frozenset( strategy.value for strategy in SourceWriteStrategy ) @@ -60,6 +93,9 @@ def parse_sources_yml(*, contents: str, file_path: Path) -> tuple[SourceEntry, . """Parse one sources/*.yml file into raw source declarations.""" payload: dict[str, object] = _load_sources_payload(contents=contents, file_path=file_path) + _reject_unknown_source_keys( + mapping=payload, allowed=_SOURCE_FILE_KEYS, file_path=file_path, label="sources file" + ) raw_sources: object = payload.get("sources", []) if not isinstance(raw_sources, list): raise SourceParseError(f"{file_path} sources must be a list") @@ -123,6 +159,9 @@ def _parse_source_entry(*, entry: dict[str, object], file_path: Path) -> SourceE f"{file_path} source 'loader' is not supported; use managed: true and name " "the terminal loader after the source" ) + _reject_unknown_source_keys( + mapping=entry, allowed=_SOURCE_ENTRY_KEYS, file_path=file_path, label="source" + ) source_name: str = require_non_empty_string( entry=entry, key="name", @@ -357,6 +396,12 @@ def _optional_freshness_config( if not isinstance(raw_freshness, dict): raise SourceParseError(f"{file_path} source 'freshness' must be a mapping") freshness: dict[str, object] = cast(dict[str, object], raw_freshness) + _reject_unknown_source_keys( + mapping=freshness, + allowed=_SOURCE_FRESHNESS_KEYS, + file_path=file_path, + label="source freshness", + ) raw_strategy: str = require_non_empty_string( entry=freshness, key="strategy", @@ -484,6 +529,12 @@ def _optional_freshness_age_policy( label="source freshness", error_class=SourceParseError, ) + _reject_unknown_source_keys( + mapping=age_policy, + allowed=_SOURCE_FRESHNESS_AGE_POLICY_KEYS, + file_path=file_path, + label="source freshness age_policy", + ) warn_after: str | None = optional_non_empty_string( entry=age_policy, key="warn_after", @@ -580,6 +631,9 @@ def _parse_columns(*, entry: dict[str, object], file_path: Path) -> tuple[Source raise SourceParseError(f"{file_path} source columns must contain only mappings") column: dict[str, object] = cast(dict[str, object], raw_column) column_label: str = "source column" + _reject_unknown_source_keys( + mapping=column, allowed=_SOURCE_COLUMN_KEYS, file_path=file_path, label=column_label + ) column_name: str = require_non_empty_string( entry=column, key="name", @@ -645,3 +699,15 @@ def _validate_nullable_audits( raise SourceParseError( f"{file_path} column '{column_name}' cannot set nullable = true and audit not_null" ) + + +def _reject_unknown_source_keys( + *, mapping: dict[str, object], allowed: frozenset[str], file_path: Path, label: str +) -> None: + reject_unknown_mapping_keys( + mapping=mapping, + allowed=allowed, + file_path=file_path, + label=label, + error_class=SourceParseError, + ) diff --git a/src/sqlbuild/compiler/discovery/constants.py b/src/sqlbuild/compiler/discovery/constants.py index e0f95636b0..707b10863c 100644 --- a/src/sqlbuild/compiler/discovery/constants.py +++ b/src/sqlbuild/compiler/discovery/constants.py @@ -20,6 +20,78 @@ SQL_FUNCTION_HEADER_KEYS: frozenset[str] = frozenset( {"arguments", "returns", "database", "schema", "tags"} ) +SQL_MODEL_HEADER_KEYS: frozenset[str] = frozenset( + { + "alias", + "append_cursor_inclusive", + "audit_factories", + "audits", + "batch_concurrency", + "batch_size", + "check_columns", + "columns", + "config", + "constants", + "contract", + "cursor", + "cursor_end", + "cursor_future_action", + "cursor_future_max_distance", + "cursor_grain", + "cursor_inputs", + "cursor_start", + "cursor_start_max_action", + "cursor_start_max_ahead", + "cursor_type", + "cursor_watermark_mode", + "database", + "description", + "dynamic_columns", + "enabled", + "enums", + "full_refresh", + "historical_input", + "incremental_mode", + "incremental_strategy", + "initial_valid_from", + "invalidate_hard_deletes", + "lookback", + "materialized", + "max_microbatches", + "merge_exclude_columns", + "microbatch_limit", + "microbatch_strategy", + "migrate_force", + "migrate_from", + "model_schema", + "observed_at", + "old_name_view", + "on_schema_change", + "placeholders", + "post_hooks", + "pre_hooks", + "replay_on_change", + "row_diff_exclude_columns", + "row_diff_sample_rows", + "row_diff_sample_seed", + "row_diff_tolerances", + "schema", + "snapshot_full_refresh", + "snapshot_schema_change", + "snapshot_strategy", + "sql_analysis", + "sql_validation", + "table_type", + "tags", + "time_travel_retention", + "unaccounted_partition_policy", + "unique_key", + "updated_at", + "valid_from_column", + "valid_to_column", + } +) +REMOVED_SQL_MODEL_HEADER_KEYS: frozenset[str] = frozenset({"run_despite_unchanged"}) PYTHON_UDF_KEYS: frozenset[str] = frozenset( { "arguments", 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 9f3215492c..331d8fd636 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 @@ -387,3 +387,10 @@ class FunctionNameCompileTestCase: query_sql: str expected_exit_code: int expected_diagnostics: tuple[tuple[str, str, int, int], ...] + + +@dataclass(frozen=True) +class ModelHeaderKeyCompileCase: + description: str + repo_files: dict[str, str] + expected_fragments: tuple[str, ...] diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_unknown_config_keys.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_unknown_config_keys.py new file mode 100644 index 0000000000..52d9bcc933 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_unknown_config_keys.py @@ -0,0 +1,129 @@ +"""E2E coverage for rejecting unknown MODEL header, model config layer, and source keys.""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.compile._test_types import ( + ModelHeaderKeyCompileCase, +) +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project, run_sqb + +_PROJECT_TOML: str = 'name = "orders"\nadapter = "duckdb"\n' + + +@pytest.mark.parametrize( + "test_case", + [ + ModelHeaderKeyCompileCase( + description="misspelled MODEL key", + repo_files={ + "sqlbuild_project.toml": _PROJECT_TOML, + "models/stg_orders.sql": "MODEL (materialized view);\n\nSELECT 1 AS order_id\n", + "models/fct_orders.sql": ( + 'MODEL (\n materialized table,\n descripton "Orders fact.",\n);\n\n' + 'SELECT order_id FROM __ref("stg_orders")\n' + ), + }, + expected_fragments=( + "error[D002]", + "models/fct_orders.sql:3' has unsupported keys: descripton", + "did you mean 'description'?", + ), + ), + ModelHeaderKeyCompileCase( + description="misspelled path default key", + repo_files={ + "sqlbuild_project.toml": ( + _PROJECT_TOML + '\n[path_defaults.staging]\nmaterialised = "view"\n' + ), + "models/staging/stg_orders.sql": "MODEL ();\n\nSELECT 1 AS order_id\n", + }, + expected_fragments=( + "error[D001]", + "path_defaults['staging'] contains unknown key(s): materialised", + "did you mean 'materialized'?", + ), + ), + ModelHeaderKeyCompileCase( + description="misspelled source column key", + repo_files={ + "sqlbuild_project.toml": _PROJECT_TOML, + "sources/raw.yml": ( + "sources:\n - name: raw_orders\n expression: SELECT 1 AS order_id\n" + " columns:\n - name: order_id\n tpye: INTEGER\n" + ), + "models/stg_orders.sql": ( + 'MODEL ();\n\nSELECT order_id FROM __source("raw_orders")\n' + ), + }, + expected_fragments=( + "error[D006]", + "source column has unknown keys: tpye", + "did you mean 'type'?", + ), + ), + ], + ids=lambda case: case.description, +) +def test_given_unknown_model_config_key_when_compiling_then_fails_naming_key_and_nearest_key( + test_case: ModelHeaderKeyCompileCase, tmp_path: Path +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, project_name="orders", repo_files=test_case.repo_files + ) + + result: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", "compile"), project_dir=project_dir + ) + + output: str = result.stdout + result.stderr + assert result.returncode != 0, output + assert all(fragment in output for fragment in test_case.expected_fragments), output + + +@pytest.mark.parametrize( + "test_case", + [ + ModelHeaderKeyCompileCase( + description="custom materialization values live under config", + repo_files={ + "sqlbuild_project.toml": _PROJECT_TOML, + "materializations/order_snapshot.py": ( + "from sqlbuild.executor.custom.models import (\n" + " MaterializationContext,\n MaterializationResult,\n)\n\n\n" + "def materialize(ctx: MaterializationContext) -> MaterializationResult:\n" + " return MaterializationResult(success=True)\n" + ), + "models/order_snapshot.sql": ( + "MODEL (\n materialized order_snapshot,\n" + " config (retention_days 7, partition_column order_id),\n);\n\n" + "SELECT 1 AS order_id\n" + ), + }, + expected_fragments=("Project compiled 1 model",), + ) + ], + ids=lambda case: case.description, +) +def test_given_custom_materialization_config_block_when_compiling_then_arbitrary_keys_pass( + test_case: ModelHeaderKeyCompileCase, tmp_path: Path +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, project_name="orders", repo_files=test_case.repo_files + ) + + result: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", "compile"), project_dir=project_dir + ) + + output: str = result.stdout + result.stderr + assert result.returncode == 0, output + assert all(fragment in output for fragment in test_case.expected_fragments), output + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/integration/src/sqlbuild/cli/commands/main/test_contract_command.py b/tests/integration/src/sqlbuild/cli/commands/main/test_contract_command.py index 2b65d5b144..5e8ef31528 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/test_contract_command.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/test_contract_command.py @@ -108,6 +108,47 @@ def test_given_missing_declarations_when_generating_additively_then_preserves_me assert "type: 'VARCHAR'" in source_yaml +@pytest.mark.parametrize( + "test_case", + [ + ContractCommandIntegrationTestCase( + description="generated contracts in a commented header re-compile cleanly", + expected_exit_code=0, + ) + ], + ids=lambda case: case.description, +) +def test_given_generated_contracts_when_compiling_then_project_compiles_cleanly( + test_case: ContractCommandIntegrationTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + _ = prepare_contract_project(tmp_path) + generate_exit_code: int = main( + [ + "--no-color", + "--project-dir", + str(tmp_path), + "contract", + "generate", + "--from", + "prod", + "--select", + "orders", + "source:raw_orders", + "--write", + ] + ) + assert generate_exit_code == test_case.expected_exit_code + _ = capsys.readouterr() + + compile_exit_code: int = main(["--no-color", "--project-dir", str(tmp_path), "compile"]) + + captured: CaptureResult[str] = capsys.readouterr() + assert compile_exit_code == test_case.expected_exit_code, captured.out + captured.err + assert "error[" not in captured.out + captured.err + + @pytest.mark.parametrize( "test_case", [ diff --git a/tests/unit/src/sqlbuild/compiler/compile/test_main.py b/tests/unit/src/sqlbuild/compiler/compile/test_main.py index 820e0a99a4..f320d89c0a 100644 --- a/tests/unit/src/sqlbuild/compiler/compile/test_main.py +++ b/tests/unit/src/sqlbuild/compiler/compile/test_main.py @@ -4175,28 +4175,6 @@ def dynamic_schema() -> str: run_id=None, expected_error_fragment="SQL syntax error in model 'broken'", ), - BuildCompileInputsErrorTestCase( - description="raises when legacy pre_hook config is used", - repo_files=base_repo_files() - | { - "models/staging/broken.sql": ("MODEL (pre_hook 'SELECT 1');\n\nSELECT 1 AS id\n"), - }, - selected_target=None, - run_id=None, - expected_error_fragment="uses legacy 'pre_hook'", - ), - BuildCompileInputsErrorTestCase( - description="raises when legacy post_hook config is used", - repo_files=base_repo_files() - | { - "models/staging/broken.sql": ( - "MODEL (post_hook ['SELECT 1']);\n\nSELECT 1 AS id\n" - ), - }, - selected_target=None, - run_id=None, - expected_error_fragment="uses legacy 'post_hook'", - ), BuildCompileInputsErrorTestCase( description="raises when plural hooks use bare SQL strings", repo_files=base_repo_files() diff --git a/tests/unit/src/sqlbuild/compiler/discovery/_helpers/_test_types.py b/tests/unit/src/sqlbuild/compiler/discovery/_helpers/_test_types.py index 206f0c041b..91fe3a1b76 100644 --- a/tests/unit/src/sqlbuild/compiler/discovery/_helpers/_test_types.py +++ b/tests/unit/src/sqlbuild/compiler/discovery/_helpers/_test_types.py @@ -912,3 +912,12 @@ class FunctionHeaderKeysTestCase: contents: str expected_error: str = "" expected_keys: tuple[str, ...] = () + + +@dataclass(frozen=True) +class StatementHeaderKeysTestCase: + description: str + file_name: str + contents: str + expected_error: str + expected_help: str diff --git a/tests/unit/src/sqlbuild/compiler/discovery/_helpers/helpers.py b/tests/unit/src/sqlbuild/compiler/discovery/_helpers/helpers.py index ffff94995c..5d80094ac2 100644 --- a/tests/unit/src/sqlbuild/compiler/discovery/_helpers/helpers.py +++ b/tests/unit/src/sqlbuild/compiler/discovery/_helpers/helpers.py @@ -7,6 +7,11 @@ discover_enum_files, discover_macro_files, ) +from sqlbuild.compiler.discovery._helpers.sql.audits import parse_sql_audit_file +from sqlbuild.compiler.discovery._helpers.sql.hooks import parse_sql_hook_file +from sqlbuild.compiler.discovery._helpers.sql.model_files import parse_model_sql +from sqlbuild.compiler.discovery._helpers.sql.scenarios import parse_sql_scenario_file +from sqlbuild.compiler.discovery._helpers.sql.tests import parse_sql_test_file from sqlbuild.compiler.discovery.models import ( DiscoveredConstantFile, DiscoveredEnumFile, @@ -96,3 +101,22 @@ def write_unreadable_files(*, project_dir: Path, relative_paths: tuple[str, ...] file_path: Path = project_dir / relative_path file_path.parent.mkdir(parents=True, exist_ok=True) _ = file_path.write_bytes(b"\xff\xfe\xfa") + + +_STATEMENT_HEADER_PARSERS: dict[str, Callable[..., object]] = { + "MODEL": parse_model_sql, + "AUDIT": parse_sql_audit_file, + "TEST": parse_sql_test_file, + "HOOK": lambda *, contents, file_path: parse_sql_hook_file( + contents=contents, file_path=file_path, relative_path=file_path + ), + "SCENARIO": lambda *, contents, file_path: parse_sql_scenario_file( + contents=contents, file_path=file_path, relative_path=file_path + ), +} + + +def parse_statement_header_file(*, statement: str, contents: str, file_path: Path) -> object: + """Parse one authored statement file with the discovery parser for its header kind.""" + + return _STATEMENT_HEADER_PARSERS[statement](contents=contents, file_path=file_path) diff --git a/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_sql_models.py b/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_sql_models.py index 0d3ddeefcd..12c7680daa 100644 --- a/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_sql_models.py +++ b/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_sql_models.py @@ -46,8 +46,8 @@ def test_given_unique_model_headers_when_discovering_then_native_tokenization_is ) -> None: models_dir: Path = tmp_path / "models" models_dir.mkdir() - first_header = "batch_marker_first one, columns (café (type INTEGER))" - second_header = "batch_marker_second two, columns (total (type DECIMAL(10,2)))" + first_header = 'description "one", columns (café (type INTEGER))' + second_header = 'description "two", columns (total (type DECIMAL(10,2)))' (models_dir / "first.sql").write_text( f"MODEL ({first_header});\nSELECT 1 AS café\n", encoding="utf-8" ) @@ -79,14 +79,14 @@ def recording_tokenize( assert len(native_calls) == test_case.expected_count assert native_calls == [[first_header, second_header]] assert [model.header_values for model in discovered] == [ - {"batch_marker_first": "one", "columns": {"café": {"type": "INTEGER"}}}, + {"description": "one", "columns": {"café": {"type": "INTEGER"}}}, { - "batch_marker_second": "two", + "description": "two", "columns": {"total": {"type": "DECIMAL(10,2)"}}, }, ] assert discovered[0].header_column_locations["café"] == SourceLocation( - path=Path("models/first.sql"), line=1, column=41, end_line=1, end_column=45 + path=Path("models/first.sql"), line=1, column=36, end_line=1, end_column=40 ) @@ -602,6 +602,24 @@ def test_given_deferred_output_locations_when_discovering_models_then_projection expected_header_values={"tags": ["orders);archive"]}, expected_query="SELECT 1", ), + ParseModelSqlHeaderTestCase( + description="skips line and block comments between header entries", + contents=""" + MODEL ( + materialized table -- keep model metadata + /* tags stay, it's fine */ tags [orders], + description "Orders -- not a comment" + ); + + SELECT 1 + """, + expected_header_values={ + "materialized": "table", + "tags": ["orders"], + "description": "Orders -- not a comment", + }, + expected_query="SELECT 1", + ), ], ids=lambda case: case.description, ) diff --git a/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_sql_scenarios.py b/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_sql_scenarios.py index 1afeb9a4d7..701ddc0bec 100644 --- a/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_sql_scenarios.py +++ b/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_sql_scenarios.py @@ -99,7 +99,7 @@ def test_given_sql_scenario_file_variants_when_parsing_then_it_returns_expected_ SELECT 1 """, - expected_error_fragment="only supports `description` and `tags`", + expected_error_fragment="has unsupported keys: name", ), ParseSqlScenarioFileErrorTestCase( description="raises when description is not a string", diff --git a/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_statement_header_keys.py b/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_statement_header_keys.py new file mode 100644 index 0000000000..f4a0a486dc --- /dev/null +++ b/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_statement_header_keys.py @@ -0,0 +1,177 @@ +"""Tests for rejecting unsupported keys in statement headers and model config layers.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from sqlbuild.compiler.discovery._helpers.yml.project import load_project_config +from sqlbuild.compiler.discovery.exceptions import DiscoveryError, ProjectConfigError +from tests.unit.src.sqlbuild.compiler.discovery._helpers._test_types import ( + StatementHeaderKeysTestCase, +) +from tests.unit.src.sqlbuild.compiler.discovery._helpers.helpers import parse_statement_header_file + + +@pytest.mark.parametrize( + "test_case", + [ + StatementHeaderKeysTestCase( + description="MODEL typo is located and gets the nearest key", + file_name="MODEL", + contents=( + "MODEL (\n materialized table,\n uniqe_key order_id,\n);\n\nSELECT 1 AS order_id\n" + ), + expected_error="MODEL() in 'orders.sql:3' has unsupported keys: uniqe_key", + expected_help="did you mean 'unique_key'?", + ), + StatementHeaderKeysTestCase( + description="MODEL comments are skipped and the typo line is still reported", + file_name="MODEL", + contents=( + "MODEL (\n -- owner: fulfillment team\n materialized table,\n" + " /* keep the key, it's stable */\n uniqe_key order_id,\n);\n\n" + "SELECT 1 AS order_id\n" + ), + expected_error="MODEL() in 'orders.sql:5' has unsupported keys: uniqe_key", + expected_help="did you mean 'unique_key'?", + ), + StatementHeaderKeysTestCase( + description="MODEL legacy singular hook suggests the typed list key", + file_name="MODEL", + contents="MODEL (pre_hook 'SELECT 1');\n\nSELECT 1 AS order_id\n", + expected_error="MODEL() in 'orders.sql:1' has unsupported keys: pre_hook", + expected_help="did you mean 'pre_hooks'?", + ), + StatementHeaderKeysTestCase( + description="MODEL removed cursor input key names the replacement", + file_name="MODEL", + contents=( + "MODEL (\n materialized incremental,\n cursor_filter_inputs [stg_orders],\n);\n\n" + "SELECT 1 AS order_id\n" + ), + expected_error="MODEL() in 'orders.sql:3' has unsupported keys: cursor_filter_inputs", + expected_help="did you mean 'cursor_inputs'?", + ), + StatementHeaderKeysTestCase( + description="MODEL unrelated keys list every key without a suggestion", + file_name="MODEL", + contents="MODEL (materialized table, zzz 1, qqq 2);\n\nSELECT 1 AS order_id\n", + expected_error="MODEL() in 'orders.sql:1' has unsupported keys: zzz, qqq", + expected_help="remove the key; see the reference for supported keys", + ), + StatementHeaderKeysTestCase( + description="second AUDIT block reports its own file line", + file_name="AUDIT", + contents=( + "AUDIT (name first_check);\nSELECT 1 WHERE FALSE;\n\n" + "AUDIT (\n name second_check,\n severty warn,\n);\nSELECT 1 WHERE FALSE;\n" + ), + expected_error="AUDIT() in 'orders.sql:6' has unsupported keys: severty", + expected_help="did you mean 'severity'?", + ), + StatementHeaderKeysTestCase( + description="second TEST block reports its own file line", + file_name="TEST", + contents=( + "TEST (name first_case);\nSELECT 1;\n\nTEST (\n name second_case,\n" + " mdoe model,\n);\nSELECT 1;\n" + ), + expected_error="TEST() in 'orders.sql:6' has unsupported keys: mdoe", + expected_help="did you mean 'mode'?", + ), + StatementHeaderKeysTestCase( + description="HOOK lists its small supported key set", + file_name="HOOK", + contents="\nHOOK (owner orders_team);\nSELECT 1\n", + expected_error="HOOK() in 'orders.sql:2' has unsupported keys: owner", + expected_help="supported keys: description", + ), + StatementHeaderKeysTestCase( + description="SCENARIO reports line and nearest key", + file_name="SCENARIO", + contents='SCENARIO (\n descrption "Refund flow",\n);\nSELECT 1\n', + expected_error="SCENARIO() in 'orders.sql:2' has unsupported keys: descrption", + expected_help="did you mean 'description'?", + ), + ], + ids=lambda case: case.description, +) +def test_given_unsupported_header_key_when_parsing_then_rejects_with_location_and_help( + test_case: StatementHeaderKeysTestCase, +) -> None: + with pytest.raises(DiscoveryError) as raised: + parse_statement_header_file( + statement=test_case.file_name, contents=test_case.contents, file_path=Path("orders.sql") + ) + + assert raised.value.message == test_case.expected_error + assert raised.value.help == test_case.expected_help + + +@pytest.mark.parametrize( + "test_case", + [ + StatementHeaderKeysTestCase( + description="removed virtual environment option keeps its migration message", + file_name="MODEL", + contents="MODEL (materialized table, run_despite_unchanged always);\nSELECT 1 AS id\n", + expected_error=( + "MODEL() option(s) run_despite_unchanged in 'orders.sql' were removed with " + "virtual environments; projects run in direct mode" + ), + expected_help="", + ) + ], + ids=lambda case: case.description, +) +def test_given_removed_model_key_when_parsing_then_explains_removal( + test_case: StatementHeaderKeysTestCase, +) -> None: + with pytest.raises(DiscoveryError) as raised: + parse_statement_header_file( + statement=test_case.file_name, contents=test_case.contents, file_path=Path("orders.sql") + ) + + assert raised.value.message == test_case.expected_error + assert raised.value.code == "D002" + + +@pytest.mark.parametrize( + "test_case", + [ + StatementHeaderKeysTestCase( + description="path default typo", + file_name="sqlbuild_project.toml", + contents=( + 'name = "orders"\nadapter = "duckdb"\n\n[path_defaults.staging]\n' + 'materialised = "view"\n' + ), + expected_error="path_defaults['staging'] contains unknown key(s): materialised.", + expected_help="did you mean 'materialized'?", + ), + StatementHeaderKeysTestCase( + description="project default that is not layered from TOML", + file_name="sqlbuild_project.toml", + contents='name = "orders"\nadapter = "duckdb"\n\n[defaults]\ntag = ["orders"]\n', + expected_error="[defaults] contains unknown key(s): tag.", + expected_help="did you mean 'tags'?", + ), + ], + ids=lambda case: case.description, +) +def test_given_unknown_model_config_layer_key_when_loading_project_then_rejects_with_help( + test_case: StatementHeaderKeysTestCase, tmp_path: Path +) -> None: + (tmp_path / test_case.file_name).write_text(test_case.contents, encoding="utf-8") + + with pytest.raises(ProjectConfigError) as raised: + load_project_config(project_dir=tmp_path) + + assert test_case.expected_error in raised.value.message + assert raised.value.help == test_case.expected_help + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_yml_sources.py b/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_yml_sources.py index fec22682e2..45d68ea8ce 100644 --- a/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_yml_sources.py +++ b/tests/unit/src/sqlbuild/compiler/discovery/_helpers/test_yml_sources.py @@ -539,6 +539,72 @@ def test_given_dlt_sources_yaml_when_parsing_then_expands_managed_sources( contents="- name: raw_orders\n", expected_error_fragment="must contain a top-level mapping", ), + ParseSourcesYamlErrorTestCase( + description="raises when the file declares an unknown top-level key", + contents=""" + version: 2 + sources: + - name: raw_orders + """, + expected_error_fragment="sources file has unknown keys: version", + ), + ParseSourcesYamlErrorTestCase( + description="raises when a source declares a misspelled key", + contents=""" + sources: + - name: raw_orders + descripton: Raw orders + """, + expected_error_fragment="source has unknown keys: descripton", + ), + ParseSourcesYamlErrorTestCase( + description="raises when a source column declares a misspelled key", + contents=""" + sources: + - name: raw_orders + columns: + - name: order_id + nulable: false + """, + expected_error_fragment="source column has unknown keys: nulable", + ), + ParseSourcesYamlErrorTestCase( + description="raises when source freshness declares an unknown key", + contents=""" + sources: + - name: raw_orders + freshness: + strategy: column + column: loaded_at + warn_after: 1d + """, + expected_error_fragment="source freshness has unknown keys: warn_after", + ), + ParseSourcesYamlErrorTestCase( + description="raises when source freshness age policy declares an unknown key", + contents=""" + sources: + - name: raw_orders + freshness: + strategy: column + column: loaded_at + age_policy: + warn_afer: 1d + """, + expected_error_fragment="source freshness age_policy has unknown keys: warn_afer", + ), + ParseSourcesYamlErrorTestCase( + description="raises when ingestr config declares an unknown key", + contents=""" + sources: + - name: raw_orders + ingestr: + source_uri: postgresql://localhost/shop + source_table: public.orders + incremental_column: updated_at + """, + expected_error_fragment="source ingestr has unknown keys: incremental_column", + ), ParseSourcesYamlErrorTestCase( description="raises when source meta is not a mapping", contents=""" diff --git a/website/src/content/docs/docs/concepts/models/configuration.mdx b/website/src/content/docs/docs/concepts/models/configuration.mdx index d87b8ae539..4e4e98360c 100644 --- a/website/src/content/docs/docs/concepts/models/configuration.mdx +++ b/website/src/content/docs/docs/concepts/models/configuration.mdx @@ -3,6 +3,19 @@ title: "Configuration" description: "MODEL() header fields and SQL-analysis controls." --- +Every `MODEL()` key must be one of the fields on this page. Any other key fails compilation with +`D002`, naming the key and its file and line and suggesting the nearest supported key. SQL comments +(`-- ...` and `/* ... */`) inside a header are ignored: + +```text +error[D002]: MODEL() in 'models/fct_orders.sql:3' has unsupported keys: descripton + = help: did you mean 'description'? +``` + +`[defaults]` and `[path_defaults.*]` in `sqlbuild_project.toml` reject unknown keys in the same way +with `D001`. Values that a custom materialization reads belong inside its `config (...)` block, +which accepts any keys. + ## Common fields | Field | Description | @@ -23,6 +36,8 @@ description: "MODEL() header fields and SQL-analysis controls." | `enabled` | Set to `false` to disable the model | | `contract` | `none` for an open statically checked declaration, or `enforced` for an exact declaration | | `sql_analysis` | Per-model SQL-analysis override | +| `dynamic_columns` | Named pivot column families with `pivot_column`, `value_column`, `aggregate`, `type`, and `name_pattern` | +| `audit_factories` | Audit factories that generate audit cases for this model; see [Audits](/docs/concepts/audits) | ## SQL analysis @@ -41,6 +56,8 @@ configuration. | Field | Description | |-------|-------------| +| `table_type` | Snowflake table type: `permanent`, `transient`, or `inherit` | +| `time_travel_retention` | Managed time travel retention such as `7d`, `inherit`, or `disabled` | | `run_despite_unchanged` | Removed with virtual environments. It is rejected in MODEL headers, project defaults, and path defaults. | Table promotion mode is a project setting rather than a `MODEL()` field. Staged promotion is the default. Immediate promotion is incompatible with model type enforcement and exact contracts; see [Materializations](/docs/concepts/models/materializations#table). @@ -54,6 +71,11 @@ Table promotion mode is a project setting rather than a `MODEL()` field. Staged | `cursor_type` | `timestamp` or `integer` | | `cursor_grain` | Timestamp grain such as `second`, `hour`, or `day` | | `cursor_start` | Lower cursor bound | +| `cursor_end` | Upper cursor bound | +| `cursor_start_max_ahead` | Largest distance a discovered automatic start may sit ahead of the invocation time, or `disabled` | +| `cursor_start_max_action` | `cap` or `error` when the start exceeds `cursor_start_max_ahead` | +| `cursor_future_max_distance` | Largest distance a discovered cursor watermark may sit in the future, or `disabled` | +| `cursor_future_action` | `cap` or `error` when a watermark exceeds `cursor_future_max_distance` | | `cursor_inputs` | Upstream names mapped to cursor columns | | `unique_key` | Merge or delete/insert matching columns | | `incremental_mode` | Set to `microbatch` for batched execution | @@ -62,6 +84,8 @@ Table promotion mode is a project setting rather than a `MODEL()` field. Staged | `batch_size` | Timestamp duration string such as `1d` or `1h`; use a numeric string such as `"1000"` for an integer cursor | | `batch_concurrency` | Concurrent batch workers; values above `1` require `delete_insert` and the project concurrency gate | | `microbatch_limit` | Nested `max_batches` and `action` policy for watermark execution | +| `max_microbatches` | Legacy scalar batch-count guard; use `microbatch_limit` for new models | +| `unaccounted_partition_policy` | Handling for microbatch partitions with no recorded completion: `synthesize`, `recover_empty`, or `recover_all` | | `lookback` | Backward replay extension | | `append_cursor_inclusive` | Include (`true`, default) or exclude (`false`) the current append-cursor boundary | | `merge_exclude_columns` | Columns left unchanged by matched-row merge updates | diff --git a/website/src/content/docs/docs/concepts/project-configuration.mdx b/website/src/content/docs/docs/concepts/project-configuration.mdx index bbed80c769..70642b7078 100644 --- a/website/src/content/docs/docs/concepts/project-configuration.mdx +++ b/website/src/content/docs/docs/concepts/project-configuration.mdx @@ -307,7 +307,7 @@ to inherit their project values. ## Defaults -Project-wide model defaults. Any field you can set in a `MODEL()` header can be set here as a default: +Project-wide defaults for models, seeds, and functions: ```toml [defaults] @@ -319,6 +319,21 @@ tags = ["managed"] These apply to all models unless overridden by path defaults or the model's own `MODEL()` header. +`[defaults]` accepts only the following keys. Any other key, including other `MODEL()` header +fields, fails compilation with `D001`; set those in `[path_defaults]` or the `MODEL()` header +instead. + +- Placement: `database`, `schema`, `seed_database`, `seed_schema`, `function_database`, + `function_schema` +- Materialization: `materialized`, `contract`, `full_refresh`, `replay_on_change`, `tags` +- Incremental: `incremental_strategy`, `incremental_mode`, `merge_exclude_columns`, + `append_cursor_inclusive`, `lookback`, `unaccounted_partition_policy` +- Cursors: `cursor_start`, `cursor_end`, `cursor_watermark_mode`, `cursor_future_action`, + `cursor_future_max_distance`, `cursor_start_max_action`, `cursor_start_max_ahead` +- Microbatch: `microbatch_strategy`, `batch_size`, `batch_concurrency`, `max_microbatches` +- Row diff: `row_diff_exclude_columns`, `row_diff_tolerances`, `row_diff_sample_rows`, + `row_diff_sample_seed` + ## Constants Collection constants default to parenthesized SQL value lists. Set a project-wide default when lists and sets should instead compile to first-class adapter-native arrays: @@ -334,7 +349,7 @@ This setting does not make unsupported adapter features portable. In particular, ## Path defaults -Per-directory model defaults. Useful for applying different config to different parts of your project: +Per-directory model defaults. Useful for applying different config to different parts of your project. Each path default accepts any `MODEL()` header field; an unknown key fails compilation with `D001`: ```toml [path_defaults."models/staging"] diff --git a/website/src/content/docs/docs/concepts/sources.mdx b/website/src/content/docs/docs/concepts/sources.mdx index cc0d5cc0a9..598f8e95ef 100644 --- a/website/src/content/docs/docs/concepts/sources.mdx +++ b/website/src/content/docs/docs/concepts/sources.mdx @@ -215,6 +215,8 @@ Use [`sqb freshness`](/docs/cli/freshness) to observe source freshness on demand | `column` | Column name for `column` strategy | | `query` | SQL query for `sql` strategy | | `lag_tolerance` | Duration tolerance for timestamp comparisons (e.g. `15m`, `2h`, `1d`). Only valid with `type: timestamp`. | +| `filter` | Row filter for the `column` strategy | +| `age_policy` | `warn_after` and `error_after` durations | ## Config reference @@ -233,5 +235,12 @@ Use [`sqb freshness`](/docs/cli/freshness) to observe source freshness on demand | `description` | Human-readable description | | `type_enforcement` | Override implicit type enforcement (`true`/`false`). Defaults to `true` when any column declares a type. | | `contract` | `enforced` or `none`. When enforced, downstream models validate configured column references against source columns. | -| `columns` | Column declarations with optional types and audits | +| `columns` | Column declarations with `name`, `type`, `nullable`, `description`, `meta`, and `audits` | | `audits` | Source-level audits | +| `meta` | Free-form metadata mapping | +| `load_batch_size` | Positive row batch size for managed loaders | +| `ingestr` | Declarative ingestr loader config (see [ingestr](/docs/integrations/ingestr)) | + +A sources file contains only `sources` and `dlt_sources`. Unknown keys in a sources file, a source, +a source column, `freshness`, `freshness.age_policy`, or `ingestr` fail discovery with `D006`, +naming the key and suggesting the nearest supported key. From 1df371d9b8df21cc2538ce12f71764925d0ed7f6 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 1 Oct 2026 19:52:37 +0100 Subject: [PATCH 2/5] perf(compiler): keep per-diagnostic work linear in diagnostic-heavy models One model with thousands of diagnostics compiled in quadratic time: locating each unknown column rescanned the authored SQL natively, and diagnostic recovery and explanation rebuilt per-model column sets, lineage lists, line splits and binary-expression scans for every diagnostic. Index each once per model and look positions up by bisect or hash. A one-model scaling guard compares 1k and 4k diagnostics. --- .../semantic_validation/_helpers/positions.rs | 67 ++++++++--- .../src/semantic_validation/models.rs | 1 + .../src/semantic_validation/tests.rs | 2 + .../tests/test_positions.rs | 111 ++++++++++++++++++ .../semantic_validation/tests/test_types.rs | 13 ++ .../_helpers/diagnostic_project.py | 26 ++++ .../compile/_helpers/diagnostics/details.py | 70 ++++++----- .../compile/_helpers/diagnostics/recovery.py | 27 +++-- .../_helpers/diagnostics/type_recovery.py | 28 ++++- .../test_diagnostic_compile_performance.py | 31 +++++ 10 files changed, 316 insertions(+), 60 deletions(-) create mode 100644 crates/sqlbuild-rules-native/src/semantic_validation/tests/test_positions.rs diff --git a/crates/sqlbuild-rules-native/src/semantic_validation/_helpers/positions.rs b/crates/sqlbuild-rules-native/src/semantic_validation/_helpers/positions.rs index cc2dabb355..e207e6dcc2 100644 --- a/crates/sqlbuild-rules-native/src/semantic_validation/_helpers/positions.rs +++ b/crates/sqlbuild-rules-native/src/semantic_validation/_helpers/positions.rs @@ -8,7 +8,7 @@ use pyo3::exceptions::PyValueError; use pyo3::{PyResult, pymethods}; use regex::Regex; use std::collections::HashMap; -use std::sync::LazyLock; +use std::sync::{LazyLock, OnceLock}; const POPULAR_TOKEN_THRESHOLD: usize = 200; static TOKENS: LazyLock> = @@ -42,6 +42,7 @@ impl BindingPositions { authored, offsets, passes, + words: OnceLock::new(), }) }) } @@ -59,21 +60,8 @@ impl BindingPositions { .captures(message) { let identifier = captures[1].rsplit('.').next().unwrap_or(&captures[1]); - let mut occurrences = - self.authored - .match_indices(identifier) - .filter(|(offset, _)| { - let before = self.authored[..*offset].chars().next_back(); - let after = self.authored[*offset + identifier.len()..].chars().next(); - !before.is_some_and(word) && !after.is_some_and(word) - }); - if let Some((offset, _)) = occurrences.next() - && occurrences.next().is_none() - { - return Ok(position( - &self.lines, - self.authored[..offset].chars().count(), - )); + if let Some(offset) = self.unique_word_offset(identifier) { + return Ok(position(&self.lines, offset)); } } let Some(query_start) = self.query_start else { @@ -116,6 +104,53 @@ impl BindingPositions { } } +impl BindingPositions { + /// Char offset of the only whole-word occurrence of `identifier` in the authored SQL. + pub(crate) fn unique_word_offset(&self, identifier: &str) -> Option { + if identifier.is_empty() || !identifier.chars().all(word) { + let mut occurrences = self + .authored + .match_indices(identifier) + .filter(|(offset, _)| { + let before = self.authored[..*offset].chars().next_back(); + let after = self.authored[*offset + identifier.len()..].chars().next(); + !before.is_some_and(word) && !after.is_some_and(word) + }); + let (offset, _) = occurrences.next()?; + return occurrences + .next() + .is_none() + .then(|| self.authored[..offset].chars().count()); + } + let (offset, unique) = *self + .words + .get_or_init(|| word_occurrences(&self.authored)) + .get(identifier)?; + unique.then_some(offset) + } +} + +/// Index every maximal word run once so identifier lookups do not rescan the SQL. +fn word_occurrences(sql: &str) -> HashMap { + let mut occurrences: HashMap = HashMap::new(); + let mut current = String::new(); + let mut start = 0; + for (index, character) in sql.chars().chain(std::iter::once(' ')).enumerate() { + if word(character) { + if current.is_empty() { + start = index; + } + current.push(character); + } else if !current.is_empty() { + occurrences + .entry(std::mem::take(&mut current)) + .and_modify(|entry| entry.1 = false) + .or_insert((start, true)); + } + } + occurrences +} + fn word(character: char) -> bool { character == '_' || character.is_alphanumeric() } diff --git a/crates/sqlbuild-rules-native/src/semantic_validation/models.rs b/crates/sqlbuild-rules-native/src/semantic_validation/models.rs index 7fd72ededa..e185093a05 100644 --- a/crates/sqlbuild-rules-native/src/semantic_validation/models.rs +++ b/crates/sqlbuild-rules-native/src/semantic_validation/models.rs @@ -65,6 +65,7 @@ pub(crate) struct BindingPositions { pub(super) cleaned_lines: Vec, pub(super) offsets: Vec, pub(super) passes: Vec>, + pub(super) words: std::sync::OnceLock>, } impl std::fmt::Debug for ProjectCatalog { diff --git a/crates/sqlbuild-rules-native/src/semantic_validation/tests.rs b/crates/sqlbuild-rules-native/src/semantic_validation/tests.rs index b4df2321eb..4ae2af8033 100644 --- a/crates/sqlbuild-rules-native/src/semantic_validation/tests.rs +++ b/crates/sqlbuild-rules-native/src/semantic_validation/tests.rs @@ -1,5 +1,7 @@ #[path = "tests/test_function_names.rs"] mod function_names; +#[path = "tests/test_positions.rs"] +mod positions; #[path = "tests/test_semantic_validation.rs"] mod semantic_validation; #[path = "tests/test_types.rs"] diff --git a/crates/sqlbuild-rules-native/src/semantic_validation/tests/test_positions.rs b/crates/sqlbuild-rules-native/src/semantic_validation/tests/test_positions.rs new file mode 100644 index 0000000000..4cfc66be4e --- /dev/null +++ b/crates/sqlbuild-rules-native/src/semantic_validation/tests/test_positions.rs @@ -0,0 +1,111 @@ +use crate::semantic_validation::models::BindingPositions; +use crate::semantic_validation::tests::test_types::{ + UniqueWordOffsetTestCase, UniqueWordScalingTestCase, +}; +use std::sync::OnceLock; +use std::time::{Duration, Instant}; + +#[test] +fn given_identifier_when_locating_unique_word_then_returns_its_char_offset() { + let test_cases = [ + UniqueWordOffsetTestCase { + description: "single whole-word occurrence", + authored: "SELECT o.order_id, o.amount FROM orders AS o", + identifier: "amount", + expected_offset: Some(21), + }, + UniqueWordOffsetTestCase { + description: "occurrence inside a longer word is not a match", + authored: "SELECT order_id_v2, order_id FROM orders", + identifier: "order_id", + expected_offset: Some(20), + }, + UniqueWordOffsetTestCase { + description: "repeated word is ambiguous", + authored: "SELECT amount FROM orders WHERE amount > 0", + identifier: "amount", + expected_offset: None, + }, + UniqueWordOffsetTestCase { + description: "absent word", + authored: "SELECT amount FROM orders", + identifier: "quantity", + expected_offset: None, + }, + UniqueWordOffsetTestCase { + description: "offsets count characters after non-ASCII text", + authored: "SELECT 'café' AS label, total FROM orders", + identifier: "total", + expected_offset: Some(24), + }, + UniqueWordOffsetTestCase { + description: "identifier with non-word characters falls back to a substring scan", + authored: "SELECT \"order total\" FROM orders", + identifier: "order total", + expected_offset: Some(8), + }, + ]; + + for test_case in test_cases { + let located = BindingPositions { + authored: test_case.authored.to_owned(), + query_start: Some(0), + lines: vec![0], + cleaned_lines: vec![0], + offsets: Vec::new(), + passes: Vec::new(), + words: OnceLock::new(), + }; + for lookup in ["first", "cached"] { + assert_eq!( + located.unique_word_offset(test_case.identifier), + test_case.expected_offset, + "{} ({lookup} lookup)", + test_case.description + ); + } + } +} + +#[test] +fn given_many_unique_identifiers_when_locating_each_then_work_stays_linear() { + let test_cases = [UniqueWordScalingTestCase { + description: "one wide query with an unknown column per projection", + identifiers: 20_000, + expected_max_duration: Duration::from_secs(2), + }]; + + for test_case in test_cases { + let located = BindingPositions { + authored: (0..test_case.identifiers) + .map(|index| format!("missing_{index}")) + .collect::>() + .join(",\n"), + query_start: Some(0), + lines: vec![0], + cleaned_lines: vec![0], + offsets: Vec::new(), + passes: Vec::new(), + words: OnceLock::new(), + }; + let started = Instant::now(); + let located_count = (0..test_case.identifiers) + .filter(|index| { + located + .unique_word_offset(&format!("missing_{index}")) + .is_some() + }) + .count(); + assert_eq!( + located_count, test_case.identifiers, + "{}", + test_case.description + ); + assert!( + started.elapsed() < test_case.expected_max_duration, + "{}: took {:?}", + test_case.description, + started.elapsed() + ); + } +} diff --git a/crates/sqlbuild-rules-native/src/semantic_validation/tests/test_types.rs b/crates/sqlbuild-rules-native/src/semantic_validation/tests/test_types.rs index 5228220a9a..cf71c10603 100644 --- a/crates/sqlbuild-rules-native/src/semantic_validation/tests/test_types.rs +++ b/crates/sqlbuild-rules-native/src/semantic_validation/tests/test_types.rs @@ -24,3 +24,16 @@ pub(crate) struct LocatedFunctionDiagnosticTestCase { pub(crate) expected_location: (Option, Option), pub(crate) expected_span: (Option, Option), } + +pub(crate) struct UniqueWordOffsetTestCase { + pub(crate) description: &'static str, + pub(crate) authored: &'static str, + pub(crate) identifier: &'static str, + pub(crate) expected_offset: Option, +} + +pub(crate) struct UniqueWordScalingTestCase { + pub(crate) description: &'static str, + pub(crate) identifiers: usize, + pub(crate) expected_max_duration: std::time::Duration, +} diff --git a/scripts/cold_compile_performance/_helpers/diagnostic_project.py b/scripts/cold_compile_performance/_helpers/diagnostic_project.py index 4ef3339755..93d4ffbe2b 100644 --- a/scripts/cold_compile_performance/_helpers/diagnostic_project.py +++ b/scripts/cold_compile_performance/_helpers/diagnostic_project.py @@ -98,3 +98,29 @@ def write_function_name_project(*, project_dir: Path, model_count: int, calls: i + ",\n".join(projections) + "\nFROM (SELECT 'anvil' AS sku) AS products\nWHERE NOT (sku IN ('a', 'b'))\n" ) + + +def write_single_model_diagnostic_project(*, project_dir: Path, diagnostics: int) -> None: + """Generate one wide model whose projections each raise an unknown-column or type error.""" + project_dir.mkdir(parents=True, exist_ok=True) + models: Path = project_dir / "models" + models.mkdir(exist_ok=True) + (project_dir / "sqlbuild_project.toml").write_text( + 'name = "dense_diagnostic_orders"\nadapter = "duckdb"\n[rules]\nselect = []\n' + ) + (models / "orders.sql").write_text( + "MODEL (materialized view, contract enforced, columns (order_id (type INTEGER), " + "status (type VARCHAR)));\nSELECT CAST(1 AS INTEGER) AS order_id, " + "CAST('open' AS VARCHAR) AS status" + ) + projections: list[str] = [ + f" o.missing_{index:05} AS broken_{index:05}" + if index % 2 == 0 + else f" o.order_id = 'shipped_{index:05}' AS risky_{index:05}" + for index in range(diagnostics) + ] + (models / "order_diagnostics.sql").write_text( + "MODEL (materialized view);\nSELECT\n" + + ",\n".join(projections) + + '\nFROM __ref("orders") AS o\n' + ) diff --git a/src/sqlbuild/compiler/compile/_helpers/diagnostics/details.py b/src/sqlbuild/compiler/compile/_helpers/diagnostics/details.py index b1eb7de178..fead671200 100644 --- a/src/sqlbuild/compiler/compile/_helpers/diagnostics/details.py +++ b/src/sqlbuild/compiler/compile/_helpers/diagnostics/details.py @@ -3,6 +3,7 @@ from __future__ import annotations import re +from bisect import bisect_right from dataclasses import replace from typing import Any @@ -30,6 +31,8 @@ r"\b(timestamp|integer|varchar|boolean|date|interval|double|float|decimal|numeric|" r"bigint|smallint|text|time|string|binary|array|struct)\b" ) +type _LineIndex = tuple[tuple[str, ...], tuple[int, ...]] +type _BinaryIndex = tuple[tuple[int, ...], tuple[re.Match[str], ...]] _MISSING: re.Pattern[str] = re.compile(r"Unknown column '([^']+)'(?: in table '([^']+)')?") _OPERAND: str = ( r"(?:TIMESTAMP\s+'[^']*'|DATE\s+'[^']*'|'(?:[^']|'')*'|-?\d+(?:\.\d+)?|" @@ -140,6 +143,8 @@ def explain_diagnostics(project: CompiledProject) -> CompiledProject: shapes: dict[str, dict[str, str]] = semantic_shapes(project=project) models: dict[str, CompiledModel] = {model.name: model for model in project.models} aliases: dict[str, dict[str, str]] = {} + line_indexes: dict[str, _LineIndex] = {} + binary_indexes: dict[str, _BinaryIndex] = {} diagnostics: list[CompilerDiagnostic] = [] diagnostic: CompilerDiagnostic for diagnostic in project.diagnostics: @@ -149,12 +154,17 @@ def explain_diagnostics(project: CompiledProject) -> CompiledProject: aliases[model.name] = input_aliases( model=model, dialect=project.sql_analysis_dialect ) + line_indexes[model.name] = _line_index(model.authored_sql) + if diagnostic.code in _TYPE_CODES and model.name not in binary_indexes: + binary_indexes[model.name] = _binary_index(model.authored_sql) diagnostic = _explain_model( diagnostic=diagnostic, model=model, aliases=aliases[model.name], shapes=shapes, dialect=project.sql_analysis_dialect, + lines=line_indexes[model.name], + binaries=binary_indexes.get(model.name, ((), ())), ) elif semantic_help(diagnostic.code) is not None: diagnostic = replace( @@ -188,14 +198,31 @@ def update_binding_models( return tuple(updated) -def _binary_at(*, sql: str, offset: int) -> re.Match[str] | None: - """Matches are ordered and disjoint, so none after the offset can contain it.""" - for match in _BINARY.finditer(sql): - if match.start() > offset: - return None - if offset < match.end(): - return match - return None +def _line_index(sql: str) -> _LineIndex: + """Index authored lines once per model so each diagnostic maps positions in O(log n).""" + starts: list[int] = [0] + for line in sql.splitlines(keepends=True): + starts.append(starts[-1] + len(line)) + return tuple(sql.splitlines()), tuple(starts) + + +def _binary_index(sql: str) -> _BinaryIndex: + matches: tuple[re.Match[str], ...] = tuple(_BINARY.finditer(sql)) + return tuple(match.start() for match in matches), matches + + +def _line_offset(*, lines: _LineIndex, line: int, column: int) -> int: + starts: tuple[int, ...] = lines[1] + return starts[min(max(line - 1, 0), len(starts) - 1)] + column - 1 + + +def _binary_at(*, binaries: _BinaryIndex, offset: int) -> re.Match[str] | None: + """Matches are ordered and disjoint, so only the last one starting at or before can match.""" + position: int = bisect_right(binaries[0], offset) - 1 + if position < 0: + return None + match: re.Match[str] = binaries[1][position] + return match if offset < match.end() else None def _explain_model( @@ -205,6 +232,8 @@ def _explain_model( aliases: dict[str, str], shapes: dict[str, dict[str, str]], dialect: str | None, + lines: _LineIndex, + binaries: _BinaryIndex, ) -> CompilerDiagnostic: message: str = sentence_message(diagnostic.message) help_text: str | None = semantic_help(diagnostic.code) @@ -226,8 +255,7 @@ def _explain_model( help_text = f"did you mean '{suggestion}'?" if suggestion else None message = f"Unknown column '{name}'" + (f" in {table}" if table else "") if location is not None: - lines: list[str] = model.authored_sql.splitlines() - prefix: str = lines[location.line - 1][: location.column - 1] + prefix: str = lines[0][location.line - 1][: location.column - 1] qualifier: re.Match[str] | None = re.search( r'(?:"((?:[^"]|"")+)"|([A-Za-z_]\w*))\.$', prefix ) @@ -245,15 +273,8 @@ def _explain_model( ) notes.insert(0, f"{table} has: {', '.join(available[:10])}{suffix}") if diagnostic.code in _TYPE_CODES and location is not None: - offset: int = ( - sum( - len(line) - for line in model.authored_sql.splitlines(keepends=True)[: location.line - 1] - ) - + location.column - - 1 - ) - binary: re.Match[str] | None = _binary_at(sql=model.authored_sql, offset=offset) + offset: int = _line_offset(lines=lines, line=location.line, column=location.column) + binary: re.Match[str] | None = _binary_at(binaries=binaries, offset=offset) if binary: left: str = binary.group("left") right: str = binary.group("right") @@ -269,15 +290,8 @@ def _explain_model( ) end: int = binary.end() if location.end_line is not None and location.end_column is not None: - native_end: int = ( - sum( - len(line) - for line in model.authored_sql.splitlines(keepends=True)[ - : location.end_line - 1 - ] - ) - + location.end_column - - 1 + native_end: int = _line_offset( + lines=lines, line=location.end_line, column=location.end_column ) end = max(end, native_end) begin: int = min(binary.start(), offset) diff --git a/src/sqlbuild/compiler/compile/_helpers/diagnostics/recovery.py b/src/sqlbuild/compiler/compile/_helpers/diagnostics/recovery.py index 32b29aa426..982b0bab7f 100644 --- a/src/sqlbuild/compiler/compile/_helpers/diagnostics/recovery.py +++ b/src/sqlbuild/compiler/compile/_helpers/diagnostics/recovery.py @@ -20,7 +20,6 @@ ) from sqlbuild.compiler.compile._helpers.diagnostics.type_recovery import recover_output_types from sqlbuild.compiler.compile.models import ( - CompiledLineageSourceFact, CompiledModel, CompiledProject, CompilerDiagnostic, @@ -61,13 +60,17 @@ def recover_diagnostics(project: CompiledProject) -> CompiledProject: poisoned: dict[tuple[str, str], CompilerDiagnostic] = {} origins: dict[CompilerDiagnostic, tuple[str, str]] = {} projections: dict[str, set[str]] = {} + inferred_names: dict[str, set[str]] = {} + upstream_sources: dict[str, dict[str, set[tuple[str, str]]]] = {} for diagnostic in roots: model: CompiledModel | None = models.get(diagnostic.resource_name or "") missing: tuple[str, str | None] | None = missing_column(diagnostic.message) if model is None or missing is None: continue column, table = missing - if column not in {item.name for item in model.inferred_columns or ()}: + if model.name not in inferred_names: + inferred_names[model.name] = {item.name for item in model.inferred_columns or ()} + if column not in inferred_names[model.name]: continue if model.name not in projections: projections[model.name] = unaliased_output_columns( @@ -75,14 +78,9 @@ def recover_diagnostics(project: CompiledProject) -> CompiledProject: ) if column not in projections[model.name]: continue - upstream_columns: list[CompiledLineageSourceFact] = [] - for output in model.fast_lineage_columns or (): - if output.output_column == column: - upstream_columns.extend(output.upstream_columns) - if not any( - source.resource_name == table and source.column_name == column - for source in upstream_columns - ): + if model.name not in upstream_sources: + upstream_sources[model.name] = _upstream_sources_by_output(model=model) + if table is None or (table, column) not in upstream_sources[model.name].get(column, set()): continue suggestion: str | None = closest_column(name=column, columns=shapes.get(table or "", {})) if suggestion is not None: @@ -132,3 +130,12 @@ def recover_diagnostics(project: CompiledProject) -> CompiledProject: diagnostics=diagnostics, models=update_binding_models(models=project.models, diagnostics=diagnostics), ) + + +def _upstream_sources_by_output(*, model: CompiledModel) -> dict[str, set[tuple[str, str]]]: + sources: dict[str, set[tuple[str, str]]] = {} + for output in model.fast_lineage_columns or (): + sources.setdefault(output.output_column, set()).update( + (source.resource_name, source.column_name) for source in output.upstream_columns + ) + return sources diff --git a/src/sqlbuild/compiler/compile/_helpers/diagnostics/type_recovery.py b/src/sqlbuild/compiler/compile/_helpers/diagnostics/type_recovery.py index f0468fc25f..b0e3b165a2 100644 --- a/src/sqlbuild/compiler/compile/_helpers/diagnostics/type_recovery.py +++ b/src/sqlbuild/compiler/compile/_helpers/diagnostics/type_recovery.py @@ -1,5 +1,6 @@ """Propagate unknown output types from located root errors without opening other columns.""" +from bisect import bisect_right from collections import Counter, deque from dataclasses import replace from typing import Any @@ -72,16 +73,17 @@ def recover_output_types( columns: tuple[InferredColumn, ...] = model.inferred_columns or () if len(spans) != len(columns): continue + span_starts: list[int] = [start for start, _ in spans] for raw in result.diagnostics: if raw.start is None: continue root: CompilerDiagnostic | None = roots_by_model[model.name].get(raw) - if root is None or not root.is_error: - continue - for column, (start, end) in zip(columns, spans, strict=True): - if start <= raw.start < end: - poisoned[(model.name, column.name)] = root - outputs_by_diagnostic.setdefault(root, set()).add(column.name) + column: InferredColumn | None = _column_at( + columns=columns, spans=spans, span_starts=span_starts, offset=raw.start + ) + if root is not None and root.is_error and column is not None: + poisoned[(model.name, column.name)] = root + outputs_by_diagnostic.setdefault(root, set()).add(column.name) if not poisoned: return project for _ in project.models: @@ -257,6 +259,20 @@ def _request( ) +def _column_at( + *, + columns: tuple[InferredColumn, ...], + spans: list[tuple[int, int]], + span_starts: list[int], + offset: int, +) -> InferredColumn | None: + """Projection spans are ordered and disjoint, so bisect finds the only candidate.""" + position: int = bisect_right(span_starts, offset) - 1 + if position < 0 or offset >= spans[position][1]: + return None + return columns[position] + + def _projection_spans(*, sql: str, dialect: str | None) -> list[tuple[int, int]]: module: Any = import_polyglot_sql() tokens: list[dict[str, Any]] = module.tokenize(sql, dialect=dialect) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_diagnostic_compile_performance.py b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_diagnostic_compile_performance.py index 3d495a09fa..509a435296 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_diagnostic_compile_performance.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/compile/test_diagnostic_compile_performance.py @@ -17,6 +17,7 @@ write_cascade_project, write_diagnostic_project, write_function_name_project, + write_single_model_diagnostic_project, ) from tests.e2e.src.sqlbuild.cli.commands.main.compile._test_types import DiagnosticPerformanceCase @@ -71,6 +72,36 @@ def test_given_many_spans_on_large_sql_when_mapping_then_scales_linearly( assert doubled <= single * 2.8 + 0.1 +@pytest.mark.parametrize( + "test_case", + [ + DiagnosticPerformanceCase( + "many_diagnostics_in_one_model", + diagnostic_count=1000, + expected_max_wall_seconds=20.0, + ) + ], + ids=lambda case: case.description, +) +def test_given_one_model_with_many_diagnostics_when_compiling_then_scales_linearly( + test_case: DiagnosticPerformanceCase, tmp_path: Path +) -> None: + timings: list[float] = [] + for multiplier in (1, 4): + project: Path = tmp_path / f"diagnostics_{multiplier}" + count: int = test_case.diagnostic_count * multiplier + write_single_model_diagnostic_project(project_dir=project, diagnostics=count) + elapsed, payload = measure_diagnostic_compile( + project_dir=project, timeout=test_case.expected_timeout_seconds + ) + codes: Counter[str] = Counter(item["code"] for item in payload["diagnostics"]) + assert codes == Counter({"B002": count // 2, "B218": count // 2}) + timings.append(elapsed) + _LOGGER.info("single-model diagnostics 1x/4x wall=%s", timings) + assert max(timings) < test_case.expected_max_wall_seconds + assert timings[1] <= timings[0] * 4.5 + 1.0 + + @pytest.mark.parametrize( "test_case", [ From 671791abd89fcc9f51e2e9817166501b267911ac Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 1 Oct 2026 20:00:30 +0100 Subject: [PATCH 3/5] fix(format): keep comma spacing before adjacent string literals When the layout oracle rewrote a substituted region, for example DuckDB 'north' 'east' printed as CONCAT('north', 'east'), the token after the region took the oracle's spacing after the CONCAT( it introduced, so `replace, 'north'` became `replace,'north'` and lost its line break. The spacing now comes from the oracle's own copy of the authored token that precedes it. --- .../src/sql_lint/_helpers/token_layout.rs | 5 ++++- .../src/sql_lint/tests/test_authored_tokens.rs | 12 ++++++++++++ .../commands/main/format/test_token_preservation.py | 13 +++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/crates/sqlbuild-rules-native/src/sql_lint/_helpers/token_layout.rs b/crates/sqlbuild-rules-native/src/sql_lint/_helpers/token_layout.rs index 488b057c41..83be59d0bd 100644 --- a/crates/sqlbuild-rules-native/src/sql_lint/_helpers/token_layout.rs +++ b/crates/sqlbuild-rules-native/src/sql_lint/_helpers/token_layout.rs @@ -554,7 +554,10 @@ fn separators( " ".to_string() }; separators[next] = if substitution { - let oracle_gap = gap(next_at - 1, next_at); + let oracle_gap = (previous_at + 1..next_at) + .rev() + .find(|&at| generated[at].key == authored[last].key) + .map_or_else(|| gap(next_at - 1, next_at), |at| gap(at, at + 1)); let unspaced = default_space(&authored[last], &authored[next]).is_empty(); if unspaced && !oracle_gap.contains('\n') { String::new() diff --git a/crates/sqlbuild-rules-native/src/sql_lint/tests/test_authored_tokens.rs b/crates/sqlbuild-rules-native/src/sql_lint/tests/test_authored_tokens.rs index 05b8c4601d..3d98f99548 100644 --- a/crates/sqlbuild-rules-native/src/sql_lint/tests/test_authored_tokens.rs +++ b/crates/sqlbuild-rules-native/src/sql_lint/tests/test_authored_tokens.rs @@ -58,6 +58,18 @@ fn given_generator_restructuring_when_formatting_then_authored_tokens_are_laid_o sql: "select substring(code, 1, 2) as region from orders", expected_sql: "SELECT SUBSTRING(code, 1, 2) AS region\nFROM orders", }, + test_types::DialectFormatTestCase { + description: "DuckDB adjacent string literals after a quoted-in-oracle alias keep comma spacing", + dialect: "duckdb", + sql: "select sku as replace, 'north' 'east' as region, sku from products", + expected_sql: "SELECT\n sku AS replace,\n 'north' 'east' AS region,\n sku\nFROM products", + }, + test_types::DialectFormatTestCase { + description: "DuckDB adjacent string literals after a plain column keep comma spacing", + dialect: "duckdb", + sql: "select sku, 'north' 'east' as region from products", + expected_sql: "SELECT\n sku,\n 'north' 'east' AS region\nFROM products", + }, test_types::DialectFormatTestCase { description: "DuckDB SELECT ALL keeps ALL", dialect: "duckdb", diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/format/test_token_preservation.py b/tests/e2e/src/sqlbuild/cli/commands/main/format/test_token_preservation.py index 117b786ce7..c8cecc40ce 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/format/test_token_preservation.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/format/test_token_preservation.py @@ -37,6 +37,19 @@ "SELECT\n product_id, # on-hand units\n quantity\nFROM inventory.view\n" ), ), + TokenPreservationFormatTestCase( + description="DuckDB adjacent string literals keep the space after the comma", + project_toml='name = "products"\nadapter = "duckdb"\n', + model_name="product_regions", + authored_sql=( + 'MODEL (description "Product regions");\n\n' + "select sku as replace, 'north' 'east' as region from products\n" + ), + expected_sql=( + 'MODEL (description "Product regions");\n\n' + "SELECT\n sku AS replace,\n 'north' 'east' AS region\nFROM products\n" + ), + ), ], ids=lambda case: case.description, ) From 8a42599a370d8e790de684697a484f84f63198b3 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 1 Oct 2026 20:19:13 +0100 Subject: [PATCH 4/5] fix(dbt): apply sqb build plan safety policies on dbt run and build sqb dbt run and build enforced only the missing-origin and retention decrease policies before executing SQLBuild work, so a table_type_downgrade, snapshot full-refresh or max_models policy could be bypassed through the dbt interop path. Both commands now share the sqb build gate sequence. sqb dbt has no allow flags: a policy that needs confirmation prompts on a terminal and otherwise refuses with guidance naming the matching sqb build flag or policy setting. --- .../sqlbuild/references/docs/cli/dbt.md | 11 ++ .../_helpers/build_planning/defer_clone.py | 2 + .../build_planning/execution_limits.py | 3 +- .../_helpers/build_planning/full_refresh.py | 3 +- .../_helpers/build_planning/plan_policies.py | 100 +++++++++++--- .../build_planning/retention_decrease.py | 3 +- .../_helpers/build_planning/table_type.py | 3 +- .../commands/_helpers/dbt/sqlbuild_work.py | 24 ++-- src/sqlbuild/cli/commands/constants.py | 26 ++++ src/sqlbuild/cli/commands/models.py | 24 +++- .../dbt/_helpers/pipeline/execution_phases.py | 20 +++ .../cli/commands/main/dbt/_test_types.py | 8 ++ .../sqlbuild/cli/commands/main/dbt/helpers.py | 31 +++++ .../cli/commands/main/dbt/test_plan_safety.py | 115 ++++++++++++++++ .../_helpers/build_planning/_test_types.py | 11 ++ .../build_planning/test_execution_limits.py | 3 + .../build_planning/test_full_refresh.py | 3 + .../build_planning/test_plan_policies.py | 128 ++++++++++++++++++ .../build_planning/test_retention_decrease.py | 3 + .../build_planning/test_table_type.py | 3 + website/src/content/docs/docs/cli/dbt.mdx | 11 ++ 21 files changed, 496 insertions(+), 39 deletions(-) create mode 100644 tests/e2e/src/sqlbuild/cli/commands/main/dbt/test_plan_safety.py create mode 100644 tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_plan_policies.py diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/dbt.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/dbt.md index 676c9733f5..e645276c60 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/dbt.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/dbt.md @@ -34,6 +34,17 @@ Build dbt models first (including dbt tests), then SQLBuild models with audits. sqb dbt build [--select ...] [--exclude ...] ``` +### Plan safety policies + +After dbt finishes, `sqb dbt run` and `sqb dbt build` apply the same plan safety policies as +[`sqb build`](build.md) before any SQLBuild model executes: blocked model and column +migrations, compatibility-view conflicts, the target's `execution_limits.max_models`, the +snapshot full-refresh policy, `table_type_downgrade`, `time_travel_retention_decrease`, and +`missing_migration_origin`. `sqb dbt` has no `--allow-*` flags. A policy that requires +confirmation asks for the typed confirmation on an interactive terminal and refuses otherwise; +build the affected models with `sqb build` and its allow flag instead, or relax the policy. A +refusal at this point leaves completed dbt work in place and builds no SQLBuild models. + ## sqb dbt debug Run dbt diagnostics followed by SQLBuild diagnostics. diff --git a/src/sqlbuild/cli/commands/_helpers/build_planning/defer_clone.py b/src/sqlbuild/cli/commands/_helpers/build_planning/defer_clone.py index c433987284..eb14232679 100644 --- a/src/sqlbuild/cli/commands/_helpers/build_planning/defer_clone.py +++ b/src/sqlbuild/cli/commands/_helpers/build_planning/defer_clone.py @@ -13,6 +13,7 @@ from sqlbuild.cli.commands._helpers.planning.external_refs import ( resolve_external_sql_reference_resolver, ) +from sqlbuild.cli.commands.constants import EXECUTION_LIMIT_BUILD_NOTE from sqlbuild.cli.commands.exceptions import CliUserError from sqlbuild.cli.commands.models import ( BuildCommandRequest, @@ -196,6 +197,7 @@ def run_defer_clone_boundary_prephase( model_count=selected_model_count, target_name=invocation.effective_target_name, limits=invocation.execution_limits, + refusal_note=EXECUTION_LIMIT_BUILD_NOTE, ) run_defer_clone_prephase( inputs=DeferClonePrephaseInputs( diff --git a/src/sqlbuild/cli/commands/_helpers/build_planning/execution_limits.py b/src/sqlbuild/cli/commands/_helpers/build_planning/execution_limits.py index fdacb3efbd..f73b3b6d87 100644 --- a/src/sqlbuild/cli/commands/_helpers/build_planning/execution_limits.py +++ b/src/sqlbuild/cli/commands/_helpers/build_planning/execution_limits.py @@ -19,6 +19,7 @@ def enforce_model_execution_limit( model_count: int, target_name: str | None, limits: ExecutionLimitsConfig, + refusal_note: str, ) -> None: """Reject a build whose expanded model count exceeds its target policy.""" @@ -33,7 +34,7 @@ def enforce_model_execution_limit( f"Maximum models: {maximum}\n" "Configuration: " f"targets.{target_label}.execution_limits.max_models\n\n" - "No warehouse changes were made.", + f"{refusal_note}", code="C413", help=limits.remediation, ) diff --git a/src/sqlbuild/cli/commands/_helpers/build_planning/full_refresh.py b/src/sqlbuild/cli/commands/_helpers/build_planning/full_refresh.py index c55457e488..0dcce87c5a 100644 --- a/src/sqlbuild/cli/commands/_helpers/build_planning/full_refresh.py +++ b/src/sqlbuild/cli/commands/_helpers/build_planning/full_refresh.py @@ -25,6 +25,7 @@ def enforce_snapshot_full_refresh_policy( plan: PlanOutput, snapshots_config: SnapshotsConfig, allow_snapshot_full_refresh: bool, + non_interactive_help: str, input_stream: TextIO, output_stream: TextIO, ) -> None: @@ -69,7 +70,7 @@ def enforce_snapshot_full_refresh_policy( raise CliUserError( "snapshot full refresh requires confirmation", code="C239", - help="Pass --allow-snapshot-full-refresh to confirm in non-interactive runs.", + help=non_interactive_help, ) if not _confirm( diff --git a/src/sqlbuild/cli/commands/_helpers/build_planning/plan_policies.py b/src/sqlbuild/cli/commands/_helpers/build_planning/plan_policies.py index 152ea7bb45..dbbbaac063 100644 --- a/src/sqlbuild/cli/commands/_helpers/build_planning/plan_policies.py +++ b/src/sqlbuild/cli/commands/_helpers/build_planning/plan_policies.py @@ -19,9 +19,20 @@ from sqlbuild.cli.commands._helpers.build_planning.table_type import ( enforce_table_type_downgrade_policy, ) -from sqlbuild.cli.commands.constants import MISSING_ORIGIN_BUILD_HELP +from sqlbuild.cli.commands.constants import ( + EXECUTION_LIMIT_BUILD_NOTE, + EXECUTION_LIMIT_DBT_NOTE, + MISSING_ORIGIN_BUILD_HELP, + MISSING_ORIGIN_DBT_HELP, + RETENTION_DECREASE_BUILD_HELP, + RETENTION_DECREASE_DBT_HELP, + SNAPSHOT_FULL_REFRESH_BUILD_HELP, + SNAPSHOT_FULL_REFRESH_DBT_HELP, + TABLE_TYPE_DOWNGRADE_BUILD_HELP, + TABLE_TYPE_DOWNGRADE_DBT_HELP, +) from sqlbuild.cli.commands.exceptions import CliUserError -from sqlbuild.cli.commands.models import BuildCommandRequest, BuildInvocation +from sqlbuild.cli.commands.models import BuildCommandRequest, BuildInvocation, PlanSafetyGates from sqlbuild.compiler.migrations.types import ColumnMigrationDecision, MigrationDecision from sqlbuild.compiler.planner.constants import HIDDEN_ORIGIN_REMEDY from sqlbuild.compiler.planner.models import ( @@ -31,6 +42,7 @@ PlanWarning, ) from sqlbuild.compiler.planner.types import WarningSeverity +from sqlbuild.spec.contracts.models import ExecutionLimitsConfig, SnapshotsConfig from sqlbuild.spec.contracts.types import MissingMigrationOriginPolicy _OLD_NAME_CODES: frozenset[str] = frozenset({"M114", "P008"}) @@ -44,40 +56,92 @@ def enforce_build_plan_policies( *, request: BuildCommandRequest, invocation: BuildInvocation, plan: PlanOutput +) -> None: + """Apply every plan safety gate before a direct build executes anything.""" + + enforce_plan_safety_policies( + plan=plan, + gates=PlanSafetyGates( + allow_missing_migration_origin=request.allow_missing_migration_origin, + allow_snapshot_full_refresh=request.allow_snapshot_full_refresh, + allow_table_type_downgrade=request.allow_table_type_downgrade, + allow_retention_decrease=request.allow_retention_decrease, + missing_origin_help=MISSING_ORIGIN_BUILD_HELP, + snapshot_full_refresh_help=SNAPSHOT_FULL_REFRESH_BUILD_HELP, + table_type_downgrade_help=TABLE_TYPE_DOWNGRADE_BUILD_HELP, + retention_decrease_help=RETENTION_DECREASE_BUILD_HELP, + execution_limit_note=EXECUTION_LIMIT_BUILD_NOTE, + ), + snapshots_config=invocation.discovered_inputs.project_config.snapshots, + target_name=invocation.effective_target_name, + execution_limits=invocation.execution_limits, + streams=(sys.stdin, sys.stdout), + ) + + +def dbt_interop_plan_safety_gates() -> PlanSafetyGates: + """Gates for sqb dbt, which has no allow flags and confirms only on a terminal.""" + + return PlanSafetyGates( + allow_missing_migration_origin=False, + allow_snapshot_full_refresh=False, + allow_table_type_downgrade=False, + allow_retention_decrease=False, + missing_origin_help=MISSING_ORIGIN_DBT_HELP, + snapshot_full_refresh_help=SNAPSHOT_FULL_REFRESH_DBT_HELP, + table_type_downgrade_help=TABLE_TYPE_DOWNGRADE_DBT_HELP, + retention_decrease_help=RETENTION_DECREASE_DBT_HELP, + execution_limit_note=EXECUTION_LIMIT_DBT_NOTE, + ) + + +def enforce_plan_safety_policies( + *, + plan: PlanOutput, + gates: PlanSafetyGates, + snapshots_config: SnapshotsConfig, + target_name: str | None, + execution_limits: ExecutionLimitsConfig, + streams: tuple[TextIO, TextIO], ) -> None: """Apply migration, execution-limit, and storage safety gates in their required order.""" + input_stream, output_stream = streams enforce_migration_plan_policies( plan=plan, - allow_missing_migration_origin=request.allow_missing_migration_origin, - non_interactive_help=MISSING_ORIGIN_BUILD_HELP, - input_stream=sys.stdin, - output_stream=sys.stdout, + allow_missing_migration_origin=gates.allow_missing_migration_origin, + non_interactive_help=gates.missing_origin_help, + input_stream=input_stream, + output_stream=output_stream, ) _enforce_old_name_policy(plan=plan) enforce_model_execution_limit( model_count=executable_model_count(plan=plan), - target_name=invocation.effective_target_name, - limits=invocation.execution_limits, + target_name=target_name, + limits=execution_limits, + refusal_note=gates.execution_limit_note, ) enforce_snapshot_full_refresh_policy( plan=plan, - snapshots_config=invocation.discovered_inputs.project_config.snapshots, - allow_snapshot_full_refresh=request.allow_snapshot_full_refresh, - input_stream=sys.stdin, - output_stream=sys.stdout, + snapshots_config=snapshots_config, + allow_snapshot_full_refresh=gates.allow_snapshot_full_refresh, + non_interactive_help=gates.snapshot_full_refresh_help, + input_stream=input_stream, + output_stream=output_stream, ) enforce_table_type_downgrade_policy( plan=plan, - allow_table_type_downgrade=request.allow_table_type_downgrade, - input_stream=sys.stdin, - output_stream=sys.stdout, + allow_table_type_downgrade=gates.allow_table_type_downgrade, + non_interactive_help=gates.table_type_downgrade_help, + input_stream=input_stream, + output_stream=output_stream, ) enforce_retention_decrease_policy( plan=plan, - allow_retention_decrease=request.allow_retention_decrease, - input_stream=sys.stdin, - output_stream=sys.stdout, + allow_retention_decrease=gates.allow_retention_decrease, + non_interactive_help=gates.retention_decrease_help, + input_stream=input_stream, + output_stream=output_stream, ) diff --git a/src/sqlbuild/cli/commands/_helpers/build_planning/retention_decrease.py b/src/sqlbuild/cli/commands/_helpers/build_planning/retention_decrease.py index 2ae5bda812..c321c59df4 100644 --- a/src/sqlbuild/cli/commands/_helpers/build_planning/retention_decrease.py +++ b/src/sqlbuild/cli/commands/_helpers/build_planning/retention_decrease.py @@ -13,6 +13,7 @@ def enforce_retention_decrease_policy( *, plan: PlanOutput, allow_retention_decrease: bool, + non_interactive_help: str, input_stream: TextIO, output_stream: TextIO, ) -> None: @@ -42,7 +43,7 @@ def enforce_retention_decrease_policy( if not input_stream.isatty(): raise CliUserError( "time travel retention decrease requires confirmation", - help="Pass --allow-retention-decrease to confirm in non-interactive runs.", + help=non_interactive_help, ) expected: str = _confirmation_text(confirmation) output_stream.write( diff --git a/src/sqlbuild/cli/commands/_helpers/build_planning/table_type.py b/src/sqlbuild/cli/commands/_helpers/build_planning/table_type.py index 8a24d39fe9..146af850fd 100644 --- a/src/sqlbuild/cli/commands/_helpers/build_planning/table_type.py +++ b/src/sqlbuild/cli/commands/_helpers/build_planning/table_type.py @@ -13,6 +13,7 @@ def enforce_table_type_downgrade_policy( *, plan: PlanOutput, allow_table_type_downgrade: bool, + non_interactive_help: str, input_stream: TextIO, output_stream: TextIO, ) -> None: @@ -38,7 +39,7 @@ def enforce_table_type_downgrade_policy( if not input_stream.isatty(): raise CliUserError( "table-type downgrade requires confirmation", - help="Pass --allow-table-type-downgrade to confirm in non-interactive runs.", + help=non_interactive_help, ) expected: str = _confirmation_text(confirmation) output_stream.write( diff --git a/src/sqlbuild/cli/commands/_helpers/dbt/sqlbuild_work.py b/src/sqlbuild/cli/commands/_helpers/dbt/sqlbuild_work.py index 3e8c27f173..a338b0337e 100644 --- a/src/sqlbuild/cli/commands/_helpers/dbt/sqlbuild_work.py +++ b/src/sqlbuild/cli/commands/_helpers/dbt/sqlbuild_work.py @@ -8,16 +8,13 @@ from sqlbuild.adapter.contract.classes.base_adapter import BaseAdapter from sqlbuild.cli.commands._helpers.build_planning.plan_policies import ( - enforce_migration_plan_policies, -) -from sqlbuild.cli.commands._helpers.build_planning.retention_decrease import ( - enforce_retention_decrease_policy, + dbt_interop_plan_safety_gates, + enforce_plan_safety_policies, ) from sqlbuild.cli.commands.classes.build_progress_callbacks import ( BuildProgressCallbacks, format_build_footer, ) -from sqlbuild.cli.commands.constants import MISSING_ORIGIN_DBT_HELP from sqlbuild.cli.commands.models import DbtSqlbuildWorkContext from sqlbuild.cli.progress.classes.connection_progress_reporter import ConnectionProgressReporter from sqlbuild.cli.progress.main._execution_header import format_execution_header @@ -54,18 +51,13 @@ def execute_sqlbuild_build_work( adapter_name: str = context.adapter_name output_stream: TextIO = context.output_stream use_color: bool = context.use_color - enforce_migration_plan_policies( - plan=plan_output, - allow_missing_migration_origin=False, - non_interactive_help=MISSING_ORIGIN_DBT_HELP, - input_stream=sys.stdin, - output_stream=output_stream, - ) - enforce_retention_decrease_policy( + enforce_plan_safety_policies( plan=plan_output, - allow_retention_decrease=False, - input_stream=sys.stdin, - output_stream=output_stream, + gates=dbt_interop_plan_safety_gates(), + snapshots_config=context.snapshots_config, + target_name=project.effective_target_name, + execution_limits=context.execution_limits, + streams=(sys.stdin, output_stream), ) callbacks: BuildProgressCallbacks = BuildProgressCallbacks( plan=plan_output, use_color=use_color, verbose=verbose, debug=False diff --git a/src/sqlbuild/cli/commands/constants.py b/src/sqlbuild/cli/commands/constants.py index ad8a34121e..9aab032ea6 100644 --- a/src/sqlbuild/cli/commands/constants.py +++ b/src/sqlbuild/cli/commands/constants.py @@ -111,4 +111,30 @@ "models with sqb build --allow-missing-migration-origin and leave them out of this dbt " "selection, or set the target's missing_migration_origin to 'allow'." ) +SNAPSHOT_FULL_REFRESH_BUILD_HELP: str = ( + "Pass --allow-snapshot-full-refresh to confirm in non-interactive runs." +) +SNAPSHOT_FULL_REFRESH_DBT_HELP: str = ( + "sqb dbt has no confirmation flag: confirm on an interactive terminal, build the affected " + "snapshots with sqb build --allow-snapshot-full-refresh and leave them out of this dbt " + "selection, or relax the snapshot_full_refresh policy." +) +TABLE_TYPE_DOWNGRADE_BUILD_HELP: str = ( + "Pass --allow-table-type-downgrade to confirm in non-interactive runs." +) +TABLE_TYPE_DOWNGRADE_DBT_HELP: str = ( + "sqb dbt has no confirmation flag: confirm on an interactive terminal, build the affected " + "models with sqb build --allow-table-type-downgrade and leave them out of this dbt " + "selection, or set the target's table_type_downgrade to 'allow'." +) +RETENTION_DECREASE_BUILD_HELP: str = ( + "Pass --allow-retention-decrease to confirm in non-interactive runs." +) +RETENTION_DECREASE_DBT_HELP: str = ( + "sqb dbt has no confirmation flag: confirm on an interactive terminal, build the affected " + "models with sqb build --allow-retention-decrease and leave them out of this dbt " + "selection, or set the target's time_travel_retention_decrease to 'allow'." +) +EXECUTION_LIMIT_BUILD_NOTE: str = "No warehouse changes were made." +EXECUTION_LIMIT_DBT_NOTE: str = "dbt work has already run; no SQLBuild models were built." QUERY_DIFF_INCOMPLETE_EXECUTION_CODES: frozenset[str] = frozenset({"C237", "C238", "C239"}) diff --git a/src/sqlbuild/cli/commands/models.py b/src/sqlbuild/cli/commands/models.py index 00ec60585e..906928cc66 100644 --- a/src/sqlbuild/cli/commands/models.py +++ b/src/sqlbuild/cli/commands/models.py @@ -83,7 +83,12 @@ from sqlbuild.presentation.classes.transient_status_reporter import TransientStatusReporter from sqlbuild.python_nodes.models import SqlResourceRef from sqlbuild.runtime.contracts.types import NodeStartCallback -from sqlbuild.spec.contracts.models import CostConfig, ExecutionLimitsConfig, SourceEntry +from sqlbuild.spec.contracts.models import ( + CostConfig, + ExecutionLimitsConfig, + SnapshotsConfig, + SourceEntry, +) @dataclass(frozen=True) @@ -390,6 +395,23 @@ class DbtSqlbuildWorkContext: adapter_name: str output_stream: TextIO use_color: bool + snapshots_config: SnapshotsConfig + execution_limits: ExecutionLimitsConfig + + +@dataclass(frozen=True) +class PlanSafetyGates: + """Pre-confirmed plan safety gates and the guidance shown when one needs a terminal.""" + + allow_missing_migration_origin: bool + allow_snapshot_full_refresh: bool + allow_table_type_downgrade: bool + allow_retention_decrease: bool + missing_origin_help: str + snapshot_full_refresh_help: str + table_type_downgrade_help: str + retention_decrease_help: str + execution_limit_note: str @dataclass(frozen=True) diff --git a/src/sqlbuild/integrations/dbt/_helpers/pipeline/execution_phases.py b/src/sqlbuild/integrations/dbt/_helpers/pipeline/execution_phases.py index b684757d49..5c1ab0793e 100644 --- a/src/sqlbuild/integrations/dbt/_helpers/pipeline/execution_phases.py +++ b/src/sqlbuild/integrations/dbt/_helpers/pipeline/execution_phases.py @@ -11,6 +11,7 @@ from sqlbuild.cli.commands.models import DbtSqlbuildWorkContext from sqlbuild.cli.progress.classes.connection_progress_reporter import ConnectionProgressReporter from sqlbuild.compiler.compile.main.effective_config import build_effective_connection_config +from sqlbuild.compiler.discovery.models import DiscoveredProjectInputs from sqlbuild.compiler.planner.models import PlanOutput from sqlbuild.integrations.dbt._helpers.pipeline.execute import ( execute_dbt_commands, @@ -34,6 +35,8 @@ ) from sqlbuild.presentation.models import DisplayOptions from sqlbuild.runtime.contracts.models import ConnectionHooks +from sqlbuild.spec.contracts.main.resolve_target_config import resolve_target_config +from sqlbuild.spec.contracts.models import ExecutionLimitsConfig def resolve_dbt_connection_config( @@ -190,6 +193,11 @@ def run_dbt_sqlbuild_work( adapter_name=compiled.adapter_name, output_stream=invocation.output_stream, use_color=request.use_color, + snapshots_config=invocation.discovered_inputs.project_config.snapshots, + execution_limits=_execution_limits( + discovered_inputs=invocation.discovered_inputs, + target_name=compiled.project.effective_target_name, + ), ), command=request.command, project=compiled.project, @@ -207,3 +215,15 @@ def write_sqlbuild_skip_notice( invocation.output_stream.write("\n") invocation.output_stream.flush() report_progress(on_progress=request.on_progress, message=message) + + +def _execution_limits( + *, discovered_inputs: DiscoveredProjectInputs, target_name: str | None +) -> ExecutionLimitsConfig: + if target_name is None: + return ExecutionLimitsConfig() + return resolve_target_config( + project_config=discovered_inputs.project_config, + local_config=discovered_inputs.local_config, + target_name=target_name, + ).execution_limits diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/dbt/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/dbt/_test_types.py index a6c9f07cff..dee4c8baf4 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/dbt/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/dbt/_test_types.py @@ -114,3 +114,11 @@ class DbtMissingOriginE2ETestCase: description: str command: tuple[str, ...] expected_output: str + + +@dataclass(frozen=True) +class DbtPlanSafetyE2ETestCase: + description: str + command: tuple[str, ...] + expected_fragments: tuple[str, ...] + unexpected_fragments: tuple[str, ...] diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/dbt/helpers.py b/tests/e2e/src/sqlbuild/cli/commands/main/dbt/helpers.py index 3cb72f0257..e8ab8658ae 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/dbt/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/dbt/helpers.py @@ -207,3 +207,34 @@ def _assert_dbt_local_replay_row_query( db_path: Path = project_dir / "target" / "run" / "scenarios" / scenario_name / "local.duckdb" rows: list[tuple[object, ...]] = query_duckdb(db_path=db_path, sql=rows_sql) assert tuple(rows) == expected_rows + + +def declare_sqlbuild_main_target(*, project_dir: Path, target_settings: str) -> None: + """Make `main` the default SQLBuild target of a dbt interop fixture with extra settings.""" + + config_path: Path = project_dir / "sqlbuild_project.toml" + config_path.write_text( + config_path.read_text(encoding="utf-8").replace( + 'adapter = "duckdb"\n', 'adapter = "duckdb"\ndefault_target = "main"\n' + ) + + '\n[targets.main]\nschema = "main"\n' + + target_settings, + encoding="utf-8", + ) + + +def write_sqlbuild_order_status_snapshot(*, project_dir: Path) -> None: + """Add a current-state SQLBuild snapshot whose full refresh needs confirmation.""" + + config_path: Path = project_dir / "sqlbuild_project.toml" + config_path.write_text( + config_path.read_text(encoding="utf-8") + + '\n[snapshots]\ncurrent_state_full_refresh = "require_confirmation"\n', + encoding="utf-8", + ) + project_dir.joinpath("models", "order_status_history.sql").write_text( + "MODEL (\n materialized snapshot,\n unique_key [order_id],\n" + " snapshot_strategy check,\n check_columns [status],\n);\n\n" + "SELECT 10 AS order_id, 'open' AS status\n", + encoding="utf-8", + ) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/dbt/test_plan_safety.py b/tests/e2e/src/sqlbuild/cli/commands/main/dbt/test_plan_safety.py new file mode 100644 index 0000000000..4a799e9533 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/dbt/test_plan_safety.py @@ -0,0 +1,115 @@ +"""E2E coverage for sqb build plan safety policies on the dbt interop execution path.""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.dbt._test_types import DbtPlanSafetyE2ETestCase +from tests.e2e.src.sqlbuild.cli.commands.main.dbt.helpers import ( + declare_sqlbuild_main_target, + prepare_dbt_interop_project, + skip_unless_dbt_is_runnable, + write_sqlbuild_order_status_snapshot, +) +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import run_sqb, table_exists + +pytestmark: pytest.MarkDecorator = pytest.mark.dbt + + +@pytest.mark.parametrize( + "test_case", + [ + DbtPlanSafetyE2ETestCase( + description="dbt run refuses SQLBuild work above the target model limit", + command=("--no-color", "dbt", "run", "--select", "local_only", "local_order_ids"), + expected_fragments=( + "C413", + "Selected models: 2", + "Maximum models: 1", + "dbt work has already run; no SQLBuild models were built.", + ), + unexpected_fragments=("No warehouse changes were made.",), + ), + ], + ids=lambda case: case.description, +) +def test_given_target_model_limit_when_running_dbt_interop_then_refuses_sqlbuild_work( + test_case: DbtPlanSafetyE2ETestCase, tmp_path: Path +) -> None: + skip_unless_dbt_is_runnable() + project_dir: Path = prepare_dbt_interop_project(tmp_path=tmp_path) + declare_sqlbuild_main_target( + project_dir=project_dir, + target_settings="\n[targets.main.execution_limits]\nmax_models = 1\n", + ) + project_dir.joinpath("models", "local_order_ids.sql").write_text( + "MODEL (materialized table);\n\nSELECT 11 AS order_id\n", encoding="utf-8" + ) + + result: subprocess.CompletedProcess[str] = run_sqb( + command=test_case.command, project_dir=project_dir, input_text="" + ) + + 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 not any(fragment in output for fragment in test_case.unexpected_fragments), output + database: Path = project_dir / "dbt_interop.duckdb" + assert not table_exists(db_path=database, table_name="local_only") + assert not table_exists(db_path=database, table_name="local_order_ids") + + +@pytest.mark.parametrize( + "test_case", + [ + DbtPlanSafetyE2ETestCase( + description="dbt run refuses an unconfirmed snapshot full refresh", + command=( + "--no-color", + "dbt", + "run", + "--full-refresh", + "--select", + "order_status_history", + ), + expected_fragments=( + "snapshot full refresh requires confirmation", + "sqb build --allow-snapshot-full-refresh", + ), + unexpected_fragments=("Pass --allow-snapshot-full-refresh",), + ), + ], + ids=lambda case: case.description, +) +def test_given_snapshot_full_refresh_requiring_confirmation_when_running_dbt_then_refuses( + test_case: DbtPlanSafetyE2ETestCase, tmp_path: Path +) -> None: + skip_unless_dbt_is_runnable() + project_dir: Path = prepare_dbt_interop_project(tmp_path=tmp_path) + declare_sqlbuild_main_target(project_dir=project_dir, target_settings="") + write_sqlbuild_order_status_snapshot(project_dir=project_dir) + first: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", "dbt", "run", "--select", "order_status_history"), + project_dir=project_dir, + input_text="", + ) + assert first.returncode == 0, first.stdout + first.stderr + + result: subprocess.CompletedProcess[str] = run_sqb( + command=test_case.command, project_dir=project_dir, input_text="" + ) + + 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 not any(fragment in output for fragment in test_case.unexpected_fragments), output + assert table_exists( + db_path=project_dir / "dbt_interop.duckdb", table_name="order_status_history" + ) + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/_test_types.py b/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/_test_types.py index 9c030abe85..52a57aba83 100644 --- a/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/_test_types.py +++ b/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/_test_types.py @@ -57,3 +57,14 @@ class RetentionDecreasePolicyTestCase: expected_output: str = "" input_text: str = "" input_is_tty: bool = False + + +@dataclass(frozen=True) +class DbtPlanSafetyPolicyTestCase: + description: str + plan_output: PlanOutput + expected_error_fragment: str | None = None + expected_help_fragment: str = "" + expected_output: str = "" + input_text: str = "" + input_is_tty: bool = False diff --git a/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_execution_limits.py b/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_execution_limits.py index d6080c2b89..12f300399f 100644 --- a/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_execution_limits.py +++ b/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_execution_limits.py @@ -6,6 +6,7 @@ enforce_model_execution_limit, ) from sqlbuild.cli.commands._helpers.build_planning.invocation import resolve_build_invocation +from sqlbuild.cli.commands.constants import EXECUTION_LIMIT_BUILD_NOTE from sqlbuild.cli.commands.exceptions import CliUserError from sqlbuild.cli.commands.models import BuildCommandRequest from sqlbuild.spec.contracts.models import ExecutionLimitsConfig @@ -34,6 +35,7 @@ def test_given_model_count_within_limit_when_enforcing_then_build_is_allowed( model_count=test_case.model_count, target_name="dev", limits=ExecutionLimitsConfig(max_models=test_case.maximum_models), + refusal_note=EXECUTION_LIMIT_BUILD_NOTE, ) assert result is test_case.expected_code @@ -65,6 +67,7 @@ def test_given_model_count_above_limit_when_enforcing_then_error_has_custom_reme max_models=test_case.maximum_models, remediation=test_case.remediation, ), + refusal_note=EXECUTION_LIMIT_BUILD_NOTE, ) assert raised.value.code == test_case.expected_code diff --git a/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_full_refresh.py b/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_full_refresh.py index 077057e2df..4fb59e14a1 100644 --- a/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_full_refresh.py +++ b/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_full_refresh.py @@ -9,6 +9,7 @@ from sqlbuild.cli.commands._helpers.build_planning.full_refresh import ( enforce_snapshot_full_refresh_policy, ) +from sqlbuild.cli.commands.constants import SNAPSHOT_FULL_REFRESH_BUILD_HELP from sqlbuild.cli.commands.exceptions import CliUserError from sqlbuild.compiler.planner.models import PlanOutput from sqlbuild.spec.contracts.models import SnapshotsConfig @@ -82,6 +83,7 @@ def test_given_unsafe_snapshot_full_refresh_when_enforcing_policy_then_raises_us plan=test_case.plan_output, snapshots_config=test_case.snapshots_config, allow_snapshot_full_refresh=test_case.allow_snapshot_full_refresh, + non_interactive_help=SNAPSHOT_FULL_REFRESH_BUILD_HELP, input_stream=_InputStream(test_case.input_text, is_tty=test_case.input_is_tty), output_stream=StringIO(), ) @@ -140,6 +142,7 @@ def test_given_safe_or_confirmed_snapshot_full_refresh_when_enforcing_policy_the plan=test_case.plan_output, snapshots_config=test_case.snapshots_config, allow_snapshot_full_refresh=test_case.allow_snapshot_full_refresh, + non_interactive_help=SNAPSHOT_FULL_REFRESH_BUILD_HELP, input_stream=_InputStream(test_case.input_text, is_tty=test_case.input_is_tty), output_stream=output_stream, ) diff --git a/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_plan_policies.py b/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_plan_policies.py new file mode 100644 index 0000000000..406e6f8b0d --- /dev/null +++ b/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_plan_policies.py @@ -0,0 +1,128 @@ +from __future__ import annotations + +from io import StringIO + +import pytest + +from sqlbuild.cli.commands._helpers.build_planning.plan_policies import ( + dbt_interop_plan_safety_gates, + enforce_plan_safety_policies, +) +from sqlbuild.cli.commands.exceptions import CliUserError +from sqlbuild.compiler.planner.models import PlanOutput +from sqlbuild.spec.contracts.models import ExecutionLimitsConfig, SnapshotsConfig +from tests.unit.src.sqlbuild.cli.commands._helpers.build_planning._test_types import ( + DbtPlanSafetyPolicyTestCase, +) +from tests.unit.src.sqlbuild.cli.commands._helpers.build_planning.helpers import ( + build_retention_entry, + build_snapshot_full_refresh_entry, + build_table_type_entry, +) + + +class _InputStream(StringIO): + def __init__(self, initial_value: str, *, is_tty: bool) -> None: + super().__init__(initial_value) + self._is_tty = is_tty + + def isatty(self) -> bool: + return self._is_tty + + +@pytest.mark.parametrize( + "test_case", + [ + DbtPlanSafetyPolicyTestCase( + description="unconfirmed table-type downgrade names the sqb build flag", + plan_output=PlanOutput(table_type_entries=(build_table_type_entry(),)), + expected_error_fragment="table-type downgrade requires confirmation", + expected_help_fragment="sqb build --allow-table-type-downgrade", + ), + DbtPlanSafetyPolicyTestCase( + description="denied table-type downgrade is refused even on a terminal", + plan_output=PlanOutput(table_type_entries=(build_table_type_entry(policy="deny"),)), + expected_error_fragment="table-type downgrade is denied for 'orders'", + input_is_tty=True, + ), + DbtPlanSafetyPolicyTestCase( + description="wrong typed table-type confirmation cancels", + plan_output=PlanOutput(table_type_entries=(build_table_type_entry(),)), + expected_error_fragment="table-type downgrade cancelled", + input_text="yes\n", + input_is_tty=True, + ), + DbtPlanSafetyPolicyTestCase( + description="unconfirmed retention decrease names the sqb build flag", + plan_output=PlanOutput( + retention_entries=(build_retention_entry(policy="require_confirmation"),) + ), + expected_error_fragment="time travel retention decrease requires confirmation", + expected_help_fragment="sqb build --allow-retention-decrease", + ), + DbtPlanSafetyPolicyTestCase( + description="unconfirmed historical snapshot full refresh names the sqb build flag", + plan_output=PlanOutput( + model_entries=(build_snapshot_full_refresh_entry(observed_at_column="seen_at"),) + ), + expected_error_fragment="snapshot full refresh requires confirmation", + expected_help_fragment="sqb build --allow-snapshot-full-refresh", + ), + ], + ids=lambda case: case.description, +) +def test_given_unsafe_plan_when_enforcing_dbt_interop_gates_then_refuses_with_dbt_guidance( + test_case: DbtPlanSafetyPolicyTestCase, +) -> None: + with pytest.raises(CliUserError) as raised: + enforce_plan_safety_policies( + plan=test_case.plan_output, + gates=dbt_interop_plan_safety_gates(), + snapshots_config=SnapshotsConfig(), + target_name="prod", + execution_limits=ExecutionLimitsConfig(), + streams=(_InputStream(test_case.input_text, is_tty=test_case.input_is_tty), StringIO()), + ) + + assert test_case.expected_error_fragment is not None + assert test_case.expected_error_fragment in raised.value.message + assert test_case.expected_help_fragment in (raised.value.help or "") + assert "Pass --allow-" not in (raised.value.help or "") + + +@pytest.mark.parametrize( + "test_case", + [ + DbtPlanSafetyPolicyTestCase( + description="typed confirmation on a terminal allows the downgrade", + plan_output=PlanOutput(table_type_entries=(build_table_type_entry(),)), + expected_output="Type `downgrade table type for orders` to continue: ", + input_text="downgrade table type for orders\n", + input_is_tty=True, + ), + DbtPlanSafetyPolicyTestCase( + description="allow policy proceeds without a prompt", + plan_output=PlanOutput(table_type_entries=(build_table_type_entry(policy="allow"),)), + ), + ], + ids=lambda case: case.description, +) +def test_given_confirmed_or_allowed_plan_when_enforcing_dbt_interop_gates_then_proceeds( + test_case: DbtPlanSafetyPolicyTestCase, +) -> None: + output_stream: StringIO = StringIO() + + enforce_plan_safety_policies( + plan=test_case.plan_output, + gates=dbt_interop_plan_safety_gates(), + snapshots_config=SnapshotsConfig(), + target_name="prod", + execution_limits=ExecutionLimitsConfig(), + streams=(_InputStream(test_case.input_text, is_tty=test_case.input_is_tty), output_stream), + ) + + assert test_case.expected_output in output_stream.getvalue() + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_retention_decrease.py b/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_retention_decrease.py index 563da4b4fc..1570d995eb 100644 --- a/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_retention_decrease.py +++ b/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_retention_decrease.py @@ -7,6 +7,7 @@ from sqlbuild.cli.commands._helpers.build_planning.retention_decrease import ( enforce_retention_decrease_policy, ) +from sqlbuild.cli.commands.constants import RETENTION_DECREASE_BUILD_HELP from sqlbuild.cli.commands.exceptions import CliUserError from sqlbuild.compiler.planner.models import PlanOutput from sqlbuild.compiler.planner.types import RetentionDirection @@ -80,6 +81,7 @@ def test_given_unsafe_retention_decrease_when_enforcing_then_raises_user_error( enforce_retention_decrease_policy( plan=test_case.plan_output, allow_retention_decrease=test_case.allow_retention_decrease, + non_interactive_help=RETENTION_DECREASE_BUILD_HELP, input_stream=_InputStream(test_case.input_text, is_tty=test_case.input_is_tty), output_stream=StringIO(), ) @@ -137,6 +139,7 @@ def test_given_safe_or_confirmed_retention_change_when_enforcing_then_allows_exe enforce_retention_decrease_policy( plan=test_case.plan_output, allow_retention_decrease=test_case.allow_retention_decrease, + non_interactive_help=RETENTION_DECREASE_BUILD_HELP, input_stream=_InputStream(test_case.input_text, is_tty=test_case.input_is_tty), output_stream=output_stream, ) diff --git a/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_table_type.py b/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_table_type.py index 3413958315..50f57acdae 100644 --- a/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_table_type.py +++ b/tests/unit/src/sqlbuild/cli/commands/_helpers/build_planning/test_table_type.py @@ -7,6 +7,7 @@ from sqlbuild.cli.commands._helpers.build_planning.table_type import ( enforce_table_type_downgrade_policy, ) +from sqlbuild.cli.commands.constants import TABLE_TYPE_DOWNGRADE_BUILD_HELP from sqlbuild.cli.commands.exceptions import CliUserError from sqlbuild.compiler.planner.models import PlanOutput from tests.unit.src.sqlbuild.cli.commands._helpers.build_planning._test_types import ( @@ -65,6 +66,7 @@ def test_given_unsafe_table_type_downgrade_when_enforcing_then_raises_user_error enforce_table_type_downgrade_policy( plan=test_case.plan_output, allow_table_type_downgrade=test_case.allow_table_type_downgrade, + non_interactive_help=TABLE_TYPE_DOWNGRADE_BUILD_HELP, input_stream=_InputStream(test_case.input_text, is_tty=test_case.input_is_tty), output_stream=StringIO(), ) @@ -124,6 +126,7 @@ def test_given_safe_or_confirmed_table_type_change_when_enforcing_then_allows_ex enforce_table_type_downgrade_policy( plan=test_case.plan_output, allow_table_type_downgrade=test_case.allow_table_type_downgrade, + non_interactive_help=TABLE_TYPE_DOWNGRADE_BUILD_HELP, input_stream=_InputStream(test_case.input_text, is_tty=test_case.input_is_tty), output_stream=output, ) diff --git a/website/src/content/docs/docs/cli/dbt.mdx b/website/src/content/docs/docs/cli/dbt.mdx index c99381b9a0..b04633f72b 100644 --- a/website/src/content/docs/docs/cli/dbt.mdx +++ b/website/src/content/docs/docs/cli/dbt.mdx @@ -33,6 +33,17 @@ Build dbt models first (including dbt tests), then SQLBuild models with audits. sqb dbt build [--select ...] [--exclude ...] ``` +### Plan safety policies + +After dbt finishes, `sqb dbt run` and `sqb dbt build` apply the same plan safety policies as +[`sqb build`](/docs/cli/build) before any SQLBuild model executes: blocked model and column +migrations, compatibility-view conflicts, the target's `execution_limits.max_models`, the +snapshot full-refresh policy, `table_type_downgrade`, `time_travel_retention_decrease`, and +`missing_migration_origin`. `sqb dbt` has no `--allow-*` flags. A policy that requires +confirmation asks for the typed confirmation on an interactive terminal and refuses otherwise; +build the affected models with `sqb build` and its allow flag instead, or relax the policy. A +refusal at this point leaves completed dbt work in place and builds no SQLBuild models. + ## sqb dbt debug Run dbt diagnostics followed by SQLBuild diagnostics. From 62fe1b864e3b95f1d2caaa9037abb0cd287675ac Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 1 Oct 2026 20:29:29 +0100 Subject: [PATCH 5/5] fix(scenario): run scenario fixture and check SQL as authored Scenario expected, assertion, helper and fixture SQL was parsed and regenerated with the generic dialect to swap relation markers, so dialect syntax was rewritten or rejected (S506) and the executed SQL differed from what was authored. Markers are now replaced in the authored text, skipping comments and quoted text under the project adapter's lexical rules, also for --local replay: backslash escapes on Snowflake, BigQuery and Databricks, E'...' strings on DuckDB and Postgres, nested block comments where the dialect nests them, and BigQuery '#' and Snowflake '//' line comments. A marker the scenario cannot resolve, or SQL with an unclosed string or comment, fails planning with S511 naming the scenario file instead of being left in the executed SQL. Local replay no longer regenerates SQL captured from DuckDB. --- Makefile | 1 + .../references/docs/concepts/scenarios.md | 3 +- .../adapter/contract/classes/base_adapter.py | 2 + .../contract/classes/duckdb_backed_adapter.py | 4 + .../bigquery/classes/bigquery_adapter.py | 6 + .../databricks/classes/databricks_adapter.py | 6 + .../postgres/classes/postgres_adapter.py | 4 + .../snowflake/classes/snowflake_adapter.py | 4 + .../sqlserver/classes/sqlserver_adapter.py | 2 + .../_helpers/scenario_execution/local_run.py | 8 +- .../_helpers/scenario_execution/runner.py | 9 +- .../compiler/planner/_helpers/resolve/refs.py | 44 +- .../compiler/planner/_helpers/scenario/cli.py | 3 + .../planner/_helpers/scenario/relations.py | 500 +++++------------- src/sqlbuild/compiler/planner/constants.py | 3 +- .../planner/main/scenarios/scenario.py | 3 + src/sqlbuild/compiler/planner/types.py | 4 - .../sql_analysis/_helpers/scanning.py | 94 ++++ .../compiler/sql_analysis/constants.py | 7 + .../main/_skip_dialect_non_code.py | 12 + src/sqlbuild/compiler/sql_analysis/models.py | 12 + .../executor/pipeline/_helpers/scenario.py | 16 +- .../executor/scenario/_helpers/local/sql.py | 11 +- src/sqlbuild/executor/scenario/constants.py | 1 + src/sqlbuild/executor/scenario/models.py | 10 + .../cli/commands/main/scenario/_test_types.py | 29 + .../cli/commands/main/scenario/helpers.py | 56 ++ .../main/scenario/test_authored_check_sql.py | 194 +++++++ .../compiler/planner/_helpers/_test_types.py | 31 +- .../planner/_helpers/test_scenario_cli.py | 2 + .../_helpers/test_scenario_relations.py | 372 ++++++++++--- .../sql_analysis/_helpers/_test_types.py | 43 ++ .../sql_analysis/_helpers/test_scanning.py | 229 ++++++++ .../executor/pipeline/_helpers/helpers.py | 4 +- .../pipeline/_helpers/test_scenario.py | 4 + .../executor/scenario/_helpers/_test_types.py | 8 + .../scenario/_helpers/test_local_sql.py | 44 ++ .../content/docs/docs/concepts/scenarios.mdx | 3 +- 38 files changed, 1310 insertions(+), 478 deletions(-) create mode 100644 src/sqlbuild/compiler/sql_analysis/main/_skip_dialect_non_code.py create mode 100644 tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_authored_check_sql.py create mode 100644 tests/unit/src/sqlbuild/executor/scenario/_helpers/test_local_sql.py diff --git a/Makefile b/Makefile index 4b634b5dec..6673fb6922 100644 --- a/Makefile +++ b/Makefile @@ -116,6 +116,7 @@ E2E_DUCKDB_BUILD_INCREMENTAL_PATHS := \ E2E_DUCKDB_CLI_DATA_PATHS := \ tests/e2e/src/sqlbuild/cli/commands/main/load \ tests/e2e/src/sqlbuild/cli/commands/main/providers \ + tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_authored_check_sql.py \ tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_scenario.py \ tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_capture.py \ tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_namespace.py diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md index 9e1727b616..cbc47164d4 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/scenarios.md @@ -269,7 +269,7 @@ This: 1. Checks snapshot freshness via the input fingerprint (missing/stale snapshots are skipped by default) 2. Creates a temporary DuckDB database at `target/run/scenarios//local.duckdb` 3. Loads JSONL snapshots into typed DuckDB tables using column metadata from `scenario.json` -4. Transpiles model and check SQL from the project adapter dialect to DuckDB +4. Transpiles model and check SQL from the project adapter dialect to DuckDB; SQL captured from a DuckDB project runs as authored 5. Builds functions, models, and runs expected/assertion checks in DuckDB 6. Keeps the local DuckDB file for inspection (it lives under `target/`, so it's always retained) @@ -329,3 +329,4 @@ See the [CLI reference](../cli/scenario.md) for full command documentation. - Custom materializations are not supported in scenarios yet. Scenario models using custom materializations will fail with a clear error. - Local replay transpiles SQL from the project adapter dialect to DuckDB. Adapter-specific SQL that cannot be translated will produce a clear error with the failing resource name and reason. +- Outside local replay, scenario fixture, expected and assertion SQL runs as authored: SQLBuild replaces `__ref`, `__source`, `__seed` and `__dbt_ref` calls with relation names and may wrap a partial fixture to add its missing columns, but never regenerates the query. Calls inside comments and quoted text are left unchanged. Quoted text and comments follow the project adapter's rules, including with `--local`: backslash escapes on Snowflake, BigQuery and Databricks, `E'...'` strings on DuckDB and Postgres, nested block comments on DuckDB, Postgres, Databricks and SQL Server, `#` line comments on BigQuery, and `//` line comments on Snowflake. A call naming a relation the scenario does not provide, or SQL with an unclosed string or comment, fails planning with `S511` naming the scenario file. diff --git a/src/sqlbuild/adapter/contract/classes/base_adapter.py b/src/sqlbuild/adapter/contract/classes/base_adapter.py index f0aff72de9..1880fed65f 100644 --- a/src/sqlbuild/adapter/contract/classes/base_adapter.py +++ b/src/sqlbuild/adapter/contract/classes/base_adapter.py @@ -75,6 +75,7 @@ ) from sqlbuild.compiler.compile.types import FunctionLanguage from sqlbuild.compiler.source_freshness.models import SourceFreshnessRecord +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from sqlbuild.spec.contracts.constants import DEFAULT_SEED_CSV_SETTINGS from sqlbuild.spec.contracts.models import SeedCsvSettings from sqlbuild.sql_values.exceptions import SqlValueRenderingError @@ -101,6 +102,7 @@ class BaseAdapter(RetentionAdapterMixin, StrictAdapter): adapter_name: ClassVar[str] sql_analysis_dialect_name: ClassVar[str | None] = None + sql_lexical_syntax: ClassVar[SqlLexicalSyntax] = SqlLexicalSyntax() max_identifier_length: ClassVar[int] = 63 state_tables_transient: ClassVar[bool] = False relation_grants_supported: ClassVar[bool] = True diff --git a/src/sqlbuild/adapter/contract/classes/duckdb_backed_adapter.py b/src/sqlbuild/adapter/contract/classes/duckdb_backed_adapter.py index 7766951cba..14958f616c 100644 --- a/src/sqlbuild/adapter/contract/classes/duckdb_backed_adapter.py +++ b/src/sqlbuild/adapter/contract/classes/duckdb_backed_adapter.py @@ -80,6 +80,7 @@ from sqlbuild.adapter.type_system.main.types_equal import types_equal from sqlbuild.compiler.compile.types import FunctionLanguage from sqlbuild.compiler.source_freshness.models import SourceFreshnessRecord +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from sqlbuild.diagnostics.main.log_sql import log_sql from sqlbuild.spec.contracts.constants import DEFAULT_SEED_CSV_SETTINGS from sqlbuild.spec.contracts.models import SeedCsvSettings @@ -100,6 +101,9 @@ def get_columns_for_relations( ) sql_analysis_dialect_name: ClassVar[str | None] = "duckdb" + sql_lexical_syntax: ClassVar[SqlLexicalSyntax] = SqlLexicalSyntax( + escape_string_prefix=True, nested_block_comments=True + ) relation_grants_supported: ClassVar[bool] = False def supports_zero_copy_clone(self) -> bool: diff --git a/src/sqlbuild/adapters/bigquery/classes/bigquery_adapter.py b/src/sqlbuild/adapters/bigquery/classes/bigquery_adapter.py index 595820a0f5..b870624511 100644 --- a/src/sqlbuild/adapters/bigquery/classes/bigquery_adapter.py +++ b/src/sqlbuild/adapters/bigquery/classes/bigquery_adapter.py @@ -127,6 +127,7 @@ ) from sqlbuild.compiler.compile.types import FunctionLanguage from sqlbuild.compiler.source_freshness.models import SourceFreshnessRecord +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from sqlbuild.diagnostics.main.log_sql import log_sql from sqlbuild.runtime.observability.classes.statement_lifecycle import StatementLifecycle from sqlbuild.spec.contracts.constants import DEFAULT_SEED_CSV_SETTINGS @@ -150,6 +151,11 @@ def get_columns_for_relations( adapter_name: ClassVar[str] = BuiltinAdapter.BIGQUERY.value sql_analysis_dialect_name: ClassVar[str | None] = "bigquery" + sql_lexical_syntax: ClassVar[SqlLexicalSyntax] = SqlLexicalSyntax( + backslash_escape_quotes=frozenset({"'", '"', "`"}), + triple_quoted_strings=True, + line_comment_prefixes=frozenset({"--", "#"}), + ) views_read_with_reader_access: ClassVar[bool] = True max_identifier_length: ClassVar[int] = 1024 _snapshot_sql_dialect: ClassVar[SnapshotSqlDialect] = SnapshotSqlDialect( diff --git a/src/sqlbuild/adapters/databricks/classes/databricks_adapter.py b/src/sqlbuild/adapters/databricks/classes/databricks_adapter.py index d3ef4c12eb..55325d3429 100644 --- a/src/sqlbuild/adapters/databricks/classes/databricks_adapter.py +++ b/src/sqlbuild/adapters/databricks/classes/databricks_adapter.py @@ -115,6 +115,7 @@ ) from sqlbuild.compiler.compile.types import FunctionLanguage from sqlbuild.compiler.source_freshness.models import SourceFreshnessRecord +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from sqlbuild.diagnostics.main.log_sql import log_sql from sqlbuild.spec.contracts.constants import DEFAULT_SEED_CSV_SETTINGS from sqlbuild.spec.contracts.models import SeedCsvSettings @@ -136,6 +137,11 @@ def get_columns_for_relations( adapter_name: ClassVar[str] = BuiltinAdapter.DATABRICKS.value sql_analysis_dialect_name: ClassVar[str | None] = "databricks" + sql_lexical_syntax: ClassVar[SqlLexicalSyntax] = SqlLexicalSyntax( + backslash_escape_quotes=frozenset({"'", '"'}), + raw_string_prefix=True, + nested_block_comments=True, + ) max_identifier_length: ClassVar[int] = 255 _snapshot_sql_dialect: ClassVar[SnapshotSqlDialect] = SnapshotSqlDialect( timestamp_type="TIMESTAMP", diff --git a/src/sqlbuild/adapters/postgres/classes/postgres_adapter.py b/src/sqlbuild/adapters/postgres/classes/postgres_adapter.py index bcc08cc0a0..5bd8eeb213 100644 --- a/src/sqlbuild/adapters/postgres/classes/postgres_adapter.py +++ b/src/sqlbuild/adapters/postgres/classes/postgres_adapter.py @@ -98,6 +98,7 @@ from sqlbuild.adapters.postgres.constants import TABLE_FUNCTION_RETURN_TYPE from sqlbuild.compiler.compile.types import FunctionLanguage from sqlbuild.compiler.source_freshness.models import SourceFreshnessRecord +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from sqlbuild.diagnostics.main.log_sql import log_sql from sqlbuild.spec.contracts.constants import DEFAULT_SEED_CSV_SETTINGS from sqlbuild.spec.contracts.models import SeedCsvSettings @@ -119,6 +120,9 @@ def get_columns_for_relations( adapter_name: ClassVar[str] = BuiltinAdapter.POSTGRES.value sql_analysis_dialect_name: ClassVar[str | None] = "postgres" + sql_lexical_syntax: ClassVar[SqlLexicalSyntax] = SqlLexicalSyntax( + escape_string_prefix=True, nested_block_comments=True + ) max_identifier_length: ClassVar[int] = 63 _snapshot_sql_dialect: ClassVar[SnapshotSqlDialect] = SnapshotSqlDialect( timestamp_type="TIMESTAMP", diff --git a/src/sqlbuild/adapters/snowflake/classes/snowflake_adapter.py b/src/sqlbuild/adapters/snowflake/classes/snowflake_adapter.py index 7555d23333..c7a601b7ab 100644 --- a/src/sqlbuild/adapters/snowflake/classes/snowflake_adapter.py +++ b/src/sqlbuild/adapters/snowflake/classes/snowflake_adapter.py @@ -138,6 +138,7 @@ ) from sqlbuild.compiler.compile.types import FunctionLanguage from sqlbuild.compiler.source_freshness.models import SourceFreshnessRecord +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from sqlbuild.cost.main.collection import collect_snowflake_cost from sqlbuild.cost.models import RunCostSummary from sqlbuild.cost.types import CostCapability, CostStatus @@ -164,6 +165,9 @@ class SnowflakeAdapter(MicrobatchMixin, UnkeyedDiffMixin, BaseAdapter): adapter_name: ClassVar[str] = BuiltinAdapter.SNOWFLAKE.value sql_analysis_dialect_name: ClassVar[str | None] = "snowflake" + sql_lexical_syntax: ClassVar[SqlLexicalSyntax] = SqlLexicalSyntax( + backslash_escape_quotes=frozenset({"'"}), line_comment_prefixes=frozenset({"--", "//"}) + ) execution_duration_limit_seconds: ClassVar[int | None] = MAX_STATEMENT_TIMEOUT_SECONDS metadata_inspection_concurrency: ClassVar[int] = _METADATA_INSPECTION_CONCURRENCY exact_column_inspection_limit: ClassVar[int] = _EXACT_COLUMN_INSPECTION_LIMIT diff --git a/src/sqlbuild/adapters/sqlserver/classes/sqlserver_adapter.py b/src/sqlbuild/adapters/sqlserver/classes/sqlserver_adapter.py index dbdef8b84a..a7271349bb 100644 --- a/src/sqlbuild/adapters/sqlserver/classes/sqlserver_adapter.py +++ b/src/sqlbuild/adapters/sqlserver/classes/sqlserver_adapter.py @@ -105,6 +105,7 @@ ) from sqlbuild.compiler.compile.types import FunctionLanguage from sqlbuild.compiler.source_freshness.models import SourceFreshnessRecord +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from sqlbuild.diagnostics.main.log_sql import log_sql from sqlbuild.spec.contracts.constants import DEFAULT_SEED_CSV_SETTINGS from sqlbuild.spec.contracts.models import SeedCsvSettings @@ -135,6 +136,7 @@ def get_columns_for_relations( adapter_name: ClassVar[str] = BuiltinAdapter.SQLSERVER.value sql_analysis_dialect_name: ClassVar[str | None] = "tsql" + sql_lexical_syntax: ClassVar[SqlLexicalSyntax] = SqlLexicalSyntax(nested_block_comments=True) max_identifier_length: ClassVar[int] = 128 _snapshot_sql_dialect: ClassVar[SnapshotSqlDialect] = SnapshotSqlDialect( timestamp_type="DATETIME2", diff --git a/src/sqlbuild/cli/commands/_helpers/scenario_execution/local_run.py b/src/sqlbuild/cli/commands/_helpers/scenario_execution/local_run.py index 9ee8a048cc..60489e5f29 100644 --- a/src/sqlbuild/cli/commands/_helpers/scenario_execution/local_run.py +++ b/src/sqlbuild/cli/commands/_helpers/scenario_execution/local_run.py @@ -19,7 +19,7 @@ from sqlbuild.compiler.compile.models import CompiledSqlScenario from sqlbuild.compiler.pipeline.models import CompilePipelineResult from sqlbuild.executor.pipeline.main.run import run_scenario_local_test_pipeline -from sqlbuild.executor.scenario.models import ScenarioRunResult +from sqlbuild.executor.scenario.models import ScenarioLocalReplaySource, ScenarioRunResult from sqlbuild.executor.scenario.types import ScenarioLocalRunStatus from sqlbuild.presentation.classes.cli_style import CliStyle from sqlbuild.presentation.classes.transient_status_reporter import TransientStatusReporter @@ -34,8 +34,7 @@ def run_local_scenarios( adapter: BaseAdapter, project_name: str, strict: bool, - capture_adapter: str, - capture_dialect: str, + replay_source: ScenarioLocalReplaySource, target_dir: Path, output_context: ScenarioRunOutputContext, ) -> int: @@ -66,8 +65,7 @@ def run_local_scenarios( adapter=adapter, project_name=project_name, strict=strict, - capture_adapter=capture_adapter, - capture_dialect=capture_dialect, + replay_source=replay_source, on_scenario_start=lambda _scenario: ( scenario_status.start(activity) if status_is_tty else None ), diff --git a/src/sqlbuild/cli/commands/_helpers/scenario_execution/runner.py b/src/sqlbuild/cli/commands/_helpers/scenario_execution/runner.py index f32081a3f1..e15a98f620 100644 --- a/src/sqlbuild/cli/commands/_helpers/scenario_execution/runner.py +++ b/src/sqlbuild/cli/commands/_helpers/scenario_execution/runner.py @@ -66,6 +66,7 @@ select_scenario_snapshot_capture_candidates, ) from sqlbuild.executor.scenario.models import ( + ScenarioLocalReplaySource, ScenarioSnapshotCaptureLimits, ScenarioSnapshotCaptureRelationResult, ScenarioSnapshotCaptureRunResult, @@ -266,8 +267,11 @@ def run_scenario(request: ScenarioTestCommandRequest) -> int: adapter=adapter, project_name=discovered_inputs.project_config.name, strict=strict, - capture_adapter=project_adapter_name, - capture_dialect=local_capture_dialect, + replay_source=ScenarioLocalReplaySource( + lexical_syntax=project_adapter.sql_lexical_syntax, + capture_adapter=project_adapter_name, + capture_dialect=local_capture_dialect, + ), target_dir=effective_project_dir / "target", output_context=ScenarioRunOutputContext( namespace=namespace, @@ -335,6 +339,7 @@ def _sync_local_snapshots( capture_adapter=project_adapter_name, capture_dialect=capture_dialect, refresh=refresh, + source_lexical_syntax=project_adapter.sql_lexical_syntax, ) if not capture_names: progress_stream.write("\nSnapshots are fresh.\n") diff --git a/src/sqlbuild/compiler/planner/_helpers/resolve/refs.py b/src/sqlbuild/compiler/planner/_helpers/resolve/refs.py index ddd53bdd28..c15c31ce09 100644 --- a/src/sqlbuild/compiler/planner/_helpers/resolve/refs.py +++ b/src/sqlbuild/compiler/planner/_helpers/resolve/refs.py @@ -3,7 +3,7 @@ from __future__ import annotations import re -from collections.abc import Iterator +from collections.abc import Callable, Iterator from sqlbuild.adapter.contract.classes.base_adapter import BaseAdapter from sqlbuild.adapter.relations.main.resolve_relation_location_qualified_name import ( @@ -32,8 +32,10 @@ from sqlbuild.compiler.references.types import ExternalSqlReferenceResolver, SqlReferenceKind from sqlbuild.compiler.sql_analysis.main._find_matching_paren import find_matching_paren from sqlbuild.compiler.sql_analysis.main._skip_block_comment import skip_block_comment +from sqlbuild.compiler.sql_analysis.main._skip_dialect_non_code import dialect_non_code_end from sqlbuild.compiler.sql_analysis.main._skip_line_comment import skip_line_comment from sqlbuild.compiler.sql_analysis.main._skip_quoted_text import skip_quoted_text +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from sqlbuild.cursor_algebra.main.sentinel_to_token import sentinel_to_token _REF_PATTERN: re.Pattern[str] = quoted_reference_call_pattern(SqlReferenceKind.REF) @@ -255,6 +257,27 @@ def resolve_table_function_references( return "".join(parts) +def replace_executable_matches( + *, + sql: str, + pattern: re.Pattern[str], + replace: Callable[[re.Match[str]], str], + lexical_syntax: SqlLexicalSyntax, +) -> str: + """Replace pattern matches in executable SQL, leaving quoted text and comments untouched.""" + + parts: list[str] = [] + last_index: int = 0 + for match in _iter_dialect_executable_matches( + sql=sql, pattern=pattern, lexical_syntax=lexical_syntax + ): + parts.append(sql[last_index : match.start()]) + parts.append(replace(match)) + last_index = match.end() + parts.append(sql[last_index:]) + return "".join(parts) + + def _iter_executable_matches(*, sql: str, pattern: re.Pattern[str]) -> Iterator[re.Match[str]]: index: int = 0 while index < len(sql): @@ -275,6 +298,25 @@ def _iter_executable_matches(*, sql: str, pattern: re.Pattern[str]) -> Iterator[ index = match.end() +def _iter_dialect_executable_matches( + *, sql: str, pattern: re.Pattern[str], lexical_syntax: SqlLexicalSyntax +) -> Iterator[re.Match[str]]: + index: int = 0 + while index < len(sql): + non_code_end: int | None = dialect_non_code_end( + sql=sql, start=index, syntax=lexical_syntax, context="SQL reference resolution" + ) + if non_code_end is not None: + index = non_code_end + continue + match: re.Match[str] | None = pattern.match(sql, index) + if match is None: + index += 1 + continue + yield match + index = match.end() + + def _build_cursor_subquery( *, qualified_name: str, diff --git a/src/sqlbuild/compiler/planner/_helpers/scenario/cli.py b/src/sqlbuild/compiler/planner/_helpers/scenario/cli.py index cb65ad336c..3d8c00965e 100644 --- a/src/sqlbuild/compiler/planner/_helpers/scenario/cli.py +++ b/src/sqlbuild/compiler/planner/_helpers/scenario/cli.py @@ -29,6 +29,7 @@ ScenarioRelationPlan, ) from sqlbuild.compiler.planner.types import ScenarioArtifactKind, WarningSeverity +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax def build_cli_scenario_plan( @@ -37,6 +38,7 @@ def build_cli_scenario_plan( pipeline_result: CompilePipelineResult, adapter: BaseAdapter, project_name: str, + source_lexical_syntax: SqlLexicalSyntax, ) -> ScenarioExecutionPlan: """Build a scenario execution plan for CLI execution.""" @@ -77,6 +79,7 @@ def build_cli_scenario_plan( adapter=adapter, graph_plan=graph_plan, relation_plan=relation_plan, + source_lexical_syntax=source_lexical_syntax, ) _raise_for_error_warnings(execution_warnings) return scenario_plan diff --git a/src/sqlbuild/compiler/planner/_helpers/scenario/relations.py b/src/sqlbuild/compiler/planner/_helpers/scenario/relations.py index 4a80df2b60..2961e816cd 100644 --- a/src/sqlbuild/compiler/planner/_helpers/scenario/relations.py +++ b/src/sqlbuild/compiler/planner/_helpers/scenario/relations.py @@ -2,12 +2,9 @@ from __future__ import annotations -import logging import re from collections.abc import Callable -from copy import deepcopy from dataclasses import replace -from typing import Any from sqlbuild.adapter.contract.classes.base_adapter import BaseAdapter from sqlbuild.adapter.contract.models import ColumnInfo @@ -17,6 +14,7 @@ SEED_TEST_CTE_PREFIX, SOURCE_TEST_CTE_PREFIX, ) +from sqlbuild.compiler.compile.exceptions import CompileInputError from sqlbuild.compiler.compile.models import ( CompiledFunction, CompiledModel, @@ -39,16 +37,18 @@ build_compiled_function_fingerprint_sql, ) from sqlbuild.compiler.planner._helpers.output.plan_entry import extract_seed_columns, plan_model -from sqlbuild.compiler.planner._helpers.resolve.refs import build_function_locations +from sqlbuild.compiler.planner._helpers.resolve.refs import ( + build_function_locations, + replace_executable_matches, +) from sqlbuild.compiler.planner._helpers.resolve.resolve import resolve_function_sql from sqlbuild.compiler.planner.constants import ( - POLYGLOT_ALIAS_VALUE_KEY, SCENARIO_PLAN_INTERNAL, SCENARIO_PLAN_INVALID_FIXTURE, SCENARIO_PLAN_MISSING_FIXTURE_SQL, SCENARIO_PLAN_MISSING_RELATION_TARGET, - SCENARIO_PLAN_SQLGLOT_PARSE, SCENARIO_PLAN_UNKNOWN_SEED, + SCENARIO_PLAN_UNRESOLVED_RELATION_MARKER, ) from sqlbuild.compiler.planner.exceptions import PlannerInputError from sqlbuild.compiler.planner.models import ( @@ -71,7 +71,6 @@ ) from sqlbuild.compiler.planner.types import ( FixtureKey, - RelationMarkerTargetResolver, ScenarioArtifactKind, ) from sqlbuild.compiler.references.main.reference_call_prefix_pattern_text import ( @@ -79,11 +78,9 @@ ) from sqlbuild.compiler.references.main.render_source_relation import render_source_relation from sqlbuild.compiler.references.types import SqlReferenceKind -from sqlbuild.compiler.sql_analysis.main.import_polyglot_sql import import_polyglot_sql -from sqlbuild.diagnostics.main.log_debug_event import log_debug_event +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from sqlbuild.spec.contracts.models import SourceEntry -_DEBUG_LOGGER: logging.Logger = logging.getLogger("sqlbuild.planner") _REF_PATTERN: re.Pattern[str] = re.compile( rf"{reference_call_prefix_pattern_text(SqlReferenceKind.REF)}\s*" r"[\"']?(?P[A-Za-z_][A-Za-z0-9_.]*)[\"']?\s*\)" @@ -223,53 +220,97 @@ def resolve_scenario_check_sql( sql: str, relation_plan: ScenarioRelationPlan, adapter: BaseAdapter, - sql_analysis_enabled: bool = True, - sql_analysis_dialect: str | None = None, + source_label: str, + source_lexical_syntax: SqlLexicalSyntax, ) -> str: - """Resolve refs, seeds, and sources in scenario expected/assertion SQL.""" + """Resolve refs, seeds, and sources in authored scenario expected/assertion SQL.""" - if sql_analysis_enabled: - return _resolve_scenario_check_sql_with_sql_analysis( - sql=sql, - relation_plan=relation_plan, - adapter=adapter, - sql_analysis_dialect=sql_analysis_dialect, - ) + def _ref_target(match: re.Match[str]) -> str | None: + return _location_name(relation_plan.model_locations.get(match.group("name"))) - def _replace_ref(match: re.Match[str]) -> str: - target: CompiledRelationLocation | None = relation_plan.model_locations.get( - match.group("name") - ) - if target is None or target.qualified_name is None: - return match.group(0) - return target.qualified_name + def _seed_target(match: re.Match[str]) -> str | None: + return _location_name(relation_plan.seed_locations.get(match.group("name"))) - def _replace_seed(match: re.Match[str]) -> str: - target: CompiledRelationLocation | None = relation_plan.seed_locations.get( - match.group("name") + def _dbt_ref_target(match: re.Match[str]) -> str | None: + return _location_name( + relation_plan.dbt_ref_fixture_locations.get(_dbt_ref_fixture_name(match)) ) - if target is None or target.qualified_name is None: - return match.group(0) - return target.qualified_name - def _replace_source(match: re.Match[str]) -> str: - source: SourceEntry | None = relation_plan.source_map.get(match.group("name")) - if source is None: - return match.group(0) - return render_source_relation(entry=source, adapter=adapter) + return _replace_relation_markers( + sql=sql, + resolvers=( + (_REF_PATTERN, _ref_target), + (_SEED_PATTERN, _seed_target), + ( + _SOURCE_PATTERN, + _source_resolver(source_map=relation_plan.source_map, adapter=adapter), + ), + (_DBT_REF_PATTERN, _dbt_ref_target), + ), + source_label=source_label, + source_lexical_syntax=source_lexical_syntax, + ) - def _replace_dbt_ref(match: re.Match[str]) -> str: - target: CompiledRelationLocation | None = relation_plan.dbt_ref_fixture_locations.get( - _dbt_ref_fixture_name(match) - ) - if target is None or target.qualified_name is None: - return match.group(0) - return target.qualified_name - result: str = _REF_PATTERN.sub(_replace_ref, sql) - result = _SEED_PATTERN.sub(_replace_seed, result) - result = _SOURCE_PATTERN.sub(_replace_source, result) - return _DBT_REF_PATTERN.sub(_replace_dbt_ref, result) +def _source_resolver( + *, source_map: dict[str, SourceEntry], adapter: BaseAdapter +) -> Callable[[re.Match[str]], str | None]: + def _source_target(match: re.Match[str]) -> str | None: + source: SourceEntry | None = source_map.get(match.group("name")) + return None if source is None else render_source_relation(entry=source, adapter=adapter) + + return _source_target + + +def _location_name(location: CompiledRelationLocation | None) -> str | None: + return None if location is None else location.qualified_name + + +def _marker_replacer( + *, resolve_target: Callable[[re.Match[str]], str | None], source_label: str +) -> Callable[[re.Match[str]], str]: + def _replace(match: re.Match[str]) -> str: + target: str | None = resolve_target(match) + if target is None: + raise PlannerInputError( + f"Scenario SQL in {source_label} references {match.group(0)!r}, which is " + "not a relation in this scenario", + code=SCENARIO_PLAN_UNRESOLVED_RELATION_MARKER, + help="Reference a model, seed, source, or dbt ref the scenario builds or " + "provides as a fixture.", + ) + return target + + return _replace + + +def _replace_relation_markers( + *, + sql: str, + resolvers: tuple[tuple[re.Pattern[str], Callable[[re.Match[str]], str | None]], ...], + source_label: str, + source_lexical_syntax: SqlLexicalSyntax, +) -> str: + result: str = sql + pattern: re.Pattern[str] + resolve_target: Callable[[re.Match[str]], str | None] + for pattern, resolve_target in resolvers: + try: + result = replace_executable_matches( + sql=result, + pattern=pattern, + replace=_marker_replacer(resolve_target=resolve_target, source_label=source_label), + lexical_syntax=source_lexical_syntax, + ) + except CompileInputError as error: + raise PlannerInputError( + f"Scenario SQL in {source_label} could not be scanned for relation markers: " + f"{error}", + code=SCENARIO_PLAN_UNRESOLVED_RELATION_MARKER, + help="Close every quoted string and block comment using the project dialect's " + "quoting and comment rules.", + ) from None + return result def build_scenario_execution_plan( @@ -279,10 +320,10 @@ def build_scenario_execution_plan( adapter: BaseAdapter, graph_plan: ScenarioGraphPlan, relation_plan: ScenarioRelationPlan, + source_lexical_syntax: SqlLexicalSyntax, snapshot: WarehouseSnapshot | None = None, source_warehouse_columns: dict[str, tuple[ColumnInfo, ...]] | None = None, sql_analysis_enabled: bool = True, - sql_analysis_dialect: str | None = None, ) -> tuple[ScenarioExecutionPlan, tuple[PlanWarning, ...]]: """Build a dry-run execution plan for one SQL scenario.""" @@ -329,7 +370,7 @@ def build_scenario_execution_plan( fixture_groups=scenario_fixture_groups, ) if fixture_completion.diagnostics: - source_path: str = str(scenario.source_path or scenario.name) + source_path: str = _scenario_source_label(scenario) details: str = "\n".join( f"- {diagnostic.message}" for diagnostic in fixture_completion.diagnostics ) @@ -344,8 +385,7 @@ def build_scenario_execution_plan( graph_plan=graph_plan, relation_plan=relation_plan, adapter=adapter, - sql_analysis_enabled=sql_analysis_enabled, - sql_analysis_dialect=sql_analysis_dialect, + source_lexical_syntax=source_lexical_syntax, fixture_sql_overrides=fixture_sql_overrides, ) seed_entries: tuple[SeedPlanEntry, ...] = build_scenario_seed_entries( @@ -408,8 +448,8 @@ def build_scenario_execution_plan( expected_cte=expected_cte, relation_plan=relation_plan, adapter=adapter, - sql_analysis_enabled=sql_analysis_enabled, - sql_analysis_dialect=sql_analysis_dialect, + source_label=_scenario_source_label(scenario), + source_lexical_syntax=source_lexical_syntax, ) for expected_cte in scenario.expected_ctes ) @@ -420,8 +460,8 @@ def build_scenario_execution_plan( sql=assertion_cte.sql_body, relation_plan=relation_plan, adapter=adapter, - sql_analysis_enabled=sql_analysis_enabled, - sql_analysis_dialect=sql_analysis_dialect, + source_label=_scenario_source_label(scenario), + source_lexical_syntax=source_lexical_syntax, ), ) for assertion_cte in scenario.assertion_ctes @@ -487,12 +527,12 @@ def build_scenario_fixture_plans( graph_plan: ScenarioGraphPlan, relation_plan: ScenarioRelationPlan, adapter: BaseAdapter, - sql_analysis_enabled: bool = True, - sql_analysis_dialect: str | None = None, + source_lexical_syntax: SqlLexicalSyntax, fixture_sql_overrides: dict[FixtureKey, str] | None = None, ) -> tuple[ScenarioFixturePlan, ...]: """Build self-contained fixture SQL plans, including shared helper CTEs.""" + source_label: str = _scenario_source_label(scenario) helper_ctes: tuple[CompileSqlScenarioCte, ...] = _extract_helper_ctes(scenario) resolved_helper_ctes: tuple[CompileSqlScenarioCte, ...] = tuple( replace( @@ -501,8 +541,8 @@ def build_scenario_fixture_plans( sql=helper_cte.sql_body, source_map=relation_plan.project_source_map, adapter=adapter, - sql_analysis_enabled=sql_analysis_enabled, - sql_analysis_dialect=sql_analysis_dialect, + source_label=source_label, + source_lexical_syntax=source_lexical_syntax, ), ) for helper_cte in helper_ctes @@ -559,8 +599,8 @@ def build_scenario_fixture_plans( ), source_map=relation_plan.project_source_map, adapter=adapter, - sql_analysis_enabled=sql_analysis_enabled, - sql_analysis_dialect=sql_analysis_dialect, + source_label=source_label, + source_lexical_syntax=source_lexical_syntax, ), helper_ctes=resolved_helper_ctes, ), @@ -585,8 +625,8 @@ def build_scenario_fixture_plans( ), source_map=relation_plan.project_source_map, adapter=adapter, - sql_analysis_enabled=sql_analysis_enabled, - sql_analysis_dialect=sql_analysis_dialect, + source_label=source_label, + source_lexical_syntax=source_lexical_syntax, ), helper_ctes=resolved_helper_ctes, ), @@ -611,8 +651,8 @@ def build_scenario_fixture_plans( ), source_map=relation_plan.project_source_map, adapter=adapter, - sql_analysis_enabled=sql_analysis_enabled, - sql_analysis_dialect=sql_analysis_dialect, + source_label=source_label, + source_lexical_syntax=source_lexical_syntax, ), helper_ctes=resolved_helper_ctes, ), @@ -639,8 +679,8 @@ def build_scenario_fixture_plans( ), source_map=relation_plan.project_source_map, adapter=adapter, - sql_analysis_enabled=sql_analysis_enabled, - sql_analysis_dialect=sql_analysis_dialect, + source_label=source_label, + source_lexical_syntax=source_lexical_syntax, ), helper_ctes=resolved_helper_ctes, ), @@ -697,52 +737,13 @@ def build_scenario_seed_entries( return tuple(seed_entries) -def _resolve_scenario_check_sql_with_sql_analysis( - *, - sql: str, - relation_plan: ScenarioRelationPlan, - adapter: BaseAdapter, - sql_analysis_dialect: str | None, -) -> str: - polyglot_module: Any = import_polyglot_sql() - try: - parsed: Any = polyglot_module.parse_one(sql, dialect=sql_analysis_dialect or "generic") - except polyglot_module.PolyglotError as error: - raise PlannerInputError( - f"Scenario SQL could not be parsed with Polyglot: {error}", - code=SCENARIO_PLAN_SQLGLOT_PARSE, - ) from None - - parsed_dict: dict[str, Any] = parsed.to_dict() - replacement_result: bool = _replace_relation_markers_in_polyglot_dict( - node=parsed_dict, - polyglot_module=polyglot_module, - sql_analysis_dialect=sql_analysis_dialect, - target_for_marker=lambda function_name, referenced_name: _scenario_target_name_for_marker( - function_name=function_name, - referenced_name=referenced_name, - relation_plan=relation_plan, - adapter=adapter, - ), - ) - if not replacement_result: - return sql - generated: list[str] = polyglot_module.generate( - parsed_dict, - dialect=sql_analysis_dialect or "generic", - ) - if len(generated) != 1: - return sql - return generated[0] - - def _build_expected_check_plan( *, expected_cte: CompileSqlScenarioCte, relation_plan: ScenarioRelationPlan, adapter: BaseAdapter, - sql_analysis_enabled: bool, - sql_analysis_dialect: str | None, + source_label: str, + source_lexical_syntax: SqlLexicalSyntax, ) -> ScenarioExpectedExpectationPlan: model_name: str = expected_cte.name.removeprefix("__expected__") actual_destination: CompiledRelationLocation = _required_target( @@ -757,8 +758,8 @@ def _build_expected_check_plan( sql=expected_cte.sql_body, relation_plan=relation_plan, adapter=adapter, - sql_analysis_enabled=sql_analysis_enabled, - sql_analysis_dialect=sql_analysis_dialect, + source_label=source_label, + source_lexical_syntax=source_lexical_syntax, ), ) @@ -803,254 +804,19 @@ def _resolve_project_source_refs( sql: str, source_map: dict[str, SourceEntry], adapter: BaseAdapter, - sql_analysis_enabled: bool, - sql_analysis_dialect: str | None, + source_label: str, + source_lexical_syntax: SqlLexicalSyntax, ) -> str: - if sql_analysis_enabled: - sql_analysis_result: str | None = _try_resolve_project_source_refs_with_sql_analysis( - sql=sql, - source_map=source_map, - adapter=adapter, - sql_analysis_dialect=sql_analysis_dialect, - ) - if sql_analysis_result is not None: - return sql_analysis_result - - def _replace_source(match: re.Match[str]) -> str: - source: SourceEntry | None = source_map.get(match.group("name")) - if source is None: - return match.group(0) - return render_source_relation(entry=source, adapter=adapter) - - return _SOURCE_PATTERN.sub(_replace_source, sql) - - -def _try_resolve_project_source_refs_with_sql_analysis( - *, - sql: str, - source_map: dict[str, SourceEntry], - adapter: BaseAdapter, - sql_analysis_dialect: str | None, -) -> str | None: - if SqlReferenceKind.SOURCE.function_name not in sql.lower(): - return None - polyglot_module: Any = import_polyglot_sql() - try: - parsed: Any = polyglot_module.parse_one(sql, dialect=sql_analysis_dialect or "generic") - except polyglot_module.PolyglotError as error: - log_debug_event( - logger=_DEBUG_LOGGER, - message="scenario source ref resolution parse failed; falling back", - sqlbuild_error=str(error), - ) - return None - parsed_dict: dict[str, Any] = parsed.to_dict() - - class _ExpressionSourceState: - def __init__(self, sources: dict[str, SourceEntry], relation_adapter: BaseAdapter) -> None: - self.names: set[str] = set() - self.sources = sources - self.relation_adapter = relation_adapter - - def __call__(self, *, function_name: str, referenced_name: str) -> str | None: - if function_name != SqlReferenceKind.SOURCE.function_name: - return None - source: SourceEntry | None = self.sources.get(referenced_name) - if source is None: - return None - if source.expression is not None: - self.names.add(referenced_name) - return None - return render_source_relation(entry=source, adapter=self.relation_adapter) - - expression_sources: _ExpressionSourceState = _ExpressionSourceState(source_map, adapter) - - replacement_result: bool = _replace_relation_markers_in_polyglot_dict( - node=parsed_dict, - polyglot_module=polyglot_module, - sql_analysis_dialect=sql_analysis_dialect, - target_for_marker=expression_sources, - ) - if expression_sources.names or not replacement_result: - return None - generated: list[str] = polyglot_module.generate( - parsed_dict, - dialect=sql_analysis_dialect or "generic", + return _replace_relation_markers( + sql=sql, + resolvers=((_SOURCE_PATTERN, _source_resolver(source_map=source_map, adapter=adapter)),), + source_label=source_label, + source_lexical_syntax=source_lexical_syntax, ) - if len(generated) != 1: - return None - return generated[0] -def _replace_relation_markers_in_polyglot_dict( - *, - node: Any, - polyglot_module: Any, - sql_analysis_dialect: str | None, - target_for_marker: RelationMarkerTargetResolver, -) -> bool: - """Rewrite reference-marker relations in a parsed polyglot dict in place.""" - - relation_cache: dict[str, dict[str, Any] | None] = {} - - def _cached_relation( - *, - target_name: str, - cache: dict[str, dict[str, Any] | None], - ) -> tuple[dict[str, Any] | None, dict[str, dict[str, Any] | None]]: - if target_name not in cache: - cache = { - **cache, - target_name: _polyglot_relation_dict( - target_name=target_name, - polyglot_module=polyglot_module, - sql_analysis_dialect=sql_analysis_dialect, - ), - } - return cache[target_name], cache - - def _replacement( - *, - expression: Any, - cache: dict[str, dict[str, Any] | None], - ) -> tuple[dict[str, Any] | None, dict[str, dict[str, Any] | None]]: - if not isinstance(expression, dict): - return None, cache - alias_payload: Any | None = expression.get("alias") - if isinstance(alias_payload, dict) and POLYGLOT_ALIAS_VALUE_KEY in alias_payload: - inner_replacement, cache = _replacement( - expression=alias_payload.get(POLYGLOT_ALIAS_VALUE_KEY), - cache=cache, - ) - if inner_replacement is None: - return None, cache - alias_payload[POLYGLOT_ALIAS_VALUE_KEY] = inner_replacement - return expression, cache - - function_payload: Any | None = expression.get("function") - if not isinstance(function_payload, dict): - return None, cache - function_name: str = str(function_payload.get("name", "")).lower() - referenced_name: str | None = _polyglot_marker_reference_name( - function_name=function_name, - function_payload=function_payload, - ) - if referenced_name is None: - return None, cache - target_name: str | None = target_for_marker( - function_name=function_name, - referenced_name=referenced_name, - ) - if target_name is None: - return None, cache - relation, cache = _cached_relation(target_name=target_name, cache=cache) - return (None if relation is None else deepcopy(relation)), cache - - def _walk( - *, - walk_node: Any, - cache: dict[str, dict[str, Any] | None], - ) -> tuple[bool, dict[str, dict[str, Any] | None]]: - changed: bool = False - if isinstance(walk_node, dict): - from_clause: Any | None = walk_node.get("from") - if isinstance(from_clause, dict): - expressions: Any = from_clause.get("expressions") - if isinstance(expressions, list): - for index, expression in enumerate(expressions): - replacement, cache = _replacement(expression=expression, cache=cache) - if replacement is not None: - expressions[index] = replacement - changed = True - joins: Any | None = walk_node.get("joins") - if isinstance(joins, list): - join: Any - for join in joins: - if not isinstance(join, dict): - continue - replacement, cache = _replacement(expression=join.get("this"), cache=cache) - if replacement is not None: - join["this"] = replacement - changed = True - value: Any - for value in walk_node.values(): - if isinstance(value, dict | list): - child_changed, cache = _walk(walk_node=value, cache=cache) - changed = child_changed or changed - elif isinstance(walk_node, list): - item: Any - for item in walk_node: - if isinstance(item, dict | list): - child_changed, cache = _walk(walk_node=item, cache=cache) - changed = child_changed or changed - return changed, cache - - changed, relation_cache = _walk(walk_node=node, cache=relation_cache) - return changed - - -def _polyglot_marker_reference_name( - *, function_name: str, function_payload: dict[str, Any] -) -> str | None: - args: Any = function_payload.get("args") - if not isinstance(args, list): - return None - if function_name == SqlReferenceKind.DBT_REF.function_name: - if len(args) == 1: - return _polyglot_column_arg_name(args[0]) - two_argument_count: int = 2 - if len(args) == two_argument_count: - first: str | None = _polyglot_column_arg_name(args[0]) - second: str | None = _polyglot_column_arg_name(args[1]) - if first is None or second is None: - return None - return f"{first}__{second}" - return None - if len(args) != 1: - return None - return _polyglot_column_arg_name(args[0]) - - -def _polyglot_column_arg_name(argument: Any) -> str | None: - if not isinstance(argument, dict): - return None - column_payload: Any | None = argument.get("column") - if not isinstance(column_payload, dict): - return None - name_payload: Any | None = column_payload.get("name") - if isinstance(name_payload, dict): - raw_name: Any | None = name_payload.get("name") - return str(raw_name) if raw_name is not None else None - return str(name_payload) if name_payload is not None else None - - -def _polyglot_relation_dict( - *, target_name: str, polyglot_module: Any, sql_analysis_dialect: str | None -) -> dict[str, Any] | None: - try: - parsed: Any = polyglot_module.parse_one( - f"SELECT * FROM {target_name}", - dialect=sql_analysis_dialect or "generic", - ) - except polyglot_module.PolyglotError as error: - log_debug_event( - logger=_DEBUG_LOGGER, - message="scenario relation dict parse failed; falling back", - sqlbuild_error=str(error), - ) - return None - parsed_dict: dict[str, Any] = parsed.to_dict() - select_payload: Any | None = parsed_dict.get("select") - if not isinstance(select_payload, dict): - return None - from_payload: Any | None = select_payload.get("from") - if not isinstance(from_payload, dict): - return None - expressions: Any | None = from_payload.get("expressions") - if not isinstance(expressions, list) or len(expressions) != 1: - return None - relation: Any = expressions[0] - return relation if isinstance(relation, dict) else None +def _scenario_source_label(scenario: CompiledSqlScenario) -> str: + return str(scenario.source_path or scenario.name) def _required_fixture_sql(*, fixture_sql: dict[str, str], logical_name: str, kind: str) -> str: @@ -1079,28 +845,6 @@ def _required_target( return target -def _scenario_target_name_for_marker( - *, - function_name: str, - referenced_name: str, - relation_plan: ScenarioRelationPlan, - adapter: BaseAdapter, -) -> str | None: - if function_name == SqlReferenceKind.REF.function_name: - target: CompiledRelationLocation | None = relation_plan.model_locations.get(referenced_name) - return None if target is None else target.qualified_name - if function_name == SqlReferenceKind.SEED.function_name: - target = relation_plan.seed_locations.get(referenced_name) - return None if target is None else target.qualified_name - if function_name == SqlReferenceKind.SOURCE.function_name: - source: SourceEntry | None = relation_plan.source_map.get(referenced_name) - return None if source is None else render_source_relation(entry=source, adapter=adapter) - if function_name == SqlReferenceKind.DBT_REF.function_name: - target = relation_plan.dbt_ref_fixture_locations.get(referenced_name) - return None if target is None else target.qualified_name - return None - - def _resolve_model_dbt_ref_fixtures(*, query_sql: str, relation_plan: ScenarioRelationPlan) -> str: def _replace_dbt_ref(match: re.Match[str]) -> str: target: CompiledRelationLocation | None = relation_plan.dbt_ref_fixture_locations.get( diff --git a/src/sqlbuild/compiler/planner/constants.py b/src/sqlbuild/compiler/planner/constants.py index 4694c625d3..3434990806 100644 --- a/src/sqlbuild/compiler/planner/constants.py +++ b/src/sqlbuild/compiler/planner/constants.py @@ -27,7 +27,6 @@ SOURCE_ALIAS_BOUNDARY_CHARACTERS: frozenset[str] = frozenset(",);") SQL_BRACKETED_IDENTIFIER_START: str = "[" SQL_QUOTED_IDENTIFIER_DELIMITERS: frozenset[str] = frozenset({'"', "`"}) -POLYGLOT_ALIAS_VALUE_KEY: str = "this" SOURCE_DEFERRAL_CONTEXT_FIELDS: frozenset[str] = frozenset({"schema", "database"}) UNIFIED_DIFF_ADDITION_PREFIX: str = "+" UNIFIED_DIFF_ADDITION_HEADER_PREFIX: str = "+++" @@ -50,11 +49,11 @@ SCENARIO_PLAN_HASH_COLLISION: str = "S502" SCENARIO_PLAN_RELATION_COLLISION: str = "S503" SCENARIO_PLAN_GRAPH_VALIDATION: str = "S504" -SCENARIO_PLAN_SQLGLOT_PARSE: str = "S506" SCENARIO_PLAN_UNKNOWN_SEED: str = "S507" SCENARIO_PLAN_MISSING_FIXTURE_SQL: str = "S508" SCENARIO_PLAN_MISSING_RELATION_TARGET: str = "S509" SCENARIO_PLAN_INVALID_FIXTURE: str = "S510" +SCENARIO_PLAN_UNRESOLVED_RELATION_MARKER: str = "S511" SCENARIO_PLAN_INTERNAL: str = "S599" WHOLE_DAY_CURSOR_GRAINS: frozenset[str] = frozenset({"day", "month", "year"}) diff --git a/src/sqlbuild/compiler/planner/main/scenarios/scenario.py b/src/sqlbuild/compiler/planner/main/scenarios/scenario.py index e21946b632..c56dc7f37d 100644 --- a/src/sqlbuild/compiler/planner/main/scenarios/scenario.py +++ b/src/sqlbuild/compiler/planner/main/scenarios/scenario.py @@ -7,6 +7,7 @@ from sqlbuild.compiler.pipeline.models import CompilePipelineResult from sqlbuild.compiler.planner._helpers.scenario.cli import build_cli_scenario_plan from sqlbuild.compiler.planner.models import ScenarioExecutionPlan +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax def build_scenario_plan( @@ -15,6 +16,7 @@ def build_scenario_plan( pipeline_result: CompilePipelineResult, adapter: BaseAdapter, project_name: str, + source_lexical_syntax: SqlLexicalSyntax, ) -> ScenarioExecutionPlan: """Build a scenario execution plan for an external entrypoint.""" @@ -23,4 +25,5 @@ def build_scenario_plan( pipeline_result=pipeline_result, adapter=adapter, project_name=project_name, + source_lexical_syntax=source_lexical_syntax, ) diff --git a/src/sqlbuild/compiler/planner/types.py b/src/sqlbuild/compiler/planner/types.py index 1e00c3c70a..1ae4ff8cab 100644 --- a/src/sqlbuild/compiler/planner/types.py +++ b/src/sqlbuild/compiler/planner/types.py @@ -30,10 +30,6 @@ class CursorInputRole(StrEnum): WATERMARK = "watermark" -class RelationMarkerTargetResolver(Protocol): - def __call__(self, *, function_name: str, referenced_name: str) -> str | None: ... - - class SelectorKind(StrEnum): NAME = "name" SEED = "seed" diff --git a/src/sqlbuild/compiler/sql_analysis/_helpers/scanning.py b/src/sqlbuild/compiler/sql_analysis/_helpers/scanning.py index 9dae55936e..9352b90638 100644 --- a/src/sqlbuild/compiler/sql_analysis/_helpers/scanning.py +++ b/src/sqlbuild/compiler/sql_analysis/_helpers/scanning.py @@ -7,13 +7,23 @@ from sqlbuild.compiler.compile.exceptions import CompileInputError from sqlbuild.compiler.sql_analysis.constants import ( + SQL_BLOCK_COMMENT_CLOSE, + SQL_BLOCK_COMMENT_OPEN, SQL_CLOSE_PARENTHESIS, SQL_DOLLAR_QUOTE_CHARACTER, SQL_ESCAPABLE_QUOTE_CHARACTERS, + SQL_ESCAPE_CHARACTER, + SQL_ESCAPE_STRING_PREFIX, SQL_IDENTIFIER_PREFIX, SQL_OPEN_PARENTHESIS, + SQL_RAW_STRING_PREFIX, + SQL_STRING_PREFIX_CHARACTERS, + SQL_STRING_PREFIX_MAX_LENGTH, + SQL_STRING_QUOTE_CHARACTER, SQL_TEXT_START_CHARACTERS, + SQL_TRIPLE_QUOTE_LENGTH, ) +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax _PAREN_SCAN_SPECIAL: re.Pattern[str] = re.compile(r"[-/'\"`$()]") _DOLLAR_QUOTE_DELIMITER: re.Pattern[str] = re.compile(r"\$(?:[A-Za-z_][A-Za-z0-9_]*)?\$") @@ -47,6 +57,90 @@ def quoted_text_end_impl(*, sql: str, start: int, context: str = "SQL") -> int | return index + 1 +def dialect_non_code_end_impl( + *, sql: str, start: int, syntax: SqlLexicalSyntax, context: str = "SQL" +) -> int | None: + """Return the end of the quoted text or comment starting at the position, if any.""" + + if sql.startswith(SQL_BLOCK_COMMENT_OPEN, start): + return _dialect_block_comment_end(sql=sql, start=start, syntax=syntax, context=context) + if any(sql.startswith(prefix, start) for prefix in syntax.line_comment_prefixes): + return skip_line_comment_impl(sql=sql, start=start) + if sql[start] in SQL_TEXT_START_CHARACTERS: + return dialect_quoted_text_end_impl(sql=sql, start=start, syntax=syntax, context=context) + return None + + +def _dialect_block_comment_end( + *, sql: str, start: int, syntax: SqlLexicalSyntax, context: str +) -> int: + depth: int = 1 + index: int = start + len(SQL_BLOCK_COMMENT_OPEN) + while depth: + closing_index: int = sql.find(SQL_BLOCK_COMMENT_CLOSE, index) + if closing_index == -1: + raise CompileInputError(f"{context} contains an unclosed block comment") + opening_index: int = ( + sql.find(SQL_BLOCK_COMMENT_OPEN, index, closing_index) + if syntax.nested_block_comments + else -1 + ) + if opening_index == -1: + depth -= 1 + index = closing_index + len(SQL_BLOCK_COMMENT_CLOSE) + else: + depth += 1 + index = opening_index + len(SQL_BLOCK_COMMENT_OPEN) + return index + + +def dialect_quoted_text_end_impl( + *, sql: str, start: int, syntax: SqlLexicalSyntax, context: str = "SQL" +) -> int | None: + """Return the end of quoted text starting at the position under the dialect's escape rules.""" + + quote_character: str = sql[start] + if quote_character == SQL_DOLLAR_QUOTE_CHARACTER: + return _dollar_quoted_text_end(sql=sql, start=start, context=context) + prefix: str = _string_literal_prefix(sql=sql, start=start) + backslash_escapes: bool = quote_character in syntax.backslash_escape_quotes or ( + syntax.escape_string_prefix + and quote_character == SQL_STRING_QUOTE_CHARACTER + and prefix == SQL_ESCAPE_STRING_PREFIX + ) + if syntax.raw_string_prefix and SQL_RAW_STRING_PREFIX in prefix: + backslash_escapes = False + delimiter: str = quote_character + triple_quote: str = quote_character * SQL_TRIPLE_QUOTE_LENGTH + if syntax.triple_quoted_strings and sql.startswith(triple_quote, start): + delimiter = triple_quote + index: int = start + len(delimiter) + while index < len(sql): + if backslash_escapes and sql.startswith(SQL_ESCAPE_CHARACTER, index): + index += 2 + continue + if sql.startswith(delimiter, index): + if delimiter == quote_character and sql.startswith(quote_character, index + 1): + index += 2 + continue + return index + len(delimiter) + index += 1 + raise CompileInputError(f"{context} contains an unclosed quoted string") + + +def _string_literal_prefix(*, sql: str, start: int) -> str: + prefix_start: int = start + while ( + prefix_start > 0 + and start - prefix_start < SQL_STRING_PREFIX_MAX_LENGTH + and sql[prefix_start - 1] in SQL_STRING_PREFIX_CHARACTERS + ): + prefix_start -= 1 + if prefix_start > 0 and _continues_dollar_word(sql[prefix_start - 1]): + return "" + return sql[prefix_start:start].lower() + + def _dollar_quoted_text_end(*, sql: str, start: int, context: str) -> int | None: delimiter: re.Match[str] | None = _DOLLAR_QUOTE_DELIMITER.match(sql, start) if delimiter is None or (start > 0 and _continues_dollar_word(sql[start - 1])): diff --git a/src/sqlbuild/compiler/sql_analysis/constants.py b/src/sqlbuild/compiler/sql_analysis/constants.py index 85b17433fc..bec3ddd0bf 100644 --- a/src/sqlbuild/compiler/sql_analysis/constants.py +++ b/src/sqlbuild/compiler/sql_analysis/constants.py @@ -11,6 +11,13 @@ SQL_CLOSE_BRACKET: str = "]" SQL_STRING_QUOTE_CHARACTER: str = "'" SQL_ESCAPE_CHARACTER: str = "\\" +SQL_ESCAPE_STRING_PREFIX: str = "e" +SQL_RAW_STRING_PREFIX: str = "r" +SQL_STRING_PREFIX_CHARACTERS: frozenset[str] = frozenset("bBeErR") +SQL_STRING_PREFIX_MAX_LENGTH: int = 2 +SQL_TRIPLE_QUOTE_LENGTH: int = 3 +SQL_BLOCK_COMMENT_OPEN: str = "/*" +SQL_BLOCK_COMMENT_CLOSE: str = "*/" SQL_DOLLAR_QUOTE_DELIMITER: str = "$$" SQL_DOLLAR_QUOTE_CHARACTER: str = "$" SQL_IDENTIFIER_PREFIX: str = "_" diff --git a/src/sqlbuild/compiler/sql_analysis/main/_skip_dialect_non_code.py b/src/sqlbuild/compiler/sql_analysis/main/_skip_dialect_non_code.py new file mode 100644 index 0000000000..2fa5b8c4ba --- /dev/null +++ b/src/sqlbuild/compiler/sql_analysis/main/_skip_dialect_non_code.py @@ -0,0 +1,12 @@ +"""Dialect-aware SQL quoted text and comment scanning entrypoint.""" + +from sqlbuild.compiler.sql_analysis._helpers.scanning import dialect_non_code_end_impl +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax + + +def dialect_non_code_end( + *, sql: str, start: int, syntax: SqlLexicalSyntax, context: str = "SQL" +) -> int | None: + """Return the end of the quoted text or comment at the position under the dialect's rules.""" + + return dialect_non_code_end_impl(sql=sql, start=start, syntax=syntax, context=context) diff --git a/src/sqlbuild/compiler/sql_analysis/models.py b/src/sqlbuild/compiler/sql_analysis/models.py index c0f594b38e..2a2a74dafd 100644 --- a/src/sqlbuild/compiler/sql_analysis/models.py +++ b/src/sqlbuild/compiler/sql_analysis/models.py @@ -38,3 +38,15 @@ class SqlSchemaValidationRequest: known_types: tuple[str, ...] = () quoted_identifiers_ignore_case: bool = False catalog: Any | None = field(default=None, repr=False, compare=False) + + +@dataclass(frozen=True) +class SqlLexicalSyntax: + """Dialect rules for where SQL quoted text and comments end.""" + + backslash_escape_quotes: frozenset[str] = frozenset() + escape_string_prefix: bool = False + raw_string_prefix: bool = False + triple_quoted_strings: bool = False + nested_block_comments: bool = False + line_comment_prefixes: frozenset[str] = frozenset({"--"}) diff --git a/src/sqlbuild/executor/pipeline/_helpers/scenario.py b/src/sqlbuild/executor/pipeline/_helpers/scenario.py index 12bc3f66d3..4338f30534 100644 --- a/src/sqlbuild/executor/pipeline/_helpers/scenario.py +++ b/src/sqlbuild/executor/pipeline/_helpers/scenario.py @@ -18,6 +18,7 @@ from sqlbuild.compiler.pipeline.models import CompilePipelineResult from sqlbuild.compiler.planner.main.scenarios.scenario import build_scenario_plan from sqlbuild.compiler.planner.models import ScenarioExecutionPlan +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from sqlbuild.diagnostics.main.log_debug_event import log_debug_event from sqlbuild.errors.contracts.main.error_code import error_code from sqlbuild.errors.contracts.main.error_help import error_help @@ -34,6 +35,7 @@ from sqlbuild.executor.scenario.main._snapshots import classify_scenario_snapshot_state from sqlbuild.executor.scenario.models import ( ScenarioCaptureSettings, + ScenarioLocalReplaySource, ScenarioRunResult, ScenarioSnapshotCaptureRunResult, ScenarioSnapshotStateResult, @@ -123,6 +125,7 @@ def failed(*, scenario_name: str, exc: Exception) -> ScenarioRunResult: scenarios=scenarios, adapter=adapter, project_name=project_name, + source_lexical_syntax=adapter.sql_lexical_syntax, execute=execute, failed=failed, on_scenario_start=on_scenario_start, @@ -138,8 +141,7 @@ def run_scenario_local_test_pipeline( adapter: BaseAdapter, project_name: str, strict: bool, - capture_adapter: str | None = None, - capture_dialect: str | None = None, + replay_source: ScenarioLocalReplaySource, on_scenario_start: Callable[[CompiledSqlScenario], None] | None = None, on_scenario_complete: Callable[ [CompiledSqlScenario, ScenarioExecutionPlan | None, ScenarioRunResult], None @@ -154,8 +156,8 @@ def execute(scenario_plan: ScenarioExecutionPlan) -> ScenarioRunResult: scenario_plan=scenario_plan, adapter=adapter, strict=strict, - capture_adapter=capture_adapter, - capture_dialect=capture_dialect, + capture_adapter=replay_source.capture_adapter, + capture_dialect=replay_source.capture_dialect, ) def failed(*, scenario_name: str, exc: Exception) -> ScenarioRunResult: @@ -174,6 +176,7 @@ def failed(*, scenario_name: str, exc: Exception) -> ScenarioRunResult: scenarios=scenarios, adapter=adapter, project_name=project_name, + source_lexical_syntax=replay_source.lexical_syntax, execute=execute, failed=failed, on_scenario_start=on_scenario_start, @@ -234,6 +237,7 @@ def failed(*, scenario_name: str, exc: Exception) -> ScenarioSnapshotCaptureRunR scenarios=scenarios, adapter=adapter, project_name=project_name, + source_lexical_syntax=adapter.sql_lexical_syntax, execute=execute, failed=failed, on_scenario_start=on_scenario_start, @@ -275,6 +279,7 @@ def _run_scenarios[ResultT: _ScenarioPipelineResult]( scenarios: tuple[CompiledSqlScenario, ...], adapter: BaseAdapter, project_name: str, + source_lexical_syntax: SqlLexicalSyntax, execute: Callable[[ScenarioExecutionPlan], ResultT], failed: _ScenarioFailureResult[ResultT], on_scenario_start: Callable[[CompiledSqlScenario], None] | None, @@ -306,6 +311,7 @@ def _run_scenarios[ResultT: _ScenarioPipelineResult]( pipeline_result=pipeline_result, adapter=adapter, project_name=project_name, + source_lexical_syntax=source_lexical_syntax, ) result: ResultT = execute(scenario_plan) except Exception as exc: @@ -343,6 +349,7 @@ def select_scenario_snapshot_capture_candidates( capture_adapter: str, capture_dialect: str, refresh: bool, + source_lexical_syntax: SqlLexicalSyntax, ) -> tuple[str, ...]: """Return selected scenario names that need snapshot capture before local replay.""" @@ -358,6 +365,7 @@ def select_scenario_snapshot_capture_candidates( pipeline_result=pipeline_result, adapter=adapter, project_name=project_name, + source_lexical_syntax=source_lexical_syntax, ) snapshot_state: ScenarioSnapshotStateResult = classify_scenario_snapshot_state( project_dir=project_dir, diff --git a/src/sqlbuild/executor/scenario/_helpers/local/sql.py b/src/sqlbuild/executor/scenario/_helpers/local/sql.py index 27acba5c3e..a0e7a81d0b 100644 --- a/src/sqlbuild/executor/scenario/_helpers/local/sql.py +++ b/src/sqlbuild/executor/scenario/_helpers/local/sql.py @@ -6,7 +6,10 @@ from sqlbuild.compiler.sql_analysis.main.import_polyglot_sql import import_polyglot_sql from sqlbuild.errors.contracts.exceptions import ExecutorInputError -from sqlbuild.executor.scenario.constants import SCENARIO_LOCAL_SQL_TRANSPILE_FAILED +from sqlbuild.executor.scenario.constants import ( + LOCAL_SCENARIO_DIALECT, + SCENARIO_LOCAL_SQL_TRANSPILE_FAILED, +) def transpile_sql_for_local_duckdb( @@ -17,14 +20,16 @@ def transpile_sql_for_local_duckdb( resource_kind: str, resource_name: str, ) -> str: - """Parse SQL in the source dialect and render DuckDB SQL for local execution.""" + """Render captured-dialect SQL as DuckDB SQL; DuckDB SQL is executed as authored.""" + if source_dialect == LOCAL_SCENARIO_DIALECT: + return sql polyglot_module: Any = import_polyglot_sql() try: transpiled: list[str] = polyglot_module.transpile( sql, read=source_dialect, - write="duckdb", + write=LOCAL_SCENARIO_DIALECT, ) except (polyglot_module.PolyglotError, ValueError) as exc: _raise_transpile_error( diff --git a/src/sqlbuild/executor/scenario/constants.py b/src/sqlbuild/executor/scenario/constants.py index d99913ec48..44ca0876c3 100644 --- a/src/sqlbuild/executor/scenario/constants.py +++ b/src/sqlbuild/executor/scenario/constants.py @@ -161,3 +161,4 @@ MALFORMED_RENDERED_TYPE_TOKEN: str = ")(" UNTYPED_ARRAY_TYPE: str = "[]" JSONL_BLANK_LINE: str = "" +LOCAL_SCENARIO_DIALECT: str = "duckdb" diff --git a/src/sqlbuild/executor/scenario/models.py b/src/sqlbuild/executor/scenario/models.py index fb54de7660..436836a53c 100644 --- a/src/sqlbuild/executor/scenario/models.py +++ b/src/sqlbuild/executor/scenario/models.py @@ -12,6 +12,7 @@ ScenarioRelationMap, ) from sqlbuild.compiler.planner.types import MaterializationType, ScenarioArtifactKind +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from sqlbuild.executor.build.models import FunctionExecutionResult, SeedExecutionResult from sqlbuild.executor.run.models import ModelExecutionResult from sqlbuild.executor.scenario.types import ScenarioLocalRunStatus, ScenarioSnapshotState @@ -110,6 +111,15 @@ class ScenarioSnapshotCaptureLimits: force: bool = False +@dataclass(frozen=True) +class ScenarioLocalReplaySource: + """Project adapter whose dialect locally replayed scenario SQL is authored in.""" + + lexical_syntax: SqlLexicalSyntax + capture_adapter: str | None = None + capture_dialect: str | None = None + + @dataclass(frozen=True) class ScenarioCaptureSettings: """Capture provenance and safety settings for one scenario snapshot run.""" diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py index 685ddac907..a867076339 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/_test_types.py @@ -151,3 +151,32 @@ class ScenarioLocalCommittedSnapshotE2ETestCase: expected_count: int retained_rows_sql: str expected_rows: tuple[tuple[object, ...], ...] + + +@dataclass(frozen=True) +class ScenarioAuthoredCheckSqlE2ETestCase: + """Scenario whose check SQL uses DuckDB syntax a generic regeneration cannot parse.""" + + description: str + scenario_sql: str + expected_stdout_fragment: str + + +@dataclass(frozen=True) +class ScenarioUnresolvableCheckSqlE2ETestCase: + """Scenario whose check SQL contains an unclosed string in the target dialect.""" + + description: str + scenario_sql: str + expected_output_fragments: tuple[str, ...] + + +@dataclass(frozen=True) +class ScenarioLocalReplayProjectDialectE2ETestCase: + """Project-dialect scenario SQL replayed locally from a captured snapshot.""" + + description: str + scenario_name: str + source_fixture_sql: str + assertion_sql: str + expected_stdout_fragment: str diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/helpers.py b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/helpers.py index ca9fce031c..2ff0dbebef 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/helpers.py @@ -8,6 +8,11 @@ from pathlib import Path from typing import cast +from sqlbuild.compiler.planner.types import ScenarioArtifactKind +from sqlbuild.executor.scenario._helpers.snapshots.core import ( + build_scenario_snapshot_input_fingerprint, +) +from sqlbuild.executor.scenario.models import ScenarioSnapshotInputSpec from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import ( prepare_inline_project, query_duckdb, @@ -508,3 +513,54 @@ def assert_optional_local_replay_rows( sql=query_sql, ) assert tuple(rows) == expected_local_rows + + +def build_offline_snowflake_project_toml() -> str: + """Return a Snowflake project config that compiles offline for local replay.""" + + return ( + 'name = "scenario_demo"\n' + 'adapter = "snowflake"\n\n' + "[connection]\n" + 'account = "example"\n' + 'user = "example"\n' + 'database = "analytics"\n' + 'warehouse = "compute"\n\n' + "[defaults]\n" + 'materialized = "table"\n' + 'database = "analytics"\n' + 'schema = "main"\n' + ) + + +def restamp_scenario_snapshot_capture_adapter( + *, + project_dir: Path, + scenario_name: str, + source_fixture_sql: dict[str, str], + capture_adapter: str, +) -> None: + """Rewrite a captured snapshot so it reads as captured by another project adapter.""" + + manifest_path: Path = ( + project_dir / "tests" / "_scenario_snapshots" / scenario_name / "scenario.json" + ) + manifest_data: object = json.loads(manifest_path.read_text(encoding="utf-8")) + assert isinstance(manifest_data, dict) + manifest_data["capture_adapter"] = capture_adapter + manifest_data["capture_dialect"] = capture_adapter + manifest_data["input_fingerprint"] = build_scenario_snapshot_input_fingerprint( + scenario_name=scenario_name, + input_specs=tuple( + ScenarioSnapshotInputSpec( + kind=ScenarioArtifactKind.SOURCE, + logical_name=source_name, + file_path=Path("sources") / f"{source_name}.jsonl", + capture_sql=fixture_sql, + ) + for source_name, fixture_sql in source_fixture_sql.items() + ), + capture_adapter=capture_adapter, + capture_dialect=capture_adapter, + ) + manifest_path.write_text(json.dumps(manifest_data, indent=2) + "\n", encoding="utf-8") diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_authored_check_sql.py b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_authored_check_sql.py new file mode 100644 index 0000000000..6d734d3b2a --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/scenario/test_authored_check_sql.py @@ -0,0 +1,194 @@ +"""E2E coverage that scenario check and fixture SQL run as authored, not regenerated.""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.scenario._test_types import ( + ScenarioAuthoredCheckSqlE2ETestCase, + ScenarioLocalReplayProjectDialectE2ETestCase, + ScenarioUnresolvableCheckSqlE2ETestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.scenario.helpers import ( + build_offline_snowflake_project_toml, + build_scenario_project_files, + restamp_scenario_snapshot_capture_adapter, +) +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project, run_sqb + + +@pytest.mark.parametrize( + "test_case", + [ + ScenarioAuthoredCheckSqlE2ETestCase( + description="DuckDB integer division in fixture, expected and assertion SQL", + scenario_sql=( + "SCENARIO ();\n\n" + "WITH\n" + "__source__raw_orders AS (\n" + " SELECT 1 AS id, 21 // 2 AS amount -- integer division\n" + " UNION ALL\n" + " SELECT 2 AS id, 5 AS amount\n" + "),\n" + "__expected__order_totals AS (\n" + " SELECT 30 // 2 AS total_amount\n" + "),\n" + "__assert__no_odd_halves AS (\n" + ' SELECT * FROM __ref("order_totals") WHERE total_amount // 5 <> 3\n' + ")\n" + "SELECT 1\n" + ), + expected_stdout_fragment="PASS=1 FAIL=0", + ), + ScenarioAuthoredCheckSqlE2ETestCase( + description="DuckDB escape strings between relation markers in assertion SQL", + scenario_sql=( + "SCENARIO ();\n\n" + "WITH\n" + "__source__raw_orders AS (\n" + " SELECT 1 AS id, 10 AS amount\n" + " UNION ALL\n" + " SELECT 2 AS id, 5 AS amount\n" + "),\n" + "__assert__no_named_totals AS (\n" + " SELECT * FROM __ref(\"order_totals\") WHERE E'O\\'Brien' = 'x'\n" + " UNION ALL\n" + ' SELECT * FROM __ref("order_totals") WHERE total_amount <> 15\n' + " AND E'D\\'Arcy' IS NOT NULL\n" + ")\n" + "SELECT 1\n" + ), + expected_stdout_fragment="PASS=1 FAIL=0", + ), + ], + ids=lambda case: case.description, +) +def test_given_dialect_specific_scenario_sql_when_testing_then_runs_authored_sql( + test_case: ScenarioAuthoredCheckSqlE2ETestCase, tmp_path: Path +) -> None: + repo_files: dict[str, str] = build_scenario_project_files() + del repo_files["tests/scenarios/order_totals_pass.sql"] + del repo_files["tests/scenarios/order_totals_fail.sql"] + del repo_files["tests/scenarios/nested/orders_assert_pass.sql"] + repo_files["tests/scenarios/order_halves.sql"] = test_case.scenario_sql + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, project_name="authored_scenario_sql", repo_files=repo_files + ) + + result: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", "scenario", "test"), project_dir=project_dir + ) + + assert result.returncode == 0, result.stdout + result.stderr + assert test_case.expected_stdout_fragment in result.stdout + + +@pytest.mark.parametrize( + "test_case", + [ + ScenarioUnresolvableCheckSqlE2ETestCase( + description="DuckDB backslash before a quote leaves the assertion string unclosed", + scenario_sql=( + "SCENARIO ();\n\n" + "WITH\n" + "__source__raw_orders AS (\n" + " SELECT 1 AS id, 10 AS amount\n" + "),\n" + "__assert__no_named_totals AS (\n" + " SELECT * FROM __ref(\"order_totals\") WHERE 'O\\'Brien' = 'x'\n" + " UNION ALL\n" + ' SELECT * FROM __ref("order_totals")\n' + ")\n" + "SELECT 1\n" + ), + expected_output_fragments=("unclosed quoted string",), + ), + ], + ids=lambda case: case.description, +) +def test_given_unclosed_scenario_string_when_testing_then_fails_without_running( + test_case: ScenarioUnresolvableCheckSqlE2ETestCase, tmp_path: Path +) -> None: + repo_files: dict[str, str] = build_scenario_project_files() + del repo_files["tests/scenarios/order_totals_pass.sql"] + del repo_files["tests/scenarios/order_totals_fail.sql"] + del repo_files["tests/scenarios/nested/orders_assert_pass.sql"] + repo_files["tests/scenarios/order_halves.sql"] = test_case.scenario_sql + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, project_name="unresolvable_scenario_sql", repo_files=repo_files + ) + + result: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", "scenario", "test"), project_dir=project_dir + ) + + output: str = result.stdout + result.stderr + assert result.returncode != 0, output + for fragment in test_case.expected_output_fragments: + assert fragment in output, output + + +@pytest.mark.parametrize( + "test_case", + [ + ScenarioLocalReplayProjectDialectE2ETestCase( + description="Snowflake backslash-escaped strings around a later marker", + scenario_name="named_orders", + source_fixture_sql="SELECT 1 AS id, 10 AS amount UNION ALL SELECT 2 AS id, 5 AS amount", + assertion_sql=( + "SELECT * FROM __ref(\"order_totals\") WHERE 'O\\'Brien' = 'x'\n" + " UNION ALL\n" + ' SELECT * FROM __ref("order_totals")\n' + " WHERE total_amount <> 15 AND 'D\\'Arcy' IS NULL" + ), + expected_stdout_fragment="PASS=1 FAIL=0 ERROR=0 SKIP=0 TOTAL=1", + ), + ], + ids=lambda case: case.description, +) +def test_given_project_dialect_scenario_sql_when_replaying_locally_then_resolves_every_marker( + test_case: ScenarioLocalReplayProjectDialectE2ETestCase, tmp_path: Path +) -> None: + repo_files: dict[str, str] = build_scenario_project_files() + del repo_files["tests/scenarios/order_totals_pass.sql"] + del repo_files["tests/scenarios/order_totals_fail.sql"] + del repo_files["tests/scenarios/nested/orders_assert_pass.sql"] + repo_files[f"tests/scenarios/{test_case.scenario_name}.sql"] = ( + "SCENARIO ();\n\n" + "WITH\n" + f"__source__raw_orders AS (\n {test_case.source_fixture_sql}\n),\n" + f"__assert__no_named_totals AS (\n {test_case.assertion_sql}\n)\n" + "SELECT 1\n" + ) + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, project_name="project_dialect_local_replay", repo_files=repo_files + ) + capture_result: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", "scenario", "capture", test_case.scenario_name), + project_dir=project_dir, + ) + assert capture_result.returncode == 0, capture_result.stdout + capture_result.stderr + (project_dir / "sqlbuild_project.toml").write_text( + build_offline_snowflake_project_toml(), encoding="utf-8" + ) + restamp_scenario_snapshot_capture_adapter( + project_dir=project_dir, + scenario_name=test_case.scenario_name, + source_fixture_sql={"raw_orders": test_case.source_fixture_sql}, + capture_adapter="snowflake", + ) + + result: subprocess.CompletedProcess[str] = run_sqb( + command=("--no-color", "scenario", "test", test_case.scenario_name, "--local"), + project_dir=project_dir, + ) + + assert result.returncode == 0, result.stdout + result.stderr + assert test_case.expected_stdout_fragment in result.stdout + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/unit/src/sqlbuild/compiler/planner/_helpers/_test_types.py b/tests/unit/src/sqlbuild/compiler/planner/_helpers/_test_types.py index 4428fb9bb9..d121f10896 100644 --- a/tests/unit/src/sqlbuild/compiler/planner/_helpers/_test_types.py +++ b/tests/unit/src/sqlbuild/compiler/planner/_helpers/_test_types.py @@ -38,6 +38,7 @@ WarningSeverity, ) from sqlbuild.compiler.references.types import SqlReferenceKind +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from sqlbuild.spec.contracts.models import SourceEntry from tests.unit.src.sqlbuild.compiler.planner._helpers.sql_test_assembly._test_types import ( PlanTestChainTestCase as PlanTestChainTestCase, @@ -604,8 +605,32 @@ class ScenarioCheckSqlResolutionTestCase: description: str sql: str expected_sql: str - sql_analysis_enabled: bool = True - sql_analysis_dialect: str | None = None + + +@dataclass(frozen=True) +class ScenarioDialectCheckSqlResolutionTestCase: + description: str + lexical_syntaxes: tuple[SqlLexicalSyntax, ...] + sql: str + expected_sql: str + + +@dataclass(frozen=True) +class ScenarioCheckSqlResolutionErrorTestCase: + description: str + lexical_syntaxes: tuple[SqlLexicalSyntax, ...] + sql: str + expected_error_code: str + expected_error_fragment: str + + +@dataclass(frozen=True) +class ScenarioFixtureSqlResolutionErrorTestCase: + description: str + lexical_syntax: SqlLexicalSyntax + fixture_sql: str + expected_error_code: str + expected_error_fragment: str @dataclass(frozen=True) @@ -622,8 +647,6 @@ class ScenarioFixturePlanTestCase: expected_fixture_sql: dict[str, str] expected_fixture_targets: dict[str, str] fixture_sql_body: str | None = None - sql_analysis_enabled: bool = True - sql_analysis_dialect: str | None = None @dataclass(frozen=True) diff --git a/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_cli.py b/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_cli.py index 25133de097..27fe0d89d3 100644 --- a/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_cli.py +++ b/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_cli.py @@ -9,6 +9,7 @@ from sqlbuild.compiler.pipeline.models import CompilePipelineResult from sqlbuild.compiler.planner._helpers.scenario.cli import build_cli_scenario_plan from sqlbuild.compiler.planner.models import ScenarioArtifactName, ScenarioExecutionPlan +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from tests.unit.src.sqlbuild.compiler.planner._helpers._test_types import ( ScenarioCliPlanIdentifierLimitTestCase, ) @@ -50,6 +51,7 @@ def test_given_adapter_identifier_limit_when_building_cli_scenario_plan_then_use pipeline_result=pipeline_result, adapter=WideIdentifierPlannerTestAdapter(), project_name="scenario_demo", + source_lexical_syntax=SqlLexicalSyntax(), ) artifacts_by_kind: defaultdict[str, list[ScenarioArtifactName]] = defaultdict(list) diff --git a/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_relations.py b/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_relations.py index 82722cedca..b087bcff96 100644 --- a/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_relations.py +++ b/tests/unit/src/sqlbuild/compiler/planner/_helpers/test_scenario_relations.py @@ -6,6 +6,12 @@ import pytest +from sqlbuild.adapters.bigquery.classes.bigquery_adapter import BigQueryAdapter +from sqlbuild.adapters.databricks.classes.databricks_adapter import DatabricksAdapter +from sqlbuild.adapters.duckdb.classes.duckdb_adapter import DuckDbAdapter +from sqlbuild.adapters.postgres.classes.postgres_adapter import PostgresAdapter +from sqlbuild.adapters.snowflake.classes.snowflake_adapter import SnowflakeAdapter +from sqlbuild.adapters.sqlserver.classes.sqlserver_adapter import SqlServerAdapter from sqlbuild.compiler.compile.models import ( CompiledProject, CompiledSqlScenario, @@ -18,16 +24,21 @@ build_scenario_relation_plan, resolve_scenario_check_sql, ) +from sqlbuild.compiler.planner.exceptions import PlannerInputError from sqlbuild.compiler.planner.models import ( ScenarioExecutionPlan, ScenarioFixturePlan, ScenarioGraphPlan, ScenarioRelationPlan, ) +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from tests.unit.src.sqlbuild.compiler.planner._helpers._test_types import ( + ScenarioCheckSqlResolutionErrorTestCase, ScenarioCheckSqlResolutionTestCase, + ScenarioDialectCheckSqlResolutionTestCase, ScenarioExecutionPlanTestCase, ScenarioFixturePlanTestCase, + ScenarioFixtureSqlResolutionErrorTestCase, ScenarioRelationPlanErrorTestCase, ScenarioRelationPlanTestCase, ScenarioUnmockedSeedExecutionPlanTestCase, @@ -44,6 +55,9 @@ HASH_PREFIX: str = "51b385aebe20" SCENARIO_NAME: str = "revenue__customer_refund" +SCENARIO_PATH: str = "tests/scenarios/customer_refund.sql" +DAILY_REVENUE_TARGET: str = "scenario_schema.__sqb_51b385aebe20__model__daily_revenue" +STG_CUSTOMERS_TARGET: str = "scenario_schema.__sqb_51b385aebe20__ref__stg_customers" @pytest.mark.parametrize( @@ -208,6 +222,7 @@ def test_given_scenario_helpers_when_building_fixture_plans_then_fixtures_are_se graph_plan=test_case.graph_plan, relation_plan=relation_plan, adapter=PlannerTestAdapter(), + source_lexical_syntax=SqlLexicalSyntax(), ) assert { @@ -240,7 +255,7 @@ def test_given_scenario_helpers_when_building_fixture_plans_then_fixtures_are_se fixture_sql_body='SELECT * FROM __source("raw__orders") WHERE order_id <= 10', ), ScenarioFixturePlanTestCase( - description="resolves project source refs with polyglot without changing literals", + description="resolves project source refs in authored sql without touching literals", graph_plan=ScenarioGraphPlan( key=build_scenario_relation_test_project().models[0].key, name=SCENARIO_NAME, @@ -250,7 +265,8 @@ def test_given_scenario_helpers_when_building_fixture_plans_then_fixtures_are_se ), expected_fixture_sql={ "source:raw__orders": ( - "SELECT '__source(\"raw__orders\")' AS marker_text FROM public.raw__orders AS o" + "SELECT '__source(\"raw__orders\")' AS marker_text " + 'FROM public.raw__orders o -- __source("raw__orders")' ), }, expected_fixture_targets={ @@ -261,24 +277,6 @@ def test_given_scenario_helpers_when_building_fixture_plans_then_fixtures_are_se 'FROM __source("raw__orders") o -- __source("raw__orders")' ), ), - ScenarioFixturePlanTestCase( - description="falls back to regex source resolution when sql_analysis is disabled", - graph_plan=ScenarioGraphPlan( - key=build_scenario_relation_test_project().models[0].key, - name=SCENARIO_NAME, - target_model_names=("daily_revenue",), - model_names=("daily_revenue",), - source_fixture_names=("raw__orders",), - ), - expected_fixture_sql={ - "source:raw__orders": "SELECT * FROM public.raw__orders WHERE order_id <= 10", - }, - expected_fixture_targets={ - "source:raw__orders": "scenario_schema.__sqb_51b385aebe20__source__raw__orders", - }, - fixture_sql_body='SELECT * FROM __source("raw__orders") WHERE order_id <= 10', - sql_analysis_enabled=False, - ), ], ids=lambda case: case.description, ) @@ -311,8 +309,7 @@ def test_given_project_source_ref_in_scenario_fixture_when_building_fixture_plan graph_plan=test_case.graph_plan, relation_plan=relation_plan, adapter=PlannerTestAdapter(), - sql_analysis_enabled=test_case.sql_analysis_enabled, - sql_analysis_dialect=test_case.sql_analysis_dialect, + source_lexical_syntax=SqlLexicalSyntax(), ) assert { @@ -430,6 +427,7 @@ def notify() -> None: scenario=build_scenario_relation_test_scenario(), project=project, adapter=PlannerTestAdapter(), + source_lexical_syntax=SqlLexicalSyntax(), graph_plan=test_case.graph_plan, relation_plan=relation_plan, ) @@ -527,6 +525,7 @@ def test_given_required_unmocked_seed_when_building_execution_plan_then_loads_pr scenario=build_scenario_relation_test_scenario(include_seed_fixture=False), project=project, adapter=PlannerTestAdapter(), + source_lexical_syntax=SqlLexicalSyntax(), graph_plan=test_case.graph_plan, relation_plan=relation_plan, ) @@ -551,8 +550,8 @@ def test_given_required_unmocked_seed_when_building_execution_plan_then_loads_pr "JOIN __ref(stg_customers) sc ON dr.customer_id = sc.customer_id" ), expected_sql=( - "SELECT * FROM scenario_schema.__sqb_51b385aebe20__model__daily_revenue AS dr " - "JOIN scenario_schema.__sqb_51b385aebe20__ref__stg_customers AS sc " + "SELECT * FROM scenario_schema.__sqb_51b385aebe20__model__daily_revenue dr " + "JOIN scenario_schema.__sqb_51b385aebe20__ref__stg_customers sc " "ON dr.customer_id = sc.customer_id" ), ), @@ -563,8 +562,8 @@ def test_given_required_unmocked_seed_when_building_execution_plan_then_loads_pr "JOIN __source(raw__orders) o ON c.country_code = o.country_code" ), expected_sql=( - "SELECT * FROM scenario_schema.__sqb_51b385aebe20__seed__country_codes AS c " - "JOIN scenario_schema.__sqb_51b385aebe20__source__raw__orders AS o " + "SELECT * FROM scenario_schema.__sqb_51b385aebe20__seed__country_codes c " + "JOIN scenario_schema.__sqb_51b385aebe20__source__raw__orders o " "ON c.country_code = o.country_code" ), ), @@ -572,32 +571,31 @@ def test_given_required_unmocked_seed_when_building_execution_plan_then_loads_pr description="resolves dbt ref markers to scenario fixture relations", sql='SELECT * FROM __dbt_ref("stripe", "payments") p', expected_sql=( - "SELECT * FROM scenario_schema.__sqb_51b385aebe20__dbt_ref__stripe__payments AS p" + "SELECT * FROM scenario_schema.__sqb_51b385aebe20__dbt_ref__stripe__payments p" ), ), ScenarioCheckSqlResolutionTestCase( - description="polyglot check sql resolution ignores strings and comments", + description="check sql resolution keeps strings and comments as authored", sql=( "SELECT '__ref(daily_revenue)' AS marker_text " "FROM __ref(daily_revenue) dr -- __source(raw__orders)" ), expected_sql=( "SELECT '__ref(daily_revenue)' AS marker_text " - "FROM scenario_schema.__sqb_51b385aebe20__model__daily_revenue AS dr" + "FROM scenario_schema.__sqb_51b385aebe20__model__daily_revenue dr " + "-- __source(raw__orders)" ), ), ScenarioCheckSqlResolutionTestCase( - description="regex fallback resolves markers when sql_analysis is disabled", + description="authored function spellings and keyword case are not regenerated", sql=( - "SELECT * FROM __seed(country_codes) c " - "JOIN __source(raw__orders) o ON c.country_code = o.country_code" + "select STARTSWITH(name, 'a') as flagged, IFNULL(total, 0) as total " + "from __ref(daily_revenue) where total != 0" ), expected_sql=( - "SELECT * FROM scenario_schema.__sqb_51b385aebe20__seed__country_codes c " - "JOIN scenario_schema.__sqb_51b385aebe20__source__raw__orders o " - "ON c.country_code = o.country_code" + "select STARTSWITH(name, 'a') as flagged, IFNULL(total, 0) as total " + "from scenario_schema.__sqb_51b385aebe20__model__daily_revenue where total != 0" ), - sql_analysis_enabled=False, ), ], ids=lambda case: case.description, @@ -627,73 +625,303 @@ def test_given_scenario_check_sql_when_resolving_then_uses_scenario_relations( sql=test_case.sql, relation_plan=relation_plan, adapter=PlannerTestAdapter(), - sql_analysis_enabled=test_case.sql_analysis_enabled, - sql_analysis_dialect=test_case.sql_analysis_dialect, + source_lexical_syntax=SqlLexicalSyntax(), + source_label=SCENARIO_PATH, ) assert result == test_case.expected_sql +BACKSLASH_LEXICAL_SYNTAXES: tuple[SqlLexicalSyntax, ...] = ( + SnowflakeAdapter.sql_lexical_syntax, + BigQueryAdapter.sql_lexical_syntax, + DatabricksAdapter.sql_lexical_syntax, +) +ESCAPE_LEXICAL_SYNTAXES: tuple[SqlLexicalSyntax, ...] = ( + DuckDbAdapter.sql_lexical_syntax, + PostgresAdapter.sql_lexical_syntax, +) +CHECK_SQL_GRAPH_PLAN: ScenarioGraphPlan = ScenarioGraphPlan( + key=build_scenario_relation_test_project().models[0].key, + name=SCENARIO_NAME, + target_model_names=("daily_revenue",), + model_names=("daily_revenue",), + source_fixture_names=("raw__orders",), + ref_fixture_names=("stg_customers",), +) + + @pytest.mark.parametrize( "test_case", [ - ScenarioRelationPlanErrorTestCase( - description="raises when relation map is missing required artifact", - graph_plan=ScenarioGraphPlan( - key=build_scenario_relation_test_project().models[0].key, - name=SCENARIO_NAME, - target_model_names=("daily_revenue",), - model_names=("daily_revenue", "missing_model"), + ScenarioDialectCheckSqlResolutionTestCase( + description="backslash-escaped quote before a later marker", + lexical_syntaxes=BACKSLASH_LEXICAL_SYNTAXES, + sql=( + "select * from __ref(\"daily_revenue\") where note = 'O\\'Brien' " + 'union all select * from __ref("stg_customers")' ), - expected_error_fragment="missing model artifact 'missing_model'", - ) + expected_sql=( + f"select * from {DAILY_REVENUE_TARGET} where note = 'O\\'Brien' " + f"union all select * from {STG_CUSTOMERS_TARGET}" + ), + ), + ScenarioDialectCheckSqlResolutionTestCase( + description="even backslash-escaped quotes around a marker", + lexical_syntaxes=BACKSLASH_LEXICAL_SYNTAXES, + sql=( + "select * from __ref(daily_revenue) where note = 'O\\'Brien' " + "union all select * from __ref(stg_customers) where note = 'D\\'Arcy'" + ), + expected_sql=( + f"select * from {DAILY_REVENUE_TARGET} where note = 'O\\'Brien' " + f"union all select * from {STG_CUSTOMERS_TARGET} where note = 'D\\'Arcy'" + ), + ), + ScenarioDialectCheckSqlResolutionTestCase( + description="escape strings around a marker", + lexical_syntaxes=ESCAPE_LEXICAL_SYNTAXES, + sql=( + "select * from __ref(daily_revenue) where note = E'O\\'Brien' " + "union all select * from __ref(stg_customers) where note = e'D\\'Arcy'" + ), + expected_sql=( + f"select * from {DAILY_REVENUE_TARGET} where note = E'O\\'Brien' " + f"union all select * from {STG_CUSTOMERS_TARGET} where note = e'D\\'Arcy'" + ), + ), + ScenarioDialectCheckSqlResolutionTestCase( + description="dollar-quoted text hides quotes and markers", + lexical_syntaxes=(SnowflakeAdapter.sql_lexical_syntax, *ESCAPE_LEXICAL_SYNTAXES), + sql="select $$O'Brien __ref(stg_customers)$$ as note from __ref(daily_revenue)", + expected_sql=( + f"select $$O'Brien __ref(stg_customers)$$ as note from {DAILY_REVENUE_TARGET}" + ), + ), + ScenarioDialectCheckSqlResolutionTestCase( + description="snowflake quoted identifier ending in a backslash", + lexical_syntaxes=(SnowflakeAdapter.sql_lexical_syntax,), + sql='select "note\\", "it""s" from __ref(daily_revenue)', + expected_sql=f'select "note\\", "it""s" from {DAILY_REVENUE_TARGET}', + ), + ScenarioDialectCheckSqlResolutionTestCase( + description="bigquery backtick identifier with an escaped backtick", + lexical_syntaxes=(BigQueryAdapter.sql_lexical_syntax,), + sql="select `note\\`s` from __ref(daily_revenue)", + expected_sql=f"select `note\\`s` from {DAILY_REVENUE_TARGET}", + ), + ScenarioDialectCheckSqlResolutionTestCase( + description="bigquery triple-quoted string with an apostrophe", + lexical_syntaxes=(BigQueryAdapter.sql_lexical_syntax,), + sql="select '''it's''' as note from __ref(daily_revenue)", + expected_sql=f"select '''it's''' as note from {DAILY_REVENUE_TARGET}", + ), + ScenarioDialectCheckSqlResolutionTestCase( + description="nested block comment with an apostrophe", + lexical_syntaxes=( + PostgresAdapter.sql_lexical_syntax, + DuckDbAdapter.sql_lexical_syntax, + DatabricksAdapter.sql_lexical_syntax, + SqlServerAdapter.sql_lexical_syntax, + ), + sql="select * /* a /* b */ it's __ref(stg_customers) */ from __ref(daily_revenue)", + expected_sql=( + f"select * /* a /* b */ it's __ref(stg_customers) */ from {DAILY_REVENUE_TARGET}" + ), + ), + ScenarioDialectCheckSqlResolutionTestCase( + description="bigquery hash line comment with an apostrophe", + lexical_syntaxes=(BigQueryAdapter.sql_lexical_syntax,), + sql="select * # it's __ref(stg_customers)\nfrom __ref(daily_revenue)", + expected_sql=f"select * # it's __ref(stg_customers)\nfrom {DAILY_REVENUE_TARGET}", + ), + ScenarioDialectCheckSqlResolutionTestCase( + description="snowflake double slash line comment with an apostrophe", + lexical_syntaxes=(SnowflakeAdapter.sql_lexical_syntax,), + sql="select * // it's __ref(stg_customers)\nfrom __ref(daily_revenue)", + expected_sql=f"select * // it's __ref(stg_customers)\nfrom {DAILY_REVENUE_TARGET}", + ), + ScenarioDialectCheckSqlResolutionTestCase( + description="databricks raw string ending in a backslash", + lexical_syntaxes=(DatabricksAdapter.sql_lexical_syntax,), + sql="select r'C:\\' as path from __ref(daily_revenue)", + expected_sql=f"select r'C:\\' as path from {DAILY_REVENUE_TARGET}", + ), ], ids=lambda case: case.description, ) -def test_given_missing_scenario_artifact_when_building_relation_plan_then_raises( - test_case: ScenarioRelationPlanErrorTestCase, +def test_given_dialect_lexical_syntax_when_resolving_check_sql_then_replaces_every_marker( + test_case: ScenarioDialectCheckSqlResolutionTestCase, ) -> None: - with pytest.raises(ValueError, match=test_case.expected_error_fragment): - build_scenario_relation_plan( - project=build_scenario_relation_test_project(), - graph_plan=test_case.graph_plan, - relation_map=build_scenario_relation_test_map(), - render_qualified_name=PlannerTestAdapter().render_qualified_name, - schema="scenario_schema", + relation_plan: ScenarioRelationPlan = build_scenario_relation_plan( + project=build_scenario_relation_test_project(), + graph_plan=CHECK_SQL_GRAPH_PLAN, + relation_map=build_scenario_relation_test_map(), + render_qualified_name=PlannerTestAdapter().render_qualified_name, + schema="scenario_schema", + ) + + results: tuple[str, ...] = tuple( + resolve_scenario_check_sql( + sql=test_case.sql, + relation_plan=relation_plan, + adapter=PlannerTestAdapter(), + source_lexical_syntax=lexical_syntax, + source_label=SCENARIO_PATH, ) + for lexical_syntax in test_case.lexical_syntaxes + ) + + assert results == tuple(test_case.expected_sql for _ in test_case.lexical_syntaxes) @pytest.mark.parametrize( "test_case", [ - ScenarioCheckSqlResolutionTestCase( - description="raises on polyglot parse failure without regex fallback", - sql="SELECT * FROM __ref(daily_revenue) WHERE (", - expected_sql="could not be parsed with Polyglot", - ) + ScenarioCheckSqlResolutionErrorTestCase( + description="backslash before a quote is not an escape outside escape strings", + lexical_syntaxes=ESCAPE_LEXICAL_SYNTAXES, + sql=( + "select * from __ref(daily_revenue) where note = 'O\\'Brien' " + "union all select * from __ref(stg_customers)" + ), + expected_error_code="S511", + expected_error_fragment="unclosed quoted string", + ), + ScenarioCheckSqlResolutionErrorTestCase( + description="escaped closing quote leaves the string unclosed", + lexical_syntaxes=BACKSLASH_LEXICAL_SYNTAXES, + sql="select * from __ref(daily_revenue) where note = 'O\\'", + expected_error_code="S511", + expected_error_fragment="unclosed quoted string", + ), + ScenarioCheckSqlResolutionErrorTestCase( + description="non-nesting block comment ends before the apostrophe", + lexical_syntaxes=( + SnowflakeAdapter.sql_lexical_syntax, + BigQueryAdapter.sql_lexical_syntax, + ), + sql="select * /* a /* b */ it's */ from __ref(daily_revenue)", + expected_error_code="S511", + expected_error_fragment="unclosed quoted string", + ), + ScenarioCheckSqlResolutionErrorTestCase( + description="marker naming a relation outside the scenario", + lexical_syntaxes=(SnowflakeAdapter.sql_lexical_syntax,), + sql="select * from __ref(daily_revenue) join __ref(unknown_orders) using (id)", + expected_error_code="S511", + expected_error_fragment="references '__ref(unknown_orders)'", + ), ], ids=lambda case: case.description, ) -def test_given_invalid_check_sql_when_sql_analysis_enabled_then_raises_without_regex_fallback( - test_case: ScenarioCheckSqlResolutionTestCase, +def test_given_unresolvable_check_sql_when_resolving_then_raises_scenario_error( + test_case: ScenarioCheckSqlResolutionErrorTestCase, ) -> None: relation_plan: ScenarioRelationPlan = build_scenario_relation_plan( project=build_scenario_relation_test_project(), - graph_plan=ScenarioGraphPlan( - key=build_scenario_relation_test_project().models[0].key, - name=SCENARIO_NAME, - target_model_names=("daily_revenue",), - model_names=("daily_revenue",), + graph_plan=CHECK_SQL_GRAPH_PLAN, + relation_map=build_scenario_relation_test_map(), + render_qualified_name=PlannerTestAdapter().render_qualified_name, + schema="scenario_schema", + ) + + for lexical_syntax in test_case.lexical_syntaxes: + with pytest.raises(PlannerInputError) as error_info: + resolve_scenario_check_sql( + sql=test_case.sql, + relation_plan=relation_plan, + adapter=PlannerTestAdapter(), + source_lexical_syntax=lexical_syntax, + source_label=SCENARIO_PATH, + ) + + assert error_info.value.code == test_case.expected_error_code + assert SCENARIO_PATH in error_info.value.message + assert test_case.expected_error_fragment in error_info.value.message + + +@pytest.mark.parametrize( + "test_case", + [ + ScenarioFixtureSqlResolutionErrorTestCase( + description="unclosed string in duckdb fixture sql", + lexical_syntax=DuckDbAdapter.sql_lexical_syntax, + fixture_sql="SELECT 'O\\'Brien' AS note FROM __source(raw__orders)", + expected_error_code="S511", + expected_error_fragment="unclosed quoted string", + ), + ScenarioFixtureSqlResolutionErrorTestCase( + description="fixture sql naming an unknown project source", + lexical_syntax=SnowflakeAdapter.sql_lexical_syntax, + fixture_sql="SELECT * FROM __source(raw__returns)", + expected_error_code="S511", + expected_error_fragment="references '__source(raw__returns)'", + ), + ], + ids=lambda case: case.description, +) +def test_given_unresolvable_fixture_sql_when_planning_fixtures_then_raises_naming_file( + test_case: ScenarioFixtureSqlResolutionErrorTestCase, +) -> None: + scenario: CompiledSqlScenario = replace( + build_scenario_relation_test_scenario(include_seed_fixture=False), + authored_ctes=( + CompileSqlScenarioCte(name="__source__raw__orders", sql_body=test_case.fixture_sql), ), + source_fixture_names=("raw__orders",), + ref_fixture_names=(), + dbt_ref_fixture_names=(), + seed_fixture_names=(), + source_path=Path(SCENARIO_PATH), + ) + graph_plan: ScenarioGraphPlan = replace(CHECK_SQL_GRAPH_PLAN, ref_fixture_names=()) + relation_plan: ScenarioRelationPlan = build_scenario_relation_plan( + project=build_scenario_relation_test_project(), + graph_plan=graph_plan, relation_map=build_scenario_relation_test_map(), render_qualified_name=PlannerTestAdapter().render_qualified_name, schema="scenario_schema", ) - with pytest.raises(ValueError, match=test_case.expected_sql): - resolve_scenario_check_sql( - sql=test_case.sql, + with pytest.raises(PlannerInputError) as error_info: + build_scenario_fixture_plans( + scenario=scenario, + graph_plan=graph_plan, relation_plan=relation_plan, adapter=PlannerTestAdapter(), - sql_analysis_enabled=True, + source_lexical_syntax=test_case.lexical_syntax, + ) + + assert error_info.value.code == test_case.expected_error_code + assert SCENARIO_PATH in error_info.value.message + assert test_case.expected_error_fragment in error_info.value.message + + +@pytest.mark.parametrize( + "test_case", + [ + ScenarioRelationPlanErrorTestCase( + description="raises when relation map is missing required artifact", + graph_plan=ScenarioGraphPlan( + key=build_scenario_relation_test_project().models[0].key, + name=SCENARIO_NAME, + target_model_names=("daily_revenue",), + model_names=("daily_revenue", "missing_model"), + ), + expected_error_fragment="missing model artifact 'missing_model'", + ) + ], + ids=lambda case: case.description, +) +def test_given_missing_scenario_artifact_when_building_relation_plan_then_raises( + test_case: ScenarioRelationPlanErrorTestCase, +) -> None: + with pytest.raises(ValueError, match=test_case.expected_error_fragment): + build_scenario_relation_plan( + project=build_scenario_relation_test_project(), + graph_plan=test_case.graph_plan, + relation_map=build_scenario_relation_test_map(), + render_qualified_name=PlannerTestAdapter().render_qualified_name, + schema="scenario_schema", ) diff --git a/tests/unit/src/sqlbuild/compiler/sql_analysis/_helpers/_test_types.py b/tests/unit/src/sqlbuild/compiler/sql_analysis/_helpers/_test_types.py index 4fa311faf2..a7db4a553d 100644 --- a/tests/unit/src/sqlbuild/compiler/sql_analysis/_helpers/_test_types.py +++ b/tests/unit/src/sqlbuild/compiler/sql_analysis/_helpers/_test_types.py @@ -2,6 +2,8 @@ from dataclasses import dataclass +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax + @dataclass(frozen=True) class SkipQuotedTextSuccessTestCase: @@ -29,3 +31,44 @@ class IterCodePositionsTestCase: description: str sql: str expected_positions: tuple[tuple[int, int], ...] + + +@dataclass(frozen=True) +class DialectQuotedTextSuccessTestCase: + """Successful dialect-aware quoted-text scan case.""" + + description: str + syntax: SqlLexicalSyntax + sql: str + start: int + expected_end: int + + +@dataclass(frozen=True) +class DialectQuotedTextErrorTestCase: + """Dialect-aware quoted-text scan that cannot find the end of the text.""" + + description: str + syntax: SqlLexicalSyntax + sql: str + expected_error: str + + +@dataclass(frozen=True) +class DialectNonCodeSuccessTestCase: + """Dialect-aware quoted-text or comment scan case.""" + + description: str + syntax: SqlLexicalSyntax + sql: str + expected_end: int | None + + +@dataclass(frozen=True) +class DialectNonCodeErrorTestCase: + """Dialect-aware comment scan that cannot find the end of the comment.""" + + description: str + syntax: SqlLexicalSyntax + sql: str + expected_error: str diff --git a/tests/unit/src/sqlbuild/compiler/sql_analysis/_helpers/test_scanning.py b/tests/unit/src/sqlbuild/compiler/sql_analysis/_helpers/test_scanning.py index b57f732b56..abe21d87b2 100644 --- a/tests/unit/src/sqlbuild/compiler/sql_analysis/_helpers/test_scanning.py +++ b/tests/unit/src/sqlbuild/compiler/sql_analysis/_helpers/test_scanning.py @@ -4,10 +4,17 @@ from sqlbuild.compiler.compile.exceptions import CompileInputError from sqlbuild.compiler.sql_analysis._helpers.scanning import ( + dialect_non_code_end_impl, + dialect_quoted_text_end_impl, iter_code_positions_impl, skip_quoted_text_impl, ) +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from tests.unit.src.sqlbuild.compiler.sql_analysis._helpers._test_types import ( + DialectNonCodeErrorTestCase, + DialectNonCodeSuccessTestCase, + DialectQuotedTextErrorTestCase, + DialectQuotedTextSuccessTestCase, IterCodePositionsTestCase, SkipQuotedTextErrorTestCase, SkipQuotedTextSuccessTestCase, @@ -95,6 +102,228 @@ def test_given_unclosed_quote_when_skipping_then_raises_contextual_error( skip_quoted_text_impl(sql=test_case.sql, start=0, context=test_case.context) +ANSI_SYNTAX: SqlLexicalSyntax = SqlLexicalSyntax() +ESCAPE_PREFIX_SYNTAX: SqlLexicalSyntax = SqlLexicalSyntax(escape_string_prefix=True) +SINGLE_QUOTE_BACKSLASH_SYNTAX: SqlLexicalSyntax = SqlLexicalSyntax( + backslash_escape_quotes=frozenset({"'"}) +) +RAW_PREFIX_SYNTAX: SqlLexicalSyntax = SqlLexicalSyntax( + backslash_escape_quotes=frozenset({"'", '"'}), raw_string_prefix=True +) +TRIPLE_QUOTE_SYNTAX: SqlLexicalSyntax = SqlLexicalSyntax( + backslash_escape_quotes=frozenset({"'", '"', "`"}), triple_quoted_strings=True +) + + +@pytest.mark.parametrize( + "test_case", + ( + DialectQuotedTextSuccessTestCase( + description="backslash escapes the quote when the dialect supports it", + syntax=SINGLE_QUOTE_BACKSLASH_SYNTAX, + sql="'O\\'Brien' tail", + start=0, + expected_end=len("'O\\'Brien'"), + ), + DialectQuotedTextSuccessTestCase( + description="escaped backslash before the closing quote ends the text", + syntax=SINGLE_QUOTE_BACKSLASH_SYNTAX, + sql="'path\\\\' tail", + start=0, + expected_end=len("'path\\\\'"), + ), + DialectQuotedTextSuccessTestCase( + description="doubling still escapes the quote in backslash dialects", + syntax=SINGLE_QUOTE_BACKSLASH_SYNTAX, + sql="'customer''s' tail", + start=0, + expected_end=len("'customer''s'"), + ), + DialectQuotedTextSuccessTestCase( + description="backslash is literal where the dialect does not escape", + syntax=ANSI_SYNTAX, + sql="'O\\' tail", + start=0, + expected_end=len("'O\\'"), + ), + DialectQuotedTextSuccessTestCase( + description="quoted identifier does not take backslash escapes from string rules", + syntax=SINGLE_QUOTE_BACKSLASH_SYNTAX, + sql='"note\\" tail', + start=0, + expected_end=len('"note\\"'), + ), + DialectQuotedTextSuccessTestCase( + description="escape string prefix enables backslash escapes", + syntax=ESCAPE_PREFIX_SYNTAX, + sql="x = E'O\\'Brien' tail", + start=5, + expected_end=len("x = E'O\\'Brien'"), + ), + DialectQuotedTextSuccessTestCase( + description="identifier ending in e is not an escape string prefix", + syntax=ESCAPE_PREFIX_SYNTAX, + sql="note'O\\' tail", + start=4, + expected_end=len("note'O\\'"), + ), + DialectQuotedTextSuccessTestCase( + description="raw string prefix disables backslash escapes", + syntax=RAW_PREFIX_SYNTAX, + sql="r'C:\\' tail", + start=1, + expected_end=len("r'C:\\'"), + ), + DialectQuotedTextSuccessTestCase( + description="triple-quoted text ends at the closing triple quote", + syntax=TRIPLE_QUOTE_SYNTAX, + sql="'''it's ok''' tail", + start=0, + expected_end=len("'''it's ok'''"), + ), + DialectQuotedTextSuccessTestCase( + description="dollar-quoted text ignores quotes and backslashes", + syntax=SINGLE_QUOTE_BACKSLASH_SYNTAX, + sql="$$O\\'Brien$$ tail", + start=0, + expected_end=len("$$O\\'Brien$$"), + ), + ), + ids=lambda case: case.description, +) +def test_given_dialect_quoted_text_when_scanning_then_returns_end_position( + test_case: DialectQuotedTextSuccessTestCase, +) -> None: + result: int | None = dialect_quoted_text_end_impl( + sql=test_case.sql, start=test_case.start, syntax=test_case.syntax + ) + + assert result == test_case.expected_end + + +@pytest.mark.parametrize( + "test_case", + ( + DialectQuotedTextErrorTestCase( + description="escaped closing quote leaves the text unclosed", + syntax=SINGLE_QUOTE_BACKSLASH_SYNTAX, + sql="'O\\'", + expected_error="unclosed quoted string", + ), + DialectQuotedTextErrorTestCase( + description="unclosed triple-quoted text", + syntax=TRIPLE_QUOTE_SYNTAX, + sql="'''it's'", + expected_error="unclosed quoted string", + ), + ), + ids=lambda case: case.description, +) +def test_given_unclosed_dialect_quoted_text_when_scanning_then_raises( + test_case: DialectQuotedTextErrorTestCase, +) -> None: + with pytest.raises(CompileInputError, match=test_case.expected_error): + dialect_quoted_text_end_impl(sql=test_case.sql, start=0, syntax=test_case.syntax) + + +NESTED_COMMENT_SYNTAX: SqlLexicalSyntax = SqlLexicalSyntax(nested_block_comments=True) +HASH_COMMENT_SYNTAX: SqlLexicalSyntax = SqlLexicalSyntax( + line_comment_prefixes=frozenset({"--", "#"}) +) +SLASH_COMMENT_SYNTAX: SqlLexicalSyntax = SqlLexicalSyntax( + line_comment_prefixes=frozenset({"--", "//"}) +) + + +@pytest.mark.parametrize( + "test_case", + ( + DialectNonCodeSuccessTestCase( + description="nested block comment hides an apostrophe", + syntax=NESTED_COMMENT_SYNTAX, + sql="/* a /* b */ it's */ tail", + expected_end=len("/* a /* b */ it's */"), + ), + DialectNonCodeSuccessTestCase( + description="non-nesting block comment ends at the first close", + syntax=ANSI_SYNTAX, + sql="/* a /* b */ it's */ tail", + expected_end=len("/* a /* b */"), + ), + DialectNonCodeSuccessTestCase( + description="sibling nested comments close back to the outer comment", + syntax=NESTED_COMMENT_SYNTAX, + sql="/* /* a */ /* b */ c */ tail", + expected_end=len("/* /* a */ /* b */ c */"), + ), + DialectNonCodeSuccessTestCase( + description="hash line comment where the dialect supports it", + syntax=HASH_COMMENT_SYNTAX, + sql="# it's\ntail", + expected_end=len("# it's\n"), + ), + DialectNonCodeSuccessTestCase( + description="hash is code where the dialect has no hash comments", + syntax=ANSI_SYNTAX, + sql="# it's\ntail", + expected_end=None, + ), + DialectNonCodeSuccessTestCase( + description="double slash line comment where the dialect supports it", + syntax=SLASH_COMMENT_SYNTAX, + sql="// it's\ntail", + expected_end=len("// it's\n"), + ), + DialectNonCodeSuccessTestCase( + description="quoted text is scanned with the dialect string rules", + syntax=SINGLE_QUOTE_BACKSLASH_SYNTAX, + sql="'O\\'Brien' tail", + expected_end=len("'O\\'Brien'"), + ), + DialectNonCodeSuccessTestCase( + description="code character is not quoted text or a comment", + syntax=NESTED_COMMENT_SYNTAX, + sql="select 1", + expected_end=None, + ), + ), + ids=lambda case: case.description, +) +def test_given_dialect_sql_when_scanning_non_code_then_returns_end_position( + test_case: DialectNonCodeSuccessTestCase, +) -> None: + result: int | None = dialect_non_code_end_impl( + sql=test_case.sql, start=0, syntax=test_case.syntax + ) + + assert result == test_case.expected_end + + +@pytest.mark.parametrize( + "test_case", + ( + DialectNonCodeErrorTestCase( + description="nested block comment missing its outer close", + syntax=NESTED_COMMENT_SYNTAX, + sql="/* a /* b */ c", + expected_error="unclosed block comment", + ), + DialectNonCodeErrorTestCase( + description="unclosed block comment", + syntax=ANSI_SYNTAX, + sql="/* a", + expected_error="unclosed block comment", + ), + ), + ids=lambda case: case.description, +) +def test_given_unclosed_dialect_comment_when_scanning_then_raises( + test_case: DialectNonCodeErrorTestCase, +) -> None: + with pytest.raises(CompileInputError, match=test_case.expected_error): + dialect_non_code_end_impl(sql=test_case.sql, start=0, syntax=test_case.syntax) + + @pytest.mark.parametrize( "test_case", ( diff --git a/tests/unit/src/sqlbuild/executor/pipeline/_helpers/helpers.py b/tests/unit/src/sqlbuild/executor/pipeline/_helpers/helpers.py index 42efe089f2..9c576bb450 100644 --- a/tests/unit/src/sqlbuild/executor/pipeline/_helpers/helpers.py +++ b/tests/unit/src/sqlbuild/executor/pipeline/_helpers/helpers.py @@ -35,6 +35,7 @@ ScenarioRelationPlan, SeedPlanEntry, ) +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from sqlbuild.errors.contracts.exceptions import ExecutorInputError from sqlbuild.executor.auditing.models import AuditExecutionResult from sqlbuild.executor.scenario.constants import SCENARIO_LOCAL_JSONL_INVALID @@ -195,8 +196,9 @@ def __call__( pipeline_result: CompilePipelineResult, adapter: ScenarioPipelineTestAdapter, project_name: str, + source_lexical_syntax: SqlLexicalSyntax, ) -> ScenarioExecutionPlan: - del pipeline_result, adapter, project_name + del pipeline_result, adapter, project_name, source_lexical_syntax strategy: Callable[..., ScenarioExecutionPlan] = { self.planning_failure_name: self._raise_planning_error, }.get(scenario.name, build_scenario_pipeline_plan) diff --git a/tests/unit/src/sqlbuild/executor/pipeline/_helpers/test_scenario.py b/tests/unit/src/sqlbuild/executor/pipeline/_helpers/test_scenario.py index 795c0b8b82..08132e7cc4 100644 --- a/tests/unit/src/sqlbuild/executor/pipeline/_helpers/test_scenario.py +++ b/tests/unit/src/sqlbuild/executor/pipeline/_helpers/test_scenario.py @@ -10,6 +10,7 @@ from sqlbuild.compiler.compile.models import CompiledSqlScenario from sqlbuild.compiler.pipeline.models import CompilePipelineResult from sqlbuild.compiler.planner.models import ScenarioExecutionPlan +from sqlbuild.compiler.sql_analysis.models import SqlLexicalSyntax from sqlbuild.executor.pipeline._helpers import scenario as scenario_pipeline from sqlbuild.executor.scenario._helpers.local import execution as local_scenario from sqlbuild.executor.scenario.constants import ( @@ -19,6 +20,7 @@ ) from sqlbuild.executor.scenario.models import ( ScenarioCaptureSettings, + ScenarioLocalReplaySource, ScenarioRunResult, ScenarioSnapshotStateResult, ) @@ -268,6 +270,7 @@ def test_given_local_scenario_planning_failure_when_running_then_canonical_attem adapter=ScenarioLocalPipelineTestAdapter(), project_name="waffle_shop", strict=False, + replay_source=ScenarioLocalReplaySource(lexical_syntax=SqlLexicalSyntax()), ) assert results[0].status == ExecutionStatus.FAILED @@ -392,6 +395,7 @@ def classify_snapshot( adapter=adapter, project_name="waffle_shop", strict=test_case.strict, + replay_source=ScenarioLocalReplaySource(lexical_syntax=SqlLexicalSyntax()), ) result: ScenarioRunResult = results[0] diff --git a/tests/unit/src/sqlbuild/executor/scenario/_helpers/_test_types.py b/tests/unit/src/sqlbuild/executor/scenario/_helpers/_test_types.py index be60a39820..1abc8b0deb 100644 --- a/tests/unit/src/sqlbuild/executor/scenario/_helpers/_test_types.py +++ b/tests/unit/src/sqlbuild/executor/scenario/_helpers/_test_types.py @@ -185,3 +185,11 @@ class FirstFailureDetailsTestCase: expected_error_code: str | None expected_error_help: str | None expected_error_message: str | None + + +@dataclass(frozen=True) +class LocalScenarioSqlTestCase: + description: str + source_dialect: str | None + sql: str + expected_sql: str diff --git a/tests/unit/src/sqlbuild/executor/scenario/_helpers/test_local_sql.py b/tests/unit/src/sqlbuild/executor/scenario/_helpers/test_local_sql.py new file mode 100644 index 0000000000..4f10779043 --- /dev/null +++ b/tests/unit/src/sqlbuild/executor/scenario/_helpers/test_local_sql.py @@ -0,0 +1,44 @@ +from __future__ import annotations + +import pytest + +from sqlbuild.executor.scenario._helpers.local.sql import transpile_sql_for_local_duckdb +from tests.unit.src.sqlbuild.executor.scenario._helpers._test_types import ( + LocalScenarioSqlTestCase, +) + + +@pytest.mark.parametrize( + "test_case", + [ + LocalScenarioSqlTestCase( + description="DuckDB captures run their authored SQL without regeneration", + source_dialect="duckdb", + sql="select ifnull(total, 0) as total, list_value(1, 2) as ids from orders", + expected_sql="select ifnull(total, 0) as total, list_value(1, 2) as ids from orders", + ), + LocalScenarioSqlTestCase( + description="other captured dialects are translated for local DuckDB", + source_dialect="snowflake", + sql="select IFNULL(total, 0) as total from orders", + expected_sql="SELECT COALESCE(total, 0) AS total FROM orders", + ), + ], + ids=lambda case: case.description, +) +def test_given_captured_dialect_when_preparing_local_scenario_sql_then_only_translates_foreign_sql( + test_case: LocalScenarioSqlTestCase, +) -> None: + result: str = transpile_sql_for_local_duckdb( + sql=test_case.sql, + source_dialect=test_case.source_dialect, + scenario_name="order_totals", + resource_kind="model", + resource_name="order_totals", + ) + + assert result == test_case.expected_sql + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/website/src/content/docs/docs/concepts/scenarios.mdx b/website/src/content/docs/docs/concepts/scenarios.mdx index 94526bed1c..b7dd4bbfeb 100644 --- a/website/src/content/docs/docs/concepts/scenarios.mdx +++ b/website/src/content/docs/docs/concepts/scenarios.mdx @@ -257,7 +257,7 @@ This: 1. Checks snapshot freshness via the input fingerprint (missing/stale snapshots are skipped by default) 2. Creates a temporary DuckDB database at `target/run/scenarios//local.duckdb` 3. Loads JSONL snapshots into typed DuckDB tables using column metadata from `scenario.json` -4. Transpiles model and check SQL from the project adapter dialect to DuckDB +4. Transpiles model and check SQL from the project adapter dialect to DuckDB; SQL captured from a DuckDB project runs as authored 5. Builds functions, models, and runs expected/assertion checks in DuckDB 6. Keeps the local DuckDB file for inspection (it lives under `target/`, so it's always retained) @@ -317,3 +317,4 @@ See the [CLI reference](/docs/cli/scenario) for full command documentation. - Custom materializations are not supported in scenarios yet. Scenario models using custom materializations will fail with a clear error. - Local replay transpiles SQL from the project adapter dialect to DuckDB. Adapter-specific SQL that cannot be translated will produce a clear error with the failing resource name and reason. +- Outside local replay, scenario fixture, expected and assertion SQL runs as authored: SQLBuild replaces `__ref`, `__source`, `__seed` and `__dbt_ref` calls with relation names and may wrap a partial fixture to add its missing columns, but never regenerates the query. Calls inside comments and quoted text are left unchanged. Quoted text and comments follow the project adapter's rules, including with `--local`: backslash escapes on Snowflake, BigQuery and Databricks, `E'...'` strings on DuckDB and Postgres, nested block comments on DuckDB, Postgres, Databricks and SQL Server, `#` line comments on BigQuery, and `//` line comments on Snowflake. A call naming a relation the scenario does not provide, or SQL with an unclosed string or comment, fails planning with `S511` naming the scenario file.