Fixed the ordering of prefetch lookups so that latter lookups can refer to former lookups.

Thanks Anssi Kääriäinen and Tim Graham for the reviews. Refs #17001 and #22650.
This commit is contained in:
Loic Bistuer 2014-05-19 11:14:55 +07:00
parent bdf3473e64
commit 870b0a1f86
2 changed files with 22 additions and 23 deletions

View File

@ -2,8 +2,8 @@
The main QuerySet implementation. This provides the public API for the ORM. The main QuerySet implementation. This provides the public API for the ORM.
""" """
from collections import deque
import copy import copy
import itertools
import sys import sys
from django.conf import settings from django.conf import settings
@ -1680,6 +1680,9 @@ class Prefetch(object):
return self.prefetch_to == other.prefetch_to return self.prefetch_to == other.prefetch_to
return False return False
def __hash__(self):
return hash(self.__class__) ^ hash(self.prefetch_to)
def normalize_prefetch_lookups(lookups, prefix=None): def normalize_prefetch_lookups(lookups, prefix=None):
""" """
@ -1713,11 +1716,12 @@ def prefetch_related_objects(result_cache, related_lookups):
# ensure we don't do duplicate work. # ensure we don't do duplicate work.
done_queries = {} # dictionary of things like 'foo__bar': [results] done_queries = {} # dictionary of things like 'foo__bar': [results]
auto_lookups = [] # we add to this as we go through. auto_lookups = set() # we add to this as we go through.
followed_descriptors = set() # recursion protection followed_descriptors = set() # recursion protection
all_lookups = itertools.chain(related_lookups, auto_lookups) all_lookups = deque(related_lookups)
for lookup in all_lookups: while all_lookups:
lookup = all_lookups.popleft()
if lookup.prefetch_to in done_queries: if lookup.prefetch_to in done_queries:
if lookup.queryset: if lookup.queryset:
raise ValueError("'%s' lookup was already seen with a different queryset. " raise ValueError("'%s' lookup was already seen with a different queryset. "
@ -1788,7 +1792,9 @@ def prefetch_related_objects(result_cache, related_lookups):
# the new lookups from relationships we've seen already. # the new lookups from relationships we've seen already.
if not (lookup in auto_lookups and descriptor in followed_descriptors): if not (lookup in auto_lookups and descriptor in followed_descriptors):
done_queries[prefetch_to] = obj_list done_queries[prefetch_to] = obj_list
auto_lookups.extend(normalize_prefetch_lookups(additional_lookups, prefetch_to)) new_lookups = normalize_prefetch_lookups(additional_lookups, prefetch_to)
auto_lookups.update(new_lookups)
all_lookups.extendleft(new_lookups)
followed_descriptors.add(descriptor) followed_descriptors.add(descriptor)
else: else:
# Either a singly related object that has already been fetched # Either a singly related object that has already been fetched
@ -1827,7 +1833,6 @@ def get_prefetcher(instance, attr):
a boolean that is True if the attribute has already been fetched) a boolean that is True if the attribute has already been fetched)
""" """
prefetcher = None prefetcher = None
attr_found = False
is_fetched = False is_fetched = False
# For singly related objects, we have to avoid getting the attribute # For singly related objects, we have to avoid getting the attribute
@ -1835,16 +1840,7 @@ def get_prefetcher(instance, attr):
# on the class, in order to get the descriptor object. # on the class, in order to get the descriptor object.
rel_obj_descriptor = getattr(instance.__class__, attr, None) rel_obj_descriptor = getattr(instance.__class__, attr, None)
if rel_obj_descriptor is None: if rel_obj_descriptor is None:
try: attr_found = hasattr(instance, attr)
rel_obj = getattr(instance, attr)
attr_found = True
# If we are following a lookup path which leads us through a previous
# fetch from a custom Prefetch then we might end up into a list
# instead of related qs. This means the objects are already fetched.
if isinstance(rel_obj, list):
is_fetched = True
except AttributeError:
pass
else: else:
attr_found = True attr_found = True
if rel_obj_descriptor: if rel_obj_descriptor:
@ -1889,10 +1885,10 @@ def prefetch_one_level(instances, prefetcher, lookup, level):
rel_qs, rel_obj_attr, instance_attr, single, cache_name = ( rel_qs, rel_obj_attr, instance_attr, single, cache_name = (
prefetcher.get_prefetch_queryset(instances, lookup.get_current_queryset(level))) prefetcher.get_prefetch_queryset(instances, lookup.get_current_queryset(level)))
# We have to handle the possibility that the default manager itself added # We have to handle the possibility that the QuerySet we just got back
# prefetch_related lookups to the QuerySet we just got back. We don't want to # contains some prefetch_related lookups. We don't want to trigger the
# trigger the prefetch_related functionality by evaluating the query. # prefetch_related functionality by evaluating the query. Rather, we need
# Rather, we need to merge in the prefetch_related lookups. # to merge in the prefetch_related lookups.
additional_lookups = getattr(rel_qs, '_prefetch_related_lookups', []) additional_lookups = getattr(rel_qs, '_prefetch_related_lookups', [])
if additional_lookups: if additional_lookups:
# Don't need to clone because the manager should have given us a fresh # Don't need to clone because the manager should have given us a fresh

View File

@ -195,7 +195,7 @@ class PrefetchRelatedTests(TestCase):
def test_reverse_one_to_one_then_m2m(self): def test_reverse_one_to_one_then_m2m(self):
""" """
Test that we can follow a m2m relation after going through Test that we can follow a m2m relation after going through
the select_related reverse of a o2o. the select_related reverse of an o2o.
""" """
qs = Author.objects.prefetch_related('bio__books').select_related('bio') qs = Author.objects.prefetch_related('bio__books').select_related('bio')
@ -559,13 +559,16 @@ class CustomPrefetchTests(TestCase):
inner_rooms_qs = Room.objects.filter(pk__in=[self.room1_1.pk, self.room1_2.pk]) inner_rooms_qs = Room.objects.filter(pk__in=[self.room1_1.pk, self.room1_2.pk])
houses_qs_prf = House.objects.prefetch_related( houses_qs_prf = House.objects.prefetch_related(
Prefetch('rooms', queryset=inner_rooms_qs, to_attr='rooms_lst')) Prefetch('rooms', queryset=inner_rooms_qs, to_attr='rooms_lst'))
with self.assertNumQueries(3): with self.assertNumQueries(4):
lst2 = list(Person.objects.prefetch_related( lst2 = list(Person.objects.prefetch_related(
Prefetch('houses', queryset=houses_qs_prf.filter(pk=self.house1.pk), to_attr='houses_lst'))) Prefetch('houses', queryset=houses_qs_prf.filter(pk=self.house1.pk), to_attr='houses_lst'),
Prefetch('houses_lst__rooms_lst__main_room_of')
))
self.assertEqual(len(lst2[0].houses_lst[0].rooms_lst), 2) self.assertEqual(len(lst2[0].houses_lst[0].rooms_lst), 2)
self.assertEqual(lst2[0].houses_lst[0].rooms_lst[0], self.room1_1) self.assertEqual(lst2[0].houses_lst[0].rooms_lst[0], self.room1_1)
self.assertEqual(lst2[0].houses_lst[0].rooms_lst[1], self.room1_2) self.assertEqual(lst2[0].houses_lst[0].rooms_lst[1], self.room1_2)
self.assertEqual(lst2[0].houses_lst[0].rooms_lst[0].main_room_of, self.house1)
self.assertEqual(len(lst2[1].houses_lst), 0) self.assertEqual(len(lst2[1].houses_lst), 0)
# Test ReverseSingleRelatedObjectDescriptor. # Test ReverseSingleRelatedObjectDescriptor.