Skip to content

Commit e2a1966

Browse files
committed
index source name change migration step
1 parent 8fa8584 commit e2a1966

10 files changed

Lines changed: 340 additions & 13 deletions

File tree

crates/core/src/db/relational_db.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,6 +1014,15 @@ impl RelationalDB {
10141014
Ok(self.inner.alter_table_primary_key_mut_tx(tx, name, primary_key)?)
10151015
}
10161016

1017+
pub(crate) fn alter_index_source_name(
1018+
&self,
1019+
tx: &mut MutTx,
1020+
index_id: IndexId,
1021+
source_name: spacetimedb_sats::raw_identifier::RawIdentifier,
1022+
) -> Result<(), DBError> {
1023+
Ok(self.inner.alter_index_source_name_mut_tx(tx, index_id, source_name)?)
1024+
}
1025+
10171026
pub(crate) fn alter_table_row_type(
10181027
&self,
10191028
tx: &mut MutTx,

crates/core/src/db/update.rs

Lines changed: 134 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -209,11 +209,34 @@ fn auto_migrate_database(
209209
.indexes
210210
.iter()
211211
.find(|index| index.index_name[..] == index_name[..])
212-
.unwrap();
212+
.ok_or_else(|| anyhow::anyhow!("Index `{index_name}` not found in table `{}`", table_def.name))?;
213213

214214
log!(logger, "Dropping index `{}` on table `{}`", index_name, table_def.name);
215215
stdb.drop_index(tx, index_schema.index_id)?;
216216
}
217+
spacetimedb_schema::auto_migrate::AutoMigrateStep::ChangeIndexSourceName(index_name) => {
218+
let old_table_def = plan.old.stored_in_table_def(index_name).unwrap();
219+
let new_table_def = plan.new.stored_in_table_def(index_name).unwrap();
220+
let new_index_def = new_table_def.indexes.get(index_name).unwrap();
221+
222+
let table_id = stdb.table_id_from_name_mut(tx, &old_table_def.name)?.unwrap();
223+
let table_schema = stdb.schema_for_table_mut(tx, table_id)?;
224+
let index_schema = table_schema
225+
.indexes
226+
.iter()
227+
.find(|index| index.index_name[..] == index_name[..])
228+
.ok_or_else(|| anyhow::anyhow!("Index `{index_name}` not found in table `{}`", old_table_def.name))?;
229+
230+
log!(
231+
logger,
232+
"Changing index source name for `{}` on table `{}` from `{}` to `{}`",
233+
index_name,
234+
old_table_def.name,
235+
index_schema.alias.as_deref().unwrap_or(""),
236+
new_index_def.source_name,
237+
);
238+
stdb.alter_index_source_name(tx, index_schema.index_id, new_index_def.source_name.clone())?;
239+
}
217240
spacetimedb_schema::auto_migrate::AutoMigrateStep::RemoveConstraint(constraint_name) => {
218241
let table_def = plan.old.stored_in_table_def(constraint_name).unwrap();
219242

@@ -340,9 +363,13 @@ mod test {
340363
host::module_host::create_table_from_def,
341364
};
342365
use spacetimedb_datastore::locking_tx_datastore::PendingSchemaChange;
343-
use spacetimedb_lib::db::raw_def::v9::{btree, RawIndexAlgorithm, RawModuleDefV9Builder, TableAccess};
344-
use spacetimedb_sats::{product, AlgebraicType, AlgebraicType::U64};
345-
use spacetimedb_schema::{auto_migrate::ponder_migrate, def::ModuleDef};
366+
use spacetimedb_lib::db::raw_def::{
367+
v10::{ExplicitNames, RawModuleDefV10Builder},
368+
v9::{btree, RawIndexAlgorithm, RawModuleDefV9Builder, TableAccess},
369+
};
370+
use spacetimedb_sats::{product, raw_identifier::RawIdentifier, AlgebraicType, AlgebraicType::U64, ProductType};
371+
use spacetimedb_schema::auto_migrate::{ponder_migrate, AutoMigrateStep, MigratePlan};
372+
use spacetimedb_schema::def::ModuleDef;
346373

347374
struct TestLogger;
348375
impl UpdateLogger for TestLogger {
@@ -424,6 +451,109 @@ mod test {
424451
Ok(())
425452
}
426453

454+
#[test]
455+
fn update_db_change_index_source_name_updates_lookup_and_persists() -> anyhow::Result<()> {
456+
let auth_ctx = AuthCtx::for_testing();
457+
let stdb = TestDB::durable()?;
458+
459+
fn module_def(table_source_name: &str, index_source_name: &str) -> ModuleDef {
460+
let mut builder = RawModuleDefV10Builder::new();
461+
builder
462+
.build_table_with_new_type(
463+
table_source_name.to_owned(),
464+
ProductType::from([("id", U64), ("emailAddress", AlgebraicType::String)]),
465+
true,
466+
)
467+
.with_access(TableAccess::Public)
468+
.with_index(btree(1), index_source_name.to_owned(), "emailAddress")
469+
.finish();
470+
471+
if table_source_name != "users" {
472+
let mut explicit_names = ExplicitNames::default();
473+
explicit_names.insert_table(table_source_name.to_owned(), "users");
474+
builder.add_explicit_names(explicit_names);
475+
}
476+
477+
builder
478+
.finish()
479+
.try_into()
480+
.expect("builder should create a valid database definition")
481+
}
482+
483+
let old_source_name = "users_emailAddress_idx_btree";
484+
let new_source_name = "appUsers_emailAddress_idx_btree";
485+
let old = module_def("users", old_source_name);
486+
let new = module_def("appUsers", new_source_name);
487+
488+
let mut tx = begin_mut_tx(&stdb);
489+
for def in old.tables() {
490+
create_table_from_def(&stdb, &mut tx, &old, def)?;
491+
}
492+
stdb.commit_tx(tx)?;
493+
494+
let tx = begin_mut_tx(&stdb);
495+
let table_id = stdb
496+
.table_id_from_name_mut(&tx, "users")?
497+
.expect("there should be a table named users");
498+
let table_schema = stdb.schema_for_table_mut(&tx, table_id)?;
499+
let index_schema = table_schema
500+
.indexes
501+
.first()
502+
.expect("there should be a single index")
503+
.clone();
504+
let canonical_index_name = index_schema.index_name.to_string();
505+
let index_id = index_schema.index_id;
506+
assert_eq!(stdb.index_id_from_name_mut(&tx, old_source_name)?, Some(index_id));
507+
assert_eq!(stdb.index_id_from_name_mut(&tx, new_source_name)?, None);
508+
assert_eq!(stdb.index_id_from_name_mut(&tx, &canonical_index_name)?, Some(index_id));
509+
drop(tx);
510+
511+
let MigratePlan::Auto(plan) = ponder_migrate(&old, &new)? else {
512+
panic!("expected automatic migration");
513+
};
514+
let index_name = RawIdentifier::new(canonical_index_name.as_str());
515+
assert!(
516+
plan.steps.contains(&AutoMigrateStep::ChangeIndexSourceName(&index_name)),
517+
"plan steps: {:?}",
518+
plan.steps
519+
);
520+
assert!(
521+
!plan.steps.contains(&AutoMigrateStep::RemoveIndex(&index_name)),
522+
"plan steps: {:?}",
523+
plan.steps
524+
);
525+
assert!(
526+
!plan.steps.contains(&AutoMigrateStep::AddIndex(&index_name)),
527+
"plan steps: {:?}",
528+
plan.steps
529+
);
530+
let mut tx = begin_mut_tx(&stdb);
531+
let res = update_database(&stdb, &mut tx, auth_ctx, MigratePlan::Auto(plan), &TestLogger)?;
532+
assert!(matches!(res, UpdateResult::Success));
533+
534+
assert_eq!(stdb.index_id_from_name_mut(&tx, old_source_name)?, None);
535+
assert_eq!(stdb.index_id_from_name_mut(&tx, new_source_name)?, Some(index_id));
536+
assert_eq!(stdb.index_id_from_name_mut(&tx, &canonical_index_name)?, Some(index_id));
537+
assert!(
538+
tx.pending_schema_changes().iter().any(|change| matches!(
539+
change,
540+
PendingSchemaChange::IndexAlterSourceName(tid, iid, Some(old_alias))
541+
if *tid == table_id && *iid == index_id && old_alias.as_ref() == old_source_name
542+
)),
543+
"pending schema changes: {:?}",
544+
tx.pending_schema_changes()
545+
);
546+
stdb.commit_tx(tx)?;
547+
548+
let stdb = stdb.reopen()?;
549+
let tx = begin_mut_tx(&stdb);
550+
assert_eq!(stdb.index_id_from_name_mut(&tx, old_source_name)?, None);
551+
assert_eq!(stdb.index_id_from_name_mut(&tx, new_source_name)?, Some(index_id));
552+
assert_eq!(stdb.index_id_from_name_mut(&tx, &canonical_index_name)?, Some(index_id));
553+
554+
Ok(())
555+
}
556+
427557
/// Regression test for #3934: removing a primary key annotation and then
428558
/// re-publishing causes "Primary key mismatch" on the NEXT publish.
429559
#[test]

crates/datastore/src/locking_tx_datastore/committed_state.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,18 @@ impl CommittedState {
707707
table.with_mut_schema(|s| s.remove_index(index_id));
708708
self.index_id_map.remove(&index_id);
709709
}
710+
// An index alias/source-name changed. Change it back.
711+
IndexAlterSourceName(table_id, index_id, old_alias) => {
712+
let table = self.tables.get_mut(&table_id)?;
713+
let mut index_schema = table
714+
.get_schema()
715+
.indexes
716+
.iter()
717+
.find(|x| x.index_id == index_id)?
718+
.clone();
719+
index_schema.alias = old_alias;
720+
table.with_mut_schema(|s| s.update_index(index_schema));
721+
}
710722
// A table was removed. Add it back.
711723
TableRemoved(table_id, table) => {
712724
let is_view_table = table.schema.is_view();

crates/datastore/src/locking_tx_datastore/datastore.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,15 @@ impl Locking {
294294
tx.alter_table_primary_key(table_id, primary_key)
295295
}
296296

297+
pub fn alter_index_source_name_mut_tx(
298+
&self,
299+
tx: &mut MutTxId,
300+
index_id: IndexId,
301+
source_name: spacetimedb_sats::raw_identifier::RawIdentifier,
302+
) -> Result<()> {
303+
tx.alter_index_source_name(index_id, source_name)
304+
}
305+
297306
pub fn alter_table_row_type_mut_tx(
298307
&self,
299308
tx: &mut MutTxId,
@@ -527,6 +536,15 @@ impl MutTxDatastore for Locking {
527536
tx.drop_index(index_id)
528537
}
529538

539+
fn alter_index_source_name_mut_tx(
540+
&self,
541+
tx: &mut Self::MutTx,
542+
index_id: IndexId,
543+
source_name: spacetimedb_sats::raw_identifier::RawIdentifier,
544+
) -> Result<()> {
545+
tx.alter_index_source_name(index_id, source_name)
546+
}
547+
530548
fn index_id_from_name_mut_tx(&self, tx: &Self::MutTx, index_name: &str) -> Result<Option<IndexId>> {
531549
tx.index_id_from_name_or_alias(index_name)
532550
}

crates/datastore/src/locking_tx_datastore/mut_tx.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1163,6 +1163,46 @@ impl MutTxId {
11631163
Ok(())
11641164
}
11651165

1166+
/// Change the runtime source-name alias of the index identified by `index_id`.
1167+
pub(crate) fn alter_index_source_name(
1168+
&mut self,
1169+
index_id: IndexId,
1170+
source_name: spacetimedb_sats::raw_identifier::RawIdentifier,
1171+
) -> Result<()> {
1172+
let st_index_ref = self
1173+
.iter_by_col_eq(ST_INDEX_ID, StIndexFields::IndexId, &index_id.into())?
1174+
.next()
1175+
.ok_or_else(|| TableError::IdNotFound(SystemTable::st_index, index_id.into()))?;
1176+
let st_index_row = StIndexRow::try_from(st_index_ref)?;
1177+
let table_id = st_index_row.table_id;
1178+
1179+
let old_alias = self
1180+
.find_st_index_accessor_row_by_index_name(st_index_row.index_name.as_ref())?
1181+
.map(|row| row.accessor_name);
1182+
1183+
if old_alias.as_ref() == Some(&source_name) {
1184+
return Ok(());
1185+
}
1186+
1187+
let ((tx_table, ..), (commit_table, ..)) = self.get_or_create_insert_table_mut(table_id)?;
1188+
let mut index_schema = tx_table
1189+
.get_schema()
1190+
.indexes
1191+
.iter()
1192+
.find(|index| index.index_id == index_id)
1193+
.cloned()
1194+
.ok_or_else(|| TableError::IdNotFound(SystemTable::st_index, index_id.into()))?;
1195+
index_schema.alias = Some(source_name.clone());
1196+
tx_table.with_mut_schema_and_clone(commit_table, |s| s.update_index(index_schema));
1197+
1198+
self.drop_st_index_accessor(&st_index_row.index_name)?;
1199+
self.insert_st_index_accessor(&st_index_row.index_name, Some(&source_name))?;
1200+
1201+
self.push_schema_change(PendingSchemaChange::IndexAlterSourceName(table_id, index_id, old_alias));
1202+
1203+
Ok(())
1204+
}
1205+
11661206
/// Change the row type of the table identified by `table_id`.
11671207
///
11681208
/// In practice, this should not error,

crates/datastore/src/locking_tx_datastore/tx_state.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,13 @@ pub enum PendingSchemaChange {
111111
/// If adding this index caused the pointer map to be removed,
112112
/// it will be present here.
113113
IndexAdded(TableId, IndexId, Option<PointerMap>),
114+
/// The source-name alias of the index with [`IndexId`] changed.
115+
/// The old alias is stored for rollback.
116+
IndexAlterSourceName(
117+
TableId,
118+
IndexId,
119+
Option<spacetimedb_sats::raw_identifier::RawIdentifier>,
120+
),
114121
/// The [`Table`] with [`TableId`] was removed.
115122
TableRemoved(TableId, Table),
116123
/// The table with [`TableId`] was added.
@@ -145,6 +152,9 @@ impl MemoryUsage for PendingSchemaChange {
145152
Self::IndexAdded(table_id, index_id, pointer_map) => {
146153
table_id.heap_usage() + index_id.heap_usage() + pointer_map.heap_usage()
147154
}
155+
Self::IndexAlterSourceName(table_id, index_id, alias) => {
156+
table_id.heap_usage() + index_id.heap_usage() + alias.heap_usage()
157+
}
148158
Self::TableRemoved(table_id, table) => table_id.heap_usage() + table.heap_usage(),
149159
Self::TableAdded(table_id) => table_id.heap_usage(),
150160
Self::TableAlterAccess(table_id, st_access) => table_id.heap_usage() + st_access.heap_usage(),

crates/datastore/src/traits.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,12 @@ pub trait MutTxDatastore: TxDatastore + MutTx {
627627

628628
fn create_index_mut_tx(&self, tx: &mut Self::MutTx, index_schema: IndexSchema, is_unique: bool) -> Result<IndexId>;
629629
fn drop_index_mut_tx(&self, tx: &mut Self::MutTx, index_id: IndexId) -> Result<()>;
630+
fn alter_index_source_name_mut_tx(
631+
&self,
632+
tx: &mut Self::MutTx,
633+
index_id: IndexId,
634+
source_name: spacetimedb_sats::raw_identifier::RawIdentifier,
635+
) -> Result<()>;
630636
fn index_id_from_name_mut_tx(&self, tx: &Self::MutTx, index_name: &str) -> super::Result<Option<IndexId>>;
631637

632638
// TODO: Index data

crates/schema/src/auto_migrate.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,9 @@ pub enum AutoMigrateStep<'def> {
284284
/// no `ChangeColumns` steps will be, for the same table.
285285
AddColumns(<TableDef as ModuleDefLookup>::Key<'def>),
286286

287+
/// Change the runtime source-name alias of an existing index.
288+
ChangeIndexSourceName(<IndexDef as ModuleDefLookup>::Key<'def>),
289+
287290
/// Add a table, including all indexes, constraints, and sequences.
288291
/// There will NOT be separate steps in the plan for adding indexes, constraints, and sequences.
289292
AddTable(<TableDef as ModuleDefLookup>::Key<'def>),
@@ -981,6 +984,8 @@ fn auto_migrate_indexes(
981984
if old.algorithm != new.algorithm {
982985
plan.steps.push(AutoMigrateStep::RemoveIndex(old.key()));
983986
plan.steps.push(AutoMigrateStep::AddIndex(old.key()));
987+
} else if old.source_name != new.source_name {
988+
plan.steps.push(AutoMigrateStep::ChangeIndexSourceName(old.key()));
984989
}
985990
Ok(())
986991
}
@@ -2039,6 +2044,42 @@ mod tests {
20392044
);
20402045
}
20412046

2047+
#[test]
2048+
fn migrate_index_with_changed_source_name() {
2049+
fn module_def(source_name: &str) -> ModuleDef {
2050+
create_module_def_v10(|builder| {
2051+
builder
2052+
.build_table_with_new_type(
2053+
"FruitBasket",
2054+
ProductType::from([("basket_id", AlgebraicType::U64), ("fruit_name", AlgebraicType::String)]),
2055+
true,
2056+
)
2057+
.with_index(btree([0, 1]), source_name.to_owned(), "fruitNameIndex")
2058+
.finish();
2059+
})
2060+
}
2061+
2062+
let old_def = module_def("OldBasketLookup");
2063+
let new_def = module_def("NewBasketLookup");
2064+
let index_name = RawIdentifier::new("fruit_basket_basket_id_fruit_name_idx_btree");
2065+
2066+
let plan = ponder_auto_migrate(&old_def, &new_def).expect("auto migration should succeed");
2067+
let steps = &plan.steps[..];
2068+
2069+
assert!(
2070+
steps.contains(&AutoMigrateStep::ChangeIndexSourceName(&index_name)),
2071+
"steps: {steps:?}"
2072+
);
2073+
assert!(
2074+
!steps.contains(&AutoMigrateStep::RemoveIndex(&index_name)),
2075+
"steps: {steps:?}"
2076+
);
2077+
assert!(
2078+
!steps.contains(&AutoMigrateStep::AddIndex(&index_name)),
2079+
"steps: {steps:?}"
2080+
);
2081+
}
2082+
20422083
#[test]
20432084
fn migrate_view_disconnect_clients() {
20442085
struct TestCase {

crates/schema/src/auto_migrate/formatter.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ fn format_step<F: MigrationFormatter>(
5959
let index_info = extract_index_info(*index, plan.old)?;
6060
f.format_index(&index_info, Action::Removed)
6161
}
62+
AutoMigrateStep::ChangeIndexSourceName(index) => {
63+
let index_info = extract_index_info(*index, plan.new)?;
64+
f.format_index(&index_info, Action::Changed)
65+
}
6266
AutoMigrateStep::RemoveConstraint(constraint) => {
6367
let constraint_info = extract_constraint_info(*constraint, plan.old)?;
6468
f.format_constraint(&constraint_info, Action::Removed)

0 commit comments

Comments
 (0)