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.
--
Michael