[Django] #35788: Order By using column number with Annotated fields

[Django]

#35788: Order By using column number with Annotated fields
-------------------------------------+-------------------------------------
Reporter: Adrian Garcia | Type:
| Cleanup/optimization
Status: new | Component: Database
| layer (models, ORM)
Version: 5.1 | Severity: Normal
Keywords: order_by, annotate, | Triage Stage:
column number, | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
As the title states, `.order_by()` is using a deprecated method of
selecting which column to order by. While most modern DBs still allow
this, the use of constants is discouraged.
{{{
>>> from django.db import models
>>> class Test(models.Model):
>>> name = models.CharField()
>>> class Meta:
>>> app_label = "test"
>>>
>>> # Use of a constant when referencing an annotation
>>> Test.objects.all().annotate(test_annotation =
models.F("name")).order_by("test_annotation").query
SELECT "test_test"."id", "test_test"."name", "test_test"."name" AS
"test_annotation" FROM "test_test" ORDER BY 3 ASC
>>>
>>> # Use of column name when not referencing annotations:
>>> Test.objects.all().annotate(test_annotation =
models.F("name")).order_by("name").query
SELECT "test_test"."id", "test_test"."name", "test_test"."name" AS
"test_annotation" FROM "test_test" ORDER BY "test_test"."name" ASC
}}}


I'd be happy to draft a PR for this if this is deemed something worth
addressing.
--
Ticket URL: <https://code.djangoproject.com/ticket/35788>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35788: Order By using column number with Annotated fields
-------------------------------------+-------------------------------------

Reporter: Adrian Garcia | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: 5.1
(models, ORM) |
Severity: Normal | Resolution: needsinfo

Keywords: order_by, annotate, | Triage Stage:
column number, | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* resolution: => needsinfo
* status: new => closed

Comment:

Hi Adrian, can you share some references as to why this shouldn't be used?
I can't find anything saying any reasons this should be a concern
--
Ticket URL: <https://code.djangoproject.com/ticket/35788#comment:1>

[Django]

#35788: Order By using column number with Annotated fields
-------------------------------------+-------------------------------------
Reporter: Adrian Garcia | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: 5.1
(models, ORM) |
Severity: Normal | Resolution: needsinfo
Keywords: order_by, annotate, | Triage Stage:
column number, | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Adrian Garcia):

The
[https://dev.mysql.com/doc/refman/8.4/en/select.html#:~:text=use%20of%20column%20positions%20is%20deprecated%20because%20the%20syntax%20has%20been%20removed%20from%20the%20sql%20standard.%20
MySQL documentation] references its removal from the standard.
Additionally, [https://www.red-gate.com/hub/product-learning/sql-prompt
/avoid-using-constants-in-an-order-by-clause this article] specifically
references that the use of constants was defined in the ANSI SQL-92
standard, and subsequently removed in ANSI SQL-99, but goes on to say that
most RDBMS vendors still support this practice.

All DBs that Django currently supports (and many that are supported via
third party libraries) appear to honor the deprecated column numbers, and
given how long ago this change was made it's likely they will continue to
support this. It's mostly the inconsistency of `order_by` using an integer
constant _only_ with annotated fields that bothers me, rather than any
risk of the feature suddenly breaking, which is why I offered to make the
change if that's acceptable.

----

Since these older versions of the SQL spec can be found online for free, I
was able to find the actual definitions.
From page pages 371 and 372 of
[https://www.contrib.andrew.cmu.edu/~shadow/sql/sql1992.txt ANSI SQL-92]:
{{{
<order by clause> ::=
ORDER BY <sort specification list>

<sort specification list> ::=
<sort specification> [ { <comma> <sort specification> }... ]

<sort specification> ::=
<sort key> [ <collate clause > ] [ <ordering specification> ]


<sort key> ::=
<column name>
| <unsigned integer>

<ordering specification> ::= ASC | DESC
...

10)If ORDER BY is specified, then each <sort specification> in the <order
by clause> shall identify a column of T.
Case:
a) If a <sort specification> contains a <column name>, then T shall
contain exactly one column with that <column name> and the <sort
specification> identifies that column.
b) If a <sort specification> contains an <unsigned integer>, then the
<unsigned integer> shall be greater than 0 and not greater than the degree
of T. The <sort specification> identifies the column of T with the ordinal
position specified by the <unsigned integer>.
}}}

From page 651 of [[https://web.cecs.pdx.edu/~len/sql1999.pdf ANSI SQL-99]]
{{{
<order by clause> ::=
ORDER BY <sort specification list>
<sort specification list> ::=
<sort specification> [ { <comma> <sort specification> }... ]
<sort specification> ::=
<sort key> [ <collate clause> ] [ <ordering specification> ]
<sort key> ::=
<value expression>
<ordering specification> ::= ASC | DESC

...

NOTE 287 – A previous version of ISO/IEC 9075 allows <sort specification>
to be a <signed in-
teger> to denote a column reference of a column of T. That facility no
longer exists. See Annex E,
‘‘Incompatibilities with ISO/IEC 9075:1992 and ISO/IEC 9075-4:1996’’.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35788#comment:2>

[Django]

#35788: Order By using column number with Annotated fields
-------------------------------------+-------------------------------------
Reporter: Adrian Garcia | Owner: (none)

Type: | Status: new

Cleanup/optimization |
Component: Database layer | Version: 5.1
(models, ORM) |
Severity: Normal | Resolution:

Keywords: order_by, annotate, | Triage Stage:
column number, | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* resolution: needsinfo =>
* status: closed => new

--
Ticket URL: <https://code.djangoproject.com/ticket/35788#comment:3>

[Django]

#35788: Order By using column number with Annotated fields
-------------------------------------+-------------------------------------
Reporter: Adrian Garcia | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 5.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: order_by, annotate, | Triage Stage:
column number, | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* cc: Simon Charette (added)

Comment:

I'm curious about whether there is any bug or breakage that using index
based `ORDER BY` or `GROUP BY` causes or if it's solely an ideological
request to adhere to the SQL spec (which the queries the ORM generates
often diverges from).

I'm asking because there are two legitimate reasons I can think of why the
ORM does that for `GROUP BY` and `ORDER BY` clauses

1. To avoid ambiguity when referencing columns with colliding aliases; see
#34346 and #34176 which introduced the changes
2. To support prepared statements and server-side parameters binding
without requiring a significant rewrite of how SQL is generated for `GROUP
BY` and `ORDER BY` clause; see #35028 and #34255 which uncovered the
problem and #20516 which discussed the addition of prepared statements.
--
Ticket URL: <https://code.djangoproject.com/ticket/35788#comment:4>

[Django]

#35788: Order By using column number with Annotated fields
-------------------------------------+-------------------------------------
Reporter: Adrian Garcia | Owner: (none)

Type: | Status: closed

Cleanup/optimization |
Component: Database layer | Version: 5.1
(models, ORM) |

Severity: Normal | Resolution: wontfix

Keywords: order_by, annotate, | Triage Stage:
column number, | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* resolution: => wontfix

* status: new => closed

Comment:

Given the input from Simon, I feel we can accept this ticket if we have
encountered a bug or breakage due to using index based `ORDER BY`.
Adrian, if you find something, feel free to comment and reopen the ticket
--
Ticket URL: <https://code.djangoproject.com/ticket/35788#comment:5>