on_delete should be a required parameter for ForeignKey
|Reported by:||carljm||Owned by:||fcurella|
|Component:||Database layer (models, ORM)||Version:||master|
|Cc:||hv@…, flavio.curella@…||Triage Stage:||Accepted|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
Description (last modified by carljm)
(Update: consensus below was to make on_delete a required argument, rather than twiddling with its defaults.)
This wasn't done when we added the on_delete feature due to concerns about breaking backwards-compatibility with the previous always-cascade-deletes behavior. But if you set aside legacy considerations, it seems intuitively obvious (to me anyway) that SET_NULL is a superior default for a nullable FK than CASCADE. And even if others might have a different intuition, the consequences of your intuition being wrong are far from symmetrical: one way you get a model instance with a null FK hanging around that you didn't expect to still have, the other way you get data loss.
The trick, of course, is that changing default values is hard to do with a deprecation process. For a deprecation warning to catch the people it needs to catch, you have to warn anytime an explicit value is not specified. So you have to force everybody to stop relying on the default in order to silence the deprecation warnings, which reduces the value of having a new default, in the short run anyway.
I think it's still worth doing for the long run, though.
We could also consider doing it without a deprecation process, just a backwards-incompatible note in the release notes. Since the consequences are the opposite of data loss, that might be ok. Thoughts?
Change History (23)
comment:19 Changed 6 weeks ago by carljm
- Description modified (diff)
- Summary changed from on_delete=models.SET_NULL should be the default for nullable FKs to on_delete should be a required parameter for ForeignKey
comment:20 Changed 6 weeks ago by fcurella
- Owner changed from nobody to fcurella
- Status changed from new to assigned