Opened 7 years ago
Last modified 4 years ago
#28560 new Bug
distinct() on ordered queryset with restricted list of columns returns incorrect result
Reported by: | Mariusz Felisiak | Owned by: | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | distinct values |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
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()).
Change History (14)
comment:2 by , 7 years ago
Summary: | distinct() on None values → distinct() on ordered queryset with restricted list of columns returns incorrect result |
---|
comment:3 by , 7 years ago
Description: | modified (diff) |
---|
comment:4 by , 7 years ago
comment:5 by , 7 years ago
Good idea! it should be feasible. I will try to prepare patch in this week.
comment:6 by , 7 years ago
By the way Mariusz, do you have an opinion on https://code.djangoproject.com/ticket/14357#comment:11?
comment:7 by , 7 years ago
Description: | modified (diff) |
---|
comment:8 by , 7 years ago
Description: | modified (diff) |
---|
comment:10 by , 7 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:11 by , 7 years ago
Patch needs improvement: | set |
---|
comment:12 by , 7 years ago
Has patch: | unset |
---|---|
Owner: | removed |
Patch needs improvement: | unset |
Status: | assigned → new |
comment:13 by , 4 years ago
comment:14 by , 4 years ago
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.
comment:15 by , 4 years ago
Description: | modified (diff) |
---|
Couldn't we wrap the query in a subquery like we did in #24254 in this case?