Opened 13 years ago
Closed 13 years ago
#17535 closed Cleanup/optimization (fixed)
list_detail call to len(queryset) should be queryset.exists()
Reported by: | Owned by: | Aymeric Augustin | |
---|---|---|---|
Component: | Generic views | Version: | 1.3 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Refer to https://code.djangoproject.com/browser/django/trunk/django/views/generic/list_detail.py#L97. The line if not allow_empty and len(queryset) == 0
executes slowly for large querysets. We are already dealing with a real Queryset and not just any iterable (there is an earlier call queryset._clone) so we can use the exists
method. I propose if not allow_empty and not queryset.exists()
.
Change History (3)
comment:1 by , 13 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 13 years ago
Owner: | changed from | to
---|
#18087 was extremely similar, let's apply the same technique here.
comment:3 by , 13 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Note:
See TracTickets
for help on using tickets.
The code you mention is in deprecated code. Moreover, this queryset evaluation is done in the non paginated section of the context build. The query, even if large, would be evaluated anyway later in the code. So this part seems a 'won't fix' for me.
However, the issue is similar in the new class-based view code (https://code.djangoproject.com/browser/django/trunk/django/views/generic/list.py#L116). And in this case, the len(queryset)==0 is done before the pagination take place. So this might be an issue.
If you take the case of a costly query with not so many results, your proposal would call the query twice (once for exists and once for the real query), hence resulting in poorer performance when the query is not empty. Your solution might be better for a quick query that returns many results. The real solution would be probably to follow the same logic than the old function-based view and do this test later in the process (get_context_data).