Fix storage of time.Time objects with non-local timezones in DATETIME columns.

[seehuhn]

Storing time.Time objects with non-local timezones in DATETIME columns didn't work with the mymysql driver: when the value was stored, timezone information was simply ignored; when the value was retrieved, it was assumed to be local time. This commit fixes the problem, by converting all time.Time objects to the local timezone before sending them to the MySQL server.

I've included a test case to illustrate the problem and to verify that the problem is fixed by my change.

[ziutek]

This is known MySQL issue. The problem is that DATETIME type doesn't contain any information about timezone. Your patch fixes your problem but introduces issues in code that relies on current godrv behavior.

Current behavior:

1. Store time.Time in current presentation format without timezone information, so what is presented by database is equal to what is presented by time.Time.Format.
2. Retrieve DATETIME as it was in local timezone. Using local timezone instead of eg. UTC is arbitrary decision but still presentations are equal.

Unfortunately database/sql doesn't provide any functionality to specify timezone for retrieved dates. I think that for now I can add some workaround to the godrv to specify default timezone, other than local. This doesn't solve the problem but maybe useful for most applications and doesn't break any of existing ones.

mymysql native interface does this better.

[seehuhn]

Dear Michał,

On 13 Aug 2013, at 13:45, Michał Derkacz [email protected] wrote:

This is known MySQL issue. The problem is that DATETIME type doesn't contain any information about timezone. Your patch fixes your problem but introduces issues in code that relies on current godrv behavior.

Current behavior:

• Store time.Time in current presentation format without timezone information, so what is presented by database is equal to what is presented by time.Time.Format.

• Retrieve DATETIME as it was in local timezone. Using local timezone instead of eg. UTC is arbitrary decision but still presentations are equal.

The problem with this is that on storage the information about timezone is just dropped, and on load the timezone is assumed to be the local timezone. Result: if you store a date with a non-local timezone and read it back, you get a time.Time object representing a different point in time (i.e. the .UnixNano values are different). What I would expect is, that only information about the timezone is lost, but that I get back the same point in time. The unit test I added checks for this.

Unfortunately database/sql doesn't provide any functionality to specify timezone for retrieved dates. I think that for now I can add some workaround to the godrv to specify default timezone, other than local. This doesn't solve the problem but maybe useful for most applications and doesn't break any of existing ones.

Having a way to tell mymysql which timezone the values in the database should be stored in would be great! But I think this is a separate issue from the not-getting-the-right-time-back issue above.

What do you think?

All the best,

Jochen

https://seehuhn.de/

[seehuhn]

I may have misunderstood what you wrote: there are two timezones which could be customisable:

1. The timezone used for the column in the database. In my patch I follow the idea that mymysql would convert the time.Time object into this timezone before sending it to the database; an alternative would be to require the caller to convert the time.Time object to this timezone.

2. The timezone the returned time.Time object is in when it is returned from a database query.

When you wrote " I think that for now I can add some workaround to the godrv to specify default timezone, other than local", which of the two possibilities did you mean?

[ziutek]

I mean following rule:

1. Store date in database in current presentation form (simply drop timezone information). I prefer this because what you see in your program is what you see in database (but without timezone). If you want to always store date in local (or other) timezone, you have to explicitly change it location before store it.
2. Treat date retrieved from database as in some specified timezone. This timezone will be stored in some godrv global variable, set by default to the local timezone.

So if your whole application works in some timezone, set this timezone only for retrieved dates will be enough. Dates presentation will be equal in Go and in MySQL so you can easy examine dates in database using any tool.

If you have dozen of applications that work worldwide in different timezones, you need to rethink how to handle timezones correctly. I think that godrv should work transparently and doesn't try to fix this MySQL issue in any specific way. This is especially important in complicated an heterogeneous environment where database is used not only by Go programs. In such environment you need to develop some uniform standard to treat dates in MySQL and implement it in any application you use.

[seehuhn]

Hi Michał,

On 14 Aug 2013, at 11:42, Michał Derkacz [email protected] wrote:

• Store date in database in current presentation form (simply drop timezone information). I prefer this because what you see in your program is what you see in database (but without timezone). If you want to always store date in local (or other) timezone, you have to explicitly change it location before store it.

Oh, I see, that's a possible plan. An alternative would be to use a fixed time zone for the values in the database column and convert on storage retrival.

Advantage of simply dropping the timezone information:

• you keep the digits of the time unchanged

Advantages of using a fixed timezone for storage:

• you keep the point in time, i.e. UnixNano() doesn't change
• you don't loose information during the hour which happens
  twice when summer time is changed to winter time.
  Example: in the UK where I currently live, the two times
  "2013-10-27 01:30:00 +0100 BST" and
  "2013-10-27 01:30:00 +0000 GMT" happen within an hour of
  each other when summer time ends, and it would be great
  if they wouldn't map to the same value in the database.

I think both methods are valid approaches, but for my own use
I'd love to see the second approach available with mymysql.
(I store times in UTC, and it would be great if the db adapter
could handle this transparently/. Also, the problem "coopernurse"
encountered at
https://github.com/coopernurse/gorp/issues/14#issuecomment-19882220
could be solved this way.)

Would you be willing to take a patch which optionally converts
all times on storage/retrival to a user-specified database time-zone?
I'd be happy to code something up and write unit tests to prove
it works. If you are ok with this plan, do you have a favourite
way of telling mymysql whether to convert timezones and if
so which time zone to use? Or should I make up something
and then we discuss?

• Treat date retrieved from database as in some specified timezone. This timezone will be stored in some godrv global variable, set by default to the local timezone.

Yes, this sounds good to me (and local timezone is all I need
here for myself).

All the best,

Jochen

https://seehuhn.de/

[ziutek]

I think that it is possible to add optional timezone for stored datetimes. By default it will be nil, which means drop timezone (current behavior).

Default timezone can be a global godrv variable or it can be per connection variable. The first approach is easier to implement (only for godrv) but second can be more efficient and also useful for users of mymysql native interface.

Now and for next three weeks I have a holidays so I don't want to code to much. But you are welcome to send me a pull request that I'll try review if my Internet connection will be good enough.

[seehuhn]

Hi Ziutek,

I've had a go at implementing your suggestion, looking forward to hear what you think. The new pull request is in #80 and replaces the one I suggested here.

Some comments:

• I found that time conversions were scattered over many places. I am not totally sure whether I fixed them up all; if I missed something, please let me know. To get some confidence I added unit tests, and at least these tests pass without problem.
• I wasn't quite sure what kind of user interface you wanted to set the new values, so I made up my own: there is a new global variable mysql.DefaultTimeZone to set the default, and you can overwrite the default on a per-connection basis by setting the new field native.Conn.TimeZone. If you don't do anything, you get the old behaviour.
• For the godrv interface I struggled a bit to get the timezone information into the rowsRes.Next() method. I finally settled on adding a new .GetTimeZone() method to mysql.Row, and I'm also using slightly ugly type assertions in "godrv/driver.go". Can you think of a better way of doing this?

Please let me know if anything needs fixing, I'd be happy to change things if you spot anything wrong or too ugly.

All the best,
Jochen

PS.: happy holidays! (and I'll also be mostly away for the next two weeks or so)

[ziutek]

There was dozen of things that wait for me after my holidays. Now I have some time to try review your changes Jochen. I think that there isn't a good idea to add timezone information to the pktReader. I think that timezone should be handled at upper layer.

We have two function for obtain time from a row: (Row.Time, where you specify a location, and Row.Localtime, which assumes local location). I don't want to introduce next function. Instead, there can be added something like Conn.SetLocalLocation(l *time.Location) that will change location used by default.

[ziutek]

I think that this problem can't be fixed at mymysql.Conn level without broke current behavior because of mysql.Row definition. It is defined as an ordinary slice ([]interface{}) so there is no place for any timezone information for text results (text results contains only text dates/times as returned by server).

It seems to be inconvenient to use some global variable for timezone in mymysql.native because it forces users to directly use mymysql.native (or mysql.thrsafe) variables/functions, but this package is intended to be only an engine for mymysql.mysql. We can get around this problem by add mymysql.location package that will be imported by all packages that need obtain/change location but it seems to be not a good idea.

For other direction (queries sent to the server) text queries are formatted using fmt.Sprintf function. We can use reflection to find all time.Time in query parameters and change its location but this seems to be something confusing for users because they obtain different results from following commands:

c.Query("select * from T where d > '%s'", t)

q := fmt.Sprinf("select * from T where d > '%s', t)
c.Query(q)

I think that we should concentrate to found solution only for godrv which doesn't provide any functionality for different timezones and leave current behavior in mymysql. It probably needs some support from mymysql.native to not sacrifice performance.

[seehuhn]

Just to let you know: term started and I'm to busy at the moment to look into this. Maybe somebody else can have a go? Or maybe I'll get back to this when times are quieter ...

[RickyS]

In the worst case the application knows about a timezone variable it stores in the database. The app may need to be conscious of this anyway for user forms. The app has to get the user (customer) location somehow. There is a net-wide timezone database that can be used that is reliable.
See https://en.wikipedia.org/wiki/Tz_database

Store UTC time, ignoring Daylight saving time (summer time) and store the timezone, and maybe store summer time rules. Do this automatically as is easy to implement, document it and give examples of usage.

Don't even think about Antarctica or the Mars colonies. Maybe we'll be dead by the time the Mars colonies get going. If there is more than one colony, make sure they're in the same timezone.

Better yet, beat on Oracle and the Mariadb people to fix their damn database. Rubber hoses only, please. The fools have Unicode but not timezones — do they think the earth is flat?

[RickyS]

Also:

• See this timezone movie:
• When I made the previous comment, after my name 'RickyS', it said
  "commented in a few seconds". And then switched to
  "commented just now", as I watched.
  MySql isn't the only software with time problems.