Opened 19 years ago

Closed 17 years ago

#391 closed defect (fixed)

date_based generic views might be off by one

Reported by: Brendan O'Connor <brenocon@…> Owned by: nobody
Component: Generic views Version:
Severity: normal Keywords: sprintsept14
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

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

object_detail_tests.diff (3.4 KB ) - added by Rob Hunter <robertjhunter@…> 17 years ago.
Basic tests for the date_based generic view, object_detail

Download all attachments as: .zip

Change History (10)

comment:1 by Brendan O'Connor <brenocon@…>, 19 years ago

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

comment:2 by robh, 17 years ago

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.

comment:3 by Simon G. <dev@…>, 17 years ago

Keywords: qs-rf added
Summary: date comparsions broken? sqlite?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.

comment:4 by Nis Jørgensen <nis@…>, 17 years ago

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:

comment:5 by Philippe Raoult, 17 years ago

Component: Database wrapperGeneric views
Keywords: qs-rf removed
Summary: date comparsions in sqlite are off-by-onedate_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.

comment:6 by Rob Hunter <robertjhunter@…>, 17 years ago

Has patch: set
Keywords: sprintsept14 added
Resolution: worksforme
Status: newclosed

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

by Rob Hunter <robertjhunter@…>, 17 years ago

Attachment: object_detail_tests.diff added

Basic tests for the date_based generic view, object_detail

comment:7 by Rob Hunter <robertjhunter@…>, 17 years ago

Resolution: worksforme
Status: closedreopened

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.

comment:8 by robh, 17 years ago

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.

comment:9 by robh, 17 years ago

Resolution: fixed
Status: reopenedclosed

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

Note: See TracTickets for help on using tickets.
Back to Top