Support UTC offset in pg_timestampz::encode/2#10
Conversation
chiroptical
force-pushed
the
chiroptical/issue-70
branch
from
43b8aeb to
b940f4e
Compare
| encode_timestamp('-infinity') -> | ||
| -16#8000000000000000; | ||
| encode_timestamp({{Year, Month, Day}, {Hour, Minute, Seconds}, {HourOffset, MinuteOffset}}) when is_integer(Seconds) -> | ||
| Sign = determine_sign(HourOffset), |
There was a problem hiding this comment.
The offset is either positive or negative. calendar:time_to_seconds doesn't work with negative values though.
There was a problem hiding this comment.
I think this is just wrong?
1> calendar:time_to_seconds({0, -100, 0}).
-6000
2> calendar:time_to_seconds({-10, 100, 0}).
-30000
3> calendar:time_to_seconds({-10, 0, 0}).
-36000
4> calendar:time_to_seconds({-100, 0, 0}).
-360000
| utc_offset_gen() -> | ||
| {proper_types:integer(-15, 15), | ||
| proper_types:integer(0, 59)}. |
There was a problem hiding this comment.
I just tested various values in psql until they failed.
chiroptical
force-pushed
the
chiroptical/issue-70
branch
from
24fd546 to
1d2bc57
Compare
chiroptical
force-pushed
the
chiroptical/issue-70
branch
from
1d2bc57 to
330d9a2
Compare
tsloughter
left a comment
tsloughter
left a comment
There was a problem hiding this comment.
Couple changes I think are needed. I may be wrong about the -12 to 14. If so then we can just merge, otherwise just need those quick changes and I'll merge asap.
|
Ugh, no idea why pg_floag proper test is failing now... |
| - name: Eunit tests | ||
| run: rebar3 eunit |
There was a problem hiding this comment.
I added basic unit tests to make the offset code more obvious
| <<Size:32, Encoded:Size/binary>> = iolist_to_binary(Mod:encode(Data, TypeInfo)), | ||
| Canonical(Data) =:= Canonical(Mod:decode(Encoded, TypeInfo)). | ||
| <<Size:32, Encoded:Size/binary>> = iolist_to_binary(Mod:encode(Input, TypeInfo)), | ||
| Mod:decode(Encoded, TypeInfo). |
There was a problem hiding this comment.
I found the unit tests more readable with ?assertEqual and therefore it is useful to simply have the value back after encoding and then decoding.
|
|
||
| % Passing a function will apply that function to the input Data and | ||
| % encoded/decoded Data before comparison. | ||
| codec(Mod, Opts, Data, Canonical) when is_function(Canonical) -> |
There was a problem hiding this comment.
This is the original codec function
| Canonical(Data) =:= Canonical(Decoded); | ||
| % Passing anything else will simply compare the Output with the encoded/decoded | ||
| % Input | ||
| codec(Mod, Opts, Data, Output) -> |
There was a problem hiding this comment.
This version is more convenient in the property test. Without this, I need a more complicated function to deal with the round-trip. Something like,
f({{_X, _Y, _Z}, {_A, _B, _C}, {_D, _E}} = Timestamp) -> apply_offset(Timestamp);
f({{_X, _Y, _Z}, {_A, _B, _C} = Timestamp} -> Timestamp.
It isn't that complicated, but it is much easier to just provide the expected output.
4 participants
Working on this issue erleans/pgo#70
The first step is allowing the UTC offtset in the encoding function.
IIUC, you'll need to add a new tag for
v0.4.1after we commit this. https://github.com/erleans/pgo/blob/7f39da0b570af5929c0985fc9ca92af661defd6e/rebar.config#L4 <- is how this is used in pgo so it should automatically pick up the new minor version.Note, I was already able to test a round trip encode/decode in erleans/pgo#98 using https://rebar3.org/docs/configuration/dependencies/#checkout-dependencies