Opened 4 months ago
Closed 4 months ago
#35652 closed Bug (duplicate)
Unapplying a data migration that removes data fails with relations
Reported by: | Timothy Schilling | Owned by: | |
---|---|---|---|
Component: | Migrations | Version: | 5.0 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I believe there's a bug in the plan logic for the MigrationExecutor
class. When a related model has been removed in an earlier migration, a later data migration that removes data from a model will attempt to run a query to collect/delete data from the related model. Unfortunately, that related model's table has been removed from the database.
Given the following app and model structure:
# App A class A(models.Model): name = models.CharField() # App B class B(models.Model): a = models.ForeignKey(A, on_delete=models.CASCADE)
- Create migrations
- Remove model
B
, create migration - Apply all migrations forwards
python manage.py makemigrations a --empty --name create_data
def create_data(apps, schema_editor): A = apps.get_model('a', 'A') A.objects.create(name='test') def remove_data(apps, schema_editor): A = apps.get_model('a', 'A') A.objects.filter(name='test').delete() # In migration class operations = [migrations.RunPython(create_data, remove_data)]
- Attempt to reverse back to A 0001 (
python manage.py migrate a 0001
)
It should break on the reverse of A 0002_create_data, because it will attempt to run a query to delete related B
instances, but that table has been removed.
It appears that the migration app state isn't removing the B
model.
I was able to track this down to at least the MigrationExecutor.migration_plan
method returning a full plan that puts the all app A migrations before the second app B migration.
I found that the RemoveModel operation mutates the state properly, but that MigrationExecturor._migrate_all_backwards
is using a different state from states[migration]
. That state is from the full_plan
that gets passed in which comes from MigrationExectutor.migration_plan
.
Interestingly enough, if we change this part of the code: https://github.com/django/django/blob/main/django/db/migrations/executor.py#L195-L214
To
for migration, _ in full_plan: if not migrations_to_run: # We remove every migration that we applied from this set so # that we can bail out once the last migration has been applied # and don't always run until the very end of the migration # process. break if migration not in migrations_to_run and migration in applied_migrations: # Only mutate the state if the migration is actually applied # to make sure the resulting state doesn't include changes # from unrelated migrations. migration.mutate_state(state, preserve=False) for migration, _ in full_plan: if not migrations_to_run: # We remove every migration that we applied from this set so # that we can bail out once the last migration has been applied # and don't always run until the very end of the migration # process. break if migration in migrations_to_run: if "apps" not in state.__dict__: state.apps # Render all -- performance critical # The state before this migration states[migration] = state # The old state keeps as-is, we continue with the new state state = migration.mutate_state(state, preserve=True) migrations_to_run.remove(migration)
It unapply successfully because all the applied_migrations
are mutating the state before the migrations_to_run
stores any state.
Note: I haven't reproduced this on a fresh project.
Change History (2)
comment:1 by , 4 months ago
comment:2 by , 4 months ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
Ack. I looked at that one, but clearly not close enough. I'm pretty sure it's a duplicate of 33586 since the test case is literally the same.
Hi Tim thanks for the report, are you able to confirm whether this is a possible duplicate of #33586? Cheers 👍