Opened 5 years ago
Closed 5 years ago
#31243 closed New feature (needsinfo)
Implement set/remove/clear for non-nullable Reverse FKs
| Reported by: | Clayton Daley | Owned by: | nobody |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Unreviewed | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
In #31242 I note that the current implementation of set for non-nullable reverse FKs is semantically incorrect and, to fix the immediate bug, propose removing the method (as Django does for clear and remove).
However, I'd also like to make the case for implementing the full stack (set/clear/remove) to support deletion. This makes sense because I'm about to be the next in a long line of people who are duplicating the exact same logical flow as set (using delete instead of the absent remove) for non-nullable reverse FKs.
On the surface, it makes sense that a set would delete non-nullable objects, but I appreciate that there are cases where forced reassignment is desirable. However, I believe these cases are coincident with on_delete=PROTECT.
Can you think of a common case where you need to preserve the related object under full deletion but not under partial (or vice versa)? For example, consider House and Owner (where house has one owner by FK). If we're building a system for a county assessor, it makes sense to preserve the House (and related objects) so you don't want owner.houses.set() to delete the House. But you also don't want owner.delete() (e.g. on death) to cascade to House either. If on_delete already expresses the model-level constraint we need to respect, the implementation is relatively easy.
If you took this opinion to its logical extreme:
- PROTECT prevents removal by reverse FK (even for nullables)
- SET_NULL (only valid when nullable) allows removal by setting to
None - CASCADE positively indicates that removal is allowed (and that it should result in deletion)
The idea of forcing deletes under CASCADE is a breaking change. I think it best reflects the model-level semantics of ON_DELETE, but am happy to preserve backwards compatibility (i.e. sets null for nullable) if that were the only major objection to the proposal.
Similar to #31242, I think this needs discussion on the DevelopersMailingList. I guess it makes sense to make one post suggesting what you'd like to do.(Showing your sample implementation might help...)