#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)
Change History (10)
by , 16 years ago
Attachment: | mysql-datetime.diff added |
---|
comment:1 by , 16 years ago
milestone: | → 1.0 |
---|---|
Patch needs improvement: | set |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 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 , 16 years ago
Keywords: | 1.0-blocker added |
---|---|
Needs tests: | set |
This looks fine, but we need tests for this.
by , 16 years ago
Attachment: | failing-test.patch added |
---|
comment:4 by , 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 , 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 , 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).
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).