Fix seconds being returned as seconds.milliseconds in timestamps

[ghivert]

Hi!

Sometimes, timestamps are returned as #(#(Int, Int, Int), #(Int, Int, Float)). The last Float contains the milliseconds of the timestamp. That PR fixes this by rounding the milliseconds.

I suggest to use the rounding by default, because it seems to be the most recurrent use case amongst Postgres developers. I suggest that if users want the timestamp to be returned as the full timestamp including milliseconds, then they should do the query and decoding by hand.

[ghivert]

I can confirm that behaviour is enforced here in pg_types here. decode_timestamp0 add micro-seconds if there's any (which will probably happen when using current_timestamp in SQL).

[lpil]

Oh! Should that last one be a float then? I presume people would want to lose that level of precision.

[lpil]

Remove this please, we don't recommend any particular package

[ghivert]

OK, I wasn't sure if we should indicate it or not. 🙂

[ghivert]

Oh! Should that last one be a float then? I presume people would want to lose that level of precision.

That is my opinion too. Plus, to make it work easily with package like birl that expects two tuples of three ints, I think it's best. I think users that will want to handle milliseconds and microseconds will be able to manage this by themselves.

[lpil]

I think users that will want to handle milliseconds and microseconds will be able to manage this by themselves.

Sorry, what do you mean by this?

[voneiden]

I think users that will want to handle milliseconds and microseconds will be able to manage this by themselves.

I'm new here, but that seems like a bit poor developer experience...? There's also a cascading effect to consider, as libraries that use pgo will end up with the same limitations.

[ghivert]

Wow, time ran out, and I forgot to answer. What I meant is that by default, I suppose we could truncate to seconds, and simply not take into account milliseconds and microseconds, and let users deal with them if they need. I suppose I'm biased, but I have no real use case in mind where you want to handle such precision with PG (in case I'm reaching such a level of precision, I'll probably look for POSIX time or manage it directly in code and not in PH to avoid costly TCP requests that are consuming precious ms).

If users want to get a simple POSIX timestamp, they can simply ask for it in SQL requests.

---

I think users that will want to handle milliseconds and microseconds will be able to manage this by themselves.

I'm new here, but that seems like a bit poor developer experience...? There's also a cascading effect to consider, as libraries that use pgo will end up with the same limitations.

To be honest I see absolutely no limitation here. PGO will never cancel you from managing timestamp by your own. Actually, we're only talking about the decoder we're offering by default. If you want to handle things differently with milliseconds handling, you can. It's about DX only 🙂

[voneiden]

IMO as a general principle if the database representation has microseconds they should be present on the client side as well. If I'm being fully honest, implicit truncation of values feels dangerous. In cases where a database does contain timestamps with a precision greater than one second it places responsibility on the developer to make sure they don't accidentally lose that precision when reading or writing data.

I do realize that #(#(Int, Int, Int), (#(Int, Int, Int)) is practical since it matches Erlang's DateTime. But I do feel like it should be an explicit alternative rather than the implicit norm.

For the case of microseconds, there's also the consideration of #(#(Int, Int, Int), (#(Int, Int, Float)) vs #(#(Int, Int, Int), (#(Int, Int, Int, Int)). Intuitively I feel like the latter would be a better from the perspective of types. Having seconds as floats seems jarring.

If you want to handle things differently with milliseconds handling, you can.

If I'm using pgo directly, but if I'm an indirect user via something like squirrel, I get this tingly feeling that I might be in for a headache?

Anyway, my apologies for crashing into the PR out of the blue. I do not expect that my concerns will be addressed right here and now, but I just wanted to raise them here as it seemed timely. 🙏

[ghivert]

Do not worry, while you can make a constructive argument about a solution or another, and it's not off-topic. I think it's nice to have different opinions on the subject!

I understand your point of view. Unfortunately we have no control on PGO itself, and it returns timestamps as #(#(Int, Int, Int), #(Int, Int, Float)). Maybe we should simply provide the default decoder without truncating. Isn't Float less precise than Int? I have no real opinion on this.

In any case, as of now, you have high chances the decoder will not work. I suspect squirrel to implement its own decoder. birl also handles #(#(Int, Int, Int), #(Int, Int, Int)). 🤔

[ghivert]

Sorry for taking so long time to take action. Finally I suggest to use the correct format output by pgo, and to not keep precision. Users will always be able to truncate numbers if they want.

[ghivert]

Implemented as Timestamp in newer release. Thanks!