From 1299421cadc4fcf63585f2f88337078e43e660e0 Mon Sep 17 00:00:00 2001 From: oliver Date: Tue, 16 Oct 2018 00:01:57 +0900 Subject: [PATCH] Fixed #29725 -- Removed unnecessary join in QuerySet.count() and exists() on a many-to-many relation. --- .../db/models/fields/related_descriptors.py | 27 +++++++++++ tests/many_to_many/models.py | 10 ++++ tests/many_to_many/tests.py | 46 +++++++++++++++++-- 3 files changed, 80 insertions(+), 3 deletions(-) diff --git a/django/db/models/fields/related_descriptors.py b/django/db/models/fields/related_descriptors.py index e69fd41e8a..342cbb3465 100644 --- a/django/db/models/fields/related_descriptors.py +++ b/django/db/models/fields/related_descriptors.py @@ -912,6 +912,33 @@ def create_forward_many_to_many_manager(superclass, rel, reverse): False, ) + @property + def constrained_target(self): + # If the through relation's target field's foreign integrity is + # enforced, the query can be performed solely against the through + # table as the INNER JOIN'ing against target table is unnecessary. + if not self.target_field.db_constraint: + return None + db = router.db_for_read(self.through, instance=self.instance) + if not connections[db].features.supports_foreign_keys: + return None + hints = {'instance': self.instance} + manager = self.through._base_manager.db_manager(db, hints=hints) + filters = {self.source_field_name: self.instance.pk} + # Nullable target rows must be excluded as well as they would have + # been filtered out from an INNER JOIN. + if self.target_field.null: + filters['%s__isnull' % self.target_field_name] = False + return manager.filter(**filters) + + def exists(self): + constrained_target = self.constrained_target + return constrained_target.exists() if constrained_target else super().exists() + + def count(self): + constrained_target = self.constrained_target + return constrained_target.count() if constrained_target else super().count() + def add(self, *objs): if not rel.through._meta.auto_created: opts = self.through._meta diff --git a/tests/many_to_many/models.py b/tests/many_to_many/models.py index 22911654ab..7047a4e5d0 100644 --- a/tests/many_to_many/models.py +++ b/tests/many_to_many/models.py @@ -55,3 +55,13 @@ class InheritedArticleA(AbstractArticle): class InheritedArticleB(AbstractArticle): pass + + +class NullableTargetArticle(models.Model): + headline = models.CharField(max_length=100) + publications = models.ManyToManyField(Publication, through='NullablePublicationThrough') + + +class NullablePublicationThrough(models.Model): + article = models.ForeignKey(NullableTargetArticle, models.CASCADE) + publication = models.ForeignKey(Publication, models.CASCADE, null=True) diff --git a/tests/many_to_many/tests.py b/tests/many_to_many/tests.py index 5360b978c2..933eb23a7a 100644 --- a/tests/many_to_many/tests.py +++ b/tests/many_to_many/tests.py @@ -1,7 +1,13 @@ -from django.db import transaction -from django.test import TestCase +from unittest import mock -from .models import Article, InheritedArticleA, InheritedArticleB, Publication +from django.db import connection, transaction +from django.test import TestCase, skipUnlessDBFeature +from django.test.utils import CaptureQueriesContext + +from .models import ( + Article, InheritedArticleA, InheritedArticleB, NullablePublicationThrough, + NullableTargetArticle, Publication, +) class ManyToManyTests(TestCase): @@ -554,3 +560,37 @@ class ManyToManyTests(TestCase): ] ) self.assertQuerysetEqual(b.publications.all(), ['']) + + +class ManyToManyQueryTests(TestCase): + @classmethod + def setUpTestData(cls): + cls.article = Article.objects.create(headline='Django lets you build Web apps easily') + cls.nullable_target_article = NullableTargetArticle.objects.create(headline='The python is good') + NullablePublicationThrough.objects.create(article=cls.nullable_target_article, publication=None) + + @skipUnlessDBFeature('supports_foreign_keys') + def test_count_join_optimization(self): + with CaptureQueriesContext(connection) as query: + self.article.publications.count() + self.assertNotIn('JOIN', query[0]['sql']) + self.assertEqual(self.nullable_target_article.publications.count(), 0) + + def test_count_join_optimization_disabled(self): + with mock.patch.object(connection.features, 'supports_foreign_keys', False), \ + CaptureQueriesContext(connection) as query: + self.article.publications.count() + self.assertIn('JOIN', query[0]['sql']) + + @skipUnlessDBFeature('supports_foreign_keys') + def test_exists_join_optimization(self): + with CaptureQueriesContext(connection) as query: + self.article.publications.exists() + self.assertNotIn('JOIN', query[0]['sql']) + self.assertIs(self.nullable_target_article.publications.exists(), False) + + def test_exists_join_optimization_disabled(self): + with mock.patch.object(connection.features, 'supports_foreign_keys', False), \ + CaptureQueriesContext(connection) as query: + self.article.publications.exists() + self.assertIn('JOIN', query[0]['sql'])