Fixed #30325 -- Reverted "Fixed #29725 -- Removed unnecessary join in QuerySet.count() and exists() on a many-to-many relation."

This reverts commit 1299421cad due to
a regression with custom managers.
This commit is contained in:
Mariusz Felisiak 2019-04-15 10:35:37 +02:00
parent bfae195b0a
commit 5f7991c42c
4 changed files with 6 additions and 77 deletions

View File

@ -929,33 +929,6 @@ def create_forward_many_to_many_manager(superclass, rel, reverse):
False, 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, through_defaults=None): def add(self, *objs, through_defaults=None):
self._remove_prefetched_objects() self._remove_prefetched_objects()
db = router.db_for_write(self.through, instance=self.instance) db = router.db_for_write(self.through, instance=self.instance)

View File

@ -29,3 +29,7 @@ Bugfixes
* Prevented :djadmin:`makemigrations` from generating infinite migrations for * Prevented :djadmin:`makemigrations` from generating infinite migrations for
check constraints and partial indexes when ``condition`` contains check constraints and partial indexes when ``condition`` contains
a :class:`~python:range` object (:ticket:`30350`). a :class:`~python:range` object (:ticket:`30350`).
* Reverted an optimization in Django 2.2 (:ticket:`29725`) that caused the
inconsistent behavior of ``count()`` and ``exists()`` on a reverse
many-to-many relationship with a custom manager (:ticket:`30325`).

View File

@ -55,13 +55,3 @@ class InheritedArticleA(AbstractArticle):
class InheritedArticleB(AbstractArticle): class InheritedArticleB(AbstractArticle):
pass 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)

View File

@ -1,13 +1,9 @@
from unittest import mock from unittest import mock
from django.db import connection, transaction from django.db import transaction
from django.test import TestCase, skipUnlessDBFeature from django.test import TestCase, skipUnlessDBFeature
from django.test.utils import CaptureQueriesContext
from .models import ( from .models import Article, InheritedArticleA, InheritedArticleB, Publication
Article, InheritedArticleA, InheritedArticleB, NullablePublicationThrough,
NullableTargetArticle, Publication,
)
class ManyToManyTests(TestCase): class ManyToManyTests(TestCase):
@ -584,37 +580,3 @@ class ManyToManyTests(TestCase):
] ]
) )
self.assertQuerysetEqual(b.publications.all(), ['<Publication: Science Weekly>']) self.assertQuerysetEqual(b.publications.all(), ['<Publication: Science Weekly>'])
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'])