On Tue, Dec 23, 2025 at 2:55 PM Hayato Kuroda (Fujitsu)
<[email protected]> wrote:
>
> Dear Fujii-san,
>
> Thanks for the patch! They basically look good to me.
Thanks to Amit and Hayato for the review! I'll commit the patches.
> > First, in subscriptioncmds.c this check rejects enabling retain_dead_tuples
> > when the publisher is running an older version. However, the comparison uses
> > 19000 as v19 value. Since server versions are encoded as 190000 for v19,
> > this appears to be a typo and allows the option to be enabled unexpectedly
> > on pre-v19 publishers. The attached 0001 patch fixes this by correcting
> > the version constant.
>
> One idea is to introduce a constant like RETAIN_DEAD_TUPLES_MIN_VERSION_NUM,
> which avoids similar typo. Is it overengineering?
I don't think this would be a sufficient guard against typos. Even with
this approach, we could still introduce a mistake like the following,
for example:
#define RETAIN_DEAD_TUPLES_MIN_VERSION_NUM 19000
Also, there are many existing checks that compare against version constants
like 190000, so changing the code to use such a macro everywhere feels
like overkill to me.
Regards,
--
Fujii Masao
