database/sql: "Format3339Nano" not implemented; SQL TIMESTAMP -> Go Time doesn't work

[John-Nagle]

What version of Go are you using (go version)? go version go1.6.2 linux/amd64

Does this issue reproduce with the latest release? Yes

What operating system and processor architecture are you using (go env)? Ubuntu 16.04LTS

What did you do?

Go manual says at "https://golang.org/pkg/database/sql/#DB.Query":

Source values of type time.Time may be scanned into values of type *time.Time, *interface{}, *string, or *[]byte. When converting to the latter two, time.Format3339Nano is used. 

Format3339Nano doesn't seem to be documented anywhere, although it's mentioned in places that copy this SQL interface documentation. It's undefined in "time". Apparently that feature was never implemented.

Time.time should have
const Format3339Nano = "2006-01-02 15:04:05" // reference to RFC 3339
to match the documentation.

So, reading Time items doesn't work out of the box:

row := db.QueryRow("SELECT tripid, stamp FROM tripstodo WHERE TIMESTAMPDIFF(SECOND, stamp, NOW()) > ? ORDER BY stamp LIMIT 1", minSummarizeSecs)
var tripid string   // trip ID to be processed
var stamp time.Time // timestamp
err := row.Scan(&tripid, &stamp)

where "stamp" is of time MySQL TIMESTAMP, produces the error
*sql: Scan error on column index 1: unsupported Scan, storing driver.Value type []uint8 into type time.Time

The MySQL database module has an obscure variable to turn on a workaround: https://github.com/go-sql-driver/mysql#parsetime

but SQLite does not, according to this bug report from another package: https://github.com/jinzhu/gorm/issues/1361

This should Just Work.

[andybons]

I believe it should be: time.RFC3339Nano, which is defined in the time package as "2006-01-02T15:04:05.999999999Z07:00", not "2006-01-02 15:04:05"

This is, in fact, what it uses when it encounters a *string, *[]byte or *RawBytes: https://golang.org/src/database/sql/convert.go#L260

But this seems separate from your issue of

var stamp time.Time // timestamp
err := row.Scan(&tripid, &stamp)

which shouldn’t hit the logic above.

Given this, what fix are you suggesting? (aside from correcting the documentation?)

[John-Nagle]

RFC3339Nano is different from the format used by MySQL. Error message from trying to convert a MySQL TIMESTAMP field to a Go time.Time using an explicit call to "time.Parse(time.RFC3339Nano, stamp)"

parsing time "2018-03-26 14:08:01" as "2006-01-02T15:04:05.999999999Z07:00": cannot parse " 14:08:01" as "T"

RFC3339Nano requires a "T" before the time. MySQL has a space. So that's a no-go workaround.

This is, in fact, what it uses when it encounters a *string, *[]byte or *RawBytes: https://golang.org/src/database/sql/convert.go#L260

The Format side (Go time.time -> SQL) is OK. It's the Parse side that's the problem.

Here's the real problem. Each SQL driver sends over the connection to the server some binary time format, which is supposed to be converted to the client platform (Go) time format down at that level. There's a little-known mode switch to enable this. See

https://github.com/go-sql-driver/mysql/blob/bc14601d1bd56421dd60f561e6052c9ed77f9daf/packets.go#L783

To make it work right, add "

?parseTime=true"

to the end of your DSN when opening the MySQL connection. Then, MySQL TIMESTAMP values convert to time.Time values properly. This is off by default. So, by default, the MySQL driver turns a TIMESTAMP into a string, The generic SQL driver has no parsing function to turn that into a time.Time.

Now, if you turn that switch on, TIMESTAMP to time.Time conversions work fine. But TIMESTAMP to Go "string" conversions then work differently. Now you get "2018-03-26T14:36:56Z" delivered into the Go string instead of "2018-03-26 14:08:01". So, if you turn that string on by default, it will probably break code that's expecting "2018-03-26 14:08:01" That's probably why somebody stuck in that mode switch.

So you're screwed. If you fix it right, you'll break code that's compatible with the legacy format.

There was discussion and a patch to fix all this back in 2016. See https://go-review.googlesource.com/c/go/+/18935 See that for background.

At least document this properly so it doesn't take two hours to figure it out. And tell the Gorm people, who've hit this problem.

[gopherbot]

Change https://golang.org/cl/102607 mentions this issue: database/sql: fix docs to correctly refer to time.RFC3339Nano

[andybons]

Apologies as I’m still not clear on what documentation you’d like to see in Go’s sql package. It seems to me that this behavior is driver-specific and, as such, should be documented in the driver code.

perhaps @bradfitz can chime in.

[John-Nagle]

TIMESTAMP and DATETIME should always convert to time.Time properly for all databases. No options should be needed to get that behavior. What happens on conversion to "string" is a separate issue.

[andybons]

I see. Thanks for being patient with me. So more concrete guidance for driver authors in these scenarios along the lines of what you said?

[John-Nagle]

The organizational dynamics of driver authors vs class-of-driver package authors I leave to you. From a user perspective, it should Just Work.

(I didn't classify this as a documentation bug. Gopherbot did. It's really a code bug. Classifier training problem?)

[andybons]

Well it was a documentation bug as you pointed out a mistake in our docs in addition to what we’re left with. Now it’s not one.

“Classifier” is far too generous a description for what GopherBot is doing ;)

[cznic]

From a user perspective, it should Just Work.

Let me please note that "Just Work" is a rather subjective term. I know of a company producing things said to "Just Work", but I am really not compatible with anything that company produces. So the quoted above has quite a different meaning to me.

[kardianos]

I'm going to throw this issue in the same bucket as #22544 and other issues.

One way to phrase this: some databases aren't picky about types (sqlite, mysql) and sometimes we return strings from a db that we want to interpret as a time. Doing this conversion generically (as we currently attempt to do) is fine for many cases, but it is more inefficient then optimal and misses edge cases.

In the next week or two I want to come up with a better Scan implementation. But it will probably be substantially different then the current implementation.

[gopherbot]

Change https://golang.org/cl/107995 mentions this issue: database/sql: add the driver.ValueScanner interface

[kardianos]

I don't think this is issue is actionable. To resolve this, we would need to resolve #22544 .