Implement support for postgresql 'interval' type#60
Implement support for postgresql 'interval' type#60amarqueslee wants to merge 5 commits intohaskellari:masterfrom
Conversation
| . fromString | ||
| -- from the docs: "Beware: fromString truncates multi-byte characters to octets". | ||
| -- However, I think this is a safe usage, because ISO8601-encoding seems restricted | ||
| -- to ASCII output. | ||
| . iso8601Show |
There was a problem hiding this comment.
I decided to avoid too much logic to do with Builders and Parsers for this type, I thought it would be more maintainable in the long-term to trust the time package's out-of-the-box ISO formatter where possible.
| contents <- takeByteString | ||
| iso8601ParseM $ B8.unpack contents |
There was a problem hiding this comment.
Parser is used here to leverage its instance of MonadFail, which iso8601ParseM needs. Ideally I wanted to use some instance MonadFail (Either String) instance, but apparently this deliberately doesn't exist in base.
See https://gitlab.haskell.org/ghc/ghc/-/issues/12160 for more.
| -- | interval. Requires you to configure intervalstyle as @iso_8601@. | ||
| -- | ||
| -- You can configure intervalstyle on every connection with a @SET@ command, | ||
| -- but for better performance you may want to configure it permanently in the | ||
| -- file found with @SHOW config_file;@ . | ||
| -- |
There was a problem hiding this comment.
See https://www.postgresql.org/docs/12/datatype-datetime.html#DATATYPE-INTERVAL-OUTPUT for documentation on different interval output formats.
This was an unexpected stumbling block for me, it seems you have to ask Postgres to give you ISO-compliant intervals, with the default alternative being a vendor-specific postgres format. Searching for intervalstyle reveals that it's quite popular for developers to want to set this to something ISO8601-compliant, so hopefully this won't detract too much from adoption.
The long-term alternative is probably to support the four possible intervalstyles available, so that the user doesn't need to worry about this problem.
Also, this may be a latent issue with UTCTime as well. timestamptz also seems to offer multiple output formats, and I got the sense from browsing the parsers that this library only supports the default one. Check https://www.postgresql.org/docs/12/datatype-datetime.html#DATATYPE-DATETIME-OUTPUT .
| , template-haskell >=2.8.0.0 && <2.17 | ||
| , text >=1.2.3.0 && <1.3 | ||
| , time >=1.4.0.1 && <1.12 | ||
| , time >=1.9.0.0 && <1.12 |
There was a problem hiding this comment.
CI noticed that time only supports CalendarDiffTime from version 1.9 onwards. Since 1.9 was released in January 2018, I just bumped the version, but if using #if MIN_VERSION_time would be more appropriate, I'm open to doing that too.
phadej
left a comment
phadej
left a comment
There was a problem hiding this comment.
This definitely needs tests.
|
Don't spend time with CI (changing bounds is not good idea, and I rather fix that afterwards). Make it work where it works. Add tests. |
|
I'll shift my focus to tests, thanks. |
|
Cleaned up in #64. |
|
Released in |
|
Likewise, thanks for hoisting my change out of dependency hell and releasing it. |
2 participants
This PR implements support for
CalendarDiffTimeas an analogue of the Postgresqlintervaltype. My decision to use this type representation is based on the fact that both types claim to support bidirectional conversion to ISO 8601:2004(E) sec. 4.4.3.2.I'll do a self-review to hopefully help ease the review process.