[Django] #24828: AlterUniqueTogether prevents squashmigrations optimizations
Django
-------------------------------+--------------------
Reporter: riklaunim | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
I have a model that has a AlterUniqueTogether so in initial migration
after CreateModel there was a AlterUniqueTogether operation. After that I
got some migrations that add/alter/remove some fields to that model (but
not those in AlterUniqueTogether). squashmigrations couldn't optimize
those additional operations into one CreateModel operation. When I moved
AlterUniqueTogether to the end of the squash operations list and
resquashed then it did that. (Django 1.8.1)
--
Ticket URL: <https://code.djangoproject.com/ticket/24828>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: riklaunim | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by riklaunim):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
AlterIndexTogether also. I've also noticed that if there are two or more
models the optimization is quite fragile to order of operations. When I
move all model A simple add/remove/alter operations after CreateModel,
then move similar operations after CreateModel B then it will resquash it
nicely. If those operations are mixes and CreateModel A, B are at start
then it won't merge operations.
--
Ticket URL: <https://code.djangoproject.com/ticket/24828#comment:1>
Django
Reporter: riklaunim | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
I can't confirm this can be definitely be fixed, but investigating it
seems worthwhile.
--
Ticket URL: <https://code.djangoproject.com/ticket/24828#comment:2>
Django
Reporter: riklaunim | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* version: 1.8 => master
Comment:
Yes, it's possible, but I think it's not a trivial thing like adding
another rule to the `MigrationOptimizer`
(`django.db.migrations.optimizer`). `AlterFOOTogether` and `AlterField`
have nothing in common that would require a merge / reordering. Thus it's
not picked up, I guess. But that's just from a brief look.
--
Ticket URL: <https://code.djangoproject.com/ticket/24828#comment:3>
Django
Reporter: riklaunim | Owner: MarkusH
Type: Cleanup/optimization | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* owner: nobody => MarkusH
* status: new => assigned
* has_patch: 0 => 1
Comment:
PR: https://github.com/django/django/pull/4858
--
Ticket URL: <https://code.djangoproject.com/ticket/24828#comment:4>
Django
Reporter: riklaunim | Owner: MarkusH
Type: | Status: assigned
Cleanup/optimization |
Component: Migrations | Version: master
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/24828#comment:5>
Django
Reporter: riklaunim | Owner: MarkusH
Cleanup/optimization |
Component: Migrations | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Markus Holtermann <info@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"e470f311d654267ec86f9a6325ec500345b9dff2" e470f311]:
{{{
#!CommitTicketReference repository=""
revision="e470f311d654267ec86f9a6325ec500345b9dff2"
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
128caa1e16ec2627737748f75c8e55600a3df97f, these operations are optimized
in a later round of the optimizer.
Thanks Tim Graham for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24828#comment:6>
Django
Reporter: Piotr Maliński | Owner: Markus
Type: | Holtermann
Cleanup/optimization | Status: closed
Component: Migrations | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"ed7898e1b58c29cda648a799ac4bd5bc7e193b8b" ed7898e1]:
{{{
#!CommitTicketReference repository=""
revision="ed7898e1b58c29cda648a799ac4bd5bc7e193b8b"
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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24828#comment:7>