[Django] #23034: Migrations fail with modified ManyToMany "through" model
Django
-------------------------------+----------------------
Reporter: lorinkoz@… | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.7-rc-1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+----------------------
Adding a "through" model on a ManyToMany field make "makemigrations" fail
with an explicit error stating that it is not possible. I have worked it
around by renaming the field, creating the migration (so as to make Django
believe it's a new field) and then renaming the field back to the original
name and creating another migration (which is correctly acknowledged by
Django as a simple "renaming"). I understand the implications of adding a
through model from the migrations perspective, but maybe a more friendly
output could be achieved. Is it even possible to create some sort of
interaction, like when you create a field with not null and no default
value, so as to ask the user if they want to destroy/create a new field??
--
Ticket URL: <https://code.djangoproject.com/ticket/23034>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-----------------------------+------------------------------------
Reporter: lorinkoz@… | Owner: nobody
Type: New feature | Status: new
Component: Migrations | Version: 1.7-rc-1
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 timo):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
* type: Uncategorized => New feature
* stage: Unreviewed => Accepted
Comment:
As Andrew noted in #22476, "as the error message says, you cannot alter an
M2M without a through model into one with a through model, at least not in
the current system." I'm not sure how feasible it will be to add support
for thits, but tentatively accepting.
--
Ticket URL: <https://code.djangoproject.com/ticket/23034#comment:1>
Django
Reporter: lorinkoz@… | Owner: nobody
Type: New feature | Status: new
Component: Migrations | Version: 1.7-rc-1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Someday/Maybe
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by andrewgodwin):
* stage: Accepted => Someday/Maybe
Comment:
Triaging this to "someday/maybe". It would be nice, but it's a complex
thing to add.
--
Ticket URL: <https://code.djangoproject.com/ticket/23034#comment:2>
Django
-----------------------------+------------------------------------
Type: New feature | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by knbk):
* owner: nobody => knbk
* status: new => assigned
* version: 1.7-rc-1 => master
* stage: Someday/Maybe => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/23034#comment:3>
Django
-----------------------------+------------------------------------
Reporter: lorinkoz@… | Owner: knbk
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 knbk):
* has_patch: 0 => 1
Comment:
PR: https://github.com/django/django/pull/4312
--
Ticket URL: <https://code.djangoproject.com/ticket/23034#comment:4>
Django
-----------------------------+------------------------------------
Reporter: lorinkoz@… | Owner: knbk
Type: New feature | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by knbk):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/23034#comment:5>
Django
-----------------------------+------------------------------------
Reporter: lorinkoz@… | Owner: knbk
Type: New feature | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by knbk):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/23034#comment:6>
Django
-----------------------------+------------------------------------
Reporter: lorinkoz@… | Owner: knbk
Type: New feature | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
Markus reviewed the PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/23034#comment:7>
Django
-----------------------------+------------------------------------
Reporter: lorinkoz@… | Owner: knbk
Type: New feature | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by CarstenF):
* cc: carsten.fuchs@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/23034#comment:8>
Django
Reporter: lorinkoz@… | Owner: Marten Kenbeek
Type: New feature | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by Simon Charette):
FWIW a workaround for the current state of things, as long as
`through._meta.db_table` points to the previously automatically generated
`db_table`, is to wrap the generated operations in a
`SeparateDatabaseAndState`.
--
Ticket URL: <https://code.djangoproject.com/ticket/23034#comment:9>
Django
-----------------------------+------------------------------------
Type: New feature | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by Marten Kenbeek):
* owner: Marten Kenbeek => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/23034#comment:10>
Django
Reporter: lorinkoz@… | Owner: Simon Charette
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by Simon Charette):
* owner: (none) => Simon Charette
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/23034#comment:11>
Django
Reporter: lorinkoz@… | Owner: (none)
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by Simon Charette):
* owner: Simon Charette => (none)
* status: assigned => new
Comment:
I'm not actively working on this ticket so I'll deassign myself but I made
some progress by using a different approach.
My idea was to teach the auto-detector to detect `ManyToMany(through)`
addition where through is a model compatible with the previously auto-
created model. When it detected this case it would use the questioner to
ask the user whether or not the previous table should be reused. On a
"yes" a `AddManyToManyThrough` operation that has a noop
`database_forwards` but an appropriate `state_forwards` to deal with the
`ModelState` insertion would be added to the operations. If the new
`db_table` wasn't matching or fields were added then the appropriate
`AlterModelTable` and `AddField` operations would be added after the
`AddManyToManyThrough` as well.
`[CreateModel('Foo'), AddManyToManyThrough('Foo', 'bars',
ManyToMany('Bar', through='FooBar')]` could be optimized into
`[CreateModel('Foo'), CreateModel('FooBar', fks), AddField('Foo', 'bars',
ManyToMany('Bar', through='FooBar')]` and the normal chain of
optimizations could then take place.
The work so far is available on this
[https://github.com/django/django/compare/master...charettes:ticket-23034
branch]
--
Ticket URL: <https://code.djangoproject.com/ticket/23034#comment:12>
Django
-----------------------------+------------------------------------
Reporter: lorinkoz@… | Owner: (none)
Type: New feature | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by agger-magenta):
As a developer bitten by this I'd like to second this motion: A customer
changed their data model mid-project, and now we need to add fields to a
many-to-many relationship & I'm looking at a) deleting all migrations and
reinitializing the DB or b) make one of the workarounds here play without
being too much of a hack.
I'd be grateful to be able to do this automatically one day, I think the
use case is common enough.
--
Ticket URL: <https://code.djangoproject.com/ticket/23034#comment:13>
Django
-----------------------------+------------------------------------
Reporter: lorinkoz@… | Owner: (none)
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by Mariusz Felisiak):
As far as I'm aware, this transformation is too fragile to be automated
(see https://youtu.be/4QF9UHEFPCY). It's also
[https://docs.djangoproject.com/en/stable/howto/writing-
migrations/#changing-a-manytomanyfield-to-use-a-through-model documented]
how to do this with `SeparateDatabaseAndState()`.
--
Ticket URL: <https://code.djangoproject.com/ticket/23034#comment:14>