Automigrate views using new schema for backing table

joshua-spacetime · joshua-spacetime · commit bfc200307ff1 · 2026-06-24T16:02:03.000-07:00
diff --git a/crates/core/src/host/host_controller.rs b/crates/core/src/host/host_controller.rs
@@ -1276,13 +1276,18 @@ impl Host {
         );
 
         let res = match ponder_migrate(&old_module.module_def, &module_def) {
-            Ok(plan) => MigratePlanResult::Success {
-                old_module_hash: old_module.module_hash,
-                new_module_hash: program.hash,
-                breaks_client: plan.breaks_client(),
-                plan: plan.pretty_print(style)?.into(),
-                major_version_upgrade,
-            },
+            Ok(mut plan) => {
+                let stdb = self.replica_ctx.relational_db();
+                db::update::add_view_backing_table_recreate_steps(stdb, &mut plan)?;
+
+                MigratePlanResult::Success {
+                    old_module_hash: old_module.module_hash,
+                    new_module_hash: program.hash,
+                    breaks_client: plan.breaks_client(),
+                    plan: plan.pretty_print(style)?.into(),
+                    major_version_upgrade,
+                }
+            }
             Err(e) => MigratePlanResult::AutoMigrationError {
                 error: e,
                 major_version_upgrade,
diff --git a/crates/core/src/host/wasm_common/module_host_actor.rs b/crates/core/src/host/wasm_common/module_host_actor.rs
@@ -49,7 +49,7 @@ use spacetimedb_lib::{bsatn, http as st_http, ConnectionId, Hash, ProductType, R
 use spacetimedb_primitives::{HttpHandlerId, ProcedureId, TableId, ViewFnPtr, ViewId};
 use spacetimedb_sats::algebraic_type::fmt::fmt_algebraic_type;
 use spacetimedb_sats::{AlgebraicType, AlgebraicTypeRef, Deserialize, ProductValue, Typespace, WithTypespace};
-use spacetimedb_schema::auto_migrate::{MigratePlan, MigrationPolicy, MigrationPolicyError};
+use spacetimedb_schema::auto_migrate::{ponder_migrate, MigratePlan, MigrationPolicy, MigrationPolicyError};
 use spacetimedb_schema::def::deserialize::FunctionDef;
 use spacetimedb_schema::def::{ModuleDef, ViewDef};
 use spacetimedb_schema::identifier::Identifier;
@@ -641,21 +641,26 @@ impl InstanceCommon {
         let system_logger = replica_ctx.logger.system_logger();
         let stdb = &replica_ctx.relational_db();
 
-        let plan: MigratePlan = match policy.try_migrate(
+        let mut plan: MigratePlan = match ponder_migrate(&old_module_info.module_def, &self.info.module_def) {
+            Ok(plan) => plan,
+            Err(e) => return Ok(UpdateDatabaseResult::AutoMigrateError(e.into())),
+        };
+
+        if let Err(e) = crate::db::update::add_view_backing_table_recreate_steps(stdb, &mut plan) {
+            return Ok(UpdateDatabaseResult::ErrorExecutingMigration(e));
+        }
+
+        if let Err(e) = policy.permits_migrate_plan(
             self.info.database_identity,
             old_module_info.module_hash,
-            &old_module_info.module_def,
             self.info.module_hash,
-            &self.info.module_def,
+            &plan,
         ) {
-            Ok(plan) => plan,
-            Err(e) => {
-                return match e {
-                    MigrationPolicyError::AutoMigrateFailure(e) => Ok(UpdateDatabaseResult::AutoMigrateError(e.into())),
-                    _ => Ok(UpdateDatabaseResult::ErrorExecutingMigration(e.into())),
-                }
-            }
-        };
+            return match e {
+                MigrationPolicyError::AutoMigrateFailure(e) => Ok(UpdateDatabaseResult::AutoMigrateError(e.into())),
+                _ => Ok(UpdateDatabaseResult::ErrorExecutingMigration(e.into())),
+            };
+        }
 
         let program_hash = program.hash;
         let host_type = HostType::from(program.kind);
diff --git a/crates/engine/src/update.rs b/crates/engine/src/update.rs
@@ -1,13 +1,16 @@
 use super::relational_db::RelationalDB;
 use crate::sql::rls::RowLevelExpr;
 use anyhow::Context;
-use spacetimedb_datastore::locking_tx_datastore::MutTxId;
+use spacetimedb_datastore::execution_context::Workload;
+use spacetimedb_datastore::locking_tx_datastore::state_view::StateView;
+use spacetimedb_datastore::locking_tx_datastore::{MutTxId, TxId};
+use spacetimedb_datastore::system_tables::{StViewFields, StViewRow, ST_VIEW_ID};
 use spacetimedb_lib::db::auth::StTableType;
 use spacetimedb_lib::identity::AuthCtx;
 use spacetimedb_lib::AlgebraicValue;
 use spacetimedb_primitives::{ColSet, TableId};
-use spacetimedb_schema::auto_migrate::{AutoMigratePlan, ManualMigratePlan, MigratePlan};
-use spacetimedb_schema::def::{ModuleDef, TableDef, ViewDef};
+use spacetimedb_schema::auto_migrate::{AutoMigratePlan, AutoMigrateStep, ManualMigratePlan, MigratePlan};
+use spacetimedb_schema::def::{ModuleDef, ModuleDefLookup, TableDef, ViewDef};
 use spacetimedb_schema::schema::{
     column_schemas_from_defs, ConstraintSchema, IndexSchema, Schema, SequenceSchema, TableSchema,
 };
@@ -26,6 +29,100 @@ pub enum UpdateResult {
     EvaluateSubscribedViews,
 }
 
+type ViewKey<'a> = <ViewDef as ModuleDefLookup>::Key<'a>;
+
+/// Force view recreation when the stored backing table's row layout no longer
+/// matches the layout expected by the new module definition.
+pub fn add_view_backing_table_recreate_steps<'def>(
+    stdb: &RelationalDB,
+    plan: &mut MigratePlan<'def>,
+) -> anyhow::Result<()> {
+    let MigratePlan::Auto(plan) = plan else {
+        return Ok(());
+    };
+
+    stdb.with_read_only(Workload::Internal, |tx| -> anyhow::Result<()> {
+        let mut changed = false;
+
+        for view in plan.new.views() {
+            let key = view.key();
+
+            // A normal view schema change is already handled by the public migration plan.
+            // This pass only upgrades stale backing tables.
+            if view_recreate_is_planned(&plan.steps, key) {
+                continue;
+            }
+
+            let Some(table_id) = view_backing_table_id(tx, view)? else {
+                continue;
+            };
+
+            let actual = stdb.schema_for_table(tx, table_id)?;
+            let expected = TableSchema::from_view_def_for_datastore(plan.new, view);
+
+            if !view_backing_row_layout_changed(&actual, &expected) {
+                continue;
+            }
+
+            // UpdateView would recompute rows into the stale physical layout.
+            // Dropping and recreating the view rebuilds the backing table.
+            replace_view_update_with_recreate_steps(&mut plan.steps, key);
+            changed = true;
+        }
+
+        if changed {
+            plan.steps.sort();
+        }
+
+        Ok(())
+    })
+}
+
+fn view_recreate_is_planned<'def>(steps: &[AutoMigrateStep<'def>], key: ViewKey<'def>) -> bool {
+    steps.iter().any(|step| {
+        matches!(
+            step,
+            AutoMigrateStep::AddView(existing) | AutoMigrateStep::RemoveView(existing) if *existing == key
+        )
+    })
+}
+
+fn view_backing_table_id(tx: &mut TxId, view: &ViewDef) -> anyhow::Result<Option<TableId>> {
+    let view_name = AlgebraicValue::from(<Box<str>>::from(&*view.name));
+    let Some(row) = tx
+        .iter_by_col_eq(ST_VIEW_ID, StViewFields::ViewName, &view_name)?
+        .next()
+    else {
+        return Ok(None);
+    };
+
+    Ok(StViewRow::try_from(row)?.table_id)
+}
+
+fn view_backing_row_layout_changed(actual: &TableSchema, expected: &TableSchema) -> bool {
+    actual.row_type != expected.row_type
+}
+
+fn replace_view_update_with_recreate_steps<'def>(steps: &mut Vec<AutoMigrateStep<'def>>, key: ViewKey<'def>) {
+    steps.retain(|step| {
+        !matches!(
+            step,
+            AutoMigrateStep::UpdateView(existing) | AutoMigrateStep::ChangeAccess(existing) if *existing == key
+        )
+    });
+    steps.extend([AutoMigrateStep::RemoveView(key), AutoMigrateStep::AddView(key)]);
+    ensure_disconnect_all_users(steps);
+}
+
+fn ensure_disconnect_all_users(steps: &mut Vec<AutoMigrateStep<'_>>) {
+    if !steps
+        .iter()
+        .any(|step| matches!(step, AutoMigrateStep::DisconnectAllUsers))
+    {
+        steps.push(AutoMigrateStep::DisconnectAllUsers);
+    }
+}
+
 /// Update the database according to the migration plan.
 ///
 /// The update is performed within the transactional context `tx`.
@@ -575,6 +672,95 @@ mod test {
             .expect("should be a valid module definition")
     }
 
+    fn view_module() -> ModuleDef {
+        let mut builder = RawModuleDefV10Builder::new();
+        let return_type_ref = builder.add_algebraic_type(
+            [],
+            "my_view_return_type",
+            AlgebraicType::product([("a", AlgebraicType::U64)]),
+            true,
+        );
+        builder.add_view(
+            "my_view",
+            0,
+            true,
+            true,
+            ProductType::unit(),
+            AlgebraicType::array(AlgebraicType::Ref(return_type_ref)),
+        );
+        builder.add_view_primary_key("my_view", ["a"]);
+        builder
+            .finish()
+            .try_into()
+            .expect("should be a valid module definition")
+    }
+
+    fn old_view_backing_schema(module_def: &ModuleDef) -> TableSchema {
+        let view = module_def.view("my_view").unwrap();
+        let mut schema = TableSchema::from_view_def_for_datastore(module_def, view);
+
+        schema.columns.remove(0);
+        for (pos, col) in schema.columns.iter_mut().enumerate() {
+            col.col_pos = pos.into();
+        }
+        schema.indexes.clear();
+        schema.constraints.clear();
+        schema.reset();
+
+        schema
+    }
+
+    fn create_backing_table(stdb: &TestDB, schema: TableSchema) -> anyhow::Result<()> {
+        let mut tx = begin_mut_tx(stdb);
+        stdb.create_table(&mut tx, schema)?;
+        stdb.commit_tx(tx)?;
+        Ok(())
+    }
+
+    #[test]
+    fn stale_view_backing_schema_forces_remove_add_view() -> anyhow::Result<()> {
+        let stdb = TestDB::durable()?;
+        let module_def = view_module();
+        create_backing_table(&stdb, old_view_backing_schema(&module_def))?;
+
+        let mut plan = ponder_migrate(&module_def, &module_def)?;
+        add_view_backing_table_recreate_steps(&stdb, &mut plan)?;
+
+        let MigratePlan::Auto(plan) = &plan else {
+            panic!("expected auto migration");
+        };
+        let my_view = module_def.view("my_view").unwrap().key();
+        assert!(plan.steps.contains(&AutoMigrateStep::RemoveView(my_view)));
+        assert!(plan.steps.contains(&AutoMigrateStep::AddView(my_view)));
+        assert!(plan.steps.contains(&AutoMigrateStep::DisconnectAllUsers));
+        assert!(!plan.steps.contains(&AutoMigrateStep::UpdateView(my_view)));
+
+        Ok(())
+    }
+
+    #[test]
+    fn current_view_backing_schema_does_not_force_remove_add_view() -> anyhow::Result<()> {
+        let stdb = TestDB::durable()?;
+        let module_def = view_module();
+        let view = module_def.view("my_view").unwrap();
+        let backing_schema = TableSchema::from_view_def_for_datastore(&module_def, view);
+        create_backing_table(&stdb, backing_schema)?;
+
+        let mut plan = ponder_migrate(&module_def, &module_def)?;
+        add_view_backing_table_recreate_steps(&stdb, &mut plan)?;
+
+        let MigratePlan::Auto(plan) = &plan else {
+            panic!("expected auto migration");
+        };
+        let my_view = view.key();
+        assert!(plan.steps.contains(&AutoMigrateStep::UpdateView(my_view)));
+        assert!(!plan.steps.contains(&AutoMigrateStep::RemoveView(my_view)));
+        assert!(!plan.steps.contains(&AutoMigrateStep::AddView(my_view)));
+        assert!(!plan.steps.contains(&AutoMigrateStep::DisconnectAllUsers));
+
+        Ok(())
+    }
+
     enum TakeSnapshot {
         None,
         BeforeAutomigration,
diff --git a/crates/schema/src/auto_migrate.rs b/crates/schema/src/auto_migrate.rs
@@ -131,14 +131,25 @@ impl MigrationPolicy {
         new_module_def: &'def ModuleDef,
     ) -> anyhow::Result<MigratePlan<'def>, MigrationPolicyError> {
         let plan = ponder_migrate(old_module_def, new_module_def).map_err(MigrationPolicyError::AutoMigrateFailure)?;
+        self.permits_migrate_plan(database_identity, old_module_hash, new_module_hash, &plan)?;
+        Ok(plan)
+    }
 
+    /// Validate an already-generated migration plan under this policy.
+    pub fn permits_migrate_plan(
+        &self,
+        database_identity: Identity,
+        old_module_hash: spacetimedb_lib::Hash,
+        new_module_hash: spacetimedb_lib::Hash,
+        plan: &MigratePlan<'_>,
+    ) -> anyhow::Result<(), MigrationPolicyError> {
         let token = MigrationToken {
             database_identity,
             old_module_hash,
             new_module_hash,
         };
-        self.permits_plan(&plan, &token)?;
-        Ok(plan)
+        self.permits_plan(plan, &token)?;
+        Ok(())
     }
 }
 
