Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#24184 closed Bug (fixed)

Migrate auto-fake behavior is not safe

Reported by: Carl Meyer Owned by: Markus Holtermann
Component: Migrations Version: 1.7
Severity: Normal Keywords:
Cc: Markus Holtermann 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

Currently, manage.py migrate will auto-fake-apply initial migrations if it sees that the tables already exist, but it doesn't do any checking that the schema of those tables actually matches the schema that the migration would have created. And our documentation doesn't call out the importance of making sure that your existing schema matches the models you used to generate the initial migrations.

In an ideal world, we might keep the current behavior and automatically verify a full schema match before auto-faking, but I doubt that's feasible and in any case would be a major new feature.

Short of that, I think the unsafe auto-fake behavior should not be the default; it should require a flag like --fake-if-exists or something (needs bikeshedding) to turn it on. Wherever that flag is documented/demonstrated in the docs (e.g. in the https://docs.djangoproject.com/en/1.7/topics/migrations/#upgrading-from-south upgrade-from-South docs), we should be very clear that it is only safe to use if you are sure that your schema is properly up-to-date with your models used to generate your initial migration.

(This ticket was motivated by #24178.)

Change History (10)

comment:1 Changed 3 years ago by Markus Holtermann

Cc: Markus Holtermann added
Triage Stage: UnreviewedAccepted

comment:2 Changed 3 years ago by Markus Holtermann

Owner: changed from nobody to Markus Holtermann
Status: newassigned

comment:3 Changed 3 years ago by Markus Holtermann

Has patch: set
Needs documentation: set

Initial PR: https://github.com/django/django/pull/3967

Should that be backported to 1.7 given that it's the first version you upgrade to when you move away from South.

comment:4 Changed 3 years ago by Carl Meyer

As discussed in IRC, I don't think we should backport this to 1.7 (much as I'd like to), since it's a backwards-incompatible change in user-facing behavior. We'll just have to put a warning in the docs for 1.7.

comment:5 Changed 3 years ago by Markus Holtermann

Needs documentation: unset

PR for master and 1.8 is ready.

comment:6 Changed 3 years ago by Carl Meyer

Reviewed PR, looks good to me, just needs some doc tweaks and to be merged up with latest master.

comment:7 Changed 3 years ago by Carl Meyer

Triage Stage: AcceptedReady for checkin

comment:8 Changed 3 years ago by Markus Holtermann <info@…>

Resolution: fixed
Status: assignedclosed

In f287bec5833d75750fa6368bc2802741b7924533:

Fixed #24184 -- Prevented automatic soft-apply of migrations

Previously Django only checked for the table name in CreateModel
operations in initial migrations and faked the migration automatically.
This led to various errors and unexpected behavior. The newly introduced
--fake-initial flag to the migrate command must be passed to get the
same behavior again. With this change Django will bail out in with a
"duplicate relation / table" error instead.

Thanks Carl Meyer and Tim Graham for the documentation update, report
and review.

comment:9 Changed 3 years ago by Markus Holtermann <info@…>

In bd80fa6b0f7e5a0cc4ea26cedd56d0c4c4894420:

[1.8.x] Fixed #24184 -- Prevented automatic soft-apply of migrations

Previously Django only checked for the table name in CreateModel
operations in initial migrations and faked the migration automatically.
This led to various errors and unexpected behavior. The newly introduced
--fake-initial flag to the migrate command must be passed to get the
same behavior again. With this change Django will bail out in with a
"duplicate relation / table" error instead.

Thanks Carl Meyer and Tim Graham for the documentation update, report
and review.

Backport of f287bec5833d75750fa6368bc2802741b7924533 from master

comment:10 Changed 3 years ago by Markus Holtermann

I opened #24337 to track the documentation update for 1.7

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