Actually ... it now seems to me that there's live bugs in the
interaction between pipeline mode and error-buffer clearing.
Namely, that places like PQsendQueryStart will unconditionally
clear the buffer even though we may be in the middle of a pipeline
sequence, and the buffer might hold an error that's yet to be
reported to the application. So I think we need something
more like the attached.
regards, tom lane
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 45dddaf556..0c39bc9abf 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -1380,10 +1380,7 @@ pqAppendCmdQueueEntry(PGconn *conn, PGcmdQueueEntry *entry)
* state, we don't have to do anything.
*/
if (conn->asyncStatus == PGASYNC_IDLE)
- {
- pqClearConnErrorState(conn);
pqPipelineProcessQueue(conn);
- }
break;
}
}
@@ -1730,8 +1727,10 @@ PQsendQueryStart(PGconn *conn, bool newQuery)
/*
* If this is the beginning of a query cycle, reset the error state.
+ * However, in pipeline mode with something already queued, the error
+ * buffer belongs to that command and we shouldn't clear it.
*/
- if (newQuery)
+ if (newQuery && conn->cmd_queue_head == NULL)
pqClearConnErrorState(conn);
/* Don't try to send if we know there's no live connection. */
@@ -2149,11 +2148,8 @@ PQgetResult(PGconn *conn)
/*
* We're about to return the NULL that terminates the round of
* results from the current query; prepare to send the results
- * of the next query when we're called next. Also, since this
- * is the start of the results of the next query, clear any
- * prior error message.
+ * of the next query when we're called next.
*/
- pqClearConnErrorState(conn);
pqPipelineProcessQueue(conn);
}
break;
@@ -2362,6 +2358,14 @@ PQexecStart(PGconn *conn)
if (!conn)
return false;
+ /*
+ * Since this is the beginning of a query cycle, reset the error state.
+ * However, in pipeline mode with something already queued, the error
+ * buffer belongs to that command and we shouldn't clear it.
+ */
+ if (conn->cmd_queue_head == NULL)
+ pqClearConnErrorState(conn);
+
if (conn->pipelineStatus != PQ_PIPELINE_OFF)
{
appendPQExpBufferStr(&conn->errorMessage,
@@ -2369,11 +2373,6 @@ PQexecStart(PGconn *conn)
return false;
}
- /*
- * Since this is the beginning of a query cycle, reset the error state.
- */
- pqClearConnErrorState(conn);
-
/*
* Silently discard any prior query result that application didn't eat.
* This is probably poor design, but it's here for backward compatibility.
@@ -2928,8 +2927,11 @@ PQfn(PGconn *conn,
/*
* Since this is the beginning of a query cycle, reset the error state.
+ * However, in pipeline mode with something already queued, the error
+ * buffer belongs to that command and we shouldn't clear it.
*/
- pqClearConnErrorState(conn);
+ if (conn->cmd_queue_head == NULL)
+ pqClearConnErrorState(conn);
if (conn->pipelineStatus != PQ_PIPELINE_OFF)
{
@@ -3099,6 +3101,12 @@ pqPipelineProcessQueue(PGconn *conn)
conn->cmd_queue_head == NULL)
return;
+ /*
+ * Reset the error state. This and the next couple of steps correspond to
+ * what PQsendQueryStart didn't do for this query.
+ */
+ pqClearConnErrorState(conn);
+
/* Initialize async result-accumulation state */
pqClearAsyncResult(conn);
@@ -3809,9 +3817,11 @@ PQsetnonblocking(PGconn *conn, int arg)
* behavior. this is ok because either they are making a transition _from_
* or _to_ blocking mode, either way we can block them.
*
- * Clear error state in case pqFlush adds to it.
+ * Clear error state in case pqFlush adds to it, unless we're actively
+ * pipelining, in which case it seems best not to.
*/
- pqClearConnErrorState(conn);
+ if (conn->cmd_queue_head == NULL)
+ pqClearConnErrorState(conn);
/* if we are going from blocking to non-blocking flush here */
if (pqFlush(conn))
@@ -4003,7 +4013,8 @@ PQescapeStringConn(PGconn *conn,
return 0;
}
- pqClearConnErrorState(conn);
+ if (conn->cmd_queue_head == NULL)
+ pqClearConnErrorState(conn);
return PQescapeStringInternal(conn, to, from, length, error,
conn->client_encoding,
@@ -4041,7 +4052,8 @@ PQescapeInternal(PGconn *conn, const char *str, size_t len, bool as_ident)
if (!conn)
return NULL;
- pqClearConnErrorState(conn);
+ if (conn->cmd_queue_head == NULL)
+ pqClearConnErrorState(conn);
/* Scan the string for characters that must be escaped. */
for (s = str; (s - str) < len && *s != '\0'; ++s)
@@ -4306,7 +4318,8 @@ PQescapeByteaConn(PGconn *conn,
if (!conn)
return NULL;
- pqClearConnErrorState(conn);
+ if (conn->cmd_queue_head == NULL)
+ pqClearConnErrorState(conn);
return PQescapeByteaInternal(conn, from, from_length, to_length,
conn->std_strings,
