Re: Executing pg_createsubscriber with a non-compatible control file

On Tue, Oct 14, 2025 at 1:04 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Mon, Oct 13, 2025 at 03:35:06PM -0700, Masahiko Sawada wrote:
> > v1-0001-Introduce-API-able-to-retrieve-contents-of-PG_VER.patch:
> >
> > v1-0002-pg_upgrade-Use-PG_VERSION-generic-routine.patch:
> > v1-0003-pg_createsubscriber-Use-PG_VERSION-generic-routin.patch:
>
> Applied both of these.
>
> > The new log detail message uses the same message as what pg_resetwal
> > uses, but pg_createsubscriber shows an integer major version whereas
> > pg_resetwal shows the raw version string. I guess it's better to unify
> > the usage for better consistency.
>
> OK, done as suggested to limit the translation work.
>
> > v1-0004-pg_combinebackup-use-PG_VERSION-generic-routine.patch:
> >
> > +   pg_log_debug("read server version %u from file \"%s\"",
> > +                GET_PG_MAJORVERSION_NUM(version), "PG_VERSION");
> >
> > We used to show the full path of PG_VERSION file in the debug message.
> > While probably we can live with such a small difference, do you think
> > it's a good idea to move the debug message to get_pg_version()?
>
> I cannot get into doing that, leaving that up to the caller with the
> error message they want.  That's a minor point, I guess, I can see
> both sides of the coin.
>
> Switched this one to report the full path, like previously.
>
> > v1-0005-pg_resetwal-use-PG_VERSION-generic-routine.patch:
> >
> >     /* Check that data directory matches our server version */
> > -   CheckDataVersion();
> > +   CheckDataVersion(DataDir);
> >
> > With the patch, pg_resetwal fails to handle a relative path of PGDATA
> > as follows:
> >
> > $ bin/pg_resetwal data
> > pg_resetwal: error: could not open version file "data/PG_VERSION": No
> > such file or directory
> >
> > This is because pg_resetwal does chdir() to the given path of the data
> > directory before checking the version.
>
> Right.  I've tested with absolute paths and forgot relative paths.
> For this one, I would just use a ".", as the chdir is before the
> version check.  Or we could reverse the chdir() and the version check,
> but there is no real benefit in doing so.
>
> Updated patch set attached.

Thank you for updating the patch!

All patches look good to me.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com