On Tue, Oct 7, 2025 at 5:23 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Tue, Oct 07, 2025 at 11:51:45AM -0700, Masahiko Sawada wrote:
> > Just to be clear, did you mean that pg_checksums and pg_rewind already
> > do such checks? IIUC pg_checksums does CRC check for the control file,
> > and if we execute v18-pg_checksums against v17-cluster we end up with
> > a similar error but with a different log message like "pg_checksums:
> > error: pg_control CRC value is incorrect". Those log messages are not
> > helpful either.
>
> For the case of pg_checksums, we don't really have a constraint based
> on the major version, currently. However, there is a PG_VERSION_NUM
> hardcoded when syncing the data folder, so we are pretty much assuming
> that it is the case already.
>
> A check based on PG_VERSION has more benefits in the long-term, IMO.
> When the CRC check of the control file fails, it would be tempting to
> use some of the contents read from the control file but that would
> also mean that we could expose some corrupted values to the user, so
> that would not be useful.
>
> How about extracting from pg_rewind what it does with the on-disk
> PG_VERSION and create an API that returns a major version number with
> a data folder given in input? We could then reuse this API for the
> other tools, at least pg_createsubscriber and pg_checksums. I am not
> sure if there is a point in backpatching any of that, but it would
> lead to more user-friendly errors for all of these.
+1, sounds like a good idea to improve user experience. I think we can
use the API in pg_combinebackup.c too since it has a similar function,
read_pg_version_file().
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
