Opened 16 months ago
Closed 4 months ago
#35595 closed Bug (fixed)
Generated migration for removing related models can't migrate backwards
| Reported by: | Timothy Schilling | Owned by: | wookkl |
|---|---|---|---|
| Component: | Migrations | Version: | 5.0 |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
The migration that's created when removing two related models, results in a migration that can't be migrated backwards. However, this is only the case when the model with the foreign key has a Meta.indexes specified that references the foreign key field.
Given the following models:
class Animal(models.Model):
name = models.CharField(max_length=100)
class Dog(models.Model):
name = models.CharField(max_length=100)
animal = models.ForeignKey(Animal, on_delete=models.CASCADE)
class Meta:
indexes = [models.Index(fields=("animal", "name"))]
Doing the following results in the error at the end.
python manage.py makemigrations [app]-> 0001_initial.py (in my test it was the second migration, in case you can't reproduce it)- Remove both models
python manage.py makemigrations [app]-> 0002_remove.pypython manage.py sqlmigrate [app] 0002 --backwards
Error:
❯ ./manage.py sqlmigrate endpoint 0003 --backwards
Traceback (most recent call last):
File "site-packages/django/db/models/options.py", line 681, in get_field
return self.fields_map[field_name]
~~~~~~~~~~~~~~~^^^^^^^^^^^^
KeyError: 'animal'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/Users/schillingt/Projects/aspiredu/./manage.py", line 11, in <module>
execute_from_command_line(sys.argv)
File "site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line
utility.execute()
File "site-packages/django/core/management/__init__.py", line 436, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "site-packages/django/core/management/base.py", line 413, in run_from_argv
self.execute(*args, **cmd_options)
File "site-packages/django/core/management/commands/sqlmigrate.py", line 38, in execute
return super().execute(*args, **options)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "site-packages/django/core/management/base.py", line 459, in execute
output = self.handle(*args, **options)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "site-packages/django/core/management/commands/sqlmigrate.py", line 80, in handle
sql_statements = loader.collect_sql(plan)
^^^^^^^^^^^^^^^^^^^^^^^^
File "site-packages/django/db/migrations/loader.py", line 383, in collect_sql
state = migration.unapply(state, schema_editor, collect_sql=True)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "site-packages/django/db/migrations/migration.py", line 193, in unapply
operation.database_backwards(
File "site-packages/django/db/migrations/operations/models.py", line 394, in database_backwards
schema_editor.create_model(model)
File "site-packages/django/db/backends/base/schema.py", line 513, in create_model
self.deferred_sql.extend(self._model_indexes_sql(model))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "site-packages/django/db/backends/base/schema.py", line 1624, in _model_indexes_sql
output.append(index.create_sql(model, self))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "site-packages/django/db/models/indexes.py", line 112, in create_sql
model._meta.get_field(field_name)
File "site-packages/django/db/models/options.py", line 683, in get_field
raise FieldDoesNotExist(
django.core.exceptions.FieldDoesNotExist: Dog has no field named 'animal'
The generated migrations for me are:
0001:
operations = [
migrations.CreateModel(
name="Animal",
fields=[
(
"id",
models.AutoField(
auto_created=True,
primary_key=True,
serialize=False,
verbose_name="ID",
),
),
("name", models.CharField(max_length=100)),
],
),
migrations.CreateModel(
name="Dog",
fields=[
(
"id",
models.AutoField(
auto_created=True,
primary_key=True,
serialize=False,
verbose_name="ID",
),
),
("name", models.CharField(max_length=100)),
(
"animal",
models.ForeignKey(
on_delete=django.db.models.deletion.CASCADE,
to="[app].animal",
),
),
],
options={
"indexes": [
models.Index(
fields=["animal", "name"], name="[app]_do_animal__b82a2f_idx"
)
],
},
),
]
0002:
operations = [
migrations.RemoveField(
model_name="dog",
name="animal",
),
migrations.DeleteModel(
name="Animal",
),
migrations.DeleteModel(
name="Dog",
),
]
My environment is:
- Django 5.0.6
- Postgres 15
- python 3.12.4
- M1 mac
Removing the Meta.indexes results in a generated migration that can be migrated backwards.
This appears by solveable in at least two ways:
- Adding the following operation to the top of the removal migration solves the problem.
migrations.RemoveIndex(
model_name="dog",
name="[app]_do_animal__b82a2f_idx",
),
- Removing the
RemoveFieldoperation from the removal migration and rearranging the models.
operations = [
migrations.DeleteModel(
name="Dog",
),
migrations.DeleteModel(
name="Animal",
),
]
Change History (14)
comment:1 by , 16 months ago
| Triage Stage: | Unreviewed → Accepted |
|---|---|
| Type: | Uncategorized → Bug |
comment:2 by , 16 months ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:3 by , 8 months ago
| Owner: | changed from to |
|---|
comment:5 by , 7 months ago
| Patch needs improvement: | set |
|---|
I think the actual issue is less about the migration sqlmigrate crashing but more about the auto-detector generating an invalid migration.
The resulting migration operations of removing all models should include a RemoveIndex operation that precedes the RemoveField
operations = [ migrations.RemoveIndex( model_name="dog", name="animal", ), migrations.RemoveField( model_name="dog", name="animal", ), migrations.DeleteModel( name="Animal", ), migrations.DeleteModel( name="Dog", ), ]
This is a test that should pass
-
tests/migrations/test_autodetector.py
diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index ac725d317e..30085e5c9b 100644
a b def test_remove_indexes(self): 2798 2798 changes, "otherapp", 0, 0, model_name="book", name="book_title_author_idx" 2799 2799 ) 2800 2800 2801 def test_remove_field_and_index(self): 2802 before_state = [ 2803 ModelState("testapp", "Animal", []), 2804 ModelState( 2805 "testapp", 2806 "Dog", 2807 fields=[ 2808 ("name", models.CharField(max_length=100)), 2809 ( 2810 "animal", 2811 models.ForeignKey("testapp.Animal", on_delete=models.CASCADE), 2812 ), 2813 ], 2814 options={ 2815 "indexes": [ 2816 models.Index(fields=("animal", "name"), name="animal_name_idx") 2817 ] 2818 }, 2819 ), 2820 ] 2821 changes = self.get_changes(before_state, []) 2822 # Right number/type of migrations? 2823 self.assertNumberMigrations(changes, "testapp", 1) 2824 self.assertOperationTypes( 2825 changes, 2826 "testapp", 2827 0, 2828 ["RemoveIndex", "RemoveField", "DeleteModel", "DeleteModel"], 2829 ) 2830 2801 2831 def test_rename_indexes(self): 2802 2832 book_renamed_indexes = ModelState( 2803 2833 "otherapp",
and the likely culprit lies in MigrationAutodetector.generate_deleted_models which should add a RemoveIndex operations for each index like we do for unique_together being turned into None. We'll likely need to add a new OperationDependency.Type.REMOVE_INDEX_OR_CONSTRAINT so RemoveField have their dependencies are always sorted after the newly introduced RemoveIndex.
Small demonstration of how this can be done.
FWIW the same issue will happens for constraints so we should add a test for that as well.
comment:7 by , 7 months ago
Thank you for the review. I'll try to resolve it based on the small demonstration!
comment:8 by , 7 months ago
| Patch needs improvement: | unset |
|---|
comment:10 by , 6 months ago
| Needs tests: | set |
|---|
comment:11 by , 5 months ago
| Needs tests: | unset |
|---|
comment:12 by , 5 months ago
| Needs tests: | set |
|---|
comment:13 by , 4 months ago
| Needs tests: | unset |
|---|---|
| Triage Stage: | Accepted → Ready for checkin |
Thank you for the report! Replicated on main 👍