mz-deploy: identifier-overflow panics and reference validation (DEX-52, DEX-63, DEX-59, DEX-51, DEX-47, DEX-62) - #37367
Open
sjwiesman wants to merge 5 commits into
Conversation
Applying a profile suffix to a cluster name that is already near the identifier length limit produced a name longer than the limit. The cluster rewrite passed that name to `Ident::new(...).expect(...)`, so compiling such a project under a suffixing profile panicked. Validate suffixed cluster names once, where the name map is built, and surface an over-length name as a `LoadError` so it reports as a normal compile error. The rewrite now consumes pre-validated `Ident`s. Ticket: DEX-52 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hema name Rewriting database and schema names in mod statements appended the profile/staging suffix and built the result with `Ident::new(...).expect(...)`. A name already near the identifier length limit overflowed it once suffixed, so compiling or applying such a project panicked. Record the over-length condition as a `LoadError` through the rewrite visitors and propagate it, so it surfaces as a normal error. Ticket: DEX-63 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Converting an `UnresolvedItemName` to a `DatabaseIdent` matched only one, two, or three qualification levels and treated anything else as `unreachable!()`. The SQL parser places no cap on dotted names, so a supporting statement referencing a four-part name (e.g. `COMMENT ON COLUMN a.b.c.d.id`) panicked during reference validation. Make the conversion fallible. The four reference validators (index, grant, comment target, column comment) now report an `InvalidIdentifier` validation error for an unusable reference name. The object's own-name extraction falls back to a non-matching identifier so name validation reports it rather than panicking. Ticket: DEX-59 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The cross-database reference rewrite looked up the database name map with `Ident::to_string()`, which renders a non-bare identifier in its quoted SQL form (`café` -> `"café"`). The map is keyed by the raw directory name, so any database whose name needs quoting missed the lookup and kept its unsuffixed name under a profile. Key the lookup off the raw identifier via `as_str()`. Ticket: DEX-51 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Supporting statements (GRANT, COMMENT, CREATE INDEX) are normalized to fully qualified names, but the object they were checked against came from the un-normalized CREATE statement. For an unqualified object (the idiomatic style) that identifier carried no database or schema, so the schema comparison short-circuited and a supporting statement pointing at a different schema was silently accepted. Build the object identifier from the file's fully qualified name so the schema is always present. Ticket: DEX-47, DEX-62 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sjwiesman
marked this pull request as ready for review
July 1, 2026 14:20
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.
Supporting statements (GRANT, COMMENT, CREATE INDEX) are normalized to
fully qualified names, but the object they were checked against came from
the un-normalized CREATE statement. For an unqualified object (the
idiomatic style) that identifier carried no database or schema, so the
schema comparison short-circuited and a supporting statement pointing at
a different schema was silently accepted. Build the object identifier
from the file's fully qualified name so the schema is always present.
Ticket: DEX-47, DEX-62
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
The cross-database reference rewrite looked up the database name map with
Ident::to_string(), which renders a non-bare identifier in its quotedSQL form (
café->"café"). The map is keyed by the raw directoryname, so any database whose name needs quoting missed the lookup and kept
its unsuffixed name under a profile. Key the lookup off the raw
identifier via
as_str().Ticket: DEX-51
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
Converting an
UnresolvedItemNameto aDatabaseIdentmatched only one,two, or three qualification levels and treated anything else as
unreachable!(). The SQL parser places no cap on dotted names, so asupporting statement referencing a four-part name (e.g.
COMMENT ON COLUMN a.b.c.d.id) panicked during reference validation.Make the conversion fallible. The four reference validators
(index, grant, comment target, column comment) now report an
InvalidIdentifiervalidation error for an unusable reference name. Theobject's own-name extraction falls back to a non-matching identifier so
name validation reports it rather than panicking.
Ticket: DEX-59
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
Rewriting database and schema names in mod statements appended the
profile/staging suffix and built the result with
Ident::new(...).expect(...). A name already near the identifier lengthlimit overflowed it once suffixed, so compiling or applying such a
project panicked. Record the over-length condition as a
LoadErrorthrough the rewrite visitors and propagate it, so it surfaces as a
normal error.
Ticket: DEX-63
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
Applying a profile suffix to a cluster name that is already near the
identifier length limit produced a name longer than the limit. The
cluster rewrite passed that name to
Ident::new(...).expect(...), socompiling such a project under a suffixing profile panicked. Validate
suffixed cluster names once, where the name map is built, and surface an
over-length name as a
LoadErrorso it reports as a normal compileerror. The rewrite now consumes pre-validated
Idents.Ticket: DEX-52
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
🤖 Generated with Claude Code
https://claude.ai/code/session_01VAcnVpSQi8ZgF5LekQRwvw