From c9b9a52edc66be117c6e5b5214fa788a4d5db7a8 Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Tue, 1 Aug 2023 16:16:28 +0200 Subject: [PATCH] Fixed #34750 -- Fixed QuerySet.count() when grouping by unused multi-valued annotations. Thanks Toan Vuong for the report. Thanks Simon Charette for the review. Regression in 59bea9efd2768102fc9d3aedda469502c218e9b7. --- django/db/models/sql/query.py | 5 +++++ docs/releases/4.2.4.txt | 4 ++++ tests/aggregation/tests.py | 41 +++++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+) diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index ac735012f11..220ae92754d 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -498,6 +498,11 @@ class Query(BaseExpression): annotation_mask |= expr.get_refs() for aggregate in aggregates.values(): annotation_mask |= aggregate.get_refs() + # Avoid eliding expressions that might have an incidence on + # the implicit grouping logic. + for annotation_alias, annotation in self.annotation_select.items(): + if annotation.get_group_by_cols(): + annotation_mask.add(annotation_alias) inner_query.set_annotation_mask(annotation_mask) # Add aggregates to the outer AggregateQuery. This requires making diff --git a/docs/releases/4.2.4.txt b/docs/releases/4.2.4.txt index a39818e3b87..9e3c2fc1fd7 100644 --- a/docs/releases/4.2.4.txt +++ b/docs/releases/4.2.4.txt @@ -15,3 +15,7 @@ Bugfixes * Fixed a regression in Django 4.2 that caused a crash when grouping by a reference in a subquery (:ticket:`34748`). + +* Fixed a regression in Django 4.2 that caused aggregation over query that + uses explicit grouping by multi-valued annotations to group against the wrong + columns (:ticket:`34750`). diff --git a/tests/aggregation/tests.py b/tests/aggregation/tests.py index 9f2a7c88418..ad00afdcc12 100644 --- a/tests/aggregation/tests.py +++ b/tests/aggregation/tests.py @@ -2165,6 +2165,47 @@ class AggregateAnnotationPruningTests(TestCase): self.assertEqual(sql.count("select"), 2, "Subquery wrapping required") self.assertNotIn("authors_count", sql) + def test_unused_aliased_aggregate_and_annotation_reverse_fk(self): + Book.objects.create( + name="b3", + publisher=self.p2, + pages=1000, + rating=4.2, + price=50, + contact=self.a2, + pubdate=datetime.date.today(), + ) + qs = Publisher.objects.annotate( + total_pages=Sum("book__pages"), + good_book=Case( + When(book__rating__gt=4.0, then=Value(True)), + default=Value(False), + ), + ) + self.assertEqual(qs.count(), 3) + + def test_unused_aliased_aggregate_and_annotation_reverse_fk_grouped(self): + Book.objects.create( + name="b3", + publisher=self.p2, + pages=1000, + rating=4.2, + price=50, + contact=self.a2, + pubdate=datetime.date.today(), + ) + qs = ( + Publisher.objects.values("id", "name") + .annotate(total_pages=Sum("book__pages")) + .annotate( + good_book=Case( + When(book__rating__gt=4.0, then=Value(True)), + default=Value(False), + ) + ) + ) + self.assertEqual(qs.count(), 3) + def test_non_aggregate_annotation_pruned(self): with CaptureQueriesContext(connection) as ctx: Book.objects.annotate(