Skip to content

Commit

Permalink
Fixed #31880 -- Made QuerySet.aggregate() raise FieldError when aggre…
Browse files Browse the repository at this point in the history
…gating over aggregation aliases.
  • Loading branch information
David-Wobrock authored and felixxm committed Sep 29, 2020
1 parent 01a7af0 commit 058747b
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 2 deletions.
12 changes: 10 additions & 2 deletions django/db/models/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from django.db.models import AutoField, DateField, DateTimeField, sql
from django.db.models.constants import LOOKUP_SEP
from django.db.models.deletion import Collector
from django.db.models.expressions import Case, Expression, F, Value, When
from django.db.models.expressions import Case, Expression, F, Ref, Value, When
from django.db.models.functions import Cast, Trunc
from django.db.models.query_utils import FilteredRelation, Q
from django.db.models.sql.constants import CURSOR, GET_ITERATOR_CHUNK_SIZE
Expand Down Expand Up @@ -386,8 +386,16 @@ def aggregate(self, *args, **kwargs):
query = self.query.chain()
for (alias, aggregate_expr) in kwargs.items():
query.add_annotation(aggregate_expr, alias, is_summary=True)
if not query.annotations[alias].contains_aggregate:
annotation = query.annotations[alias]
if not annotation.contains_aggregate:
raise TypeError("%s is not an aggregate expression" % alias)
for expr in annotation.get_source_expressions():
if expr.contains_aggregate and isinstance(expr, Ref) and expr.refs in kwargs:
name = expr.refs
raise exceptions.FieldError(
"Cannot compute %s('%s'): '%s' is an aggregate"
% (annotation.name, name, name)
)
return query.get_aggregation(self.db, kwargs)

def count(self):
Expand Down
10 changes: 10 additions & 0 deletions tests/aggregation/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1004,6 +1004,16 @@ def test_annotate_over_annotate(self):

self.assertEqual(author.sum_age, other_author.sum_age)

def test_aggregate_over_aggregate(self):
msg = "Cannot compute Avg('age'): 'age' is an aggregate"
with self.assertRaisesMessage(FieldError, msg):
Author.objects.annotate(
age_alias=F('age'),
).aggregate(
age=Sum(F('age')),
avg_age=Avg(F('age')),
)

def test_annotated_aggregate_over_annotated_aggregate(self):
with self.assertRaisesMessage(FieldError, "Cannot compute Sum('id__max'): 'id__max' is an aggregate"):
Book.objects.annotate(Max('id')).annotate(Sum('id__max'))
Expand Down

0 comments on commit 058747b

Please sign in to comment.