Support a related_default on OneToOne fields

[rvdrijst]

EDIT: scroll to comment 13 to see what this ticket is about.

Referencing a OneToOneField with null=True (i.e. it's optional) when there is no value set will return None, as expected.
However, referencing the reverse side of the relation does not follow this behavior and raises a DoesNotExist exception.

For example, in the following situation where Shops have optionally a Place (e.g. webshops need not have a physical location):

class Place(models.Model)
    address = models.CharField(max_length=80)
class Shop(models.Model)
    place = models.OneToOneField(Place, null=True)
    name = models.CharField(max_length=50)
    website = models.URLField()

This does work as expected:

>>> s1 = Shop.objects.create(name='Shop', website='shop.com')
>>> print s1.place
None

But this doesn't work as expected:

>>> p1 = Place.objects.create(address='123 somestr')
>>> p1.shop
... [exception stack trace] ...
DoesNotExist: Shop matching query does not exist.

I would expect this to be None when null is allowed on the OneToOneField.

Please correct my if I'm wrong.

I have attached a patch to fix this (checking if null is allowed and returning None or raising the exception appropriately), including tests.

Unfortunately this is slightly backwards incompatible, since someone may currently rely on the exception being thrown.

[rvdrijst]

Patch with fix and tests

[(none)]

Milestone post-1.0 deleted

[Jacob]

I'm not entirely sure what the correct behavior here ought to be: the fact that Shop.place may be null doesn't necessarily imply that Place.shop also may be null. That is, it may be correct for some situations that a Shop may not have a Place but that Place always has an associated Shop.

[Malcolm Tredinnick]

This has come up before (#3106). I've closed that as dupe, simply because the description here is more useful.

[Erik Allik]

I just ran into the same issue. I think this is very unintuitive... The way I see it is that a OneToOneField should behave just like a foreign key from both sides.

[Erik Allik]

Assuming related_name='foobar' is desired, the workaround would be to use related_name='_foobar' and then write a property in the related model that looks like this:

def foobar(self):
    try:
        return self._foobar
    except Foobar.DoesNotExist:
        return None

[Ole Laursen]

I've run into the case where every Shop has a Place, but some places don't have a shop. So I think we need to model it with a separate parameter, e.g. related_null=True. That would also be backwards-compatible. Aren't there four cases? So we need two bits to describe them:

one           to  one
zero or one   to  one
one           to  zero or one
zero or one   to  zero or one

[Jonas von Poser]

Replying to olau:

So I think we need to model it with a separate parameter, e.g. related_null=True. That would also be backwards-compatible.

This seems an elegant solution. I would really like to have this functionality, as it makes extending the User model much easier. For instance, we have a Member model with a OneToOneField to User. Since I can't add a property to User (except by monkeypatching it), I have to wrap every access to request.user.member with a try/except block.

[George Sakkis]

I ran into the same problem although in my case a hypothetical related_null=True would not help; a new default object would have be created. So instead of related_null I wrote a OneToOneField that uses related_default, a callable lambda instance: value. I posted the snippet at ​http://gist.github.com/601977.

To me this seems more elegant, not only because it is a generalization of the related_null (just set related_default=lambda instance: None) but because null (and by association related_null) is a DB-level parameter, it determines whether a DB column is created as NULL or NOT NULL. In the case of a OneToOneField, the related model does not have a column linking to the model where the OneToOneField was defined (no shop_id column in the place table), so it doesn't make sense to say it's null.

[Luke Plant]

It looks to me that the consensus is that raising the DoesNotExist exception does make sense (at least in some cases, and we have to take the general case). The argument gsakkis makes seems sensible, so his/her solution is preferable. It would also make it very easy to solve the original problem i.e. just provide related_default=lambda: None.

But this is definitely a feature, not a bug, so changing accordingly.

I'm removing the 'has patch' flag since we don't have a patch that implements the chosen solution.

[anonymous]

I came up with this work around as well, to check that when a user is saved it has an associated userprofile

@receiver(post_save, sender=User)
def user_saved(sender, instance, created, **kwargs):
    if not hasattr(instance, 'userprofile'):
        user_profile = UserProfile()
        user_profile.user = instance
        user_profile.save()

[Ole Laursen]

Rereading this, can I suggest that related_default does not have to be a function? It would make things a bit clearer if one could say related_default=None instead =lambda x: None.

I realize this means you have to apply slight cleverness when defining the parameter since you can't use None, e.g.

class RelatedDefaultRaiseException: pass

def __init__(..., related_default=RelatedDefaultRaiseException):
    if related_default == RelatedDefaultRaiseException:
        # turn on exception raising behaviour

[Sebastian Goll]

I'd also like to see the functionality of the reverse side not throwing an exception. This would also make both sides of the one-to-one relation behave more symmetric. I agree that an additional parameter is the way to go, since all four types of blank/non-blank make sense (comment:7).

Can gsakkis' snippet (comment:10) be adapted to the current trunk version? I agree that passing None or constant values to related_default should be possible in addition to passing in functions/lambdas, as it makes the model definition much cleaner in the case of simple values.

[Sebastian Goll]

Alright, here is another inconsistent behavior that falls in the lines of raising or not raising an exception.

The behavior of the reverse lookup for one-to-one-fields changes when select_related has been called on the reverse relation. In that case, accessing the reverse relation does not raise an exception but returns None in the case of a missing related object. This feels rather strange to me, it should either always raise an exception or never.

I am not sure if gsakkis' snippet addresses this issue.

Here is an example:

class A(models.Model):
    pass

class B(models.Model):
    a = models.OneToOneField(A)

########
a = A.objects.create()
A.objects.get(pk=a.pk).b  # raises B.DoesNotExist
A.objects.select_related('b').get(pk=a.pk).b  # returns None

[Łukasz Rekucki]

Replying to sebastian:

Alright, here is another inconsistent behavior that falls in the lines of raising or not raising an exception.

The behavior of the reverse lookup for one-to-one-fields changes when select_related has been called on the reverse relation. In that case, accessing the reverse relation does not raise an exception but returns None in the case of a missing related object. This feels rather strange to me, it should either always raise an exception or never.

This is a bug described in #13839. In context of this ticket, it should always raise an exception, unless related_null=True is given, in which case it should always return None.

[Aymeric Augustin]

I just spent some time on this bug while investigating related problems.

I agree that the proper solution is to add a related_default parameter to OneToOneField. Like default, this parameter could be a value or a callable.

Independently, there are some bugs and inconsistencies in the caching of the lack of a value (#13839 and #17439). Fixing this bug would provide a way to hide some of these problems, but not address their root cause.

[Aymeric Augustin]

This problem was discussed again in #18153. I still believe that Luke's comment 13 nails it.

I'm updating the title to better reflect the design decision.

[Anssi Kääriäinen]

The problem is that changing reverse o2o field handling will break existing code. The API might not be ideal, but that is what we got.

Adding a related_default argument doesn't seem like a nice solution to me. Specifically, if the related_default can be anything else than None it will lead to surprising situations. Also, having the API of obj.o2ofield change based on field setting could be somewhat annoying...

I would love to have a more generic approach to this, something like a "nget()" method which returns None if no objects found. We could hook this into the fields by:

# works like obj.o2ofield except no DoesNotExist...
obj.nget('o2ofield')
# Could also be used as
Code.objects.nget(type='imei', value='bar')

This is nicer to write than:

try:
   res = obj.o2ofield
except obj.DoesNotExist:
   res = None

To me this seems like an explicit and easy to implement approach. Does this approach seem workable to others? If not, IMO we should just wontfix this issue, as I don't see any nice way to change the API without breaking existing code.

[Ole Laursen]

As much as I would love to see a A.objects.none_get() or perhaps a more Pythonic A.objects.getdefault(), IMHO that is orthogonal to this issue. I don't think your obj.nget('o2ofield') is intuitive.

Regarding related_default: I just need None (that's why I proposed related_null), but you can conceivably do a lambda that makes sense, e.g.

class UserProfile(models.Model):
    user = models.OneToOneField(User, related_default=lambda u: UserProfile(user=u))
    ...

if not request.user.profile.name:
    request.user.profile.name = ...
    request.user.profile.save()

And it is arguably relatively in line with the existing API, except the lambda must receive the instance which I don't believe it does for the ordinary default (without the instance, you can't set the one-to-one key to the original object and then it's really confusing :).

I don't understand why you think it would be a problem to change the API based on a field setting. That's already happening with null and default today?

[Anssi Kääriäinen]

The example of related_default usage is exactly what I am afraid. That code contains a serious pitfall: the user.profile isn't saved, and it will not be associated to the user if you don't save it. And, you do get that default value even for saved user instances.

There is a big difference to how foreign key default argument works:

class A(models.Model):
    pass

def get_a():
    return A.objects.get_or_create(pk=1)[0]

class B(models.Model):
    a = models.ForeignKey(A, null=True, default=get_a)

b = B()
print b.a
b = B(a=None)
print b.a
b = B(a=None)
b.save()
b = B.objects.get()
print b.a

The output is:

A object
None
None

My understanding is that the "related_default" would return "A object" for each case. This is broken - what you get from b.a doesn't reflect the database state.

The related_none could work. Here I am mainly worried of not knowing what happens when I write:

p = request.user.profile

does it raise DoesNotExist if the profile doesn't exists, or will p just be None? However, I do not feel too strongly about this case.

The good thing about b.nget('a') is that when you see the code, you immediately know what the code does. You are explicitly asking for a value that can be None value, so you will be prepared to handle the None case. This is relatively simple to do in user code, so there is no strong reason to have this convenience method in Django core.

[Ole Laursen]

Well, if the conceptual model is adding objects lazily, it's not really a problem that you need to save the user profile to get your changes to stick - they wouldn't stick otherwise. My example would never have worked without the save if the intention was to change the name persistently.

But it's true that one would have to careful to make sure the user object was saved so it had an id. You have the same problem if you write the code manually, though.

I had some trouble understanding your example on the difference to the foreign key default parameter - it seems what you are thinking about is a zero-or-one to one mapping, not a zero-or-one to zero-to-one (that would be related_default=None). Yes, it's less powerful in the related end in that you can't avoid confounding null and default. But there's no way around that. When I said relatively in line with the existing API, I just meant that the conceptual idea of returning an in-memory object when nothing exists yet was the same.

Thanks for looking at this, by the way. :)

[Aymeric Augustin]

Replying to akaariai:

The example of related_default usage is exactly what I am afraid. That code contains a serious pitfall: the user.profile isn't saved, and it will not be associated to the user if you don't save it. And, you do get that default value even for saved user instances.

This was (probably) fixed by #10811.