Preserve plans on RENAME for MV / Index / CT#36145
Merged
ggevay merged 1 commit intoMaterializeInc:mainfrom Apr 21, 2026
Merged
Preserve plans on RENAME for MV / Index / CT#36145ggevay merged 1 commit intoMaterializeInc:mainfrom
ggevay merged 1 commit intoMaterializeInc:mainfrom
Conversation
6a4f4e2 to
722dfd2
Compare
def-
reviewed
Apr 20, 2026
Contributor
def-
left a comment
There was a problem hiding this comment.
The added tests lgtm, thanks for adding!
Since MaterializeInc#35834 the `optimized_plan`, `physical_plan`, and `dataflow_metainfo` fields live on the `CatalogItem` itself rather than in the separate `CatalogPlans` side table. However, `parse_item_inner` hardcoded `None` for those fields when reconstructing a `CatalogItem` from `create_sql`. A `RENAME` on a materialized view (or index / continual task) goes through the retract+add path in `apply_item_update`, which calls `deserialize_item` → `parse_item` → `parse_item_inner` with the previous `CatalogItem` as `previous_item`. The plan fields would be silently dropped, so a subsequent `EXPLAIN OPTIMIZED PLAN FOR MATERIALIZED VIEW ...` would fail with "cannot find dataflow metainformation for materialized view ... in catalog". Fix: carry the three plan fields from `previous_item` through to the newly-reconstructed item. This is done as a post-match stamp to avoid duplicating the logic into each of the three per-variant construction sites and to keep the change local. Also add a regression test to `test/sqllogictest/rename.slt` that exercises `ALTER MATERIALIZED VIEW ... RENAME TO ...` and `ALTER INDEX ... RENAME TO ...` followed by `EXPLAIN OPTIMIZED / PHYSICAL PLAN`. Co-authored-by: Junie <junie@jetbrains.com>
722dfd2 to
83050ba
Compare
mtabebe
approved these changes
Apr 20, 2026
Contributor
mtabebe
left a comment
There was a problem hiding this comment.
Simple fix for the temporary problem. Thanks for the tests too
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.
Fixes https://github.com/MaterializeInc/database-issues/issues/11316
Since #35834 the
optimized_plan,physical_plan, anddataflow_metainfofields live on theCatalogItemitself rather than in the separateCatalogPlansside table. However,parse_item_innerhardcodedNonefor those fields when reconstructing aCatalogItemfromcreate_sql. ARENAMEon a materialized view (or index / continual task) goes through the retract+add path inapply_item_update, which callsdeserialize_item→parse_item→parse_item_innerwith the previousCatalogItemasprevious_item. The plan fields would be silently dropped, so a subsequentEXPLAIN OPTIMIZED PLAN FOR MATERIALIZED VIEW ...would fail with "cannot find dataflow metainformation for materialized view ... in catalog".Fix: carry the three plan fields from
previous_itemthrough to the newly-reconstructed item. This is done as a post-match stamp to avoid duplicating the logic into each of the three per-variant construction sites and to keep the change local.(Note: I'm still planning to make the plans inside catalog items non-optional, as discussed here, so this is a temporary fix.)