Adjust interaction of libpq pipeline mode with errorMessage resets.

Since commit ffa2e4670, libpq resets conn->errorMessage only when
starting a new query.  However, the later introduction of pipelining
requires a further refinement: the "start of query" isn't necessarily
when it's submitted to PQsendQueryStart.  If we clear at that point
then we risk dropping text for an error that the application has not
noticed yet.  Instead, when queuing a query while a previous query is
still in flight, leave errorMessage alone; reset it when we begin
to process the next query in pqPipelineProcessQueue.

Perhaps this should be back-patched to v14 where ffa2e4670 came in.
However I'm uncertain about whether it interacts with 618c16707.
In the absence of user complaints, leave v14 alone.

Discussion: https://postgr.es/m/1421785.1645723238@sss.pgh.pa.us

diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 45dddaf5562944e54768e910080073bcc728d53d..0c39bc9abfd787587c9396c3c638c27f02137797 100644 (file)
--- 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,