Django raises DoesNotExist when consulting an empty ForeignKey field

[Luc Saffre]

"""

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)

[Luke Plant]

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 set o.journal, which results in an exception.

To put simply, None is never the value of non-nullable foreign key object - you cannot set or get None as the value. If you want the foreign key value, rather than the object, use journal_id, not journal.

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.

[Luis]

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).

[Luc Saffre]

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*.

[Luis]

What BW incompatibilities could arise? (Beware: this question is really important and not to take as so lightweight).

[orshansk@…]

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.

[Luc Saffre]

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).

[Simon Litchfield]

@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.

[Kevin Christopher Henry]

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.

[Tim Graham]

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.