Feature proposal: models.CALL_DELETE or effective equivalent

41 views
Skip to first unread message

Yo-Yo Ma

unread,
Dec 2, 2011, 12:26:14 AM12/2/11
to Django developers
It's quite a common practice to customize a model's delete method.
This normally leads to the recommended practice of "just loop through
instances and delete each in lieu of using QuerySet.delete". That
works well, within limited context. However, when you've got a model
that's high up in the food chain (e.g. Tenant) with more than a few
foreign keys pointing to it, having to override Tenant.delete to
account for each of it's dependents that have custom delete methods
leads to tight coupling. Furthermore, pointing foreign keys to out-of-
band models like User leads to more bad practices, like monkey
patching.

My proposal is to add a new on_delete=models.SET-ish feature (or a new
kwarg altogether) which would, when specified, cause the parent
object's delete to loop through an iterator of the related instances
and call the delete method on each for me.  I wouldn't think that a
transaction would need to be applied in-place, as the docs could
merely mention the warning about multiple queries, inferring the need
for a transaction (leaving flexibility).

table = models.ForeignKey(Restaurant, on_delete=models.CALL_DELETE)

or

table = models.ForeignKey(Restaurant, bulk_delete=False)

I prefer the former to the latter.

Yo-Yo Ma

unread,
Dec 2, 2011, 12:45:20 AM12/2/11
to Django developers
My "table" typo was intended to be the "restaurant" of a Table model
(it's late :)

Adrian Holovaty

unread,
Dec 2, 2011, 12:51:48 AM12/2/11
to django-d...@googlegroups.com
On Thu, Dec 1, 2011 at 11:26 PM, Yo-Yo Ma <baxters...@gmail.com> wrote:
> My proposal is to add a new on_delete=models.SET-ish feature (or a new
> kwarg altogether) which would, when specified, cause the parent
> object's delete to loop through an iterator of the related instances
> and call the delete method on each for me.

Hello Yo-Yo Ma,

I don't quite understand the use-case for this. So if the model is this...

class Table(models.Model):
restaurant = models.ForeignKey(Restaurant, on_delete=models.CALL_DELETE)

Wouldn't that suggest that deleting the table would delete the
underlying restaurant? That seems backwards -- cascading deletion
works the *other* way (e.g., if you delete a restaurant, then any
Table with a ForeignKey to it would get deleted). Sorry if I'm being
dense and am missing something.

Adrian

Yo-Yo Ma

unread,
Dec 2, 2011, 12:57:00 AM12/2/11
to Django developers
Adrian,

My apologies, what I'm meaning is: on deletion of the restaurant,
rather than issuing a bulk SQL deletion of the Table instances, call
the delete method for each.

Yo-Yo Ma

unread,
Dec 4, 2011, 3:14:13 PM12/4/11
to Django developers
Did my last post answer the question you had, Adrian?

Justin Holmes

unread,
Dec 4, 2011, 5:53:22 PM12/4/11
to django-d...@googlegroups.com
Doesn't using the pre_delete signal accomplish this?

Or is your main objection having to branch against the specific
models, leading to the coupling you are talking about? It surely
solves the monkey-patching problem, though, no?

On Sun, Dec 4, 2011 at 3:14 PM, Yo-Yo Ma <baxters...@gmail.com> wrote:
> Did my last post answer the question you had, Adrian?
>

> --
> You received this message because you are subscribed to the Google Groups "Django developers" group.
> To post to this group, send email to django-d...@googlegroups.com.
> To unsubscribe from this group, send email to django-develop...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
>

--
Justin Holmes

Head Instructor, SlashRoot Collective
SlashRoot: Coffee House and Tech Dojo
60 Main Street
New Paltz, NY 12561
845.633.8330

Yo-Yo Ma

unread,
Dec 4, 2011, 11:28:53 PM12/4/11
to Django developers
> Or is your main objection having to branch against the specific

Simply put, there should be an *optional* way to ensure a model's
*explicitly* defined delete behavior is honored without having to
write hacks, signals, and/or patches of any kind (ie, make it DRY, and
therefore less error-prone). This seems like common sense, so I really
don't know how much more I can clarify the topic.

Justin Holmes

unread,
Dec 5, 2011, 12:46:15 AM12/5/11
to django-d...@googlegroups.com
"hacks, signals, and/or patches"

One of these things is not like the other.

The signals framework is made for precisely for cases like the one you
describe. Why do you compare it to hacks / patches?

Your signal can be utterly DRY and you can write unit tests for it
(although, if you are using a template in the callable, you may fall
prey to the problem I tried to patch in #17032).

In every way I am currently able to contemplate, using a signal is
more straightforward and decoupled than the solution you propose. I
may well be missing something; please point it out if that's so.

Yo-Yo Ma

unread,
Dec 5, 2011, 10:31:35 AM12/5/11
to Django developers

On Dec 4, 10:46 pm, Justin Holmes <jus...@justinholmes.com> wrote:
> "hacks, signals, and/or patches"
>
> One of these things is not like the other

While I agree that signals allow for neat decoupling, they aren't as
DRY or quick as a simple field kwarg. Imagine you have 8 models that
have custom delete methods. Which is easier: A) avstracting the
functionality of each of the 8 delete methods into helper functions
and writing signals for all 8, or B) typing on_delete=CALL_DELETE? How
will this question be answered, objectively?

Carl Meyer

unread,
Dec 5, 2011, 10:58:34 AM12/5/11
to django-d...@googlegroups.com
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/05/2011 08:31 AM, Yo-Yo Ma wrote:
> While I agree that signals allow for neat decoupling, they aren't as
> DRY or quick as a simple field kwarg. Imagine you have 8 models that
> have custom delete methods. Which is easier: A) avstracting the
> functionality of each of the 8 delete methods into helper functions
> and writing signals for all 8, or B) typing on_delete=CALL_DELETE? How
> will this question be answered, objectively?

Clearly the latter. But "what is easier to type" is not, of course, the
only relevant consideration. A couple notes:

1. Implementing the proposed on_delete handler would require a major
reworking of how on_delete handlers function. Currently, they allow for
modifying the list of objects to be deleted, but not changing how the
delete actually happens. To make the latter possible, while still doing
things correctly w.r.t. deletion ordering in the presence of FKs, etc,
would be far from a trivial patch.

2. The proposal would have abysmal performance characteristics on a
large delete. Thus, it would have to come with big red warnings in the
docs. I'm generally averse to adding features that require big red
warnings in the docs.

Carl
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk7c6ioACgkQ8W4rlRKtE2d2mgCgq0bOAHbMj4b9qbAeeeTi9VUx
PFYAoNiYP5+g9r/EbNiy6qj17oKlPmfP
=TblP
-----END PGP SIGNATURE-----

Yo-Yo Ma

unread,
Dec 5, 2011, 11:03:41 AM12/5/11
to Django developers
Hi Carl,

Thanks for the reply. My guess is that adding a new, optional Meta
argument, (e.g. ``call_delete_on_fk_delete = True``) would be much
simpler to implement. I used ``on_delete`` to make it easy to
understand what I'm asking for, though if it is possible to achieve
this way, I'd be willing to work in that direction.

> Comment: Using GnuPG with Mozilla -http://enigmail.mozdev.org/

Reply all
Reply to author
Forward
0 new messages