mirror of https://github.com/django/django.git
[4.2.x] 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 in04518e310d
. Backport of278881e376
from main
This commit is contained in:
parent
312d0f88b4
commit
aab25a69dd
|
@ -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,
|
||||
|
|
|
@ -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,
|
||||
)
|
||||
|
|
|
@ -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(
|
||||
|
|
Loading…
Reference in New Issue