[Django] #23805: query `first` method clears cached queryset for prefetch_related

50 views
Skip to first unread message

Django

unread,
Nov 12, 2014, 1:37:53 PM11/12/14
to django-...@googlegroups.com
#23805: query `first` method clears cached queryset for prefetch_related
----------------------------------------------+-------------------------
Reporter: alecklandgraf | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer (models, ORM) | Version: master
Severity: Normal | Keywords: first query
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+-------------------------
When a cached queryset is iterated through with `prefetch_related`, if
`.first()` is called during the iteration, a new DB query is fired off.
This is potentially due to the LIMIT 1 `[:1]` or the `order_by` added to
the queryset with the `first` method.

https://github.com/django/django/blob/master/django/db/models/query.py#L518

ex.
{{{
from django.db import connection
connection.queries = []

pizzas = Pizza.objects.all().prefetch_related('toppings')
print len(connection.queries) # 2

for pizza in pizzas:
first_topping = pizza.toppings.first()

print len(connection.queries) # 2 + number of pizzas
}}}

This fixes in my code at least:
{{{
from django.db import connection
connection.queries = []

pizzas = Pizza.objects.all().prefetch_related('toppings')
print len(connection.queries) # 2

for pizza in pizzas:
try:
first_topping = pizza.toppings.all()[0]
except IndexError:
first_topping = None

print len(connection.queries) # 2
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/23805>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Nov 13, 2014, 5:31:17 AM11/13/14
to django-...@googlegroups.com
#23805: query `first` method clears cached queryset for prefetch_related
-------------------------------------+-------------------------------------

Reporter: alecklandgraf | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: first query | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by tchaumeny):

* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0


Comment:

In the first code example, `first()` is used on an unordered queryset. As
`first()` is intended to be deterministic, it isn't surprising that it
tries to add an order condition (which default to `order_by('pk')`.

Your second code example might work, but it isn't deterministic either as
there is no guarantee on the ordering of `pizza.toppings.all()`.

Do you need an arbitrary element or the first one ? If you need the first
one, there should be some notion of ordering involved, otherwise your
second solution is fine.

Also, you might wanna have a look at
https://docs.djangoproject.com/en/1.7/ref/models/queries/#django.db.models.Prefetch
(I've never used it myself but it should allow more complex
prefetch_related queryset, including ORDER clause as described here
https://docs.djangoproject.com/en/1.7/ref/models/querysets/#prefetch-
related).

--
Ticket URL: <https://code.djangoproject.com/ticket/23805#comment:1>

Django

unread,
Nov 25, 2014, 7:00:05 PM11/25/14
to django-...@googlegroups.com
#23805: query `first` method clears cached queryset for prefetch_related
-------------------------------------+-------------------------------------
Reporter: alecklandgraf | Owner: nobody
Type: Uncategorized | Status: closed

Component: Database layer | Version: master
(models, ORM) | Resolution: invalid

Severity: Normal | Triage Stage:
Keywords: first query | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timgraham):

* status: new => closed
* resolution: => invalid


Comment:

tchaumeny has explained the issue and I don't see any bug here.

--
Ticket URL: <https://code.djangoproject.com/ticket/23805#comment:2>

Reply all
Reply to author
Forward
0 new messages