#23076: Cascaded deletion of polymorphic models fails
-------------------------------------+-------------------------------------
Reporter: jernej@… | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) | Resolution: wontfix
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by carljm):
Replying to [comment:6 kostko]:
> Sorry, but the `FIXME` you mention is also present in current Django
master and is not introduced by this patch. The patch simply augments
current code and leaves existing stuff as it is (including the comment).
The only part that it adds is the one that iterates over actual models.
Please see:
>
>
https://github.com/django/django/blob/master/django/db/models/deletion.py#L191
Yes, you're right, sorry about that. The diff showed it in green (probably
because of indentation changes) and I didn't notice that it had already
been there previously. My review of the patch was cursory, since I don't
think this is a bug in Django regardless.
> As for the fix, please advise how would one then properly handle
cascaded deletion of polymorphic models? Or are you saying that
polymorphic models should not be supported and/or used with the Django's
ORM?
I'm saying that until/unless polymorphic models are part of Django and
fully supported by the ORM, Django's code should not be made more complex
in order to accommodate them (unless the changes are otherwise generally
beneficial, for instance providing additional customization points), and
it is the responsibility of a third-party add-on that implements them to
make it work.
For an illustration of the problem, see the fact that the pull request had
no tests, and it would be impossible to implement tests for it without
deep poking into ORM internals. If you can't reproduce the problem using
supported APIs, it's a good sign that the fix for the problem may not
belong in Django.
I'm not familiar enough with django-polymorphic to say how it should fix
the problem. It seems to me that an overridden `queryset.delete()` might
be able to adjust things such that the existing collector could handle it.
If an additional hook or customization point in Django would allow django-
polymorphic to insert the delete-cascade behavior it needs, I'd be more
inclined to support a patch like that, rather than a patch that adds
support to Django's delete-collector for a scenario that Django doesn't
support.
--
Ticket URL: <https://code.djangoproject.com/ticket/23076#comment:8>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/071.59a48465ac6384da589e01a0abf5bdfb%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
