diff options
| author | Simon Charette <charette.s@gmail.com> | 2023-05-21 23:57:49 -0400 |
|---|---|---|
| committer | Mariusz Felisiak <felisiak.mariusz@gmail.com> | 2023-05-23 06:25:58 +0200 |
| commit | e5c844d6f2a4ac6ae674d741b5f1fa2a688cedf4 (patch) | |
| tree | b7f47b9b83726965bd1d97284555e701cb8a48ba | |
| parent | 2ee01747c32a7275a7a1a5f7862acba7db764921 (diff) | |
Fixed #34551 -- Fixed QuerySet.aggregate() crash when referencing subqueries.
Regression in 59bea9efd2768102fc9d3aedda469502c218e9b7.
Refs #28477.
Thanks Denis Roldán and Mariusz for the test.
| -rw-r--r-- | django/db/models/expressions.py | 1 | ||||
| -rw-r--r-- | django/db/models/sql/query.py | 6 | ||||
| -rw-r--r-- | docs/releases/4.2.2.txt | 4 | ||||
| -rw-r--r-- | tests/aggregation/tests.py | 20 |
4 files changed, 31 insertions, 0 deletions
diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py index e1861759c4..280cb967b4 100644 --- a/django/db/models/expressions.py +++ b/django/db/models/expressions.py @@ -1541,6 +1541,7 @@ class Subquery(BaseExpression, Combinable): template = "(%(subquery)s)" contains_aggregate = False empty_result_set_value = None + subquery = True def __init__(self, queryset, output_field=None, **extra): # Allow the usage of both QuerySet and sql.Query objects. diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index c4b435df2d..781dfd5499 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -402,6 +402,7 @@ class Query(BaseExpression): return {} # Store annotation mask prior to temporarily adding aggregations for # resolving purpose to facilitate their subsequent removal. + refs_subquery = False replacements = {} annotation_select_mask = self.annotation_select_mask for alias, aggregate_expr in aggregate_exprs.items(): @@ -414,6 +415,10 @@ class Query(BaseExpression): # Temporarily add aggregate to annotations to allow remaining # members of `aggregates` to resolve against each others. self.append_annotation_mask([alias]) + refs_subquery |= any( + getattr(self.annotations[ref], "subquery", False) + for ref in aggregate.get_refs() + ) aggregate = aggregate.replace_expressions(replacements) self.annotations[alias] = aggregate replacements[Ref(alias, aggregate)] = aggregate @@ -445,6 +450,7 @@ class Query(BaseExpression): isinstance(self.group_by, tuple) or self.is_sliced or has_existing_aggregation + or refs_subquery or qualify or self.distinct or self.combinator diff --git a/docs/releases/4.2.2.txt b/docs/releases/4.2.2.txt index d17fb7c2d3..df3f2a2710 100644 --- a/docs/releases/4.2.2.txt +++ b/docs/releases/4.2.2.txt @@ -32,3 +32,7 @@ Bugfixes * Fixed a regression in Django 4.2 that caused a crash of ``QuerySet.aggregate()`` with expressions referencing other aggregates (:ticket:`34551`). + +* Fixed a regression in Django 4.2 that caused a crash of + ``QuerySet.aggregate()`` with aggregates referencing subqueries + (:ticket:`34551`). diff --git a/tests/aggregation/tests.py b/tests/aggregation/tests.py index 5135458668..366b8434e5 100644 --- a/tests/aggregation/tests.py +++ b/tests/aggregation/tests.py @@ -2187,3 +2187,23 @@ class AggregateAnnotationPruningTests(TestCase): mod_count=Count("*") ) self.assertEqual(queryset.count(), 1) + + def test_referenced_subquery_requires_wrapping(self): + total_books_qs = ( + Author.book_set.through.objects.values("author") + .filter(author=OuterRef("pk")) + .annotate(total=Count("book")) + ) + with self.assertNumQueries(1) as ctx: + aggregate = ( + Author.objects.annotate( + total_books=Subquery(total_books_qs.values("total")) + ) + .values("pk", "total_books") + .aggregate( + sum_total_books=Sum("total_books"), + ) + ) + sql = ctx.captured_queries[0]["sql"].lower() + self.assertEqual(sql.count("select"), 3, "Subquery wrapping required") + self.assertEqual(aggregate, {"sum_total_books": 3}) |
