1
0
mirror of https://github.com/django/django.git synced 2024-12-22 17:16:24 +00:00

Refs #34551 -- Fixed QuerySet.aggregate() crash on precending aggregation reference.

Regression in 1297c0d0d7.

Refs #31679.
This commit is contained in:
Simon Charette 2023-05-21 23:49:05 -04:00 committed by Mariusz Felisiak
parent 89f10a80d7
commit 2ee01747c3
4 changed files with 37 additions and 9 deletions

View File

@ -65,7 +65,14 @@ class Aggregate(Func):
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()

View File

@ -400,7 +400,10 @@ class Query(BaseExpression):
"""
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(
@ -408,7 +411,16 @@ class Query(BaseExpression):
)
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()

View File

@ -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`).

View File

@ -1260,7 +1260,7 @@ class AggregateTestCase(TestCase):
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")),
@ -2102,12 +2102,17 @@ class AggregateTestCase(TestCase):
)
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,
},
)