diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py index afdf3402e50..8642e1f1511 100644 --- a/django/db/models/expressions.py +++ b/django/db/models/expressions.py @@ -1181,12 +1181,13 @@ class Subquery(BaseExpression, Combinable): return sql, sql_params def get_group_by_cols(self, alias=None): + # If this expression is referenced by an alias for an explicit GROUP BY + # through values() a reference to this expression and not the + # underlying .query must be returned to ensure external column + # references are not grouped against as well. if alias: return [Ref(alias, self)] - external_cols = self.get_external_cols() - if any(col.possibly_multivalued for col in external_cols): - return [self] - return external_cols + return self.query.get_group_by_cols() class Exists(Subquery): diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index faca57da56f..96f25830f00 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -1053,6 +1053,14 @@ class Query(BaseExpression): if col.alias in self.external_aliases ] + def get_group_by_cols(self, alias=None): + if alias: + return [Ref(alias, self)] + external_cols = self.get_external_cols() + if any(col.possibly_multivalued for col in external_cols): + return [self] + return external_cols + def as_sql(self, compiler, connection): # Some backends (e.g. Oracle) raise an error when a subquery contains # unnecessary ORDER BY clause. diff --git a/tests/aggregation/tests.py b/tests/aggregation/tests.py index 72575a526c5..87ae945a7eb 100644 --- a/tests/aggregation/tests.py +++ b/tests/aggregation/tests.py @@ -1280,10 +1280,17 @@ class AggregateTestCase(TestCase): ).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}) + groups = [ + Subquery(long_books_count_qs), + long_books_count_qs, + long_books_count_qs.query, + ] + for group in groups: + with self.subTest(group=group.__class__.__name__): + long_books_count_breakdown = Publisher.objects.values_list( + group, + ).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): @@ -1341,6 +1348,25 @@ class AggregateTestCase(TestCase): ).values_list('publisher_count', flat=True) self.assertSequenceEqual(books_breakdown, [1] * 6) + def test_filter_in_subquery_or_aggregation(self): + """ + Filtering against an aggregate requires the usage of the HAVING clause. + + If such a filter is unionized to a non-aggregate one the latter will + also need to be moved to the HAVING clause and have its grouping + columns used in the GROUP BY. + + When this is done with a subquery the specialized logic in charge of + using outer reference columns to group should be used instead of the + subquery itself as the latter might return multiple rows. + """ + authors = Author.objects.annotate( + Count('book'), + ).filter( + Q(book__count__gt=0) | Q(pk__in=Book.objects.values('authors')) + ) + self.assertQuerysetEqual(authors, Author.objects.all(), ordered=False) + def test_aggregation_random_ordering(self): """Random() is not included in the GROUP BY when used for ordering.""" authors = Author.objects.annotate(contact_count=Count('book')).order_by('?')