Opened 5 years ago

Closed 3 years ago

Last modified 3 years ago

#24109 closed New feature (fixed)

Provide a way to mark a migration operation as elidable when squashing

Reported by: Carl Meyer Owned by: Simon Charette
Component: Migrations Version: master
Severity: Normal Keywords:
Cc: Marc Tamlyn, ryan@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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?

Change History (13)

comment:1 Changed 5 years ago by Shai Berger

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.

comment:2 Changed 5 years ago by Carl Meyer

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).

comment:3 Changed 5 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

comment:4 Changed 5 years ago by Markus Holtermann

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.

comment:5 Changed 4 years ago by Carl Meyer

#25413 was a duplicate that proposed an alternative solution: a flag you'd pass to squashmigrations to automatically elide all RunSQL and RunPython operations.

comment:6 Changed 4 years ago by Marc Tamlyn

Cc: Marc Tamlyn added

comment:7 Changed 4 years ago by Simon Charette

Has patch: set

Tentative implementation given we want to/can move the operation reduction logic to their respective class definition.

comment:8 Changed 4 years ago by Simon Charette

Owner: changed from nobody to Simon Charette
Status: newassigned

comment:9 Changed 4 years ago by Ryan Hiebert

Cc: ryan@… added

comment:10 Changed 3 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:11 Changed 3 years ago by Simon Charette <charette.s@…>

Resolution: fixed
Status: assignedclosed

In 729e0b08:

Fixed #24109 -- Allowed RunSQL and RunPython operations to be elided.

Thanks to Markus Holtermann and Tim Graham for their review.

comment:12 Changed 3 years ago by Tim Graham <timograham@…>

In 04446b60:

[1.10.x] Refs #24109 -- Doc'd the elidable feature in squashing migrations docs.

Backport of d1eda9b4ad47c7773e65d90fd882e9d07759fe41 from master

comment:13 Changed 3 years ago by Tim Graham <timograham@…>

In d1eda9b:

Refs #24109 -- Doc'd the elidable feature in squashing migrations docs.

Note: See TracTickets for help on using tickets.
Back to Top