From b7b28c7c189615543218e81319473888bc46d831 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Wed, 28 Sep 2022 01:34:21 -0400 Subject: [PATCH] Refs #31150 -- Enabled implicit GROUP BY aliases. This ensures implicit grouping from aggregate function annotations groups by uncollapsed selected aliases if supported. The feature is disabled on Oracle because it doesn't support it. --- django/db/models/sql/compiler.py | 12 ++++++++---- tests/aggregation/tests.py | 12 ++++++------ 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index e9f99fa838..32b88c8960 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -123,6 +123,7 @@ class SQLCompiler: if self.query.group_by is None: return [] expressions = [] + allows_group_by_refs = self.connection.features.allows_group_by_refs if self.query.group_by is not True: # If the group by is set to a list (by .values() call most likely), # then we need to add everything in it to the GROUP BY clause. @@ -132,20 +133,21 @@ class SQLCompiler: for expr in self.query.group_by: if not hasattr(expr, "as_sql"): expr = self.query.resolve_ref(expr) - if not self.connection.features.allows_group_by_refs and isinstance( - expr, Ref - ): + if not allows_group_by_refs and isinstance(expr, Ref): expr = expr.source expressions.append(expr) # Note that even if the group_by is set, it is only the minimal # set to group by. So, we need to add cols in select, order_by, and # having into the select in any case. ref_sources = {expr.source for expr in expressions if isinstance(expr, Ref)} - for expr, _, _ in select: + aliased_exprs = {} + for expr, _, alias in select: # Skip members of the select clause that are already included # by reference. if expr in ref_sources: continue + if alias: + aliased_exprs[expr] = alias cols = expr.get_group_by_cols() for col in cols: expressions.append(col) @@ -163,6 +165,8 @@ class SQLCompiler: expressions = self.collapse_group_by(expressions, having_group_by) for expr in expressions: + if allows_group_by_refs and (alias := aliased_exprs.get(expr)): + expr = Ref(alias, expr) try: sql, params = self.compile(expr) except EmptyResultSet: diff --git a/tests/aggregation/tests.py b/tests/aggregation/tests.py index 044dcdc690..ae94af8eaa 100644 --- a/tests/aggregation/tests.py +++ b/tests/aggregation/tests.py @@ -1639,12 +1639,12 @@ class AggregateTestCase(TestCase): ) .annotate(count=Count("authors")) ) - self.assertSequenceEqual(books_qs, [book]) - # FIXME: GROUP BY doesn't need to include a subquery with - # non-multivalued JOINs, see Col.possibly_multivalued (refs #31150): - # with self.assertNumQueries(1) as ctx: - # self.assertSequenceEqual(books_qs, [book]) - # self.assertEqual(ctx[0]['sql'].count('SELECT'), 2) + with self.assertNumQueries(1) as ctx: + self.assertSequenceEqual(books_qs, [book]) + # Outerquery SELECT, annotation SELECT, and WHERE SELECT but GROUP BY + # selected alias, if allowed. + if connection.features.allows_group_by_refs: + self.assertEqual(ctx[0]["sql"].count("SELECT"), 3) @skipUnlessDBFeature("supports_subqueries_in_group_by") def test_aggregation_nested_subquery_outerref(self):