On Fri, 26 Jul 2024 at 11:46, Peter Smith <smithpb2...@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
