From 681f7e2b13846def9eff3d26ce1371d05fa90c4c Mon Sep 17 00:00:00 2001 From: Alex Aktsipetrov Date: Tue, 15 Oct 2019 01:59:43 +0300 Subject: [PATCH] Fixed #20577 -- Deferred filtering of prefetched related querysets. Added internal interface to QuerySet that allows to defer next filter call till .query is accessed. Used it to optimize prefetch_related(). Thanks Simon Charette for the review. --- AUTHORS | 1 + .../db/models/fields/related_descriptors.py | 1 + django/db/models/query.py | 29 ++++++++++++++--- tests/prefetch_related/tests.py | 31 +++++++++++++++++++ tests/queryset_pickle/tests.py | 6 ++++ 5 files changed, 64 insertions(+), 4 deletions(-) diff --git a/AUTHORS b/AUTHORS index a3f9d4d736..06998e7c3c 100644 --- a/AUTHORS +++ b/AUTHORS @@ -40,6 +40,7 @@ answer newbie questions, and generally made Django that much better: Alexander Dutton Alexander Myodov Alexandr Tatarinov + Alex Aktsipetrov Alex Becker Alex Couper Alex Dedul diff --git a/django/db/models/fields/related_descriptors.py b/django/db/models/fields/related_descriptors.py index 0b51b68ebe..8468a0b01f 100644 --- a/django/db/models/fields/related_descriptors.py +++ b/django/db/models/fields/related_descriptors.py @@ -883,6 +883,7 @@ def create_forward_many_to_many_manager(superclass, rel, reverse): queryset._add_hints(instance=self.instance) if self._db: queryset = queryset.using(self._db) + queryset._defer_next_filter = True return queryset._next_is_sticky().filter(**self.core_filters) def _remove_prefetched_objects(self): diff --git a/django/db/models/query.py b/django/db/models/query.py index b855720f56..b61c76f80f 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -189,7 +189,7 @@ class QuerySet: self.model = model self._db = using self._hints = hints or {} - self.query = query or sql.Query(self.model) + self._query = query or sql.Query(self.model) self._result_cache = None self._sticky_filter = False self._for_write = False @@ -198,6 +198,20 @@ class QuerySet: self._known_related_objects = {} # {rel_field: {pk: rel_obj}} self._iterable_class = ModelIterable self._fields = None + self._defer_next_filter = False + self._deferred_filter = None + + @property + def query(self): + if self._deferred_filter: + negate, args, kwargs = self._deferred_filter + self._filter_or_exclude_inplace(negate, *args, **kwargs) + self._deferred_filter = None + return self._query + + @query.setter + def query(self, value): + self._query = value def as_manager(cls): # Address the circular dependency between `Queryset` and `Manager`. @@ -914,12 +928,19 @@ class QuerySet: "Cannot filter a query once a slice has been taken." clone = self._chain() - if negate: - clone.query.add_q(~Q(*args, **kwargs)) + if self._defer_next_filter: + self._defer_next_filter = False + clone._deferred_filter = negate, args, kwargs else: - clone.query.add_q(Q(*args, **kwargs)) + clone._filter_or_exclude_inplace(negate, *args, **kwargs) return clone + def _filter_or_exclude_inplace(self, negate, *args, **kwargs): + if negate: + self._query.add_q(~Q(*args, **kwargs)) + else: + self._query.add_q(Q(*args, **kwargs)) + def complex_filter(self, filter_obj): """ Return a new QuerySet instance with filter_obj added to the filters. diff --git a/tests/prefetch_related/tests.py b/tests/prefetch_related/tests.py index 930ba9fbc8..5b944a456b 100644 --- a/tests/prefetch_related/tests.py +++ b/tests/prefetch_related/tests.py @@ -5,6 +5,7 @@ from django.core.exceptions import ObjectDoesNotExist from django.db import connection from django.db.models import Prefetch, QuerySet from django.db.models.query import get_prefetcher, prefetch_related_objects +from django.db.models.sql import Query from django.test import TestCase, override_settings from django.test.utils import CaptureQueriesContext @@ -291,6 +292,20 @@ class PrefetchRelatedTests(TestDataMixin, TestCase): sql = queries[-1]['sql'] self.assertWhereContains(sql, self.author1.id) + def test_filter_deferred(self): + """ + Related filtering of prefetched querysets is deferred until necessary. + """ + add_q = Query.add_q + with mock.patch.object( + Query, + 'add_q', + autospec=True, + side_effect=lambda self, q: add_q(self, q), + ) as add_q_mock: + list(Book.objects.prefetch_related('authors')) + self.assertEqual(add_q_mock.call_count, 1) + class RawQuerySetTests(TestDataMixin, TestCase): def test_basic(self): @@ -823,6 +838,22 @@ class CustomPrefetchTests(TestCase): with self.assertNumQueries(0): self.assertEqual(person.cached_all_houses, all_houses) + def test_filter_deferred(self): + """ + Related filtering of prefetched querysets is deferred until necessary. + """ + add_q = Query.add_q + with mock.patch.object( + Query, + 'add_q', + autospec=True, + side_effect=lambda self, q: add_q(self, q), + ) as add_q_mock: + list(House.objects.prefetch_related( + Prefetch('occupants', queryset=Person.objects.all()) + )) + self.assertEqual(add_q_mock.call_count, 1) + class DefaultManagerTests(TestCase): diff --git a/tests/queryset_pickle/tests.py b/tests/queryset_pickle/tests.py index 1b3f237819..9ef19cfa9d 100644 --- a/tests/queryset_pickle/tests.py +++ b/tests/queryset_pickle/tests.py @@ -212,6 +212,12 @@ class PickleabilityTestCase(TestCase): qs = Happening.objects.annotate(latest_time=models.Max('when')) self.assert_pickles(qs) + def test_filter_deferred(self): + qs = Happening.objects.all() + qs._defer_next_filter = True + qs = qs.filter(id=0) + self.assert_pickles(qs) + def test_missing_django_version_unpickling(self): """ #21430 -- Verifies a warning is raised for querysets that are