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/>

Reply via email to