Add Scan() support for time.Time

[julienschmidt]

This is Issue 9 moved from a Google Code project.
Added by 2012-07-31T17:38:51.000Z by t...@jadedpixel.com.
Please review that bug for more context and additional comments, but update this bug.

Original labels: Type-Enhancement, Priority-Medium

Original description

<b>Problem summary:</b>

A table that has datetime fields can't be read as time.Time using Scan() 

<b>Used go-mysql-version (e.g. May 26, 2012):</b>

latest

<b>MySQL-Server version (e.g. MySQL-Server 5.5 or MariaDB 5.1):</b>

Server version: 5.1.37 MySQL Community Server (GPL)

Golang: 1.0.2

<b>Used Operation System (e.g. Windows 7 64bit, Ubuntu 12.04 64bit):</b>

OSX Mountain Lion

<b>Example code which reproduces the error (optional):</b>

Test case here: https://gist.github.com/3218753

[keks]

I would appreciate this and would help if I can. Can you give me a pointer on where to look at? I'm not sure I can do it, but then, I'll try at least.

[julienschmidt]

Well, there are quite a few problems:

• MySQL has in fact two protocols for Queries: A string-based (for normal queries) and a binary protocol for prepared statements. Remember that we prepare every Query which has args internally. So to avoid duplicate code, the time.Time should be created in rows.Next (file rows.go). The column types are available in rows.content.columns[i].fieldType
• time.Time is localized (time zone), time / date values in MySQL aren't: At least we have no information about that. So we could either ask the user for the current time zone or just assume UTC, which is wrong in most cases and would lead to unexpected behavior if the user uses Time.In(), Time.Location() etc
• The driver gets no information about the destination type:
  So we don't know WHEN to return time.Time. If we would just return time.Time for every value in a column with DATETIME type, this would return an unexpected result if you just want the value as a String. According to the MySQL specification this returns a string in the format YYYY-MM-DD HH:MM:SS. If we return time.Time and the database/sql package converts this back to a string (which adds unnecessary overhead) this would return a string in the format YYYY-MM-DD HH:MM:SS +0000 UTC instead, which a) doesn't conform with the specification and b) could break existing scripts.

At this point I can't imagine a good solutions without having information about the destination type. So I guess the database/sql package needs a patch so that it gives us this information.

[julienschmidt]

For now I added an example workaround to the wiki.

[arnehormann]

I guess this requires some serious magic. For DATE and DATETIME, it probably also depends on the STRICT MODE setting and can contain invalid dates even if STRICT MODE was activated after inserting invalid dates. I think the best option is to return an error with the retrieved date in string format (so it can be salvaged if invalid dates are misused for signaling), but at least the rather common default case '0000-00-00' is problematic (that's not even covered by ALLOW_INVALID_DATES = 0). TIMESTAMP also has the '0000-00-00' issue.

[arnehormann]

About the timezone issue: you could sidestep that problem by advanced guessing (SELECT @@global.time_zone, @@session.time_zone and/or comparing the client's system time with SELECT NOW()). It would be messy - and you couldn't be 100% sure if server time and the local system time are wrong - but it should catch most timezone related problems for most users.

Still, I'd go with UTC for everything. Document it and leave the conversion to the user - he/she should know the timezone used to fill the database. Totally oblivious DB users probably wouldn't use a language like Go in the first place. Besides, UTC for everything is the only sane way to use date/time types in MySQL anyway.

[arnehormann]

I've got one more... does this help with your third issue?

Reflect example for *time.Time in interface{}

[julienschmidt]

No, sorry. This doesn't change the fact, that the driver never directly comes in contact with the dest[] slice (destination variables wrapped into interface{}). The database/sql package just asks the driver for the values and then converts + copies them.
The database/sql package needs either a way to pass the destination values along or a way to pass the dest[] slice along, in other words something like the Valuer interface but for the destination values instead of args.

[julienschmidt]

An experimental branch with Scan() support for time.Time is now available.
I'm still not sure if this is the right approach since this makes it impossible to scan DATETIME / DATE to string, uint64 & co. Therefore it breaks compatibility with previous releases.
Any thoughts?

[xaprb]

I think this would be best to implement as an optional feature, to be enabled by a feature flag in the connection string perhaps.

[coocood]

I tried the exp-time branch, it works fine. thanks.

[coocood]

When will exp-time branch merged to master?
I've used the exp-time branch with no problem.