From d1e9c25162eecb24dee50fcfe834a2735e53fb0e Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Fri, 8 Mar 2019 15:54:03 -0500 Subject: [PATCH] Refs #30188 -- Prevented double annotation of subquery when aggregated over. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thanks Can Sarıgöl for the suggested trimming approach. --- django/db/models/sql/query.py | 16 ++++++++++------ tests/expressions/tests.py | 22 +++++++++++++--------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 74aed551a3..d421efa7c3 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -383,12 +383,16 @@ class Query(BaseExpression): new_expr, col_cnt = self.rewrite_cols(expr, col_cnt) new_exprs.append(new_expr) elif isinstance(expr, (Col, Subquery)) or (expr.contains_aggregate and not expr.is_summary): - # Reference to column. Make sure the referenced column - # is selected. - col_cnt += 1 - col_alias = '__col%d' % col_cnt - self.annotations[col_alias] = expr - self.append_annotation_mask([col_alias]) + # Reference to column, subquery, or another aggregate. Make + # sure the expression is selected and reuse its alias if so. + for col_alias, selected_annotation in self.annotation_select.items(): + if selected_annotation == expr: + break + else: + col_cnt += 1 + col_alias = '__col%d' % col_cnt + self.annotations[col_alias] = expr + self.append_annotation_mask([col_alias]) new_exprs.append(Ref(col_alias, expr)) else: # Some other expression not referencing database values diff --git a/tests/expressions/tests.py b/tests/expressions/tests.py index bb448745c8..a51080ef13 100644 --- a/tests/expressions/tests.py +++ b/tests/expressions/tests.py @@ -553,16 +553,20 @@ class BasicExpressionsTests(TestCase): @skipUnlessDBFeature('supports_subqueries_in_group_by') def test_aggregate_subquery_annotation(self): - aggregate = Company.objects.annotate( - ceo_salary=Subquery( - Employee.objects.filter( - id=OuterRef('ceo_id'), - ).values('salary') - ), - ).aggregate( - ceo_salary_gt_20=Count('pk', filter=Q(ceo_salary__gt=20)), - ) + with self.assertNumQueries(1) as ctx: + aggregate = Company.objects.annotate( + ceo_salary=Subquery( + Employee.objects.filter( + id=OuterRef('ceo_id'), + ).values('salary') + ), + ).aggregate( + ceo_salary_gt_20=Count('pk', filter=Q(ceo_salary__gt=20)), + ) self.assertEqual(aggregate, {'ceo_salary_gt_20': 1}) + # Aggregation over a subquery annotation doesn't annotate the subquery + # twice in the inner query. + self.assertLessEqual(ctx.captured_queries[0]['sql'].count('SELECT'), 4,) def test_explicit_output_field(self): class FuncA(Func):