[Django] #27731: Squashmigrations doesn't optimize AlterField related_name across AlterUniqueTogether/AlterIndexTogether
Django
AlterUniqueTogether/AlterIndexTogether
-------------------------------------+-------------------------------------
Reporter: Ed Morley | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: | Version: master
Migrations |
Severity: Normal | Keywords: squashmigrations
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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).
--
Ticket URL: <https://code.djangoproject.com/ticket/27731>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
AlterUniqueTogether/AlterIndexTogether
Reporter: Ed Morley | Owner: nobody
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: squashmigrations | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/27731#comment:1>
Django
AlterUniqueTogether/AlterIndexTogether
Reporter: Ed Morley | Owner: nobody
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: squashmigrations | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27731#comment:2>
Django
AlterUniqueTogether/AlterIndexTogether
-------------------------------------+-------------------------------------
Type: | Nester
Cleanup/optimization | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: squashmigrations | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: nobody => Andrew Nester
* has_patch: 0 => 1
Comment:
Any I've added PR that fixes this and could be discussed.
It's a bit straightforward implementation
[https://github.com/django/django/pull/7964 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/27731#comment:3>
Django
AlterUniqueTogether/AlterIndexTogether
-------------------------------------+-------------------------------------
Type: | Nester
Cleanup/optimization | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: squashmigrations | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27731#comment:4>
Django
AlterUniqueTogether/AlterIndexTogether
-------------------------------------+-------------------------------------
Type: | Charette
Cleanup/optimization | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: squashmigrations | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Andrew Nester => Simon Charette
* needs_better_patch: 1 => 0
Comment:
This is handled by [https://github.com/django/django/pull/10178 PR] which
implements the `CreateModel`/`AlterTogetherOptionOperation` reduction into
`CreateModel.options` and then allows the `CreateModel`/`AlterField`
reduction into `CreateModel.fields` to take place.
--
Ticket URL: <https://code.djangoproject.com/ticket/27731#comment:5>
Django
AlterUniqueTogether/AlterIndexTogether
-------------------------------------+-------------------------------------
Type: | Charette
Component: Migrations | Version: master
Severity: Normal | Resolution: fixed
Keywords: squashmigrations | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"8e3f22f2513a5b64153ea9903690a38ac159031b" 8e3f22f2]:
{{{
#!CommitTicketReference repository=""
revision="8e3f22f2513a5b64153ea9903690a38ac159031b"
Fixed #27731 -- Implemented CreateModel/AlterFooOperation reduction.
This should alleviate the side effects of disabling the AlterFooOperation
reduction with RemoveField to fix refs #28862 during migration squashing
because CreateModel can perform a reduction with RemoveField.
Thanks Nick Pope for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27731#comment:6>