On 2026-Feb-12, Andres Freund wrote:
> I think the code after the proposed change is considerably harder to read.
Perhaps gratuitously so ... For instance, AFAICS the first block could
be:
if (result.status == PGAIO_RS_ERROR)
{
Assert(!zeroed_any); /* can't have invalid pages when zeroing them */
if (zeroed_or_error_count == 1)
ereport(elevel,
errcode(ERRCODE_DATA_CORRUPTED),
errmsg("invalid page in block %u of relation \"%s\"",
first + first_off, rpath.str));
else
ereport(elevel,
errcode(ERRCODE_DATA_CORRUPTED),
errmsg("%u invalid pages among blocks %u..%u of relation
\"%s\"",
zeroed_or_error_count, first, last, rpath.str),
errdetail("Block %u held the first invalid page.",
first + first_off),
errhint("See server log for the other %u invalid block(s).",
zeroed_or_error_count - 1));
}
which does the same, is easier to read, contains no duplicate messages,
and still allows the whole sequence fit in one screenful.
I'm confused about the meaning of "Block [first+first_off] held the
first invalid page.", though ... what exactly does that mean? What does
(first+first_off) represent? Block of what? How is (first) different
from (first+first_off)? The comments on buffer_readv_decode_error() and
buffer_readv_encode_error() leave me none the wiser. Is the segment
size relevant to how I must interpret that number?
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Los trabajadores menos efectivos son sistematicamente llevados al lugar
donde pueden hacer el menor daño posible: gerencia." (El principio Dilbert)
