From f2388a4b73ee74bd077854798a0ac1669d037304 Mon Sep 17 00:00:00 2001 From: Laurent Lyaudet Date: Sat, 16 Mar 2024 06:51:38 +0100 Subject: [PATCH] Fixed #35309 -- Made prefetch clear ordering for single-valued relationships. --- .../db/models/fields/related_descriptors.py | 6 ++ .../test_prefetch_related_objects.py | 72 ++++++++++++++++++- 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/django/db/models/fields/related_descriptors.py b/django/db/models/fields/related_descriptors.py index a2f00eb172..c7848ee63a 100644 --- a/django/db/models/fields/related_descriptors.py +++ b/django/db/models/fields/related_descriptors.py @@ -195,6 +195,9 @@ class ForwardManyToOneDescriptor: else: query = {"%s__in" % self.field.related_query_name(): instances} queryset = queryset.filter(**query) + # There can be only one object prefetched for each instance so clear + # ordering if the query allows it without side effects. + queryset.query.clear_ordering() # Since we're going to assign directly in the cache, # we must manage the reverse relation cache manually. @@ -469,6 +472,9 @@ class ReverseOneToOneDescriptor: instances_dict = {instance_attr(inst): inst for inst in instances} query = {"%s__in" % self.related.field.name: instances} queryset = queryset.filter(**query) + # There can be only one object prefetched for each instance so clear + # ordering if the query allows it without side effects. + queryset.query.clear_ordering() # Since we're going to assign directly in the cache, # we must manage the reverse relation cache manually. diff --git a/tests/prefetch_related/test_prefetch_related_objects.py b/tests/prefetch_related/test_prefetch_related_objects.py index ca1f904c52..eea9a7fff7 100644 --- a/tests/prefetch_related/test_prefetch_related_objects.py +++ b/tests/prefetch_related/test_prefetch_related_objects.py @@ -1,7 +1,7 @@ from django.db.models import Prefetch, prefetch_related_objects from django.test import TestCase -from .models import Author, Book, Reader +from .models import Author, Book, House, Reader, Room class PrefetchRelatedObjectsTests(TestCase): @@ -33,6 +33,17 @@ class PrefetchRelatedObjectsTests(TestCase): cls.reader1.books_read.add(cls.book1, cls.book4) cls.reader2.books_read.add(cls.book2, cls.book4) + cls.house1 = House.objects.create(name="b1", address="1") + cls.house2 = House.objects.create(name="b2", address="2") + + cls.room1 = Room.objects.create(name="a1", house=cls.house1) + cls.room2 = Room.objects.create(name="a2", house=cls.house2) + + cls.house1.main_room = cls.room1 + cls.house1.save() + cls.house2.main_room = cls.room2 + cls.house2.save() + def test_unknown(self): book1 = Book.objects.get(id=self.book1.id) with self.assertRaises(AttributeError): @@ -58,20 +69,75 @@ class PrefetchRelatedObjectsTests(TestCase): def test_foreignkey_forward(self): authors = list(Author.objects.all()) - with self.assertNumQueries(1): + with self.assertNumQueries(1) as ctx: prefetch_related_objects(authors, "first_book") + self.assertNotIn("ORDER BY", ctx.captured_queries[0]["sql"]) with self.assertNumQueries(0): [author.first_book for author in authors] + authors = list(Author.objects.all()) + with self.assertNumQueries(1) as ctx: + prefetch_related_objects( + authors, + Prefetch("first_book", queryset=Book.objects.order_by("-title")), + ) + self.assertNotIn("ORDER BY", ctx.captured_queries[0]["sql"]) + def test_foreignkey_reverse(self): books = list(Book.objects.all()) - with self.assertNumQueries(1): + with self.assertNumQueries(1) as ctx: prefetch_related_objects(books, "first_time_authors") + self.assertIn("ORDER BY", ctx.captured_queries[0]["sql"]) with self.assertNumQueries(0): [list(book.first_time_authors.all()) for book in books] + books = list(Book.objects.all()) + with self.assertNumQueries(1) as ctx: + prefetch_related_objects( + books, + Prefetch( + "first_time_authors", + queryset=Author.objects.order_by("-name"), + ), + ) + self.assertIn("ORDER BY", ctx.captured_queries[0]["sql"]) + + def test_one_to_one_forward(self): + houses = list(House.objects.all()) + with self.assertNumQueries(1) as ctx: + prefetch_related_objects(houses, "main_room") + self.assertNotIn("ORDER BY", ctx.captured_queries[0]["sql"]) + + with self.assertNumQueries(0): + [house.main_room for house in houses] + + houses = list(House.objects.all()) + with self.assertNumQueries(1) as ctx: + prefetch_related_objects( + houses, + Prefetch("main_room", queryset=Room.objects.order_by("-name")), + ) + self.assertNotIn("ORDER BY", ctx.captured_queries[0]["sql"]) + + def test_one_to_one_reverse(self): + rooms = list(Room.objects.all()) + with self.assertNumQueries(1) as ctx: + prefetch_related_objects(rooms, "main_room_of") + self.assertNotIn("ORDER BY", ctx.captured_queries[0]["sql"]) + + with self.assertNumQueries(0): + [room.main_room_of for room in rooms] + + rooms = list(Room.objects.all()) + with self.assertNumQueries(1) as ctx: + prefetch_related_objects( + rooms, + Prefetch("main_room_of", queryset=House.objects.order_by("-name")), + ) + self.assertNotIn("ORDER BY", ctx.captured_queries[0]["sql"]) + def test_m2m_then_m2m(self): """A m2m can be followed through another m2m.""" authors = list(Author.objects.all())