Fixed #724 -- Ensured get_next_by_FOO() and get_previous_by_FOO() met…

…hods 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

@@ -604,8 +604,8 @@ def __new__(cls, name, bases, attrs):
                 # 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)
 

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

tests/testapp/models/lookup.py

@@ -30,6 +30,8 @@ def __repr__(self):
 >>> 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 @@ def __repr__(self):
 Article 4
 Article 2
 Article 3
+Article 7
 Article 1
 
 # get_iterator() takes the same lookup arguments as get_list().
@@ -48,9 +51,9 @@ def __repr__(self):
 
 # 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 @@ def __repr__(self):
 # 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 @@ def __repr__(self):
 [('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
 """