Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #18354 -- Performance issue in CBV.

Prevented repeating a query twice when the model isn't ordered by
-date_field (in Meta), allow_empty is False and pagination isn't
enabled.
  • Loading branch information...
commit 03f86a5adb930eac55dea1e903fb958c002d5bc4 1 parent b0c1e5c
@aaugustin aaugustin authored
View
25 django/views/generic/dates.py
@@ -315,7 +315,7 @@ def get_dated_items(self):
"""
raise NotImplementedError('A DateView must provide an implementation of get_dated_items()')
- def get_dated_queryset(self, **lookup):
+ def get_dated_queryset(self, ordering=None, **lookup):
"""
Get a queryset properly filtered according to `allow_future` and any
extra lookup kwargs.
@@ -326,6 +326,9 @@ def get_dated_queryset(self, **lookup):
allow_empty = self.get_allow_empty()
paginate_by = self.get_paginate_by(qs)
+ if ordering is not None:
+ qs = qs.order_by(ordering)
+
if not allow_future:
now = timezone.now() if self.uses_datetime_field else timezone_today()
qs = qs.filter(**{'%s__lte' % date_field: now})
@@ -370,15 +373,13 @@ def get_dated_items(self):
"""
Return (date_list, items, extra_context) for this request.
"""
- qs = self.get_dated_queryset()
+ qs = self.get_dated_queryset(ordering='-%s' % self.get_date_field())
date_list = self.get_date_list(qs, 'year')
- if date_list:
- object_list = qs.order_by('-' + self.get_date_field())
- else:
- object_list = qs.none()
+ if not date_list:
+ qs = qs.none()
- return (date_list, object_list, {})
+ return (date_list, qs, {})
class ArchiveIndexView(MultipleObjectTemplateResponseMixin, BaseArchiveIndexView):
@@ -410,17 +411,15 @@ def get_dated_items(self):
'%s__lt' % date_field: until,
}
- qs = self.get_dated_queryset(**lookup_kwargs)
+ qs = self.get_dated_queryset(ordering='-%s' % date_field, **lookup_kwargs)
date_list = self.get_date_list(qs, 'month')
- if self.get_make_object_list():
- object_list = qs.order_by('-' + date_field)
- else:
+ if not self.get_make_object_list():
# We need this to be a queryset since parent classes introspect it
# to find information about the model.
- object_list = qs.none()
+ qs = qs.none()
- return (date_list, object_list, {'year': year})
+ return (date_list, qs, {'year': year})
def get_make_object_list(self):
"""
View
14 tests/regressiontests/generic_views/dates.py
@@ -86,10 +86,15 @@ def test_paginated_archive_view_does_not_load_entire_table(self):
# 1 query for years list + 1 query for books
with self.assertNumQueries(2):
self.client.get('/dates/books/')
- # same as above + 1 query to test if books exist
- with self.assertNumQueries(3):
+ # same as above + 1 query to test if books exist + 1 query to count them
+ with self.assertNumQueries(4):
self.client.get('/dates/books/paginated/')
+ def test_no_duplicate_query(self):
+ # Regression test for #18354
+ with self.assertNumQueries(2):
+ self.client.get('/dates/books/reverse/')
+
def test_datetime_archive_view(self):
BookSigning.objects.create(event_date=datetime.datetime(2008, 4, 2, 12, 0))
res = self.client.get('/dates/booksignings/')
@@ -155,6 +160,11 @@ def test_year_view_invalid_pattern(self):
res = self.client.get('/dates/books/no_year/')
self.assertEqual(res.status_code, 404)
+ def test_no_duplicate_query(self):
+ # Regression test for #18354
+ with self.assertNumQueries(2):
+ self.client.get('/dates/books/2008/reverse/')
+
def test_datetime_year_view(self):
BookSigning.objects.create(event_date=datetime.datetime(2008, 4, 2, 12, 0))
res = self.client.get('/dates/booksignings/2008/')
View
2  tests/regressiontests/generic_views/templates/generic_views/book_archive.html
@@ -1 +1 @@
-Archive of books from {{ date_list }}.
+Archive of books from {{ date_list }}. {{ object_list|length }} books found.
View
2  tests/regressiontests/generic_views/templates/generic_views/book_archive_year.html
@@ -1 +1 @@
-Archive of books from {{ year }}.
+Archive of books from {{ year }}. {{ object_list|length }} books found.
View
5 tests/regressiontests/generic_views/urls.py
@@ -4,6 +4,7 @@
from django.views.decorators.cache import cache_page
from django.views.generic import TemplateView
+from . import models
from . import views
@@ -108,6 +109,8 @@
views.BookArchive.as_view(queryset=None)),
(r'^dates/books/paginated/$',
views.BookArchive.as_view(paginate_by=10)),
+ (r'^dates/books/reverse/$',
+ views.BookArchive.as_view(queryset=models.Book.objects.order_by('pubdate'))),
(r'^dates/booksignings/$',
views.BookSigningArchive.as_view()),
@@ -160,6 +163,8 @@
views.BookYearArchive.as_view(make_object_list=True, paginate_by=30)),
(r'^dates/books/no_year/$',
views.BookYearArchive.as_view()),
+ (r'^dates/books/(?P<year>\d{4})/reverse/$',
+ views.BookYearArchive.as_view(queryset=models.Book.objects.order_by('pubdate'))),
(r'^dates/booksignings/(?P<year>\d{4})/$',
views.BookSigningYearArchive.as_view()),
Please sign in to comment.
Something went wrong with that request. Please try again.