Horton

Re: Two issues with version checks in CREATE SUBSCRIPTION

Fujii Masao <masao.fujii@gmail.com>

View thread
From: Fujii Masao <masao.fujii@gmail.com>
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-12-23T08:51:27Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix CREATE SUBSCRIPTION failure when the publisher runs on pre-PG19.

  2. Fix version check for retain_dead_tuples subscription option.

On Tue, Dec 23, 2025 at 2:55 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> 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