Add ON DELETE and ON UPDATE support to Django

[glassfordm]

The attached patch adds two levels of ON DELETE and ON UPDATE
support to Django.

1. Just applying the patch changes nothing: Django should continue

to behave exactly as before, emulating ON DELETE CASCADE behavior
just as it has all along.
However:
1) Adding on_delete=RESTRICT or on_delete=SET_NULL to a ForeignKey
definition will cause Django to emulate the specified behavior
for that particular foreign key. Django will continue to emulate
CASCADE behavior for ForeignKeys which have no on_delete behavior
specified; it will also emulate CASCADE behavior for foreign keys
having on_delete=CASCADE defined, of course.
2) The new setting, settings.ON_DELETE_NONE_HANDLING = <option>,
specifies the behavior of foreign keys that have no on_delete
option specified. It defaults to CASCADE (because this is what
Django has done in the past), but can be set to RESTRICT (essentially
the default for SQL) or SET_NULL as well (although SET_NULL is not
recommended: it will cause errors for ForeignKeys that are not nullable).
3) Finally, the patch does not provide any Django emulation for
ON UPDATE behavior, so, while the on_update option can be added to
foreign key definitions, it does nothing (but see below).

2. Defining settings.ON_DELETE_HANDLED_BY_DB = True changes Django's

ON DELETE behavior completely:
1) It disables Django emulation of ON DELETE behavior entirely.
2) It adds appropriate ON DELETE statements to the SQL generated
by Django (by "python manage.py sqlall", for instance).

3. Defining settings.ON_UPDATE_HANDLED_BY_DB = True adds appropriate

ON UPDATE statements to the SQL generated by Django
(by "python manage.py sqlall", for instance).

[anonymous]

Can not apply the patch to latest django svn trunk (admin newforms merged).
I've manually modified the files, but get the following error when trying to run the application:

 File "/usr/lib/python2.5/site-packages/django/db/models/fields/related.py", line 596, in __init__
    self.on_delete = on_delete
NameError: global name 'on_delete' is not defined

[glassfordm]

Updated patch for the latest version of Django (revision 9845 at the time the patch was created).

[Erin Kelly]

Is there a relevant difference between ON DELETE RESTRICT and the default? I ask because Oracle supports ON DELETE CASCADE and ON DELETE SET NULL but not ON DELETE RESTRICT. For this reason, I think it would be preferable to omit the clause entirely rather than specify ON DELETE RESTRICT.

[glassfordm]

Currently, the default behavior of Django is to emulate ON DELETE CASCADE; for compatibility reasons, I think this needs to remain the same (unless explicitly changed by the developer of a Django site). The ON DELETE RESTRICT option is necessary to tell Django to deviate from its default behavior and prevent the delete from happening rather than cascading it. This is pretty much what ON DELETE RESTRICT means in MySQL (although, now that you mention it, I think I remember that MySQL's implementation is somewhat non-standard). Is there a better wording than "RESTRICT" that means the right thing for Oracle as well as other backends? Is ON DELETE NO ACTION better?

[Erin Kelly]

"ON DELETE NO ACTION" is also not supported. The only way to get the "RESTRICT" behavior with Oracle is to omit the clause entirely.

If I'm understanding correctly, the difference between "RESTRICT" and "NO ACTION" is that "NO ACTION" uses deferred constraint checking. With Oracle, this is specified by omitting the "ON DELETE" clause and adding "DEFERRABLE INITIALLY DEFERRED". This is actually independent of the "ON DELETE" clause and would also affect updates.

I guess the upshot of all this is just that the Oracle backend will need to override the on_delete_and_update_clauses method.

[(none)]

Milestone post-1.0 deleted

[Dokter Bob]

Has anyone been working on this bug? I would very much like to see a current patch for it.

[Russell Keith-Magee]

@Dokter Bob: This isn't a bug, it's a feature request. It's a fairly big feature request, too, so it's going to require a bit of discussion before it gets anywhere near trunk. However, now is not the time for that discussion, as we're in the final stages of the v1.1 release.

[Mathijs de Bruin]

Patch updated for r10558. Very fresh and barely tested, see more info in ticket.

[Mathijs de Bruin]

Note: the patch I've just uploaded does not propagate the new cascading behaviour to the admin. I am not sure whether the 'old version' of this patch did but it is something someone needs to fix. ;)

[benjaoming]

Someone requested a discussion...

I just had an encounter with this, and what I lacked was *knowing* what's going on with regards to cascading deletion, because it's quite dangerous (you loose data!!) and steals lots of time. Preventing it is simple, though. Overriding the delete() method of a model and calling clear() on the related model's foreign keys is simple, and it's a manual implementation, that all programmers should be able to understand.

But I can think of another alternative: If null=True for the foreign key in question, why not automatically use SET NULL when the related instance is deleted? For me, this is even more "intuitive" than CASCADE. After all, null=True is something that the programmer specifies and has to deal with anywhere in the implementation, which is also why he doesn't need cascading deletion of such a relation.

Also, if on_delete is possible to set in a key field, it would have to comply with the null option.. and together with the "intuition argument" that creates a 1:1 correspondence between the two options.

And then the "logic argument": Django handles its logic in Python code, not in the Database, which is kept as a simple storage engine. The RESTRICT option is a validation issue, and will probably be handled this way in most cases, so having the database enforcing it, would be redundant. To enable it on model-level could pave the way for some nice new automatic validation in ModelForms, so I think it sounds like a nice feature.

If all this is implemented, I would suggest to remove the null option from key fields and have it set according to on_delete.

[Malcolm Tredinnick]

Discussion should happen on django-dev, so that everybody can be involved (not everybody monitors every single ticket). We ask that all design discussion happens there. As Russell mentioned, now is not a good time to restart the discussion, since the people with a large piece to contribute are much more focused on the next release and we know we can put this off for a few weeks or months without harm. So keep an eye on django-dev and when discussion opens up regarding 1.2 features, that's the time to pipe up, on the mailing list.

[benjaoming]

I know, but gotta serve the soup while it's still hot. I meant it as a kind of suspended discussion input, to be read in due time. Apologies if that isn't usual procedure.

[Thomas Güttler]

Related: #6108

[daybreaker]

I'm also interested on this issue.

[glassfordm]

Very preliminary patch for 1.2 release

[glassfordm]

I've attached a new patch containing a very preliminary version of what I'm working on for possible inclusion in the 1.2 release. As discussed in the django developer mailing list some time ago, it has a much more limited scope than my original patch in these ways:
1) It removes all ON UPDATE type behavior.
2) It removes the feature that allows the backend to provide the ON DELETE support and to cause Django to generate SQL with ON DELETE clauses in it.
3) It removes the settings that could be added to the settings files to control these behaviors.
It is my hope that #2, and possibly #1, can be added in future versions of Django 1.2.

The patch I've attached is very preliminary in these ways:
1) It contains some debugging code (commented out) and some TODO comments indicating things that need to be improved.
2) Some areas could use efficiency improvements.
3) It contains several intentionally failing tests illustrating problems I haven't yet solved.
4) There are no docs yet.
5) I've made no attempt to support any of the alternate terminology suggestions from this discussion: ​http://groups.google.com/group/django-developers/browse_thread/thread/4ece22d6053f774b/8be0b9be9b6d0b5f. This doesn't mean I won't, just that I haven't. Ideally, I'd prefer comments from more people on the suggestion that were made before choosing one of them.

I've posted this patch to give an idea what the final patch is attempting to implement and to ask for help on the problems I haven't solved yet.

What's here: OneToOne and ForeignKey field types in models have a new attribute, on_delete. This can be set to CASCADE, PROTECT, SET_NULL, or SET_DEFAULT. If any of these attributes are present in a foreign key definition, Django implements the appropriate behavior when an attempt is made to delete an item referenced by that foreign key.

[glassfordm]

Same as previous patch, but with correct extension

[anonymous]

@glassfordm: Is it possible that you forgot to svn add your tests before making the diff? I can't find any on_delete related tests, except a handful of low level tests for CollectedFields.

The patch I just attached is based on on_delete_on_update-r11620.diff. I made the following changes:

• Removed checks for default and null in _handle_sub_obj() (as these are static properties of fields and already checked in contribute_to_class()). These checks should perhaps be moved to django.core.management.validation.

• Moved instance-update and sql-update code inside CollectedFields so the actual data structure doesn't have to be known to code outside. Is there a reason field updates are not simply collected in CollectedObjects, too?

• Changed how update queries are performed: instead of one per instance, it now uses one (batch) per field. Except for corner cases (more than GET_ITERATOR_CHUNK_SIZE foreign keys on a single model affected), this results in fewer queries.

• Added a pony of mine: on_delete=DO_NOTHING, which would help with #10829.

I didn't write any tests (bad), but all existing tests pass. Which hopefully means that at least I haven't broken CASCADE and SET_NULL.

Regarding stale related object caches, I'd do nothing special. Django currently doesn't refresh them, so any change here (for SET_NULL or CASCADE) would be backwards incompatible. And there is no guarantee the cache won't be stale again, when _handle_sub_obj() is called anyway.
So this is a documentation issue with Model.delete() at best, since QuerySet.delete() always uses "fresh" data.

[Johannes Dollinger]

Changes between 7539.on_delete.diff and 7539.on_delete.r11706.diff:

• Added tests.

• Replaced class based constants with callables that do what _handle_sub_object() did before. That cleans up _collect_sub_objects() and allows on_delete behaviors to be added without touching any existing code (I added on_delete=SET(value), because it's just a four line utility function).

• Merged CollectedFields into CollectedObjects

• Made all operations that require knowledge of internals methods on CollectedObjects. This makes django.db.models.base.delete_objects more readable.

• Couldn't resist and rewrote parts of CollectedObjects.

[Johannes Dollinger]

Changes between 7539.on_delete.r11706.diff and 7539.on_delete.r11724.diff:

• Updated to work with m2m-refactor
• Moved delete_objects() inside CollectedObjects: CollectedObjects.delete()
• Got rid of DeleteQuery.delete_batch_related(): use the normal code path for m2m intermediary models. This is required to enable on_delete for foreign keys on intermediary models. But it results in a dramatical increase of SELECT queries. Therefore ..
• Moved _collect_sub_objects() to CollectedObjects and changed its algorithm to collect related objects in batches. This reduces the number of SELECT queries for related objects. The delete call in the following example requires a total of 103 queries in trunk, while the patch only requires 4 queries: related C instances will be looked up in one bulk query.

  class A(models.Model): pass
  
  class B(models.Model):
   a = models.ForeignKey(A)
  
  class C(models.Model):
   b = models.ForeignKey(B)        
  
  a = A.objects.create()
  for i in xrange(100): B.objects.create(a=a)    
  a.delete()
  
  

• Introduced a batch (don't select, just delete) option to get rid of the one remaining extra query for auto_created intermediary models.
• Altered collect() to collect inheritance parents without additional queries.
• Moved CollectedObjects into its own module, as it now encapsulates the whole deletion logic and depends on db.models.sql.

TODO:

• There is an ugly contrib.contenttypes import in subqueries.py. Deleting generic relations should be handled by a custom on_delete handler instead.

[Johannes Dollinger]

Changes between 7539.on_delete.r11724.diff and 7539.on_delete.r11724.2.diff:

• Corrected the default behavior for nullable foreign keys (SET_NULL did not cause any test failures for those)
• Moved CASCADE, PROTECT, SET, SET_NULL, SET_DEFAULT, and DO_NOTHING to db.models.deletion
• Renamed CollectedObjects to Collector
• Added a couple of docstrings
• Moved sanity checks to core.management.validation

[glassfordm]

Same as my previous patch, but including unintentionally omitted unit tests

[anonymous]

related: #12382

Related to this issue is the way that the admin displays delete confirmations:

such as:

Contact: John Smith
 Phone number: 555-1212 (work)
   Contact: John Smith
     Phone number: 555-1212 (work)
       Contact: John Smith
         Phone number: 555-1212 (work)
           Contact: John Smith
             Phone number: 555-1212 (work)
               Contact: John Smith

[cgieringer]

Related: #6870

[David Pratten]

Replying to jakub:

[Luc Saffre]

I tried to apply the patch to revision 14140, but there were quite some failures. Is it asked much to post a new patch against the latest version? Here are the error messages I got:

L:\snapshots\django>patch -p0 < 7539.on_delete.r12380.diff
patching file tests/modeltests/invalid_models/models.py
Hunk #1 FAILED at 181.
Hunk #2 FAILED at 285.
2 out of 2 hunks FAILED -- saving rejects to file tests/modeltests/invalid_models/models.py.rej
patching file tests/modeltests/delete/models.py
Hunk #1 FAILED at 44.
Hunk #2 FAILED at 79.
Hunk #3 FAILED at 93.
Hunk #4 FAILED at 107.
Hunk #5 FAILED at 128.
5 out of 5 hunks FAILED -- saving rejects to file tests/modeltests/delete/models.py.rej
patching file tests/modeltests/on_delete/__init__.py
patching file tests/modeltests/on_delete/models.py
patching file django/db/models/sql/subqueries.py
Hunk #1 FAILED at 26.
Hunk #2 succeeded at 45 (offset -43 lines).
1 out of 2 hunks FAILED -- saving rejects to file django/db/models/sql/subqueries.py.rej
patching file django/db/models/base.py
Hunk #1 FAILED at 7.
Hunk #2 FAILED at 536.
2 out of 2 hunks FAILED -- saving rejects to file django/db/models/base.py.rej
patching file django/db/models/options.py
Hunk #1 succeeded at 370 (offset -6 lines).
patching file django/db/models/fields/related.py
Hunk #2 succeeded at 734 (offset 46 lines).
Hunk #3 succeeded at 744 (offset 46 lines).
Hunk #4 succeeded at 764 (offset 46 lines).
Hunk #5 succeeded at 820 with fuzz 1 (offset 49 lines).
patching file django/db/models/__init__.py
patching file django/db/models/deletion.py
patching file django/db/models/query.py

[Johannes Dollinger]

Replying to lsaffre:

I tried to apply the patch to revision 14140, but there were quite some failures. Is it asked much to post a new patch against the latest version?

I ported the patch to r14218. Most of the changes that broke the old code were introduced by the multi-db refactoring. It works now (as in: the tests pass), but I haven't added explicit tests for on_delete behavior in the presence of multiple databases.

And there's a whole new problem caused by a ​a piece of code with a TODO comment in contrib.admin. I haven't bothered to fix the test failures that are related to this dependency.

Feedback welcome.

[Johannes Dollinger]

For reference: r12598 introduced the offending contrib.admin code to fix #6191 and #11296.

[Alex Gaynor]

Do those admin tests pass with the latest code (I'll be looking at reviewing this and I know carl meyer is as well)?

[Carl Meyer]

Looks like the admin code can be updated to use the new API without too much difficulty. We may also want to add an additional warning in the admin regarding objects that won't be deleted, but will be modified (have an FK nulled out or set to some other value).

[Carl Meyer]

Done some work on this patch, more to be done yet (see remaining FIXME comments in deletion.py). Work in progress here: ​http://github.com/carljm/django/compare/master...t7539

[Carl Meyer]

The github branch linked in my last comment passes all tests and, once I get some docs in it, could be RFC.

What it still lacks is support for GenericForeignKey / GenericRelation; these still have hardcoded cascade-delete behavior, as they did previously. I could be open to landing this without GFK support: there's no regression in GFK behavior, they just don't get the new cascade-delete configurability.

I do have some working proof-of-concept support for GFKs in a separate branch (​http://github.com/carljm/django/compare/t7539...t7539-gfk), which also passes all tests (including a new one demonstrating on_delete=SET_NULL for a GFK), but I'm not entirely happy with the API. It requires GFK on-deletes to be selected from a separate set of functions (found in django.contrib.contenttypes.generic) from the ones used for regular FKs (found in django.db.models). This may be OK, but seems prone to user error (and would certainly need some added validation code to make the error at least intelligible).

I think it would be possible to allow GFK on_delete to use the same set of behavior-functions as regular FK on_delete, but it would require a significant refactor of the patch. The approach I envision would push some key chunks of logic (e.g. bulk-fetching related objects, what it means to update a field) into methods on RelatedObject/GenericRelation, so the stuff in models/deletion.py can be agnostic about what kind of relation it's dealing with.

Feedback welcome. One way or the other, I'm hoping to land this in the next week or two (with a wider call for feedback on django-developers first).

[Carl Meyer]

The current patch here also fixes #13067 and #14599. Just so I don't forget.

[Carl Meyer]

(In [14507]) Fixed #7539, #13067 -- Added on_delete argument to ForeignKey to control cascade behavior. Also refactored deletion for efficiency and code clarity. Many thanks to Johannes Dollinger and Michael Glassford for extensive work on the patch, and to Alex Gaynor, Russell Keith-Magee, and Jacob Kaplan-Moss for review.