[Django] #25614: MigrationAutodetector is too greedy when ForeignKey(on_delete=...) is changed

11 views
Skip to first unread message

Django

unread,
Oct 26, 2015, 7:41:54 PM10/26/15
to django-...@googlegroups.com
#25614: MigrationAutodetector is too greedy when ForeignKey(on_delete=...) is
changed
--------------------------------------+------------------------
Reporter: jdunck | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
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 |
--------------------------------------+------------------------
Recently I changed the on_delete attributes of many ForeignKeys on a MySQL
database.

This resulted in an auto-detected migration that drops and recreates the
constraint for each FK, despite the effective constraint remaining the
same (e.g. FOREIGN KEY (`modified_by_id`) REFERENCES `common_user`
(`id`); )

The
[https://github.com/django/django/blob/1.8.5/django/db/migrations/autodetector.py#L848
autodetection] just compares the deconstruction.

The
[https://github.com/django/django/blob/1.8.5/django/db/backends/base/schema.py#L486
schema editor] drops the FK and recreates it.

It's not entirely clear to me which part needs to be changed - whether the
deconstruction should not include on_delete, or whether the autodetector
should discard on_delete from consideration, or whether the editor should
check for parameters that actually change the effective FK.

What is clear is that dropping and recreating FKs isn't a fast operation
on large databases, and it would be good to avoid where possible.

--
Ticket URL: <https://code.djangoproject.com/ticket/25614>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Oct 26, 2015, 7:51:00 PM10/26/15
to django-...@googlegroups.com
#25614: MigrationAutodetector is too greedy when ForeignKey(on_delete=...) is
changed
----------------------------+--------------------------------------

Reporter: jdunck | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:

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 jdunck):

Here's the [https://github.com/django/django/commit/b5784048e085e8d
commit] that introduced on_delete on ForeignKey.on_delete. Perhaps
Andrew recalls the rationale of adding it.

--
Ticket URL: <https://code.djangoproject.com/ticket/25614#comment:1>

Django

unread,
Oct 26, 2015, 8:02:24 PM10/26/15
to django-...@googlegroups.com
#25614: MigrationAutodetector is too greedy when ForeignKey(on_delete=...) is
changed
----------------------------+--------------------------------------

Reporter: jdunck | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:

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 jdunck):

And now I've tracked it to #23288, but still think adding on_delete was
likely unneeded.

--
Ticket URL: <https://code.djangoproject.com/ticket/25614#comment:2>

Django

unread,
Oct 27, 2015, 8:43:12 AM10/27/15
to django-...@googlegroups.com
#25614: Changing ForeignKey(on_delete=...) unnecessarily drops and recreates
constraints
----------------------------+--------------------------------------
Reporter: jdunck | Owner: nobody
Type: Bug | Status: closed
Component: Migrations | Version: 1.8
Severity: Normal | Resolution: duplicate

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+--------------------------------------
Changes (by timgraham):

* status: new => closed
* resolution: => duplicate


Comment:

Looks like a duplicate of #25253. We don't currently exclude any model
field attributes from deconstruct so that's unlikely to be the solution
unless we change the design decision around that.

--
Ticket URL: <https://code.djangoproject.com/ticket/25614#comment:3>

Reply all
Reply to author
Forward
0 new messages