On Fri, 26 Jul 2024 at 11:46, Peter Smith <smithpb2250@gmail.com> wrote:
>
> Hi Vignesh,
>
> There are still pending changes from my previous review of the
> 0720-0003 patch [1], but here are some new review comments for your
> latest patch v20240525-0003.
> 2b.
> Is it better to name these returned by-ref ptrs like 'ret_log_cnt',
> and 'ret_lsn' to emphasise they are output variables? YMMV.
I felt this is ok as we have mentioned in function header too
> ======
> src/test/subscription/t/034_sequences.pl
>
> 4.
> Q. Should we be suspicious that log_cnt changes from '32' to '31', or
> is there a valid explanation? It smells like some calculation is
> off-by-one, but without debugging I can't tell if it is right or
> wrong.
It works like this: for every 33 nextval we will get log_cnt as 0. So
for 33 * 6(198) log_cnt will be 0, then for 199 log_cnt will be 32 and
for 200 log_cnt will be 31. This pattern repeats, so this is ok.
These are handled in the v20240729 version attached at [1].
[1] - https://www.postgresql.org/message-id/CALDaNm3SucGGLe-B-a_aqWNWQZ-yfxFTiAA0JyP-SwX4jq9Y3A%40mail.gmail.com
Regards,
Vignesh
