From 29f5e1e97d660855d34c1ee5edbd5cfe094b6ca7 Mon Sep 17 00:00:00 2001 From: wookkl Date: Wed, 18 Jun 2025 01:34:10 +0900 Subject: [PATCH] Fixed #35595, #35962 -- Removed indexes and constraints before fields in migrations. --- django/db/migrations/autodetector.py | 47 ++++++++++++++- tests/migrations/test_autodetector.py | 86 +++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 1 deletion(-) diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py index 1d25101219..731612b318 100644 --- a/django/db/migrations/autodetector.py +++ b/django/db/migrations/autodetector.py @@ -29,6 +29,7 @@ class OperationDependency( ALTER = 2 REMOVE_ORDER_WRT = 3 ALTER_FOO_TOGETHER = 4 + REMOVE_INDEX_OR_CONSTRAINT = 5 @cached_property def model_name_lower(self): @@ -528,6 +529,18 @@ class MigrationAutodetector: ) and operation.name_lower == dependency.model_name_lower ) + # Field is removed and part of an index/constraint. + elif ( + dependency.field_name is not None + and dependency.type == OperationDependency.Type.REMOVE_INDEX_OR_CONSTRAINT + ): + return ( + isinstance( + operation, + (operations.RemoveIndex, operations.RemoveConstraint), + ) + and operation.model_name_lower == dependency.model_name_lower + ) # Unknown dependency. Raise an error. else: raise ValueError("Can't handle dependency %r" % (dependency,)) @@ -931,6 +944,24 @@ class MigrationAutodetector: unique_together=None, ), ) + if indexes := model_state.options.pop("indexes", None): + for index in indexes: + self.add_operation( + app_label, + operations.RemoveIndex( + model_name=model_name, + name=index.name, + ), + ) + if constraints := model_state.options.pop("constraints", None): + for constraint in constraints: + self.add_operation( + app_label, + operations.RemoveConstraint( + model_name=model_name, + name=constraint.name, + ), + ) # Then remove each related field for name in sorted(related_fields): self.add_operation( @@ -939,6 +970,14 @@ class MigrationAutodetector: model_name=model_name, name=name, ), + dependencies=[ + OperationDependency( + app_label, + model_name, + name, + OperationDependency.Type.REMOVE_INDEX_OR_CONSTRAINT, + ), + ], ) # Finally, remove the model. # This depends on both the removal/alteration of all incoming fields @@ -1180,7 +1219,7 @@ class MigrationAutodetector: name=field_name, ), # We might need to depend on the removal of an - # order_with_respect_to or index/unique_together operation; + # order_with_respect_to or index/constraint/unique_together operation; # this is safely ignored if there isn't one dependencies=[ OperationDependency( @@ -1195,6 +1234,12 @@ class MigrationAutodetector: field_name, OperationDependency.Type.ALTER_FOO_TOGETHER, ), + OperationDependency( + app_label, + model_name, + field_name, + OperationDependency.Type.REMOVE_INDEX_OR_CONSTRAINT, + ), ], ) diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index 11f7d1e801..dae8a7f3ba 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -2978,6 +2978,92 @@ class AutodetectorTests(BaseAutodetectorTests): changes, "otherapp", 0, 0, model_name="book", name="book_title_author_idx" ) + def test_remove_field_with_model_options(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") + ], + "constraints": [ + models.UniqueConstraint( + 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", + "RemoveConstraint", + "RemoveField", + "DeleteModel", + "DeleteModel", + ], + ) + + def test_remove_field_with_remove_index_or_constraint_dependency(self): + before_state = [ + ModelState("testapp", "Category", []), + ModelState( + "testapp", + "Model", + fields=[ + ("date", models.DateField(auto_now=True)), + ( + "category", + models.ForeignKey( + "testapp.Category", models.SET_NULL, null=True + ), + ), + ], + options={ + "constraints": [ + models.UniqueConstraint( + fields=("date", "category"), name="unique_category_for_date" + ), + ] + }, + ), + ] + changes = self.get_changes( + before_state, + [ + ModelState( + "testapp", + "Model", + fields=[ + ("date", models.DateField(auto_now=True)), + ], + ), + ], + ) + # Right number/type of migrations? + self.assertNumberMigrations(changes, "testapp", 1) + self.assertOperationTypes( + changes, + "testapp", + 0, + ["RemoveConstraint", "RemoveField", "DeleteModel"], + ) + def test_rename_indexes(self): book_renamed_indexes = ModelState( "otherapp",