Skip to content

Fix primary key migration causing stale schema (#3934)#4666

Open
clockwork-labs-bot wants to merge 1 commit intomasterfrom
bot/fix-primary-key-migration
Open

Fix primary key migration causing stale schema (#3934)#4666
clockwork-labs-bot wants to merge 1 commit intomasterfrom
bot/fix-primary-key-migration

Conversation

@clockwork-labs-bot
Copy link
Collaborator

Summary

Fixes #3934. Removing or changing a #[primary_key] annotation succeeds on the first re-publish, but the stored schema's table_primary_key field in st_table is never updated. On the next publish, check_compatible fails with:

Primary key mismatch: self.primary_key: Some(ColId(0)), def.primary_key: None

Root Cause

auto_migrate_table handles removing the PK's index and unique constraint, but there was no AutoMigrateStep to update the table_primary_key field in the system table.

Fix

  • Add AutoMigrateStep::ChangePrimaryKey variant to the auto-migration planner
  • Detect old.primary_key != new.primary_key in auto_migrate_table and emit the step
  • Add alter_table_primary_key to the datastore layer (mut_tx, datastore, relational_db) with proper rollback support via PendingSchemaChange::TableAlterPrimaryKey
  • Handle the step in auto_migrate_database
  • Add migration plan formatter support (termcolor output)

Repro Script

# 1. Publish with primary key
spacetime publish repro --yes  # table has #[primary_key] on name

# 2. Remove primary key — succeeds
spacetime publish repro --yes  # removed #[primary_key]

# 3. Any change — CRASHES
spacetime publish repro --yes  # Primary key mismatch error

Tests

  • Unit test in crates/core/src/db/update.rs: Reproduces the exact three-publish sequence from the issue (create table with PK → remove PK → trivial change)
  • Smoketest in crates/smoketests/tests/smoketests/auto_migration.rs: Full end-to-end publish flow exercising the same scenario

Files Changed

File Change
crates/schema/src/auto_migrate.rs Add ChangePrimaryKey variant + detection
crates/schema/src/auto_migrate/formatter.rs Format the new step
crates/schema/src/auto_migrate/termcolor_formatter.rs Colored output
crates/datastore/src/locking_tx_datastore/tx_state.rs TableAlterPrimaryKey pending change
crates/datastore/src/locking_tx_datastore/mut_tx.rs alter_table_primary_key
crates/datastore/src/locking_tx_datastore/datastore.rs Expose through datastore
crates/datastore/src/locking_tx_datastore/committed_state.rs Rollback support
crates/core/src/db/relational_db.rs Expose through RelationalDB
crates/core/src/db/update.rs Handle step + unit test
crates/smoketests/.../auto_migration.rs Smoketest

Removing or changing a `#[primary_key]` annotation would succeed on the
first re-publish, but the stored schema's `table_primary_key` field in
`st_table` was never updated. On the next publish, `check_compatible`
would see the mismatch between the stale stored schema and the old module
def, failing with:

  Primary key mismatch: self.primary_key: Some(ColId(0)), def.primary_key: None

Root cause: `auto_migrate_table` handled removing the PK's index and
unique constraint, but there was no `AutoMigrateStep` to update the
`table_primary_key` field in the system table.

Fix:
- Add `AutoMigrateStep::ChangePrimaryKey` variant
- Detect primary key changes in `auto_migrate_table`
- Add `alter_table_primary_key` to the datastore layer (mut_tx,
  datastore, relational_db) with proper rollback support
- Handle the step in `auto_migrate_database`
- Add migration plan formatter support

Tests:
- Unit test in `crates/core/src/db/update.rs` reproducing the exact
  three-publish sequence from the issue
- Smoketest in `crates/smoketests` exercising the full publish flow

Closes #3934
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.

Removing or changing a primary_key table annotation breaks module updating

1 participant