django.core.paginator.Page.__getitem__ being called multiple times when used from a template.

[joshua.fialkoff@…]

Template code (using generic ListView):

{{ page_obj.number }}
{{ page_obj.number }}
{{ page_obj.number }}

When paired with a generic ListView, this will result in three calls to page_obj.__getitem__ (presumably, because the template first tries to use 'number' as an index). Each call to __getitem__ performs a list(self.object_list) . In many cases, this will be quick - which, I suppose, is why it has escaped notice. In my specific case, object_list is a generator, and each iteration performs a variety of functions (specifically, looking up other values that are of interest). So, this ultimately takes a while to run.

My workaround was simply to set page_obj.object_list = list(page_obj.object_list) prior to it being used in the template. It seems the actual fix here would just be to perform this operation in __init__ .

[Claude Paroz]

I'd rather not consume object_list in __init__, because it should still be possible to instanciate a Page without querying the database.
But maybe something like that (untested):

         # The object_list is converted to a list so that if it was a QuerySet
         # it won't be a database hit per __getitem__.
-        return list(self.object_list)[index]
+        if not isinstance(self.object_list, list):
+            self.object_list = list(self.object_list)
+        return self.object_list[index]

[Dmitry Gladkov]

As the comment in the actual function suggests, converting self.object_list to list only makes sense if we are paginating QuerySet, so I changed the example code above to check if object_list is instance of QuerySet.

[Simon Charette]

Please don't mark your own ticket to Ready for checkin, wait/ask for someone else to review it.

[Claude Paroz]

I'm not sure the proposed patch addresses the original issue, where object_list was apparently not a QuerySet, but a more "costly" generator.

[Simon Charette]

claudep I think your initial approach is the correct way of handling this.

[andrewjesaitis]

Added a patch for the issue as suggested by claudep. ​https://github.com/django/django/pull/919

Test is incorporated into test_page_getitem

[Simon Charette]

All pagination tests pass on py273, py323 SQLite.

[Claude Paroz <claude@…>]

In 31f6421b134e4e83a459d2faa1009b33fefd6276:

Fixed #19938 -- Consumed iterator only once in paginator's Page

Thanks Joshua Fialkoff for the report.