Skip to content

Multi-operation migration support for drop_table operations #587

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

Merged
merged 9 commits into from
Jan 13, 2025

Conversation

andrew-farries
Copy link
Collaborator

Ensure that drop_table operations can be used as part of multi-operation migrations:

Add testcases for:

  • Create table, rename table, drop table:
    • Make OpRenameTable.Validate update the in-memory schema so that the following operation can validate using the new name of the table
    • MakeOpDropTable soft delete the table from the virtual schema, so that OpDropTable.Rollback can un-drop the table, making it visible to preceding operations in the table when the migration is rolled back.
    • Make OpRenameTable undo the table rename, so that the table is visible under its old name for preceding operations in the rollback.
  • Drop table, rename table and drop table, drop table:
    • Make OpDropTable.Validate remove the table from the virtual schema so that validation of the subsequent operation fails.

Theses changes ensure that drop_table operations can be used as part of multi-operation migrations.

Part of #239

@exekias
Copy link
Member

exekias commented Jan 13, 2025

what will happen if you do a drop_table + create_table with the same name? I guess this one is tricky, but we will see others like this probably 🤔

@andrew-farries andrew-farries force-pushed the op-drop-table-multi-op-tests branch from c32fd12 to f31ba70 Compare January 13, 2025 10:46
@andrew-farries
Copy link
Collaborator Author

what will happen if you do a drop_table + create_table with the same name? I guess this one is tricky, but we will see others like this probably 🤔

This won't work currently. I've described the problem here: #239 (comment).

@exekias
Copy link
Member

exekias commented Jan 13, 2025

I guess it should be fine as long as we are able to detect this situation and prevent the migration from moving forward, is that the case already?

Base automatically changed from rollback-ops-in-reverse-order to main January 13, 2025 13:50
Add a `Deleted` field to the `Table` struct in order to
allow `OpDeleteTable` to soft delete tables in the virtual schema.

Update `RemoveTable` to set the `Deleted` field to `true` instead of
removing a table from the `Tables` map.

Add an `UnRemoveTable` method to allow tables to be unremoved. This can
be used by `OpDeleteTable` to rollback the effects of a table deletion
in the virtual schema.

When looking up a table in the schema, ignore tables that have been
marked as deleted.

When creating views in the new schema, skip tables that have been marked
as deleted.
So that `Rollback` of earlier operations in the migration see the table
with the name that they expect.
So that the table is visible to `Rollback` methods of operations earlier
in the migration.
Add a test that checks that dropping a table then renaming the table
fails to validate.
Remove (soft-delete) the dropped table from the schema during
`OpDropTable.Validate` so that validation of any subsequent operation in
the same migration fails if it attempts to reference the removed table.
@andrew-farries andrew-farries force-pushed the op-drop-table-multi-op-tests branch from f31ba70 to 1a5b4a5 Compare January 13, 2025 14:05
@andrew-farries
Copy link
Collaborator Author

what will happen if you do a drop_table + create_table with the same name? I guess this one is tricky, but we will see others like this probably 🤔

I thought some more about this and I think the 'create table, drop table, create table' case is now covered by #589

@andrew-farries andrew-farries merged commit 5a6f508 into main Jan 13, 2025
28 checks passed
@andrew-farries andrew-farries deleted the op-drop-table-multi-op-tests branch January 13, 2025 16:46
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