#15116 closed (fixed)
Don't ORDER BY when using .in_bulk()
Reported by: | Chris Lamb | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.2 |
Severity: | Keywords: | ||
Cc: | 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
Specifying an ORDER BY is wasteful as the database engine will order the results (according to our model's default ordering) and then we throw that ordering away when we put them in a dictionary.
Patch attached.
Attachments (1)
Change History (5)
by , 14 years ago
Attachment: | in_bulk_ordering.diff added |
---|
comment:1 by , 14 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
comment:2 by , 14 years ago
Triage Stage: | Design decision needed → Ready for checkin |
---|
We certainly don't want to start throwing exceptions (Option 3) because there happens to be an ordering on the queryset - the semantics of in_bulk()
make it obvious that ordering becomes irrelevant, but not that ordering is illegal. The assertion is thrown for limit/offset because the method would return incorrect results otherwise, due to the adding of the filter.
Option 1 would add the overhead of a SortedDict
(which is significant), and it would probably do this in most cases, because most querysets will inherit a default ordering from the Model. If people need ordering, they have other options. So option 2 - the original patch - seems by far the best option. Tests/docs not needed, since this is a performance tweak that is virtually impossible to test, and we already have at least one test that will check with in_bulk()
still works.
It seems like there are a couple of options here: