Skip to content

Commit 618c167

Browse files
tglsfdctglsfdc
committed
Rearrange libpq's error reporting to avoid duplicated error text.
Since commit ffa2e46, libpq accumulates text in conn->errorMessage across a whole query cycle. In some situations, we may report more than one error event within a cycle: the easiest case to reach is where we report a FATAL error message from the server, and then a bit later we detect loss of connection. Since, historically, each error PGresult bears the entire content of conn->errorMessage, this results in duplication of the FATAL message in any output that concatenates the contents of the PGresults. Accumulation in errorMessage still seems like a good idea, especially in view of the number of places that did ad-hoc error concatenation before ffa2e46. So to fix this, let's track how much of conn->errorMessage has been read out into error PGresults, and only include new text in later PGresults. The tricky part of that is to be sure that we never discard an error PGresult once made (else we'd risk dropping some text, a problem much worse than duplication). While libpq formerly did that in some code paths, a little bit of rearrangement lets us postpone making an error PGresult at all until we are about to return it. A side benefit of that postponement is that it now becomes practical to return a dummy static PGresult in cases where we hit out-of-memory while trying to manufacture an error PGresult. This eliminates the admittedly-very-rare case where we'd return NULL from PQgetResult, indicating successful query completion, even though what actually happened was an OOM failure. Discussion: https://postgr.es/m/ab4288f8-be5c-57fb-2400-e3e857f53e46@enterprisedb.com
1 parent 6c417bb commit 618c167

File tree

7 files changed

+224
-65
lines changed

7 files changed

+224
-65
lines changed

‎contrib/test_decoding/expected/slot_creation_error.out

-1
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,6 @@ t
9898

9999
step s2_init: <... completed>
100100
FATAL: terminating connection due to administrator command
101-
FATAL: terminating connection due to administrator command
102101
server closed the connection unexpectedly
103102
This probably means the server terminated abnormally
104103
before or while processing the request.

‎src/interfaces/libpq/fe-auth.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -1237,7 +1237,7 @@ PQencryptPasswordConn(PGconn *conn, const char *passwd, const char *user,
12371237
if (!conn)
12381238
return NULL;
12391239

1240-
resetPQExpBuffer(&conn->errorMessage);
1240+
pqClearConnErrorState(conn);
12411241

12421242
/* If no algorithm was given, ask the server. */
12431243
if (algorithm == NULL)

‎src/interfaces/libpq/fe-connect.c

+4-4
Original file line numberDiff line numberDiff line change
@@ -3685,7 +3685,7 @@ PQconnectPoll(PGconn *conn)
36853685
* (and it seems some clients expect it to be empty after a
36863686
* successful connection).
36873687
*/
3688-
resetPQExpBuffer(&conn->errorMessage);
3688+
pqClearConnErrorState(conn);
36893689

36903690
/* We are open for business! */
36913691
conn->status = CONNECTION_OK;
@@ -4231,7 +4231,7 @@ closePGconn(PGconn *conn)
42314231

42324232
/*
42334233
* Close the connection, reset all transient state, flush I/O buffers.
4234-
* Note that this includes clearing conn->errorMessage; we're no longer
4234+
* Note that this includes clearing conn's error state; we're no longer
42354235
* interested in any failures associated with the old connection, and we
42364236
* want a clean slate for any new connection attempt.
42374237
*/
@@ -4241,7 +4241,7 @@ closePGconn(PGconn *conn)
42414241
conn->xactStatus = PQTRANS_IDLE;
42424242
conn->pipelineStatus = PQ_PIPELINE_OFF;
42434243
pqClearAsyncResult(conn); /* deallocate result */
4244-
resetPQExpBuffer(&conn->errorMessage);
4244+
pqClearConnErrorState(conn);
42454245
release_conn_addrinfo(conn);
42464246

42474247
/* Reset all state obtained from server, too */
@@ -5236,7 +5236,7 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
52365236
* Returns 0 on success, nonzero on failure. On failure, if errorMessage
52375237
* isn't null, also store an error message there. (Note: the only reason
52385238
* this function and related ones don't dump core on errorMessage == NULL
5239-
* is the undocumented fact that printfPQExpBuffer does nothing when passed
5239+
* is the undocumented fact that appendPQExpBuffer does nothing when passed
52405240
* a null PQExpBuffer pointer.)
52415241
*/
52425242
static int

0 commit comments

Comments
 (0)