database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-11 10:01:57 +03:00
Code Activity

Fix missed cases in libpq's error handling.

Browse Source 
Commit 618c16707 invented an "error_result" flag in PGconn, which
intends to represent the state that we have an error condition and
need to build a PGRES_FATAL_ERROR PGresult from the message text in
conn->errorMessage, but have not yet done so.  (Postponing construction
of the error object simplifies dealing with out-of-memory conditions
and with concatenation of messages for multiple errors.)  For nearly all
purposes, this "virtual" PGresult object should act the same as if it
were already materialized.  But a couple of places in fe-protocol3.c
didn't get that memo, and were only testing conn->result as they used
to, without also checking conn->error_result.

In hopes of reducing the probability of similar mistakes in future,
I invented a pgHavePendingResult() macro that includes both tests.

Per report from Peter Eisentraut.

Discussion: https://postgr.es/m/b52277b9-fa66-b027-4a37-fb8989c73ff8@enterprisedb.com
This commit is contained in:
Tom Lane
2022-04-21 17:12:49 -04:00
parent 2cb1272445
commit 914611ea73
3 changed files with 16 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
src/interfaces/libpq/fe-exec.c
View File

@ -1196,6 +1196,7 @@ pqSaveParameterStatus(PGconn *conn, const char *name, const char *value)
* Returns 1 if OK, 0 if error occurred. * Returns 1 if OK, 0 if error occurred.
* *
* On error, *errmsgp can be set to an error string to be returned. * On error, *errmsgp can be set to an error string to be returned.
* (Such a string should already be translated via libpq_gettext().)
* If it is left NULL, the error is presumed to be "out of memory". * If it is left NULL, the error is presumed to be "out of memory".
* *
* In single-row mode, we create a new result holding just the current row, * In single-row mode, we create a new result holding just the current row,
@ -1986,7 +1987,7 @@ PQsetSingleRowMode(PGconn *conn)
(conn->cmd_queue_head->queryclass != PGQUERY_SIMPLE && (conn->cmd_queue_head->queryclass != PGQUERY_SIMPLE &&
conn->cmd_queue_head->queryclass != PGQUERY_EXTENDED)) conn->cmd_queue_head->queryclass != PGQUERY_EXTENDED))
return 0; return 0;
if (conn->result || conn->error_result) if (pgHavePendingResult(conn))
return 0; return 0;
/* OK, set flag */ /* OK, set flag */
@ -2941,7 +2942,7 @@ PQfn(PGconn *conn,
} }
if (conn->sock == PGINVALID_SOCKET || conn->asyncStatus != PGASYNC_IDLE || if (conn->sock == PGINVALID_SOCKET || conn->asyncStatus != PGASYNC_IDLE ||
conn->result || conn->error_result) pgHavePendingResult(conn))
{ {
appendPQExpBufferStr(&conn->errorMessage, appendPQExpBufferStr(&conn->errorMessage,
libpq_gettext("connection in wrong state\n")); libpq_gettext("connection in wrong state\n"));

10
src/interfaces/libpq/fe-protocol3.c
View File

@ -209,7 +209,7 @@ pqParseInput3(PGconn *conn)
case 'C': /* command complete */ case 'C': /* command complete */
if (pqGets(&conn->workBuffer, conn)) if (pqGets(&conn->workBuffer, conn))
return; return;
if (conn->result == NULL) if (!pgHavePendingResult(conn))
{ {
conn->result = PQmakeEmptyPGresult(conn, conn->result = PQmakeEmptyPGresult(conn,
PGRES_COMMAND_OK); PGRES_COMMAND_OK);
@ -263,7 +263,7 @@ pqParseInput3(PGconn *conn)
} }
break; break;
case 'I': /* empty query */ case 'I': /* empty query */
if (conn->result == NULL) if (!pgHavePendingResult(conn))
{ {
conn->result = PQmakeEmptyPGresult(conn, conn->result = PQmakeEmptyPGresult(conn,
PGRES_EMPTY_QUERY); PGRES_EMPTY_QUERY);
@ -281,7 +281,7 @@ pqParseInput3(PGconn *conn)
if (conn->cmd_queue_head && if (conn->cmd_queue_head &&
conn->cmd_queue_head->queryclass == PGQUERY_PREPARE) conn->cmd_queue_head->queryclass == PGQUERY_PREPARE)
{ {
if (conn->result == NULL) if (!pgHavePendingResult(conn))
{ {
conn->result = PQmakeEmptyPGresult(conn, conn->result = PQmakeEmptyPGresult(conn,
PGRES_COMMAND_OK); PGRES_COMMAND_OK);
@ -362,7 +362,7 @@ pqParseInput3(PGconn *conn)
if (conn->cmd_queue_head && if (conn->cmd_queue_head &&
conn->cmd_queue_head->queryclass == PGQUERY_DESCRIBE) conn->cmd_queue_head->queryclass == PGQUERY_DESCRIBE)
{ {
if (conn->result == NULL) if (!pgHavePendingResult(conn))
{ {
conn->result = PQmakeEmptyPGresult(conn, conn->result = PQmakeEmptyPGresult(conn,
PGRES_COMMAND_OK); PGRES_COMMAND_OK);
@ -2133,7 +2133,7 @@ pqFunctionCall3(PGconn *conn, Oid fnid,
* report COMMAND_OK. Otherwise, the backend violated the * report COMMAND_OK. Otherwise, the backend violated the
* protocol, so complain. * protocol, so complain.
*/ */
if (!(conn->result || conn->error_result)) if (!pgHavePendingResult(conn))
{ {
if (status == PGRES_COMMAND_OK) if (status == PGRES_COMMAND_OK)
{ {

8
src/interfaces/libpq/libpq-int.h
View File

@ -852,6 +852,14 @@ extern void pqTraceOutputNoTypeByteMessage(PGconn *conn, const char *message);
(resetPQExpBuffer(&(conn)->errorMessage), \ (resetPQExpBuffer(&(conn)->errorMessage), \
(conn)->errorReported = 0) (conn)->errorReported = 0)
/*
* Check whether we have a PGresult pending to be returned --- either a
* constructed one in conn->result, or a "virtual" error result that we
* don't intend to materialize until the end of the query cycle.
*/
#define pgHavePendingResult(conn) \
((conn)->result != NULL || (conn)->error_result)
/* /*
* this is so that we can check if a connection is non-blocking internally * this is so that we can check if a connection is non-blocking internally
* without the overhead of a function call * without the overhead of a function call