Hi,
On Wed, Oct 08, 2025 at 09:22:48AM +0900, Michael Paquier 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.
So it seems the decision was that pg_checksums does not need this major
version check? I agree that it is not entirely obvious is does, but on
the other hand, those "pg_control CRC value is incorrect" are not
helpful either. But even if pg_controldata() manages to read the control
file, pg_checksums then errors out with "cluster is not compatible with
this version of pg_checksums" (without further information) in case the
control file version is different. I tried a few combinations, and
current HEAD works with v18 clusters, while v16 pg_checksums can read
v17 pg_control but then errors out due to that having a different
version. All other combinations I tried so far yield the CRC error.
So I think something like the attached should also be applied.
A while ago I was actually looking at doing the exact same thing you
did, i.e. extracting the PG_VERSION check out of pg_rewind for usage in
pg_checksums but got side-tracked.
While looking at the commit message of fa55be2a5, I noticed "This puts
pg_createsubscriber in line with the documentation" and now I wonder
whether the current major version dependency of pg_checksums shouldn't
be mentioned in the documentation as well?
If (that is a tall order maybe) on the other hand we agree that
pg_checksums should work across major versions, then maybe the proper
fix (on top of making sure everything else works correctly for each
version) would be to teach get_controlfile() how to read older control
files somehow, and pass it a major version as argument? Not sure that
wouldn't be a hazard/ maintenance-nightmare and I don't volunteer to do
that.
Michael
