Date: Wed, 3 Mar 2021 21:24:29 -0300
From: 'Alvaro Herrera' <alvherre@alvh.no-ip.org>
To: "k.jamison@fujitsu.com" <k.jamison@fujitsu.com>
Cc: Craig Ringer <craig.ringer@enterprisedb.com>,
	Matthieu Garrigues <matthieu.garrigues@gmail.com>,
	PostgreSQL-development <pgsql-hackers@lists.postgresql.org>
Subject: Re: PATCH: Batch/pipelining support for libpq
Message-ID: <20210304002429.GA8225@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="G4iJoqBmSsgzjUCe"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20210303234241.GA22679@alvherre.pgsql>
User-Agent: Mutt/1.10.1 (2018-07-13)
Archived-At: 
 <https://www.postgresql.org/message-id/20210304002429.GA8225%40alvherre.pgsql>
Precedence: bulk


--G4iJoqBmSsgzjUCe
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2021-Mar-03, 'Alvaro Herrera' wrote:

> This should obviously not occur.  I'm trying to figure out how to repair
> it and not break everything again ...

I think trying to set up the connection state so that the next query
appears in conn->last_query prior to PQgetResult being called again
leads to worse breakage.  The simplest fix seems to make fe-protocol3.c
aware that in this case, the query is in conn->cmd_queue_head instead,
as in the attached patch.

-- 
Álvaro Herrera                            39°49'30"S 73°17'W

--G4iJoqBmSsgzjUCe
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="error-fix.patch"

diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 584922b340..b89f500902 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -962,9 +962,21 @@ pqGetErrorNotice3(PGconn *conn, bool isError)
 	 * Save the active query text, if any, into res as well; but only if we
 	 * might need it for an error cursor display, which is only true if there
 	 * is a PG_DIAG_STATEMENT_POSITION field.
+	 *
+	 * Note that in pipeline mode, we have not yet advanced the query pointer
+	 * to the next query, so we have to look at that.
 	 */
-	if (have_position && conn->last_query && res)
-		res->errQuery = pqResultStrdup(res, conn->last_query);
+	if (have_position && res)
+	{
+		if (conn->pipelineStatus != PQ_PIPELINE_OFF &&
+			conn->asyncStatus == PGASYNC_IDLE)
+		{
+			if (conn->cmd_queue_head)
+				res->errQuery = pqResultStrdup(res, conn->cmd_queue_head->query);
+		}
+		else if (conn->last_query)
+			res->errQuery = pqResultStrdup(res, conn->last_query);
+	}
 
 	/*
 	 * Now build the "overall" error message for PQresultErrorMessage.

--G4iJoqBmSsgzjUCe--