From 24a41ecc3590f8d92d085d4a4caa5f8bf962ceae Mon Sep 17 00:00:00 2001 From: Loic Bistuer Date: Mon, 19 May 2014 03:43:13 +0700 Subject: [PATCH] [1.7.x] Fixed #22650 -- Fixed regression on prefetch_related. Regression from f51c1f59 when using select_related then prefetch_related on the reverse side of an O2O: Author.objects.select_related('bio').prefetch_related('bio__books') Thanks Aymeric Augustin for the report and tests. Refs #17001. Backport of bdf3473e64 from master --- django/db/models/query.py | 17 ++++----- tests/prefetch_related/models.py | 9 +++++ tests/prefetch_related/tests.py | 62 +++++++++++++++++++++++++++++++- 3 files changed, 77 insertions(+), 11 deletions(-) diff --git a/django/db/models/query.py b/django/db/models/query.py index abe882eaeb..d518b012b7 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -1791,15 +1791,6 @@ def prefetch_related_objects(result_cache, related_lookups): done_queries[prefetch_to] = obj_list auto_lookups.extend(normalize_prefetch_lookups(additional_lookups, prefetch_to)) followed_descriptors.add(descriptor) - elif isinstance(getattr(first_obj, through_attr), list): - # The current part of the lookup relates to a custom Prefetch. - # This means that obj.attr is a list of related objects, and - # thus we must turn the obj.attr lists into a single related - # object list. - new_list = [] - for obj in obj_list: - new_list.extend(getattr(obj, through_attr)) - obj_list = new_list else: # Either a singly related object that has already been fetched # (e.g. via select_related), or hopefully some other property @@ -1816,7 +1807,13 @@ def prefetch_related_objects(result_cache, related_lookups): continue if new_obj is None: continue - new_obj_list.append(new_obj) + # We special-case `list` rather than something more generic + # like `Iterable` because we don't want to accidentally match + # user models that define __iter__. + if isinstance(new_obj, list): + new_obj_list.extend(new_obj) + else: + new_obj_list.append(new_obj) obj_list = new_obj_list diff --git a/tests/prefetch_related/models.py b/tests/prefetch_related/models.py index 8fec5d4b7b..40540e9b26 100644 --- a/tests/prefetch_related/models.py +++ b/tests/prefetch_related/models.py @@ -66,6 +66,11 @@ class BookWithYear(Book): AuthorWithAge, related_name='books_with_year') +class Bio(models.Model): + author = models.OneToOneField(Author) + books = models.ManyToManyField(Book, blank=True) + + @python_2_unicode_compatible class Reader(models.Model): name = models.CharField(max_length=50) @@ -196,6 +201,10 @@ class Person(models.Model): # Assume business logic forces every person to have at least one house. return sorted(self.houses.all(), key=lambda house: -house.rooms.count())[0] + @property + def all_houses(self): + return list(self.houses.all()) + class Meta: ordering = ['id'] diff --git a/tests/prefetch_related/tests.py b/tests/prefetch_related/tests.py index 239e5747f6..596702d694 100644 --- a/tests/prefetch_related/tests.py +++ b/tests/prefetch_related/tests.py @@ -9,7 +9,7 @@ from django.test import TestCase, override_settings from django.utils import six from django.utils.encoding import force_text -from .models import (Author, Book, Reader, Qualification, Teacher, Department, +from .models import (Author, Bio, Book, Reader, Qualification, Teacher, Department, TaggedItem, Bookmark, AuthorAddress, FavoriteAuthors, AuthorWithAge, BookWithYear, BookReview, Person, House, Room, Employee, Comment, LessonEntry, WordEntry, Author2) @@ -192,6 +192,20 @@ class PrefetchRelatedTests(TestCase): ["Amy"], ["Amy", "Belinda"]]) + def test_reverse_one_to_one_then_m2m(self): + """ + Test that we can follow a m2m relation after going through + the select_related reverse of a o2o. + """ + qs = Author.objects.prefetch_related('bio__books').select_related('bio') + + with self.assertNumQueries(1): + list(qs.all()) + + Bio.objects.create(author=self.author1) + with self.assertNumQueries(2): + list(qs.all()) + def test_attribute_error(self): qs = Reader.objects.all().prefetch_related('books_read__xyz') with self.assertRaises(AttributeError) as cm: @@ -452,6 +466,52 @@ class CustomPrefetchTests(TestCase): ) self.assertEqual(lst1, lst2) + def test_traverse_single_item_property(self): + # Control lookups. + with self.assertNumQueries(5): + lst1 = self.traverse_qs( + Person.objects.prefetch_related( + 'houses__rooms', + 'primary_house__occupants__houses', + ), + [['primary_house', 'occupants', 'houses']] + ) + + # Test lookups. + with self.assertNumQueries(5): + lst2 = self.traverse_qs( + Person.objects.prefetch_related( + 'houses__rooms', + Prefetch('primary_house__occupants', to_attr='occupants_lst'), + 'primary_house__occupants_lst__houses', + ), + [['primary_house', 'occupants_lst', 'houses']] + ) + self.assertEqual(lst1, lst2) + + def test_traverse_multiple_items_property(self): + # Control lookups. + with self.assertNumQueries(4): + lst1 = self.traverse_qs( + Person.objects.prefetch_related( + 'houses', + 'all_houses__occupants__houses', + ), + [['all_houses', 'occupants', 'houses']] + ) + + # Test lookups. + with self.assertNumQueries(4): + lst2 = self.traverse_qs( + Person.objects.prefetch_related( + 'houses', + Prefetch('all_houses__occupants', to_attr='occupants_lst'), + 'all_houses__occupants_lst__houses', + ), + [['all_houses', 'occupants_lst', 'houses']] + ) + self.assertEqual(lst1, lst2) + def test_custom_qs(self): # Test basic. with self.assertNumQueries(2):