Preserve plans on RENAME for MV / Index / CT

src/adapter/src/catalog/state.rs

@@ -1121,7 +1121,24 @@ impl CatalogState {
 
         let mut uncached_expr = None;
 
-        let item = match plan {
+        // Carry over the plans (`optimized_plan`, `physical_plan`,
+        // `dataflow_metainfo`) from the previous incarnation of this item when
+        // re-parsing an existing item (e.g. after a RENAME). These fields live
+        // on the `CatalogItem` since #35834, but are not reconstructable from
+        // `create_sql` alone — they are populated by the sequencer `_finish`
+        // paths at create time, and by the expression-cache / bootstrap
+        // rendering path on boot. If we don't preserve them here, a RENAME
+        // silently drops the plans and dataflow metainfo for the affected
+        // MV/Index/CT.
+        let previous_plans = previous_item.as_ref().map(|item| {
+            (
+                item.optimized_plan().cloned(),
+                item.physical_plan().cloned(),
+                item.dataflow_metainfo().cloned(),
+            )
+        });
+
+        let mut item = match plan {
             Plan::CreateTable(CreateTablePlan { table, .. }) => {
                 let collections = extra_versions
                     .iter()
@@ -1508,6 +1525,18 @@ impl CatalogState {
             }
         };
 
+        // Carry over the plans (`optimized_plan`, `physical_plan`,
+        // `dataflow_metainfo`) from the previous incarnation of this item, if
+        // any. See the comment on `previous_plans` above.
+        if let Some((prev_optimized, prev_physical, prev_metainfo)) = previous_plans {
+            if let Some((optimized_plan, physical_plan, dataflow_metainfo)) = item.plan_fields_mut()
+            {
+                *optimized_plan = prev_optimized;
+                *physical_plan = prev_physical;
+                *dataflow_metainfo = prev_metainfo;
+            }
+        }
+
         Ok((item, uncached_expr))
     }
 

src/catalog/src/memory/objects.rs

@@ -1848,6 +1848,37 @@ impl CatalogItem {
         }
     }
 
+    /// Returns mutable references to the plan fields (`optimized_plan`,
+    /// `physical_plan`, `dataflow_metainfo`) on plan-bearing items
+    /// (`Index`, `MaterializedView`, `ContinualTask`), or `None` for
+    /// other item kinds.
+    pub fn plan_fields_mut(
+        &mut self,
+    ) -> Option<(
+        &mut Option<Arc<DataflowDescription<OptimizedMirRelationExpr>>>,
+        &mut Option<Arc<DataflowDescription<ComputePlan>>>,
+        &mut Option<DataflowMetainfo<Arc<OptimizerNotice>>>,
+    )> {
+        match self {
+            CatalogItem::Index(idx) => Some((
+                &mut idx.optimized_plan,
+                &mut idx.physical_plan,
+                &mut idx.dataflow_metainfo,
+            )),
+            CatalogItem::MaterializedView(mv) => Some((
+                &mut mv.optimized_plan,
+                &mut mv.physical_plan,
+                &mut mv.dataflow_metainfo,
+            )),
+            CatalogItem::ContinualTask(ct) => Some((
+                &mut ct.optimized_plan,
+                &mut ct.physical_plan,
+                &mut ct.dataflow_metainfo,
+            )),
+            _ => None,
+        }
+    }
+
     /// Whether this item represents a storage collection.
     pub fn is_storage_collection(&self) -> bool {
         match self {

test/sqllogictest/alter.slt

@@ -228,3 +228,58 @@ query T
 SELECT create_sql FROM (SHOW CREATE SOURCE s)
 ----
 CREATE SOURCE materialize.public.s IN CLUSTER quickstart FROM LOAD GENERATOR COUNTER;
+
+# Regression tests: ALTER MATERIALIZED VIEW SET CLUSTER and
+# ALTER MATERIALIZED VIEW / INDEX SET (RETAIN HISTORY ...) retract+add
+# the catalog item via `apply_item_update`. `parse_item_inner` must carry
+# the optimized_plan / physical_plan / dataflow_metainfo fields forward
+# from the previous catalog entry, so that EXPLAIN can still show them.
+# See also the analogous RENAME test in rename.slt.
+
+statement ok
+CREATE TABLE t_alter_plans (a int)
+
+statement ok
+CREATE MATERIALIZED VIEW mv_alter_plans AS SELECT a + 1 AS b FROM t_alter_plans
+
+statement ok
+CREATE DEFAULT INDEX idx_alter_plans ON mv_alter_plans
+
+statement ok
+ALTER MATERIALIZED VIEW mv_alter_plans SET CLUSTER quickstart
+
+statement ok
+EXPLAIN OPTIMIZED PLAN FOR MATERIALIZED VIEW mv_alter_plans
+
+statement ok
+EXPLAIN PHYSICAL PLAN FOR MATERIALIZED VIEW mv_alter_plans
+
+statement ok
+ALTER MATERIALIZED VIEW mv_alter_plans SET (RETAIN HISTORY FOR '1m')
+
+statement ok
+EXPLAIN OPTIMIZED PLAN FOR MATERIALIZED VIEW mv_alter_plans
+
+statement ok
+EXPLAIN PHYSICAL PLAN FOR MATERIALIZED VIEW mv_alter_plans
+
+statement ok
+ALTER MATERIALIZED VIEW mv_alter_plans RESET (RETAIN HISTORY)
+
+statement ok
+EXPLAIN OPTIMIZED PLAN FOR MATERIALIZED VIEW mv_alter_plans
+
+statement ok
+ALTER INDEX idx_alter_plans SET (RETAIN HISTORY FOR '1m')
+
+statement ok
+EXPLAIN OPTIMIZED PLAN FOR INDEX idx_alter_plans
+
+statement ok
+EXPLAIN PHYSICAL PLAN FOR INDEX idx_alter_plans
+
+statement ok
+ALTER INDEX idx_alter_plans RESET (RETAIN HISTORY)
+
+statement ok
+EXPLAIN OPTIMIZED PLAN FOR INDEX idx_alter_plans

test/sqllogictest/privilege_checks.slt

@@ -3609,8 +3609,52 @@ REVOKE USAGE ON SCHEMA mz_temp FROM PUBLIC
 db error: ERROR: cannot grant or revoke privileges on schema mz_temp because it is a temporary schema
 
 # Disable rbac checks.
-
 simple conn=mz_system,user=mz_system
 ALTER SYSTEM SET enable_rbac_checks TO false;
 ----
 COMPLETE 0
+
+# Regression tests: ALTER ... OWNER TO ... and GRANT/REVOKE retract + re-add
+# the affected catalog item via `apply_item_update`. Plans on MV / INDEX
+# items must survive, so that EXPLAIN can still show them.
+statement ok
+CREATE ROLE plan_owner_other
+
+statement ok
+CREATE TABLE t_owner_plans (a int)
+
+statement ok
+CREATE MATERIALIZED VIEW mv_owner_plans AS SELECT a + 1 AS b FROM t_owner_plans
+
+statement ok
+CREATE DEFAULT INDEX idx_owner_plans ON mv_owner_plans
+
+statement ok
+ALTER MATERIALIZED VIEW mv_owner_plans OWNER TO plan_owner_other
+
+statement ok
+EXPLAIN OPTIMIZED PLAN FOR MATERIALIZED VIEW mv_owner_plans
+
+statement ok
+EXPLAIN PHYSICAL PLAN FOR MATERIALIZED VIEW mv_owner_plans
+
+statement ok
+ALTER INDEX idx_owner_plans OWNER TO plan_owner_other
+
+statement ok
+EXPLAIN OPTIMIZED PLAN FOR INDEX idx_owner_plans
+
+statement ok
+EXPLAIN PHYSICAL PLAN FOR INDEX idx_owner_plans
+
+statement ok
+GRANT SELECT ON mv_owner_plans TO plan_owner_other
+
+statement ok
+EXPLAIN OPTIMIZED PLAN FOR MATERIALIZED VIEW mv_owner_plans
+
+statement ok
+REVOKE SELECT ON mv_owner_plans FROM plan_owner_other
+
+statement ok
+EXPLAIN OPTIMIZED PLAN FOR MATERIALIZED VIEW mv_owner_plans

test/sqllogictest/rename.slt

@@ -655,3 +655,66 @@ SHOW CREATE VIEW temp_view;
 ----
 mz_temp.temp_view
 CREATE TEMPORARY VIEW mz_temp.temp_view AS SELECT * FROM materialize.public_renamed.non_temp_table;
+
+# Regression test: RENAME on a MATERIALIZED VIEW / INDEX must preserve the
+# plans and dataflow metainfo stored on the catalog item. Before the fix,
+# `EXPLAIN OPTIMIZED PLAN FOR MATERIALIZED VIEW ...` would fail after a
+# rename with "cannot find dataflow metainformation for materialized view
+# ... in catalog", because `parse_item_inner` hardcoded `None` for those
+# fields when reconstructing the item from `create_sql`.
+
+statement ok
+CREATE TABLE t_rename_metainfo (a int)
+
+statement ok
+CREATE MATERIALIZED VIEW mv_rename_metainfo AS SELECT a + 1 AS b FROM t_rename_metainfo
+
+statement ok
+ALTER MATERIALIZED VIEW mv_rename_metainfo RENAME TO mv_rename_metainfo_renamed
+
+statement ok
+EXPLAIN OPTIMIZED PLAN FOR MATERIALIZED VIEW mv_rename_metainfo_renamed
+
+statement ok
+EXPLAIN PHYSICAL PLAN FOR MATERIALIZED VIEW mv_rename_metainfo_renamed
+
+# Same, for indexes.
+
+statement ok
+CREATE INDEX idx_rename_metainfo ON t_rename_metainfo (a)
+
+statement ok
+ALTER INDEX idx_rename_metainfo RENAME TO idx_rename_metainfo_renamed
+
+statement ok
+EXPLAIN OPTIMIZED PLAN FOR INDEX idx_rename_metainfo_renamed
+
+statement ok
+EXPLAIN PHYSICAL PLAN FOR INDEX idx_rename_metainfo_renamed
+
+# Regression test: ALTER SCHEMA ... RENAME TO ... retracts+re-adds every
+# item in the schema via `tx.update_items`; plans on MV / INDEX items in
+# the schema must survive, so that EXPLAIN can still show them.
+statement ok
+CREATE SCHEMA s_rename_metainfo
+
+statement ok
+CREATE TABLE s_rename_metainfo.t (a int)
+
+statement ok
+CREATE MATERIALIZED VIEW s_rename_metainfo.mv AS SELECT a + 1 AS b FROM s_rename_metainfo.t
+
+statement ok
+CREATE DEFAULT INDEX idx_in_schema ON s_rename_metainfo.mv
+
+statement ok
+ALTER SCHEMA s_rename_metainfo RENAME TO s_rename_metainfo_renamed
+
+statement ok
+EXPLAIN OPTIMIZED PLAN FOR MATERIALIZED VIEW s_rename_metainfo_renamed.mv
+
+statement ok
+EXPLAIN PHYSICAL PLAN FOR MATERIALIZED VIEW s_rename_metainfo_renamed.mv
+
+statement ok
+EXPLAIN OPTIMIZED PLAN FOR INDEX s_rename_metainfo_renamed.idx_in_schema

test/sqllogictest/transform/notice/index_too_wide_for_literal_constraints.slt

@@ -297,6 +297,54 @@ Index␠too␠wide␠for␠literal␠constraints  Index␠materialize.notices.t6
 Index␠too␠wide␠for␠literal␠constraints  Index␠materialize.notices.t6_idx_x_y_z␠on␠t6(x,␠y,␠z)␠is␠too␠wide␠to␠use␠for␠literal␠equalities␠`y␠=␠5`.  Index␠materialize.notices.t6_idx_x_y_z␠on␠t6(x,␠y,␠z)␠is␠too␠wide␠to␠use␠for␠literal␠equalities␠`y␠=␠█`.  If␠your␠literal␠equalities␠filter␠out␠many␠rows,␠create␠an␠index␠whose␠key␠exactly␠matches␠your␠literal␠equalities:␠(y).  NULL  CREATE␠INDEX␠ON␠t6(y);  NULL  sql_statements
 Index␠too␠wide␠for␠literal␠constraints  Index␠materialize.notices.t6_idx_x_y_z␠on␠t6(x,␠y,␠z)␠is␠too␠wide␠to␠use␠for␠literal␠equalities␠`y␠=␠7`.  Index␠materialize.notices.t6_idx_x_y_z␠on␠t6(x,␠y,␠z)␠is␠too␠wide␠to␠use␠for␠literal␠equalities␠`y␠=␠█`.  If␠your␠literal␠equalities␠filter␠out␠many␠rows,␠create␠an␠index␠whose␠key␠exactly␠matches␠your␠literal␠equalities:␠(y).  NULL  CREATE␠INDEX␠ON␠t6(y);  NULL  sql_statements
 
+# Rename a materialized view and verify its notices are preserved.
+# Regression test for a bug where RENAME dropped dataflow_metainfo (and thus
+# the associated mz_optimizer_notices rows) from the catalog item.
+statement ok
+ALTER MATERIALIZED VIEW mv2 RENAME TO mv2_renamed;
+
+query TTTTTTTT
+SELECT
+  n.notice_type, n.message, n.redacted_message, n.hint, n.redacted_hint, n.action, n.redacted_action, n.action_type
+FROM
+  mz_internal.mz_notices n JOIN
+  mz_catalog.mz_materialized_views mv ON(n.object_id = mv.id)
+WHERE
+  mv.name IN ('mv1', 'mv2_renamed');
+----
+Index␠too␠wide␠for␠literal␠constraints  Index␠materialize.notices.t6_idx_x_y␠on␠t6(x,␠y)␠is␠too␠wide␠to␠use␠for␠literal␠equalities␠`y␠=␠5`.  Index␠materialize.notices.t6_idx_x_y␠on␠t6(x,␠y)␠is␠too␠wide␠to␠use␠for␠literal␠equalities␠`y␠=␠█`.  If␠your␠literal␠equalities␠filter␠out␠many␠rows,␠create␠an␠index␠whose␠key␠exactly␠matches␠your␠literal␠equalities:␠(y).  NULL  CREATE␠INDEX␠ON␠t6(y);  NULL  sql_statements
+Index␠too␠wide␠for␠literal␠constraints  Index␠materialize.notices.t6_idx_x_y␠on␠t6(x,␠y)␠is␠too␠wide␠to␠use␠for␠literal␠equalities␠`y␠=␠7`.  Index␠materialize.notices.t6_idx_x_y␠on␠t6(x,␠y)␠is␠too␠wide␠to␠use␠for␠literal␠equalities␠`y␠=␠█`.  If␠your␠literal␠equalities␠filter␠out␠many␠rows,␠create␠an␠index␠whose␠key␠exactly␠matches␠your␠literal␠equalities:␠(y).  NULL  CREATE␠INDEX␠ON␠t6(y);  NULL  sql_statements
+Index␠too␠wide␠for␠literal␠constraints  Index␠materialize.notices.t6_idx_x_y_z␠on␠t6(x,␠y,␠z)␠is␠too␠wide␠to␠use␠for␠literal␠equalities␠`y␠=␠5`.  Index␠materialize.notices.t6_idx_x_y_z␠on␠t6(x,␠y,␠z)␠is␠too␠wide␠to␠use␠for␠literal␠equalities␠`y␠=␠█`.  If␠your␠literal␠equalities␠filter␠out␠many␠rows,␠create␠an␠index␠whose␠key␠exactly␠matches␠your␠literal␠equalities:␠(y).  NULL  CREATE␠INDEX␠ON␠t6(y);  NULL  sql_statements
+Index␠too␠wide␠for␠literal␠constraints  Index␠materialize.notices.t6_idx_x_y_z␠on␠t6(x,␠y,␠z)␠is␠too␠wide␠to␠use␠for␠literal␠equalities␠`y␠=␠7`.  Index␠materialize.notices.t6_idx_x_y_z␠on␠t6(x,␠y,␠z)␠is␠too␠wide␠to␠use␠for␠literal␠equalities␠`y␠=␠█`.  If␠your␠literal␠equalities␠filter␠out␠many␠rows,␠create␠an␠index␠whose␠key␠exactly␠matches␠your␠literal␠equalities:␠(y).  NULL  CREATE␠INDEX␠ON␠t6(y);  NULL  sql_statements
+
+# Also check that EXPLAIN OPTIMIZED PLAN FOR MATERIALIZED VIEW works after
+# the rename (this reads the stored optimized_plan + dataflow_metainfo off
+# the catalog item, which is what the bug above broke).
+query T multiline
+EXPLAIN OPTIMIZED PLAN WITH(humanized expressions) AS VERBOSE TEXT FOR MATERIALIZED VIEW mv2_renamed;
+----
+materialize.notices.mv2_renamed:
+  Filter (#1{y} = 7)
+    ReadIndex on=t6 t6_idx_x_y=[*** full scan ***]
+
+Used Indexes:
+  - materialize.notices.t6_idx_x_y (*** full scan ***)
+
+Target cluster: quickstart
+
+Notices:
+  - Notice: Index materialize.notices.t6_idx_x_y on t6(x, y) is too wide to use for literal equalities `y = 7`.
+    Hint: If your literal equalities filter out many rows, create an index whose key exactly matches your literal equalities: (y).
+  - Notice: Index materialize.notices.t6_idx_x_y_z on t6(x, y, z) is too wide to use for literal equalities `y = 7`.
+    Hint: If your literal equalities filter out many rows, create an index whose key exactly matches your literal equalities: (y).
+
+EOF
+
+# Rename back, so that later queries in this file (which filter by mv name)
+# continue to work unchanged.
+statement ok
+ALTER MATERIALIZED VIEW mv2_renamed RENAME TO mv2;
+
 # Drop one of the indexes associated with the notices.
 statement ok
 DROP INDEX t6_idx_x_y_z;