Opened 10 years ago

Closed 7 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: UI/UX:

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@…> 7 years ago.
Basic tests for the date_based generic view, object_detail

Download all attachments as: .zip

Change History (10)

comment:1 Changed 10 years ago by Brendan O'Connor <brenocon@…>

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

comment:2 Changed 7 years ago 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.

comment:3 Changed 7 years ago by Simon G. <dev@…>

  • Keywords qs-rf added
  • 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.

comment:4 Changed 7 years ago by Nis Jørgensen <nis@…>

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 Changed 7 years ago by PhiR

  • Component changed from Database wrapper to Generic views
  • Keywords qs-rf removed
  • 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.

comment:6 Changed 7 years ago by Rob Hunter <robertjhunter@…>

  • Has patch set
  • Keywords sprintsept14 added
  • Resolution set to worksforme
  • Status changed from new to closed

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

Changed 7 years ago by Rob Hunter <robertjhunter@…>

Basic tests for the date_based generic view, object_detail

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

  • Resolution worksforme deleted
  • Status changed from closed to reopened

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 Changed 7 years ago 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.

comment:9 Changed 7 years ago by robh

  • Resolution set to fixed
  • Status changed from reopened to closed

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

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