adapter: Refactor how we store query plans in the catalog by ggevay · Pull Request #35834 · MaterializeInc/materialize

ggevay · 2026-04-02T15:04:04Z

This is preparation for moving those catalog operations into the catalog implications framework that involve query plans, such as CREATE MATERIALIZED VIEW.

This PR solves a long-standing weirdness in our catalog: We used to have query plans and other metainfo in a separate part of the catalog (CatalogPlans), instead of inside the catalog items. This happened only due to historical reasons. This PR fixes this.

I recommend reviewing commit by commit.

The 1st is just some renaming to avoid confusion between locally optimized plans (with just optimize_mir_local without view inlining) and fully (globally) optimized plans (that is, after view inlining, with optimize_dataflow).

The 2nd commit is the main thing, see commit msg for details.

(Note that even after this PR, we are still adding the plans in the side effect closure of the catalog transactions that adds the catalog items, e.g., in create_materialized_view_finish. #35837 will be a follow-up PR, which will instead add the plans to the catalog::Ops that comprise the catalog transactions.)

Nightly: https://buildkite.com/materialize/nightly/builds/15973, but there is a staggering amount of unrelated redness, so it's hard to read.

github-actions · 2026-04-02T15:04:18Z

Thanks for opening this PR! Here are a few tips to help make the review process smooth for everyone.

PR title guidelines

Use imperative mood: "Fix X" not "Fixed X" or "Fixes X"
Be specific: "Fix panic in catalog sync when controller restarts" not "Fix bug" or "Update catalog code"
Prefix with area if helpful: compute: , storage: , adapter: , sql:

Pre-merge checklist

The PR title is descriptive and will make sense in the git log.
This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.
This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).

ggevay · 2026-04-09T11:39:40Z

-    /// reason we end up with two identical notices being dropped by the same
-    /// call, the result will contain only one instance of that notice.
-    #[mz_ore::instrument(level = "trace")]
-    pub fn drop_plans_and_metainfos(


(This is getting replaced by drop_optimizer_notices. Plans don't need separate dropping anymore.)

mtabebe

This change makes sense to me, a few thoughts on testing:

is there a test that would test behaviour of a drop with cascade?
is there any test for drop after a restart, or migration (I care less about this, but I'm just curious)

mtabebe · 2026-04-09T20:33:30Z

+            CatalogItem::Index(idx) => idx.optimized_plan = Some(Arc::new(plan)),
+            CatalogItem::MaterializedView(mv) => mv.optimized_plan = Some(Arc::new(plan)),
+            CatalogItem::ContinualTask(ct) => ct.optimized_plan = Some(Arc::new(plan)),
+            other => panic!("set_optimized_plan called on {:?}", other.typ()),


Would it be useful to also panic with the id?

(Same comment for other places with the panic)

Yes, thx, changed the 3 places

mtabebe · 2026-04-09T20:43:56Z

+    /// Set the optimized plan for the item identified by `id`.
+    ///
+    /// # Panics
+    /// If the item is not an `Index`, `MaterializedView`, or


I'm curious if there is a way to enforce this outside of the match? Maybe it isn't necessary since you have follow up work planned?

Well, I don't really see a way to enforce it outside at the moment. But yeah, some time later I'd like to make these plan fields non-optional, and just populate them already when we create the catalog item (mentioned also here), and then we'd just delete these setters.

Yeah that makes sense :)

mtabebe · 2026-04-09T20:52:37Z

+
+            // Clean up plans and optimizer notices for items that
+            // were retracted but not replaced (i.e., truly dropped).
+            let dropped_entries: Vec<CatalogEntry> = retractions


Does this make dropping happen atomically with the catalog operation now?

Yes, in the sense that it now happens inside the transact_inner call in transact, whereas before the PR it was happening after the tx.commit that is after the transact_inner call.

Move optimized_plan, physical_plan, and dataflow_metainfo from the CatalogPlans side-car maps onto the actual catalog objects (Index, MaterializedView, ContinualTask), eliminating the CatalogPlans struct entirely. Key changes: - Add transient plan fields (Option, #[serde(skip)]) to Index, MaterializedView, and ContinualTask. These are recomputed during bootstrap and must be skipped to avoid Testdrive catalog consistency mismatches. - Add accessor methods on CatalogItem and item_mut() on CatalogEntry. - Move notices_by_dep_id and all plan setter/getter/drop methods into CatalogState, improving encapsulation and snapshot consistency. - Automate optimizer notice cleanup: apply_updates now collects truly-dropped entries after each timestamp group, cleans up notices_by_dep_id, and generates notice retraction builtin table updates inline. This eliminates the manual cleanup calls from Catalog::transact. - Introduce pack_optimizer_notice_updates (unresolved form for apply_updates); pack_optimizer_notices now delegates to it. - Update all construction sites to initialize plan fields as None; update apply_replacement to copy plan fields. Co-authored-by: Junie <junie@jetbrains.com>

ggevay · 2026-04-17T12:50:55Z

TFTR! I've added some tests, see last commit.

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>

Fixes MaterializeInc/database-issues#11316 Since #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. (Note: I'm still planning to make the plans inside catalog items non-optional, as discussed [here](https://materializeinc.slack.com/archives/C08A62E0751/p1774627385747809?thread_ts=1774623679.171309&cid=C08A62E0751), so this is a temporary fix.) Co-authored-by: Junie <junie@jetbrains.com>

ggevay added the A-ADAPTER Topics related to the ADAPTER layer label Apr 2, 2026

ggevay mentioned this pull request Apr 2, 2026

adapter: Further preparations for handling query plans in catalog implications #35837

Merged

ggevay force-pushed the implications-optimizer-preparation branch 8 times, most recently from ebb0a8f to b55299a Compare April 9, 2026 11:37

ggevay commented Apr 9, 2026

View reviewed changes

ggevay force-pushed the implications-optimizer-preparation branch 2 times, most recently from d2db776 to 374b9f8 Compare April 9, 2026 12:03

ggevay marked this pull request as ready for review April 9, 2026 12:03

ggevay requested review from a team as code owners April 9, 2026 12:03

ggevay requested a review from mtabebe April 9, 2026 12:03

mtabebe reviewed Apr 9, 2026

View reviewed changes

ggevay force-pushed the implications-optimizer-preparation branch 2 times, most recently from f846ee4 to d22addd Compare April 10, 2026 11:49

mtabebe approved these changes Apr 10, 2026

View reviewed changes

ggevay and others added 2 commits April 17, 2026 14:03

More accurate name for locally optimized expression catalog fields

7a0fb71

ggevay force-pushed the implications-optimizer-preparation branch from d22addd to 5b2aabd Compare April 17, 2026 12:50

ggevay enabled auto-merge (squash) April 17, 2026 12:51

Add DROP CASCADE and restart tests for notices

d65857f

ggevay force-pushed the implications-optimizer-preparation branch from 5b2aabd to d65857f Compare April 17, 2026 12:52

ggevay merged commit a632912 into MaterializeInc:main Apr 17, 2026
121 checks passed

ggevay mentioned this pull request Apr 19, 2026

Preserve plans on RENAME for MV / Index / CT #36145

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adapter: Refactor how we store query plans in the catalog#35834

adapter: Refactor how we store query plans in the catalog#35834
ggevay merged 3 commits intoMaterializeInc:mainfrom
ggevay:implications-optimizer-preparation

ggevay commented Apr 2, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Apr 2, 2026

Uh oh!

ggevay Apr 9, 2026 •

edited

Loading

Uh oh!

mtabebe left a comment

Uh oh!

mtabebe Apr 9, 2026

Uh oh!

mtabebe Apr 9, 2026

Uh oh!

ggevay Apr 10, 2026 •

edited

Loading

Uh oh!

mtabebe Apr 9, 2026

Uh oh!

ggevay Apr 10, 2026

Uh oh!

mtabebe Apr 10, 2026

Uh oh!

mtabebe Apr 9, 2026

Uh oh!

ggevay Apr 10, 2026

Uh oh!

ggevay commented Apr 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ggevay commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Apr 2, 2026

PR title guidelines

Pre-merge checklist

Uh oh!

ggevay Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtabebe left a comment

Choose a reason for hiding this comment

Uh oh!

mtabebe Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

mtabebe Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

ggevay Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtabebe Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

ggevay Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

mtabebe Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

mtabebe Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

ggevay Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

ggevay commented Apr 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ggevay commented Apr 2, 2026 •

edited

Loading

ggevay Apr 9, 2026 •

edited

Loading

ggevay Apr 10, 2026 •

edited

Loading