Fixed #34346 -- Ordered selected expressions by position.

Used the same approach as for #34176 by using selected expressions
position to prevent ambiguous aliases in collisions.

Thanks henribru for the report.

Regression in 04518e310d.
This commit is contained in:
Simon Charette 2023-02-17 20:38:08 -05:00 committed by Mariusz Felisiak
parent f91e085c30
commit 278881e376
3 changed files with 55 additions and 10 deletions

View File

@ -27,6 +27,15 @@ from django.utils.hashable import make_hashable
from django.utils.regex_helper import _lazy_re_compile
class PositionRef(Ref):
def __init__(self, ordinal, refs, source):
self.ordinal = ordinal
super().__init__(refs, source)
def as_sql(self, compiler, connection):
return str(self.ordinal), ()
class SQLCompiler:
# Multiline ordering SQL clause may appear from RawSQL.
ordering_parts = _lazy_re_compile(
@ -321,6 +330,14 @@ class SQLCompiler:
else:
default_order, _ = ORDER_DIR["DESC"]
selected_exprs = {}
if select := self.select:
for ordinal, (expr, _, alias) in enumerate(select, start=1):
pos_expr = PositionRef(ordinal, alias, expr)
if alias:
selected_exprs[alias] = pos_expr
selected_exprs[expr] = pos_expr
for field in ordering:
if hasattr(field, "resolve_expression"):
if isinstance(field, Value):
@ -331,13 +348,23 @@ class SQLCompiler:
if not self.query.standard_ordering:
field = field.copy()
field.reverse_ordering()
if isinstance(field.expression, F) and (
annotation := self.query.annotation_select.get(
field.expression.name
)
select_ref = selected_exprs.get(field.expression)
if select_ref or (
isinstance(field.expression, F)
and (select_ref := selected_exprs.get(field.expression.name))
):
field.expression = Ref(field.expression.name, annotation)
yield field, isinstance(field.expression, Ref)
# Emulation of NULLS (FIRST|LAST) cannot be combined with
# the usage of ordering by position.
if (
field.nulls_first is None and field.nulls_last is None
) or self.connection.features.supports_order_by_nulls_modifier:
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.expression = Ref(select_ref.refs, select_ref.source)
yield field, select_ref is not None
continue
if field == "?": # random
yield OrderBy(Random()), False
@ -346,11 +373,11 @@ class SQLCompiler:
col, order = get_order_dir(field, default_order)
descending = order == "DESC"
if col in self.query.annotation_select:
if select_ref := selected_exprs.get(col):
# Reference to expression in SELECT clause
yield (
OrderBy(
Ref(col, self.query.annotation_select[col]),
select_ref,
descending=descending,
),
True,

View File

@ -8,6 +8,7 @@ from django.db.models import (
DateTimeField,
F,
Max,
OrderBy,
OuterRef,
Subquery,
Value,
@ -619,3 +620,21 @@ class OrderingTests(TestCase):
),
Author.objects.order_by(Length(Upper("name"))),
)
def test_ordering_select_related_collision(self):
self.assertEqual(
Article.objects.select_related("author")
.annotate(name=Upper("author__name"))
.filter(pk=self.a1.pk)
.order_by(OrderBy(F("name")))
.first(),
self.a1,
)
self.assertEqual(
Article.objects.select_related("author")
.annotate(name=Upper("author__name"))
.filter(pk=self.a1.pk)
.order_by("name")
.first(),
self.a1,
)

View File

@ -465,10 +465,9 @@ class TestQuerying(PostgreSQLTestCase):
{"field__0": 20, "arrayagg": [self.objs[3].pk]},
],
)
alias = connection.ops.quote_name("field__0")
sql = ctx[0]["sql"]
self.assertIn("GROUP BY 1", sql)
self.assertIn(f"ORDER BY {alias}", sql)
self.assertIn("ORDER BY 1", sql)
def test_index(self):
self.assertSequenceEqual(