Generated migration for removing related models can't migrate backwards

[Tim Schilling]

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.

1. python manage.py makemigrations [app] -> 0001_initial.py (in my test it was the second migration, in case you can't reproduce it)
2. Remove both models
3. python manage.py makemigrations [app] -> 0002_remove.py
4. python 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:

1. 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",
),

2. Removing the RemoveField operation from the removal migration and rearranging the models.

operations = [
    migrations.DeleteModel(
        name="Dog",
    ),
    migrations.DeleteModel(
        name="Animal",
    ),
]

[Sarah Boyce]

Thank you for the report! Replicated on main 👍

[wookkl]

PR ​https://github.com/django/django/pull/19361

[Simon Charette]

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

@@ -2798 +2798 @@
             changes, "otherapp", 0, 0, model_name="book", name="book_title_author_idx"
         )
 
+    def test_remove_field_and_index(self):
+        before_state = [
+            ModelState("testapp", "Animal", []),
+            ModelState(
+                "testapp",
+                "Dog",
+                fields=[
+                    ("name", models.CharField(max_length=100)),
+                    (
+                        "animal",
+                        models.ForeignKey("testapp.Animal", on_delete=models.CASCADE),
+                    ),
+                ],
+                options={
+                    "indexes": [
+                        models.Index(fields=("animal", "name"), name="animal_name_idx")
+                    ]
+                },
+            ),
+        ]
+        changes = self.get_changes(before_state, [])
+        # Right number/type of migrations?
+        self.assertNumberMigrations(changes, "testapp", 1)
+        self.assertOperationTypes(
+            changes,
+            "testapp",
+            0,
+            ["RemoveIndex", "RemoveField", "DeleteModel", "DeleteModel"],
+        )
+
     def test_rename_indexes(self):
         book_renamed_indexes = ModelState(
             "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.

[Simon Charette]

This is the twin ticket of #35962 for indexes instead of constraints.

[wookkl]

Thank you for the review. I'll try to resolve it based on the small demonstration!

[Simon Charette]

Patch LGTM and solves #35962 at the same time.

[Sarah Boyce <42296566+sarahboyce@…>]

In 29f5e1e9:

Fixed #35595, #35962 -- Removed indexes and constraints before fields in migrations.