summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMariusz Felisiak <felisiak.mariusz@gmail.com>2019-04-15 10:35:37 +0200
committerMariusz Felisiak <felisiak.mariusz@gmail.com>2019-04-15 12:02:26 +0200
commit5f7991c42cff73b6278106d499d719b726f85ead (patch)
tree9ed23eb0c50e548230f4f59815e5aeed25816b4c
parentbfae195b0a2c8dae755610a7e23add5c6bc37b5e (diff)
Fixed #30325 -- Reverted "Fixed #29725 -- Removed unnecessary join in QuerySet.count() and exists() on a many-to-many relation."
This reverts commit 1299421cadc4fcf63585f2f88337078e43e660e0 due to a regression with custom managers.
-rw-r--r--django/db/models/fields/related_descriptors.py27
-rw-r--r--docs/releases/2.2.1.txt4
-rw-r--r--tests/many_to_many/models.py10
-rw-r--r--tests/many_to_many/tests.py42
4 files changed, 6 insertions, 77 deletions
diff --git a/django/db/models/fields/related_descriptors.py b/django/db/models/fields/related_descriptors.py
index 2b426c37a5..c1295ee960 100644
--- a/django/db/models/fields/related_descriptors.py
+++ b/django/db/models/fields/related_descriptors.py
@@ -929,33 +929,6 @@ 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, through_defaults=None):
self._remove_prefetched_objects()
db = router.db_for_write(self.through, instance=self.instance)
diff --git a/docs/releases/2.2.1.txt b/docs/releases/2.2.1.txt
index dcad58b2fd..3a2453dbd1 100644
--- a/docs/releases/2.2.1.txt
+++ b/docs/releases/2.2.1.txt
@@ -29,3 +29,7 @@ Bugfixes
* Prevented :djadmin:`makemigrations` from generating infinite migrations for
check constraints and partial indexes when ``condition`` contains
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`).
diff --git a/tests/many_to_many/models.py b/tests/many_to_many/models.py
index a2590b8b36..28d23f2a82 100644
--- a/tests/many_to_many/models.py
+++ b/tests/many_to_many/models.py
@@ -55,13 +55,3 @@ 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 f0156bbe8d..9dc53307a1 100644
--- a/tests/many_to_many/tests.py
+++ b/tests/many_to_many/tests.py
@@ -1,13 +1,9 @@
from unittest import mock
-from django.db import connection, transaction
+from django.db import transaction
from django.test import TestCase, skipUnlessDBFeature
-from django.test.utils import CaptureQueriesContext
-from .models import (
- Article, InheritedArticleA, InheritedArticleB, NullablePublicationThrough,
- NullableTargetArticle, Publication,
-)
+from .models import Article, InheritedArticleA, InheritedArticleB, Publication
class ManyToManyTests(TestCase):
@@ -584,37 +580,3 @@ class ManyToManyTests(TestCase):
]
)
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'])