Opened 9 months ago

Last modified 8 months ago

#27731 assigned Cleanup/optimization

Squashmigrations doesn't optimize AlterField related_name across AlterUniqueTogether/AlterIndexTogether

Reported by: Ed Morley Owned by: Andrew Nester
Component: Migrations Version: master
Severity: Normal Keywords: squashmigrations
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
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 (4)

comment:1 Changed 8 months ago by Tim Graham

Triage Stage: UnreviewedAccepted

comment:2 Changed 8 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 8 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 8 months ago by Andrew Nester (previous) (diff)

comment:4 Changed 8 months ago by Tim Graham

Patch needs improvement: set
Note: See TracTickets for help on using tickets.
Back to Top