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
This commit is contained in:
Simon Charette 2018-07-13 22:48:49 -04:00 committed by Tim Graham
parent 55b6f7af0c
commit ed7898e1b5
4 changed files with 11 additions and 17 deletions

View File

@ -449,9 +449,10 @@ 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)):
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)

View File

@ -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):
"""

View File

@ -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):
"""

View File

@ -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):