[Django] #35652: Unapplying a data migration that removes data fails with relations

15 views
Skip to first unread message

Django

unread,
Aug 1, 2024, 11:24:43 AM8/1/24
to django-...@googlegroups.com
#35652: Unapplying a data migration that removes data fails with relations
-----------------------------------+--------------------------------------
Reporter: Timothy Schilling | Type: Bug
Status: new | Component: Migrations
Version: 5.0 | Severity: Normal
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------------------------
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)
}}}



1. Create migrations
2. Remove model `B`, create migration
3. Apply all migrations forwards
4. `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)]
}}}


5. 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/35652>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Aug 1, 2024, 11:52:56 AM8/1/24
to django-...@googlegroups.com
#35652: Unapplying a data migration that removes data fails with relations
-----------------------------------+--------------------------------------
Reporter: Timothy Schilling | Owner: (none)
Type: Bug | Status: new
Component: Migrations | Version: 5.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------------------------
Comment (by David Sanders):

Hi Tim thanks for the report, are you able to confirm whether this is a
possible duplicate of #33586? Cheers 👍
--
Ticket URL: <https://code.djangoproject.com/ticket/35652#comment:1>

Django

unread,
Aug 1, 2024, 12:24:50 PM8/1/24
to django-...@googlegroups.com
#35652: Unapplying a data migration that removes data fails with relations
-----------------------------------+--------------------------------------
Reporter: Timothy Schilling | Owner: (none)
Type: Bug | Status: closed
Component: Migrations | Version: 5.0
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------------------------
Changes (by Timothy Schilling):

* resolution: => duplicate
* status: new => closed

Comment:

Ack. I looked at that one, but clearly not close enough. I'm pretty sure
it's a duplicate of [https://code.djangoproject.com/ticket/33586 33586]
since the test case is literally the same.
--
Ticket URL: <https://code.djangoproject.com/ticket/35652#comment:2>
Reply all
Reply to author
Forward
0 new messages