On Sat, Mar 16, 2019 at 2:22 AM Michael Paquier <mich...@paquier.xyz> wrote:
> Hi all, > (related folks in CC) > > Sergei Kornilov has reported here an issue with pg_checksums: > > https://www.postgresql.org/message-id/5217311552474...@myt2-66bcb87429e6.qloud-c.yandex.net > > If the block size the tool is compiled with does not match the data > folder block size, then users would get incorrect checksums failures, > which is confusing. As pg_checksum_block() uses directly the block > size, this cannot really be made dynamic yet, so we had better issue > an error on that. Michael Banck has sent a patch for that: > https://www.postgresql.org/message-id/1552476561.4947.67.ca...@credativ.de > > The error message proposed is like that: > + if (ControlFile->blcksz != BLCKSZ) > + { > + fprintf(stderr, _("%s: data directory block size %d is different > to compiled-in block size %d.\n"), > + progname, ControlFile->blcksz, BLCKSZ); > + exit(1); > + } > Still I think that we could do better. > > Here is a proposal of message which looks more natural to me, and more > consistent with what xlog.c complains about: > database files are incompatible with pg_checksums. > The database cluster was initialized with BLCKSZ %d, but pg_checksums > was compiled with BLCKSZ %d. > BLCKSZ is very much an internal term. The exposed name through pg_settings is block_size, so I think the original was better. Combining that one with yours into "initialized with block size %d" etc, makes it a lot nicer. The "incompatible with pg_checksums" part may be a bit redundant with the commandname at the start as well, as I now realized Fabien pointed out downthread. But I would suggest just cutting it and saying "%s: database files are incompatible" or maybe "%s: data directory is incompatible" even? -- Magnus Hagander Me: https://www.hagander.net/ <http://www.hagander.net/> Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>