distinct() on ordered queryset with restricted list of columns returns incorrect result

[Mariusz Felisiak]

When distinct() is used with values() (or values_list()) on ordered queryset and a list of fields in values() doesn't contain all fields from ORDER BY, then it doesn't return correct result because columns from ORDER BY clause must be included in SELECT.

After some discussion on the mailing list (​https://groups.google.com/g/django-developers/c/DNVRFqVBsfk/m/xDUvaq3DAAAJ) it looks like the consensus is to require an explicit opt-in or raise an error (i.e., never add a column implicitly if the user specified a list of columns via values() or values_list()).

[Simon Charette]

Couldn't we wrap the query in a subquery like we did in #24254 in this case?

[Mariusz Felisiak]

Good idea! it should be feasible. I will try to prepare patch in this week.

[Simon Charette]

By the way Mariusz, do you have an opinion on https://code.djangoproject.com/ticket/14357#comment:11?

[Mariusz Felisiak]

​PR

[Tobias McNulty]

Should/could we force the user to include in values() any ordering columns required in the SELECT per #7070?

This seems most explicit for the user and consistent with the Postgres error noted in #5321.

[Tobias McNulty]

After some discussion on the mailing list (​https://groups.google.com/g/django-developers/c/DNVRFqVBsfk/m/xDUvaq3DAAAJ) it looks like the consensus is to require an explicit opt-in or raise an error (i.e., never add a column implicitly if the user specified a list of columns via values() or values_list()).

I am not too familiar with the ORM. I believe these columns are added in get_extra_select() (django/db/models/sql/compiler.py), which could be adapted initially to raise a deprecation warning and eventually an exception. Does that sound right?

I may not have time right away so if anyone feels like picking this up, go for it.