Code

Opened 3 years ago

Closed 2 years ago

#17439 closed Bug (fixed)

prefetch_related issues extra queries for optional OneToOneFields

Reported by: gwahl@… Owned by: aaugustin
Component: Database layer (models, ORM) Version: master
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@… 3 years ago.
17439.patch (2.9 KB) - added by aaugustin 2 years ago.

Download all attachments as: .zip

Change History (15)

Changed 3 years ago by gwahl@…

comment:1 Changed 3 years ago by gwahl@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 Changed 3 years ago by aaugustin

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 2 years ago by aaugustin

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

comment:4 Changed 2 years ago by aaugustin

This could be a duplicate of #10227.

comment:5 Changed 2 years ago by lukeplant

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 Changed 2 years ago by aaugustin

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

comment:7 Changed 2 years ago by aaugustin

In [17890]:

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

comment:8 Changed 2 years ago by aaugustin

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.

Changed 2 years ago by aaugustin

comment:9 Changed 2 years ago by aaugustin

  • Owner changed from nobody to aaugustin

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 Changed 2 years ago by gwahl@…

The patch functions for me.

comment:11 Changed 2 years ago by aaugustin

  • Triage Stage changed from Accepted to Ready for checkin

comment:12 Changed 2 years ago by aaugustin

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

comment:13 Changed 2 years ago by aaugustin

  • Resolution set to fixed
  • Status changed from new to closed

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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.