Implement a NullTime type

gawry commented 9 years ago

When using gorp or other ORMs it would be nice to be able to do the same with the Time type

doxavore commented 9 years ago

I would definitely use this. pq's NullTime stops where database/sql types do, in that they won't marshal nicely.

If such a PR was opened, would it be accepted? Or is there some underlying reason this hasn't been implemented?

guregu commented 9 years ago

There's a few reasons I haven't accepted that PR #6

I'm not sure it works in non-Postgres environments (but is probably OK)
Even if it does, do we want to depend on pq?
It does too much, this library doesn't need two NullTimes including one that marshals into Unix second timestamps. Just RFC3339Nano is fine.
The naming is confusing. The TIME type in SQL has no date, it's just the time of day, Go's Time type is more like TIMESTAMP, further adding confusion to the current PR which uses NullTime for RFC3339 and NullTimestamp for Unix time.

Regarding the last concern, it's not like there's a String type in SQL, so NullTime fits with the current NullString and would be fine.

Basically, with a little clean up it would be OK

guregu commented 9 years ago

Added in #9, release v2.2

guregu / null