Fix primary key migration causing stale schema (#3934)#4666
Open
clockwork-labs-bot wants to merge 1 commit intomasterfrom
Open
Fix primary key migration causing stale schema (#3934)#4666clockwork-labs-bot wants to merge 1 commit intomasterfrom
clockwork-labs-bot wants to merge 1 commit intomasterfrom
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3934. Removing or changing a
#[primary_key]annotation succeeds on the first re-publish, but the stored schema'stable_primary_keyfield inst_tableis never updated. On the next publish,check_compatiblefails with:Root Cause
auto_migrate_tablehandles removing the PK's index and unique constraint, but there was noAutoMigrateStepto update thetable_primary_keyfield in the system table.Fix
AutoMigrateStep::ChangePrimaryKeyvariant to the auto-migration plannerold.primary_key != new.primary_keyinauto_migrate_tableand emit the stepalter_table_primary_keyto the datastore layer (mut_tx,datastore,relational_db) with proper rollback support viaPendingSchemaChange::TableAlterPrimaryKeyauto_migrate_databaseRepro Script
Tests
crates/core/src/db/update.rs: Reproduces the exact three-publish sequence from the issue (create table with PK → remove PK → trivial change)crates/smoketests/tests/smoketests/auto_migration.rs: Full end-to-end publish flow exercising the same scenarioFiles Changed
crates/schema/src/auto_migrate.rsChangePrimaryKeyvariant + detectioncrates/schema/src/auto_migrate/formatter.rscrates/schema/src/auto_migrate/termcolor_formatter.rscrates/datastore/src/locking_tx_datastore/tx_state.rsTableAlterPrimaryKeypending changecrates/datastore/src/locking_tx_datastore/mut_tx.rsalter_table_primary_keycrates/datastore/src/locking_tx_datastore/datastore.rscrates/datastore/src/locking_tx_datastore/committed_state.rscrates/core/src/db/relational_db.rscrates/core/src/db/update.rscrates/smoketests/.../auto_migration.rs