[Django] #34366: Migration optimizer does not reduce multiple AlterField
Django
------------------------------------------+------------------------
Reporter: Laurent Tramoy | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 4.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------+------------------------
Let's consider the following operations:
{{{
operations = [
migrations.AddField(
model_name="book",
name="title",
field=models.CharField(max_length=256, null=True),
),
migrations.AlterField(
model_name="book",
name="title",
field=models.CharField(max_length=128, null=True),
),
migrations.AlterField(
model_name="book",
name="title",
field=models.CharField(max_length=128, null=True,
help_text="help"),
),
migrations.AlterField(
model_name="book",
name="title",
field=models.CharField(max_length=128, null=True,
help_text="help", default=None),
),
]
}}}
If I run the optimizer, I get only the `AddField`, as we could expect.
However, if the `AddField` model is separated from the AlterField (e.g.
because of a non-elidable migration, or inside a non-squashed migration),
none of the `AlterField` are reduced:
{{{
optimizer.optimize(operations[1:], "books")
[<AlterField model_name='book', name='title',
field=<django.db.models.fields.CharField>>,
<AlterField model_name='book', name='title',
field=<django.db.models.fields.CharField>>,
<AlterField model_name='book', name='title',
field=<django.db.models.fields.CharField>>]
}}}
Indeed, the `AlterField.reduce` does not consider the the case where
`operation` is also an `AlterField`.
Is this behaviour intended? If so, could it be documented?
Otherwise, would it make sense to add something like
{{{
if isinstance(operation, AlterField) and
self.is_same_field_operation(
operation
):
return [operation]
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34366>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Laurent Tramoy | Owner: nobody
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* version: 4.1 => dev
* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
Your analysis is correct Laurent, the reduction of multiple `AlterField`
against the same model is simply not implemented today hence why you're
running this behaviour.
Given you're already half way there
[https://docs.djangoproject.com/en/4.1/intro/contributing/#running-
django-s-test-suite-for-the-first-time I would encourage you to submit a
PR] that adds these changes and
[https://github.com/django/django/blob/main/tests/migrations/test_optimizer.py
an optimizer regression test] to cover them if you'd like to see this
issue fixed in future versions of Django.
--
Ticket URL: <https://code.djangoproject.com/ticket/34366#comment:1>
Django
--------------------------------------+------------------------------------
Reporter: Laurent Tramoy | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Laurent Tramoy):
Thanks Simon, I submitted a PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/34366#comment:2>
Django
Reporter: Laurent Tramoy | Owner: Laurent
Type: | Tramoy
Cleanup/optimization | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Bhuvnesh):
* owner: nobody => Laurent Tramoy
* status: new => assigned
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/16595 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/34366#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Laurent Tramoy | Owner: Laurent
Type: | Tramoy
Cleanup/optimization | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/34366#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Laurent Tramoy | Owner: Laurent
Type: | Tramoy
Component: Migrations | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"2276ec8c21655b05bb44e14e236b499aa5d01f5b" 2276ec8c]:
{{{
#!CommitTicketReference repository=""
revision="2276ec8c21655b05bb44e14e236b499aa5d01f5b"
Fixed #34366 -- Reduced AlterField operations when optimizing migrations.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34366#comment:5>