Skip to content

Implemented auto migration support for appending new columns #2934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

egormanga
Copy link

@egormanga egormanga commented Jul 11, 2025

Description of Changes

Previously, you couldn't add new columns at the end of a table without clearing the database, even though this should not require a manual schema change.

API and ABI breaking changes

None.

Expected complexity level and risk

4:

This small change might easily be totally incorrect, possibly overwrite some unrelated data (although all existing tests pass) or collapse the Universe. Need advice from people who understand the inner workings better than your humble.

In particular, how do schema rollbacks handle this?
For adding variants (#2874), new logic had to be introduced. As I suppose, in this case the underlying data store gets duplicated and refilled entirely, rather than altered in-place? If not, perhaps PendingSchemaChange needs to be extended as well, with all the hassle this implies.

Also, I'm not sure about using column_id as position, but the comment in a latter hand is somewhat reassuring.

Testing

  • Manual testing on a mid-sized database with multiple tables
  • Needs unit tests

@egormanga
Copy link
Author

egormanga commented Jul 11, 2025

Alright, adding a column mid-table instead of appending now causes this:

ERROR client-api/src/lib.rs:377:
  internal error: failed to get or launch module host:
    TableError: Table with ID `0` not found in `st_table`.:
      Table with ID `0` not found in `st_table`.

(formatting mine.)

Rollback after that seems to work, though.
Upd: nah it's all gone, the entire db content.

Thus I conclude that using column_id as position does not work. Or, put another way, we need to add a check that all previous columns are kept the same, because adding one in between them technically causes the last one to appear as an appended, if we ignore (as currently) the preceding types.
Need help here.

P.S. There must be a bug in the TableError formatting handler, causing the message to repeat twice.

@egormanga egormanga force-pushed the auto-migration-append-columns branch from 603cb25 to c0ea9c9 Compare July 11, 2025 21:42
@egormanga
Copy link
Author

Further clues suggest that I somehow suppress generating the AutoMigrateError::ReorderTable when adding a column in the middle, although it's not clear to me, in which way I do so.

@egormanga
Copy link
Author

Seems to not be working at all, as the next publishing (no matter the changes) causes:

Error: Failed to create or update the database: Column count mismatch: expected self.columns.len() == def.columns.len():
   self.columns.len(): 15
   def.columns.len(): 17

Apparently, somewhere the schema is not getting updated on the migration.

@gefjon
Copy link
Contributor

gefjon commented Jul 15, 2025

Thank you for your contribution! We really appreciate your dedication to SpacetimeDB and your willingness to contribute. We do not think adding columns should be a fully automatic migration. Automatic migrations should only be used for changes which have a single clear, unambiguous correct transformation from the old data to the new data.

Hypothetically, the database could choose some transform for any proposed schema change, and so we could expand automatic migration support to allow literally any schema change. However, we fear that this would lead to unexpected behavior (See this discussion for an example of our concerns).

We currently have a couple members of the team designing and implementing what we're calling "supervised migrations," which will require explicit approval and some guidance from the user (in this case a default value to populate the new column with). I'm going to close this PR to avoid creating conflicts with their work.

@gefjon gefjon closed this Jul 15, 2025
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