Opened 18 years ago
Closed 16 years ago
#3016 closed defect (duplicate)
make reverse one-to-one field return None instead of DoesNotExist
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | |
Severity: | normal | Keywords: | 121-rewrite |
Cc: | mir@… | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
in django.db.models.fields.related.SingleRelatedObjectDescriptor, The
code for get:
def __get__(self, instance, instance_type=None): if instance is None: raise AttributeError, "%s must be accessed via instance" % self.related.opts.object_name params = {'%s__pk' % self.related.field.name: instance._get_pk_val()} rel_obj = self.related.model._default_manager.get(**params) return rel_obj
Here, if "instance._get_pk_val()" returns None, then the get(params)
will return *any* object.
(The primary key could be None for a new, unsaved object, for
instance.)
More sensible would either to be to return None or raise
self.related.model.DoesNotExist.
Change History (9)
comment:1 by , 18 years ago
Cc: | added |
---|
comment:2 by , 18 years ago
You're right! I was running on r3900. The latest one's ...get() returns DoesNotExist. And that behavior is documented, too! So... "oh well -- fixed again!" :)
Thanks for looking in to it; I have to remember how fast the project is moving.
comment:3 by , 18 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Fixed by downloading latest SVN according to ticket submitter.
comment:4 by , 18 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Actually, now that I think about it, I'm not sure that raising DoesNotExist is the best behaviour.
For instance, if model Foo has a ForeignKey bar, and a reverse one-to-one field baz,
foo = Foo()
Now:
if foo.bar: ...
will skip the if body if bar doesn't exist, but
if foo.baz: ...
will raise "DoesNotExist".
It seems to me that they should behave the same: dereference should return none for missing object, or both should throw DoesNotExist.
comment:6 by , 17 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
comment:7 by , 17 years ago
Summary: | related object descriptor returns arbitrary object when called from unsaved starting object → make reverse one-to-one field return None instead of DoesNotExist |
---|
comment:8 by , 17 years ago
Keywords: | 121-rewrite added |
---|---|
Triage Stage: | Design decision needed → Accepted |
We'll look at this again when we refactor one-to-one, but my gut is that returning None is the right behavior.
comment:9 by , 16 years ago
Resolution: | → duplicate |
---|---|
Status: | reopened → closed |
Closing as a dupe of #10227. I realise this is the older ticket, but the description here is less useful than the one in the other ticket (which describes the user-facing behaviour).
I think the behaviours of filter(xxx=None) and get(xxx=None) have changed recently and should return an empty QuerySet or raise model.DoesNotExist, respectively. Did you use a recent version from svn?