Change executed raw mysql queries to return timezone aware datetimes.

[Bryce Drennan]

Request

Change executed raw mysql queries to return timezone aware datetimes.

Explanation

Django 1.9 changed so that raw queries executed in mysql do not return timezone aware datetimes. Removing this feature now requires that all custom SQL queries involving datetimes have special handling to convert the datetimes to timezone aware datetimes.

The explanation for this was that the conversion was happening globally and having adverse affects on 3rd parties. There was also a desire to allow per-db timezone settings.

While sqlite converters are registered globally, mysql converters appear to be registered per connection. I'm not sure why mysql was included in this change.  It should be possible instead to use the per db TIMEZONE setting at the connection level for mysql.  

I read through the code and all related to discussion and must confess I'm still not sure I understand the reasoning for this change happening in mysql.  I'm posting here hoping to understand better why the mysql connection datetime behavior doesn’t key off the per db TIMEZONE setting and global USE_TZ settings.

In our local codebase we plan to monkeypatch django to continue returning timezone aware datetimes for mysql, thus saving us from manually fixing many many queries.  Is this a bad plan? Is there a better way?

Thank you,

Bryce Drennan

Senior Engineer @ Circleup

References

1. https://code.djangoproject.com/ticket/19738 - "manage.py shell" on a fresh project raises RuntimeWarning about naive datetime, if IPython is installed

2. https://code.djangoproject.com/ticket/23820 - Make it possible to override USE_TZ & TIME_ZONE on a per-database basis

3. https://github.com/django/django/pull/4601 - Refactor adaptation and conversion of aware datetimes

4. https://groups.google.com/forum/#!searchin/django-developers/datetimes%7Csort:date/django-developers/Ci_cs0KTagM/iAduocfXFOcJ - Discussion of removing timezone conversion

5. http://mysql-python.sourceforge.net/MySQLdb.html

[Aymeric Augustin]

Hi Bryce,

#19738 is unrelated to the problem you’re describing. It was fixed in d9521f66 which removed adapters i.e. transformations of parameters passed to the database.

You’re talking about the removal of converters i.e. transformations of values returned by the database. It is discussed in #23820 and was committed in ec186572.

When I implemented time zone support in Django 1.4, conversion of values could happen in several places. This was improved in Django 1.8 with e9103402.

This unified approach to conversions allowed to expand the functionality of the ORM which now tracks the type of returned values, even computed ones.

That said, this design cannot work well for raw queries because Django doesn’t know the type of returned values, so it cannot apply a suitable converter. You get whatever the DB-API module gives you. In a sense, this behavior is sensible: Django shouldn’t interfere with values provided by raw queries.

This is where I introduced the backwards incompatibility you’re talking about:

https://github.com/django/django/commit/ec186572e6cfde4cd4bc1491ff552c5d32211d9f#diff-dbe1511db54b8fff2cb72a199cf37b4cL90

(By the way, this is where MySQL converters used to be registered globally. I’m not sure how you came to the conclusion that they were registered per connection.)

The thread on this mailing list you linked to describes clearly how I ended up there:

https://groups.google.com/d/msg/django-developers/Ci_cs0KTagM/DchJIB8-vRsJ

I acknowledged the backwards incompatibility in the releases notes:

https://github.com/django/django/commit/ec186572e6cfde4cd4bc1491ff552c5d32211d9f#diff-c0f4f3a0c26f12ebc2e41904af710f4dR421

I wrote in a later commit that values returned by .raw() wouldn’t be affected:

https://github.com/django/django/commit/d9521f66b1851b0eacd55bc78f801dc64123e333#diff-c0f4f3a0c26f12ebc2e41904af710f4dR456

I’d like to make sure we agree on what your problem is exactly before investigating further. When you say "raw mysql queries”, do you mean MyModel.objects.raw(…) or cursor.execute(…)?

The former would mean that I mixed up converters and adapters when I wrote the release notes. That would be annoying :-(

The latter is the documented behavior, which doesn’t preclude discussing improvements, but at least means things work as intended.

As mentioned by Carl in the mailing list thread, if you’re having a problem with `cursor.execute()`, the easiest solution is to reinstall yourself the same global converter Django used to install. Is that what you meant by “monkey-patching Django”?

-- 

Aymeric.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/c7305bb1-097b-439b-8027-cb91f85c15aa%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Bryce Drennan]

I'm wondering if we're using the term "global" slightly differently.  

Yes, prior to the changes removing converters, Django had these converters globally applied to all mysql connections that it created. However, they didn't apply to mysql connections created outside of django, nor did django have to implement the converters globally like it did. Timezone support could be implemented at the connection level instead of globally being applied to all mysql connections.  

I have made this proof of concept to illustrate two distinct mysql connections, one which adds the timezone conversion, and one which does not: https://gist.github.com/brycedrennan/fc5f1cb08afe6dba034e3185368a7e2a/revisions

It seems reasonable to me that connections created by Django would have Django specific functionality (like returning timezone aware datetimes).  If a specific DB should not have this functionality then we could setup the converter to not be applied to those connections.  If specific non-django code requires naive datetimes then it can use a non-django db connection. Does that make sense? I am not super familiar with the django codebase so I worry I'm missing something.

[Bryce Drennan]

Also to directly answer the question, I'm talking abour cursor.execute()

[Aymeric Augustin]

Hello Bryce,

Oops! I didn’t realize that conversions were a parameter of the database connection on MySQL :-( So, yes, it is technically possible to implement the solution you’re describing.

However I’m not positive that we should make this change. There’s an argument to be made that database settings should only affect the ORM and other Django components.

`from django.db import connection; connection.cursor()` is supposed to give you a database cursor using the credentials from your settings file, but apart from that, it’s just a regular database cursor.  Django’s wrapper is expected to be transparent. I believe it’s important, as much as is practical, to preserve the ability to interact with the database without interference from Django.

Also reverting backwards-incompatible changes in the following release is painful for users… Even if the first backwards-incompatible change is a sunk cost at this point, reverting it would be particularly vexing for affected users who updated their code.

To sum up, the question we’re reaching is:

“When USE_TZ =True, should cursor.execute() with a Django cursor return (a) naive datetimes like a plain MySQLdb cursor (b) aware datetimes in UTC like the Django ORM?"

Who has an opinion on this question?

Best regards,

-- 

Aymeric.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/11f2190b-5300-4e98-9cd9-4761a1a45cfb%40googlegroups.com.

[Anssi Kääriäinen]

I'm mildly in favor of b), seems like most projects would want that anyways.

If there is a really easy way for picking tz conversions per cursor,
then that could be a good approach, too. Something like cursor =
connection.cursor(convert_tz=True) might work. I know then MySQL would
have a bit different API than the rest of the backends for cursor
creation, but the SQL executed is going to be backend specific in any
case...

- Anssi

> https://groups.google.com/d/msgid/django-developers/F8C0C443-CDE7-40F3-829B-F444B76697AB%40polytechnique.org.

[Bryce Drennan]

Would a per-database setting "TZ_AWARE_DATETIMES" help address the backwards compatability issue? It would default to True but would allow those who find the migration painful to do that on their own schedule.