Opened 3 years ago

Last modified 3 years ago

#24076 new Bug

Query may fail with pytz exception

Reported by: lvella Owned by: nobody
Component: Documentation Version: 1.6
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If I give date object as input to a filter on a DateTimeField field, like this:

class Ticket(models.Model):
    register_time = models.DateTimeField(auto_now_add=True)

day = datetime.strptime('2014-10-19', '%Y-%m-%d').date()
Ticket.objects.filter(register_time__gte=day)

I may get a pytz.exceptions.NonExistentTimeError exception. The exact exception was:

pytz.exceptions.NonExistentTimeError: 2014-10-19 00:00:00

This date is the start of DST in my timezone:

TIME_ZONE = 'America/Sao_Paulo'

I believe this is a bug because Django tries to convert my input to a datetime object before building the query, and in this case, this conversion yields an invalid datetime.

This bug seems very related to issue #9596, because in these cases, datetime should be converted to date, not the opposite.

A minimal showcase is attached. Unzip and run ./manage.py test

Attachments (1)

bug.zip (3.3 KB) - added by lvella 3 years ago.
Minimal showcase.

Download all attachments as: .zip

Change History (4)

Changed 3 years ago by lvella

Attachment: bug.zip added

Minimal showcase.

comment:1 Changed 3 years ago by Aymeric Augustin

That's the intended behavior -- Django won't guess what you mean if you pass invalid inputs.

However, if #9596 gets fixed with the idea of comment 24, you will be able to run Ticket.objects.filter(register_time__date__gte=day).

Until then, you're stuck with comparing with an aware datetime object. See https://www.python.org/dev/peps/pep-0431/#ambiguous-times for why this is awfully complicated.

comment:2 in reply to:  1 Changed 3 years ago by lvella

Replying to aaugustin:

That's the intended behavior -- Django won't guess what you mean if you pass invalid inputs.

But if using date to compare with datetime works 99% of the time, and there is no big warning in the documentation, saying that it must not be done, it clearly violates the principle of least astonishment.

In such cases, either it should always work, or should always fail with an error. And it is not that hard to make it work, for instead of trying to add time information to the date before passing it to the database, the database type should be converted to date, dropping the time information in the process.

Actually, my workaround in the case was to force Django to deliver my input in SQL as date, not as datetime:

q = Ticket.objects.filter(register_time__gte=day + (F('id') - F('id')))
print(q.query)

yields:

SELECT "reproduce_ticket"."id", "reproduce_ticket"."register_time" FROM "reproduce_ticket" WHERE "reproduce_ticket"."register_time" >=  2014-10-19 + ("reproduce_ticket"."id" - "reproduce_ticket"."id")

and it worked!
Compare with the original query (with another date so it won't raise the exception):

q = Ticket.objects.filter(register_time__gte=day)
print(q.query)

SELECT "reproduce_ticket"."id", "reproduce_ticket"."register_time" FROM "reproduce_ticket" WHERE "reproduce_ticket"."register_time" >= 2014-11-19 02:00:00

comment:3 Changed 3 years ago by Aymeric Augustin

Component: Database layer (models, ORM)Documentation
Triage Stage: UnreviewedAccepted

Thanks for the proposal. Unfortunately it won't work on databases other than PostgreSQL because they store datetimes in UTC. As a consequence they're vulnerable to the effect I described in question 3 in the timezone troubleshooting FAQ.

I considered this issue carefully when I implemented support for time zones in Django 1.4. I was aware of the points you're making. I decided that forbidding mixing dates and datetimes outright was too extreme. It would require many changes in projects that work just fine because they never see datetimes during the DST switch.

It's the left-hand-side of a lookup that determines the type, not the right-hand-side. Therefore the correct solution is to implement an explicit __date lookup i.e. #9596. That's the opposite of what you requested when you filed the ticket but that also what you recommended in your last comment:

the database type should be converted to date, dropping the time information in the process

This solution requires timezone conversion of datetimes in the database but it shouldn't be an issue as it already exists to support the dates() and datetimes() queryset methods.

I'm going to requalify this as a documentation issue because comparing dates with datetimes is probably a common error and I don't think the docs warn against it. I don't know where the warning should be added, though.

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