Michael Paquier <michael@paquier.xyz> writes:
> On Tue, Jul 27, 2021 at 10:31:25PM -0400, Tom Lane wrote:
>> Yeah, there are half a dozen places that currently print something
>> more specific than "out of memory". I judged that the value of this
>> was not worth the complexity it'd add to support it in this scheme.
>> Different opinions welcome of course.
> I don't mind either that this removes a bit of context. For
> unlikely-going-to-happen errors that's not worth the extra translation
> cost.
Yeah, the extra translatable strings are the main concrete cost of
keeping this behavior. But I'm dubious that labeling a small number
of the possible OOM points is worth anything, especially if they're
not providing the failed allocation request size. You can't tell if
that request was unreasonable or if it was just an unlucky victim
of bloat elsewhere. Unifying the reports into a common function
could be a starting point for more consistent/detailed OOM reports,
if anyone cared to work on that. (I hasten to add that I don't.)
> + pqReportOOMBuffer(&conn->errorMessage);
> Not much a fan of having two routines to do this job though. I would
> vote for keeping the one named pqReportOOM() with PQExpBuffer as
> argument.
Here I've got to disagree. We do need the form with a PQExpBuffer
argument, because there are some places where that isn't a pointer
to a PGconn's errorMessage. But the large majority of the calls
are "pqReportOOM(conn)", and I think having to write that as
"pqReportOOM(&conn->errorMessage)" is fairly ugly and perhaps
error-prone.
I'm not wedded to the name "pqReportOOMBuffer" though --- maybe
there's some better name for that one?
regards, tom lane
