diff options
| -rw-r--r-- | django/db/models/aggregates.py | 9 | ||||
| -rw-r--r-- | django/db/models/sql/query.py | 16 | ||||
| -rw-r--r-- | docs/releases/4.2.2.txt | 4 | ||||
| -rw-r--r-- | tests/aggregation/tests.py | 17 |
4 files changed, 37 insertions, 9 deletions
diff --git a/django/db/models/aggregates.py b/django/db/models/aggregates.py index e672f0aeb0..a778cd413b 100644 --- a/django/db/models/aggregates.py +++ b/django/db/models/aggregates.py @@ -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() diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index b615d06f28..8739a7b09c 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -387,7 +387,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( @@ -395,7 +398,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() diff --git a/docs/releases/4.2.2.txt b/docs/releases/4.2.2.txt index af2e60f2e7..d17fb7c2d3 100644 --- a/docs/releases/4.2.2.txt +++ b/docs/releases/4.2.2.txt @@ -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`). diff --git a/tests/aggregation/tests.py b/tests/aggregation/tests.py index ae68c24563..5135458668 100644 --- a/tests/aggregation/tests.py +++ b/tests/aggregation/tests.py @@ -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, + }, ) |