Ordering by F-expression resolving to a number returns wrong results

[Florian Apolloner]

Article.objects.annotate(test=Value(42)).order_by(F("test").asc())

yields

Traceback (most recent call last):
  File "/home/florian/sources/django.git/django/db/backends/utils.py", line 89, in _execute
    return self.cursor.execute(sql, params)
psycopg2.errors.InvalidColumnReference: ORDER BY position 42 is not in select list
LINE 1: ...e", 42 AS "test" FROM "ordering_article" ORDER BY 42 ASC LIM...
                                                             ^


The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/florian/sources/django.git/tests/ordering/tests.py", line 485, in test_order_by_f_expression_alias
    print(qs)
  File "/home/florian/sources/django.git/django/db/models/query.py", line 370, in __repr__
    data = list(self[: REPR_OUTPUT_SIZE + 1])
  File "/home/florian/sources/django.git/django/db/models/query.py", line 376, in __len__
    self._fetch_all()
  File "/home/florian/sources/django.git/django/db/models/query.py", line 1841, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "/home/florian/sources/django.git/django/db/models/query.py", line 87, in __iter__
    results = compiler.execute_sql(
  File "/home/florian/sources/django.git/django/db/models/sql/compiler.py", line 1401, in execute_sql
    cursor.execute(sql, params)
  File "/home/florian/sources/django.git/django/db/backends/utils.py", line 103, in execute
    return super().execute(sql, params)
  File "/home/florian/sources/django.git/django/db/backends/utils.py", line 67, in execute
    return self._execute_with_wrappers(
  File "/home/florian/sources/django.git/django/db/backends/utils.py", line 80, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/home/florian/sources/django.git/django/db/backends/utils.py", line 84, in _execute
    with self.db.wrap_database_errors:
  File "/home/florian/sources/django.git/django/db/utils.py", line 91, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/home/florian/sources/django.git/django/db/backends/utils.py", line 89, in _execute
    return self.cursor.execute(sql, params)
django.db.utils.ProgrammingError: ORDER BY position 42 is not in select list
LINE 1: ...e", 42 AS "test" FROM "ordering_article" ORDER BY 42 ASC LIM...

the issue here is that the F expression is not resolved to a Ref like it should be

[Simon Charette]

Could we possibly error out at resolving time instead of adding logic to the compiler given ordering by a constant value is a noop? Given it's a noop we could also simply drop the ordering without raising an error.

[Florian Apolloner]

Could we possibly error out at resolving time instead of adding logic to the compiler given ordering by a constant value is a noop?

Probably.

Given it's a noop we could also simply drop the ordering without raising an error.

I'd be rather strongly against that. Given that it is a noop, it is certainly not what the user intended to write, so maybe an error is the best idea. The next question that comes to mind here is: Do we want to support ordering like this User.objects.order_by(Value(1).desc()) -- this currently works and the intent seems rather clear to me (ie sort by the first column). It at least somewhat works, but starts to break down when you add nulls_first=True there because all of a sudden we are generating ORDER BY 1 IS NULL, 1 DESC on MySQL which is wrong again (See also #33768).

We probably should merge those two tickets together and consider them as one, what do you think?

[Simon Charette]

yeah I think we should merge the two tickets together.

Ultimately the issue covered by ​the submitted PR is to make sure that we properly order by selected references when possible?

[Florian Apolloner]

Replying to Simon Charette:

yeah I think we should merge the two tickets together.

Ok, I am closing this one since the other includes more context (#33768)

Ultimately the issue covered by ​the submitted PR is to make sure that we properly order by selected references when possible?

Yes; while trying to fix it I looked for other problems we might have in that area which led to the current findings.