time.Time support

[robfig]

I'm not sure if this is something you want to support, but Sqlite (and presumably other databases) does not support a formal datetime type. It should be possible to .Format() and .Parse() the time.Time field from a String type. I'm not sure how this would be configured, though, or if it's too niche of a feature.

FWIW, I tried to implement time.Time support in the go-sqlite3 driver, but I couldn't find a way to do so within the constraints of database/sql

[coopernurse]

It's a good idea. I'm curious if this is mostly an issue for the driver implementations though. For example, mymysql appears to support Scan() into time.Time:

https://github.com/ziutek/mymysql/blob/master/godrv/driver.go

Do you see any place where gorp would need to handle this type specially? It seems we could pass it through like all other types and let the driver Scan into it. Perhaps the sqlite driver doesn't support it and we should fork/patch that?

[robfig]

Yes, mysql supports it, but sqlite does not.

Before opening this issue, I tried to create a patch to go-sqlite3 that could allow Scanning into a time.Time, but I found it was not possible because sqlite does not have a real datetime type. When scanning, we only have the driver-provided type, which is one of the values you can see in sqlite3.h: (SQLITE_INTEGER, _FLOAT, _BLOB, _NULL, _TEXT). At the driver level, it appeared to be impossible to conditionally produce a time.Time value only if the caller is scanning into a time.Time, and a string otherwise.

This does seem like a case that applies to sqlite but not mysql, postgres, oracle, etc, so I'm not sure if you want to provide a fix in Gorp, but it could be useful since sqlite is pretty common.

The changes to support it in Gorp seem to be:

• Using the "datetime" column type for time.Time struct fields. (Which is a valid sqlite type even though it's just sugar for string, I think)
• When scanning into struct fields, notice that the destination struct field is a time.Time and instead scan into a string, which you then time.Time.Parse() into the time.Time.

(I believe that queries that pass a time.Time as an argument may already work)

[chrisfarms]

The Postgres pq driver supports time.Time in Scan ... it would be a nice simple addition to the PostgresDialect#ToSqlType type switch statement to add something like:

    case "Time":
        return "timestamp"

which should allow CreateTables to do it's thing

[sqs]

FWIW, I have been using the patch from above with success (with PostgreSQL).

@coopernurse, adding this would be somewhat backwards incompatible (because CreateTables would use a different schema when people update gorp). Is that OK, or do you have any ideas on how to mitigate it?

[ottob]

Yep. Same here: ottob@e237b53

[coopernurse]

I think that change makes sense. Honestly I'm not sure gorp has explicitly defined behavior for time.Time types. When I first wrote it, Go1 wasn't out yet, and time.Time didn't exist.

Perhaps we should just start a "breaking changes" section of the README and make it clear when we add something like this. Every once in a while we'll need to break current behavior for the sake of progress/sanity.

[miafoo]

I had the same issue, but with MySQL. Applying @chrisfarms' patch under the MySQLDialect solved it for me.

edit: The fix only works with MyMySQL and not Go-SQL-Driver, not sure why.
edit2: This might be related? https://github.com/go-sql-driver/mysql#timetime-support

[coopernurse]

I tried modifying the Postgres and Mysql dialects today and wrote a trivial test for it. This test passes for postgres, but fails for mysql. It appears the timezone gets set to the local timezone instead of UTC. Anyone have an opinion on why this would occur?

I don't want to commit this patch until I understand why a trivial insert/select isn't symmetric.

func TestWithTime(t *testing.T) {
    dbmap := initDbMap()
    defer dbmap.DropTables()

    t1, err := time.Parse("2006-01-02 15:04", "2011-01-19 22:15")
    if err != nil {
        panic(err)
    }

    w1 := WithTime{-1, t1, "hi"}
    _insert(dbmap, &w1)

    obj := _get(dbmap, WithTime{}, w1.Id)
    w2 := obj.(*WithTime)
    if w1.Date.UnixNano() != w2.Date.UnixNano() {
        t.Errorf("%v != %v", w1, w2)
    }
}

Output from mysql:

--- FAIL: TestWithTime (0.05 seconds)
gorp_test.go:875:   {1 2011-01-19 22:15:00 +0000 UTC hi} != &{1 2011-01-19 22:15:00 -0800 PST hi}
FAIL
exit status 1

[coopernurse]

To clarify, for Mysql I mapped time.Time to datetime. The create table command was:

gorptest: 14:46:59.983278 create table `time_test` (`Id` bigint not null primary key auto_increment, `Date` datetime, `Name` varchar(255))  engine=InnoDB charset=UTF8; [[]]

[adam-frisby]

"MySQL converts TIMESTAMP values from the current time zone to UTC for storage, and back from UTC to the current time zone for retrieval. (This does not occur for other types such as DATETIME.)"
http://dev.mysql.com/doc/refman/5.0/en/datetime.html

You want to use TIMESTAMP instead of DATETIME for MySQL.

PostgresSQL should use "timestamp with time zone"

"For timestamp with time zone, the internally stored value is always in UTC (Universal Coordinated Time, traditionally known as Greenwich Mean Time, GMT)"
http://www.postgresql.org/docs/9.1/static/datatype-datetime.html

[robfig]

Hm, I'm not sure that's relevant here -- the snippet you quote for MySQL seems like a detail of how the TIMESTAMP is stored. For example it says that MySQL does the conversion on its end, not that the user has to. For both TIMESTAMP and DATETIME, if you store a value and then read it back, you should read exactly what you wrote. (That's what @coopernurse means about being symmetric).

As to what the issue is, I don't have an idea just from reading this thread. I would have guessed that it is an unwanted timezone conversion happening on the Go side in time.Time (I've used it a lot and still don't feel like I have a good grasp of how it handles timezones), but the Postgres test works properly, so I'm not sure.

I can potentially debug it later this week (busy with the day job though)

[sqs]

BTW, for PostgreSQL, I would suggest mapping pq.NullTime to timestamp as well:

I.e.,

--- a/dialect.go
+++ b/dialect.go
@@ -170,7 +170,7 @@ func (d PostgresDialect) ToSqlType(val reflect.Type, maxsize int, isAutoIncr boo
                return "smallint"
        case "NullableBytes":
                return "bytea"
+       case "Time", "NullTime":
+                return "timestamp"
        }

[seehuhn]

Hi,

Not sure if this is relevant (and my apologies if it is not), but just for the record: the first post here seems to indicate that sqlite3 is not able to store time.Time objects natively. Did I understand this right? If so, this problem seems to be fixed in the mean time. The following program successfully stores a time.Time value in an sqlite3 database for me: https://gist.github.com/seehuhn/6197736 .

Timezone information is lost (i.e. the result is always int UTC), but the result after retrieval denotes the same point in time, i.e. .UnixNano() returns the same value.

All the best,
Jochen

[coopernurse]

merged to master and released in v1.1