Django

Code

Ticket #391 (closed: fixed)

Opened 3 years ago

Last modified 10 months ago

date_based generic views might be off by one

Reported by: Brendan O'Connor <brenocon@gmail.com> Assigned to: nobody
Milestone: Component: Generic views
Version: Keywords: sprintsept14
Cc: Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

I created an item today, and now am trying to view it with date_based.object_detail. Unfortunately, the condition condition "date must be <= today" isn't working right.

blog.notes does not exist for {'title__exact': 'test-title', 'created__lte': datetime.datetime(2005, 8, 21, 18, 34, 26, 74000), 'created__range': (datetime.datetime(2005, 8, 21, 0, 0), datetime.datetime(2005, 8, 21, 23, 59, 59, 999999))}

Turns out it's some sort of off-by-one bug in the lte handler. If the <= comparison compares against tomorrow, it works fine, e.g. with

Index: django/views/generic/date_based.py
===================================================================
--- django/views/generic/date_based.py  (revision 544)
+++ django/views/generic/date_based.py  (working copy)
@@ -221,6 +221,8 @@
         '%s__range' % date_field: (datetime.datetime.combine(date, datetime.time.min), datetime.datetime.combine(date, datetime.time.max)),
     }
     # Only bother to check current date if the date isn't in the past.
+    from datetime import timedelta
+    now += timedelta(days = 1)
     if date >= now.date():
         lookup_kwargs['%s__lte' % date_field] = now
     if object_id:

I'm on SQLite, windows, python 2.4, svn [544]. Maybe it's a sqlite date comparison bug or something.

Attachments

object_detail_tests.diff (3.4 kB) - added by Rob Hunter <robertjhunter@gmail.com> on 09/14/07 20:38:20.
Basic tests for the date_based generic view, object_detail

Change History

08/21/05 20:49:20 changed by Brendan O'Connor <brenocon@gmail.com>

that is to say: date__lte is off-by-one, but date__range is OK here.

09/13/07 18:09:51 changed by robh

I've reproduced that here, and I'm convinced that it's related to the difference between a "date" (whole days) and a "datetime" (fractional days).

SQLite columns are stored without regard to type, so if you store a datetime (via the Django model API or otherwise) and compare it to a plain date, the date will be considered as midnight on that date.

SELECT DATE('now') <= DATETIME('now'); -- 1: midnight today is earlier than right now

I might take a look and see if I can reproduce this in a Django context on the upcoming Django sprint.

09/13/07 18:23:54 changed by Simon G. <dev@simon.net.nz>

  • keywords set to qs-rf.
  • summary changed from date comparsions broken? sqlite? to date comparsions in sqlite are off-by-one.

robh: if you could dig deeper in to this, that would be great, but I think this might be something we can't do anything about. Also adding qs-rf to this so Malcolm knows about it for the query set refactor.

09/14/07 08:04:30 changed by Nis Jørgensen <nis@superlativ.dk>

I am unable to reproduce the problem in the generic view - ie I just created a note object, and I am able and see it in the view for today. So the original problem Works For Me

It seems to me that the problem described in the comments is really one of expectations:

<datetimefield>lte=<date object>

does not "cast" the field to a date before doing the comparison, it "casts" the date to a datetime. This is true for all backends, not just sqlite.

This is what I would expect as well - so I believe the ticket should be closed as wontfix. Alternatively, a design decision needs to be taken. Some questions:

09/14/07 08:30:46 changed by PhiR

  • keywords deleted.
  • component changed from Database wrapper to Generic views.
  • summary changed from date comparsions in sqlite are off-by-one to date_based generic views might be off by one.

the database is working fine, it is (was ?) only an issue of the view using a date which implicitly has a time of 0:00 when comparing to a datetime.

09/14/07 20:36:57 changed by Rob Hunter <robertjhunter@gmail.com>

  • keywords set to sprintsept14.
  • status changed from new to closed.
  • has_patch set to 1.
  • resolution set to worksforme.

I agree the database (and ORM) is doing what it should, and I've been unable to reproduce the reported problem "in real life" or test cases.

I've written some tests for the basic behaviour of the django.views.generic.date_based.object_detail, although I don't like that they use the Test Client. It seems too coarse-grained.

(The generic views appear to have no tests at all, save for what [1282] might add.)

09/14/07 20:38:20 changed by Rob Hunter <robertjhunter@gmail.com>

  • attachment object_detail_tests.diff added.

Basic tests for the date_based generic view, object_detail

09/14/07 20:43:07 changed by Rob Hunter <robertjhunter@gmail.com>

  • status changed from closed to reopened.
  • resolution deleted.

I noticed a comment on SprintIdeas that asks us not to close tickets until the code is committed.

I'm not sure if that applies to this patch or not, but I'm re-opening this one anyway.

09/15/07 23:52:45 changed by robh

I've included patch for this in #5506 -- the particlar bug this ticket talks about is apparently no longer present, but the test in #5506 should make sure that it never comes back.

09/16/07 00:00:21 changed by robh

  • status changed from reopened to closed.
  • resolution set to fixed.

Malcolm reckons it's OK to move discussion to #5506.


Add/Change #391 (date_based generic views might be off by one)




Change Properties
Action