[Django] #24109: Provide a way to mark a migration operation as elidable when squashing
Django
---------------------------------------+------------------------
Reporter: carljm | Owner: nobody
Type: New feature | Status: new
Component: Migrations | Version: master
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 |
---------------------------------------+------------------------
The vast majority of my RunPython/RunSQL operations are data migrations
related to an enclosing pair of schema migrations: for instance, add a new
field, migrate data to it from an old field, then remove the old field.
Such operations are generally safe to elide when squashing: in the
squashed and optimized list of operations, old-field should never exist at
all, and thus there is no need for a migration to move data from old-field
to new-field. But the migration system doesn't know this, so it
(correctly) refuses to optimize across the RunPython/RunSQL, causing
schema-altering migrations to be preserved that shouldn't be (i.e.
creation and removal of old-field won't be optimized away, because of the
intervening RunPython/RunSQL).
It's possible to manually remove such RunPython/RunSQL operations before
squashing, but it's a pain to review the whole history for such cases and
manually remove them.
I would find it useful to have an attribute like `elidable` or
`optimize_across` on migration operations (at least on `RunPython` and
`RunSQL`), settable via a kwarg to the constructor, which would default to
`False` (preserving the current behavior), but if set to `True` would mark
the migration as "safe to elide and optimize across". This would never be
set to `True` by the autodetector, it would only ever be set manually by
the user.
Thoughts?
--
Ticket URL: <https://code.djangoproject.com/ticket/24109>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: carljm | Owner: nobody
Type: New feature | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by shaib):
While this appears to be a generally good idea, I think a better solution
for the use-case would be a specific data-move operation, if we can come
up with one that is general enough. Usually, the data-migration is needed
because old-field and new-field are not on the same model (incl. moving
from FK to M2M) -- or because some explicit transformation is required
(e.g. turning some field from string to number). These, I suspect, can be
collected into an operation that the optimizer and the executor can
understand, and perhaps could even be auto-detected in some way.
--
Ticket URL: <https://code.djangoproject.com/ticket/24109#comment:1>
Django
Reporter: carljm | Owner: nobody
Type: New feature | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
That sounds very nice, though it's a much more complex proposal and I'm
not totally convinced about feasibility. If someone has a PR or POC
demonstrating how it would be done, I'm interested, but barring that I'd
still like to have at least the simpler manual version in place. (It seems
possible that your proposal could still build on top of the simpler API I
proposed, exposing both as options for when the higher-level approach
doesn't quite meet your case).
--
Ticket URL: <https://code.djangoproject.com/ticket/24109#comment:2>
Django
Reporter: carljm | Owner: nobody
Type: New feature | Status: new
Component: Migrations | Version: master
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 timgraham):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/24109#comment:3>
Django
Reporter: carljm | Owner: nobody
Type: New feature | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
The idea sounds good, Carl. I'm up for a review.
An automatic detection and output of alterations that includes migrating
data from A to B would be quite some feature, but I doubt that would work
reliable.
--
Ticket URL: <https://code.djangoproject.com/ticket/24109#comment:4>
Django
Reporter: carljm | Owner: nobody
Type: New feature | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
#25413 was a duplicate that proposed an alternative solution: a flag you'd
pass to `squashmigrations` to automatically elide all `RunSQL` and
`RunPython` operations.
--
Ticket URL: <https://code.djangoproject.com/ticket/24109#comment:5>
Django
Reporter: carljm | Owner: nobody
Type: New feature | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by mjtamlyn):
* cc: mjtamlyn (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/24109#comment:6>
Django
Reporter: carljm | Owner: nobody
Type: New feature | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/5957 Tentative implementation]
given we want to/can move the operation reduction logic to their
respective class definition.
--
Ticket URL: <https://code.djangoproject.com/ticket/24109#comment:7>
Django
Reporter: carljm | Owner: charettes
Type: New feature | 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 charettes):
* status: new => assigned
* owner: nobody => charettes
--
Ticket URL: <https://code.djangoproject.com/ticket/24109#comment:8>
Django
Reporter: carljm | Owner: charettes
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by ryanhiebert):
* cc: ryan@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/24109#comment:9>
Django
Reporter: carljm | Owner: charettes
Type: New feature | Status: assigned
Severity: Normal | Resolution:
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
Changes (by timgraham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/24109#comment:10>
Django
Reporter: carljm | Owner: charettes
Component: Migrations | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette <charette.s@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"729e0b086daac92bb9c6eb16f01cd2e13d258fdd" 729e0b08]:
{{{
#!CommitTicketReference repository=""
revision="729e0b086daac92bb9c6eb16f01cd2e13d258fdd"
Fixed #24109 -- Allowed RunSQL and RunPython operations to be elided.
Thanks to Markus Holtermann and Tim Graham for their review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24109#comment:11>
Django
Reporter: Carl Meyer | Owner: Simon Charette
Type: New feature | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"04446b60f29dcbadc1a73bd3a916dcf9a4b0f445" 04446b60]:
{{{
#!CommitTicketReference repository=""
revision="04446b60f29dcbadc1a73bd3a916dcf9a4b0f445"
[1.10.x] Refs #24109 -- Doc'd the elidable feature in squashing migrations
docs.
Backport of d1eda9b4ad47c7773e65d90fd882e9d07759fe41 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24109#comment:12>
Django
Reporter: Carl Meyer | Owner: Simon Charette
Type: New feature | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"d1eda9b4ad47c7773e65d90fd882e9d07759fe41" d1eda9b]:
{{{
#!CommitTicketReference repository=""
revision="d1eda9b4ad47c7773e65d90fd882e9d07759fe41"
Refs #24109 -- Doc'd the elidable feature in squashing migrations docs.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24109#comment:13>