#35595: Generated migration for removing related models can't migrate backwards
-----------------------------------+------------------------------------
Reporter: Timothy Schilling | Owner: wookkl
Type: Bug | Status: assigned
Component: Migrations | Version: 5.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-----------------------------------+------------------------------------
Changes (by Simon Charette):
* needs_better_patch: 0 => 1
Comment:
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`
{{{#!python
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
{{{#!diff
diff --git a/tests/migrations/test_autodetector.py
b/tests/migrations/test_autodetector.py
index ac725d317e..30085e5c9b 100644
--- a/tests/migrations/test_autodetector.py
+++ b/tests/migrations/test_autodetector.py
@@ -2798,6 +2798,36 @@ def test_remove_indexes(self):
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
[https://github.com/django/django/compare/main...charettes:django:ticket-35595
this can be done].
FWIW the same issue will happens for `constraints` so we should add a test
for that as well.
--
Ticket URL: <https://code.djangoproject.com/ticket/35595#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion visit
https://groups.google.com/d/msgid/django-updates/010701961668d23f-3c6a91a0-0065-4e05-869a-7e73a5e0ca62-000000%40eu-central-1.amazonses.com.
