Opened 10 years ago
Closed 10 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 )
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)
Change History (6)
by , 10 years ago
Attachment: | Fix#23662.patch added |
---|
comment:1 by , 10 years ago
Component: | Uncategorized → Database layer (models, ORM) |
---|---|
Has patch: | set |
Type: | Uncategorized → Cleanup/optimization |
UI/UX: | set |
comment:2 by , 10 years ago
Description: | modified (diff) |
---|
comment:4 by , 10 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 , 10 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
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()
.
Edited bug description to properly format example code