postgresql-simple icon indicating copy to clipboard operation
postgresql-simple copied to clipboard

Missing instance FromField NominalDiffTime

Open fizruk opened this issue 9 years ago • 7 comments

There is a ToField instance, but FromField is missing.

fizruk avatar Feb 08 '16 17:02 fizruk

There's actually a reason for that: you can't really blindly convert months to days, or days to hours/minutes/seconds, without further context regarding the specific time and location you are talking about.

Basically, NominalDiffTime is a proper subset of postgresql's interval type (modulo precision and range issues), and interval is a strict superset of NominalDiffTime.

lpsmith avatar Feb 08 '16 20:02 lpsmith

That said, I have considered adding a FromField NominalDiffTime instance that assumes any intervals can be converted to seconds directly; any intervals that contain non-convertible components would result in an exception.

lpsmith avatar Feb 08 '16 21:02 lpsmith

Hmm, maybe interval is not the right field type for NominalDiffTime time then?

I've also noticed that you don't have instances for Data.Fixed, is there a reason for that too?

If not, I could use ToField/FromField instances of Pico to store NominalDiffTime.

fizruk avatar Feb 09 '16 11:02 fizruk

My apologies for forgetting to respond to this issue.

I guess, what's your goal here? If all you care about is storing NominalDiffTimes, and getting them back out with full fidelity, and you don't care about the semantics or having postgresql perform computations on them server-side, then you would have a few options. However, these options probably would not be a good fit for inclusion in vanilla postgresql-simple.

If you care about semantic similarity, which is the primary goal of vanilla postgresql-simple, then what option is there other than interval? A postgresql interval corresponds to data Interval = Interval { months :: Int32, days :: Int32, microseconds :: Int64 }, whereas a NominalDiffTime is an Integer of picoseconds. So, there's a lot of overlap, and NominalDiffTime is almost a subset of interval, but each type can represent some values the other can't.

I've also noticed that you don't have instances for Data.Fixed, is there a reason for that too?

Not that I'm aware of.

lpsmith avatar Mar 15 '16 19:03 lpsmith

@lpsmith Is there any reason not to include your Interval ADT in this library with the appropriate ToField/FromField conversions? It sounds like that's a pretty safe addition from a correctness perspective and it would allow first-class two-way conversion of Intervals <-> Haskells.

SamProtas avatar Jul 01 '17 16:07 SamProtas

I think it would be a worthy addition to postgresql-simple, and I would welcome a pull request of reasonable quality.

lpsmith avatar Jul 01 '17 18:07 lpsmith

In addition, I would probably accept a pull request that adds a FromField instance for NominalDiffTime, as well as Fixed.

Though I may have looked into supporting Fixed once, and was a bit scared off by some unfortunate ways postgres's numeric type works. My memory is a bit fuzzy on that count.

lpsmith avatar Jul 01 '17 18:07 lpsmith