p15693087
/
django
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Refs #30188 -- Avoided GROUP BY when aggregating over non-aggregates.

This commit is contained in:
Simon Charette 2019-03-08 18:15:44 -05:00 committed by Tim Graham
parent d1e9c25162
commit 3f32154f40
2 changed files with 16 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
django/db/models/sql/query.py
View File

@ -409,11 +409,11 @@ class Query(BaseExpression):
if not self.annotation_select: if not self.annotation_select:
return {} return {}
has_limit = self.low_mark != 0 or self.high_mark is not None has_limit = self.low_mark != 0 or self.high_mark is not None
has_existing_annotations = any( existing_annotations = [
annotation for alias, annotation annotation for alias, annotation
in self.annotations.items() in self.annotations.items()
if alias not in added_aggregate_names if alias not in added_aggregate_names
) ]
# Decide if we need to use a subquery. # Decide if we need to use a subquery.
# #
# Existing annotations would cause incorrect results as get_aggregation() # Existing annotations would cause incorrect results as get_aggregation()
@ -425,13 +425,14 @@ class Query(BaseExpression):
# those operations must be done in a subquery so that the query # those operations must be done in a subquery so that the query
# aggregates on the limit and/or distinct results instead of applying # aggregates on the limit and/or distinct results instead of applying
# the distinct and limit after the aggregation. # the distinct and limit after the aggregation.
if (isinstance(self.group_by, tuple) or has_limit or has_existing_annotations or if (isinstance(self.group_by, tuple) or has_limit or existing_annotations or
self.distinct or self.combinator): self.distinct or self.combinator):
from django.db.models.sql.subqueries import AggregateQuery from django.db.models.sql.subqueries import AggregateQuery
outer_query = AggregateQuery(self.model) outer_query = AggregateQuery(self.model)
inner_query = self.clone() inner_query = self.clone()
inner_query.select_for_update = False inner_query.select_for_update = False
inner_query.select_related = False inner_query.select_related = False
inner_query.set_annotation_mask(self.annotation_select)
if not has_limit and not self.distinct_fields: if not has_limit and not self.distinct_fields:
# Queries with distinct_fields need ordering and when a limit # Queries with distinct_fields need ordering and when a limit
# is applied we must take the slice from the ordered query. # is applied we must take the slice from the ordered query.
@ -443,7 +444,11 @@ class Query(BaseExpression):
# query is grouped by the main model's primary key. However, # query is grouped by the main model's primary key. However,
# clearing the select clause can alter results if distinct is # clearing the select clause can alter results if distinct is
# used. # used.
if inner_query.default_cols and has_existing_annotations: has_existing_aggregate_annotations = any(
annotation for annotation in existing_annotations
if getattr(annotation, 'contains_aggregate', True)
)
if inner_query.default_cols and has_existing_aggregate_annotations:
inner_query.group_by = (self.model._meta.pk.get_col(inner_query.get_initial_alias()),) inner_query.group_by = (self.model._meta.pk.get_col(inner_query.get_initial_alias()),)
inner_query.default_cols = False inner_query.default_cols = False
@ -453,10 +458,12 @@ class Query(BaseExpression):
# and move them to the outer AggregateQuery. # and move them to the outer AggregateQuery.
col_cnt = 0 col_cnt = 0
for alias, expression in list(inner_query.annotation_select.items()): for alias, expression in list(inner_query.annotation_select.items()):
annotation_select_mask = inner_query.annotation_select_mask
if expression.is_summary: if expression.is_summary:
expression, col_cnt = inner_query.rewrite_cols(expression, col_cnt) expression, col_cnt = inner_query.rewrite_cols(expression, col_cnt)
outer_query.annotations[alias] = expression.relabeled_clone(relabels) outer_query.annotations[alias] = expression.relabeled_clone(relabels)
del inner_query.annotations[alias] del inner_query.annotations[alias]
annotation_select_mask.remove(alias)
# Make sure the annotation_select wont use cached results. # Make sure the annotation_select wont use cached results.
inner_query.set_annotation_mask(inner_query.annotation_select_mask) inner_query.set_annotation_mask(inner_query.annotation_select_mask)
if inner_query.select == () and not inner_query.default_cols and not inner_query.annotation_select_mask: if inner_query.select == () and not inner_query.default_cols and not inner_query.annotation_select_mask:

7
tests/expressions/tests.py
View File

@ -551,7 +551,6 @@ class BasicExpressionsTests(TestCase):
) )
self.assertEqual(qs.get().float, 1.2) self.assertEqual(qs.get().float, 1.2)
@skipUnlessDBFeature('supports_subqueries_in_group_by')
def test_aggregate_subquery_annotation(self): def test_aggregate_subquery_annotation(self):
with self.assertNumQueries(1) as ctx: with self.assertNumQueries(1) as ctx:
aggregate = Company.objects.annotate( aggregate = Company.objects.annotate(
@ -566,7 +565,11 @@ class BasicExpressionsTests(TestCase):
self.assertEqual(aggregate, {'ceo_salary_gt_20': 1}) self.assertEqual(aggregate, {'ceo_salary_gt_20': 1})
# Aggregation over a subquery annotation doesn't annotate the subquery # Aggregation over a subquery annotation doesn't annotate the subquery
# twice in the inner query. # twice in the inner query.
self.assertLessEqual(ctx.captured_queries[0]['sql'].count('SELECT'), 4,) sql = ctx.captured_queries[0]['sql']
self.assertLessEqual(sql.count('SELECT'), 3)
# GROUP BY isn't required to aggregate over a query that doesn't
# contain nested aggregates.
self.assertNotIn('GROUP BY', sql)
def test_explicit_output_field(self): def test_explicit_output_field(self):
class FuncA(Func): class FuncA(Func):