Re: Odd usage of errmsg_internal in bufmgr.c - Mailing list pgsql-hackers

> On Feb 13, 2026, at 05:02, Andres Freund <andres@anarazel.de> wrote:
>
>
> Hi,
>
> On 2026-02-12 20:25:19 +0000, Zsolt Parragi wrote:
>>> No, I don't think so. This is just about errors on the bufmgr layer.
>>
>> I see. Looks like I misinterpreted the comment in md.c where it sets
>> this flag when it reads 0 blocks.
>
> With AIO an IO can fail at multiple levels, so the error stored in the handle
> is associated with the layer at which the error was "diagnosed".
>
> If we e.g. were to support am smgr implementation other than md.c it might
> need very different IO error conditions (e.g. network failures if a networked
> smgr implementation) than md.c, but the errors at the level of bufmgr.c would
> stay the same as today.
>
> It's also conceivable that a higher layer could just ignore the error by a
> lower layer and would thereby just "override" the lower layer's error.  E.g. a
> non-existant freespacemap could be recreated by freespacemap.c after a failure
> to read, instead of having to check how large the on-disk FSM is before trying
> to read it (check fsm_readbuf() for how it currently works).
>
>
>>> I apparently may be alone in this, but I find 6 repetitions of ereports, with
>>> differently indented messages and arguments, depending on whether it's an
>>> errmsg, errdetails, errhint way harder to scan and modify than something that
>>> just shows the different messages with consistent indentation.
>>
>> Is changing the messages to follow the same pattern an option?
>>
>> For example the error messages:
>>
>> "read error in block %u of relation \"%s\": %s"
>> "%u read errors among blocks %u..%u of relation \"%s\": %s"
>>
>> When the last string is conditional:
>>
>> * invalid page(s)
>> * zeroing out invalid page(s)
>> * ignoring checksum error(s)
>
> That gets complicated with translation, because you need to translate the %s
> arguments need to be translated separately, as the translation cannot be done
> together with the error/detail message. Doable, but it's not an no-cost win.
>
>
>
>> or maybe
>>
>> "read error: %s in block %u of relation \"%s\""
>
> That's not really following our message guidelines...  I'm not sure I believe
> in all the tenants stated in our guidelines, but we do try to follow them...
>
> Greetings,
>
> Andres Freund

The discussion has gone quite a bit beyond the original patch intention. I always appreciate the insights from these
exchanges.Since I started the thread, let me try to follow up with an updated version. 

PFA v3:

    * I tried to simplify the logging logic in buffer_readv_report() by leveraging errmsg_plural() and introducing a
function-scopedmacro to reduce the repetitive ereport() structure. I kept in mind that we should not use “%s” to inject
thefailure type (e.g., “invalid page”), as that would break translation. But I am not entirely certain whether using a
macroin this way could have also broken translation; if that turns out to be a concern, I am happy to rework it. 
    * I also removed the repeated wording in the comment that Andres pointed out.

At this point, the only change that relates directly to the original patch topic is the header comment adjustment for
errmsg_internal(),which is otherwise unrelated to the refactoring in bufmgr.c. I am fine with either splitting that out
intoa separate patch or dropping it altogether, whichever is preferred. 

Please let me know if this direction makes sense, or if further simplification would be better.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/