#12801 : Allow empty non-nullable ForeignKey fields until save()

45 views
Skip to first unread message

Luc Saffre

unread,
Feb 6, 2010, 11:32:35 AM2/6/10
to django-d...@googlegroups.com
Hello,

I am trying to understand why Luke closed my ticket #12801
(http://code.djangoproject.com/ticket/12801).

Luke, don't get me wrong. Thank you for having taken the time to decide
upon this and my other ticket #12708. I agreed with you for marking
#12708 as invalid, because I didn't understand the real problem when I
wrote it, so the formulations in that ticket are rather confusing.

But #12801 now shows so clearly an odd behaviour which should at least
be documented if it cannot be fixed. Marking #12801 as a duplicate of an
invalid ticket means to me that I should not even *try* to write a
patch. Is that really what you want to say?

I'd agree if you say that this is not an urgent problem. But shouldn't
we mark it then as "Someday/Maybe"?

Luc

Luke Plant

unread,
Feb 6, 2010, 8:06:15 PM2/6/10
to django-d...@googlegroups.com
On Saturday 06 February 2010 16:32:35 Luc Saffre wrote:
> Hello,
>
> I am trying to understand why Luke closed my ticket #12801
> (http://code.djangoproject.com/ticket/12801).
>
> Luke, don't get me wrong. Thank you for having taken the time to
> decide upon this and my other ticket #12708. I agreed with you for
> marking #12708 as invalid, because I didn't understand the real
> problem when I wrote it, so the formulations in that ticket are
> rather confusing.
>
> But #12801 now shows so clearly an odd behaviour which should at
> least be documented if it cannot be fixed. Marking #12801 as a
> duplicate of an invalid ticket means to me that I should not even
> *try* to write a patch. Is that really what you want to say?

Yes, that is what I meant to say. I'll defend my opinion here, and
let others weigh in.

The issue is what should happen with the following model:

>>> class Foo(Model)
>>> a_date = DateTimeField()
>>> bar = ForeignKey(Bar)
>>>
>>> f = Foo()
>>> f.bar

The current behaviour is that a 'DoesNotExist' exception is thrown,
because f.bar_id = None, and the field is not nullable. This is
different from other uninitialised fields, which return None e.g.

>>> assert f.a_date is None

Luc wants the behaviour to be the same i.e. no DoesNotExist exception
for an uninitialised ForeignKey field. My response (in a more
expanded form here than on the ticket):

1. ForeignKey fields are different from simple values, in that they
cause database lookups (the only logical exception being nullable
foreign keys with a PK of None), so it's reasonable for them to behave
differently.

2. When that lookup occurs, whenever the PK value is not in the
database, you get an exception. So if the PK value was 123456 and
that did not exist, you would get an exception. So this behaviour is
consistent with other values of Foo.bar_id, and changing it would
break that consistency.

3. You currently get an exception if you attempt to set f.bar = None.
So, 'None' is never the value of a non-nullable foreign key field.
The proposed change would break that symmetry - you would not expect
the 2nd line of the following code to throw an exception if the first
did not:

>>> assert f.bar is None
>>> f.bar = None

If we changed Django to accept the first line, surely we also need to
change it to accept the second (which would be a bad idea, I think
we'd agree).

4. There is an easy work-around if you are allergic to exceptions,
which is to check the PK value (f.bar_id in the above case).

Thanks,

Luke

--
"Yes, wearily I sit here, pain and misery my only companions. And
vast intelligence of course. And infinite sorrow. And..." (Marvin
the paranoid android)

Luke Plant || http://lukeplant.me.uk/

Luc Saffre

unread,
Feb 6, 2010, 11:07:22 PM2/6/10
to django-d...@googlegroups.com
On 7.02.2010 3:06, Luke Plant wrote:
> 1. ForeignKey fields are different from simple values, in that they
> cause database lookups (the only logical exception being nullable
> foreign keys with a PK of None), so it's reasonable for them to behave
> differently.

Luke, I disagree with your explanations. Django behaves oddly.

Model.save() is the place where empty non-nullable fields cause an
exception. There is no reason for ForeignKey to behave differently.
ForeignKey fields are different in that they cause database lookups when
they are not None, and that they can cause exceptions in some special
situations, for example
- asking them to save before the foreign instance has been saved
- database integrity errors (invalid non-None pk)

That said, it is clear that I don't know Django well enough to decide
whether it is feasible/necessary to fix this odd behaviour.

Luc

Luke Plant

unread,
Feb 7, 2010, 5:38:13 PM2/7/10
to django-d...@googlegroups.com
On Sunday 07 February 2010 04:07:22 Luc Saffre wrote:

> Luke, I disagree with your explanations. Django behaves oddly.
>
> Model.save() is the place where empty non-nullable fields cause an
> exception.
> There is no reason for ForeignKey to behave differently.
> ForeignKey fields are different in that they cause database lookups
> when they are not None, and that they can cause exceptions in some
> special situations, for example
> - asking them to save before the foreign instance has been saved
> - database integrity errors (invalid non-None pk)
>
> That said, it is clear that I don't know Django well enough to
> decide whether it is feasible/necessary to fix this odd behaviour.

It is easy to fix - a few lines in
ReverseSingleRelatedObjectDescriptor - because the behaviour there is
not accidental, but quite deliberate. The behaviour is only odd from
one angle, and it is not very strange, because ForeignKey fields *are*
a fundamentally different type of field. The corresponding 'normal'
field is the '_id' field - this is a dumb value.

I think you are missing the fact that this behaviour occurs *whenever*
the FK val is an invalid value, not just with creating new instances.
You are therefore asking either for special treatment of instances
that are not saved to the database, or for it to be broken in general.

Consider this code:

>>> o = SomeModel.objects.get(id=1)
>>> o.fk_field_id = None
>>> print o.fk_field

Are you are proposing that the above should print 'None' even when
fk_field is non-nullable, rather than a DoesNotExist? 'o.fk_field' is
a shortcut for a DB lookup of 'o.fk_field_id', and clearly the result
of that lookup is *not* None/NULL. The query returns no results, and,
just like every other case when a query returns no results and we
expect at least one, you get a DoesNotExist exception. (In this case
we skip doing the actual database query as an optimisation, but the
same logic applies).

Luc Saffre

unread,
Feb 8, 2010, 5:31:39 PM2/8/10
to django-d...@googlegroups.com
Luke, thank you for your explanations. If I answer once more, then I do
it in the hope that our discussion may help to improve Django.

On 8.02.2010 0:38, Luke Plant wrote:
> It is easy to fix - a few lines in
> ReverseSingleRelatedObjectDescriptor - because the behaviour there is
> not accidental, but quite deliberate.

I admit that I am surprised to hear this.

> The behaviour is only odd from
> one angle, and it is not very strange, because ForeignKey fields *are*
> a fundamentally different type of field. The corresponding 'normal'
> field is the '_id' field - this is a dumb value.

Sorry, but I still don't understand the angle from which this behaviour
is *not* odd. And the corresponding '_id' field is added 'behind the
scenes', so using it should be avoided as much as possible.

> I think you are missing the fact that this behaviour occurs *whenever*
> the FK val is an invalid value, not just with creating new instances.

No, I considered this. I wrote already the two possible causes of
exceptions that I can imagine.

> You are therefore asking either for special treatment of instances
> that are not saved to the database, or for it to be broken in general.

That's no special treatment. I'm asking to have the same behaviour for
nullable and non-nullable ForeignKey fields (until save()).

Having 'None' in a non-nullable field is a normal case of an instance
containing invalid data. Imagine an application that runs a complex
series of handlers on an instance that decide how to set some fields
depending of the (valid or invalid) values contained in some other
fields. You cannot ask user code to not even look at invalid data. I'm
not allergic to exceptions, but raising an exception when I ask for the
content of a field is not appropriate behaviour.

> Consider this code:
>
>>>> o = SomeModel.objects.get(id=1)
>>>> o.fk_field_id = None
>>>> print o.fk_field
>
> Are you are proposing that the above should print 'None' even when
> fk_field is non-nullable, rather than a DoesNotExist?

Yes. Though I'd say "o.fk_field = None" instead of "o.fk_field_id =
None", because the '_id' field should remain behind the scenes. Anyway
both should have the same effect.

> 'o.fk_field' is
> a shortcut for a DB lookup of 'o.fk_field_id', and clearly the result
> of that lookup is *not* None/NULL. The query returns no results, and,
> just like every other case when a query returns no results and we
> expect at least one, you get a DoesNotExist exception. (In this case
> we skip doing the actual database query as an optimisation, but the
> same logic applies).

'o.fk_field' is an attribute of a Model instance, and Django should
never refuse to show what it contains, even if that value is illegal.

Okay, it's obvious that Luke's and Luc's understandings of that problem
are quite different. We both explained our points of views. The next
step would be to hear other people's opinions. I personally won't insist
further, especially since Luke knows Django better than me. May I
suggest again to mark this ticket to something different than "duplicate
of an invalid ticket"?

Luc

Karen Tracey

unread,
Feb 8, 2010, 6:09:34 PM2/8/10
to django-d...@googlegroups.com
On Mon, Feb 8, 2010 at 5:31 PM, Luc Saffre <luc.s...@gmx.net> wrote:
[snip] 
Okay, it's obvious that Luke's and Luc's understandings of that problem
are quite different. We both explained our points of views. The next
step would be to hear other people's opinions.

I agree with Luke. I'm also puzzled by your resistance to utilizing the field with _id appended to access the "empty" value in cases where you don't want to deal with the possibility of a DoesNotExist exception. That field, the one with _id on the end, is the direct map to a column in the database. The other is a convenience for accessing the entire model instance that is the representation of a row from another table. If that row doesn't exist, it makes sense to me that attempting to access it, whenever, raises DoesNotExist.
 
I personally won't insist
further, especially since Luke knows Django better than me. May I
suggest again to mark this ticket to something different than "duplicate
of an invalid ticket"?

What difference does it make? 

Karen

Luc Saffre

unread,
Feb 9, 2010, 3:02:12 AM2/9/10
to django-d...@googlegroups.com
On 9.02.2010 1:09, Karen Tracey wrote:
> On Mon, Feb 8, 2010 at 5:31 PM, Luc Saffre <luc.s...@gmx.net
>
> I personally won't insist
> further, especially since Luke knows Django better than me. May I
> suggest again to mark this ticket to something different than "duplicate
> of an invalid ticket"?
>
> What difference does it make?

"Closed" as "Invalid", as I understand it, means that this ticket is not
worth to get more consideration and that further comments are not welcome.

"Open" with a triage stage of "Someday/Maybe" would help people who
stumble on this behaviour to find our discussion. Especially if we add a
link to this discussion thread.

Luc

Karen Tracey

unread,
Feb 9, 2010, 10:03:40 AM2/9/10
to django-d...@googlegroups.com
On Tue, Feb 9, 2010 at 3:02 AM, Luc Saffre <luc.s...@gmx.net> wrote:
"Closed" as "Invalid", as I understand it, means that this ticket is not
worth to get more consideration and that further comments are not welcome.

"Open" with a triage stage of "Someday/Maybe" would help people who
stumble on this behaviour to find our discussion. Especially if we add a
link to this discussion thread.

Open with a triage state of Someday/Maybe would imply that the current behavior has been acknowledged as wrong and that a change in the behavior might be expected. It would also imply that if someone provided a patch to change the behavior, it would have a chance of getting checked in. With two core devs stating they see the current behavior as logical and consistent and none stepping up to say the opposite, a change in behavior here isn't going to happen. Thus an open state for this ticket would give people the wrong impression of the state of the issue. 

Closed tickets are as easily found as open ones -- anyone doing research in an attempt to understand current behavior should not be limiting their searches to only open tickets.

The right state for this ticket is closed, which means it's been reviewed and determined that the current behavior is correct and nothing needs to be done to fix it. I would not term further comemnts "unwelcome", but they are not particularly useful unless someone has some new light to shed on the issue.

Karen

hcarvalhoalves

unread,
Feb 9, 2010, 11:28:12 PM2/9/10
to Django developers
Maybe I can help Luc? I had similar questions once, because I started
learning Django while deploying with a legacy database.

On 8 fev, 20:31, Luc Saffre <luc.saf...@gmx.net> wrote:
> You cannot ask user code to not even look at invalid data. I'm
> not allergic to exceptions, but raising an exception when I ask for the
> content of a field is not appropriate behaviour.
>

> Luc

Raising the exception *is* appropriate behaviour, because when you
access `a_model.b_related`, and `b_related` is a ForeignKey, you're
not really accessing a value, but doing a lookup. This is consistent
with both the ER and ORM model, where FK's are pointers, not values.
And a FK pointing to a non-existant row *is* an exception. It means
broken data.

If you want the content of the *field*, what you really should check
is `a_model.b_related_id is None`, which is checking if a pointer was
set. Still, it doesn't guarantee that this pointer leads to anywhere
valid. That's why dealing with the exception is a common idiom, "fail
early, fail loudly", and particularly useful in this case to maintain
DB consistency.

I hope it helps in understanding the rationale behind the behaviour
and why Luke marked as invalid.

Luc Saffre

unread,
Feb 10, 2010, 8:30:55 AM2/10/10
to django-d...@googlegroups.com
Thanks, Karen, for your explanations which are very clear. I accept the
community's decision and won't bother you any longer. And maybe one day
I will even understand why this behaviour is not odd.

Luc

> --
> You received this message because you are subscribed to the Google
> Groups "Django developers" group.
> To post to this group, send email to django-d...@googlegroups.com.
> To unsubscribe from this group, send email to
> django-develop...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.

Luc Saffre

unread,
Feb 10, 2010, 9:42:49 AM2/10/10
to django-d...@googlegroups.com
Thank you, Henrique, for dropping in.

As I said earlier, I don't want to bother the community with my
stubbornness or whatever it may be, so feel free to STOP READING HERE if
there is danger that this topic wastes your time.

But to be honest, I must unfortunately say that I disagree also with
your explanations and that they (for me) confirmed my opinion.

The expression `a_model.b_related` (where `a_model` is an instance of a
Model subclass that defines a field `b_related`) must always yield a
value in the Pythonic meaning (that is, it may yield None which is also
a value in Python).

This same expression should raise an exception only if the field name is
wrongly spelled, or if an error occured when constructing the related
instance to be yielded (e.g. a failed DB lookup in the case of a FK
field other than None).

A FK containing None (which is already now possible for nullable FK
fields) will never cause a DB lookup.

Django guarantees that the value of a FK field will always be either
None or an instance of the related Model.

Django never guaratees that every instance of a Model exists as a row in
the database.

Django's current behaviour is not correct because it forces me to access
non-nullable FK fields differently than nullable ones. "In Python,
throwing exceptions for expected outcomes is considered very bad form"
[1]. Django should raise an exception only if I try to save an instance
with invalid data (for example None in a non-nullable FK field), but not
when I try to access any data, may it be valid or not

[1]
http://groups.google.com/group/comp.lang.python/browse_thread/thread/7d6191ecba652daf

Luc

Tamas Szabo

unread,
Feb 10, 2010, 12:30:30 PM2/10/10
to django-d...@googlegroups.com
Hi,

Well, I'm not a django core developer, so my post probably won't weight that much, but I can't leave Luc with the feeling that that aren't other people that have the same opinion as him.
Luc, I'm with you on this one :)

First of all the problem is in an area of handling the mismatches between the OO and the relational models, an area that should always cause discussions. So, Luc I guess you have to accept that this is a gray area and this is how Django's ORM works. Life goes on ...

On the other hand, I do not understand how you can't see the point that Luc is making and how can you say that this makes sense from both the ER and ORM point of view. When you are working with your OO model the relationship between your models aren't represented by FK pointers. They should map to attributes of models or collections of models.
FK based relations from the DB are represented by attributes and collections in the OO world and the ORM should take care of this mapping and making it transparent.

In an ideal world you shouldn't use or even know about those _id fields when working with your OO models.
Your working with your OO domain model that consists of Python classes and they should work like any other normal Python classes until you use save or load them (then the ORM kicks in and does the mapping).
But until you do that, they should behave like Python classes so throwing DoesNotExist Exception when you access an attribute of a class feels a little bit strange.

But of course our world is more pragmatic and I can live with the current implementation (and I hope Luc can do the same), but I can certainly understand why it feels strange to him and why he resists using _id fields while working with his models.

All The Best,

Tamas


When you are working with your models

Luc Saffre

unread,
Feb 11, 2010, 1:26:16 AM2/11/10
to django-d...@googlegroups.com
Thank you, Tamas. Yes, that's what I mean. Without you I might have come
to believe that something is wrong with my brain...

And yes, I can live with it. It's just a pity that others will stumble
on it just because it isn't documented.

Luc

hcarvalhoalves

unread,
Feb 11, 2010, 11:24:40 PM2/11/10
to Django developers
On 10 fev, 12:42, Luc Saffre <luc.saf...@gmx.net> wrote:
> Thank you, Henrique, for dropping in.
>
> Django's current behaviour is not correct because it forces me to access
> non-nullable FK fields differently than nullable ones. "In Python,
> throwing exceptions for expected outcomes is considered very bad form"
> [1]. Django should raise an exception only if I try to save an instance
> with invalid data (for example None in a non-nullable FK field), but not
> when I try to access any data, may it be valid or not
>
> Luc

The exception doesn't come from the fact you're accessing an
attribute. It comes from the related lookup, Django is just doing the
right thing and letting the DoesNotExist exception flow up. It's
better if you see things this way.

Also, the DoesNotExist exception is far from undocumented. Thru the
docs and tutorials you see the idiom on how to handle it quite a few
times. [1]

It also makes perfect sense to handle nullable FK's different than non-
nullable ones. One have a constraint imposed, the other not. Returning
None to a non-nullable FK, *this* would be what I call unexpected
behaviour.

Remember also that this is not a pure OO world. If we were seeing
things as just relations between classes, I would agree with you. But
we're dealing with a cumbersome thing that is the database, and
there's always a mismatch between the ER and OO models. [2]

Anyway, there's certainly nothing wrong or strange on throwing
exceptions on accessing attributes in Python (or in any dynamic
language, I would say), as any attribute on Python can be, in fact, a
method (@property). I'm surprised how opposing you feel on this, my
feeling is that you come from a different background, working with
languages that enforce exception handling in a special way (Java, C#).
But it's really getting into the field of personal taste now.

[1] http://docs.djangoproject.com/en/1.1/ref/models/querysets/#id5
[2] http://www.sbql.pl/Topics/ImpedanceMismatch.html

Luc Saffre

unread,
Feb 12, 2010, 2:47:55 AM2/12/10
to django-d...@googlegroups.com
Sorry, but the longer we discuss, the more it becomes clear: Django's
behaviour is certainly wrong.

I'm puzzled how no Django core developer seems to understand it. I hope
that this is just because they are not following this thread because
there is admittedly more urgent work to do right now.

With the danger of repeating myself I explain once more:

Example models:

class Owner(models.Model):
pass

class Animal(models.Model): # nullable owner
owner = models.ForeignKey(Owner,blank=True,null=True)

class Thing(models.Model): # non-nullable owner
owner = models.ForeignKey(Owner)

There should never be a related lookup when the fk_id is None. For
nullable FK Django behaves correctly:

>>> a = Animal()
>>> print a.owner
None

The same doesn't work for non-nullable FK:

>>> t = Thing()
>>> print t.owner
Traceback (most recent call last):
...
DoesNotExist

This is whay I say that Django treats non-nullable FK in a special way.
It is as if Django thinks "Since Thing.owner may not be None, I can do
the lookup without even testing for a None value".

The correct exception is risen when you try to save it:

>>> t.save()
Traceback (most recent call last):
...
IntegrityError: 20100212_thing.owner_id may not be NULL

How can you not understand that the DoesNotExist exception above is
risen too early? It is a bug!

Luc

Gary Reynolds

unread,
Feb 12, 2010, 5:08:01 AM2/12/10
to django-d...@googlegroups.com
The correct exception is risen when you try to save it:

 >>> t.save()
 Traceback (most recent call last):
 ...
 IntegrityError: 20100212_thing.owner_id may not be NULL

How can you not understand that the DoesNotExist exception above is
risen too early? It is a bug!

Are you saying that the correct behaviour is to throw an IntegrityError as opposed to a DoesNotExist on accessing the field?

If so, why would accessing a value on an unsaved instance (which by definition isn't in the database yet) be an IntegrityError rather than a DoesNotExist as a result of a lookup?

It's clearly a design decision. You are free to disagree with that decision, but it's not a bug - it's behaving as designed (and documented).

Gary

Russell Keith-Magee

unread,
Feb 12, 2010, 6:39:00 AM2/12/10
to django-d...@googlegroups.com
On Fri, Feb 12, 2010 at 3:47 PM, Luc Saffre <luc.s...@gmx.net> wrote:
> Sorry, but the longer we discuss, the more it becomes clear: Django's
> behaviour is certainly wrong.

No, it really isn't *certainly* wrong. It's *arguably* wrong, and I
join my colleagues Luke and Karen in arguing that it isn't wrong at
all.

> I'm puzzled how no Django core developer seems to understand it. I hope
> that this is just because they are not following this thread because
> there is admittedly more urgent work to do right now.
>
> With the danger of repeating myself I explain once more:
>
> Example models:
>
>  class Owner(models.Model):
>      pass
>
>  class Animal(models.Model):  # nullable owner
>      owner = models.ForeignKey(Owner,blank=True,null=True)
>
>  class Thing(models.Model):   # non-nullable owner
>      owner = models.ForeignKey(Owner)
>
> There should never be a related lookup when the fk_id is None. For
> nullable FK Django behaves correctly:
>
>  >>> a = Animal()
>  >>> print a.owner
>  None

Yes. The owner hasn't been specified. Therefore, the field falls back
to it's default value -- the owner is None.

> The same doesn't work for non-nullable FK:
>
>  >>> t = Thing()
>  >>> print t.owner
>  Traceback (most recent call last):
>  ...
>  DoesNotExist

Yes. The owner hasn't been specified. The owner *must* be specified,
so any attempt to access it is an error - the owner Does Not Exist.

> This is whay I say that Django treats non-nullable FK in a special way.

Yes. Personally, I see the nullable case as the special case, but it
doesn't really matter which way you look at it -- the two behave
differently because they *are* different.

> It is as if Django thinks "Since Thing.owner may not be None, I can do
> the lookup without even testing for a None value".

Exactly. There's no point looking for a None value, because the field
*can't have a value of None*.

> The correct exception is risen when you try to save it:
>
>  >>> t.save()
>  Traceback (most recent call last):
>  ...
>  IntegrityError: 20100212_thing.owner_id may not be NULL
>
> How can you not understand that the DoesNotExist exception above is
> risen too early? It is a bug!

How can you *not* understand that this is a difference of opinion?
Three Django core developers (Luke, Karen and myself) have
independently told you that they disagree that this is "certainly
wrong".

In one last attempt to explain why Django works the way it does: it
may help to think of the problem in these terms. Thing.owner *isn't* a
field on Thing. It may be defined there, but what it is defining is a
relationship with the Owner table. Thing.owner is a descriptor that
substitutes for a query on the "Owner" table.

When you ask for Thing.owner, you're really saying "Give me the Owner
with a relationship to Thing X". If Thing X hasn't defined a
relationship with an owner, then the Owner.DoesNotExist.

In the case of the Animal, a legitimate answer to the question "Give
me the owner with a relationship to Animal X' is "no owner" - hence,
None is a legal return value.

Hopefully that clarifies why Django works the way it does. However,
even if, hypothetically, we were to accept that Django's current
behavior is in error, and your interpretation is the only correct
interpretation, there is an enormous issue of practicality. This
behavior was introduced as part of the 'magic-removal' branch, which
is 4 years old. The current behavior closely mirrors the analogous
behavior that preceded it. The current behavior has been a formal,
documented part of the 0.96, 0.96, 1.0, and 1.1 releases. There are
quite literally *thousands* of projects in production that rely on
Django's behavior being predictable and consistent between versions -
even if that behavior isn't "correct".

Changing this behavior would be a *huge* undertaking. If we were
talking about some esoteric corner of the aggregation API, we might be
able to entertain making a backwards-incompatible change -- but you're
talking about changing a fundamental aspect of the way foreign keys
work, and have always worked, and have been documented to work.

Django has been used by many people, over a long period of time. The
criticism of the API you are raising is *not* a regular feature of
complaints on Django-users, so there is very little reason to believe
that it is causing significant practical difficulties. Although the
change would be a minor code modification, it would be *very*
complicated to change it in practice, given Django's API backwards
compatibility guarantee. And as a result, we're *not* going to change
it.

Yours,
Russ Magee %-)

Tom Evans

unread,
Feb 12, 2010, 7:01:33 AM2/12/10
to django-d...@googlegroups.com
I think the easiest way of understanding the behaviour is that
specifying a field like this:

owner = models.ForeignKey(Owner)

specifies a contract. The contract says that when you access this
attribute on an instance, it will only return an instance of an Owner
object or raise an DoesNotExist exception.

Specifying a field like this:

owner = models.ForeignKey(Owner,blank=True,null=True)

specifies a very different contract. This contract says that accessing
this attribute will return either an instance of an Owner object, or
None if no owner has been specified.

Both contracts are fully described in the documentation and fulfilled
by the ORM, so clearly this is no bug. Both flavours are available for
you to use, you simply have to decide what sort of contract you want
that attribute to have - if you expect instance.owner to return None,
then clearly you should be using a nullable foreign key.

Cheers

Tom

Luc Saffre

unread,
Feb 12, 2010, 7:12:45 AM2/12/10
to django-d...@googlegroups.com
On 12.02.2010 12:08, Gary Reynolds wrote:
>
> Are you saying that the correct behaviour is to throw an IntegrityError
> as opposed to a DoesNotExist on accessing the field?

No. It should raise no exception at all. The expression should yield
None. It should not be forbidden for an instance to contain invalid data
*until* you try to save it.

> If so, why would accessing a value on an unsaved instance (which by
> definition isn't in the database yet) be an IntegrityError rather than a
> DoesNotExist as a result of a lookup?
>
> It's clearly a design decision. You are free to disagree with that
> decision, but it's not a bug - it's behaving as designed (and documented).

If this is a topic that has been discussed earlier, then it would be
good to find pointers to this discussion. I didn't find any.

If at least one core developer would say "maybe Luc is right", then Luke
and Karen would maybe agree to reopen the ticket and mark it "needs
design decision".

And please: I continue to post here because I hope that my explanations
can help to understand the problem and to make Django better. I don't
want to offend anybody, I am not trying to "win" this "battle", I have
no personal interest in having this ticket reopened. If some day I
understand that *I* was wrong, then I will apologize for the noise I made.

Luc

Luc Saffre

unread,
Feb 12, 2010, 7:38:59 AM2/12/10
to django-d...@googlegroups.com
On 12.02.2010 13:39, Russell Keith-Magee wrote:
>
> Hopefully that clarifies why Django works the way it does.

Yes, it does. Thank you, Russel.

> However,
> even if, hypothetically, we were to accept that Django's current

> behavior is in error, (...) there is an enormous issue of practicality.
> (...)

> Although the
> change would be a minor code modification, it would be *very*
> complicated to change it in practice, given Django's API backwards
> compatibility guarantee. And as a result, we're *not* going to change
> it.

Yes, this is a good reason to not change Django's behaviour,
independently of whether it is "wrong" or not.

Luc

Luc Saffre

unread,
Feb 12, 2010, 8:07:14 AM2/12/10
to django-d...@googlegroups.com

Wow! This formulation is really clear. Thank you, Tom.

My summary: Django's behaviour is un-pythonic and not the most elegant
one, but it is possible to get used to it, even to rely on it, and it
certainly won't change.

I didn't find such a good explanation in the docs, though. IMHO it
should be here:

http://docs.djangoproject.com/en/dev/topics/db/queries/#forward

I suggest to add a paragraph about the surprising DoesNotExist there.

Luc

Tamas Szabo

unread,
Feb 12, 2010, 10:42:07 AM2/12/10
to django-d...@googlegroups.com
Nice explanation, but I can't see this documented it like this anywhere. If it isn't documented people have to interpret it based on what they would expect and apparently our expectations are different.

Does Django enforce any other contracts defined on the models? What would happen if I accessed a null=false, IntegerField for example?
The question to ask here is WHEN are any contracts defined on the models enforced?

I wouldn't expect anything to be enforced on a Model object that hasn't been persisted yet. So if I created a Model obejct but I haven't saved it yet and it hasn't been loaded from the DB I don't expect Django handling the ForeignKey attribute specially just as Django wouldn't prevent me setting a null=false IntegerField to a string or to null or throwing an exception when I access an attribute that hasn't been initialized.

I'm saying that FK attributes should work consistently with other fields.
I guess you are saying that FK attributes are different and they should work differently and this is where we disagree.

In the

owner = models.ForeignKey(Owner)

example, yes I would expect getting an owner every time I access the owner attribute, but ONLY if I loaded the class from the DB or I already saved it.

If the class isn't persisted it's just a plain Python class, there aren't any guarantees from Django and I can manipulate it however I like it. I don't see why a FK attribute would have to be special in this stage, because I don't even see a reason why you would do a select to fetch the Owner if the class isn't persisted.

I completely understand that you don't want to change the existing behavior, I wouldn't change it myself. The difference is very small from a practical point of view so it isn't worth changing the behavior of Django.

The docs might be improved, although you could just point people with similar views to this thread from now on :)

All The Best,

Tamas



Reply all
Reply to author
Forward
0 new messages