Add support for time.Time objects to gorp.#67
Add support for time.Time objects to gorp.#67coopernurse merged 2 commits intogo-gorp:masterfrom seehuhn:datetimefix
Conversation
This commit adds support for storing and retriving time.Time objects, by passing through such objects to the underlying driver. This resolves gorp issue #14. The commit is tested and works with the github.com/mattn/go-sqlite3 driver. The commit does not currently work with the github.com/ziutek/mymysql/godrv driver, due to a bug in the mysql driver. This problem is resolved by mymysql pull request #77. I have successfully tested that with the fix from pull request #77 applied to mymysql, and with the current commit applied, gorp can correctly store and retrieve time.Time objects from mysql databases. This commit is NOT tested with the github.com/lib/pq driver.
|
With Really, the option should be left up to the user, but Gorp's column mapping doesn't support that yet. Keeping the timezone in postgres works (and without timezone doesn't), so it's the safer option for the time being if you want Time object in Gorp. Some exploration of the underlying SQL drivers might be warranted as well, since there might be other intricacies that differ between SQL drivers. I know Go-SQL-Driver/MySQL has a location setting for timezones (and a pending pull request to correct conversions). Other drivers may behave differently, especially for MySQL. |
|
At this point, gorp is already failing to handle time.Time types. I don't think it's worth holding up this pull request just to figure out the timezones problem. Users already have to work around Time compatibility on their own, they can figure out how to work around timezones if they need to. |
|
I have now updated the pull request to use "timestamp with time zone" as suggested by Joe Shaw. Since I don't have a PostgreSQL installation around, I can't test the change. Could somebody test the new pull request and see whether it all works as expected? |
|
I've created a "develop" branch. I'm going to merge into that and test locally. If tests pass I'll push this branch along with a set of other pending pull requests so we can test all of them together before merging to master. |
|
postgres and sqlite are passing now. mysql still fails, due to tz issues that @seehuhn notes. I'm ok with merging this, but I'm going to modify the gorp README to warn users about using time.Time in their mapped structs, particularly on MySQL. Since the test fails I'm also going to disable the test for now (change it to lowercase so go test doesn't run it). I'll file another issue to remind us to add it back in when ziutek/mymysql#77 lands. |
This commit adds support for storing and retriving time.Time objects,
by passing through such objects to the underlying driver. This
resolves gorp issue #14.
The commit is tested and works with the github.com/mattn/go-sqlite3
driver.
The commit does not currently work with the
github.com/ziutek/mymysql/godrv driver, due to a bug in the mysql
driver. This problem is resolved by mymysql pull request #77. I have
successfully tested that with the fix from pull request #77 applied to
mymysql, and with the current commit applied, gorp can correctly store
and retrieve time.Time objects from mysql databases.
This commit is NOT tested with the github.com/lib/pq driver.