diff --git a/django_tables2/data.py b/django_tables2/data.py index 1f89dba6..daefded9 100644 --- a/django_tables2/data.py +++ b/django_tables2/data.py @@ -133,11 +133,14 @@ def validate(data): ) def __len__(self): + '''Cached data length''' if not hasattr(self, '_length') or self._length is None: - # Use the queryset count() method to get the length, instead of - # loading all results into memory. This allows, for example, - # smart paginators that use len() to perform better. - self._length = self.data.count() + if hasattr(self.table, 'paginator'): + # for paginated tables, use QuerySet.count() as we are interested in total number of records. + self._length = self.data.count() + else: + # for non-paginated tables, use the length of the QuerySet + self._length = len(self.data) return self._length diff --git a/tests/test_config.py b/tests/test_config.py index 39b0cbf0..2687164e 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,10 +1,11 @@ # coding: utf-8 from django.core.paginator import EmptyPage, PageNotAnInteger -from django.test import SimpleTestCase +from django.test import SimpleTestCase, TestCase from fudge import Fake from django_tables2 import Column, RequestConfig, Table +from .app.models import Person from .utils import build_request NOTSET = object() # unique value @@ -73,6 +74,32 @@ def test_passing_request_to_constructor(self): class SimpleTable(Table): abc = Column() - table = SimpleTable([], request=request) - + table = SimpleTable([{}], request=request) assert table.columns['abc'].is_ordered + + +class NoPaginationQueriesTest(TestCase): + + def test_should_not_count_with_paginate_False(self): + ''' + No extra queries with pagination turned off. + + https://github.com/jieter/django-tables2/issues/551 + ''' + class MyTable(Table): + first_name = Column() + + class Meta: + template_name = 'minimal.html' + + request = build_request() + + Person.objects.create(first_name='Talip', last_name='Molenschot') + + table = MyTable(Person.objects.all()) + RequestConfig(request, paginate=False).configure(table) + + with self.assertNumQueries(1): + html = table.as_html(request) + + self.assertIn('', html) diff --git a/tests/test_models.py b/tests/test_models.py index db70ee39..b2dd954c 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -391,7 +391,7 @@ def order_first_name(self, queryset, is_descending): assert table.rows[0].get_cell('first_name') == 'VeryLongFirstName' -class ModelSantityTest(TestCase): +class ModelSanityTest(TestCase): def setUp(self): for i in range(10): Person.objects.create(first_name='Bob %d' % i, last_name='Builder') @@ -488,13 +488,18 @@ class Meta: with self.assertNumQueries(1): list(table.as_values()) - def test_as_html_db_queries(self): + def test_as_html_db_queries_nonpaginated(self): + ''' + Basic tables without pagination should NOT result in a COUNT(*) being done, + but only fetch the rows. + ''' class PersonTable(tables.Table): class Meta: model = Person - with self.assertNumQueries(2): - PersonTable(Person.objects.all()).as_html(build_request()) + with self.assertNumQueries(1): + html = PersonTable(Person.objects.all()).as_html(build_request()) + self.assertIn('Bob 0', html) class TableFactoryTest(TestCase): diff --git a/tests/test_templates.py b/tests/test_templates.py index a004c4fb..572d62a8 100644 --- a/tests/test_templates.py +++ b/tests/test_templates.py @@ -113,6 +113,11 @@ class Meta: PersonTable(Person.objects.all()).as_html(request) def test_render_table_db_queries(self): + ''' + Paginated tables should result in two queries: + - one query for pagination: .count() + - one query for records on the current page: .all()[start:end] + ''' Person.objects.create(first_name='brad', last_name='ayers') Person.objects.create(first_name='davina', last_name='adisusila') @@ -123,14 +128,14 @@ class Meta: request = build_request('/') with self.assertNumQueries(2): - # one query for pagination: .count() - # one query for page records: .all()[start:end] request = build_request('/') table = PersonTable(Person.objects.all()) RequestConfig(request).configure(table) - # render - (Template('{% load django_tables2 %}{% render_table table %}') - .render(Context({'table': table, 'request': request}))) + html = Template('{% load django_tables2 %}{% render_table table %}') \ + .render(Context({'table': table, 'request': request})) + + self.assertIn('brad', html) + self.assertIn('ayers', html) class TemplateLocalizeTest(TestCase):