Skip to content

fix(test): run model SQL in SQL tests exactly as build sends it - #893

Merged
kvlonge merged 7 commits into
mainfrom
feat/chi-452-test-sql-parity
Oct 1, 2026
Merged

kvlonge merged 7 commits into
mainfrom
feat/chi-452-test-sql-parity

Conversation

@kvlonge

@kvlonge kvlonge commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Why

sqb test did not run the SQL that build sends. For model bodies starting with WITH, the test renderer parsed the SQL, renamed every CTE and regenerated it with the SQL library, then patched known spelling changes (Snowflake STARTS_WITH back to STARTSWITH) with a regex. Tests could pass on SQL the warehouse never runs (CHI-452, split from CHI-402).

Changes

  • The renderer lifts the model's CTEs as slices of the authored text (dialect-aware: quotes, comments, dollar quotes, escapes, T-SQL brackets); no parsing, renaming or regeneration. Name collisions with different text use the existing nested fallback; identical CTEs appear once.
  • T-SQL (no nested WITH): a colliding CTE is renamed by token span only when provably safe (definition, relation references, column qualifiers), verified against the rest of the token stream; otherwise, and for fixture-vs-model collisions, the test is refused with an error naming both CTEs.
  • Trailing statement terminators are stripped from lifted bodies.
  • The STARTS_WITH patch and sql_test_dialect.py are removed.

Verification

  • Parity test: each model's build SQL appears in the rendered test SQL, apart from fixture substitution.
  • Render tests for Snowflake synonyms kept as written; Rust tests for the splitter and T-SQL rename; T-SQL rendered vs nested chains give identical rows on DuckDB; DuckDB E2E through sqb test.
  • make test, make check-ci; one independent review plus a follow-up.

Split a model's leading WITH into authored CTE and body slices and lift
them verbatim instead of parsing, renaming every CTE and regenerating
SQL. A CTE name already lifted with different text keeps the existing
nested fallback; on T-SQL, which rejects nested WITH, the test is
refused with a diagnostic naming the colliding CTEs.
Rendered SQL-test queries now keep the authored function spelling, so
the regex that rewrote STARTS_WITH back to STARTSWITH is no longer
needed.
Check that rendered SQL tests contain each model's build SQL verbatim,
apart from fixture relation substitution, keep Snowflake function
synonyms as authored, refuse T-SQL CTE collisions, and run authored CTEs
with comments, dollar quotes and nested WITH through the DuckDB CLI.
On T-SQL, rename a CTE whose name collides with an already lifted CTE,
together with its relation references, by token span to a fresh name
when every other token is provably unchanged; refuse otherwise. Drop
trailing statement terminators from lifted bodies, compare lifted CTE
text ignoring surrounding whitespace, and name upstream models in
collision messages.
A CTE name followed by a dot in a relation position is the leading part
of a database or schema qualified object, so it is kept rather than
renamed. A column qualifier beside such an object is ambiguous and the
rename is refused.
@kvlonge
kvlonge enabled auto-merge (squash) October 1, 2026 09:59
@kvlonge
kvlonge merged commit b6cbd5f into main Oct 1, 2026
33 checks passed
@kvlonge
kvlonge deleted the feat/chi-452-test-sql-parity branch October 1, 2026 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant