From ed7898e1b58c29cda648a799ac4bd5bc7e193b8b Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Fri, 13 Jul 2018 22:48:49 -0400 Subject: [PATCH] Fixed #28862 -- Disabled optimization of AlterFooTogether and RemoveField. AlterFooTogether operations cannot be swapped with RemoveField operations on the same model as they could be removing the the same field as well. Since AlterFooTogether operations don't track what their previous value was, it's impossible to determine whether or not the optimization is safe so the only way to proceed is to disable the optimization. Thanks Ramiro Morales for the in-depth analysis of the issue. Refs #24828 --- django/db/migrations/operations/models.py | 9 +++++---- tests/migrations/test_autodetector.py | 8 ++++---- tests/migrations/test_commands.py | 2 +- tests/migrations/test_optimizer.py | 9 +-------- 4 files changed, 11 insertions(+), 17 deletions(-) diff --git a/django/db/migrations/operations/models.py b/django/db/migrations/operations/models.py index ad436cf611..549b2bfc62 100644 --- a/django/db/migrations/operations/models.py +++ b/django/db/migrations/operations/models.py @@ -449,10 +449,11 @@ class ModelOptionOperation(ModelOperation): class FieldRelatedOptionOperation(ModelOptionOperation): def reduce(self, operation, app_label=None): - if (isinstance(operation, FieldOperation) and - self.name_lower == operation.model_name_lower and - not self.references_field(operation.model_name, operation.name)): - return [operation, self] + if isinstance(operation, FieldOperation) and self.name_lower == operation.model_name_lower: + if isinstance(operation, RemoveField): + return False + if not self.references_field(operation.model_name, operation.name): + return [operation, self] return super().reduce(operation, app_label=app_label) diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index e3ce3a129a..b181b084be 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -1529,10 +1529,10 @@ class AutodetectorTests(TestCase): ) # Right number/type of migrations? self.assertNumberMigrations(changes, "otherapp", 1) - self.assertOperationTypes(changes, "otherapp", 0, ["RemoveField", "AlterUniqueTogether", "AlterIndexTogether"]) - self.assertOperationAttributes(changes, "otherapp", 0, 0, model_name="book", name="newfield") - self.assertOperationAttributes(changes, "otherapp", 0, 1, name="book", unique_together={("author", "title")}) - self.assertOperationAttributes(changes, "otherapp", 0, 2, name="book", index_together={("author", "title")}) + self.assertOperationTypes(changes, "otherapp", 0, ["AlterUniqueTogether", "AlterIndexTogether", "RemoveField"]) + self.assertOperationAttributes(changes, "otherapp", 0, 0, name="book", unique_together={("author", "title")}) + self.assertOperationAttributes(changes, "otherapp", 0, 1, name="book", index_together={("author", "title")}) + self.assertOperationAttributes(changes, "otherapp", 0, 2, model_name="book", name="newfield") def test_rename_field_and_foo_together(self): """ diff --git a/tests/migrations/test_commands.py b/tests/migrations/test_commands.py index f625a47c7f..3c42755917 100644 --- a/tests/migrations/test_commands.py +++ b/tests/migrations/test_commands.py @@ -1335,7 +1335,7 @@ class SquashMigrationsTests(MigrationTestBase): out = io.StringIO() with self.temporary_migration_module(module="migrations.test_migrations"): call_command("squashmigrations", "migrations", "0002", interactive=False, verbosity=1, stdout=out) - self.assertIn("Optimized from 8 operations to 3 operations.", out.getvalue()) + self.assertIn("Optimized from 8 operations to 4 operations.", out.getvalue()) def test_ticket_23799_squashmigrations_no_optimize(self): """ diff --git a/tests/migrations/test_optimizer.py b/tests/migrations/test_optimizer.py index a408db9ef2..b841b531b1 100644 --- a/tests/migrations/test_optimizer.py +++ b/tests/migrations/test_optimizer.py @@ -712,7 +712,7 @@ class OptimizerTests(SimpleTestCase): ], ) - self.assertOptimizesTo( + self.assertDoesNotOptimize( [ migrations.CreateModel("Foo", [ ("a", models.IntegerField()), @@ -722,13 +722,6 @@ class OptimizerTests(SimpleTestCase): alter, migrations.RemoveField("Foo", "c"), ], - [ - migrations.CreateModel("Foo", [ - ("a", models.IntegerField()), - ("b", models.IntegerField()), - ]), - alter, - ], ) def test_create_alter_unique_field(self):