Skip to content

Commit 40af039

Browse files
committed
feat: Allow toggling event flag during auto-migration for empty tables
## Summary Currently, toggling `#[spacetimedb::table(event)]` on an existing table fails with `AutoMigrateError::ChangeTableEventFlag` and requires a manual migration. This PR allows the flip in either direction as a live auto-migration step when the table has zero committed rows. Non-empty tables fail with an actionable error guiding the user to clear the table first. Clients are disconnected on the flip because the change is observable to subscribers (event tables have no committed state). ## Changes - **`tx_state.rs`**: New `PendingSchemaChange::TableAlterEventFlag` variant storing the old `is_event` value for rollback. - **`committed_state.rs`**: Rollback branch restores the old value on the live schema. - **`mut_tx.rs`**: New `alter_table_event_flag` — dual-write to the tx + commit table schemas and to `st_event_table`. Idempotent no-op early returns before pushing a pending change. New `delete_st_event_table_row` helper using the existing `delete_col_eq` utility (hits the unique btree index on col 0). - **`replay.rs`**: New hook on `ST_EVENT_TABLE_ID` insert/delete mirrors `reschema_table_for_st_table_update` — flips `is_event` on the referenced user-table's cached schema during replay. This is load-bearing for cold replay across the flip point. - **`relational_db.rs`**: Thin `alter_table_event_flag` wrapper. - **`auto_migrate.rs`**: `event_ok` error branch replaced with `AutoMigrateStep::ChangeEventFlag` + `ensure_disconnect_all_users`. Removed dead `AutoMigrateError::ChangeTableEventFlag` variant. - **`formatter.rs` / `termcolor_formatter.rs`**: New `format_change_event_flag` mirroring `format_change_access`. - **`update.rs`**: New `ChangeEventFlag` handler with an O(1) row-count precheck before any mutation. ## Safety - **Transaction safety**: Precheck (row count) and all three writes (st_event_table, tx schema, commit schema) run in the same `MutTx`. - **Rollback**: `TableAlterEventFlag` stores the old flag value so failed txs revert `is_event` on the live schema. Idempotent flips do not push a pending change. - **Replay correctness**: Without the replay hook, cold replay from a pre-migration snapshot would miss the schema flip and post-migration inserts would silently land in committed state. The hook mirrors the existing `st_table`/`st_column` pattern. - **Client contract**: Flipping `event` changes observability — v1 subscribers stop seeing updates; v2 subscribers see a different message variant. `ensure_disconnect_all_users` forces reconnection. ## Example error output ``` Cannot change `event` flag on table `my_table`: table contains data. Clear the table's rows (e.g. via a reducer) before toggling the `event` annotation. ``` ## Test plan - [x] `cargo test -p spacetimedb-datastore --features test` — 87 pass (including 4 new `alter_table_event_flag` tests) - [x] `cargo test -p spacetimedb-schema` — 103 pass (including 3 new `change_event_flag` plan tests) - [x] `cargo test -p spacetimedb-core` — 192 pass (including 2 new empty/non-empty integration tests) - [x] `cargo clippy -p spacetimedb-datastore -p spacetimedb-schema -p spacetimedb-core --tests` clean - [x] Pre-existing event-table tests still pass (10 tests)
1 parent 2d67d76 commit 40af039

10 files changed

Lines changed: 507 additions & 29 deletions

File tree

crates/core/src/db/relational_db.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -989,6 +989,10 @@ impl RelationalDB {
989989
Ok(self.inner.alter_table_access_mut_tx(tx, name, access)?)
990990
}
991991

992+
pub(crate) fn alter_table_event_flag(&self, tx: &mut MutTx, name: &str, is_event: bool) -> Result<(), DBError> {
993+
Ok(self.inner.alter_table_event_flag_mut_tx(tx, name, is_event)?)
994+
}
995+
992996
pub(crate) fn alter_table_primary_key(
993997
&self,
994998
tx: &mut MutTx,

crates/core/src/db/update.rs

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,27 @@ fn auto_migrate_database(
282282
let table_def = plan.new.stored_in_table_def(&table_name.clone().into()).unwrap();
283283
stdb.alter_table_access(tx, table_name, table_def.table_access.into())?;
284284
}
285+
spacetimedb_schema::auto_migrate::AutoMigrateStep::ChangeEventFlag(table_name) => {
286+
let table_def: &TableDef = plan.new.expect_lookup(table_name);
287+
let table_id = stdb
288+
.table_id_from_name_mut(tx, table_name)?
289+
.expect("ChangeEventFlag references a table that should exist");
290+
291+
// Pre-validate: flipping is only safe when the table has no committed rows.
292+
if stdb.table_row_count_mut(tx, table_id).unwrap_or(0) > 0 {
293+
anyhow::bail!(
294+
"Cannot change `event` flag on table `{table_name}`: table contains data. \
295+
Clear the table's rows (e.g. via a reducer) before toggling the `event` annotation."
296+
);
297+
}
298+
299+
log!(
300+
logger,
301+
"Changing `event` flag on table `{table_name}` to `{}`",
302+
table_def.is_event
303+
);
304+
stdb.alter_table_event_flag(tx, table_name, table_def.is_event)?;
305+
}
285306
spacetimedb_schema::auto_migrate::AutoMigrateStep::ChangePrimaryKey(table_name) => {
286307
let table_def = plan.new.stored_in_table_def(&table_name.clone().into()).unwrap();
287308
log!(logger, "Changing primary key for table `{table_name}`");
@@ -339,6 +360,7 @@ mod test {
339360
db::relational_db::tests_utils::{begin_mut_tx, insert, TestDB},
340361
host::module_host::create_table_from_def,
341362
};
363+
use spacetimedb_datastore::locking_tx_datastore::state_view::StateView;
342364
use spacetimedb_datastore::locking_tx_datastore::PendingSchemaChange;
343365
use spacetimedb_lib::db::raw_def::v9::{btree, RawModuleDefV9Builder, TableAccess};
344366
use spacetimedb_sats::{product, AlgebraicType, AlgebraicType::U64};
@@ -580,4 +602,112 @@ mod test {
580602
);
581603
Ok(())
582604
}
605+
606+
/// Build a minimal v10 module with a single user table `events` whose
607+
/// `is_event` flag matches `is_event`.
608+
fn single_event_table_module_v10(is_event: bool) -> ModuleDef {
609+
use spacetimedb_lib::db::raw_def::v10::RawModuleDefV10Builder;
610+
611+
let mut builder = RawModuleDefV10Builder::new();
612+
let t = builder.build_table_with_new_type("events", [("id", U64)], true);
613+
let t = if is_event { t.with_event(true) } else { t };
614+
t.finish();
615+
builder
616+
.finish()
617+
.try_into()
618+
.expect("should be a valid v10 module definition")
619+
}
620+
621+
#[test]
622+
fn change_event_flag_empty_table_succeeds() -> anyhow::Result<()> {
623+
let auth_ctx = AuthCtx::for_testing();
624+
let stdb = TestDB::durable()?;
625+
626+
let old = single_event_table_module_v10(false);
627+
let new = single_event_table_module_v10(true);
628+
629+
// Create the non-event table.
630+
let mut tx = begin_mut_tx(&stdb);
631+
for def in old.tables() {
632+
create_table_from_def(&stdb, &mut tx, &old, def)?;
633+
}
634+
let table_id = stdb
635+
.table_id_from_name_mut(&tx, "events")?
636+
.expect("table should exist");
637+
assert!(
638+
!tx.get_schema(table_id)
639+
.map(|s| s.is_event)
640+
.expect("schema should exist"),
641+
"fresh non-event table should have `is_event=false`"
642+
);
643+
stdb.commit_tx(tx)?;
644+
645+
// Apply the auto-migration flipping `is_event=true` on an empty table.
646+
let mut tx = begin_mut_tx(&stdb);
647+
let plan = ponder_migrate(&old, &new)?;
648+
let res = update_database(&stdb, &mut tx, auth_ctx, plan, &TestLogger)?;
649+
650+
assert!(
651+
matches!(res, UpdateResult::RequiresClientDisconnect),
652+
"flipping the `event` flag should disconnect clients"
653+
);
654+
assert!(
655+
tx.get_schema(table_id)
656+
.map(|s| s.is_event)
657+
.expect("schema should exist"),
658+
"live schema should reflect the new `is_event=true`"
659+
);
660+
assert!(
661+
tx.pending_schema_changes()
662+
.iter()
663+
.any(|c| matches!(c, PendingSchemaChange::TableAlterEventFlag(_, false))),
664+
"flipping `is_event` should record a TableAlterEventFlag pending change: {:?}",
665+
tx.pending_schema_changes()
666+
);
667+
Ok(())
668+
}
669+
670+
#[test]
671+
fn change_event_flag_nonempty_table_fails() -> anyhow::Result<()> {
672+
let auth_ctx = AuthCtx::for_testing();
673+
let stdb = TestDB::durable()?;
674+
675+
let old = single_event_table_module_v10(false);
676+
let new = single_event_table_module_v10(true);
677+
678+
// Create the table, insert a row, then attempt to flip `is_event`.
679+
let mut tx = begin_mut_tx(&stdb);
680+
for def in old.tables() {
681+
create_table_from_def(&stdb, &mut tx, &old, def)?;
682+
}
683+
let table_id = stdb
684+
.table_id_from_name_mut(&tx, "events")?
685+
.expect("table should exist");
686+
insert(&stdb, &mut tx, table_id, &product![42u64])?;
687+
stdb.commit_tx(tx)?;
688+
689+
let mut tx = begin_mut_tx(&stdb);
690+
let plan = ponder_migrate(&old, &new)?;
691+
let result = update_database(&stdb, &mut tx, auth_ctx, plan, &TestLogger);
692+
let err = result
693+
.err()
694+
.expect("flipping `is_event` on a non-empty table should fail");
695+
assert!(
696+
err.to_string().contains("contains data"),
697+
"error should mention that the table contains data, got: {err}"
698+
);
699+
// The schema should be untouched and there should be no pending changes to roll back.
700+
assert!(
701+
!tx.get_schema(table_id)
702+
.map(|s| s.is_event)
703+
.expect("schema should exist"),
704+
"failed migration must leave the `is_event` flag unchanged"
705+
);
706+
assert!(
707+
tx.pending_schema_changes().is_empty(),
708+
"failed migration should leave no pending schema changes: {:?}",
709+
tx.pending_schema_changes()
710+
);
711+
Ok(())
712+
}
583713
}

crates/datastore/src/locking_tx_datastore/committed_state.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -921,6 +921,11 @@ impl CommittedState {
921921
let table = self.tables.get_mut(&table_id)?;
922922
table.with_mut_schema(|s| s.table_access = access);
923923
}
924+
// A table's `is_event` flag was changed. Change back to the old one.
925+
TableAlterEventFlag(table_id, old_is_event) => {
926+
let table = self.tables.get_mut(&table_id)?;
927+
table.with_mut_schema(|s| s.is_event = old_is_event);
928+
}
924929
// A table's primary key was changed. Change back to the old one.
925930
TableAlterPrimaryKey(table_id, old_pk) => {
926931
let table = self.tables.get_mut(&table_id)?;

crates/datastore/src/locking_tx_datastore/datastore.rs

Lines changed: 169 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,14 @@ impl Locking {
320320
tx.alter_table_access(table_id, access)
321321
}
322322

323+
pub fn alter_table_event_flag_mut_tx(&self, tx: &mut MutTxId, name: &str, is_event: bool) -> Result<()> {
324+
let table_id = self
325+
.table_id_from_name_mut_tx(tx, name)?
326+
.ok_or_else(|| TableError::NotFound(name.into()))?;
327+
328+
tx.alter_table_event_flag(table_id, is_event)
329+
}
330+
323331
pub fn alter_table_primary_key_mut_tx(
324332
&self,
325333
tx: &mut MutTxId,
@@ -1011,7 +1019,7 @@ pub(crate) mod tests {
10111019
use crate::locking_tx_datastore::tx_state::PendingSchemaChange;
10121020
use crate::system_tables::{
10131021
system_tables, StColumnRow, StConnectionCredentialsFields, StConstraintData, StConstraintFields,
1014-
StConstraintRow, StEventTableFields, StIndexAlgorithm, StIndexFields, StIndexRow, StRowLevelSecurityFields,
1022+
StConstraintRow, StEventTableFields, StFields as _, StIndexAlgorithm, StIndexFields, StIndexRow, StRowLevelSecurityFields,
10151023
StScheduledFields, StSequenceFields, StSequenceRow, StTableRow, StVarFields, StViewArgFields, StViewFields,
10161024
ST_CLIENT_ID, ST_CLIENT_NAME, ST_COLUMN_ACCESSOR_ID, ST_COLUMN_ACCESSOR_NAME, ST_COLUMN_ID, ST_COLUMN_NAME,
10171025
ST_CONNECTION_CREDENTIALS_ID, ST_CONNECTION_CREDENTIALS_NAME, ST_CONSTRAINT_ID, ST_CONSTRAINT_NAME,
@@ -3181,6 +3189,166 @@ pub(crate) mod tests {
31813189
Ok(())
31823190
}
31833191

3192+
/// Returns whether `st_event_table` contains a row referencing `table_id`.
3193+
fn st_event_table_has_row(datastore: &Locking, tx: &MutTxId, table_id: TableId) -> bool {
3194+
datastore
3195+
.iter_by_col_eq_mut_tx(
3196+
tx,
3197+
ST_EVENT_TABLE_ID,
3198+
ColList::from(StEventTableFields::TableId.col_id()),
3199+
&table_id.into(),
3200+
)
3201+
.expect("st_event_table lookup should succeed")
3202+
.next()
3203+
.is_some()
3204+
}
3205+
3206+
#[test]
3207+
fn test_alter_table_event_flag_non_event_to_event() -> ResultTest<()> {
3208+
// Create a non-event table.
3209+
let (datastore, tx, table_id) = setup_table()?;
3210+
commit(&datastore, tx)?;
3211+
3212+
// Flip `is_event` from `false` to `true`.
3213+
let mut tx = begin_mut_tx(&datastore);
3214+
assert_eq!(
3215+
tx.get_schema(table_id)
3216+
.map(|s| s.is_event)
3217+
.expect("schema should exist"),
3218+
false
3219+
);
3220+
assert!(
3221+
!st_event_table_has_row(&datastore, &tx, table_id),
3222+
"fresh non-event table must not have a row in `st_event_table`"
3223+
);
3224+
3225+
tx.alter_table_event_flag(table_id, true)?;
3226+
assert_matches!(
3227+
tx.pending_schema_changes(),
3228+
&[PendingSchemaChange::TableAlterEventFlag(t, false)] if t == table_id
3229+
);
3230+
commit(&datastore, tx)?;
3231+
3232+
// After commit, the schema should reflect the flipped flag
3233+
// and `st_event_table` should contain the row.
3234+
let tx = begin_mut_tx(&datastore);
3235+
assert_eq!(
3236+
tx.get_schema(table_id)
3237+
.map(|s| s.is_event)
3238+
.expect("schema should exist"),
3239+
true
3240+
);
3241+
assert!(
3242+
st_event_table_has_row(&datastore, &tx, table_id),
3243+
"after flipping to event, `st_event_table` should have the row"
3244+
);
3245+
Ok(())
3246+
}
3247+
3248+
#[test]
3249+
fn test_alter_table_event_flag_event_to_non_event() -> ResultTest<()> {
3250+
// Create an event table.
3251+
let (datastore, tx, table_id) = setup_event_table()?;
3252+
commit(&datastore, tx)?;
3253+
3254+
// Sanity check: `st_event_table` should have the row.
3255+
let mut tx = begin_mut_tx(&datastore);
3256+
assert_eq!(
3257+
tx.get_schema(table_id)
3258+
.map(|s| s.is_event)
3259+
.expect("schema should exist"),
3260+
true
3261+
);
3262+
assert!(
3263+
st_event_table_has_row(&datastore, &tx, table_id),
3264+
"event table should have a row in `st_event_table`"
3265+
);
3266+
3267+
// Flip `is_event` from `true` to `false`.
3268+
tx.alter_table_event_flag(table_id, false)?;
3269+
assert_matches!(
3270+
tx.pending_schema_changes(),
3271+
&[PendingSchemaChange::TableAlterEventFlag(t, true)] if t == table_id
3272+
);
3273+
commit(&datastore, tx)?;
3274+
3275+
// After commit, the schema should reflect the flipped flag
3276+
// and `st_event_table` should NOT contain the row.
3277+
let tx = begin_mut_tx(&datastore);
3278+
assert_eq!(
3279+
tx.get_schema(table_id)
3280+
.map(|s| s.is_event)
3281+
.expect("schema should exist"),
3282+
false
3283+
);
3284+
assert!(
3285+
!st_event_table_has_row(&datastore, &tx, table_id),
3286+
"after flipping to non-event, `st_event_table` should not have the row"
3287+
);
3288+
Ok(())
3289+
}
3290+
3291+
#[test]
3292+
fn test_alter_table_event_flag_rollback_reverts_live_state_and_st_event_table() -> ResultTest<()> {
3293+
// Create a non-event table.
3294+
let (datastore, tx, table_id) = setup_table()?;
3295+
commit(&datastore, tx)?;
3296+
3297+
// Start a new tx, flip, check pending change, then rollback.
3298+
let mut tx = begin_mut_tx(&datastore);
3299+
assert!(!st_event_table_has_row(&datastore, &tx, table_id));
3300+
3301+
tx.alter_table_event_flag(table_id, true)?;
3302+
assert_matches!(
3303+
tx.pending_schema_changes(),
3304+
&[PendingSchemaChange::TableAlterEventFlag(t, false)] if t == table_id
3305+
);
3306+
// The in-tx view must reflect the flip.
3307+
assert_eq!(
3308+
tx.get_schema(table_id)
3309+
.map(|s| s.is_event)
3310+
.expect("schema should exist"),
3311+
true
3312+
);
3313+
assert!(
3314+
st_event_table_has_row(&datastore, &tx, table_id),
3315+
"after flipping within the tx, `st_event_table` should have the row"
3316+
);
3317+
let _ = datastore.rollback_mut_tx(tx);
3318+
3319+
// After rollback, the schema and `st_event_table` should be back to pre-state.
3320+
let tx = begin_mut_tx(&datastore);
3321+
assert!(
3322+
tx.pending_schema_changes().is_empty(),
3323+
"rollback should clear pending schema changes"
3324+
);
3325+
assert_eq!(
3326+
tx.get_schema(table_id)
3327+
.map(|s| s.is_event)
3328+
.expect("schema should exist"),
3329+
false,
3330+
"rollback should revert `is_event` to its pre-tx value"
3331+
);
3332+
assert!(
3333+
!st_event_table_has_row(&datastore, &tx, table_id),
3334+
"rollback should revert the `st_event_table` row"
3335+
);
3336+
Ok(())
3337+
}
3338+
3339+
#[test]
3340+
fn test_alter_table_event_flag_idempotent_no_pending_change() -> ResultTest<()> {
3341+
// Create a non-event table.
3342+
let (datastore, tx, table_id) = setup_table()?;
3343+
commit(&datastore, tx)?;
3344+
3345+
// Flipping to the same value must be a no-op with no pending change.
3346+
let mut tx = begin_mut_tx(&datastore);
3347+
tx.alter_table_event_flag(table_id, false)?;
3348+
assert_matches!(tx.pending_schema_changes(), &[]);
3349+
Ok(())
3350+
}
3351+
31843352
#[test]
31853353
fn test_alter_table_row_type_rejects_some_bad_changes() -> ResultTest<()> {
31863354
let datastore = get_datastore()?;

0 commit comments

Comments
 (0)