Skip to content

Commit

Permalink
[4.2.x] Fixed #34750 -- Fixed QuerySet.count() when grouping by unuse…
Browse files Browse the repository at this point in the history
…d multi-valued annotations.

Thanks Toan Vuong for the report.
Thanks Simon Charette for the review.

Regression in 59bea9e.
Backport of c9b9a52 from main
  • Loading branch information
felixxm committed Aug 1, 2023
1 parent 2ef2b2f commit 8808d9d
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 0 deletions.
5 changes: 5 additions & 0 deletions django/db/models/sql/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,11 @@ def get_aggregation(self, using, aggregate_exprs):
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
Expand Down
4 changes: 4 additions & 0 deletions docs/releases/4.2.4.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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`).
41 changes: 41 additions & 0 deletions tests/aggregation/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -2165,6 +2165,47 @@ def test_unused_aliased_aggregate_pruned(self):
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(
Expand Down

0 comments on commit 8808d9d

Please sign in to comment.