Do not perform extra count queries with non-paginated tables. fixes #551

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
 

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('<table>', html)

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):

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):