Repository navigation
fix: align SQL scanners and remove obsolete implementations - #815
Merged
Merged
Conversation
SQL lint's interpolation-site lexer treated backticks as ordinary code, so in BigQuery and Databricks projects a close paren inside a backtick identifier ended a macro call early, and an apostrophe inside a backtick identifier hid every later macro site. `sqb format` then rewrote the hidden macro (for example `@label(...)` became `@LABEL(...)`), breaking compile. Lint now derives backtick quoting from the configured lint dialect using the same policy rules use, in both the native preparation pass and the Python fallback used for formatting and non-ASCII bodies. Postgres and SQL Server keep treating backticks as code; backslash escapes are unchanged.
When a model's dialect parser rejected it, the generic fallback rewrote `:` paths and `->` lambdas while skipping only quoted text. An apostrophe in a `--` or `/* */` comment (for example `-- don't`) opened a phantom quote that never closed, so normalisation stopped and the fallback parse failed. The fallback now skips comments and quotes through the shared scanner.
…ot fallback When Polyglot could not parse an expected CTE, the fallback split set operations with its own scanner: it ignored comments (an apostrophe in `-- don't` hid every later branch, and `UNION` inside a comment split), left `DISTINCT` at the start of a `UNION DISTINCT` branch so the branch parse raised a raw parser error, and split on INTERSECT/EXCEPT even though the parsed path rejects them. Both the fallback and the textual expected-projection path now use one shared `split_union_branches` over the comment- and quote-aware `iter_code_positions`, handling `UNION [ALL|DISTINCT]`. The fallback rejects top-level INTERSECT/EXCEPT with the parsed path's error, and defers to the textual path when a split branch still cannot be parsed.
The textual assertion fallback prefixed lifted CTEs as `WITH ... {body}`
even when the assertion body began with its own `WITH`, producing
`WITH a AS (...) WITH b AS (...) SELECT ...`. That resolved SQL is what the
renderer emits when an assertion's lifted CTEs conflict with earlier steps.
The fallback now uses the shared `with_leading_ctes`, merging lifted CTEs
into the body's leading WITH clause like every other planning path.
…ctor `split_unions`, `split_top_level` and `find_top_level_keyword` each repeated the same loop around the shared scanner: skip comments and quotes, track parenthesis depth, then act on top-level code. They now run on a single `scan_code` walker whose visitor advances, resumes after a consumed keyword, or stops with a result. Characterisation tests pin union splits, comma splits, keyword offsets, multibyte text, unbalanced parentheses and unclosed-input errors; results are unchanged.
Loader version identity hashed the destination, write strategy, cursor, unique key and contract but not the declared `columns`, so editing a loader's column declarations (for example through a module-level column list) left its recorded version unchanged. Loaders now include `columns` in their identity decorator config, matching assets. Retry and connection mode remain excluded for every kind. Upgrading gives one round of new versions for loaders with declared columns: their first run after the upgrade records a new version hash.
…ck every branch Expected CTEs now accept UNION, INTERSECT and EXCEPT, each with an optional ALL or DISTINCT, in model, macro, UDF and table-function tests, and every branch's projection names and count must match the first branch. - Native extractor: `split_unions` becomes `split_set_operations`, so an INTERSECT or EXCEPT branch is validated instead of being folded into the first branch; `* EXCEPT (...)` star modifiers are not treated as operators. - Polyglot path: Intersect and Except nodes are flattened like Union instead of being rejected as non-SELECT branches. - Shared Python fallback: `split_union_branches` becomes `split_set_operation_branches` and no longer rejects INTERSECT/EXCEPT. - Model-test expected-CTE errors now name the actual CTE (for example `__expected__orders`) instead of `__expected__<model>`, matching direct-logic tests; the message wording is otherwise unchanged.
Drop render_query_with_cursor_bounds, render_seed_select_after_cursor, get_relation_max_cursor and count_rows from BaseAdapter, StrictAdapter, DiffMixin, DuckDbBackedAdapter and every first-class adapter, plus PromotionStrategy.CREATE_NEW. No engine code called them. Tests that used count_rows for verification use a small COUNT(*) helper; the unsupported promotion strategy test now uses the catalogued but unhandled build_aside strategy.
same_target_audit_gate_reuse_decision was never called by the executor. Remove it, the BINDING_SET_CHANGED reason only it produced, and its tests. The invalid-metadata diagnostic test now exercises the live reuse_from decision, which shares the parsing path.
…methods Remove the Python _ModelHeaderParser, which only served as a cross-check oracle for the native MODEL header parser, together with its corpus parity test. Remove the StateBackend methods replace_virtual_environment_function_refs, replace_virtual_environment_seed_refs and get_physical_relation_ancestry, which only tests called; tests now write refs through the node-ref APIs production uses. No state-table layout, stored format or reader used by the engine changes. append_microbatch_event is kept: retargeting its Postgres tests to append_microbatch_events exposed a type-resolution failure in the bulk path that needs a separate fix.
This removes a deprecated public alias: sqlbuild.execution_history.EventLogStorage is gone. Use LifecycleEventLogStorage, which it pointed to. docs/execution-history.md is updated accordingly.
Remove the unused fix-run machinery (FixRunResult, LintFixRecord, fix planning helpers and their CLI renderers), other unreferenced lint helpers and constants, and the test-only map_neutralized_offset. LINT_ENGINE_SQLBUILD is kept and now used by native.py instead of the duplicated "sqlbuild" literal.
Remove unreferenced SQL helper modules (decision signatures, model-name grammar, passthrough detection), table_parts, ModelNameParts and unused constants. The public custom-rule API is unchanged: SqlDocument.subqueries and RuleOption.integer_list stay.
… code Remove unused Polyglot analysis constants, the scenario _dedupe_names helper and the never-emitted S505 scenario code, the unused _LEGACY_MODEL_HOOK_KEYS copy in SQL-test attachment, and the SQL-test CTE SQLite cache module, which has had no production caller since #758.
Remove the unused loader-to-node conversion module, the placeholder DiscoveredPython{Task,Asset,Check}Node classes, PythonSqlRunLifecyclePlan.python_phase, and fields that were set but never read: PythonNodeIdentity.object_module/object_qualname, PythonNodeDependencyEdge.upstream_function/downstream_function and PythonSqlRunLifecyclePlan.read_side_sql_keys. The identity definition payload and hashes are unchanged.
ModelExecutionResult.microbatch_applied_intervals and MicrobatchPhaseOutcome.applied_intervals were accumulated for every batch but never read. Remove the fields and their bookkeeping; execution behaviour is unchanged.
virtual diff and promote each carried an identical private _read_model_versions. Both now use one read_ref_model_versions entry in virtual/state/main/environments.
sqlglot was removed as a runtime dependency; the member is now SQL_ANALYSIS. Only the Python name changes: the enum value stays "sql_analysis", so nothing serialised or displayed changes.
…cleanup # Conflicts: # src/sqlbuild/adapter/contract/classes/base_adapter.py # src/sqlbuild/adapter/contract/classes/duckdb_backed_adapter.py # src/sqlbuild/adapter/contract/classes/strict_adapter.py # src/sqlbuild/adapters/bigquery/classes/bigquery_adapter.py # src/sqlbuild/adapters/databricks/classes/databricks_adapter.py # src/sqlbuild/adapters/postgres/classes/postgres_adapter.py # src/sqlbuild/adapters/snowflake/classes/snowflake_adapter.py # src/sqlbuild/adapters/sqlserver/classes/sqlserver_adapter.py # src/sqlbuild/compiler/compile/constants.py # src/sqlbuild/executor/run/_helpers/reuse/fingerprint_metadata.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Scanner fallbacks disagreed on valid SQL, loader schemas were absent from identity, and unused implementations obscured live code. Cleanup also exposed a PostgreSQL bulk-event typing failure.
Changes
Verification