Skip to content

Commit

Permalink
Refs #34551 -- Fixed QuerySet.aggregate() crash on precending aggrega…
Browse files Browse the repository at this point in the history
…tion reference.

Regression in 1297c0d.

Refs #31679.
  • Loading branch information
charettes authored and felixxm committed May 23, 2023
1 parent 89f10a8 commit 2ee0174
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 9 deletions.
9 changes: 8 additions & 1 deletion django/db/models/aggregates.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,14 @@ def resolve_expression(
c.filter = c.filter and c.filter.resolve_expression(
query, allow_joins, reuse, summarize
)
if not summarize:
if summarize:
# Summarized aggregates cannot refer to summarized aggregates.
for ref in c.get_refs():
if query.annotations[ref].is_summary:
raise FieldError(
f"Cannot compute {c.name}('{ref}'): '{ref}' is an aggregate"
)
elif not self.is_summary:
# Call Aggregate.get_source_expressions() to avoid
# returning self.filter and including that in this loop.
expressions = super(Aggregate, c).get_source_expressions()
Expand Down
16 changes: 14 additions & 2 deletions django/db/models/sql/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -400,15 +400,27 @@ def get_aggregation(self, using, aggregate_exprs):
"""
if not aggregate_exprs:
return {}
aggregates = {}
# Store annotation mask prior to temporarily adding aggregations for
# resolving purpose to facilitate their subsequent removal.
replacements = {}
annotation_select_mask = self.annotation_select_mask
for alias, aggregate_expr in aggregate_exprs.items():
self.check_alias(alias)
aggregate = aggregate_expr.resolve_expression(
self, allow_joins=True, reuse=None, summarize=True
)
if not aggregate.contains_aggregate:
raise TypeError("%s is not an aggregate expression" % alias)
aggregates[alias] = aggregate
# Temporarily add aggregate to annotations to allow remaining
# members of `aggregates` to resolve against each others.
self.append_annotation_mask([alias])
aggregate = aggregate.replace_expressions(replacements)
self.annotations[alias] = aggregate
replacements[Ref(alias, aggregate)] = aggregate
# Stash resolved aggregates now that they have been allowed to resolve
# against each other.
aggregates = {alias: self.annotations.pop(alias) for alias in aggregate_exprs}
self.set_annotation_mask(annotation_select_mask)
# Existing usage of aggregation can be determined by the presence of
# selected aggregates but also by filters against aliased aggregates.
_, having, qualify = self.where.split_having_qualify()
Expand Down
4 changes: 4 additions & 0 deletions docs/releases/4.2.2.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,7 @@ Bugfixes

* Fixed a regression in Django 4.2 where nonexistent stylesheet was linked on a
“Congratulations!” page (:ticket:`34588`).

* Fixed a regression in Django 4.2 that caused a crash of
``QuerySet.aggregate()`` with expressions referencing other aggregates
(:ticket:`34551`).
17 changes: 11 additions & 6 deletions tests/aggregation/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1260,7 +1260,7 @@ def test_annotate_over_annotate(self):
self.assertEqual(author.sum_age, other_author.sum_age)

def test_aggregate_over_aggregate(self):
msg = "Cannot resolve keyword 'age_agg' into field."
msg = "Cannot compute Avg('age_agg'): 'age_agg' is an aggregate"
with self.assertRaisesMessage(FieldError, msg):
Author.objects.aggregate(
age_agg=Sum(F("age")),
Expand Down Expand Up @@ -2102,12 +2102,17 @@ def test_exists_extra_where_with_aggregate(self):
)
self.assertEqual(len(qs), 6)

def test_aggregation_over_annotation_shared_alias(self):
def test_multiple_aggregate_references(self):
aggregates = Author.objects.aggregate(
total_books=Count("book"),
coalesced_total_books=Coalesce("total_books", 0),
)
self.assertEqual(
Publisher.objects.annotate(agg=Count("book__authors")).aggregate(
agg=Count("agg"),
),
{"agg": 5},
aggregates,
{
"total_books": 10,
"coalesced_total_books": 10,
},
)


Expand Down

0 comments on commit 2ee0174

Please sign in to comment.