On Tue, 11 Nov 2025 20:16:11 +0900 Fujii Masao <[email protected]> wrote:
> On Tue, Nov 11, 2025 at 2:43 PM Tom Lane <[email protected]> wrote: > > > > Fujii Masao <[email protected]> writes: > > > To address this, callers need a way to distinguish between > > > PGRES_FATAL_ERROR > > > and OOM. Functions that loop until PQgetResult() returns NULL should > > > continue > > > if the result is PGRES_FATAL_ERROR, but should break if it's an OOM. > > > > Not sure about that. We might or might not be able to make progress > > if the caller keeps calling PQgetResult. But if it stops after an > > OOM report then we might as well just close the connection, because > > there is no hope of getting back in sync. > > > > I'm inclined to think that it's correct that we should return > > OOM_result not NULL if we couldn't make a PGresult, but further > > analysis is needed to make sure that libpq can make progress > > afterwards. I don't think we should expect applications to > > involve themselves in that recovery logic, because they won't, > > and most certainly won't test it carefully. > > > > The whole project might be doomed to failure really. I think > > that one very likely failure if we are approaching OOM is that > > we can't enlarge libpq's input buffer enough to accept all of > > a (long) incoming server message. What hope have we got of > > getting out of that situation? > > When pqCheckInBufferSpace() fails to enlarge the input buffer and > PQgetResult() > returns PGRES_FATAL_ERROR, I noticed that PQgetResult() sets asyncStatus to > PGASYNC_IDLE. This means subsequent calls to PQgetResult() will return NULL > even in OOM case, so functions looping until NULL won't end up in an > infinite loop. > Of course, issuing another query on the same connection afterward could be > problematic, though. > > I'm still not sure this behavior is correct, but I'm just wondering if > asyncStatus should be reset to PGASYNC_IDLE also when OOM_result is returned. I agree that PQgetResult() should return NULL after returning OOM_result, and resetting asyncStatus to PGASYNC_IDLE would work in the normal mode. I also agree that continuing to use the same connection seems problematic, especially in pipeline mode; the pipeline status remains PQ_PIPELINE_OK, PQgetResult() never returns PGRES_PIPELINE_SYNC, and the sent commands are left in the command queue. Therefore, I wonder about closing the connection and resetting the status when OOM_result is retunred, by callling pqDropConnection() as handleFatalError() does. In this case, the connection status should also be set to CONNECTINO_BAD. This would prevent applications from continueing to use potentially problamatic connection without providing the way to distinguish OOM from other errors. What do you think? I've attached an updated patch in this direction. Regards, Yugo Nagata -- Yugo Nagata <[email protected]>
>From f293f516d226fd20729a360adbd5c731dcc8a40d Mon Sep 17 00:00:00 2001 From: Yugo Nagata <[email protected]> Date: Tue, 11 Nov 2025 01:51:01 +0900 Subject: [PATCH v2] Make PQgetResult() not return NULL on out-of-memory error Previously, PQgetResult() returned NULL not only when no results remained for a sent query, but also when an out-of-memory error occurred, except when PGconn itself was NULL. As a result, users could not distinguish between query completion and an out-of-memory error when PQgetResult() returned NULL. This commit changes PQgetResult() to not return NULL in the case of an out-of-memory error by modifying getCopyResult() so that it never returns NULL, but instead returns OOM_result, as pqPipelineProcessQueue() does. --- src/interfaces/libpq/fe-exec.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index 0b1e37ec30b..dfc25755b85 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -2061,8 +2061,7 @@ PQisBusy(PGconn *conn) /* * PQgetResult * Get the next PGresult produced by a query. Returns NULL if no - * query work remains or an error has occurred (e.g. out of - * memory). + * query work remains. * * In pipeline mode, once all the result of a query have been returned, * PQgetResult returns NULL to let the user know that the next @@ -2234,6 +2233,8 @@ PQgetResult(PGconn *conn) static PGresult * getCopyResult(PGconn *conn, ExecStatusType copytype) { + PGresult *res; + /* * If the server connection has been lost, don't pretend everything is * hunky-dory; instead return a PGRES_FATAL_ERROR result, and reset the @@ -2254,7 +2255,16 @@ getCopyResult(PGconn *conn, ExecStatusType copytype) return pqPrepareAsyncResult(conn); /* Otherwise, invent a suitable PGresult */ - return PQmakeEmptyPGresult(conn, copytype); + res = PQmakeEmptyPGresult(conn, copytype); + + /* + * Return the static OOM_result if out-of-memory. See the comments + * in pqPrepareAsyncResult(). + */ + if (!res) + res = unconstify(PGresult *, &OOM_result); + + return res; } -- 2.43.0
