Opened 12 years ago

Closed 12 years ago

#17439 closed Bug (fixed)

prefetch_related issues extra queries for optional OneToOneFields

Reported by: gwahl@… Owned by: Aymeric Augustin
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Given these models:

class ModelA(models.Model):
    pass

class ModelB(models.Model):
    a = models.OneToOneField(ModelA, related_name='b')

I expect that after doing ModelA.objects.prefetch_related('b'), the b
attribute of all the results can be accessed without causing another query. This is not the case:

>>> from app.models import *
>>> a = ModelA.objects.all()[0]
SELECT "app_modela"."id"
FROM "app_modela" LIMIT 1

>>> a.b  # should do a query, prefetch_related was not used
SELECT "app_modelb"."id",
       "app_modelb"."a_id"
FROM "app_modelb"
WHERE "app_modelb"."a_id" = 1

Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/srv/python-environments/egg/src/django/django/db/models/fields/related.py", line 255, in __get__
    rel_obj = self.get_query_set(instance=instance).get(**params)
  File "/srv/python-environments/egg/src/django/django/db/models/query.py", line 361, in get
    % self.model._meta.object_name)
DoesNotExist: ModelB matching query does not exist.
## it did another query, and raised DoesNotExist, just like expected.
>>> a = ModelA.objects.all().prefetch_related('b')[0]
SELECT "app_modela"."id"
FROM "app_modela" LIMIT 1

SELECT "app_modelb"."id",
       "app_modelb"."a_id"
FROM "app_modelb"

>>> a.b  # should not do a query, we already got all the `b`s
SELECT "app_modelb"."id",
       "app_modelb"."a_id"
FROM "app_modelb"
WHERE "app_modelb"."a_id" = 1

Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/srv/python-environments/egg/src/django/django/db/models/fields/related.py", line 255, in __get__
    rel_obj = self.get_query_set(instance=instance).get(**params)
  File "/srv/python-environments/egg/src/django/django/db/models/query.py", line 361, in get
    % self.model._meta.object_name)
DoesNotExist: ModelB matching query does not exist.
## Another query was issued even though we know it can not return any results
>>> 

Any possible b object was already fetched by prefetch_related, but
accessing the property causes another query anyway. Note that this is only the
case when a does not have a b object, the extra query does not happen if
there was a b fetched by prefetch_related.

Attachments (2)

attribute_cache_test.diff (755 bytes ) - added by gwahl@… 12 years ago.
17439.patch (2.9 KB ) - added by Aymeric Augustin 12 years ago.

Download all attachments as: .zip

Change History (15)

by gwahl@…, 12 years ago

Attachment: attribute_cache_test.diff added

comment:1 by gwahl@…, 12 years ago

It looks like this is part of a bigger problem -- DoesNotExist is never cached:

>>> from app.models import *
>>> ModelA().save()
INSERT INTO "app_modela" ("id")
VALUES (NULL)

>>> ModelB(a=ModelA.objects.all()[0]).save()
SELECT "app_modela"."id"
FROM "app_modela" LIMIT 1

INSERT INTO "app_modelb" ("a_id")
VALUES (1)

>>> a = ModelA.objects.all()[0]
SELECT "app_modela"."id"
FROM "app_modela" LIMIT 1

>>> a.b
SELECT "app_modelb"."id",
       "app_modelb"."a_id"
FROM "app_modelb"
WHERE "app_modelb"."a_id" = 1

<ModelB: ModelB object>
>>> a.b  # no query, just as expected
<ModelB: ModelB object>
>>> a = ModelA()
>>> a.save()
INSERT INTO "app_modela" ("id")
VALUES (NULL)

>>> a.b
SELECT "app_modelb"."id",
       "app_modelb"."a_id"
FROM "app_modelb"
WHERE "app_modelb"."a_id" = 3

Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/srv/python-environments/egg/src/django/django/db/models/fields/related.py", line 255, in __get__
    rel_obj = self.get_query_set(instance=instance).get(**params)
  File "/srv/python-environments/egg/src/django/django/db/models/query.py", line 361, in get
    % self.model._meta.object_name)
DoesNotExist: ModelB matching query does not exist.
>>> a.b  # does the same query again!
SELECT "app_modelb"."id",
       "app_modelb"."a_id"
FROM "app_modelb"
WHERE "app_modelb"."a_id" = 3

Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/srv/python-environments/egg/src/django/django/db/models/fields/related.py", line 255, in __get__
    rel_obj = self.get_query_set(instance=instance).get(**params)
  File "/srv/python-environments/egg/src/django/django/db/models/query.py", line 361, in get
    % self.model._meta.object_name)
DoesNotExist: ModelB matching query does not exist.

According to https://docs.djangoproject.com/en/1.2/topics/db/optimization/#understand-cached-attributes, the second time accessing an attribute should not cause a query. I've added a patch with a failing test for this.

comment:2 by Aymeric Augustin, 12 years ago

Triage Stage: UnreviewedAccepted

comment:3 by Aymeric Augustin, 12 years ago

The same problem also exists when traversing the relation in the reverse direction.

comment:4 by Aymeric Augustin, 12 years ago

This could be a duplicate of #10227.

comment:5 by Luke Plant, 12 years ago

I think #10227 will need to be fixed first, but that probably won't fix this bug without some additional work in the prefetch code.

comment:6 by Aymeric Augustin, 12 years ago

Agreed. #13839 is also in the same area (ie. caching the absence of a related object).

comment:7 by Aymeric Augustin, 12 years ago

In [17890]:

Made the caching of related and reverse related objects consistent in OneToOneFields. Fixed #13839. Refs #17439.

comment:8 by Aymeric Augustin, 12 years ago

This is easier to fix after r17890. I think the attached patch does the job. Luke, I'd appreciate a sanity check if you have some time.

by Aymeric Augustin, 12 years ago

Attachment: 17439.patch added

comment:9 by Aymeric Augustin, 12 years ago

Owner: changed from nobody to Aymeric Augustin

I'm assigning the ticket to myself in order to keep track of it. Feel free to commit the patch if you like it.

comment:10 by gwahl@…, 12 years ago

The patch functions for me.

comment:11 by Aymeric Augustin, 12 years ago

Triage Stage: AcceptedReady for checkin

comment:12 by Aymeric Augustin, 12 years ago

In fact, since r17890, prefetch_related can die with an exception on missing reverse-related objects -- it relied on a bug.

comment:13 by Aymeric Augustin, 12 years ago

Resolution: fixed
Status: newclosed

In [17899]:

Fixed #17439 -- Prevented spurious queries for missing objects after prefetch_related has run.

That affects nullable foreign key, nullable one-to-one, and reverse one-to-one relations.

Note: See TracTickets for help on using tickets.
Back to Top