Skip to content

Commit

Permalink
Fixed #34372 -- Fixed queryset crash on order by aggregation using Or…
Browse files Browse the repository at this point in the history
…derBy.

Regression in 278881e caused by a lack
of expression copying when an OrderBy expression is explicitly provided.

Thanks Jannis Vajen for the report and regression test.
  • Loading branch information
charettes authored Feb 27, 2023
1 parent 2276ec8 commit b15f162
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 0 deletions.
2 changes: 2 additions & 0 deletions django/db/models/sql/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -358,11 +358,13 @@ def _order_by_pairs(self):
if (
field.nulls_first is None and field.nulls_last is None
) or self.connection.features.supports_order_by_nulls_modifier:
field = field.copy()
field.expression = select_ref
# Alias collisions are not possible when dealing with
# combined queries so fallback to it if emulation of NULLS
# handling is required.
elif self.query.combinator:
field = field.copy()
field.expression = Ref(select_ref.refs, select_ref.source)
yield field, select_ref is not None
continue
Expand Down
6 changes: 6 additions & 0 deletions tests/ordering/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -638,3 +638,9 @@ def test_ordering_select_related_collision(self):
.first(),
self.a1,
)

def test_order_by_expr_query_reuse(self):
qs = Author.objects.annotate(num=Count("article")).order_by(
F("num").desc(), "pk"
)
self.assertCountEqual(qs, qs.iterator())

0 comments on commit b15f162

Please sign in to comment.