adapter: handle types and functions in ALTER SCHEMA RENAME by ggevay · Pull Request #37364 · MaterializeInc/materialize

ggevay · 2026-06-30T16:13:19Z

Fixes SQL-265. Edit: Now it also fixes SQL-401, see 2nd commit. (I've added it to this PR rather than a new one, because this PR was making the occurrence of SQL-401 more likely.)

Motivation

ALTER SCHEMA RENAME never updated references to a schema's user-defined types,
which could corrupt persisted create_sql and panic on the next boot:

thread 'main' panicked at src/adapter/src/catalog/apply.rs:
PlanError(Catalog(UnknownSchema("s1"))): invalid persisted SQL:
CREATE TYPE "materialize"."s1"."mytype" AS LIST (ELEMENT TYPE = [s20 AS "pg_catalog"."int4"])

A schema keeps items, types, and functions in three separate maps. Three code
paths only looked at items:

RenameSchema (transact.rs) iterated only schema.items, so the renamed
schema's own types kept create_sql pointing at the old schema name. That
string fails to re-parse on restart (the panic above).
The rewrite visitor CreateSqlRewriteSchema (transform.rs) never descended
into data types. A type is referenced by a schema-qualified name only in
"data type" position (a cast, a column type, a nested LIST/MAP element
type), reached through a different AST node than item references. So every
dependent that used one of the schema's types (view, materialized view,
table, another type) kept a stale reference, regardless of object kind.
check_items (consistency.rs) iterated only schema.items, so the
consistency checker never flagged the corrupted type create_sql.

This has been broken since ALTER SCHEMA RENAME was introduced (#22365).

Changes

RenameSchema and check_items now iterate types and functions
alongside items.
CreateSqlRewriteSchema now overrides visit_data_type_mut to rewrite
schema-qualified names in data type position, descending through
Array/List/Map into the element type. Because every object kind reaches
a type through this node, the one override covers all of them.

The in-memory catalog resolves references by id, so the corruption is invisible
until the persisted create_sql is re-parsed on boot. SHOW CREATE likewise
re-resolves by id and masks it, so the tests assert against the raw create_sql
column.

Tests

test/restart/mzcompose.py: a restart regression test that renames a schema
containing a type referenced by a view, materialized view, table column, and a
nested type in another schema, then asserts all of them survive a restart and
their raw create_sql references the new schema name.
test/sqllogictest/rename.slt: an in-process test over the same object kinds,
asserting against the raw create_sql column.

🤖 Generated with Claude Code

def-

Small extra test:

diff --git a/test/sqllogictest/rename.slt b/test/sqllogictest/rename.slt
index 5505839cbd..d591df9fe5 100644
--- a/test/sqllogictest/rename.slt
+++ b/test/sqllogictest/rename.slt
@@ -798,3 +798,40 @@ ty_mv
 ty_outer
 ty_table
 ty_view
+
+# Regression test: a temporary object that depends on more than one type in the
+# renamed schema must be rewritten exactly once. Extending RENAME to walk
+# schema.types makes temporary type-dependents reachable, but the dedup guard in
+# the rename loop only covers persistent items (it keys off items_to_update,
+# which temporary items never enter). A temp object reached once per type it uses
+# is therefore enqueued once per type: N retractions and N additions of the same
+# state. Catalog apply consolidates updates by kind, so two identical retractions
+# collapse to a diff of -2, which trips `expect("catalog state cannot have diff
+# other than -1 or 1")` and panics the coordinator. Before the dedup fix, the
+# ALTER SCHEMA below aborts the rename; after it, the temp table survives and
+# stays queryable through both renamed types.
+#
+# See SQL-401 for the underlying temp-object rename hazard.
+
+statement ok
+CREATE SCHEMA tydup1
+
+statement ok
+CREATE TYPE tydup1.tdl AS LIST (ELEMENT TYPE = int4)
+
+statement ok
+CREATE TYPE tydup1.tdr AS LIST (ELEMENT TYPE = int4)
+
+# One temp object depending on two types in the schema is what triggers the
+# double enqueue: the rename loop reaches it once via tdl and once via tdr.
+
+statement ok
+CREATE TEMP TABLE tmp_two_types (a tydup1.tdl, b tydup1.tdr)
+
+statement ok
+ALTER SCHEMA tydup1 RENAME TO tydup2
+
+query I
+SELECT count(*) FROM tmp_two_types
+----
+0

Fails:

thread 'coordinator' panicked at src/adapter/src/catalog/apply.rs:200:22:
catalog state cannot have diff other than -1 or 1: "invalid diff -2"
   5: core::panicking::panic_fmt
   6: core::result::unwrap_failed
   7: <core::result::Result<mz_catalog::memory::objects::StateDiff, alloc::string::String>>::expect
   8: <mz_adapter::catalog::state::CatalogState>::consolidate_updates::{closure#1}
   9: core::iter::adapters::map::map_try_fold::<(mz_catalog::memory::objects::StateUpdateKind, mz_repr::timestamp::Timestamp, mz_ore::overflowing::Overflowing<i64>), mz_catalog::memory::objects::StateUpdate, alloc::vec::in_place_drop::InPlaceDrop<mz_catalog::memory::objects::StateUpdate>, core::result::Result<alloc::vec::in_place_drop::InPlaceDrop<mz_catalog::memory::objects::StateUpdate>, !>, <mz_adapter::catalog::state::CatalogState>::consolidate_updates::{closure#1}, alloc::vec::in_place_collect::write_in_place_with_drop<mz_catalog::memory::objects::StateUpdate>::{closure#0}>::{closure#0}
  10: <alloc::vec::into_iter::IntoIter<(mz_catalog::memory::objects::StateUpdateKind, mz_repr::timestamp::Timestamp, mz_ore::overflowing::Overflowing<i64>)> as core::iter::traits::iterator::Iterator>::try_fold::<alloc::vec::in_place_drop::InPlaceDrop<mz_catalog::memory::objects::StateUpdate>, core::iter::adapters::map::map_try_fold<(mz_catalog::memory::objects::StateUpdateKind, mz_repr::timestamp::Timestamp, mz_ore::overflowing::Overflowing<i64>), mz_catalog::memory::objects::StateUpdate, alloc::vec::in_place_drop::InPlaceDrop<mz_catalog::memory::objects::StateUpdate>, core::result::Result<alloc::vec::in_place_drop::InPlaceDrop<mz_catalog::memory::objects::StateUpdate>, !>, <mz_adapter::catalog::state::CatalogState>::consolidate_updates::{closure#1}, alloc::vec::in_place_collect::write_in_place_with_drop<mz_catalog::memory::objects::StateUpdate>::{closure#0}>::{closure#0}, core::result::Result<alloc::vec::in_place_drop::InPlaceDrop<mz_catalog::memory::objects::StateUpdate>, !>>
  11: <core::iter::adapters::map::Map<alloc::vec::into_iter::IntoIter<(mz_catalog::memory::objects::StateUpdateKind, mz_repr::timestamp::Timestamp, mz_ore::overflowing::Overflowing<i64>)>, <mz_adapter::catalog::state::CatalogState>::consolidate_updates::{closure#1}> as core::iter::traits::iterator::Iterator>::try_fold::<alloc::vec::in_place_drop::InPlaceDrop<mz_catalog::memory::objects::StateUpdate>, alloc::vec::in_place_collect::write_in_place_with_drop<mz_catalog::memory::objects::StateUpdate>::{closure#0}, core::result::Result<alloc::vec::in_place_drop::InPlaceDrop<mz_catalog::memory::objects::StateUpdate>, !>>
  12: <core::iter::adapters::map::Map<alloc::vec::into_iter::IntoIter<(mz_catalog::memory::objects::StateUpdateKind, mz_repr::timestamp::Timestamp, mz_ore::overflowing::Overflowing<i64>)>, <mz_adapter::catalog::state::CatalogState>::consolidate_updates::{closure#1}> as alloc::vec::in_place_collect::SpecInPlaceCollect<mz_catalog::memory::objects::StateUpdate, core::iter::adapters::map::Map<alloc::vec::into_iter::IntoIter<(mz_catalog::memory::objects::StateUpdateKind, mz_repr::timestamp::Timestamp, mz_ore::overflowing::Overflowing<i64>)>, <mz_adapter::catalog::state::CatalogState>::consolidate_updates::{closure#1}>>>::collect_in_place
  13: alloc::vec::in_place_collect::from_iter_in_place::<core::iter::adapters::map::Map<alloc::vec::into_iter::IntoIter<(mz_catalog::memory::objects::StateUpdateKind, mz_repr::timestamp::Timestamp, mz_ore::overflowing::Overflowing<i64>)>, <mz_adapter::catalog::state::CatalogState>::consolidate_updates::{closure#1}>, mz_catalog::memory::objects::StateUpdate>
  14: <alloc::vec::Vec<mz_catalog::memory::objects::StateUpdate> as alloc::vec::spec_from_iter::SpecFromIter<mz_catalog::memory::objects::StateUpdate, core::iter::adapters::map::Map<alloc::vec::into_iter::IntoIter<(mz_catalog::memory::objects::StateUpdateKind, mz_repr::timestamp::Timestamp, mz_ore::overflowing::Overflowing<i64>)>, <mz_adapter::catalog::state::CatalogState>::consolidate_updates::{closure#1}>>>::from_iter
  15: <alloc::vec::Vec<mz_catalog::memory::objects::StateUpdate> as core::iter::traits::collect::FromIterator<mz_catalog::memory::objects::StateUpdate>>::from_iter::<core::iter::adapters::map::Map<alloc::vec::into_iter::IntoIter<(mz_catalog::memory::objects::StateUpdateKind, mz_repr::timestamp::Timestamp, mz_ore::overflowing::Overflowing<i64>)>, <mz_adapter::catalog::state::CatalogState>::consolidate_updates::{closure#1}>>
  16: <core::iter::adapters::map::Map<alloc::vec::into_iter::IntoIter<(mz_catalog::memory::objects::StateUpdateKind, mz_repr::timestamp::Timestamp, mz_ore::overflowing::Overflowing<i64>)>, <mz_adapter::catalog::state::CatalogState>::consolidate_updates::{closure#1}> as core::iter::traits::iterator::Iterator>::collect::<alloc::vec::Vec<mz_catalog::memory::objects::StateUpdate>>
  17: <core::iter::adapters::map::Map<alloc::vec::into_iter::IntoIter<(mz_catalog::memory::objects::StateUpdateKind, mz_repr::timestamp::Timestamp, mz_ore::overflowing::Overflowing<i64>)>, <mz_adapter::catalog::state::CatalogState>::consolidate_updates::{closure#1}> as itertools::Itertools>::collect_vec
  18: <mz_adapter::catalog::state::CatalogState>::consolidate_updates
  19: <mz_adapter::catalog::state::CatalogState>::apply_updates::{closure#0}::{closure#0}
  20: <mz_adapter::catalog::state::CatalogState>::apply_updates::{closure#0}
  21: <mz_adapter::catalog::Catalog>::transact_inner::{closure#0}::{closure#0}
  22: <mz_adapter::catalog::Catalog>::transact_inner::{closure#0}
  23: <mz_adapter::catalog::Catalog>::transact::{closure#0}::{closure#0}
  24: <mz_adapter::catalog::Catalog>::transact::{closure#0}
  25: <mz_adapter::coord::Coordinator>::catalog_transact_inner::{closure#0}::{closure#0}
  26: <mz_adapter::coord::Coordinator>::catalog_transact_inner::{closure#0}
  27: <mz_adapter::coord::Coordinator>::catalog_transact_with_side_effects::<<mz_adapter::coord::Coordinator>::sequence_alter_schema_rename::{closure#0}::{closure#0}::{closure#0}>::{closure#0}::{closure#0}
  28: <mz_adapter::coord::Coordinator>::catalog_transact_with_side_effects::<<mz_adapter::coord::Coordinator>::sequence_alter_schema_rename::{closure#0}::{closure#0}::{closure#0}>::{closure#0}
  29: <mz_adapter::coord::Coordinator>::catalog_transact_with_ddl_transaction::<<mz_adapter::coord::Coordinator>::sequence_alter_schema_rename::{closure#0}::{closure#0}::{closure#0}>::{closure#0}::{closure#0}
  30: <mz_adapter::coord::Coordinator>::catalog_transact_with_ddl_transaction::<<mz_adapter::coord::Coordinator>::sequence_alter_schema_rename::{closure#0}::{closure#0}::{closure#0}>::{closure#0}
  31: <mz_adapter::coord::Coordinator>::sequence_alter_schema_rename::{closure#0}::{closure#0}
  32: <mz_adapter::coord::Coordinator>::sequence_alter_schema_rename::{closure#0}
  33: <mz_adapter::coord::Coordinator>::sequence_plan::{closure#0}
  34: <tracing::instrument::Instrumented<<mz_adapter::coord::Coordinator>::sequence_plan::{closure#0}> as core::future::future::Future>::poll
  35: <core::pin::Pin<alloc::boxed::Box<dyn core::future::future::Future<Output = ()>>> as core::future::future::Future>::poll
  36: <mz_adapter::coord::Coordinator>::handle_execute_inner::{closure#0}::{closure#0}
  37: <mz_adapter::coord::Coordinator>::handle_execute_inner::{closure#0}
  38: <mz_adapter::coord::Coordinator>::handle_execute::{closure#0}::{closure#0}
  39: <mz_adapter::coord::Coordinator>::handle_execute::{closure#0}
  40: <mz_adapter::coord::Coordinator>::handle_command::{closure#0}
  41: <tracing::instrument::Instrumented<<mz_adapter::coord::Coordinator>::handle_command::{closure#0}> as core::future::future::Future>::poll
  42: <core::pin::Pin<alloc::boxed::Box<dyn core::future::future::Future<Output = ()>>> as core::future::future::Future>::poll
  43: <mz_adapter::coord::Coordinator>::message_command::{closure#0}::{closure#0}
  44: <mz_adapter::coord::Coordinator>::message_command::{closure#0}
  45: <tracing::instrument::Instrumented<<mz_adapter::coord::Coordinator>::message_command::{closure#0}> as core::future::future::Future>::poll
  46: <mz_adapter::coord::Coordinator>::handle_message::{closure#0}::{closure#0}
  47: <mz_adapter::coord::Coordinator>::handle_message::{closure#0}
  48: <tracing::instrument::Instrumented<<mz_adapter::coord::Coordinator>::handle_message::{closure#0}> as core::future::future::Future>::poll
  49: <mz_adapter::coord::Coordinator>::serve::{closure#0}
  50: <core::pin::Pin<alloc::boxed::Box<dyn core::future::future::Future<Output = ()>>> as core::future::future::Future>::poll
  51: <tracing::instrument::Instrumented<core::pin::Pin<alloc::boxed::Box<dyn core::future::future::Future<Output = ()>>>> as core::future::future::Future>::poll
  52: <tokio::runtime::park::CachedParkThread>::block_on::<tracing::instrument::Instrumented<core::pin::Pin<alloc::boxed::Box<dyn core::future::future::Future<Output = ()>>>>>::{closure#0}
  53: <tokio::runtime::park::CachedParkThread>::block_on::<tracing::instrument::Instrumented<core::pin::Pin<alloc::boxed::Box<dyn core::future::future::Future<Output = ()>>>>>
  54: <tokio::runtime::context::blocking::BlockingRegionGuard>::block_on::<tracing::instrument::Instrumented<core::pin::Pin<alloc::boxed::Box<dyn core::future::future::Future<Output = ()>>>>>
  55: <tokio::runtime::handle::Handle>::block_on_inner::<core::pin::Pin<alloc::boxed::Box<dyn core::future::future::Future<Output = ()>>>>::{closure#0}
  56: tokio::runtime::context::runtime::enter_runtime::<<tokio::runtime::handle::Handle>::block_on_inner<core::pin::Pin<alloc::boxed::Box<dyn core::future::future::Future<Output = ()>>>>::{closure#0}, ()>
  57: <tokio::runtime::handle::Handle>::block_on_inner::<core::pin::Pin<alloc::boxed::Box<dyn core::future::future::Future<Output = ()>>>>
  58: <tokio::runtime::handle::Handle>::block_on::<core::pin::Pin<alloc::boxed::Box<dyn core::future::future::Future<Output = ()>>>>
  59: mz_adapter::coord::serve::{closure#0}::{closure#4}

Without this PR the test succeeds.
See https://linear.app/materializeinc/issue/SQL-401/thread-coordinator-panicked-at-srcadaptersrccatalogapplyrs21222

ALTER SCHEMA RENAME never updated references to a schema's types. The RenameSchema op only iterated schema.items, so the schema's own types kept stale create_sql that failed to re-parse on restart. The rewrite visitor also never descended into data types, so casts, column types, and nested element types in dependents kept pointing at the old schema name. The consistency checker shared the same blind spot. Iterate types and functions alongside items in RenameSchema and check_items, and rewrite schema-qualified names in data type position. Adds a restart regression test covering views, materialized views, tables, and nested types, plus an in-process sqllogictest over the same object kinds. Also completes the privilege restore in the restart suite's drop_materialize_database workflow, which recreated the database without re-granting the materialize role CREATE on it, so the new test (the first to create a schema) can run after it in a shared catalog. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

ggevay · 2026-07-01T12:47:48Z

Nice catch! My PR made SQL-401 trigger more often. So, now I've pushed a commit that fixes SQL-401 both in its original form that is in its issue description and in the form that got triggered by this PR.

Nightly: https://buildkite.com/materialize/nightly/builds/16931

The rename loop's dedup guard only tracked persistent items (via items_to_update). Temporary items go to temporary_item_updates with no dedup, so a temp object that references two objects in the renamed schema was enqueued once per reference: two identical retractions and two identical additions. Catalog apply consolidates by kind, collapsing them to a diff of +/-2 and tripping the "diff other than -1 or 1" assertion, panicking the coordinator. Use a single seen set covering both persistent and temporary items so each is processed exactly once. Adds sqllogictest regression cases for a temp object depending on two types and on two persistent items in a renamed schema. Fixes SQL-401 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

ggevay added the A-ADAPTER Topics related to the ADAPTER layer label Jun 30, 2026

ggevay marked this pull request as ready for review June 30, 2026 16:15

ggevay requested a review from a team as a code owner June 30, 2026 16:15

ggevay force-pushed the gabor/sql-265-alter-schema-rename-types-functions branch from ed1153b to 0532910 Compare June 30, 2026 18:09

def- reviewed Jul 1, 2026

View reviewed changes

ggevay force-pushed the gabor/sql-265-alter-schema-rename-types-functions branch from 0532910 to 9536651 Compare July 1, 2026 12:47

ggevay force-pushed the gabor/sql-265-alter-schema-rename-types-functions branch from 9536651 to a975b64 Compare July 1, 2026 13:16

ggevay mentioned this pull request Jul 1, 2026

adapter: order temp item updates after persistent ones in catalog apply #37406

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

adapter: handle types and functions in ALTER SCHEMA RENAME#37364

adapter: handle types and functions in ALTER SCHEMA RENAME#37364
ggevay wants to merge 2 commits into
MaterializeInc:mainfrom
ggevay:gabor/sql-265-alter-schema-rename-types-functions

ggevay commented Jun 30, 2026 •

edited

Loading

Uh oh!

def- left a comment

Uh oh!

ggevay commented Jul 1, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

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

Uh oh!

Motivation

Changes

Tests

Uh oh!

def- left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ggevay commented Jun 30, 2026 •

edited

Loading

ggevay commented Jul 1, 2026 •

edited

Loading