Opened 10 years ago

Closed 10 years ago

Last modified 7 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: master
Severity: Keywords: 1.0-blocker
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: UI/UX:


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 10 years ago.
failing-test.patch (623 bytes) - added by Jacob 10 years ago.

Download all attachments as: .zip

Change History (10)

Changed 10 years ago by John Millikin

Attachment: mysql-datetime.diff added

comment:1 Changed 10 years ago by Malcolm Tredinnick

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 Changed 10 years ago by Julian Bez

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

Keywords: 1.0-blocker added
Needs tests: set

This looks fine, but we need tests for this.

Changed 10 years ago by Jacob

Attachment: failing-test.patch added

comment:4 Changed 10 years ago by Jacob

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 Changed 10 years ago by Julian Bez

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 Changed 10 years ago by Malcolm Tredinnick

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

Resolution: fixed
Status: newclosed

Fixed in [8802].

comment:8 Changed 7 years ago by Jacob

milestone: 1.0

Milestone 1.0 deleted

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