#7478 closed (fixed)
Paginator could detect if the object_list is a QuerySet or a list
| Reported by: | Owned by: | nobody | |
|---|---|---|---|
| Component: | Core (Other) | Version: | dev |
| Severity: | Keywords: | paginator | |
| Cc: | Triage Stage: | Unreviewed | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
If the Paginator is accidentally used to count a large QuerySet (instead of a QuerySetPaginator), the generated SQL request could be inefficient (all content will be retrieved just to make len on it). Why don't we automatically detect if object_list has a count attribute? The paginator API will be simpler and it will be more difficult to introduce performance issues.
def _get_count(self):
if self._count is None:
if hasattr(self.object_list, 'count'):
self._count = self.object_list.count()
else:
self._count = len(self.object_list)
return self._count
count = property(_get_count)
And by the way the Paginator is far way to simple for my needs so I have sub class it. But I can't derivate from both QuerySetPaginator and Paginator. It's antoher reason why I think it's not a good idea to have 2 classes.
Change History (8)
comment:1 by , 17 years ago
| Component: | Uncategorized → Core framework |
|---|---|
| Keywords: | paginator added |
comment:2 by , 17 years ago
comment:4 by , 17 years ago
if isinstance(self.object_list, QuerySet):
self._count = self.object_list.count()
comment:5 by , 17 years ago
I am using this one and it works nicely:
def _get_count(self):
if self._count is None:
if isinstance(self.object_list, QuerySet):
self._count = self.object_list.count()
else:
self._count = len(self.object_list)
return self._count
count = property(_get_count)
comment:7 by , 17 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
Woops count is not the good attribute to test because lists also have it. "all" seems a safe choice to me.