adapter: track ALTER SOURCE's source in the purified statement's dependencies - #38638
Open
tonydu-mz wants to merge 1 commit into
Conversation
### Motivation `ALTER SOURCE` is purified off the coordinator thread. Its target source is carried in the AST as an `UnresolvedItemName`, so name resolution never records it and the statement's `resolved_ids`, from which `PlanValidity`'s dependency set is built, does not contain it. A source dropped concurrently with that off-thread purification therefore passed the validity check when the purified statement came back, and planning then called `get_entry` on the missing id, panicking the coordinator with "catalog out of sync" (`plan_purified_alter_source_add_subsource`, `plan_purified_alter_source_refresh_references`). ### Description Extend the purified statement's dependency ids with the source that `mz_sql::pure::statement_source` resolves (introduced in #38564, where the same resolution feeds the pre-purification authorization check). With the id tracked, `PlanValidity::check` detects the drop and the coordinator repurifies the original statement, ending in a clean unknown-item error instead of a panic. For `CREATE TABLE ... FROM SOURCE` the source is already in `resolved_ids`, so the extension is a no-op there. ### Verification The race needs a drop landing between purification spawn and completion; there is no deterministic hook for that in sqllogictest. Verified by code trace: `PlanValidity::check` uses `try_get_entry` and returns `ConcurrentDependencyDrop` for a missing id, and the `PurifiedStatementReady` handler repurifies on validity failure. `ALTER SOURCE IF EXISTS` on a never-resolving name contributes no phantom dependency (`statement_source` returns `None`). Stacked on #38564.
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.
Motivation
ALTER SOURCEis purified off the coordinator thread. Its target source iscarried in the AST as an
UnresolvedItemName, so name resolution never recordsit and the statement's
resolved_ids, from whichPlanValidity's dependencyset is built, does not contain it.
A source dropped concurrently with that off-thread purification therefore
passed the validity check when the purified statement came back, and planning
then called
get_entryon the missing id, panicking the coordinator with"catalog out of sync" (
plan_purified_alter_source_add_subsource,plan_purified_alter_source_refresh_references).Description
Extend the purified statement's dependency ids with the source that
mz_sql::pure::statement_sourceresolves (introduced in #38564, where the sameresolution feeds the pre-purification authorization check). With the id
tracked,
PlanValidity::checkdetects the drop and the coordinator repurifiesthe original statement, ending in a clean unknown-item error instead of a
panic. For
CREATE TABLE ... FROM SOURCEthe source is already inresolved_ids, so the extension is a no-op there.Verification
The race needs a drop landing between purification spawn and completion; there
is no deterministic hook for that in sqllogictest. Verified by code trace:
PlanValidity::checkusestry_get_entryand returnsConcurrentDependencyDropfor a missing id, and thePurifiedStatementReadyhandler repurifies on validity failure.
ALTER SOURCE IF EXISTSon anever-resolving name contributes no phantom dependency (
statement_sourcereturns
None).Stacked on #38564.