On Mon, Mar 18, 2024 at 1:48 PM Cary Huang <cary.huang@highgo.ca> wrote:
> Attached is the v10 patch with the above changes. Thanks again for the review.
Awesome, looks good.
On my final review pass I saw one last thing that bothered me (sorry
for not seeing it before). The backend version of
ASN1_TIME_to_timestamptz returns the following:
> + return ((int64)days * 24 * 60 * 60) + (int64)seconds;
...but I think Timestamp[Tz]s are stored as microseconds, so we're off
by a factor of a million. This still works because later we cast to
double and pass it back through float8_timestamptz, which converts it:
> + if (beentry->st_sslstatus->ssl_not_before != 0)
> + values[25] = DirectFunctionCall1(float8_timestamptz,
> + Float8GetDatum((double) beentry->st_sslstatus->ssl_not_before));
But anyone who ends up inspecting the value of
st_sslstatus->ssl_not_before directly is going to find an incorrect
timestamp. I think it'd be clearer to store microseconds to begin
with, and then just use TimestampTzGetDatum rather than the
DirectFunctionCall1 we have now. (I looked for an existing
implementation to reuse and didn't find one. Maybe we should use the
overflow-aware multiplication/addition routines -- i.e.
pg_mul_s64_overflow et al -- to multiply `days` and `seconds` by
USECS_PER_DAY/USECS_PER_SEC and combine them.)
And I think sslinfo can remain as-is, because that way overflow is
caught by float8_timestamptz. WDYT?
--Jacob
