Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing instance FromField NominalDiffTime #176

Open
fizruk opened this issue Feb 8, 2016 · 7 comments
Open

Missing instance FromField NominalDiffTime #176

fizruk opened this issue Feb 8, 2016 · 7 comments

Comments

@fizruk
Copy link

fizruk commented Feb 8, 2016

There is a ToField instance, but FromField is missing.

@lpsmith
Copy link
Owner

lpsmith commented Feb 8, 2016

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
Copy link
Owner

lpsmith commented Feb 8, 2016

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.

@fizruk
Copy link
Author

fizruk commented Feb 9, 2016

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.

@lpsmith
Copy link
Owner

lpsmith commented Mar 15, 2016

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.

@SamProtas
Copy link

@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.

@lpsmith
Copy link
Owner

lpsmith commented Jul 1, 2017

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

@lpsmith
Copy link
Owner

lpsmith commented Jul 1, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants