Opened 18 months ago

Last modified 39 hours ago

#27731 assigned Cleanup/optimization

Squashmigrations doesn't optimize AlterField related_name across AlterUniqueTogether/AlterIndexTogether

Reported by: Ed Morley Owned by: Simon Charette
Component: Migrations Version: master
Severity: Normal Keywords: squashmigrations
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The squashmigrations command doesn't combine an AlterField() with CreateModel() in the case below, when all that was changed was the related_name.

ie: This fails on Django master...

diff --git a/tests/migrations/test_optimizer.py b/tests/migrations/test_optimizer.py
index fc4f0ac..93a43a7 100644
--- a/tests/migrations/test_optimizer.py
+++ b/tests/migrations/test_optimizer.py
@@ -572,6 +572,26 @@ class OptimizerTests(SimpleTestCase):
             ],
         )

+    self.assertOptimizesTo(
+        [
+            migrations.CreateModel("Bar", [("name", models.CharField(max_length=255))]),
+            migrations.CreateModel("Foo", [
+                ("a", models.ForeignKey("testapp.Bar", models.CASCADE)),
+                ("b", models.IntegerField()),
+            ]),
+            alter,
+            migrations.AlterField("Foo", "a", models.ForeignKey("testapp.Bar", models.CASCADE, related_name="baz")),
+        ],
+        [
+            migrations.CreateModel("Bar", [("name", models.CharField(max_length=255))]),
+            migrations.CreateModel("Foo", [
+                ("a", models.ForeignKey("testapp.Bar", models.CASCADE, related_name="baz")),
+                ("b", models.IntegerField()),
+            ]),
+            alter,
+        ],
+    )
+
         # RenameField
         self.assertDoesNotOptimize(
             [

This causes failures in:

  • test_create_alter_index_field
  • test_create_alter_unique_field

ie optimization should occur but doesn't, when alter is:
migrations.AlterUniqueTogether("Foo", [["a", "b"]])

...or when alter is:
migrations.AlterIndexTogether("Foo", [["a", "b"]])

(But passes in the AlterOrderWithRespectTo case).

Change History (5)

comment:1 Changed 18 months ago by Tim Graham

Triage Stage: UnreviewedAccepted

comment:2 Changed 18 months ago by Andrew Nester

I guess this behaviour is expected.
We have following assert in OptimizerTest

self.assertDoesNotOptimize(
            [
                migrations.CreateModel("Foo", [
                    ("a", models.IntegerField()),
                    ("b", models.IntegerField()),
                ]),
                alter,
                migrations.AlterField("Foo", "b", models.CharField(max_length=255)),
            ],
        )

In general, if we have AlterUniqueTogether set for some field, let's say a, and after that performs AlterField on same field then we do not perform optimisation.

Actually it could be fixed, but it requires some thoughts about how to determine what changes introduce this AlterFIeld - some of them could be safe to optimise, some of them not.

comment:3 Changed 18 months ago by Andrew Nester

Has patch: set
Owner: changed from nobody to Andrew Nester
Status: newassigned

Anyway I've added PR that fixes this and could be discussed.
It's a bit straightforward implementation PR

Last edited 18 months ago by Andrew Nester (previous) (diff)

comment:4 Changed 17 months ago by Tim Graham

Patch needs improvement: set

comment:5 Changed 39 hours ago by Simon Charette

Owner: changed from Andrew Nester to Simon Charette
Patch needs improvement: unset

This is handled by PR which implements the CreateModel/AlterTogetherOptionOperation reduction into CreateModel.options and then allows the CreateModel/AlterField reduction into CreateModel.fields to take place.

Note: See TracTickets for help on using tickets.
Back to Top