Timezone support

[liwp]

(We're using mymysql via the database/sql interface)

We're having trouble with timestamps with timezones that are different from the local timezone. We're trying to write a UTC timestamp to the DB (eg time.Now().UTC()), but when reading it back we get a timestamp that has the expected time (eg. input time is 12AM and output time is 12AM), but the timezone is the local timezone rather than UTC. Therefore the input and the output timestamps do not represent the same moment in time any more.

Example:

IN: 2015-10-10 04:34:56 +0000
Actual OUT: 2015-10-10 04:34:56 +0200
Expected OUT: 2015-10-10 06:34:56 +0200

I understand that MySQL doesn't store the timezone internally, so the best mymysql can do when reading from MySQL is to take the value read from the DB and attaching the local timezone to the time.Time instance. But it seems to me that when writing to the DB, mymysql should convert the timestamp to local time first instead of just assuming that the timestamp is local time.

Looks like the fix would be to modify parseQuery to call time.Time.Local() on the given timestamp before encoding the timestamp for MySQL. Eg:

func (c conn) parseQuery(query string, args []driver.Value) (string, error) {
...
    for _, a := range args {
...
        switch v := a.(type) {
...
        case time.Time:
            v = v.Local() // <=== add time line
            s = "'" + v.Format(mysql.TimeFormat) + "'"
...
}

This way the contract between mymysql and MySQL is that timestamps are written to and read from MySQL in the local timezone. And the contract between mymysql and my application is that timestamps are converted to local time before writing them to MySQL and their provided as local time when reading them from MySQL.

Timezones are hard, so I hope my diagnosis of the problem is correct and I've explained the problem we're facing in clear enough detail. Thanks for a great project!

P.S. I also found EncodeTime that serialises timestamps so maybe that needs to be updated as well.

P.P.S. I think it's also worth considering whether the timestamps should be written to MySQL as local time or as UTC. Obviously changing to storing them in UTC would be a backwards incompatible change, but maybe it can be hidden behind some sort of switch or included in the next major release.

[ziutek]

1. MySQL doesn't store timezone as you say.
2. Date/Time can be freely send to MySQL as text embeded i query which mymysql can't handle in any way.

Current behavior is that if time T is sent to MySQL in binary form, and next you call T.Format using MySQL layout and read T from MySQL database in text form, you should obtain the same text representation of T.

Of course returned time will be always in Local timezone and this is arbitrary and not the most fortunate decision. Bu It works well in most cases.

I think the simplest solution will be add some global variable in godrv that user can set to timezone he want in results.

[ziutek]

godrv.SetLocation was added to devel branch. Try it.

[liwp]

Thanks for looking into this @ziutek !

I'm not sure how useful SetLocation will be. I can get the same effect by running my app with the TZ env var set to a specific timezone. Maybe setting the env var isn't convenient in some environments and there SetLocation would help...

In any case, SetLocation doesn't fix my issue: we still end up writing a UTC timestamp to MySQL, assuming that it's local time.

By adding v = v.In(location) above the Format call fixes the issue for me by translating the UTC timestamp to the timezone specified by location (my initial issue report suggested calling v = v.Local()).

Here's a test case (I should've included this when I opened the issue - sorry). This test fails for me in the devel branch unless I add v = v.In(location) to godrv as described above.

package main

import (
    "database/sql"
    "log"
    "testing"
    "time"

    "github.com/ziutek/mymysql/godrv"
)

func checkErr(err error) {
    if err != nil {
        log.Fatalf("Error: %v", err)
    }
}

func TestTimezone(t *testing.T) {
    // NOTE: Instead of calling `SetLocation` we could run the test with `TZ=Europe/Helsinki go test` for the same effect
    loc, err := time.LoadLocation("Europe/Helsinki")
    checkErr(err)
    godrv.SetLocation(loc)

    db, err := sql.Open("mymysql", "/root/")
    checkErr(err)
    defer db.Close()

    _, err = db.Exec("CREATE DATABASE IF NOT EXISTS TimezoneTestDb")
    checkErr(err)
    _, err = db.Exec("USE TimezoneTestDb")
    checkErr(err)
    _, err = db.Exec("CREATE TABLE IF NOT EXISTS Test (timestamp DATETIME)")
    checkErr(err)
    _, err = db.Exec("DELETE FROM Test")
    checkErr(err)

    // NOTE: we specifically want to write a Time to the DB that is not in the `location` timezone.
    in := time.Now().UTC()
    log.Printf("in:  %v", in)
    _, err = db.Exec("INSERT INTO Test (timestamp) VALUE (?)", in)
    checkErr(err)

    var out time.Time
    err = db.QueryRow("SELECT timestamp from Test").Scan(&out)
    checkErr(err)
    log.Printf("out: %v", out)

    _, err = db.Exec("DROP DATABASE TimezoneTestDb")
    checkErr(err)

    // NOTE: The MySQL datetime format truncates the timestamp to seconds, so we have to do the same
    if !in.Truncate(time.Second).Equal(out) {
        t.Fail()
    }
}

[ziutek]

In my opinion we should avoid fix SQL language in client library.

SQL doesn't store TZ info in DATETIME. time.Time has this information, so we always lost it if we store Time in DATETIME.

With good approximation we can tell that DATETIME can only store text representation of Time without zone part.

We can deal with this in two ways:

1. Store current text representation of Time in its current timezone, so you can see in database what you can see from log.Println(Time).
2. Implicitly change timezone of Time before store it.

My point is that we should avoid implicit conversions, so the following two inserts have the same result:

s := fmt.Sprintf(
    "INSERT INTO Test (timestamp) VALUE ('%s')",
    t.Format("2006-01-02 15:04:05.000000000"),
)
db.Exec(s)
db.Exec("INSERT INTO Test (timestamp) VALUE (?)", t)

If you want to store DATETIME representation of t, for example in UTC timezone, explicitly use t.UTC(). This way you always known what representation was stored in database.

Whatever you do, you have to keep in mind that you are working with imperfect data type: DATETIME.

v = v.In(location) in parseQuery() fixes only simple text queries. You should add it to run() method to work with prepared statements.

But even that, this is still inconsistent fix because it doesn't handle for example such simple case:

db.Exec("INSERT INTO Test (timestamp) VALUE (?)", "2015-12-04 20:54")
db.Exec("INSERT INTO Test (timestamp) VALUE ('2015-12-04 20:54')")

What should be stored in database in above example? And why?

Many users of this library want to obtain the same result in database irrespective of type used to store in DATETIME. Implicit conversion can miss some special cases, so it isn't definitive fix. Therefore, is it worth to implement?

Reading DATETIME from database is easier. We have no any information about timezone so every choice is good.

SQL isn't strongly typed language, hence our problems.

[liwp]

Thanks for taking the time to look into this, Michał.

I hadn't considered the literal SQL statements, so it does look indeed that my suggestion to convert the timestamps to 'local' time before writing to the DB won't work.

Like you say, we lose information when using the DATETIME type and some sort of hack is necessary for working around this issue (convert to a known timezone; store the timezone in the DB; ...). Similarly, since golang's time.Time requires a timezone, mymysql has no other option than to set it to a timezone.

I did really object to current driver behaviour which I felt was asymmetric, but it looks like it's the best we can do in the driver.

I'll run our app in the UTC timezone and all will be well. I think you can probably close this issue then.

Thanks again for your efforts!