From 1a7fc0f65d5b2ef56625a8b5897496e28a5834ff Mon Sep 17 00:00:00 2001 From: Clifford Gama Date: Sat, 5 Jul 2025 12:47:53 +0200 Subject: [PATCH] Fixed #36438 -- Made MigrationAutodetector remove generated fields before their base fields. Thanks to Colton Saska for the report and to Simon Charette for the review. --- django/db/migrations/autodetector.py | 30 ++++++++- tests/migrations/test_autodetector.py | 96 ++++++++++++++++++++++++++- 2 files changed, 122 insertions(+), 4 deletions(-) diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py index c319cb2c03..f974c4e4ea 100644 --- a/django/db/migrations/autodetector.py +++ b/django/db/migrations/autodetector.py @@ -1221,9 +1221,9 @@ class MigrationAutodetector: model_name=model_name, name=field_name, ), - # We might need to depend on the removal of an - # order_with_respect_to or index/constraint/unique_together - # operation; this is safely ignored if there isn't one + # Include dependencies such as order_with_respect_to, constraints, + # and any generated fields that may depend on this field. These + # are safely ignored if not present. dependencies=[ OperationDependency( app_label, @@ -1243,6 +1243,9 @@ class MigrationAutodetector: field_name, OperationDependency.Type.REMOVE_INDEX_OR_CONSTRAINT, ), + *self._get_generated_field_dependencies_for_removed_field( + app_label, model_name, field_name + ), ], ) @@ -1698,6 +1701,27 @@ class MigrationAutodetector: ) return dependencies + def _get_generated_field_dependencies_for_removed_field( + self, app_label, model_name, field_name + ): + dependencies = [] + model_state = self.from_state.models[app_label, model_name] + generated_fields = (f for f in model_state.fields.values() if f.generated) + for field in generated_fields: + if any( + field_name == name + for name, *_ in models.Model._get_expr_references(field.expression) + ): + dependencies.append( + OperationDependency( + app_label, + model_name, + field.name, + OperationDependency.Type.REMOVE, + ) + ) + return dependencies + def _get_dependencies_for_model(self, app_label, model_name): """Return foreign key dependencies of the given model.""" dependencies = [] diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index c5d630293e..c044cc9a99 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -13,7 +13,7 @@ from django.db.migrations.graph import MigrationGraph from django.db.migrations.loader import MigrationLoader from django.db.migrations.questioner import MigrationQuestioner from django.db.migrations.state import ModelState, ProjectState -from django.db.models.functions import Concat, Lower +from django.db.models.functions import Concat, Lower, Upper from django.test import SimpleTestCase, TestCase, override_settings from django.test.utils import isolate_lru_cache @@ -1454,6 +1454,100 @@ class AutodetectorTests(BaseAutodetectorTests): self.assertOperationTypes(changes, "testapp", 0, ["RemoveField"]) self.assertOperationAttributes(changes, "testapp", 0, 0, name="name") + def test_remove_generated_field_before_its_base_field(self): + initial_state = [ + ModelState( + "testapp", + "Author", + [ + ("name", models.CharField(max_length=20)), + ( + "upper_name", + models.GeneratedField( + expression=Upper("name"), + db_persist=True, + output_field=models.CharField(), + ), + ), + ], + ), + ] + updated_state = [ModelState("testapp", "Author", [])] + changes = self.get_changes(initial_state, updated_state) + self.assertNumberMigrations(changes, "testapp", 1) + self.assertOperationTypes(changes, "testapp", 0, ["RemoveField", "RemoveField"]) + self.assertOperationAttributes(changes, "testapp", 0, 0, name="upper_name") + self.assertOperationAttributes(changes, "testapp", 0, 1, name="name") + + def test_remove_generated_field_before_multiple_base_fields(self): + initial_state = [ + ModelState( + "testapp", + "Author", + [ + ("first_name", models.CharField(max_length=20)), + ("last_name", models.CharField(max_length=20)), + ( + "full_name", + models.GeneratedField( + expression=Concat("first_name", "last_name"), + db_persist=True, + output_field=models.CharField(), + ), + ), + ], + ), + ] + updated_state = [ModelState("testapp", "Author", [])] + changes = self.get_changes(initial_state, updated_state) + self.assertNumberMigrations(changes, "testapp", 1) + self.assertOperationTypes( + changes, "testapp", 0, ["RemoveField", "RemoveField", "RemoveField"] + ) + self.assertOperationAttributes(changes, "testapp", 0, 0, name="full_name") + self.assertOperationAttributes(changes, "testapp", 0, 1, name="first_name") + self.assertOperationAttributes(changes, "testapp", 0, 2, name="last_name") + + def test_remove_generated_field_and_one_of_multiple_base_fields(self): + initial_state = [ + ModelState( + "testapp", + "Author", + [ + ("first_name", models.CharField(max_length=20)), + ("last_name", models.CharField(max_length=20)), + ( + "full_name", + models.GeneratedField( + expression=Concat("first_name", "last_name"), + db_persist=True, + output_field=models.CharField(), + ), + ), + ], + ), + ] + # Only remove full_name and first_name. + updated_state = [ + ModelState( + "testapp", + "Author", + [ + ("last_name", models.CharField(max_length=20)), + ], + ), + ] + changes = self.get_changes(initial_state, updated_state) + self.assertNumberMigrations(changes, "testapp", 1) + self.assertOperationTypes( + changes, + "testapp", + 0, + ["RemoveField", "RemoveField"], + ) + self.assertOperationAttributes(changes, "testapp", 0, 0, name="full_name") + self.assertOperationAttributes(changes, "testapp", 0, 1, name="first_name") + def test_alter_field(self): """Tests autodetection of new fields.""" changes = self.get_changes([self.author_name], [self.author_name_longer])