Re: fix "Success" error messages

On Mon, Aug 26, 2019 at 09:40:23PM +0200, Peter Eisentraut wrote:
> Here is an updated patch set that rearranges this a bit according to
> your suggestions, and also fixes some similar issues in pg_checksums.

Thanks for the new patch, and you are right that pg_checksums has been
slacking here. There is the same issue with pg_verify_checksums in
11. Not sure that's worth a back-patch though. Those parts could
find their way to v12 easily.

> - ereport(ERROR,
> - (errcode_for_file_access(),
> - errmsg("could not access status of transaction %u", xid),
> - errdetail("Could not read from file \"%s\" at offset %u: %m.",
> - path, offset)));
> + if (errno)
> + ereport(ERROR,
> + (errcode_for_file_access(),
> + errmsg("could not access status of transaction %u", xid),
> + errdetail("Could not read from file \"%s\" at offset %u: %m.",
> + path, offset)));
> + else
> + ereport(ERROR,
> + (errmsg("could not access status of transaction %u", xid),
> + errdetail("Could not read from file \"%s\" at offset %u: read too few bytes.", path, offset)));

Last time I worked on that, the following suggestion was made for
error messages with shorter reads or writes:
could not read file \"%s\": read %d of %zu
Still this is clearly an improvement and I that's not worth the extra
complication, so +1 for this way of doing things.

> if (r == 0)
> break;
> - if (r != BLCKSZ)
> + else if (r < 0)
> + {
> + pg_log_error("could not read block %u in file \"%s\": %m",
> + blockno, fn);
> + exit(1);
> + }
> + else if (r != BLCKSZ)
> {
> pg_log_error("could not read block %u in file \"%s\": read %d of %d",
> blockno, fn, r, BLCKSZ);

Other code areas (xlog.c, pg_waldump.c, etc.) prefer doing it this
way, after checking the size read:
if (r != BLCKSZ)
{
if (r < 0)
pg_log_error("could not read blah: %m");
else
pg_log_error("could not read blah: read %d of %d")
}

> /* Write block with checksum */
> - if (write(f, buf.data, BLCKSZ) != BLCKSZ)
> + w = write(f, buf.data, BLCKSZ);
> + if (w != BLCKSZ)
> {
> - pg_log_error("could not write block %u in file \"%s\": %m",
> - blockno, fn);
> + if (w < 0)
> + pg_log_error("could not write block %u in file \"%s\": %m",
> + blockno, fn);
> + else
> + pg_log_error("could not write block %u in file \"%s\": wrote %d of %d",
> + blockno, fn, w, BLCKSZ);
> exit(1);
> }
> }

This is consistent.
--
Michael