Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#24037 closed Bug (fixed)

Migrating a legacy table results in data loss

Reported by: Bibhas C Debnath Owned by: Tim Graham
Component: Migrations Version: 1.7
Severity: Release blocker Keywords:
Cc: Simon Charette 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

I created several models for our legacy database using inspectdb and hence all those models had their managed flag set to False. Now I need to make some changes to that model, hence I removed the managed flag and the generated migration wants to drop my table first and then create it again. Which leads to loss of all my data.

So I edited the migration file and removed the DeleteModel operation. And faked that migration to avoid the error raised by CreateModel as the table already exists.

But now I cannot go backward beyond that migration as the backward operation of CreateModel will drop my table anyway and there is no way to make a migration irreversible or ask it Not to drop my table.

What would help in such cases is a way to handle the backward migration. So that I can either choose not to drop my table or raise an exception to make the migration irreversible. Or at least make the CreateModel operation irreversible. I found that the Operation class has a flag called reversible. But I could find no way to set it to False.

For now I've added a fake RunPython operation that returns None to make that migration irreversible, as suggested by Markus / here. But I'd love some way to handle it properly.

Change History (7)

comment:1 Changed 6 years ago by Simon Charette

Cc: Simon Charette added
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Shouldn't the autodetector simply issue a AlterModelOptions to change the model state from managed=False to True in this case? This would be no-op at the database level.

comment:2 Changed 6 years ago by Markus Holtermann

Severity: NormalRelease blocker

Yes, precisely charettes. The related discussion on django-users is https://groups.google.com/forum/m/#!topic/django-users/w9aVT3NOpkM

Last edited 6 years ago by Tim Graham (previous) (diff)

comment:3 Changed 6 years ago by Tim Graham

Owner: changed from nobody to Tim Graham
Status: newassigned

comment:4 Changed 6 years ago by Tim Graham

Has patch: set

comment:5 Changed 6 years ago by Claude Paroz

Triage Stage: AcceptedReady for checkin

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

Resolution: fixed
Status: assignedclosed

In 061caa5b386681dc7bdef16918873043224a299c:

Fixed #24037 -- Prevented data loss possibility when changing Meta.managed.

The migrations autodetector now issues AlterModelOptions operations for
Meta.managed changes instead of DeleteModel + CreateModel.

Thanks iambibhas for the report and Simon and Markus for review.

comment:7 Changed 6 years ago by Tim Graham <timograham@…>

In 51ea30a43bafad40b4a24ad0be346796a9c7ab6a:

[1.7.x] Fixed #24037 -- Prevented data loss possibility when changing Meta.managed.

The migrations autodetector now issues AlterModelOptions operations for
Meta.managed changes instead of DeleteModel + CreateModel.

Thanks iambibhas for the report and Simon and Markus for review.

Backport of 061caa5b386681dc7bdef16918873043224a299c from master

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