Django

Code

Ticket #8354 (closed: fixed)

Opened 4 months ago

Last modified 3 months ago

MySQL rejects datetimes with unusual unicode() representations

Reported by: jmillikin Assigned to: nobody
Milestone: 1.0 Component: Database layer (models, ORM)
Version: SVN Keywords: 1.0-blocker
Cc: Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 1 Patch needs improvement: 1

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

mysql-datetime.diff (0.6 kB) - added by jmillikin on 08/15/08 16:48:11.
failing-test.patch (0.6 kB) - added by jacob on 08/31/08 16:45:41.

Change History

08/15/08 16:48:11 changed by jmillikin

  • attachment mysql-datetime.diff added.

08/15/08 17:01:41 changed by mtredinnick

  • needs_better_patch set to 1.
  • needs_docs changed.
  • stage changed from Unreviewed to Accepted.
  • needs_tests changed.
  • milestone set to 1.0.

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

08/15/08 18:08:25 changed by julianb

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.

08/31/08 15:56:57 changed by jacob

  • keywords set to 1.0-blocker.
  • needs_tests set to 1.

This looks fine, but we need tests for this.

08/31/08 16:45:41 changed by jacob

  • attachment failing-test.patch added.

08/31/08 16:48:24 changed 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.

08/31/08 17:04:49 changed by julianb

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.

09/01/08 12:06:38 changed by mtredinnick

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

09/01/08 12:49:20 changed by jacob

  • status changed from new to closed.
  • resolution set to fixed.

Fixed in [8802].

Add/Change #8354 (MySQL rejects datetimes with unusual unicode() representations)




Change Properties
Action