Opened 9 years ago

Closed 9 years ago

#23662 closed Cleanup/optimization (wontfix)

QuerySet __nonzero__, __len__ cause queryset evaluation

Reported by: Alexey Smishlayev Owned by: nobody
Component: Database layer (models, ORM) Version: 1.7
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: yes

Description (last modified by Akis Kesoglou)

Current implementation: https://github.com/django/django/blob/master/django/db/models/query.py

class QuerySet(object):
    ...
    def __nonzero__(self):
        self._fetch_all()
        return bool(self._result_cache)
    ...
    def __len__(self):
        self._fetch_all()
        return len(self._result_cache)

These methods call self._fetch_all(), thus evaluating the queryset. Although, this behaviour is documented, it is not obvious.

It seems logical to evaluate queryset, when casting it to a list() or iterating over it, but these particular cases have nothing to do with the queryset contents. There exist specific lazy methods (QuerySet.exists() and QuerySet.count() respectively) which, IMHO, should be used for the magic method implementation.

If I already have fetched the results of a queryset, I'm okay to call __len__() on them, but if they haven't been retrieved yet, I'd rather use SQL COUNT() instead. That is exactly, what QuerySet.count() does. The same goes for the __nonzero__() and exists().

Attachments (1)

Fix#23662.patch (1.1 KB ) - added by Alexey Smishlayev 9 years ago.

Download all attachments as: .zip

Change History (6)

by Alexey Smishlayev, 9 years ago

Attachment: Fix#23662.patch added

comment:1 by Alexey Smishlayev, 9 years ago

Component: UncategorizedDatabase layer (models, ORM)
Has patch: set
Type: UncategorizedCleanup/optimization
UI/UX: set

comment:2 by Akis Kesoglou, 9 years ago

Description: modified (diff)

Edited bug description to properly format example code

comment:3 by Akis Kesoglou, 9 years ago

Description: modified (diff)

Ditto

comment:4 by Thomas C, 9 years ago

In my opinion, that could almost be considered as a backward incompatible change as people might rely on the fact that doing len(qs) followed by for ... in qs: will only perform one query (which is important on Postgres as a COUNT(*) query is rather expensive). Same goes for bool(qs).

comment:5 by Loic Bistuer, 9 years ago

Resolution: wontfix
Status: newclosed

There is merit in your request but it's too late to change this, it would be a major backward incompatibility. Lot of code rely on the fact that a QuerySet is evaluated after being tested for equality or after a call to len().

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