summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSimon Charette <charette.s@gmail.com>2019-02-27 00:48:41 -0500
committerTim Graham <timograham@gmail.com>2019-03-21 18:48:41 -0400
commitfb3f034f1c63160c0ff13c609acd01c18be12f80 (patch)
treeea5948f50d2494abedf739a71f62246bb5f9d4ec
parent9dc367dc10594ad024c83d398a8e3c3f8f221446 (diff)
Fixed #30158 -- Avoided unnecessary subquery group by on aggregation.
Subquery annotations can be omitted from the GROUP BY clause on aggregation as long as they are not explicitly grouped against. Thanks Jonny Fuller for the report.
-rw-r--r--django/db/models/expressions.py5
-rw-r--r--tests/aggregation/tests.py48
2 files changed, 52 insertions, 1 deletions
diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py
index 30c2f465f0..cb50f73f1d 100644
--- a/django/db/models/expressions.py
+++ b/django/db/models/expressions.py
@@ -1082,6 +1082,11 @@ class Subquery(Expression):
return clone
return self
+ def get_group_by_cols(self, alias=None):
+ if alias:
+ return [Ref(alias, self)]
+ return []
+
class Exists(Subquery):
template = 'EXISTS(%(subquery)s)'
diff --git a/tests/aggregation/tests.py b/tests/aggregation/tests.py
index 3820496c9f..a336758f45 100644
--- a/tests/aggregation/tests.py
+++ b/tests/aggregation/tests.py
@@ -8,8 +8,9 @@ from django.db.models import (
Avg, Count, DecimalField, DurationField, F, FloatField, Func, IntegerField,
Max, Min, Sum, Value,
)
-from django.db.models.expressions import Case, When
+from django.db.models.expressions import Case, Exists, OuterRef, Subquery, When
from django.test import TestCase
+from django.test.testcases import skipUnlessDBFeature
from django.test.utils import Approximate, CaptureQueriesContext
from django.utils import timezone
@@ -1114,3 +1115,48 @@ class AggregateTestCase(TestCase):
Book.objects.aggregate(is_book=True)
with self.assertRaisesMessage(TypeError, msg % ', '.join([str(FloatField()), 'True'])):
Book.objects.aggregate(FloatField(), Avg('price'), is_book=True)
+
+ def test_aggregation_subquery_annotation(self):
+ """Subquery annotations are excluded from the GROUP BY if they are
+ not explicitly grouped against."""
+ latest_book_pubdate_qs = Book.objects.filter(
+ publisher=OuterRef('pk')
+ ).order_by('-pubdate').values('pubdate')[:1]
+ publisher_qs = Publisher.objects.annotate(
+ latest_book_pubdate=Subquery(latest_book_pubdate_qs),
+ ).annotate(count=Count('book'))
+ with self.assertNumQueries(1) as ctx:
+ list(publisher_qs)
+ self.assertEqual(ctx[0]['sql'].count('SELECT'), 2)
+
+ @skipUnlessDBFeature('supports_subqueries_in_group_by')
+ def test_group_by_subquery_annotation(self):
+ """
+ Subquery annotations are included in the GROUP BY if they are
+ grouped against.
+ """
+ long_books_count_qs = Book.objects.filter(
+ publisher=OuterRef('pk'),
+ pages__gt=400,
+ ).values(
+ 'publisher'
+ ).annotate(count=Count('pk')).values('count')
+ long_books_count_breakdown = Publisher.objects.values_list(
+ Subquery(long_books_count_qs, IntegerField()),
+ ).annotate(total=Count('*'))
+ self.assertEqual(dict(long_books_count_breakdown), {None: 1, 1: 4})
+
+ @skipUnlessDBFeature('supports_subqueries_in_group_by')
+ def test_group_by_exists_annotation(self):
+ """
+ Exists annotations are included in the GROUP BY if they are
+ grouped against.
+ """
+ long_books_qs = Book.objects.filter(
+ publisher=OuterRef('pk'),
+ pages__gt=800,
+ )
+ has_long_books_breakdown = Publisher.objects.values_list(
+ Exists(long_books_qs),
+ ).annotate(total=Count('*'))
+ self.assertEqual(dict(has_long_books_breakdown), {True: 2, False: 3})