Opened 16 years ago

Closed 16 years ago

Last modified 12 years ago

#8354 closed (fixed)

MySQL rejects datetimes with unusual unicode() representations

Reported by: John Millikin Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Keywords: 1.0-blocker
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

The database adapters assume that unicode(datetime.datetime) will return a string that can be passed to the database. Unfortunately, this assumption is false. Datetime objects with timezones will be formatted as a string that can't be read by MySQL:

>>> test_date
datetime.datetime(2008, 8, 15, 21, 31, 8, tzinfo=<UTC>)
>>> print unicode(test_date)
2008-08-15 21:31:08+00:00

I've patched the MySQL database adapter to use datetime.strftime() instead of unicode (patch attached). This sort of patch should probably be adapted to other database adapters, so they have more consistent datetime formatting.

Attachments (2)

mysql-datetime.diff (651 bytes ) - added by John Millikin 16 years ago.
failing-test.patch (623 bytes ) - added by Jacob 16 years ago.

Download all attachments as: .zip

Change History (10)

by John Millikin, 16 years ago

Attachment: mysql-datetime.diff added

comment:1 by Malcolm Tredinnick, 16 years ago

milestone: 1.0
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Should certainly be tested on other database backends to work out what the right backend-specific solution is in each case. However, throwing away timezone information when provided is a not nice as a general rule. For MySQL we have to do it, since they don't have a proper datetime field. For PostgreSQL (and others) it would be interesting to check if passing in a timezone-enabled datetime is accepted by the database even for non-timezone-aware datetime fields. That way when we do add proper timezone handling in that area down the track, we won't need to track down lots of problems with throwing away timezones.

I'm wondering if we should convert timezone-enabled datetimes to the equivalent time in the server's specified timezone, however. The "right" solution here needs a little thought, but the current behaviour is probably sub-optimal (crashing usually is).

comment:2 by Julian Bez, 16 years ago

I think for MySQL it needs to be converted. MySQL uses the server time zone by default, but it can be changed per-connection. However, the current time zone setting does not affect values in DATE, TIME, or DATETIME columns, but we could assume that these are in the time zone set in Django settings. So we would need to convert from the specified zone to TIME_ZONE from settings, if I'm not mistaken.

comment:3 by Jacob, 16 years ago

Keywords: 1.0-blocker added
Needs tests: set

This looks fine, but we need tests for this.

by Jacob, 16 years ago

Attachment: failing-test.patch added

comment:4 by Jacob, 16 years ago

So I missed Malcolm's comment before -- this is indeed a bit more complicated. It's not clear if silently discarding the time zone is acceptible, or weather we should convert it. I know that revisiting datetime handling is something we need to do in the future (a few threads on django-dev have discussed this), but we need to fix this particular issue for 1.0.

I've added a failing test; I can verify that it only fails on MySQL.

I see three choices:

  • Silently discard the timezone as in the patch. Not great, but easy.
  • Silently convert TZ-aware tickets to settings.TIME_ZONE. This opens up a big can of worms.
  • Raise a ValueError there and just refuse to handle tz-aware datetimes.

comment:5 by Julian Bez, 16 years ago

Maybe raising an error would be the best thing to do for 1.0. If you work with tz-aware datetimes you could then decide for yourself what to do.

comment:6 by Malcolm Tredinnick, 16 years ago

Following discussion on IRC and in light of the limited time and the need for a more comprehensive approach post-1.0 here, we're going to make it an error to pass in tz-aware datetimes here so that the caller has to explicitly handle it themselves without us either inserting the wrong values or changing their data (which can also be wrong sometimes).

comment:7 by Jacob, 16 years ago

Resolution: fixed
Status: newclosed

Fixed in [8802].

comment:8 by Jacob, 12 years ago

milestone: 1.0

Milestone 1.0 deleted

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