From c11ea292787c23981546e34332973155f65dddb4 Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" Date: Thu, 12 Feb 2026 08:10:54 +0800 Subject: [PATCH v2] bufmgr: Inline error messages and clarify errmsg_internal() usage Refactor buffer_readv_report() to inline error format strings directly in ereport() calls instead of storing them in intermediate variables and using errmsg_internal(). This preserves compiler format-string checking and simplifies the control flow. Additionally, clarify the comment for errmsg_internal() to document that it may be used when the message text has already been translated explicitly (e.g., via _()), to avoid double translation. Author: Tom Lane Author: Chao Li Discussion: https://postgr.es/m/0C78B2B4-DBCF-4DA5-B999-EC1DA6459CB9@gmail.com --- src/backend/storage/buffer/bufmgr.c | 87 ++++++++++++++--------------- src/backend/utils/error/elog.c | 3 + 2 files changed, 46 insertions(+), 44 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index d1babaff023..56dbac803d4 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -8374,83 +8374,82 @@ buffer_readv_report(PgAioResult result, const PgAioTargetData *td, uint8 zeroed_or_error_count, checkfail_count, first_off; - uint8 affected_count; - const char *msg_one, - *msg_mult, - *det_mult, - *hint_mult; buffer_readv_decode_error(result, &zeroed_any, &ignored_any, &zeroed_or_error_count, &checkfail_count, &first_off); - /* - * Treat a read that had both zeroed buffers *and* ignored checksums as a - * special case, it's too irregular to be emitted the same way as the - * other cases. - */ if (zeroed_any && ignored_any) { - Assert(zeroed_any && ignored_any); Assert(nblocks > 1); /* same block can't be both zeroed and ignored */ Assert(result.status != PGAIO_RS_ERROR); - affected_count = zeroed_or_error_count; ereport(elevel, errcode(ERRCODE_DATA_CORRUPTED), errmsg("zeroing %u page(s) and ignoring %u checksum failure(s) among blocks %u..%u of relation \"%s\"", - affected_count, checkfail_count, first, last, rpath.str), - affected_count > 1 ? + zeroed_or_error_count, checkfail_count, first, last, rpath.str), + zeroed_or_error_count > 1 ? errdetail("Block %u held the first zeroed page.", first + first_off) : 0, errhint_plural("See server log for details about the other %d invalid block.", "See server log for details about the other %d invalid blocks.", - affected_count + checkfail_count - 1, - affected_count + checkfail_count - 1)); + zeroed_or_error_count + checkfail_count - 1, + zeroed_or_error_count + checkfail_count - 1)); return; } - /* - * The other messages are highly repetitive. To avoid duplicating a long - * and complicated ereport(), gather the translated format strings - * separately and then do one common ereport. - */ if (result.status == PGAIO_RS_ERROR) { Assert(!zeroed_any); /* can't have invalid pages when zeroing them */ - affected_count = zeroed_or_error_count; - msg_one = _("invalid page in block %u of relation \"%s\""); - msg_mult = _("%u invalid pages among blocks %u..%u of relation \"%s\""); - det_mult = _("Block %u held the first invalid page."); - hint_mult = _("See server log for the other %u invalid block(s)."); + ereport(elevel, + errcode(ERRCODE_DATA_CORRUPTED), + zeroed_or_error_count == 1 ? + errmsg("invalid page in block %u of relation \"%s\"", + first + first_off, rpath.str) : + errmsg("%u invalid pages among blocks %u..%u of relation \"%s\"", + zeroed_or_error_count, first, last, rpath.str), + zeroed_or_error_count > 1 ? + errdetail("Block %u held the first invalid page.", + first + first_off) : 0, + zeroed_or_error_count > 1 ? + errhint("See server log for the other %u invalid block(s).", + zeroed_or_error_count - 1) : 0); } else if (zeroed_any && !ignored_any) { - affected_count = zeroed_or_error_count; - msg_one = _("invalid page in block %u of relation \"%s\"; zeroing out page"); - msg_mult = _("zeroing out %u invalid pages among blocks %u..%u of relation \"%s\""); - det_mult = _("Block %u held the first zeroed page."); - hint_mult = _("See server log for the other %u zeroed block(s)."); + ereport(elevel, + errcode(ERRCODE_DATA_CORRUPTED), + zeroed_or_error_count == 1 ? + errmsg("invalid page in block %u of relation \"%s\"; zeroing out page", + first + first_off, rpath.str) : + errmsg("zeroing out %u invalid pages among blocks %u..%u of relation \"%s\"", + zeroed_or_error_count, first, last, rpath.str), + zeroed_or_error_count > 1 ? + errdetail("Block %u held the first zeroed page.", + first + first_off) : 0, + zeroed_or_error_count > 1 ? + errhint("See server log for the other %u zeroed block(s).", + zeroed_or_error_count - 1) : 0); } else if (!zeroed_any && ignored_any) { - affected_count = checkfail_count; - msg_one = _("ignoring checksum failure in block %u of relation \"%s\""); - msg_mult = _("ignoring %u checksum failures among blocks %u..%u of relation \"%s\""); - det_mult = _("Block %u held the first ignored page."); - hint_mult = _("See server log for the other %u ignored block(s)."); + ereport(elevel, + errcode(ERRCODE_DATA_CORRUPTED), + checkfail_count == 1 ? + errmsg("ignoring checksum failure in block %u of relation \"%s\"", + first + first_off, rpath.str) : + errmsg("ignoring %u checksum failures among blocks %u..%u of relation \"%s\"", + checkfail_count, first, last, rpath.str), + checkfail_count > 1 ? + errdetail("Block %u held the first ignored page.", + first + first_off) : 0, + checkfail_count > 1 ? + errhint("See server log for the other %u ignored block(s).", + checkfail_count - 1) : 0); } else pg_unreachable(); - - ereport(elevel, - errcode(ERRCODE_DATA_CORRUPTED), - affected_count == 1 ? - errmsg_internal(msg_one, first + first_off, rpath.str) : - errmsg_internal(msg_mult, affected_count, first, last, rpath.str), - affected_count > 1 ? errdetail_internal(det_mult, first + first_off) : 0, - affected_count > 1 ? errhint_internal(hint_mult, affected_count - 1) : 0); } static void diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index 59315e94e3e..2a6b0bde56a 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -1166,6 +1166,9 @@ set_backtrace(ErrorData *edata, int num_skip) * We also use this for certain cases where we *must* not try to translate * the message because the translation would fail and result in infinite * error recursion. + * + * It may also be used when the message text has already been translated + * explicitly (e.g., via _()), to avoid performing translation twice. */ int errmsg_internal(const char *fmt,...) -- 2.50.1 (Apple Git-155)