Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#8044 closed (wontfix)

random filter should be smarter about querysets

Reported by: Idan Gazit Owned by: nobody
Component: Template system Version: master
Severity: Keywords:
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Apologies in advance if my terminology isn't quite right, I'm new to all of this...

Right now, the random filter's implementation naively chooses from the supplied list. In the case of querysets, this might be wildly inefficient -- converting the queryset into a list causes evaluation of the entire queryset, which might be fairly large. Ideally, the desired outcome is that random will simply limit the queryset to one of its elements (or None if the queryset is empty).

This patch should leave existing behavior intact for everything except querysets.

Patch + documentation attached. Be gentle and tell me what to do next if something is lacking. Particularly: I think I correctly did the cross-linking to the docs about limiting querysets correctly (in the templates documentation for the random filter) -- but would appreciate a check from somebody else.

Due credit for this goes to bram_ and elliott on #django; I came in there to ask about "how best to choose a random element" and this cropped up. The code for the filter is bram's from pastebin. The documentation and diffwork is mine. :)

Attachments (1)

random.diff (1.7 KB) - added by Idan Gazit 8 years ago.
Patch for 'random' template filter to smartly handle querysets

Download all attachments as: .zip

Change History (5)

Changed 8 years ago by Idan Gazit

Attachment: random.diff added

Patch for 'random' template filter to smartly handle querysets

comment:1 Changed 8 years ago by Eric Holscher

milestone: post-1.0
Triage Stage: UnreviewedDesign decision needed

comment:2 Changed 8 years ago by Malcolm Tredinnick

Resolution: wontfix
Status: newclosed

If you're passing a queryset to a template, it's reasonable to expect that it is going to be read into memory at some point (by iterating over it, for example). So that's not really a deciding issue here. This patch does introduce an extra database query however, which could also be quite expensive.

I'm inclined to say "no" on this feature. If you find yourself wanting to use random selection a lot on large querysets that wouldn't otherwise have their results used and you're happy to pay for the extra database query, you could write your own tag that does the same thing.

comment:3 Changed 8 years ago by Carl Meyer

This patch could easily be rewritten to avoid the extra DB query (by using something like value.order_by('?')[0] ), so if that's the main negative it might be worth reconsidering?

comment:4 Changed 8 years ago by (none)

milestone: post-1.0

Milestone post-1.0 deleted

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