Fixed #26290 -- Warned that paginating an unordered QuerySet may result in inconsistent results.

5 files changed, 38 insertions, 13 deletions

diff --git a/django/core/paginator.py b/django/core/paginator.py
index 29b8969b90..e64d9804c5 100644
--- a/django/core/paginator.py
+++ b/django/core/paginator.py
@@ -1,10 +1,15 @@
 import collections
+import warnings
 from math import ceil
 
 from django.utils import six
 from django.utils.functional import cached_property
 
 
+class UnorderedObjectListWarning(RuntimeWarning):
+    pass
+
+
 class InvalidPage(Exception):
     pass
 
@@ -22,6 +27,7 @@ class Paginator(object):
     def __init__(self, object_list, per_page, orphans=0,
                  allow_empty_first_page=True):
         self.object_list = object_list
+        self._check_object_list_is_ordered()
         self.per_page = int(per_page)
         self.orphans = int(orphans)
         self.allow_empty_first_page = allow_empty_first_page
@@ -94,6 +100,17 @@ class Paginator(object):
         """
         return six.moves.range(1, self.num_pages + 1)
 
+    def _check_object_list_is_ordered(self):
+        """
+        Warn if self.object_list is unordered (typically a QuerySet).
+        """
+        if hasattr(self.object_list, 'ordered') and not self.object_list.ordered:
+            warnings.warn(
+                'Pagination may yield inconsistent results with an unordered '
+                'object_list: {!r}'.format(self.object_list),
+                UnorderedObjectListWarning
+            )
+
 
 QuerySetPaginator = Paginator   # For backwards-compatibility.
 
diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py
index 2146fa66f2..a987e146dc 100644
--- a/tests/admin_views/admin.py
+++ b/tests/admin_views/admin.py
@@ -481,7 +481,7 @@ class FieldOverridePostAdmin(PostAdmin):
 
 class CustomChangeList(ChangeList):
     def get_queryset(self, request):
-        return self.root_queryset.filter(pk=9999)  # Does not exist
+        return self.root_queryset.order_by('pk').filter(pk=9999)  # Doesn't exist
 
 
 class GadgetAdmin(admin.ModelAdmin):
diff --git a/tests/pagination/tests.py b/tests/pagination/tests.py
index 8990c7fb5c..cc063c9eb8 100644
--- a/tests/pagination/tests.py
+++ b/tests/pagination/tests.py
@@ -5,6 +5,7 @@ from datetime import datetime
 
 from django.core.paginator import (
     EmptyPage, InvalidPage, PageNotAnInteger, Paginator,
+    UnorderedObjectListWarning,
 )
 from django.test import TestCase
 from django.utils import six
@@ -256,7 +257,7 @@ class ModelPaginationTests(TestCase):
             a.save()
 
     def test_first_page(self):
-        paginator = Paginator(Article.objects.all(), 5)
+        paginator = Paginator(Article.objects.order_by('id'), 5)
         p = paginator.page(1)
         self.assertEqual("<Page 1 of 2>", six.text_type(p))
         self.assertQuerysetEqual(p.object_list, [
@@ -265,9 +266,7 @@ class ModelPaginationTests(TestCase):
             "<Article: Article 3>",
             "<Article: Article 4>",
             "<Article: Article 5>"
-        ],
-            ordered=False
-        )
+        ])
         self.assertTrue(p.has_next())
         self.assertFalse(p.has_previous())
         self.assertTrue(p.has_other_pages())
@@ -278,7 +277,7 @@ class ModelPaginationTests(TestCase):
         self.assertEqual(5, p.end_index())
 
     def test_last_page(self):
-        paginator = Paginator(Article.objects.all(), 5)
+        paginator = Paginator(Article.objects.order_by('id'), 5)
         p = paginator.page(2)
         self.assertEqual("<Page 2 of 2>", six.text_type(p))
         self.assertQuerysetEqual(p.object_list, [
@@ -286,9 +285,7 @@ class ModelPaginationTests(TestCase):
             "<Article: Article 7>",
             "<Article: Article 8>",
             "<Article: Article 9>"
-        ],
-            ordered=False
-        )
+        ])
         self.assertFalse(p.has_next())
         self.assertTrue(p.has_previous())
         self.assertTrue(p.has_other_pages())
@@ -303,7 +300,7 @@ class ModelPaginationTests(TestCase):
         Tests proper behavior of a paginator page __getitem__ (queryset
         evaluation, slicing, exception raised).
         """
-        paginator = Paginator(Article.objects.all(), 5)
+        paginator = Paginator(Article.objects.order_by('id'), 5)
         p = paginator.page(1)
 
         # Make sure object_list queryset is not evaluated by an invalid __getitem__ call.
@@ -323,3 +320,14 @@ class ModelPaginationTests(TestCase):
         )
         # After __getitem__ is called, object_list is a list
         self.assertIsInstance(p.object_list, list)
+
+    def test_paginating_unordered_queryset_raises_warning(self):
+        msg = (
+            "Pagination may yield inconsistent results with an unordered "
+            "object_list: <QuerySet [<Article: Article 1>, "
+            "<Article: Article 2>, <Article: Article 3>, <Article: Article 4>, "
+            "<Article: Article 5>, <Article: Article 6>, <Article: Article 7>, "
+            "<Article: Article 8>, <Article: Article 9>]>"
+        )
+        with self.assertRaisesMessage(UnorderedObjectListWarning, msg):
+            Paginator(Article.objects.all(), 5)
diff --git a/tests/sitemaps_tests/test_http.py b/tests/sitemaps_tests/test_http.py
index aac4d69a20..33599e9ed9 100644
--- a/tests/sitemaps_tests/test_http.py
+++ b/tests/sitemaps_tests/test_http.py
@@ -194,7 +194,7 @@ class HTTPSitemapTests(SitemapTestsBase):
         Check to make sure that the raw item is included with each
         Sitemap.get_url() url result.
         """
-        test_sitemap = GenericSitemap({'queryset': TestModel.objects.all()})
+        test_sitemap = GenericSitemap({'queryset': TestModel.objects.order_by('pk').all()})
 
         def is_testmodel(url):
             return isinstance(url['item'], TestModel)
diff --git a/tests/sitemaps_tests/urls/http.py b/tests/sitemaps_tests/urls/http.py
index 72e14d5e7f..e2fc991a29 100644
--- a/tests/sitemaps_tests/urls/http.py
+++ b/tests/sitemaps_tests/urls/http.py
@@ -27,7 +27,7 @@ class SimpleI18nSitemap(Sitemap):
     i18n = True
 
     def items(self):
-        return I18nTestModel.objects.all()
+        return I18nTestModel.objects.order_by('pk').all()
 
 
 class EmptySitemap(Sitemap):
@@ -115,7 +115,7 @@ sitemaps_lastmod_descending = OrderedDict([
 ])
 
 generic_sitemaps = {
-    'generic': GenericSitemap({'queryset': TestModel.objects.all()}),
+    'generic': GenericSitemap({'queryset': TestModel.objects.order_by('pk').all()}),
 }