CalendarDiffTime/SqlInterval objects with negative components do not work

[tysonzero]

The underlying cause is haskell/time#260 so if that is fixed then Opaleye may not have to do anything. I still bring it up here because it's a bit more subjective whether or not the issue in time is truly a bug (even though personally I think it's worth addressing there), due to the iso8601 standard potentially not supporting negative intervals.

However in the context of Opaleye the end result is fairly objectively a bug, due to both CalendarDiffTime and SqlInterval internally both fully supporting negative values, but serialization back and forth between them failing. So if the bug is labelled as a wontfix in time (or just takes a while) then Opaleye using something other than time's iso8601... functions seems like the right move.

[tysonzero]

Despite the title this issue is more or less unrelated to #601, which can still be closed if the suggested QoL change does not seem worth the effort.

[tysonzero]

Looks like this got fixed in time-1.14, may be worth mentioning that version requirement in Opaleye's docs though, as I'm assuming a lot of users will still be using time-1.12.2 for the foreseeable future due to it being a boot package even for the newest compilers: https://gitlab.haskell.org/ghc/ghc/-/wikis/commentary/libraries/version-history

[tysonzero]

Hmm I'm now thinking patching Opaleye directly may be worthwhile given input-output-hk/haskell.nix#2234, those buggy time pre-1.14 iso8601 functions are going to stick around for quite some time and be annoying for many users to avoid, leading to Opaleye's SqlIntegral and CalendarDiffTime being rather problematic to use in practice.

[tomjaguarpaw]

Could you explain what needs to be done to fix this?

[tysonzero]

So basically time-1.12.2's ISO8601 CalendarDiffTime instance breaks in the presence of negative interval components but time-1.14's ISO8601 CalendarDiffTime instance works correctly.

time-1.12.2 is the boot package version for all recent ghc versions, so a huge fraction of users will be using it, and many of those users will have a hard time upgrading to time-1.14.

Given the above I'd say to fix this so that users can easily use CalendarDiffTime and SqlInterval without waiting for time-1.14 or above to be the boot package for the compilers they use (potentially years), opaleye most likely should just change the following line: https://hackage.haskell.org/package/opaleye-0.10.3.1/docs/src/Opaleye.Internal.PGTypesExternal.html#sqlInterval to not use unsafePgFormatTime since that just dispatches to ISO8601 which contains the buggy instance.

The changed code in time-1.14 could probably just be directly replicated within some opaleye serialization module. If you compare these two files:

• https://hackage.haskell.org/package/time-1.12.2/docs/src/Data.Time.Format.ISO8601.html#intDesignator
• https://hackage.haskell.org/package/time-1.14/docs/src/Data.Time.Format.ISO8601.html#intDesignator

You can see that the NoSign to NegSign switch is the key that fixed this issue.

[tomjaguarpaw]

Thanks for the detailed information. So if we make your proposed fix then the only difference is that some code that currently errors out will no longer error out but return the correct result? If so then I'm happy with that.

I can put it in my priority queue of things to do. If you'd be willing to submit the relevant PR then that would probably get it done much quicker!