datetimeoffset support #48
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Just checking in: is there anything I can do to help get this merged? We've been running on these patches in my employer's code base for a couple months now and we haven't experienced any issues. |
psibi
reviewed
May 26, 2022
| @@ -773,6 +776,43 @@ getData dbc stmt i col = fmap (col, ) $ | |||
| (fmap fromIntegral (odbc_TIME_STRUCT_hour datePtr)) <*> | |||
| (fmap fromIntegral (odbc_TIME_STRUCT_minute datePtr)) <*> | |||
| (fmap fromIntegral (odbc_TIME_STRUCT_second datePtr)))) | |||
| | colType == sql_ss_timestampoffset -> | |||
| withCallocBytes | |||
| 20 -- The TIMESTAMPOFFSET_STRUCT contains 3 SQLSMALLINTs, | |||
There was a problem hiding this comment.
Minor nit pick: I would use the #size (from hsc2hs) macro to compute it ? But free feel to ignore it.
There was a problem hiding this comment.
Agreed that the size constant is not ideal. Since you're OK with it I will leave it as is, rather than introduce hsc2hs.
|
@psibi thanks for the feedback! Made the updates you requested, except for the hardcoded size. |
|
Thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Code is thanks to @ploeh from #25 which I've rebased onto master and fixed the
Hashableinstance.Tests pass. I've also done some testing in my employer's code base and everything seems to work properly. How do we feel about merging this?