From dda387c5ad66e8c5d8f3202e5aca94d470979c0e Mon Sep 17 00:00:00 2001 From: Shubham Mishra Date: Tue, 2 Jun 2026 18:08:12 +0530 Subject: [PATCH 1/8] index source name change migration step --- crates/core/src/db/relational_db.rs | 9 ++ crates/core/src/db/update.rs | 138 +++++++++++++++++- .../locking_tx_datastore/committed_state.rs | 12 ++ .../src/locking_tx_datastore/datastore.rs | 18 +++ .../src/locking_tx_datastore/mut_tx.rs | 40 +++++ .../src/locking_tx_datastore/tx_state.rs | 10 ++ crates/datastore/src/traits.rs | 6 + crates/schema/src/auto_migrate.rs | 41 ++++++ crates/schema/src/auto_migrate/formatter.rs | 4 + .../typescript_index_source_name.rs | 61 +++++++- 10 files changed, 333 insertions(+), 6 deletions(-) diff --git a/crates/core/src/db/relational_db.rs b/crates/core/src/db/relational_db.rs index c145a9b79bd..f239645b047 100644 --- a/crates/core/src/db/relational_db.rs +++ b/crates/core/src/db/relational_db.rs @@ -1014,6 +1014,15 @@ impl RelationalDB { Ok(self.inner.alter_table_primary_key_mut_tx(tx, name, primary_key)?) } + pub(crate) fn alter_index_source_name( + &self, + tx: &mut MutTx, + index_id: IndexId, + source_name: spacetimedb_sats::raw_identifier::RawIdentifier, + ) -> Result<(), DBError> { + Ok(self.inner.alter_index_source_name_mut_tx(tx, index_id, source_name)?) + } + pub(crate) fn alter_table_row_type( &self, tx: &mut MutTx, diff --git a/crates/core/src/db/update.rs b/crates/core/src/db/update.rs index f9ca4c110d9..d53a396859b 100644 --- a/crates/core/src/db/update.rs +++ b/crates/core/src/db/update.rs @@ -209,11 +209,34 @@ fn auto_migrate_database( .indexes .iter() .find(|index| index.index_name[..] == index_name[..]) - .unwrap(); + .ok_or_else(|| anyhow::anyhow!("Index `{index_name}` not found in table `{}`", table_def.name))?; log!(logger, "Dropping index `{}` on table `{}`", index_name, table_def.name); stdb.drop_index(tx, index_schema.index_id)?; } + spacetimedb_schema::auto_migrate::AutoMigrateStep::ChangeIndexSourceName(index_name) => { + let old_table_def = plan.old.stored_in_table_def(index_name).unwrap(); + let new_table_def = plan.new.stored_in_table_def(index_name).unwrap(); + let new_index_def = new_table_def.indexes.get(index_name).unwrap(); + + let table_id = stdb.table_id_from_name_mut(tx, &old_table_def.name)?.unwrap(); + let table_schema = stdb.schema_for_table_mut(tx, table_id)?; + let index_schema = table_schema + .indexes + .iter() + .find(|index| index.index_name[..] == index_name[..]) + .ok_or_else(|| anyhow::anyhow!("Index `{index_name}` not found in table `{}`", old_table_def.name))?; + + log!( + logger, + "Changing index source name for `{}` on table `{}` from `{}` to `{}`", + index_name, + old_table_def.name, + index_schema.alias.as_deref().unwrap_or(""), + new_index_def.source_name, + ); + stdb.alter_index_source_name(tx, index_schema.index_id, new_index_def.source_name.clone())?; + } spacetimedb_schema::auto_migrate::AutoMigrateStep::RemoveConstraint(constraint_name) => { let table_def = plan.old.stored_in_table_def(constraint_name).unwrap(); @@ -340,9 +363,13 @@ mod test { host::module_host::create_table_from_def, }; use spacetimedb_datastore::locking_tx_datastore::PendingSchemaChange; - use spacetimedb_lib::db::raw_def::v9::{btree, RawIndexAlgorithm, RawModuleDefV9Builder, TableAccess}; - use spacetimedb_sats::{product, AlgebraicType, AlgebraicType::U64}; - use spacetimedb_schema::{auto_migrate::ponder_migrate, def::ModuleDef}; + use spacetimedb_lib::db::raw_def::{ + v10::{ExplicitNames, RawModuleDefV10Builder}, + v9::{btree, RawIndexAlgorithm, RawModuleDefV9Builder, TableAccess}, + }; + use spacetimedb_sats::{product, raw_identifier::RawIdentifier, AlgebraicType, AlgebraicType::U64, ProductType}; + use spacetimedb_schema::auto_migrate::{ponder_migrate, AutoMigrateStep, MigratePlan}; + use spacetimedb_schema::def::ModuleDef; struct TestLogger; impl UpdateLogger for TestLogger { @@ -424,6 +451,109 @@ mod test { Ok(()) } + #[test] + fn update_db_change_index_source_name_updates_lookup_and_persists() -> anyhow::Result<()> { + let auth_ctx = AuthCtx::for_testing(); + let stdb = TestDB::durable()?; + + fn module_def(table_source_name: &str, index_source_name: &str) -> ModuleDef { + let mut builder = RawModuleDefV10Builder::new(); + builder + .build_table_with_new_type( + table_source_name.to_owned(), + ProductType::from([("id", U64), ("emailAddress", AlgebraicType::String)]), + true, + ) + .with_access(TableAccess::Public) + .with_index(btree(1), index_source_name.to_owned(), "emailAddress") + .finish(); + + if table_source_name != "users" { + let mut explicit_names = ExplicitNames::default(); + explicit_names.insert_table(table_source_name.to_owned(), "users"); + builder.add_explicit_names(explicit_names); + } + + builder + .finish() + .try_into() + .expect("builder should create a valid database definition") + } + + let old_source_name = "users_emailAddress_idx_btree"; + let new_source_name = "appUsers_emailAddress_idx_btree"; + let old = module_def("users", old_source_name); + let new = module_def("appUsers", new_source_name); + + let mut tx = begin_mut_tx(&stdb); + for def in old.tables() { + create_table_from_def(&stdb, &mut tx, &old, def)?; + } + stdb.commit_tx(tx)?; + + let tx = begin_mut_tx(&stdb); + let table_id = stdb + .table_id_from_name_mut(&tx, "users")? + .expect("there should be a table named users"); + let table_schema = stdb.schema_for_table_mut(&tx, table_id)?; + let index_schema = table_schema + .indexes + .first() + .expect("there should be a single index") + .clone(); + let canonical_index_name = index_schema.index_name.to_string(); + let index_id = index_schema.index_id; + assert_eq!(stdb.index_id_from_name_mut(&tx, old_source_name)?, Some(index_id)); + assert_eq!(stdb.index_id_from_name_mut(&tx, new_source_name)?, None); + assert_eq!(stdb.index_id_from_name_mut(&tx, &canonical_index_name)?, Some(index_id)); + drop(tx); + + let MigratePlan::Auto(plan) = ponder_migrate(&old, &new)? else { + panic!("expected automatic migration"); + }; + let index_name = RawIdentifier::new(canonical_index_name.as_str()); + assert!( + plan.steps.contains(&AutoMigrateStep::ChangeIndexSourceName(&index_name)), + "plan steps: {:?}", + plan.steps + ); + assert!( + !plan.steps.contains(&AutoMigrateStep::RemoveIndex(&index_name)), + "plan steps: {:?}", + plan.steps + ); + assert!( + !plan.steps.contains(&AutoMigrateStep::AddIndex(&index_name)), + "plan steps: {:?}", + plan.steps + ); + let mut tx = begin_mut_tx(&stdb); + let res = update_database(&stdb, &mut tx, auth_ctx, MigratePlan::Auto(plan), &TestLogger)?; + assert!(matches!(res, UpdateResult::Success)); + + assert_eq!(stdb.index_id_from_name_mut(&tx, old_source_name)?, None); + assert_eq!(stdb.index_id_from_name_mut(&tx, new_source_name)?, Some(index_id)); + assert_eq!(stdb.index_id_from_name_mut(&tx, &canonical_index_name)?, Some(index_id)); + assert!( + tx.pending_schema_changes().iter().any(|change| matches!( + change, + PendingSchemaChange::IndexAlterSourceName(tid, iid, Some(old_alias)) + if *tid == table_id && *iid == index_id && old_alias.as_ref() == old_source_name + )), + "pending schema changes: {:?}", + tx.pending_schema_changes() + ); + stdb.commit_tx(tx)?; + + let stdb = stdb.reopen()?; + let tx = begin_mut_tx(&stdb); + assert_eq!(stdb.index_id_from_name_mut(&tx, old_source_name)?, None); + assert_eq!(stdb.index_id_from_name_mut(&tx, new_source_name)?, Some(index_id)); + assert_eq!(stdb.index_id_from_name_mut(&tx, &canonical_index_name)?, Some(index_id)); + + Ok(()) + } + /// Regression test for #3934: removing a primary key annotation and then /// re-publishing causes "Primary key mismatch" on the NEXT publish. #[test] diff --git a/crates/datastore/src/locking_tx_datastore/committed_state.rs b/crates/datastore/src/locking_tx_datastore/committed_state.rs index c479be085d9..4086bc93ec2 100644 --- a/crates/datastore/src/locking_tx_datastore/committed_state.rs +++ b/crates/datastore/src/locking_tx_datastore/committed_state.rs @@ -707,6 +707,18 @@ impl CommittedState { table.with_mut_schema(|s| s.remove_index(index_id)); self.index_id_map.remove(&index_id); } + // An index alias/source-name changed. Change it back. + IndexAlterSourceName(table_id, index_id, old_alias) => { + let table = self.tables.get_mut(&table_id)?; + let mut index_schema = table + .get_schema() + .indexes + .iter() + .find(|x| x.index_id == index_id)? + .clone(); + index_schema.alias = old_alias; + table.with_mut_schema(|s| s.update_index(index_schema)); + } // A table was removed. Add it back. TableRemoved(table_id, table) => { let is_view_table = table.schema.is_view(); diff --git a/crates/datastore/src/locking_tx_datastore/datastore.rs b/crates/datastore/src/locking_tx_datastore/datastore.rs index e9d67103b16..667990b3e75 100644 --- a/crates/datastore/src/locking_tx_datastore/datastore.rs +++ b/crates/datastore/src/locking_tx_datastore/datastore.rs @@ -294,6 +294,15 @@ impl Locking { tx.alter_table_primary_key(table_id, primary_key) } + pub fn alter_index_source_name_mut_tx( + &self, + tx: &mut MutTxId, + index_id: IndexId, + source_name: spacetimedb_sats::raw_identifier::RawIdentifier, + ) -> Result<()> { + tx.alter_index_source_name(index_id, source_name) + } + pub fn alter_table_row_type_mut_tx( &self, tx: &mut MutTxId, @@ -527,6 +536,15 @@ impl MutTxDatastore for Locking { tx.drop_index(index_id) } + fn alter_index_source_name_mut_tx( + &self, + tx: &mut Self::MutTx, + index_id: IndexId, + source_name: spacetimedb_sats::raw_identifier::RawIdentifier, + ) -> Result<()> { + tx.alter_index_source_name(index_id, source_name) + } + fn index_id_from_name_mut_tx(&self, tx: &Self::MutTx, index_name: &str) -> Result> { tx.index_id_from_name_or_alias(index_name) } diff --git a/crates/datastore/src/locking_tx_datastore/mut_tx.rs b/crates/datastore/src/locking_tx_datastore/mut_tx.rs index 3a36331dc4d..90e00e2f0c4 100644 --- a/crates/datastore/src/locking_tx_datastore/mut_tx.rs +++ b/crates/datastore/src/locking_tx_datastore/mut_tx.rs @@ -1163,6 +1163,46 @@ impl MutTxId { Ok(()) } + /// Change the runtime source-name alias of the index identified by `index_id`. + pub(crate) fn alter_index_source_name( + &mut self, + index_id: IndexId, + source_name: spacetimedb_sats::raw_identifier::RawIdentifier, + ) -> Result<()> { + let st_index_ref = self + .iter_by_col_eq(ST_INDEX_ID, StIndexFields::IndexId, &index_id.into())? + .next() + .ok_or_else(|| TableError::IdNotFound(SystemTable::st_index, index_id.into()))?; + let st_index_row = StIndexRow::try_from(st_index_ref)?; + let table_id = st_index_row.table_id; + + let old_alias = self + .find_st_index_accessor_row_by_index_name(st_index_row.index_name.as_ref())? + .map(|row| row.accessor_name); + + if old_alias.as_ref() == Some(&source_name) { + return Ok(()); + } + + let ((tx_table, ..), (commit_table, ..)) = self.get_or_create_insert_table_mut(table_id)?; + let mut index_schema = tx_table + .get_schema() + .indexes + .iter() + .find(|index| index.index_id == index_id) + .cloned() + .ok_or_else(|| TableError::IdNotFound(SystemTable::st_index, index_id.into()))?; + index_schema.alias = Some(source_name.clone()); + tx_table.with_mut_schema_and_clone(commit_table, |s| s.update_index(index_schema)); + + self.drop_st_index_accessor(&st_index_row.index_name)?; + self.insert_st_index_accessor(&st_index_row.index_name, Some(&source_name))?; + + self.push_schema_change(PendingSchemaChange::IndexAlterSourceName(table_id, index_id, old_alias)); + + Ok(()) + } + /// Change the row type of the table identified by `table_id`. /// /// In practice, this should not error, diff --git a/crates/datastore/src/locking_tx_datastore/tx_state.rs b/crates/datastore/src/locking_tx_datastore/tx_state.rs index 4e712bcf684..4b56aa1dfca 100644 --- a/crates/datastore/src/locking_tx_datastore/tx_state.rs +++ b/crates/datastore/src/locking_tx_datastore/tx_state.rs @@ -111,6 +111,13 @@ pub enum PendingSchemaChange { /// If adding this index caused the pointer map to be removed, /// it will be present here. IndexAdded(TableId, IndexId, Option), + /// The source-name alias of the index with [`IndexId`] changed. + /// The old alias is stored for rollback. + IndexAlterSourceName( + TableId, + IndexId, + Option, + ), /// The [`Table`] with [`TableId`] was removed. TableRemoved(TableId, Table), /// The table with [`TableId`] was added. @@ -145,6 +152,9 @@ impl MemoryUsage for PendingSchemaChange { Self::IndexAdded(table_id, index_id, pointer_map) => { table_id.heap_usage() + index_id.heap_usage() + pointer_map.heap_usage() } + Self::IndexAlterSourceName(table_id, index_id, alias) => { + table_id.heap_usage() + index_id.heap_usage() + alias.heap_usage() + } Self::TableRemoved(table_id, table) => table_id.heap_usage() + table.heap_usage(), Self::TableAdded(table_id) => table_id.heap_usage(), Self::TableAlterAccess(table_id, st_access) => table_id.heap_usage() + st_access.heap_usage(), diff --git a/crates/datastore/src/traits.rs b/crates/datastore/src/traits.rs index e1b99825ae2..07a1d633124 100644 --- a/crates/datastore/src/traits.rs +++ b/crates/datastore/src/traits.rs @@ -627,6 +627,12 @@ pub trait MutTxDatastore: TxDatastore + MutTx { fn create_index_mut_tx(&self, tx: &mut Self::MutTx, index_schema: IndexSchema, is_unique: bool) -> Result; fn drop_index_mut_tx(&self, tx: &mut Self::MutTx, index_id: IndexId) -> Result<()>; + fn alter_index_source_name_mut_tx( + &self, + tx: &mut Self::MutTx, + index_id: IndexId, + source_name: spacetimedb_sats::raw_identifier::RawIdentifier, + ) -> Result<()>; fn index_id_from_name_mut_tx(&self, tx: &Self::MutTx, index_name: &str) -> super::Result>; // TODO: Index data diff --git a/crates/schema/src/auto_migrate.rs b/crates/schema/src/auto_migrate.rs index f2fba813fa8..bb69c4bea55 100644 --- a/crates/schema/src/auto_migrate.rs +++ b/crates/schema/src/auto_migrate.rs @@ -284,6 +284,9 @@ pub enum AutoMigrateStep<'def> { /// no `ChangeColumns` steps will be, for the same table. AddColumns(::Key<'def>), + /// Change the runtime source-name alias of an existing index. + ChangeIndexSourceName(::Key<'def>), + /// Add a table, including all indexes, constraints, and sequences. /// There will NOT be separate steps in the plan for adding indexes, constraints, and sequences. AddTable(::Key<'def>), @@ -981,6 +984,8 @@ fn auto_migrate_indexes( if old.algorithm != new.algorithm { plan.steps.push(AutoMigrateStep::RemoveIndex(old.key())); plan.steps.push(AutoMigrateStep::AddIndex(old.key())); + } else if old.source_name != new.source_name { + plan.steps.push(AutoMigrateStep::ChangeIndexSourceName(old.key())); } Ok(()) } @@ -2039,6 +2044,42 @@ mod tests { ); } + #[test] + fn migrate_index_with_changed_source_name() { + fn module_def(source_name: &str) -> ModuleDef { + create_module_def_v10(|builder| { + builder + .build_table_with_new_type( + "FruitBasket", + ProductType::from([("basket_id", AlgebraicType::U64), ("fruit_name", AlgebraicType::String)]), + true, + ) + .with_index(btree([0, 1]), source_name.to_owned(), "fruitNameIndex") + .finish(); + }) + } + + let old_def = module_def("OldBasketLookup"); + let new_def = module_def("NewBasketLookup"); + let index_name = RawIdentifier::new("fruit_basket_basket_id_fruit_name_idx_btree"); + + let plan = ponder_auto_migrate(&old_def, &new_def).expect("auto migration should succeed"); + let steps = &plan.steps[..]; + + assert!( + steps.contains(&AutoMigrateStep::ChangeIndexSourceName(&index_name)), + "steps: {steps:?}" + ); + assert!( + !steps.contains(&AutoMigrateStep::RemoveIndex(&index_name)), + "steps: {steps:?}" + ); + assert!( + !steps.contains(&AutoMigrateStep::AddIndex(&index_name)), + "steps: {steps:?}" + ); + } + #[test] fn migrate_view_disconnect_clients() { struct TestCase { diff --git a/crates/schema/src/auto_migrate/formatter.rs b/crates/schema/src/auto_migrate/formatter.rs index 1f7377209bf..7296c89df44 100644 --- a/crates/schema/src/auto_migrate/formatter.rs +++ b/crates/schema/src/auto_migrate/formatter.rs @@ -59,6 +59,10 @@ fn format_step( let index_info = extract_index_info(*index, plan.old)?; f.format_index(&index_info, Action::Removed) } + AutoMigrateStep::ChangeIndexSourceName(index) => { + let index_info = extract_index_info(*index, plan.new)?; + f.format_index(&index_info, Action::Changed) + } AutoMigrateStep::RemoveConstraint(constraint) => { let constraint_info = extract_constraint_info(*constraint, plan.old)?; f.format_constraint(&constraint_info, Action::Removed) diff --git a/crates/smoketests/tests/smoketests/typescript_index_source_name.rs b/crates/smoketests/tests/smoketests/typescript_index_source_name.rs index 8c550f81aeb..7c848cbb298 100644 --- a/crates/smoketests/tests/smoketests/typescript_index_source_name.rs +++ b/crates/smoketests/tests/smoketests/typescript_index_source_name.rs @@ -1,6 +1,6 @@ use spacetimedb_smoketests::{random_string, require_local_server, require_pnpm, Smoketest}; -const TYPESCRIPT_MODULE_WITHOUT_NEW_COLUMNS: &str = r#"import { schema, table, t } from "spacetimedb/server"; +const TYPESCRIPT_MODULE_V1: &str = r#"import { schema, table, t } from "spacetimedb/server"; const AppUsers = table( { name: "users", public: false }, @@ -72,6 +72,34 @@ export const find_users_by_active_status = spacetimedb.reducer( ); "#; +const TYPESCRIPT_MODULE_V2_RENAMED_ACCESSOR: &str = r#"import { schema, table, t } from "spacetimedb/server"; + +const renamedUsers = table( + { name: "users", public: false }, + { + id: t.u64().primaryKey().autoInc(), + name: t.string(), + emailAddress: t.string().index("btree"), + }, +); + +const spacetimedb = schema({ + renamedUsers, +}); +export default spacetimedb; + +export const find_user_by_email = spacetimedb.reducer( + { emailAddress: t.string() }, + (ctx, { emailAddress }) => { + let count = 0; + for (const _row of ctx.db.renamedUsers.emailAddress.filter(emailAddress)) { + count += 1; + } + console.info(`matched ${count}`); + }, +); +"#; + #[test] fn test_typescript_add_optional_columns() { require_pnpm!(); @@ -84,7 +112,7 @@ fn test_typescript_add_optional_columns() { .publish_typescript_module_source( "typescript-add-optional-columns-v1", &module_name, - TYPESCRIPT_MODULE_WITHOUT_NEW_COLUMNS, + TYPESCRIPT_MODULE_V1, ) .unwrap(); @@ -103,3 +131,32 @@ fn test_typescript_add_optional_columns() { test.call("find_user_by_email", &["alice@example.com"]).unwrap(); test.call("find_users_by_active_status", &["false"]).unwrap(); } + +#[test] +fn test_typescript_change_index_source_name() { + require_pnpm!(); + require_local_server!(); + + let mut test = Smoketest::builder().autopublish(false).build(); + let module_name = format!("typescript-change-source-name-{}", random_string()); + + let database_identity = test + .publish_typescript_module_source( + "typescript-change-source-name-v1", + &module_name, + TYPESCRIPT_MODULE_V1, + ) + .unwrap(); + + test.call("insert_user", &["Alice", "alice@example.com"]).unwrap(); + + test.publish_typescript_module_source_clear( + "typescript-change-source-name-v2", + &database_identity, + TYPESCRIPT_MODULE_V2_RENAMED_ACCESSOR, + false, + ) + .unwrap(); + + test.call("find_user_by_email", &["alice@example.com"]).unwrap(); +} From c08b186d94b992ced5e8b7427e992d1812d68fb5 Mon Sep 17 00:00:00 2001 From: Shubham Mishra Date: Thu, 4 Jun 2026 15:11:58 +0530 Subject: [PATCH 2/8] Add failing tests for table/column accessor name auto-migration These tests verify that changing a table or column accessor_name in a module schema triggers ChangeTableAccessorName and ChangeColumnAccessorName steps. They currently fail because the auto-migration logic is not yet implemented. --- crates/core/src/db/update.rs | 4 + crates/schema/src/auto_migrate.rs | 84 +++++++++++++++++++++ crates/schema/src/auto_migrate/formatter.rs | 1 + crates/schema/tests/ensure_same_schema.rs | 3 + 4 files changed, 92 insertions(+) diff --git a/crates/core/src/db/update.rs b/crates/core/src/db/update.rs index d53a396859b..ef1c380f18b 100644 --- a/crates/core/src/db/update.rs +++ b/crates/core/src/db/update.rs @@ -214,6 +214,10 @@ fn auto_migrate_database( log!(logger, "Dropping index `{}` on table `{}`", index_name, table_def.name); stdb.drop_index(tx, index_schema.index_id)?; } + spacetimedb_schema::auto_migrate::AutoMigrateStep::ChangeTableAccessorName(_) + | spacetimedb_schema::auto_migrate::AutoMigrateStep::ChangeColumnAccessorName(_, _) => { + unreachable!("accessor name changes are not handled in test-only mode") + } spacetimedb_schema::auto_migrate::AutoMigrateStep::ChangeIndexSourceName(index_name) => { let old_table_def = plan.old.stored_in_table_def(index_name).unwrap(); let new_table_def = plan.new.stored_in_table_def(index_name).unwrap(); diff --git a/crates/schema/src/auto_migrate.rs b/crates/schema/src/auto_migrate.rs index bb69c4bea55..75bdde3164e 100644 --- a/crates/schema/src/auto_migrate.rs +++ b/crates/schema/src/auto_migrate.rs @@ -287,6 +287,14 @@ pub enum AutoMigrateStep<'def> { /// Change the runtime source-name alias of an existing index. ChangeIndexSourceName(::Key<'def>), + /// Change the runtime accessor name alias of an existing table. + ChangeTableAccessorName(::Key<'def>), + /// Change the runtime accessor name alias of an existing column. + ChangeColumnAccessorName( + ::Key<'def>, + &'def Identifier, + ), + /// Add a table, including all indexes, constraints, and sequences. /// There will NOT be separate steps in the plan for adding indexes, constraints, and sequences. AddTable(::Key<'def>), @@ -2080,6 +2088,82 @@ mod tests { ); } + #[test] + fn migrate_table_with_changed_accessor_name() { + let old_def = create_module_def_v10(|builder| { + builder + .build_table_with_new_type( + "my_table", + ProductType::from([("id", AlgebraicType::U64)]), + true, + ) + .finish(); + }); + + let new_def = create_module_def_v10(|builder| { + builder + .build_table_with_new_type( + "renamed_table", + ProductType::from([("id", AlgebraicType::U64)]), + true, + ) + .finish(); + let mut explicit = ExplicitNames::default(); + explicit.insert_table("renamed_table", "my_table"); + builder.add_explicit_names(explicit); + }); + + let plan = ponder_auto_migrate(&old_def, &new_def).expect("auto migration should succeed"); + let steps = &plan.steps[..]; + let table_name = expect_identifier("my_table"); + + assert!( + steps.contains(&AutoMigrateStep::ChangeTableAccessorName(&table_name)), + "steps: {steps:?}" + ); + assert!( + !steps.contains(&AutoMigrateStep::RemoveTable(&table_name)), + "steps: {steps:?}" + ); + assert!( + !steps.contains(&AutoMigrateStep::AddTable(&table_name)), + "steps: {steps:?}" + ); + } + + #[test] + fn migrate_column_with_changed_accessor_name() { + let old_def = create_module_def_v10(|builder| { + builder + .build_table_with_new_type( + "my_table", + ProductType::from([("my_field", AlgebraicType::U64)]), + true, + ) + .finish(); + }); + + let new_def = create_module_def_v10(|builder| { + builder + .build_table_with_new_type( + "my_table", + ProductType::from([("myField", AlgebraicType::U64)]), + true, + ) + .finish(); + }); + + let plan = ponder_auto_migrate(&old_def, &new_def).expect("auto migration should succeed"); + let steps = &plan.steps[..]; + let table_name = expect_identifier("my_table"); + let col_name = expect_identifier("my_field"); + + assert!( + steps.contains(&AutoMigrateStep::ChangeColumnAccessorName(&table_name, &col_name)), + "steps: {steps:?}" + ); + } + #[test] fn migrate_view_disconnect_clients() { struct TestCase { diff --git a/crates/schema/src/auto_migrate/formatter.rs b/crates/schema/src/auto_migrate/formatter.rs index 7296c89df44..9fb01cb3464 100644 --- a/crates/schema/src/auto_migrate/formatter.rs +++ b/crates/schema/src/auto_migrate/formatter.rs @@ -112,6 +112,7 @@ fn format_step( let new_columns = extract_new_columns(*table, plan)?; f.format_add_columns(&new_columns) } + AutoMigrateStep::ChangeTableAccessorName(_) | AutoMigrateStep::ChangeColumnAccessorName(_, _) => Ok(()), AutoMigrateStep::DisconnectAllUsers => f.format_disconnect_warning(), }?; diff --git a/crates/schema/tests/ensure_same_schema.rs b/crates/schema/tests/ensure_same_schema.rs index 7e5cb28f626..306fe2340b2 100644 --- a/crates/schema/tests/ensure_same_schema.rs +++ b/crates/schema/tests/ensure_same_schema.rs @@ -36,6 +36,9 @@ fn assert_identical_modules(module_name_prefix: &str, lang_name: &str, suffix: & | AutoMigrateStep::AddView(_) | AutoMigrateStep::RemoveView(_) | AutoMigrateStep::UpdateView(_) + | AutoMigrateStep::ChangeIndexSourceName(_) + | AutoMigrateStep::ChangeTableAccessorName(_) + | AutoMigrateStep::ChangeColumnAccessorName(_, _) ) }); From f314f543c40a41687498514042012609ec3a69b3 Mon Sep 17 00:00:00 2001 From: Shubham Mishra Date: Thu, 4 Jun 2026 15:19:04 +0530 Subject: [PATCH 3/8] Implement auto-migration for table and column accessor name changes Add ChangeTableAccessorName and ChangeColumnAccessorName step variants and their full execution pipeline: auto-migration planning, datastore mutations (updating st_table_accessor / st_column_accessor system tables and in-memory schema), rollback support, formatter rendering, and step execution handlers. --- crates/core/src/db/relational_db.rs | 19 +++++ crates/core/src/db/update.rs | 40 +++++++++- .../locking_tx_datastore/committed_state.rs | 14 ++++ .../src/locking_tx_datastore/datastore.rs | 38 +++++++++ .../src/locking_tx_datastore/mut_tx.rs | 80 +++++++++++++++++++ .../src/locking_tx_datastore/tx_state.rs | 24 +++++- crates/datastore/src/traits.rs | 13 +++ crates/schema/src/auto_migrate.rs | 8 ++ crates/schema/src/auto_migrate/formatter.rs | 10 ++- .../src/auto_migrate/termcolor_formatter.rs | 16 ++++ 10 files changed, 257 insertions(+), 5 deletions(-) diff --git a/crates/core/src/db/relational_db.rs b/crates/core/src/db/relational_db.rs index f239645b047..63cb24501f6 100644 --- a/crates/core/src/db/relational_db.rs +++ b/crates/core/src/db/relational_db.rs @@ -1023,6 +1023,25 @@ impl RelationalDB { Ok(self.inner.alter_index_source_name_mut_tx(tx, index_id, source_name)?) } + pub(crate) fn alter_table_accessor_name( + &self, + tx: &mut MutTx, + table_id: TableId, + new_alias: spacetimedb_schema::identifier::Identifier, + ) -> Result<(), DBError> { + Ok(self.inner.alter_table_accessor_name_mut_tx(tx, table_id, new_alias)?) + } + + pub(crate) fn alter_column_accessor_name( + &self, + tx: &mut MutTx, + table_id: TableId, + col_id: ColId, + new_alias: spacetimedb_schema::identifier::Identifier, + ) -> Result<(), DBError> { + Ok(self.inner.alter_column_accessor_name_mut_tx(tx, table_id, col_id, new_alias)?) + } + pub(crate) fn alter_table_row_type( &self, tx: &mut MutTx, diff --git a/crates/core/src/db/update.rs b/crates/core/src/db/update.rs index ef1c380f18b..d510da12469 100644 --- a/crates/core/src/db/update.rs +++ b/crates/core/src/db/update.rs @@ -214,9 +214,43 @@ fn auto_migrate_database( log!(logger, "Dropping index `{}` on table `{}`", index_name, table_def.name); stdb.drop_index(tx, index_schema.index_id)?; } - spacetimedb_schema::auto_migrate::AutoMigrateStep::ChangeTableAccessorName(_) - | spacetimedb_schema::auto_migrate::AutoMigrateStep::ChangeColumnAccessorName(_, _) => { - unreachable!("accessor name changes are not handled in test-only mode") + spacetimedb_schema::auto_migrate::AutoMigrateStep::ChangeTableAccessorName(table_name) => { + let new_table_def: &spacetimedb_schema::def::TableDef = plan.new.expect_lookup(table_name); + + let table_id = stdb.table_id_from_name_mut(tx, table_name)?.unwrap(); + + log!( + logger, + "Changing table accessor name for `{}` to `{}`", + table_name, + new_table_def.accessor_name, + ); + stdb.alter_table_accessor_name(tx, table_id, new_table_def.accessor_name.clone())?; + } + spacetimedb_schema::auto_migrate::AutoMigrateStep::ChangeColumnAccessorName(table_name, col_name) => { + let new_table_def: &spacetimedb_schema::def::TableDef = plan.new.expect_lookup(table_name); + let new_col_def = new_table_def + .columns + .iter() + .find(|col| &col.name == col_name) + .ok_or_else(|| anyhow::anyhow!("Column `{col_name}` not found in table `{table_name}`"))?; + + let table_id = stdb.table_id_from_name_mut(tx, table_name)?.unwrap(); + let table_schema = stdb.schema_for_table_mut(tx, table_id)?; + let col_schema = table_schema + .columns + .iter() + .find(|col| &col.col_name == col_name) + .ok_or_else(|| anyhow::anyhow!("Column `{col_name}` not found in table `{table_name}`"))?; + + log!( + logger, + "Changing column accessor name for `{}`.`{}` to `{}`", + table_name, + col_name, + new_col_def.accessor_name, + ); + stdb.alter_column_accessor_name(tx, table_id, col_schema.col_pos, new_col_def.accessor_name.clone())?; } spacetimedb_schema::auto_migrate::AutoMigrateStep::ChangeIndexSourceName(index_name) => { let old_table_def = plan.old.stored_in_table_def(index_name).unwrap(); diff --git a/crates/datastore/src/locking_tx_datastore/committed_state.rs b/crates/datastore/src/locking_tx_datastore/committed_state.rs index 4086bc93ec2..ec24e0d4d6f 100644 --- a/crates/datastore/src/locking_tx_datastore/committed_state.rs +++ b/crates/datastore/src/locking_tx_datastore/committed_state.rs @@ -719,6 +719,20 @@ impl CommittedState { index_schema.alias = old_alias; table.with_mut_schema(|s| s.update_index(index_schema)); } + // A table accessor name alias changed. Change it back. + TableAlterAccessorName(table_id, old_alias) => { + let table = self.tables.get_mut(&table_id)?; + table.with_mut_schema(|s| s.alias = old_alias); + } + // A column accessor name alias changed. Change it back. + ColumnAlterAccessorName(table_id, col_id, old_alias) => { + let table = self.tables.get_mut(&table_id)?; + table.with_mut_schema(|s| { + if let Some(col) = s.columns.iter_mut().find(|x| x.col_pos == col_id) { + col.alias = old_alias; + } + }); + } // A table was removed. Add it back. TableRemoved(table_id, table) => { let is_view_table = table.schema.is_view(); diff --git a/crates/datastore/src/locking_tx_datastore/datastore.rs b/crates/datastore/src/locking_tx_datastore/datastore.rs index 667990b3e75..ad655e20539 100644 --- a/crates/datastore/src/locking_tx_datastore/datastore.rs +++ b/crates/datastore/src/locking_tx_datastore/datastore.rs @@ -303,6 +303,25 @@ impl Locking { tx.alter_index_source_name(index_id, source_name) } + pub fn alter_table_accessor_name_mut_tx( + &self, + tx: &mut MutTxId, + table_id: TableId, + new_alias: spacetimedb_schema::identifier::Identifier, + ) -> Result<()> { + tx.alter_table_accessor_name(table_id, new_alias) + } + + pub fn alter_column_accessor_name_mut_tx( + &self, + tx: &mut MutTxId, + table_id: TableId, + col_id: ColId, + new_alias: spacetimedb_schema::identifier::Identifier, + ) -> Result<()> { + tx.alter_column_accessor_name(table_id, col_id, new_alias) + } + pub fn alter_table_row_type_mut_tx( &self, tx: &mut MutTxId, @@ -545,6 +564,25 @@ impl MutTxDatastore for Locking { tx.alter_index_source_name(index_id, source_name) } + fn alter_table_accessor_name_mut_tx( + &self, + tx: &mut Self::MutTx, + table_id: TableId, + new_alias: spacetimedb_schema::identifier::Identifier, + ) -> Result<()> { + tx.alter_table_accessor_name(table_id, new_alias) + } + + fn alter_column_accessor_name_mut_tx( + &self, + tx: &mut Self::MutTx, + table_id: TableId, + col_id: ColId, + new_alias: spacetimedb_schema::identifier::Identifier, + ) -> Result<()> { + tx.alter_column_accessor_name(table_id, col_id, new_alias) + } + fn index_id_from_name_mut_tx(&self, tx: &Self::MutTx, index_name: &str) -> Result> { tx.index_id_from_name_or_alias(index_name) } diff --git a/crates/datastore/src/locking_tx_datastore/mut_tx.rs b/crates/datastore/src/locking_tx_datastore/mut_tx.rs index 90e00e2f0c4..ed0764b4453 100644 --- a/crates/datastore/src/locking_tx_datastore/mut_tx.rs +++ b/crates/datastore/src/locking_tx_datastore/mut_tx.rs @@ -1203,6 +1203,86 @@ impl MutTxId { Ok(()) } + /// Change the runtime accessor name alias of the table identified by `table_id`. + pub(crate) fn alter_table_accessor_name( + &mut self, + table_id: TableId, + new_alias: Identifier, + ) -> Result<()> { + let table_name = self.find_st_table_row(table_id)?.table_name; + + let old_alias = self + .iter_by_col_eq(ST_TABLE_ACCESSOR_ID, StTableAccessorFields::TableName, &table_name.as_ref().into())? + .next() + .map(|row| StTableAccessorRow::try_from(row).ok()) + .flatten() + .map(|row| row.accessor_name); + + if old_alias.as_ref() == Some(&new_alias) { + return Ok(()); + } + + let ((tx_table, ..), (commit_table, ..)) = self.get_or_create_insert_table_mut(table_id)?; + tx_table.with_mut_schema_and_clone(commit_table, |s| s.alias = Some(new_alias.clone())); + + self.drop_st_table_accessor(&table_name)?; + self.insert_st_table_accessor(&table_name, Some(&new_alias))?; + + self.push_schema_change(PendingSchemaChange::TableAlterAccessorName(table_id, old_alias)); + + Ok(()) + } + + /// Change the runtime accessor name alias of the column identified by `table_id` and `col_id`. + pub(crate) fn alter_column_accessor_name( + &mut self, + table_id: TableId, + col_id: ColId, + new_alias: Identifier, + ) -> Result<()> { + let table_name = self.find_st_table_row(table_id)?.table_name; + + // Read old column info from the current schema + let col_info: Vec<(Identifier, Option, ColId)> = self + .schema_for_table(table_id)? + .columns + .iter() + .map(|c| (c.col_name.clone(), c.alias.clone(), c.col_pos)) + .collect(); + + let old_alias = col_info + .iter() + .find(|(_, _, pos)| *pos == col_id) + .and_then(|(_, alias, _)| alias.clone()); + + if old_alias.as_ref() == Some(&new_alias) { + return Ok(()); + } + + // Update system tables: drop and re-insert all column accessor rows for this table + self.drop_st_column_accessor(&table_name)?; + for (col_name, alias, pos) in &col_info { + let name = if *pos == col_id { + &new_alias + } else { + alias.as_ref().unwrap_or(col_name) + }; + self.insert_st_column_accessor(&table_name, col_name, Some(name))?; + } + + // Update in-memory schema + let ((tx_table, ..), (commit_table, ..)) = self.get_or_create_insert_table_mut(table_id)?; + tx_table.with_mut_schema_and_clone(commit_table, |s| { + if let Some(col) = s.columns.iter_mut().find(|c| c.col_pos == col_id) { + col.alias = Some(new_alias); + } + }); + + self.push_schema_change(PendingSchemaChange::ColumnAlterAccessorName(table_id, col_id, old_alias)); + + Ok(()) + } + /// Change the row type of the table identified by `table_id`. /// /// In practice, this should not error, diff --git a/crates/datastore/src/locking_tx_datastore/tx_state.rs b/crates/datastore/src/locking_tx_datastore/tx_state.rs index 4b56aa1dfca..6b4dea6ce9d 100644 --- a/crates/datastore/src/locking_tx_datastore/tx_state.rs +++ b/crates/datastore/src/locking_tx_datastore/tx_state.rs @@ -1,8 +1,9 @@ use super::{delete_table::DeleteTable, sequence::Sequence}; use spacetimedb_data_structures::map::IntMap; use spacetimedb_lib::db::auth::StAccess; -use spacetimedb_primitives::{ColList, ConstraintId, IndexId, SequenceId, TableId}; +use spacetimedb_primitives::{ColId, ColList, ConstraintId, IndexId, SequenceId, TableId}; use spacetimedb_sats::memory_usage::MemoryUsage; +use spacetimedb_schema::identifier::Identifier; use spacetimedb_schema::schema::{ColumnSchema, ConstraintSchema, IndexSchema, SequenceSchema}; use spacetimedb_table::{ blob_store::{BlobStore, HashMapBlobStore}, @@ -118,6 +119,19 @@ pub enum PendingSchemaChange { IndexId, Option, ), + /// The accessor name alias of the table with [`TableId`] changed. + /// The old alias is stored for rollback. + TableAlterAccessorName( + TableId, + Option, + ), + /// The accessor name alias of the column with [`ColId`] in the table with [`TableId`] changed. + /// The old alias is stored for rollback. + ColumnAlterAccessorName( + TableId, + ColId, + Option, + ), /// The [`Table`] with [`TableId`] was removed. TableRemoved(TableId, Table), /// The table with [`TableId`] was added. @@ -168,6 +182,14 @@ impl MemoryUsage for PendingSchemaChange { table_id.heap_usage() + sequence.heap_usage() + sequence_schema.heap_usage() } Self::SequenceAdded(table_id, sequence_id) => table_id.heap_usage() + sequence_id.heap_usage(), + Self::TableAlterAccessorName(table_id, alias) => { + table_id.heap_usage() + alias.as_ref().map(|a| a.as_raw().heap_usage()).unwrap_or(0) + } + Self::ColumnAlterAccessorName(table_id, col_id, alias) => { + table_id.heap_usage() + + col_id.heap_usage() + + alias.as_ref().map(|a| a.as_raw().heap_usage()).unwrap_or(0) + } } } } diff --git a/crates/datastore/src/traits.rs b/crates/datastore/src/traits.rs index 07a1d633124..207a9429d28 100644 --- a/crates/datastore/src/traits.rs +++ b/crates/datastore/src/traits.rs @@ -633,6 +633,19 @@ pub trait MutTxDatastore: TxDatastore + MutTx { index_id: IndexId, source_name: spacetimedb_sats::raw_identifier::RawIdentifier, ) -> Result<()>; + fn alter_table_accessor_name_mut_tx( + &self, + tx: &mut Self::MutTx, + table_id: TableId, + new_alias: spacetimedb_schema::identifier::Identifier, + ) -> Result<()>; + fn alter_column_accessor_name_mut_tx( + &self, + tx: &mut Self::MutTx, + table_id: TableId, + col_id: ColId, + new_alias: spacetimedb_schema::identifier::Identifier, + ) -> Result<()>; fn index_id_from_name_mut_tx(&self, tx: &Self::MutTx, index_name: &str) -> super::Result>; // TODO: Index data diff --git a/crates/schema/src/auto_migrate.rs b/crates/schema/src/auto_migrate.rs index 75bdde3164e..e707aea5426 100644 --- a/crates/schema/src/auto_migrate.rs +++ b/crates/schema/src/auto_migrate.rs @@ -699,6 +699,9 @@ fn auto_migrate_table<'def>(plan: &mut AutoMigratePlan<'def>, old: &'def TableDe if old.primary_key != new.primary_key { plan.steps.push(AutoMigrateStep::ChangePrimaryKey(key)); } + if old.accessor_name != new.accessor_name { + plan.steps.push(AutoMigrateStep::ChangeTableAccessorName(key)); + } if old.schedule != new.schedule { // Note: this handles the case where there's an altered ScheduleDef for some reason. if let Some(old_schedule) = old.schedule.as_ref() { @@ -762,6 +765,11 @@ fn auto_migrate_table<'def>(plan: &mut AutoMigratePlan<'def>, old: &'def TableDe .into()) }; + if old.accessor_name != new.accessor_name { + plan.steps + .push(AutoMigrateStep::ChangeColumnAccessorName(key, &old.name)); + } + (types_ok, positions_ok) .combine_errors() // row_type_changed, column_added diff --git a/crates/schema/src/auto_migrate/formatter.rs b/crates/schema/src/auto_migrate/formatter.rs index 9fb01cb3464..9dd6c637bda 100644 --- a/crates/schema/src/auto_migrate/formatter.rs +++ b/crates/schema/src/auto_migrate/formatter.rs @@ -63,6 +63,13 @@ fn format_step( let index_info = extract_index_info(*index, plan.new)?; f.format_index(&index_info, Action::Changed) } + AutoMigrateStep::ChangeTableAccessorName(table) => { + let table_info = extract_table_info(*table, plan)?; + f.format_change_table_accessor_name(table_info.name.as_ref()) + } + AutoMigrateStep::ChangeColumnAccessorName(table, col) => { + f.format_change_column_accessor_name(table, col) + } AutoMigrateStep::RemoveConstraint(constraint) => { let constraint_info = extract_constraint_info(*constraint, plan.old)?; f.format_constraint(&constraint_info, Action::Removed) @@ -112,7 +119,6 @@ fn format_step( let new_columns = extract_new_columns(*table, plan)?; f.format_add_columns(&new_columns) } - AutoMigrateStep::ChangeTableAccessorName(_) | AutoMigrateStep::ChangeColumnAccessorName(_, _) => Ok(()), AutoMigrateStep::DisconnectAllUsers => f.format_disconnect_warning(), }?; @@ -171,6 +177,8 @@ pub trait MigrationFormatter { fn format_rls(&mut self, rls_info: &RlsInfo, action: Action) -> io::Result<()>; fn format_change_columns(&mut self, column_changes: &ColumnChanges) -> io::Result<()>; fn format_add_columns(&mut self, new_columns: &NewColumns) -> io::Result<()>; + fn format_change_table_accessor_name(&mut self, table_name: &str) -> io::Result<()>; + fn format_change_column_accessor_name(&mut self, table_name: &str, col_name: &str) -> io::Result<()>; fn format_disconnect_warning(&mut self) -> io::Result<()>; } diff --git a/crates/schema/src/auto_migrate/termcolor_formatter.rs b/crates/schema/src/auto_migrate/termcolor_formatter.rs index 31807d14b30..fb10a09184b 100644 --- a/crates/schema/src/auto_migrate/termcolor_formatter.rs +++ b/crates/schema/src/auto_migrate/termcolor_formatter.rs @@ -417,6 +417,22 @@ impl MigrationFormatter for TermColorFormatter { Ok(()) } + fn format_change_table_accessor_name(&mut self, table_name: &str) -> io::Result<()> { + self.write_action_prefix(&Action::Changed)?; + self.buffer.write_all(b" table accessor name for ")?; + self.write_colored(table_name, Some(self.colors.table_name), true)?; + self.buffer.write_all(b"\n") + } + + fn format_change_column_accessor_name(&mut self, table_name: &str, col_name: &str) -> io::Result<()> { + self.write_action_prefix(&Action::Changed)?; + self.buffer.write_all(b" column accessor name for ")?; + self.write_colored(table_name, Some(self.colors.table_name), true)?; + self.buffer.write_all(b".")?; + self.write_colored(col_name, Some(self.colors.column_type), true)?; + self.buffer.write_all(b"\n") + } + fn format_disconnect_warning(&mut self) -> io::Result<()> { self.write_indent()?; self.write_with_background( From 5735c785ae6ff04ef1b53d74ee1abb2d3b8da747 Mon Sep 17 00:00:00 2001 From: Shubham Mishra Date: Wed, 29 Jul 2026 15:00:11 +0530 Subject: [PATCH 4/8] fmt --- .../src/locking_tx_datastore/mut_tx.rs | 16 +++++----- crates/engine/src/relational_db.rs | 4 ++- crates/schema/src/auto_migrate.rs | 29 ++++--------------- crates/schema/src/auto_migrate/formatter.rs | 4 +-- 4 files changed, 18 insertions(+), 35 deletions(-) diff --git a/crates/datastore/src/locking_tx_datastore/mut_tx.rs b/crates/datastore/src/locking_tx_datastore/mut_tx.rs index 10912768147..4ad8a9dc7de 100644 --- a/crates/datastore/src/locking_tx_datastore/mut_tx.rs +++ b/crates/datastore/src/locking_tx_datastore/mut_tx.rs @@ -1390,15 +1390,15 @@ impl MutTxId { } /// Change the runtime accessor name alias of the table identified by `table_id`. - pub(crate) fn alter_table_accessor_name( - &mut self, - table_id: TableId, - new_alias: Identifier, - ) -> Result<()> { + pub(crate) fn alter_table_accessor_name(&mut self, table_id: TableId, new_alias: Identifier) -> Result<()> { let table_name = self.find_st_table_row(table_id)?.table_name; let old_alias = self - .iter_by_col_eq(ST_TABLE_ACCESSOR_ID, StTableAccessorFields::TableName, &table_name.as_ref().into())? + .iter_by_col_eq( + ST_TABLE_ACCESSOR_ID, + StTableAccessorFields::TableName, + &table_name.as_ref().into(), + )? .next() .map(|row| StTableAccessorRow::try_from(row).ok()) .flatten() @@ -1464,7 +1464,9 @@ impl MutTxId { } }); - self.push_schema_change(PendingSchemaChange::ColumnAlterAccessorName(table_id, col_id, old_alias)); + self.push_schema_change(PendingSchemaChange::ColumnAlterAccessorName( + table_id, col_id, old_alias, + )); Ok(()) } diff --git a/crates/engine/src/relational_db.rs b/crates/engine/src/relational_db.rs index 3f0700818c1..821d40e92f7 100644 --- a/crates/engine/src/relational_db.rs +++ b/crates/engine/src/relational_db.rs @@ -1083,7 +1083,9 @@ impl RelationalDB { col_id: ColId, new_alias: spacetimedb_schema::identifier::Identifier, ) -> Result<(), DBError> { - Ok(self.inner.alter_column_accessor_name_mut_tx(tx, table_id, col_id, new_alias)?) + Ok(self + .inner + .alter_column_accessor_name_mut_tx(tx, table_id, col_id, new_alias)?) } pub(crate) fn alter_table_row_type( diff --git a/crates/schema/src/auto_migrate.rs b/crates/schema/src/auto_migrate.rs index 1e5e38635c6..d96f95308b5 100644 --- a/crates/schema/src/auto_migrate.rs +++ b/crates/schema/src/auto_migrate.rs @@ -304,10 +304,7 @@ pub enum AutoMigrateStep<'def> { /// Change the runtime accessor name alias of an existing table. ChangeTableAccessorName(::Key<'def>), /// Change the runtime accessor name alias of an existing column. - ChangeColumnAccessorName( - ::Key<'def>, - &'def Identifier, - ), + ChangeColumnAccessorName(::Key<'def>, &'def Identifier), /// Add a table, including all indexes, constraints, and sequences. /// There will NOT be separate steps in the plan for adding indexes, constraints, and sequences. @@ -2174,21 +2171,13 @@ mod tests { fn migrate_table_with_changed_accessor_name() { let old_def = create_module_def_v10(|builder| { builder - .build_table_with_new_type( - "my_table", - ProductType::from([("id", AlgebraicType::U64)]), - true, - ) + .build_table_with_new_type("my_table", ProductType::from([("id", AlgebraicType::U64)]), true) .finish(); }); let new_def = create_module_def_v10(|builder| { builder - .build_table_with_new_type( - "renamed_table", - ProductType::from([("id", AlgebraicType::U64)]), - true, - ) + .build_table_with_new_type("renamed_table", ProductType::from([("id", AlgebraicType::U64)]), true) .finish(); let mut explicit = ExplicitNames::default(); explicit.insert_table("renamed_table", "my_table"); @@ -2217,21 +2206,13 @@ mod tests { fn migrate_column_with_changed_accessor_name() { let old_def = create_module_def_v10(|builder| { builder - .build_table_with_new_type( - "my_table", - ProductType::from([("my_field", AlgebraicType::U64)]), - true, - ) + .build_table_with_new_type("my_table", ProductType::from([("my_field", AlgebraicType::U64)]), true) .finish(); }); let new_def = create_module_def_v10(|builder| { builder - .build_table_with_new_type( - "my_table", - ProductType::from([("myField", AlgebraicType::U64)]), - true, - ) + .build_table_with_new_type("my_table", ProductType::from([("myField", AlgebraicType::U64)]), true) .finish(); }); diff --git a/crates/schema/src/auto_migrate/formatter.rs b/crates/schema/src/auto_migrate/formatter.rs index 793a98a52ea..f0469e08206 100644 --- a/crates/schema/src/auto_migrate/formatter.rs +++ b/crates/schema/src/auto_migrate/formatter.rs @@ -67,9 +67,7 @@ fn format_step( let table_info = extract_table_info(*table, plan)?; f.format_change_table_accessor_name(table_info.name.as_ref()) } - AutoMigrateStep::ChangeColumnAccessorName(table, col) => { - f.format_change_column_accessor_name(table, col) - } + AutoMigrateStep::ChangeColumnAccessorName(table, col) => f.format_change_column_accessor_name(table, col), AutoMigrateStep::RemoveConstraint(constraint) => { let constraint_info = extract_constraint_info(*constraint, plan.old)?; f.format_constraint(&constraint_info, Action::Removed) From 155161290a09369af48cc2427940ab5d7fdfcebb Mon Sep 17 00:00:00 2001 From: Shubham Mishra Date: Wed, 29 Jul 2026 15:22:22 +0530 Subject: [PATCH 5/8] lint --- crates/datastore/src/locking_tx_datastore/mut_tx.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/datastore/src/locking_tx_datastore/mut_tx.rs b/crates/datastore/src/locking_tx_datastore/mut_tx.rs index 4ad8a9dc7de..0e48c67ea6d 100644 --- a/crates/datastore/src/locking_tx_datastore/mut_tx.rs +++ b/crates/datastore/src/locking_tx_datastore/mut_tx.rs @@ -1400,8 +1400,7 @@ impl MutTxId { &table_name.as_ref().into(), )? .next() - .map(|row| StTableAccessorRow::try_from(row).ok()) - .flatten() + .and_then(|row| StTableAccessorRow::try_from(row).ok()) .map(|row| row.accessor_name); if old_alias.as_ref() == Some(&new_alias) { From 325e91cbe7fc265ac5f2a109a0616115bc14a774 Mon Sep 17 00:00:00 2001 From: Shubham Mishra Date: Wed, 29 Jul 2026 19:29:11 +0530 Subject: [PATCH 6/8] comments --- crates/datastore/Cargo.toml | 1 + crates/datastore/src/locking_tx_datastore/mut_tx.rs | 6 +++--- crates/schema/src/auto_migrate.rs | 6 +++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/datastore/Cargo.toml b/crates/datastore/Cargo.toml index 86cb2aca0b6..37d005a7e64 100644 --- a/crates/datastore/Cargo.toml +++ b/crates/datastore/Cargo.toml @@ -47,6 +47,7 @@ test = ["spacetimedb-commitlog/test", "spacetimedb-schema/test"] spacetimedb-lib = { path = "../lib", features = ["proptest"] } spacetimedb-sats = { path = "../sats", features = ["proptest"] } spacetimedb-commitlog = { path = "../commitlog", features = ["test"] } +spacetimedb-schema = { workspace = true, features = ["test"] } # Also as dev-dependencies for use in _this_ crate's tests. proptest.workspace = true diff --git a/crates/datastore/src/locking_tx_datastore/mut_tx.rs b/crates/datastore/src/locking_tx_datastore/mut_tx.rs index 0e48c67ea6d..7c3b985835c 100644 --- a/crates/datastore/src/locking_tx_datastore/mut_tx.rs +++ b/crates/datastore/src/locking_tx_datastore/mut_tx.rs @@ -1349,7 +1349,7 @@ impl MutTxId { Ok(()) } - /// Change the runtime source-name alias of the index identified by `index_id`. + /// Change the source-name alias of the index identified by `index_id`. pub(crate) fn alter_index_source_name( &mut self, index_id: IndexId, @@ -1389,7 +1389,7 @@ impl MutTxId { Ok(()) } - /// Change the runtime accessor name alias of the table identified by `table_id`. + /// Change the accessor name alias of the table identified by `table_id`. pub(crate) fn alter_table_accessor_name(&mut self, table_id: TableId, new_alias: Identifier) -> Result<()> { let table_name = self.find_st_table_row(table_id)?.table_name; @@ -1418,7 +1418,7 @@ impl MutTxId { Ok(()) } - /// Change the runtime accessor name alias of the column identified by `table_id` and `col_id`. + /// Change the accessor name alias of the column identified by `table_id` and `col_id`. pub(crate) fn alter_column_accessor_name( &mut self, table_id: TableId, diff --git a/crates/schema/src/auto_migrate.rs b/crates/schema/src/auto_migrate.rs index d96f95308b5..eeb1b3fc85f 100644 --- a/crates/schema/src/auto_migrate.rs +++ b/crates/schema/src/auto_migrate.rs @@ -298,12 +298,12 @@ pub enum AutoMigrateStep<'def> { /// no `ChangeColumns` steps will be, for the same table. AddColumns(::Key<'def>), - /// Change the runtime source-name alias of an existing index. + /// Change the source-name alias of an existing index. ChangeIndexSourceName(::Key<'def>), - /// Change the runtime accessor name alias of an existing table. + /// Change the accessor name alias of an existing table. ChangeTableAccessorName(::Key<'def>), - /// Change the runtime accessor name alias of an existing column. + /// Change the accessor name alias of an existing column. ChangeColumnAccessorName(::Key<'def>, &'def Identifier), /// Add a table, including all indexes, constraints, and sequences. From 77e19a94f1956703b976fb85113c166176010e74 Mon Sep 17 00:00:00 2001 From: Shubham Mishra Date: Mon, 3 Aug 2026 17:06:46 +0530 Subject: [PATCH 7/8] insert 'PendingSchemaChanges' early --- .../src/locking_tx_datastore/mut_tx.rs | 39 +++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/crates/datastore/src/locking_tx_datastore/mut_tx.rs b/crates/datastore/src/locking_tx_datastore/mut_tx.rs index 7c3b985835c..397acfedc93 100644 --- a/crates/datastore/src/locking_tx_datastore/mut_tx.rs +++ b/crates/datastore/src/locking_tx_datastore/mut_tx.rs @@ -34,7 +34,7 @@ use crate::{ locking_tx_datastore::state_view::ScanOrIndex, traits::{InsertFlags, RowTypeForTable, TxData, UpdateFlags}, }; -use core::{cell::RefCell, iter, mem, ops::RangeBounds}; +use core::{cell::RefCell, iter, ops::RangeBounds}; use itertools::Either; use smallvec::SmallVec; use spacetimedb_data_structures::map::{HashMap, HashSet, IntMap}; @@ -1316,16 +1316,18 @@ impl MutTxId { impl MutTxId { /// Set the table access of `table_id` to `access`. pub(crate) fn alter_table_access(&mut self, table_id: TableId, access: StAccess) -> Result<()> { + let old_access = self.find_st_table_row(table_id)?.table_access; + // Write to the table in the tx state. let ((tx_table, ..), (commit_table, ..)) = self.get_or_create_insert_table_mut(table_id)?; tx_table.with_mut_schema_and_clone(commit_table, |s| s.table_access = access); - // Update system tables. - let old_access = self.update_st_table_row(table_id, |st| mem::replace(&mut st.table_access, access))?; - - // Remember the pending change so we can undo if necessary. + // Remember the pending change so we can undo it if a later system-table update fails. self.push_schema_change(PendingSchemaChange::TableAlterAccess(table_id, old_access)); + // Update system tables. + self.update_st_table_row(table_id, |st| st.table_access = access)?; + Ok(()) } @@ -1334,18 +1336,19 @@ impl MutTxId { /// Updates both the in-memory schema and the `st_table` system table. /// See: pub(crate) fn alter_table_primary_key(&mut self, table_id: TableId, new_primary_key: Option) -> Result<()> { + let old_pk = self.find_st_table_row(table_id)?.table_primary_key; + // Write to the table in the tx state. let ((tx_table, ..), (commit_table, ..)) = self.get_or_create_insert_table_mut(table_id)?; tx_table.with_mut_schema_and_clone(commit_table, |s| s.primary_key = new_primary_key); - // Update system tables. + // Remember the pending change so we can undo it if a later system-table update fails. let new_pk_col_list = new_primary_key.map(|col| col.into()); - let old_pk = - self.update_st_table_row(table_id, |st| mem::replace(&mut st.table_primary_key, new_pk_col_list))?; - - // Remember the pending change so we can undo if necessary. self.push_schema_change(PendingSchemaChange::TableAlterPrimaryKey(table_id, old_pk)); + // Update system tables. + self.update_st_table_row(table_id, |st| st.table_primary_key = new_pk_col_list)?; + Ok(()) } @@ -1380,12 +1383,11 @@ impl MutTxId { .ok_or_else(|| TableError::IdNotFound(SystemTable::st_index, index_id.into()))?; index_schema.alias = Some(source_name.clone()); tx_table.with_mut_schema_and_clone(commit_table, |s| s.update_index(index_schema)); + self.push_schema_change(PendingSchemaChange::IndexAlterSourceName(table_id, index_id, old_alias)); self.drop_st_index_accessor(&st_index_row.index_name)?; self.insert_st_index_accessor(&st_index_row.index_name, Some(&source_name))?; - self.push_schema_change(PendingSchemaChange::IndexAlterSourceName(table_id, index_id, old_alias)); - Ok(()) } @@ -1409,12 +1411,11 @@ impl MutTxId { let ((tx_table, ..), (commit_table, ..)) = self.get_or_create_insert_table_mut(table_id)?; tx_table.with_mut_schema_and_clone(commit_table, |s| s.alias = Some(new_alias.clone())); + self.push_schema_change(PendingSchemaChange::TableAlterAccessorName(table_id, old_alias)); self.drop_st_table_accessor(&table_name)?; self.insert_st_table_accessor(&table_name, Some(&new_alias))?; - self.push_schema_change(PendingSchemaChange::TableAlterAccessorName(table_id, old_alias)); - Ok(()) } @@ -1502,6 +1503,8 @@ impl MutTxId { // SAFETY: `commit_table` should have a schema identical to that of `tx_table` // prior to changing it just now. unsafe { commit_table.set_layout_and_schema_to(tx_table) }; + // Remember the pending change so we can undo it if a later system-table update fails. + self.push_schema_change(PendingSchemaChange::TableAlterRowType(table_id, old_column_schemas)); // Update system tables. // We'll simply remove all rows in `st_columns` and then add the new ones. @@ -1511,9 +1514,6 @@ impl MutTxId { self.drop_st_column_accessor(&table_name)?; self.insert_st_column(&table_name, &column_schemas)?; - // Remember the pending change so we can undo if necessary. - self.push_schema_change(PendingSchemaChange::TableAlterRowType(table_id, old_column_schemas)); - Ok(()) } @@ -1542,6 +1542,8 @@ impl MutTxId { commit_table .change_columns_of_empty_table_to(column_schemas.clone()) .map_err(|_| TableError::EventTableNotEmpty(table_id))?; + // Remember the pending change so we can undo it if a later system-table update fails. + self.push_schema_change(PendingSchemaChange::ReschemaEventTable(table_id, old_column_schemas)); // Update system tables. // We'll simply remove all rows in `st_columns` and then add the new ones. @@ -1551,9 +1553,6 @@ impl MutTxId { self.drop_st_column_accessor(&table_name)?; self.insert_st_column(&table_name, &column_schemas)?; - // Remember the pending change so we can undo if necessary. - self.push_schema_change(PendingSchemaChange::ReschemaEventTable(table_id, old_column_schemas)); - Ok(()) } From 02b17d4736ba267f95ba8f8abc066b3d51c6b34f Mon Sep 17 00:00:00 2001 From: Shubham Mishra Date: Wed, 5 Aug 2026 14:37:30 +0530 Subject: [PATCH 8/8] lints --- crates/schema/src/auto_migrate.rs | 20 +++++++++++++------- crates/schema/tests/ensure_same_schema.rs | 3 +++ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/crates/schema/src/auto_migrate.rs b/crates/schema/src/auto_migrate.rs index 71c6e07075e..97fff428830 100644 --- a/crates/schema/src/auto_migrate.rs +++ b/crates/schema/src/auto_migrate.rs @@ -2296,21 +2296,22 @@ mod tests { let old_def = module_def("OldBasketLookup"); let new_def = module_def("NewBasketLookup"); + let root = NamespacePath::root(); let index_name = RawIdentifier::new("fruit_basket_basket_id_fruit_name_idx_btree"); let plan = ponder_auto_migrate(&old_def, &new_def).expect("auto migration should succeed"); let steps = &plan.steps[..]; assert!( - steps.contains(&AutoMigrateStep::ChangeIndexSourceName(&index_name)), + steps.contains(&AutoMigrateStep::ChangeIndexSourceName((&root, &index_name))), "steps: {steps:?}" ); assert!( - !steps.contains(&AutoMigrateStep::RemoveIndex(&index_name)), + !steps.contains(&AutoMigrateStep::RemoveIndex((&root, &index_name))), "steps: {steps:?}" ); assert!( - !steps.contains(&AutoMigrateStep::AddIndex(&index_name)), + !steps.contains(&AutoMigrateStep::AddIndex((&root, &index_name))), "steps: {steps:?}" ); } @@ -2334,18 +2335,19 @@ mod tests { let plan = ponder_auto_migrate(&old_def, &new_def).expect("auto migration should succeed"); let steps = &plan.steps[..]; + let root = NamespacePath::root(); let table_name = expect_identifier("my_table"); assert!( - steps.contains(&AutoMigrateStep::ChangeTableAccessorName(&table_name)), + steps.contains(&AutoMigrateStep::ChangeTableAccessorName((&root, &table_name))), "steps: {steps:?}" ); assert!( - !steps.contains(&AutoMigrateStep::RemoveTable(&table_name)), + !steps.contains(&AutoMigrateStep::RemoveTable((&root, &table_name))), "steps: {steps:?}" ); assert!( - !steps.contains(&AutoMigrateStep::AddTable(&table_name)), + !steps.contains(&AutoMigrateStep::AddTable((&root, &table_name))), "steps: {steps:?}" ); } @@ -2366,11 +2368,15 @@ mod tests { let plan = ponder_auto_migrate(&old_def, &new_def).expect("auto migration should succeed"); let steps = &plan.steps[..]; + let root = NamespacePath::root(); let table_name = expect_identifier("my_table"); let col_name = expect_identifier("my_field"); assert!( - steps.contains(&AutoMigrateStep::ChangeColumnAccessorName(&table_name, &col_name)), + steps.contains(&AutoMigrateStep::ChangeColumnAccessorName( + (&root, &table_name), + &col_name + )), "steps: {steps:?}" ); } diff --git a/crates/schema/tests/ensure_same_schema.rs b/crates/schema/tests/ensure_same_schema.rs index 3dff113d8f5..b2427942e88 100644 --- a/crates/schema/tests/ensure_same_schema.rs +++ b/crates/schema/tests/ensure_same_schema.rs @@ -32,6 +32,9 @@ fn step_namespace<'a, 'def>(step: &'a AutoMigrateStep<'def>) -> Option<&'a Names | AutoMigrateStep::RemoveIndex((ns, _)) | AutoMigrateStep::RemoveConstraint((ns, _)) | AutoMigrateStep::RemoveSequence((ns, _)) + | AutoMigrateStep::ChangeIndexSourceName((ns, _)) + | AutoMigrateStep::ChangeTableAccessorName((ns, _)) + | AutoMigrateStep::ChangeColumnAccessorName((ns, _), _) | AutoMigrateStep::AddIndex((ns, _)) | AutoMigrateStep::AddConstraint((ns, _)) | AutoMigrateStep::AddSequence((ns, _)) => Some(ns),