Skip to content

feat: Allow reordering columns of empty tables during auto-migration - #5694

Open
Ludv1gL wants to merge 1 commit into
clockworklabs:masterfrom
Ludv1gL:feat/reorder-empty-tables
Open

feat: Allow reordering columns of empty tables during auto-migration#5694
Ludv1gL wants to merge 1 commit into
clockworklabs:masterfrom
Ludv1gL:feat/reorder-empty-tables

Conversation

@Ludv1gL

@Ludv1gL Ludv1gL commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Description of Changes

Follow-up to #4593 (allow dropping empty tables): this allows reordering the columns of an empty table during an automatic migration, and implements the AutoMigratePrecheck::CheckTableEmpty refactor requested in the review of #4875 (#4875 (comment)).

The justification is the one already encoded for event tables in auto_migrate_table: a table with no resident rows has no data to migrate, so a layout-incompatible reschema is safe. Previously, reordering columns always failed with AutoMigrateError::ReorderTable, and the only way to reorder was to drop the (empty) table in one publish and re-add it in a second one, losing its identity.

Design

Planner (crates/schema/src/auto_migrate.rs):

  • New AutoMigratePrecheck::CheckTableEmpty(table): validates at execution time — before any mutations — that the table has no rows. Emitted for RemoveTable (moving the check out of the step executor, per the feat: Allow toggling event flag during auto-migration for empty tables #4875 review) and for the new reschema step.
  • New AutoMigrateStep::ReschemaEmptyTable(table): a layout-incompatible reschema of a non-event table. Declared between the Remove* and Add* variants so that step sorting brackets it correctly with the sub-object steps below.
  • Column position changes on non-event tables now set a columns_reordered flag (4th slot of the ArrayMonoid) instead of erroring; the table then gets CheckTableEmpty + ReschemaEmptyTable + DisconnectAllUsers. AutoMigrateError::ReorderTable is removed. Type changes must still be upgradable, and the column add/remove rules are unchanged.
  • Sub-objects on a moved column follow existing machinery: indexes and sequences already diff to Remove*+Add* when their column ids change (the stale "column ids are not changed in an automigrate" comment is updated), and ChangePrimaryKey covers primary key moves. Changed unique constraints previously always errored with ChangeUniqueConstraint; they now diff to RemoveConstraint+AddConstraint, but only when the owning table has a ReschemaEmptyTable step — on an empty table re-adding the constraint is trivially valid. All other tables keep the error.

Executor (crates/engine/src/update.rs):

  • CheckTableEmpty precheck: fails the publish with a clear message (table name + row count + how to fix) before any mutations.
  • ReschemaEmptyTable step: rebuilds the table's column layout from the new definition via a new RelationalDB::alter_empty_table_row_type.
  • The RemoveTable arm's inline row-count check is removed (subsumed by the precheck).

Datastore:

  • MutTxId::alter_event_table_row_type is refactored into a general alter_empty_table_row_type (runtime emptiness check kept as a backstop); the event variant remains as a wrapper that keeps its is-an-event-table sanity check.
  • PendingSchemaChange::ReschemaEventTableReschemaEmptyTable and TableError::EventTableNotEmptyTableNotEmpty, since both now cover non-event tables too. The rollback path is unchanged — its safety argument ("the commit table is empty") already holds.
  • Replay fix (replay.rs, st_column_changed): replay applied non-event st_column changes with the layout-checked change_columns_to, which rejects a reorder — so a database that committed a reorder would fail to reopen from the commitlog. Replay now takes the unchecked empty-table path whenever the table has no rows at that point in the log, mirroring the condition under which such a change can be committed.

Table (crates/table):

  • Table::change_columns_of_empty_table_to called set_pages(Vec::new(), ..), whose Pages::set_contents debug-asserts that no pages are present — but a table that previously contained rows keeps its (row-empty) pages, so the reschema would panic in debug builds. New Pages::reset drops residual pages first. This was unreachable for event tables (never any resident rows), so it surfaces only now.

Behavior notes

  • Removing a non-empty table now fails in the precheck pass (all validation before any mutations) rather than at the RemoveTable step; the message still names the table and explains the fix.
  • Reordering such that a scheduled table's scheduled_at/primary key columns move diffs to RemoveSchedule/AddSchedule, which remain unimplemented — such a migration still fails cleanly (pre-existing limitation, unchanged by this PR).
  • Docs: the automatic-migrations page is updated; it also still listed table removal as forbidden, which feat: Allow dropping empty tables during auto-migration #4593 made legal for empty tables, so that entry is corrected as well.

API and ABI breaking changes

None on the module/client ABI. AutoMigrateError::ReorderTable is removed and TableError::EventTableNotEmpty / PendingSchemaChange::ReschemaEventTable are renamed (crate-internal APIs).

Expected complexity level and risk

2 — the mechanism reuses the event-table reschema path end to end; the new surface is the precheck plumbing, the replay condition, and the constraint carve-out, each covered by tests.

Testing

  • Planner: reorder_columns_of_empty_table asserts the exact precheck and step lists (including sub-object Remove*/Add* bracketing and DisconnectAllUsers).
  • Executor: reorder_empty_table_succeeds (including the previously-contained-rows/residual-pages case; asserts new schema order, pk, exact pending-schema-change shape, and post-migration insert/read-back), reorder_nonempty_table_fails (clear error, pending_schema_changes() == [], old layout and data intact).
  • Replay: replay_reordered_table_no_snapshot / replay_reordered_table_after_snapshot — a commitlog containing old-layout writes, the reorder, and new-layout writes replays correctly.
  • Full test suites of spacetimedb-schema, spacetimedb-table, spacetimedb-datastore (--features test), and spacetimedb-engine pass locally.

A table with no resident rows has no data to migrate, so a
layout-incompatible reschema is safe — the same justification already
encoded for event tables. Reordering previously always failed with
`AutoMigrateError::ReorderTable`.

- Add `AutoMigratePrecheck::CheckTableEmpty`, validating emptiness
  before any mutations (as requested in the clockworklabs#4875 review), and use it
  for `RemoveTable` and the new step.
- Add `AutoMigrateStep::ReschemaEmptyTable`; the planner emits it (plus
  the precheck and `DisconnectAllUsers`) for column position changes on
  non-event tables. Sub-objects on moved columns are removed and
  re-added; changed unique constraints are allowed (Remove+Add) only
  under this step.
- Generalize `alter_event_table_row_type` into
  `alter_empty_table_row_type`; rename
  `PendingSchemaChange::ReschemaEventTable` -> `ReschemaEmptyTable` and
  `TableError::EventTableNotEmpty` -> `TableNotEmpty`.
- Fix replay: `st_column_changed` now uses the unchecked empty-table
  path when the table is empty at that point in the log, so a committed
  reorder replays instead of failing layout checks on reopen.
- Fix `change_columns_of_empty_table_to` on tables that previously
  contained rows: drop residual pages before `set_pages`
  (new `Pages::reset`).
- Update the automatic-migrations docs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants