Opened 4 years ago

Last modified 4 months ago

#21961 assigned New feature

Add support for database-level cascading options

Reported by: Christophe Pettus Owned by: Nick Stefan
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: emorley@…, andre@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


Per a discussion on -developers, this ticket is to document this proposed new feature.

The general idea is to allow developers to specify that the database itself, rather than Django, should implement foreign key constraints where possible. The database can be considerably more efficient, and often can avoid locking situations, in a way that the Django backend can't.

The proposal is to add a models.CASCADE_DB, models.SET_NULL_DB, models.PROTECT_DB constants. These specify that the appropriate functionality is to be implemented in the database if the database supports it; otherwise, they are implemented in the Django core as now.

Some potential design issues with proposed solutions:

  1. It is not an error to specify the _DB version with a database that doesn't support it. Instead, you get the non-_DB version.
  2. The _DB version does not fire signals; this will need to be documented.
  3. If a model A references a model B using CASCADE_DB, but model B references model C using regular CASCADE, a deletion of A won't cascade all the way to C. This needs to be documented. Another possibility is to make this an error condition and have model validation fail, but that seems excessive.
  4. The _DB version is disallowed on generic foreign keys, because that way madness lies.
  5. The implicit foreign key created from child to parent in model inheritance is never the _DB version. This is a shame, but there are two issues:

a) Since it's created implicitly, there's no place to put it.
b) Even if we extended the mechanism to allow manual specification of the key, deleting the parent wouldn't automatically delete the child.

Change History (15)

comment:1 Changed 4 years ago by Marc Tamlyn

Triage Stage: UnreviewedAccepted

If 3 can be introspected for (which should be possible) we can at least implement a check for it in the new checks framework.

comment:2 Changed 4 years ago by Aymeric Augustin

  1. is tricky. My instinct would be to fail with an exception when the code requires something that cannot be achieved. However, I understand that pluggable apps may want to take advantage of database-level cascades, while still supporting less capable databases. Short of introducing a third value (CASCADE_DB_PROVIDED_YOU_ARE_USING_A_REAL_DATABASE), your proposal is probably the best solution.

A similar argument can be made for 3.

5 might be just another argument against MTI...

comment:3 Changed 4 years ago by Simon Charette

  1. It is possible to specify the key:
class Parent(models.Model):

class Child(Parent):
    parent = models.OneToOneField(Parent, parent_link=True)

comment:4 Changed 4 years ago by Anssi Kääriäinen

I think the description has parent and child deletion mixed. Deleting the parent will delete the child (there is a key from child to parent). The problem is child deletion. In Django deleting the child cascades to the parent row, too. But there is no column from parent to child you can cascade along if you do cascades in the DB. The real problematic case:

class Related(models.Model):

class Parent(models.Model):

class Child(Parent):
    related = models.ForeignKey(Related, on_delete=DB_CASCADE)

When you delete Related instance, Child instances pointing to it will be deleted by db cascade, but the parent instances will be left alone. If you instead use standard CASCADE, then the parent instances will be deleted, too. This is surprising behaviour. It can be documented, but erroring out would be IMO better.

comment:5 Changed 4 years ago by Shai Berger

1+2 => When using CASCADE_DB, signals will fire only on backends which do not support the feature.

Similar issues, of course, for 1+3, 1+5.

I find this result quite disturbing.

Alternative: When using CASCADE_DB on a backend where the database cannot implement it, Django tries to emulate it; this is not equivalent to CASCADE.

comment:6 Changed 3 years ago by Tim Graham

Version: 1.7-alpha-1master

comment:7 Changed 3 years ago by Alexander Schepanovski

For me it actually ok that signals won't be called and some deletion propagation could be broken. It's kind of .raw() and .extra() if you mess with database you should handle all these things.

My use case is using a database with another non-django app. So I can't rely on Djangos cascading.

comment:8 Changed 3 years ago by Anssi Kääriäinen

I won't oppose an approach where checks framework will warn about dangerous behavior.

For issue 1+2 mentioned by Shai in comment:5: we should disable signals when cascading along CASCADE_DB converted to normal CASCADE due to not having support for CASCADE by the backend.

comment:9 Changed 2 years ago by Carl Meyer

I think we should consider introducing this feature as a totally separate kwarg (on_delete_db) rather than conflating it with the existing Python-level on_delete. The various edge cases and implicit fallbacks in the existing proposal worry me, and I think it would be better to be more explicit and clear about what is happening on the database level vs the Python level.

Right now the contract of on_delete is very simple: it takes a Python callable which will be called when a delete cascades. There is nothing at all special about the built-in callables, they are just conveniences for common cases. The current API proposal complicates that contract dramatically: now you have an on_delete kwarg which sometimes accepts Python callables but can also accept magical constants which do something entirely different (and also have an implicit fallback relationship with one of the built-in callables).

So under my proposal, if you want database-level cascade, you would specify on_delete=models.DO_NOTHING, on_delete_db=models.DB_CASCADE or similar. Sure this is a bit more verbose, but I think that's worth it for the gains in clarity and simplicity.

For similar reasons, I feel pretty strongly that we should NOT automatically fallback from db-cascade to non-db-cascade depending on the backend capabilities. It introduces too many differences in behavior depending on db backend. Trying to claim to provide cross-db portability when we can't really do so consistently is worse than simply not claiming to provide it at all. DB-level cascade (like many other "advanced" db-level features) is something you should only take advantage of when you know your code will be running on a database that supports it, full stop. Use of on_delete_db on a backend that can't support it should be an error.

comment:10 Changed 2 years ago by Tim Graham

Summary: ForeignKey.on_delete supports database-level cascading optionsAdd support for database-level cascading options

comment:11 Changed 2 years ago by Gustavo Narea

In case anyone is interested in a workaround for PostgreSQL, I created this simple plugin:

That's meant as a temporary solution, until Django supports this out of the box.

comment:12 Changed 15 months ago by Ed Morley

Cc: emorley@… added

comment:13 Changed 9 months ago by André Cruz

Cc: andre@… added

comment:14 Changed 4 months ago by Nick Stefan

Owner: changed from nobody to Nick Stefan
Status: newassigned

comment:15 Changed 4 months ago by Nick Stefan

postgresql and mysql are fine. However, for SQLite to use ON DELETE, we would need a new OPTIONS flag. Something like:

settings.DATABASES.["default"]["OPTIONS"]["PRAGMA_foreign_keys"] = True

(which would then lead to the database wrapper running the correct SQL:

If I add the pragma flag, the test suite fails quite a few tests (50+). Therefore, SQLite PRAGMA foreign keys, seems to be a bigger and separate scope. It does have its own ticket here So postgresql and mysql are it, for now.

Note: See TracTickets for help on using tickets.
Back to Top