Opened 14 years ago

Closed 14 years ago

#12258 closed (fixed)

QuerySet 'get' method should clear ordering before executing query

Reported by: mattdennewitz@… Owned by: nobody
Component: Database layer (models, ORM) Version: 1.1
Severity: Keywords: ordering, database, queryset, mq, mq-200912-dallas
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

The SQL generated by the ORM when calling get includes ordering parameters specified in Meta.ordering. This might be small in the greater scheme of things, but clearing the ordering before evaluating the cloned QuerySet would prevent unnecessary filesorts by the database.

Attachments (3)

order_by.patch (472 bytes ) - added by anonymous 14 years ago.
d (458 bytes ) - added by Jeremy Dunck 14 years ago.
d.diff (458 bytes ) - added by Jeremy Dunck 14 years ago.

Download all attachments as: .zip

Change History (10)

by anonymous, 14 years ago

Attachment: order_by.patch added

comment:1 by nbv4, 14 years ago

I'm not 100% positive, but I'm pretty sure SQL databases internally optimize queries so that "LIMIT 1" automatically "cancels out" any ordering clause so that it doesn't execute the ordering code on that query.

in reply to:  1 comment:2 by Luke Plant, 14 years ago

Replying to nbv4:

I'm not 100% positive, but I'm pretty sure SQL databases internally optimize queries so that "LIMIT 1" automatically "cancels out" any ordering clause so that it doesn't execute the ordering code on that query.

If they did, they would produce wrong results. .get() does not do a LIMIT 1 anyway — the only reason we can remove the ORDER BY here is because we retrieve all results and throw an exception if there isn't exactly one i.e. we do the limiting to one after the database has returned results, and excess rows cause no data to be returned.

comment:3 by Jeremy Dunck, 14 years ago

Updated patch which passes all tests. The existing patch broke .latest() because you can't change ordering after slicing.

by Jeremy Dunck, 14 years ago

Attachment: d added

comment:4 by Jeremy Dunck, 14 years ago

Keywords: mq mq-200912-dallas added
Triage Stage: UnreviewedReady for checkin

comment:5 by Jeremy Dunck, 14 years ago

Triage Stage: Ready for checkinAccepted

by Jeremy Dunck, 14 years ago

Attachment: d.diff added

comment:6 by Alex Gaynor, 14 years ago

Triage Stage: AcceptedReady for checkin

comment:7 by Luke Plant, 14 years ago

Resolution: fixed
Status: newclosed

(In [11916]) Fixed #12258 - QuerySet.get() should clear ordering.

We only clear ordering when doing so cannot change the result returned by
the get() method i.e. when the query does not already define limits.

Thanks to mattdennewitz@… for the report, and jdunck for the patch

Note: See TracTickets for help on using tickets.
Back to Top