Skip to content

mz-deploy: report statement offsets in raw-file coordinates (DEX-60) - #37377

Open
sjwiesman wants to merge 3 commits into
MaterializeInc:mainfrom
sjwiesman:seth/dex-60-statement-offsets-raw
Open

mz-deploy: report statement offsets in raw-file coordinates (DEX-60)#37377
sjwiesman wants to merge 3 commits into
MaterializeInc:mainfrom
sjwiesman:seth/dex-60-statement-offsets-raw

Conversation

@sjwiesman

@sjwiesman sjwiesman commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Statement byte offsets were computed against the variable-resolved SQL,
but every consumer (the CLI diagnostic renderer and the LSP) reads the
raw file. A :var whose value differs in length from the reference
shifted all later offsets, so diagnostics pointed at the wrong span and
could miss the token entirely. Map each offset back to the raw file with
the substitution table at parse time, fixing both consumers at the
source.

Ticket: DEX-60

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

🤖 Generated with Claude Code

https://claude.ai/code/session_01VAcnVpSQi8ZgF5LekQRwvw

Statement byte offsets were computed against the variable-resolved SQL,
but every consumer (the CLI diagnostic renderer and the LSP) reads the
raw file. A `:var` whose value differs in length from the reference
shifted all later offsets, so diagnostics pointed at the wrong span and
could miss the token entirely. Map each offset back to the raw file with
the substitution table at parse time, fixing both consumers at the
source.

Ticket: DEX-60

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread src/mz-deploy/src/project/syntax/parser.rs Outdated
@sjwiesman
sjwiesman marked this pull request as ready for review July 1, 2026 14:15
@tonydu-mz

Copy link
Copy Markdown
Contributor

One follow-up worth adding: the clamp branch in resolved_to_original (offset inside a substitution's resolved span → returns sub.original_start) has no test, and it's the one non-obvious path here.

It's reachable when a variable's value contains a ;: the substitution injects a statement separator that isn't in the raw file, so the following statement starts inside the substitution's resolved span. Worth a test that a value like :x1; SELECT 2 maps that second statement's byte_offset back to the :x reference and stays in-bounds of the raw file. This is the branch most likely to regress silently under a future refactor.

Lower priority, also uncovered: multiple substitutions before a statement (cumulative delta), and a resolved + unresolved variable interleaved (delta staying correct across a reference that isn't recorded as a substitution).

@sjwiesman

Copy link
Copy Markdown
Contributor Author

@tonydu-mz pushed a fix

Add end-to-end coverage for the clamp branch in `resolved_to_original`,
which was only exercised by unit tests with hand-built substitutions. A
variable value containing `;` injects a statement separator absent from
the raw file, so the following statement begins inside the
substitution's resolved span. The new test asserts its offset clamps
back to the reference and stays within the raw file's bounds.

A second test covers cumulative delta across multiple substitutions
before a statement.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copy link
Copy Markdown
Contributor

will look into this tonight

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.

2 participants