From 68912e4f6f84f21322f92a2c7b6c77f68f91b9c9 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Mon, 17 Jul 2023 12:51:54 -0400 Subject: [PATCH] Fixed #34717 -- Fixed QuerySet.aggregate() crash when referencing window functions. Regression in 59bea9efd2768102fc9d3aedda469502c218e9b7. Refs #28477. Thanks younes-chaoui for the report. --- django/db/models/sql/query.py | 6 ++++++ docs/releases/4.2.4.txt | 4 +++- tests/aggregation/tests.py | 21 +++++++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 13a6809dd83..1608d194807 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -403,6 +403,7 @@ class Query(BaseExpression): # Store annotation mask prior to temporarily adding aggregations for # resolving purpose to facilitate their subsequent removal. refs_subquery = False + refs_window = False replacements = {} annotation_select_mask = self.annotation_select_mask for alias, aggregate_expr in aggregate_exprs.items(): @@ -419,6 +420,10 @@ class Query(BaseExpression): getattr(self.annotations[ref], "subquery", False) for ref in aggregate.get_refs() ) + refs_window |= any( + getattr(self.annotations[ref], "contains_over_clause", True) + for ref in aggregate.get_refs() + ) aggregate = aggregate.replace_expressions(replacements) self.annotations[alias] = aggregate replacements[Ref(alias, aggregate)] = aggregate @@ -451,6 +456,7 @@ class Query(BaseExpression): or self.is_sliced or has_existing_aggregation or refs_subquery + or refs_window or qualify or self.distinct or self.combinator diff --git a/docs/releases/4.2.4.txt b/docs/releases/4.2.4.txt index e8fd2255163..3921dd9b3e0 100644 --- a/docs/releases/4.2.4.txt +++ b/docs/releases/4.2.4.txt @@ -9,4 +9,6 @@ Django 4.2.4 fixes several bugs in 4.2.3. Bugfixes ======== -* ... +* Fixed a regression in Django 4.2 that caused a crash of + ``QuerySet.aggregate()`` with aggregates referencing window functions + (:ticket:`34717`). diff --git a/tests/aggregation/tests.py b/tests/aggregation/tests.py index 366b8434e54..db69246952f 100644 --- a/tests/aggregation/tests.py +++ b/tests/aggregation/tests.py @@ -28,6 +28,7 @@ from django.db.models import ( Value, Variance, When, + Window, ) from django.db.models.expressions import Func, RawSQL from django.db.models.functions import ( @@ -2207,3 +2208,23 @@ class AggregateAnnotationPruningTests(TestCase): sql = ctx.captured_queries[0]["sql"].lower() self.assertEqual(sql.count("select"), 3, "Subquery wrapping required") self.assertEqual(aggregate, {"sum_total_books": 3}) + + @skipUnlessDBFeature("supports_over_clause") + def test_referenced_window_requires_wrapping(self): + total_books_qs = Book.objects.annotate( + avg_publisher_pages=Coalesce( + Window(Avg("pages"), partition_by=F("publisher")), + 0.0, + ) + ) + with self.assertNumQueries(1) as ctx: + aggregate = total_books_qs.aggregate( + sum_avg_publisher_pages=Sum("avg_publisher_pages"), + books_count=Count("id"), + ) + sql = ctx.captured_queries[0]["sql"].lower() + self.assertEqual(sql.count("select"), 2, "Subquery wrapping required") + self.assertEqual( + aggregate, + {"sum_avg_publisher_pages": 1100.0, "books_count": 2}, + )