Opened 9 years ago
Closed 9 years ago
#25081 closed Bug (fixed)
QuerySet.get() ignores order_by in DISTINCT ON queries
Reported by: | Peter De Wachter | Owned by: | Simon Charette |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Simon Charette | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The following query, using the PostgreSQL-specific DISTINCT ON, is mistranslated:
Model.objects.order_by('key', '-last_updated').distinct('key').get(key=x)
This generated this SQL query:
SELECT DISTINCT ON ("bug_mymodel"."key") "bug_mymodel"."id", "bug_mymodel"."key", "bug_mymodel"."last_updated", "bug_mymodel"."text" FROM "bug_mymodel" WHERE "bug_mymodel"."key" = 'xyz'
The ORDER BY request is omitted from the query, but DISTINCT ON is meaningless without sort order. Note that it is correct to use get() in this case, as it is guaranteed that there is only one result.
We expected the following SQL:
SELECT DISTINCT ON ("bug_mymodel"."key") "bug_mymodel"."id", "bug_mymodel"."key", "bug_mymodel"."last_updated", "bug_mymodel"."text" FROM "bug_mymodel" WHERE "bug_mymodel"."key" = 'xyz' ORDER BY "bug_mymodel"."key" ASC, "bug_mymodel"."last_updated" DESC
Full test case:
class MyModel(models.Model): key = models.CharField(max_length=100) last_updated = models.DateTimeField(auto_now=True) text = models.CharField(max_length=100) def test(): return MyModel.objects.order_by('key', '-last_updated').distinct('key').get(key='xyz')
Attachments (1)
Change History (7)
comment:1 by , 9 years ago
Cc: | added |
---|---|
Triage Stage: | Unreviewed → Accepted |
Version: | 1.8 → master |
comment:2 by , 9 years ago
It looks like latest
and earliest
rely on this behavior.
@pdewacht, your proposed patch makes sense. Please submit a PR with tests.
comment:3 by , 9 years ago
Owner: | changed from | to
---|---|
Patch needs improvement: | set |
Status: | new → assigned |
WIll turn this into a PR.
comment:5 by , 9 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Managed to reproduce against master. The culprit seems to be an ordering clearing when limits have been set.
I'm actually running the test suite with those lines removed since I can't think of a reason this is required.
e.g. Why do ordering should be cleared in this case
But not this one