#12708 closed Cleanup/optimization (invalid)
Django raises DoesNotExist when consulting an empty ForeignKey field
Reported by: | Luc Saffre | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | ForeignKey DoesNotExist |
Cc: | simon@…, k@… | Triage Stage: | Unreviewed |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
""" Django raises DoesNotExist when consulting an empty ForeignKey field We create an Order without a Journal:: >>> o = Order() >>> print o.journal None >>> o.save() This works only because `Order.journal` has `null=True`. In fact I want the `save()` method to complain if `journal` is empty, so I remove `null=True` from the field definition. >>> o = Order2() >>> print o.journal None The above line raises a DoesNotExist exception. Django should raise an exception only when I try to save the instance, not already when I want to see the value of `journal`! """ from django.db import models class Journal(models.Model): pass class Order(models.Model): journal = models.ForeignKey(Journal,null=True) class Order2(models.Model): journal = models.ForeignKey(Journal)
Change History (9)
comment:1 by , 15 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:2 by , 10 years ago
Easy pickings: | unset |
---|---|
Severity: | → Normal |
Type: | → Uncategorized |
UI/UX: | unset |
It shouldn't be considered invalid, even worse to be considered as a "consistent behavior". Reason: this only happens with related fields, and not with normal fields.
If I have:
class B(Model):
name = CharField(max_length=30) #example
class A(Model):
#neither of these fields allow null
count = PositiveSmallIntegerField()
ref_a = ForeignKey(B)
And issue:
a = A()
...
a.count #will return
a.ref_a #will raise exception
That does not let me traverse model properties without asking for forgiveness everytime. Why throwing DoesNotExist error and not throwing a ValueError or AttributeError for count property? Either trigger an error (ValueError, AttributeError, whatever) in non-related properties OR (xor) stop throwing errors when a FK has a null value (and does not accept it).
comment:3 by , 10 years ago
Thanks, Luis.
Luke, Django would be better if the value of a non-nullable foreign key object was allowed be None. The nullability of a field should become significant only when we try to save it. Forcing us to use journal_id here (not journal) is odd. This behaviour might have been intended at some time, and I agree that changing it might cause some backwards incompatibility, but that does not make and odd behaviour *good*.
comment:4 by , 10 years ago
What BW incompatibilities could arise? (Beware: this question is really important and not to take as so lightweight).
comment:5 by , 10 years ago
Incompatibilities where you check if an order has a journal by trying to access it and checking for an exception. Unless people were proactive (think paranoid) and also checked for None, even though it was never allowed to be None, such a change would plainly break any and all such checks.
comment:6 by , 10 years ago
Thanks for the example of a possible incompatibility. Yes of course, such code will break. The suggested change is not backward-compatible.
No it was not "proactive (think paranoid) to check for None" in a non-nullable field of an unsaved instance. There are even people who say that the opposite is true: expecting an exception for an expectable condition is considered bad style. The essence of this ticket is to (1) admit that the current behaviour is wrong (e.g. by agreeing to the statement "Django should raise an exception only when I try to save the instance, not already when I want to see the value of journal
!"). Luis understood this and illustrated it beautifully. And only then we can start to (2) collect ideas about how to change it. We will never get ideas about (2) as long as we don't admit (1).
comment:7 by , 10 years ago
Cc: | added |
---|---|
Type: | Uncategorized → Cleanup/optimization |
@lukeplant, perhaps this is something that should be cleaned up and flagged as backward incompatible in the next major release? We ought to be able to check the value of a field without raising an artificial error, or resorting to workarounds.
comment:8 by , 8 years ago
Cc: | added |
---|
Since the resolution to #26179 allows setting a non-nullable ForeignKey
attribute to None
, the current behavior seems even more inconsistent. I recommend re-opening this ticket.
comment:9 by , 8 years ago
To reopen the ticket, we'd need to a proposal for how to proceed regarding the backwards-incompatibilities. The place for that discussion is the DevelopersMailingList. Feel free to start a thread and link to it from here.
I think the behaviour is consistent. If the
journal_id
attribute was set to an id which did not exist in the database, then you would get the same DoesNotExist behaviour. It is also symmetric with the behaviour of trying to seto.journal
, which results in an exception.To put simply,
None
is never the value of non-nullable foreign key object - you cannot set or getNone
as the value. If you want the foreign key value, rather than the object, usejournal_id
, notjournal
.I've persuaded myself that this behaviour is intended and good, and changing it could be a significant backwards incompatibility anyway, so I'm closing INVALID.
Cheers.