adapter: fix panic in EXPLAIN FILTER PUSHDOWN FOR MATERIALIZED VIEW#37649
Open
ggevay wants to merge 1 commit into
Open
adapter: fix panic in EXPLAIN FILTER PUSHDOWN FOR MATERIALIZED VIEW#37649ggevay wants to merge 1 commit into
ggevay wants to merge 1 commit into
Conversation
EXPLAIN FILTER PUSHDOWN FOR MATERIALIZED VIEW acquired read holds on all imports of the view's cached physical plan, including its compute (index) imports. An index that the view was planned against can be dropped while the view keeps running. The compute controller removes the dropped index's collection immediately, so acquiring a read hold on it panicked with "missing compute collection" and crashed environmentd. This was deterministic, not a race: any EXPLAIN FILTER PUSHDOWN on such a view panicked after the index drop. The pushdown explanation only reads persist part stats of the plan's storage imports, so compute holds serve no purpose here. Acquire read holds on the storage imports only. Those are catalog dependencies of the view and cannot be concurrently dropped while it exists. Fixes SQL-519. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Fixes SQL-519
Motivation
EXPLAIN FILTER PUSHDOWN FOR MATERIALIZED VIEWpanicked withmissing compute collection: CollectionMissing(...)and crashed environmentd when the view's cached physical plan imported an index that was later dropped. This was seen in CI in the Parallel Workload (0dt deploy) job.This was not a race. Dropping an index that a materialized view was planned against is allowed (the view's dataflow keeps running), but the compute controller removes the index's collection immediately on drop. The view's cached physical plan still imports the dropped index, so once the index was dropped, every
EXPLAIN FILTER PUSHDOWN FOR MATERIALIZED VIEWon that view deterministically panicked while acquiring read holds on the plan's imports, until the next restart.Description
explain_pushdown_materialized_viewnow acquires read holds only on the plan's storage imports instead of the full import bundle. The pushdown explanation only reads persist part stats of the plan's storage imports, so compute holds served no purpose. Storage imports are catalog dependencies of the view and cannot be dropped while it exists, so acquiring holds on them cannot hit a missing collection on the coordinator task.User-facing change:
EXPLAIN FILTER PUSHDOWN FOR MATERIALIZED VIEWno longer crashes environmentd after an index imported by the view's cached plan was dropped. The statement now succeeds and reports stats for the plan's storage imports (possibly zero rows if the plan reads entirely from the dropped index).The
EXPLAIN FILTER PUSHDOWN FOR SELECTpath is unaffected: it goes through the staged peek sequencing, which re-checks plan validity and selects indexes from the current catalog synchronously on the coordinator task.Verification
EXPLAIN FILTER PUSHDOWN FOR MATERIALIZED VIEW) crashes environmentd with the exact CI panic. After the fix the statement returns cleanly, both for an index-only plan and for a plan with mixed storage and index imports.test/sqllogictest/explain/pushdown.sltthat panics environmentd without the fix and passes with it.test/sqllogictest/explain/pushdown.slt,materialized_views.slt,statement_timeout.slt, andexplain/broken_statements.slt, all green.🤖 Generated with Claude Code