diff options
| author | Simon Charette <charette.s@gmail.com> | 2025-01-14 06:18:30 +0100 |
|---|---|---|
| committer | Sarah Boyce <42296566+sarahboyce@users.noreply.github.com> | 2025-02-06 14:31:32 +0100 |
| commit | 8aea6b802ced18a54f00db71c53e09c643f7514c (patch) | |
| tree | d69bb0459bf6905fb93062b948252765e16b5a2d | |
| parent | d57bf4618c15bc7e920fd4488b13907fe33eb786 (diff) | |
[5.2.x] Fixed #35677 -- Avoided non-sticky filtering of prefetched many-to-many.
The original queryset._next_is_sticky() call never had the intended effect as
no further filtering was applied internally after the pk__in lookup making it
a noop.
In order to be coherent with how related filters are applied when retrieving
objects from a related manager the effects of what calling _next_is_sticky()
prior to applying annotations and filters to the queryset provided for
prefetching are emulated by allowing the reuse of all pre-existing JOINs.
Thanks David Glenck and Thiago Bellini Ribeiro for the detailed reports and
tests.
Backport of 2598b371a93e21d84b7a2a99b2329535c8c0c138 from main.
| -rw-r--r-- | django/db/models/fields/related_descriptors.py | 7 | ||||
| -rw-r--r-- | django/db/models/sql/query.py | 8 | ||||
| -rw-r--r-- | tests/prefetch_related/models.py | 1 | ||||
| -rw-r--r-- | tests/prefetch_related/tests.py | 46 |
4 files changed, 55 insertions, 7 deletions
diff --git a/django/db/models/fields/related_descriptors.py b/django/db/models/fields/related_descriptors.py index d858144152..7dcec5368b 100644 --- a/django/db/models/fields/related_descriptors.py +++ b/django/db/models/fields/related_descriptors.py @@ -114,7 +114,10 @@ def _filter_prefetch_queryset(queryset, field_name, instances): if high_mark is not None: predicate &= LessThanOrEqual(window, high_mark) queryset.query.clear_limits() - return queryset.filter(predicate) + # All pre-existing JOINs must be re-used when applying the predicate to + # avoid unintended spanning of multi-valued relationships. + queryset.query.add_q(predicate, reuse_all=True) + return queryset class ForwardManyToOneDescriptor: @@ -1164,7 +1167,7 @@ def create_forward_many_to_many_manager(superclass, rel, reverse): queryset._add_hints(instance=instances[0]) queryset = queryset.using(queryset._db or self._db) queryset = _filter_prefetch_queryset( - queryset._next_is_sticky(), self.query_field_name, instances + queryset, self.query_field_name, instances ) # M2M: need to annotate the query in order to get the primary model diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index ddf6c891fa..ec47d9aa24 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -1616,7 +1616,7 @@ class Query(BaseExpression): def add_filter(self, filter_lhs, filter_rhs): self.add_q(Q((filter_lhs, filter_rhs))) - def add_q(self, q_object): + def add_q(self, q_object, reuse_all=False): """ A preprocessor for the internal _add_q(). Responsible for doing final join promotion. @@ -1630,7 +1630,11 @@ class Query(BaseExpression): existing_inner = { a for a in self.alias_map if self.alias_map[a].join_type == INNER } - clause, _ = self._add_q(q_object, self.used_aliases) + if reuse_all: + can_reuse = set(self.alias_map) + else: + can_reuse = self.used_aliases + clause, _ = self._add_q(q_object, can_reuse) if clause: self.where.add(clause, AND) self.demote_joins(existing_inner) diff --git a/tests/prefetch_related/models.py b/tests/prefetch_related/models.py index 0d9dbe6066..2f37cde1c8 100644 --- a/tests/prefetch_related/models.py +++ b/tests/prefetch_related/models.py @@ -35,6 +35,7 @@ class FavoriteAuthors(models.Model): likes_author = models.ForeignKey( Author, models.CASCADE, to_field="name", related_name="likes_me" ) + is_active = models.BooleanField(default=True) class Meta: ordering = ["id"] diff --git a/tests/prefetch_related/tests.py b/tests/prefetch_related/tests.py index 99e3cd6b80..b3531cc36f 100644 --- a/tests/prefetch_related/tests.py +++ b/tests/prefetch_related/tests.py @@ -3,7 +3,7 @@ from unittest import mock from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ObjectDoesNotExist from django.db import NotSupportedError, connection -from django.db.models import Prefetch, QuerySet, prefetch_related_objects +from django.db.models import F, Prefetch, QuerySet, prefetch_related_objects from django.db.models.fields.related import ForwardManyToOneDescriptor from django.db.models.query import get_prefetcher, prefetch_one_level from django.db.models.sql import Query @@ -364,7 +364,7 @@ class PrefetchRelatedTests(TestDataMixin, TestCase): Query, "add_q", autospec=True, - side_effect=lambda self, q: add_q(self, q), + side_effect=lambda self, q, reuse_all: add_q(self, q), ) as add_q_mock: list(Book.objects.prefetch_related(relation)) self.assertEqual(add_q_mock.call_count, 1) @@ -395,6 +395,46 @@ class PrefetchRelatedTests(TestDataMixin, TestCase): with self.assertRaisesMessage(ValueError, msg): Book.objects.prefetch_related("authors").iterator() + def test_m2m_join_reuse(self): + FavoriteAuthors.objects.bulk_create( + [ + FavoriteAuthors( + author=self.author1, likes_author=self.author3, is_active=True + ), + FavoriteAuthors( + author=self.author1, + likes_author=self.author4, + is_active=False, + ), + FavoriteAuthors( + author=self.author2, likes_author=self.author3, is_active=True + ), + FavoriteAuthors( + author=self.author2, likes_author=self.author4, is_active=True + ), + ] + ) + with self.assertNumQueries(2): + authors = list( + Author.objects.filter( + pk__in=[self.author1.pk, self.author2.pk] + ).prefetch_related( + Prefetch( + "favorite_authors", + queryset=( + Author.objects.annotate( + active_favorite=F("likes_me__is_active"), + ).filter(active_favorite=True) + ), + to_attr="active_favorite_authors", + ) + ) + ) + self.assertEqual(authors[0].active_favorite_authors, [self.author3]) + self.assertEqual( + authors[1].active_favorite_authors, [self.author3, self.author4] + ) + class RawQuerySetTests(TestDataMixin, TestCase): def test_basic(self): @@ -1049,7 +1089,7 @@ class CustomPrefetchTests(TestCase): Query, "add_q", autospec=True, - side_effect=lambda self, q: add_q(self, q), + side_effect=lambda self, q, reuse_all: add_q(self, q), ) as add_q_mock: list( House.objects.prefetch_related( |
