Django

Code

Ticket #2430 (closed: fixed)

Opened 2 years ago

Last modified 3 weeks ago

QuerySet should be evaluated when used in boolean tests

Reported by: ubernostrum Assigned to: nobody
Component: Database wrapper Version: SVN
Keywords: qs-rf-fixed Cc: ferringb@gmail.com, harish.mallipeddi@gmail.com
Triage Stage: Accepted Has patch: 1
Needs documentation: 0 Needs tests: 0
Patch needs improvement: 0

Description

Currently, a QuerySet? is not evaluated when being used in boolean contexts (e.g., doing if SomeModel.objects.all():); this leads to counterintuitive behavior, because it will always resolve to a True value regardless of whether it would return any objects when "actually" evaluated.

Adding a __nonzero__ method to QuerySet? which does the evaluation would solve this.

Attachments

nonzero_fix.diff (507 bytes) - added by anonymous on 09/12/07 15:15:14.
QuerySet? nonzero fix

Change History

07/26/06 16:05:40 changed by mtredinnick

  • status changed from new to assigned.

01/24/07 09:16:59 changed by mir@noris.de

I tried it out and this seems to have changed, at least under python 2.4. Can you test again, please?

>>> if models.Mailrule.objects.filter(quelle="aaaa"):
        print "boing"
>>> if models.Mailrule.objects.all():
...     print "yepp"
yepp

>>>

01/30/07 15:57:08 changed by mir@noris.de

  • status changed from assigned to closed.
  • resolution set to fixed.

Well, then I count it as "has been fixed."

02/12/07 16:55:26 changed by mtredinnick

  • status changed from closed to reopened.
  • resolution deleted.
  • stage changed from Unreviewed to Accepted.

This has not been fixed, because evaluating it in the above context calls len() to be called on the QuerySet. This is a very bad idea if you have, say, 100K rows in the result set (it reads them all into memory). We do need to implement a proper __nonzero___ method that only looks for the existence of one row without consuming it (and this is only mildly hard because the rowcount attribute on a database cursor in SQLite always returns -1).

Reopening. I'll fix this one day. It's something that is needed.

02/12/07 22:25:06 changed by Michael Radziej <mir@noris.de>

Well ... what you describe is actually a different from what has been reported.

Anyway, I'd propose to turn this into a query like:

select exists(select * from ...)

Unfortunately, this might turn out to be backend specific, too. For example, oracle doesn't like select queries without a table, IIRC. With mysql and postgresql, the above syntax works.

02/12/07 22:36:27 changed by mtredinnick

Using an extra SQL query is not the way to go here; what if the query is time consuming?. We already have the result set. We just need to read the first row (in the cases where "rowcount" is not honoured on cursors) and if we get data back, cache it and remember to add it to future retrievals. A little fiddly, but not impossible and imposes no extra database load.

07/09/07 11:35:01 changed by anonymous

  • cc set to ferringb@gmail.com.

09/12/07 15:15:14 changed by anonymous

  • attachment nonzero_fix.diff added.

QuerySet? nonzero fix

09/12/07 15:23:50 changed by mallipeddi

  • has_patch set to 1.

I just submitted a simple patch which essentially adds a nonzero method to the QuerySet? class. Inside the nonzero method, I fetch the first item via getitem call. This is either answered from the result cache (if any) or would result in a SELECT query with the LIMIT set to 1.

09/12/07 15:25:38 changed by mallipeddi

  • cc changed from ferringb@gmail.com to ferringb@gmail.com, harish.mallipeddi@gmail.com.

09/13/07 16:45:55 changed by mtredinnick

  • keywords set to qs-rf.

01/26/08 07:23:55 changed by mtredinnick

(In [7030]) queryset-refactor: Converted the queryset iterator to be a real iterator and only populate the result cache on demand. We actually populate the result cache 100 elements at a time, rather than one at a time for efficiency, but this is a real win when the resultset contains 10,000 objects for example.

This also provides an efficient boolean (nonzero) test that doesn't use up a lot of memory if you don't read all the results.

Refs #2430, #5987.

01/26/08 07:28:28 changed by mtredinnick

  • keywords changed from qs-rf to qs-rf-fixed.

04/26/08 21:50:16 changed by mtredinnick

  • status changed from reopened to closed.
  • resolution set to fixed.

(In [7477]) Merged the queryset-refactor branch into trunk.

This is a big internal change, but mostly backwards compatible with existing code. Also adds a couple of new features.

Fixed #245, #1050, #1656, #1801, #2076, #2091, #2150, #2253, #2306, #2400, #2430, #2482, #2496, #2676, #2737, #2874, #2902, #2939, #3037, #3141, #3288, #3440, #3592, #3739, #4088, #4260, #4289, #4306, #4358, #4464, #4510, #4858, #5012, #5020, #5261, #5295, #5321, #5324, #5325, #5555, #5707, #5796, #5817, #5987, #6018, #6074, #6088, #6154, #6177, #6180, #6203, #6658


Add/Change #2430 (QuerySet should be evaluated when used in boolean tests)




Change Properties
Action