mirror of
https://github.com/django/django.git
synced 2025-03-06 15:32:33 +00:00
[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.
This commit is contained in:
parent
d57bf4618c
commit
8aea6b802c
@ -114,7 +114,10 @@ def _filter_prefetch_queryset(queryset, field_name, instances):
|
|||||||
if high_mark is not None:
|
if high_mark is not None:
|
||||||
predicate &= LessThanOrEqual(window, high_mark)
|
predicate &= LessThanOrEqual(window, high_mark)
|
||||||
queryset.query.clear_limits()
|
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:
|
class ForwardManyToOneDescriptor:
|
||||||
@ -1164,7 +1167,7 @@ def create_forward_many_to_many_manager(superclass, rel, reverse):
|
|||||||
queryset._add_hints(instance=instances[0])
|
queryset._add_hints(instance=instances[0])
|
||||||
queryset = queryset.using(queryset._db or self._db)
|
queryset = queryset.using(queryset._db or self._db)
|
||||||
queryset = _filter_prefetch_queryset(
|
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
|
# M2M: need to annotate the query in order to get the primary model
|
||||||
|
@ -1616,7 +1616,7 @@ class Query(BaseExpression):
|
|||||||
def add_filter(self, filter_lhs, filter_rhs):
|
def add_filter(self, filter_lhs, filter_rhs):
|
||||||
self.add_q(Q((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
|
A preprocessor for the internal _add_q(). Responsible for doing final
|
||||||
join promotion.
|
join promotion.
|
||||||
@ -1630,7 +1630,11 @@ class Query(BaseExpression):
|
|||||||
existing_inner = {
|
existing_inner = {
|
||||||
a for a in self.alias_map if self.alias_map[a].join_type == 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:
|
if clause:
|
||||||
self.where.add(clause, AND)
|
self.where.add(clause, AND)
|
||||||
self.demote_joins(existing_inner)
|
self.demote_joins(existing_inner)
|
||||||
|
@ -35,6 +35,7 @@ class FavoriteAuthors(models.Model):
|
|||||||
likes_author = models.ForeignKey(
|
likes_author = models.ForeignKey(
|
||||||
Author, models.CASCADE, to_field="name", related_name="likes_me"
|
Author, models.CASCADE, to_field="name", related_name="likes_me"
|
||||||
)
|
)
|
||||||
|
is_active = models.BooleanField(default=True)
|
||||||
|
|
||||||
class Meta:
|
class Meta:
|
||||||
ordering = ["id"]
|
ordering = ["id"]
|
||||||
|
@ -3,7 +3,7 @@ from unittest import mock
|
|||||||
from django.contrib.contenttypes.models import ContentType
|
from django.contrib.contenttypes.models import ContentType
|
||||||
from django.core.exceptions import ObjectDoesNotExist
|
from django.core.exceptions import ObjectDoesNotExist
|
||||||
from django.db import NotSupportedError, connection
|
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.fields.related import ForwardManyToOneDescriptor
|
||||||
from django.db.models.query import get_prefetcher, prefetch_one_level
|
from django.db.models.query import get_prefetcher, prefetch_one_level
|
||||||
from django.db.models.sql import Query
|
from django.db.models.sql import Query
|
||||||
@ -364,7 +364,7 @@ class PrefetchRelatedTests(TestDataMixin, TestCase):
|
|||||||
Query,
|
Query,
|
||||||
"add_q",
|
"add_q",
|
||||||
autospec=True,
|
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:
|
) as add_q_mock:
|
||||||
list(Book.objects.prefetch_related(relation))
|
list(Book.objects.prefetch_related(relation))
|
||||||
self.assertEqual(add_q_mock.call_count, 1)
|
self.assertEqual(add_q_mock.call_count, 1)
|
||||||
@ -395,6 +395,46 @@ class PrefetchRelatedTests(TestDataMixin, TestCase):
|
|||||||
with self.assertRaisesMessage(ValueError, msg):
|
with self.assertRaisesMessage(ValueError, msg):
|
||||||
Book.objects.prefetch_related("authors").iterator()
|
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):
|
class RawQuerySetTests(TestDataMixin, TestCase):
|
||||||
def test_basic(self):
|
def test_basic(self):
|
||||||
@ -1049,7 +1089,7 @@ class CustomPrefetchTests(TestCase):
|
|||||||
Query,
|
Query,
|
||||||
"add_q",
|
"add_q",
|
||||||
autospec=True,
|
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:
|
) as add_q_mock:
|
||||||
list(
|
list(
|
||||||
House.objects.prefetch_related(
|
House.objects.prefetch_related(
|
||||||
|
Loading…
x
Reference in New Issue
Block a user