From 254df3a3bbc2f4df51f9e2070ab2e214553f67d0 Mon Sep 17 00:00:00 2001 From: Maxime Toussaint Date: Thu, 31 Aug 2023 08:11:03 -0400 Subject: [PATCH] Fixed #34791 -- Fixed incorrect Prefetch()'s cache for singly related objects. Changed the cache name used for singly related objects to be the to_attr parameter passed to a Prefetch object. This fixes issues with checking if values have already been fetched in cases where the Field already has some prefetched value, but not for the same model attr. --- AUTHORS | 1 + django/db/models/query.py | 38 +++++++++++++++++++-------------- tests/prefetch_related/tests.py | 25 ++++++++++++++++++++++ 3 files changed, 48 insertions(+), 16 deletions(-) diff --git a/AUTHORS b/AUTHORS index b7df889b57..2c922b4ff3 100644 --- a/AUTHORS +++ b/AUTHORS @@ -682,6 +682,7 @@ answer newbie questions, and generally made Django that much better: Max Derkachev Max Smolens Maxime Lorant + Maxime Toussaint Maxime Turcotte Maximilian Merz Maximillian Dornseif diff --git a/django/db/models/query.py b/django/db/models/query.py index 92dd31dfe5..0746dc5c6b 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -2434,11 +2434,23 @@ def get_prefetcher(instance, through_attr, to_attr): the attribute has already been fetched for that instance) """ - def has_to_attr_attribute(instance): - return hasattr(instance, to_attr) + def is_to_attr_fetched(model, to_attr): + # Special case cached_property instances because hasattr() triggers + # attribute computation and assignment. + if isinstance(getattr(model, to_attr, None), cached_property): + + def has_cached_property(instance): + return to_attr in instance.__dict__ + + return has_cached_property + + def has_to_attr_attribute(instance): + return hasattr(instance, to_attr) + + return has_to_attr_attribute prefetcher = None - is_fetched = has_to_attr_attribute + is_fetched = is_to_attr_fetched(instance.__class__, to_attr) # For singly related objects, we have to avoid getting the attribute # from the object, as this will trigger the query. So we first try @@ -2453,7 +2465,12 @@ def get_prefetcher(instance, through_attr, to_attr): # get_prefetch_queryset() method. if hasattr(rel_obj_descriptor, "get_prefetch_queryset"): prefetcher = rel_obj_descriptor - is_fetched = rel_obj_descriptor.is_cached + # If to_attr is set, check if the value has already been set, + # which is done with has_to_attr_attribute(). Do not use the + # method from the descriptor, as the cache_name it defines + # checks the field name, not the to_attr value. + if through_attr == to_attr: + is_fetched = rel_obj_descriptor.is_cached else: # descriptor doesn't support prefetching, so we go ahead and get # the attribute on the instance rather than the class to @@ -2461,18 +2478,7 @@ def get_prefetcher(instance, through_attr, to_attr): rel_obj = getattr(instance, through_attr) if hasattr(rel_obj, "get_prefetch_queryset"): prefetcher = rel_obj - if through_attr != to_attr: - # Special case cached_property instances because hasattr - # triggers attribute computation and assignment. - if isinstance( - getattr(instance.__class__, to_attr, None), cached_property - ): - - def has_cached_property(instance): - return to_attr in instance.__dict__ - - is_fetched = has_cached_property - else: + if through_attr == to_attr: def in_prefetched_cache(instance): return through_attr in instance._prefetched_objects_cache diff --git a/tests/prefetch_related/tests.py b/tests/prefetch_related/tests.py index c8b9a9538b..4566de631e 100644 --- a/tests/prefetch_related/tests.py +++ b/tests/prefetch_related/tests.py @@ -978,6 +978,31 @@ class CustomPrefetchTests(TestCase): with self.assertNumQueries(5): self.traverse_qs(list(houses), [["occupants", "houses", "main_room"]]) + def test_nested_prefetch_related_with_duplicate_prefetch_and_depth(self): + people = Person.objects.prefetch_related( + Prefetch( + "houses__main_room", + queryset=Room.objects.filter(name="Dining room"), + to_attr="dining_room", + ), + "houses__main_room", + ) + with self.assertNumQueries(4): + main_room = people[0].houses.all()[0] + + people = Person.objects.prefetch_related( + "houses__main_room", + Prefetch( + "houses__main_room", + queryset=Room.objects.filter(name="Dining room"), + to_attr="dining_room", + ), + ) + with self.assertNumQueries(4): + main_room = people[0].houses.all()[0] + + self.assertEqual(main_room.main_room, self.room1_1) + def test_values_queryset(self): msg = "Prefetch querysets cannot use raw(), values(), and values_list()." with self.assertRaisesMessage(ValueError, msg):