Commit e470f311 authored by Markus Holtermann's avatar Markus Holtermann
Browse files

Fixed #24828 -- Allowed migration optimization across AlterFooTogether 

The idea behind this change is, that AlterUniqueTogether,
AlterIndexTogether and AlterOrderWithRespectTo can always be moved after
an Add/Alter/Rename/RemoveField operation if they don't refer to the
respective field and are not empty sets / None.

Combined with the optimizations of duplicate AlterUniqueTogether,
AlterIndexTogether, and AlterOrderWithRespectTo operations from
128caa1e, these operations are optimized
in a later round of the optimizer.

Thanks Tim Graham for the review.
parent fb1ba4d6

django/db/migrations/operations/models.py

+27 −0
Original line number Diff line number Diff line
@@ -365,6 +365,15 @@ class AlterUniqueTogether(Operation): 
    def references_model(self, name, app_label=None):

        return name.lower() == self.name_lower



    def references_field(self, model_name, name, app_label=None):

        return (

            self.references_model(model_name, app_label) and

            (

                not self.unique_together or

                any((name in together) for together in self.unique_together)

            )

        )



    def describe(self):

        return "Alter %s for %s (%s constraint(s))" % (self.option_name, self.name, len(self.unique_together or ''))
@@ -417,6 +426,15 @@ class AlterIndexTogether(Operation): 
    def references_model(self, name, app_label=None):

        return name.lower() == self.name_lower



    def references_field(self, model_name, name, app_label=None):

        return (

            self.references_model(model_name, app_label) and

            (

                not self.index_together or

                any((name in together) for together in self.index_together)

            )

        )



    def describe(self):

        return "Alter %s for %s (%s constraint(s))" % (self.option_name, self.name, len(self.index_together or ''))
@@ -474,6 +492,15 @@ class AlterOrderWithRespectTo(Operation): 
    def references_model(self, name, app_label=None):

        return name.lower() == self.name_lower



    def references_field(self, model_name, name, app_label=None):

        return (

            self.references_model(model_name, app_label) and

            (

                self.order_with_respect_to is None or

                name == self.order_with_respect_to

            )

        )



    def describe(self):

        return "Set order_with_respect_to on %s to %s" % (self.name, self.order_with_respect_to)

django/db/migrations/optimizer.py

+24 −0
Original line number Diff line number Diff line
@@ -56,6 +56,20 @@ class MigrationOptimizer(object): 
            (AlterField, RenameField): self.reduce_alter_field_rename_field,

            (CreateModel, RenameField): self.reduce_create_model_rename_field,

            (RenameField, RenameField): self.reduce_rename_field_self,



            (AlterIndexTogether, AddField): self.reduce_alter_model_addalterremove_field,

            (AlterIndexTogether, AlterField): self.reduce_alter_model_addalterremove_field,

            (AlterIndexTogether, RemoveField): self.reduce_alter_model_addalterremove_field,

            (AlterOrderWithRespectTo, AddField): self.reduce_alter_model_addalterremove_field,

            (AlterOrderWithRespectTo, AlterField): self.reduce_alter_model_addalterremove_field,

            (AlterOrderWithRespectTo, RemoveField): self.reduce_alter_model_addalterremove_field,

            (AlterUniqueTogether, AddField): self.reduce_alter_model_addalterremove_field,

            (AlterUniqueTogether, AlterField): self.reduce_alter_model_addalterremove_field,

            (AlterUniqueTogether, RemoveField): self.reduce_alter_model_addalterremove_field,



            (AlterIndexTogether, RenameField): self.reduce_alter_model_rename_field,

            (AlterOrderWithRespectTo, RenameField): self.reduce_alter_model_rename_field,

            (AlterUniqueTogether, RenameField): self.reduce_alter_model_rename_field,

        }



    def optimize(self, operations, app_label=None):
@@ -310,6 +324,16 @@ class MigrationOptimizer(object): 
                ),

            ]



    def reduce_alter_model_addalterremove_field(self, operation, other, in_between):

        if (operation.name_lower == other.model_name_lower and

                not operation.references_field(other.model_name, other.name)):

            return [other, operation]



    def reduce_alter_model_rename_field(self, operation, other, in_between):

        if (operation.name_lower == other.model_name_lower and

                not operation.references_field(other.model_name, other.old_name)):

            return [other, operation]



    # THROUGH CHECKS



    def can_optimize_through(self, operation, other, app_label=None):

tests/migrations/test_autodetector.py

+4 −3
Original line number Diff line number Diff line
@@ -1149,9 +1149,10 @@ class AutodetectorTests(TestCase): 
        changes = autodetector._detect_changes()

        # Right number/type of migrations?

        self.assertNumberMigrations(changes, "otherapp", 1)

        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.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")})



    def test_rename_field_and_foo_together(self):

        """

tests/migrations/test_commands.py

+1 −1
Original line number Diff line number Diff line
@@ -889,7 +889,7 @@ class SquashMigrationsTests(MigrationTestBase): 
        out = six.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 7 operations to 5 operations.", force_text(out.getvalue()))

        self.assertIn("Optimized from 7 operations to 3 operations.", force_text(out.getvalue()))



    def test_ticket_23799_squashmigrations_no_optimize(self):

        """

tests/migrations/test_optimizer.py

+152 −0
Original line number Diff line number Diff line
@@ -29,6 +29,9 @@ class OptimizerTests(SimpleTestCase): 
        if less_than is not None and iterations >= less_than:

            raise self.failureException("Optimization did not take less than %s iterations (it took %s)" % (less_than, iterations))



    def assertDoesNotOptimize(self, operations):

        self.assertOptimizesTo(operations, operations)



    def test_single(self):

        """

        Tests that the optimizer does nothing on a single operation,
@@ -447,6 +450,155 @@ class OptimizerTests(SimpleTestCase): 
            ],

        )



    def _test_create_alter_foo_field(self, alter):

        """

        CreateModel, AlterFooTogether/AlterOrderWithRespectTo followed by an

        add/alter/rename field should optimize to CreateModel and the Alter*

        """

        # AddField

        self.assertOptimizesTo(

            [

                migrations.CreateModel("Foo", [

                    ("a", models.IntegerField()),

                    ("b", models.IntegerField()),

                ]),

                alter,

                migrations.AddField("Foo", "c", models.IntegerField()),

            ],

            [

                migrations.CreateModel("Foo", [

                    ("a", models.IntegerField()),

                    ("b", models.IntegerField()),

                    ("c", models.IntegerField()),

                ]),

                alter,

            ],

        )



        # AlterField

        self.assertDoesNotOptimize(

            [

                migrations.CreateModel("Foo", [

                    ("a", models.IntegerField()),

                    ("b", models.IntegerField()),

                ]),

                alter,

                migrations.AlterField("Foo", "b", models.CharField(max_length=255)),

            ],

        )



        self.assertOptimizesTo(

            [

                migrations.CreateModel("Foo", [

                    ("a", models.IntegerField()),

                    ("b", models.IntegerField()),

                    ("c", models.IntegerField()),

                ]),

                alter,

                migrations.AlterField("Foo", "c", models.CharField(max_length=255)),

            ],

            [

                migrations.CreateModel("Foo", [

                    ("a", models.IntegerField()),

                    ("b", models.IntegerField()),

                    ("c", models.CharField(max_length=255)),

                ]),

                alter,

            ],

        )



        # RenameField

        self.assertDoesNotOptimize(

            [

                migrations.CreateModel("Foo", [

                    ("a", models.IntegerField()),

                    ("b", models.IntegerField()),

                ]),

                alter,

                migrations.RenameField("Foo", "b", "c"),

            ],

        )



        self.assertOptimizesTo(

            [

                migrations.CreateModel("Foo", [

                    ("a", models.IntegerField()),

                    ("b", models.IntegerField()),

                ]),

                alter,

                migrations.RenameField("Foo", "b", "x"),

                migrations.RenameField("Foo", "x", "c"),

            ],

            [

                migrations.CreateModel("Foo", [

                    ("a", models.IntegerField()),

                    ("b", models.IntegerField()),

                ]),

                alter,

                migrations.RenameField("Foo", "b", "c"),

            ],

        )



        self.assertOptimizesTo(

            [

                migrations.CreateModel("Foo", [

                    ("a", models.IntegerField()),

                    ("b", models.IntegerField()),

                    ("c", models.IntegerField()),

                ]),

                alter,

                migrations.RenameField("Foo", "c", "d"),

            ],

            [

                migrations.CreateModel("Foo", [

                    ("a", models.IntegerField()),

                    ("b", models.IntegerField()),

                    ("d", models.IntegerField()),

                ]),

                alter,

            ],

        )



        # RemoveField

        self.assertDoesNotOptimize(

            [

                migrations.CreateModel("Foo", [

                    ("a", models.IntegerField()),

                    ("b", models.IntegerField()),

                ]),

                alter,

                migrations.RemoveField("Foo", "b"),

            ],

        )



        self.assertOptimizesTo(

            [

                migrations.CreateModel("Foo", [

                    ("a", models.IntegerField()),

                    ("b", models.IntegerField()),

                    ("c", models.IntegerField()),

                ]),

                alter,

                migrations.RemoveField("Foo", "c"),

            ],

            [

                migrations.CreateModel("Foo", [

                    ("a", models.IntegerField()),

                    ("b", models.IntegerField()),

                ]),

                alter,

            ],

        )



    def test_create_alter_unique_field(self):

        self._test_create_alter_foo_field(migrations.AlterUniqueTogether("Foo", [["a", "b"]]))



    def test_create_alter_index_field(self):

        self._test_create_alter_foo_field(migrations.AlterIndexTogether("Foo", [["a", "b"]]))



    def test_create_alter_owrt_field(self):

        self._test_create_alter_foo_field(migrations.AlterOrderWithRespectTo("Foo", "b"))



    def test_optimize_through_fields(self):

        """

        Checks that field-level through checking is working.