From e3e37ed1202f58e4d5e172ecd15af5ab8eda3492 Mon Sep 17 00:00:00 2001
From: Adrian Holovaty <adrian@holovaty.com>
Date: Thu, 10 Nov 2005 05:36:41 +0000
Subject: [PATCH] Fixed #724 -- Ensured get_next_by_FOO() and
 get_previous_by_FOO() methods don't skip or duplicate any records in the case
 of duplicate values. Thanks for reporting the bug, mattycakes@gmail.com

git-svn-id: http://code.djangoproject.com/svn/django/trunk@1155 bcc190cf-cafb-0310-a4f2-bffc1f526a37
---
 django/core/meta/__init__.py   | 15 +++++++------
 docs/db-api.txt                |  6 ++++++
 tests/testapp/models/lookup.py | 39 +++++++++++++++++++++++++---------
 3 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/django/core/meta/__init__.py b/django/core/meta/__init__.py
index 7fe23a4884..22ce51d0a5 100644
--- a/django/core/meta/__init__.py
+++ b/django/core/meta/__init__.py
@@ -604,8 +604,8 @@ class ModelBase(type):
                 # for all DateFields and DateTimeFields that cannot be null.
                 # EXAMPLES: Poll.get_next_by_pub_date(), Poll.get_previous_by_pub_date()
                 if not f.null:
-                    setattr(new_class, 'get_next_by_%s' % f.name, curry(method_get_next_or_previous, new_mod.get_object, f, True))
-                    setattr(new_class, 'get_previous_by_%s' % f.name, curry(method_get_next_or_previous, new_mod.get_object, f, False))
+                    setattr(new_class, 'get_next_by_%s' % f.name, curry(method_get_next_or_previous, new_mod.get_object, opts, f, True))
+                    setattr(new_class, 'get_previous_by_%s' % f.name, curry(method_get_next_or_previous, new_mod.get_object, opts, f, False))
                 # Add "get_thingie_list" for all DateFields and DateTimeFields.
                 # EXAMPLE: polls.get_pub_date_list()
                 func = curry(function_get_date_list, opts, f)
@@ -990,10 +990,13 @@ def method_get_order(ordered_obj, self):
 
 # DATE-RELATED METHODS #####################
 
-def method_get_next_or_previous(get_object_func, field, is_next, self, **kwargs):
-    kwargs.setdefault('where', []).append('%s %s %%s' % (field.column, (is_next and '>' or '<')))
-    kwargs.setdefault('params', []).append(str(getattr(self, field.attname)))
-    kwargs['order_by'] = [(not is_next and '-' or '') + field.name]
+def method_get_next_or_previous(get_object_func, opts, field, is_next, self, **kwargs):
+    op = is_next and '>' or '<'
+    kwargs.setdefault('where', []).append('(%s %s %%s OR (%s = %%s AND %s %s %%s))' % \
+        (field.column, op, field.column, opts.pk.column, op))
+    param = str(getattr(self, field.attname))
+    kwargs.setdefault('params', []).extend([param, param, getattr(self, opts.pk.attname)])
+    kwargs['order_by'] = [(not is_next and '-' or '') + field.name, (not is_next and '-' or '') + opts.pk.name]
     kwargs['limit'] = 1
     return get_object_func(**kwargs)
 
diff --git a/docs/db-api.txt b/docs/db-api.txt
index 01aacf49b1..30dece6482 100644
--- a/docs/db-api.txt
+++ b/docs/db-api.txt
@@ -515,6 +515,12 @@ previous object with respect to the date field, raising the appropriate
 Both methods accept optional keyword arguments, which should be in the format
 described in "Field lookups" above.
 
+Note that in the case of identical date values, these methods will use the ID
+as a fallback check. This guarantees that no records are skipped or duplicated.
+For a full example, see the `lookup API sample model_`.
+
+.. _lookup API sample model: http://www.djangoproject.com/documentation/models/lookup/
+
 get_FOO_filename()
 ------------------
 
diff --git a/tests/testapp/models/lookup.py b/tests/testapp/models/lookup.py
index deb5d807ef..6d8ede1905 100644
--- a/tests/testapp/models/lookup.py
+++ b/tests/testapp/models/lookup.py
@@ -30,6 +30,8 @@ API_TESTS = """
 >>> a5.save()
 >>> a6 = articles.Article(headline='Article 6', pub_date=datetime(2005, 8, 1, 8, 0))
 >>> a6.save()
+>>> a7 = articles.Article(headline='Article 7', pub_date=datetime(2005, 7, 27))
+>>> a7.save()
 
 # get_iterator() is just like get_list(), but it's a generator.
 >>> for a in articles.get_iterator():
@@ -39,6 +41,7 @@ Article 6
 Article 4
 Article 2
 Article 3
+Article 7
 Article 1
 
 # get_iterator() takes the same lookup arguments as get_list().
@@ -48,9 +51,9 @@ Article 4
 
 # get_count() returns the number of objects matching search criteria.
 >>> articles.get_count()
-6L
+7L
 >>> articles.get_count(pub_date__exact=datetime(2005, 7, 27))
-2L
+3L
 >>> articles.get_count(headline__startswith='Blah blah')
 0L
 
@@ -67,10 +70,10 @@ Article 4
 # dictionaries instead of object instances -- and you can specify which fields
 # you want to retrieve.
 >>> articles.get_values(fields=['headline'])
-[{'headline': 'Article 5'}, {'headline': 'Article 6'}, {'headline': 'Article 4'}, {'headline': 'Article 2'}, {'headline': 'Article 3'}, {'headline': 'Article 1'}]
+[{'headline': 'Article 5'}, {'headline': 'Article 6'}, {'headline': 'Article 4'}, {'headline': 'Article 2'}, {'headline': 'Article 3'}, {'headline': 'Article 7'}, {'headline': 'Article 1'}]
 >>> articles.get_values(pub_date__exact=datetime(2005, 7, 27), fields=['id'])
-[{'id': 2}, {'id': 3}]
->>> articles.get_values(fields=['id', 'headline']) == [{'id': 5, 'headline': 'Article 5'}, {'id': 6, 'headline': 'Article 6'}, {'id': 4, 'headline': 'Article 4'}, {'id': 2, 'headline': 'Article 2'}, {'id': 3, 'headline': 'Article 3'}, {'id': 1, 'headline': 'Article 1'}]
+[{'id': 2}, {'id': 3}, {'id': 7}]
+>>> articles.get_values(fields=['id', 'headline']) == [{'id': 5, 'headline': 'Article 5'}, {'id': 6, 'headline': 'Article 6'}, {'id': 4, 'headline': 'Article 4'}, {'id': 2, 'headline': 'Article 2'}, {'id': 3, 'headline': 'Article 3'}, {'id': 7, 'headline': 'Article 7'}, {'id': 1, 'headline': 'Article 1'}]
 True
 
 # get_values_iterator() is just like get_values(), but it's a generator.
@@ -83,24 +86,40 @@ True
 [('headline', 'Article 4'), ('id', 4)]
 [('headline', 'Article 2'), ('id', 2)]
 [('headline', 'Article 3'), ('id', 3)]
+[('headline', 'Article 7'), ('id', 7)]
 [('headline', 'Article 1'), ('id', 1)]
 
 # Every DateField and DateTimeField creates get_next_by_FOO() and
 # get_previous_by_FOO() methods.
+# In the case of identical date values, these methods will use the ID as a
+# fallback check. This guarantees that no records are skipped or duplicated.
+>>> a1.get_next_by_pub_date()
+Article 2
+>>> a2.get_next_by_pub_date()
+Article 3
 >>> a3.get_next_by_pub_date()
-Article 4
->>> a2.get_previous_by_pub_date()
-Article 1
-
-# get_next_by_FOO() and get_previous_by_FOO() take the time into account.
+Article 7
 >>> a4.get_next_by_pub_date()
 Article 6
 >>> a5.get_next_by_pub_date()
 Traceback (most recent call last):
     ...
 ArticleDoesNotExist: Article does not exist for ...
+>>> a6.get_next_by_pub_date()
+Article 5
+>>> a7.get_next_by_pub_date()
+Article 4
+
+>>> a7.get_previous_by_pub_date()
+Article 3
 >>> a6.get_previous_by_pub_date()
 Article 4
 >>> a5.get_previous_by_pub_date()
 Article 6
+>>> a4.get_previous_by_pub_date()
+Article 7
+>>> a3.get_previous_by_pub_date()
+Article 2
+>>> a2.get_previous_by_pub_date()
+Article 1
 """