Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ jobs:
- name: Compile
run: rebar3 compile

- name: Eunit tests
run: rebar3 eunit
Comment on lines +40 to +41
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added basic unit tests to make the offset code more obvious


- name: Proper tests
run: rebar3 as test proper

Expand Down
25 changes: 25 additions & 0 deletions src/pg_timestamp.erl
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,27 @@ encode_timestamp(infinity) ->
16#7FFFFFFFFFFFFFFF;
encode_timestamp('-infinity') ->
-16#8000000000000000;
% A timestamp with a positive offset is a time in the future, compared to UTC,
% and therefore you need to subtract the hour and minutes to generate a UTC
% time. Please see 'test/timestamptz_tests.erl' for some examples.
encode_timestamp({{Year, Month, Day}, {Hour, Minute, Seconds}, {HourOffset, MinuteOffset}}) when is_integer(Seconds), MinuteOffset >= 0 ->
Sign = determine_sign(HourOffset),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The offset is either positive or negative. calendar:time_to_seconds doesn't work with negative values though.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

OffsetFromHours = calendar:time_to_seconds({abs(HourOffset), 0, 0}),
OffsetFromMinutes = calendar:time_to_seconds({0, MinuteOffset, 0}),
DatetimeSeconds = calendar:datetime_to_gregorian_seconds({{Year, Month, Day}, {Hour, Minute, Seconds}}) - ?POSTGRESQL_GS_EPOCH,
(DatetimeSeconds + OffsetFromHours * Sign + OffsetFromMinutes * Sign) * 1000000;
encode_timestamp(Datetime={{_, _, _}, {_, _, Seconds}}) when is_integer(Seconds)->
Secs = calendar:datetime_to_gregorian_seconds(Datetime) - ?POSTGRESQL_GS_EPOCH,
Secs * 1000000;
encode_timestamp({{Year, Month, Day}, {Hours, Minutes, Seconds}, {HourOffset, MinuteOffset}}) when is_float(Seconds), MinuteOffset >= 0 ->
Sign = determine_sign(HourOffset),
OffsetFromHours = calendar:time_to_seconds({abs(HourOffset), 0, 0}),
OffsetFromMinutes = calendar:time_to_seconds({0, MinuteOffset, 0}),
IntegerSeconds = trunc(Seconds),
US = trunc((Seconds - IntegerSeconds) * 1000000),
DatetimeSeconds = calendar:datetime_to_gregorian_seconds({{Year, Month, Day},
{Hours, Minutes, IntegerSeconds}}) - ?POSTGRESQL_GS_EPOCH,
((DatetimeSeconds + OffsetFromHours * Sign + OffsetFromMinutes * Sign) * 1000000) + US;
encode_timestamp({{Year, Month, Day}, {Hours, Minutes, Seconds}}) when is_float(Seconds)->
IntegerSeconds = trunc(Seconds),
US = trunc((Seconds - IntegerSeconds) * 1000000),
Expand Down Expand Up @@ -88,3 +106,10 @@ add_usecs(Secs, 0) ->
Secs;
add_usecs(Secs, USecs) ->
Secs + (USecs / 1000000).

% When the hour offset is positive, you are in the future and therefore
% need to subtract the hours to get to UTC.
determine_sign(HourOffset) when HourOffset >= 0 ->
-1;
determine_sign(_) ->
1.
2 changes: 1 addition & 1 deletion src/pg_timestampz.erl
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ decode(Bin, _TypeInfo) ->
pg_timestamp:decode_timestamp(Bin, []).

type_spec() ->
pg_timestamp:type_spec().
"{{Year::integer(), Month::1..12, Day::1..31}, {Hours::integer(), Minutes::integer(), Seconds::integer() | float()}, {HourOffset::integer(), MinuteOffset::pos_integer()}}".
17 changes: 17 additions & 0 deletions test/prop_timestamp.erl
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,22 @@ prop_tz_int_sec_codec() ->
?FORALL(Val, int_timestamp(),
proper_lib:codec(pg_timestampz, [], Val)).

prop_tz_offset_codec() ->
?FORALL(Val, utc_offset_timestamp(),
proper_lib:codec(pg_timestampz, [], Val, apply_offset(Val))).

apply_offset({{Year, Month, Day}, {Hour, Minute, Second}, {HourOffset, MinuteOffset}}) ->
Sign = case HourOffset >= 0 of
true -> -1;
false -> 1
end,
OffsetHours = calendar:time_to_seconds({abs(HourOffset), 0, 0}),
OffsetMinutes = calendar:time_to_seconds({0, MinuteOffset, 0}),
DatetimeSeconds = calendar:datetime_to_gregorian_seconds({{Year, Month, Day}, {Hour, Minute, Second}}),
calendar:gregorian_seconds_to_datetime(DatetimeSeconds + OffsetHours * Sign + OffsetMinutes * Sign).

int_timestamp() ->
{proper_lib:date_gen(), proper_lib:int_time_gen()}.

utc_offset_timestamp() ->
{proper_lib:date_gen(), proper_lib:int_time_gen(), proper_lib:utc_offset_gen()}.
26 changes: 21 additions & 5 deletions test/proper_lib.erl
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
-module(proper_lib).

-export([codec/3, codec/4]).
-export([codec/3, codec/4, encode_decode/3]).

-export([int16/0, int32/0, int64/0,
date_gen/0, int_time_gen/0]).
date_gen/0, int_time_gen/0, utc_offset_gen/0]).

-include_lib("proper/include/proper.hrl").
-include_lib("stdlib/include/assert.hrl").
Expand All @@ -12,11 +12,23 @@
codec(Mod, Opts, Data) ->
codec(Mod, Opts, Data, fun(V) -> V end).

codec(Mod, Opts, Data, Canonical) ->
% Encode and then decode the Input
encode_decode(Mod, Opts, Input) ->
{_, Config} = Mod:init(Opts),
TypeInfo = #type_info{config = Config},
<<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).
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) ->
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the original codec function

Decoded = encode_decode(Mod, Opts, Data),
Canonical(Data) =:= Canonical(Decoded);
% Passing anything else will simply compare the Output with the encoded/decoded
% Input
codec(Mod, Opts, Data, Output) ->
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Decoded = encode_decode(Mod, Opts, Data),
Output =:= Decoded.

%%
%% Generators
Expand All @@ -39,6 +51,10 @@ date_gen() ->
proper_types:integer(1, 31)},
calendar:valid_date(Date)).

utc_offset_gen() ->
{proper_types:integer(-15, 15),
proper_types:integer(0, 59)}.
Comment on lines +54 to +56
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tested various values in psql until they failed.


int_time_gen() ->
{proper_types:integer(0, 23),
proper_types:integer(0, 59),
Expand Down
33 changes: 33 additions & 0 deletions test/timestamptz_tests.erl
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
-module(timestamptz_tests).

-include_lib("eunit/include/eunit.hrl").

negative_offset_timestamptz_test() ->
?assertEqual(
proper_lib:encode_decode(
pg_timestampz,
[],
{{2024, 3, 1}, {1, 0, 0}, {-5, 15}}
),
{{2024, 3, 1}, {6, 15, 0}}
).

positive_offset_timestamptz_test() ->
?assertEqual(
proper_lib:encode_decode(
pg_timestampz,
[],
{{2024, 3, 1}, {1, 0, 0}, {5, 15}}
),
{{2024, 2, 29}, {19, 45, 0}}
).

no_offset_timestamptz_test() ->
?assertEqual(
proper_lib:encode_decode(
pg_timestampz,
[],
{{2024, 3, 1}, {1, 0, 0}, {0, 0}}
),
{{2024, 3, 1}, {1, 0, 0}}
).
Loading