Date: Thu, 24 Jun 2021 18:32:20 -0400
From: Alvaro Herrera <alvaro.herrera@2ndquadrant.com>
To: Boris Kolpackov <boris@codesynthesis.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Pipeline mode and PQpipelineSync()
Message-ID: <202106242232.vpewlschvxfv@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="iq42jpxio57kw75o"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <boris.20210623100839@codesynthesis.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202106242232.vpewlschvxfv%40alvherre.pgsql>
Precedence: bulk


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

On 2021-Jun-23, Boris Kolpackov wrote:

> If, however, I execute it by checking for results before sending the
> next INSERT, I get the following call sequence:
> 
> PQsendQueryPrepared() # INSERT #1
> PQflush()
> PQsendQueryPrepared() # INSERT #2
> PQflush()
> ...
> PQsendQueryPrepared() # INSERT #~400
> PQflush()
> PQconsumeInput()      # At this point select() indicates we can read.
> PQgetResult()         # NULL (???)
> PQgetResult()         # INSERT #1
> PQgetResult()         # NULL
> PQgetResult()         # INSERT #2
> PQgetResult()         # NULL

IIUC the problem is that PQgetResult is indeed not prepared to deal with
a result the first time until after the queue has been "prepared", and
this happens on calling PQpipelineSync.  But I think the formulation in
the attached patch works too, and the resulting code is less surprising.

I wrote a test case that works as you describe, and indeed with the
original code it gets a NULL initially; that disappears with the
attached patch.  Can you give it a try?

Thanks

-- 
Álvaro Herrera       Valdivia, Chile
"Escucha y olvidarás; ve y recordarás; haz y entenderás" (Confucio)

--iq42jpxio57kw75o
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="0001-Fix-libpq-state-machine.patch"

From 4073abc0ef8b4404915b949b3058ba47b7c51ba9 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Thu, 24 Jun 2021 18:24:04 -0400
Subject: [PATCH] Fix libpq state machine

---
 src/interfaces/libpq/fe-exec.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 7bd5b3a7b9..8403a9bbca 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -1375,8 +1375,7 @@ PQsendQueryInternal(PGconn *conn, const char *query, bool newQuery)
 
 	/* OK, it's launched! */
 	pqAppendCmdQueueEntry(conn, entry);
-	if (conn->pipelineStatus == PQ_PIPELINE_OFF)
-		conn->asyncStatus = PGASYNC_BUSY;
+	conn->asyncStatus = PGASYNC_BUSY;
 	return 1;
 
 sendFailed:
@@ -1513,8 +1512,7 @@ PQsendPrepare(PGconn *conn,
 
 	pqAppendCmdQueueEntry(conn, entry);
 
-	if (conn->pipelineStatus == PQ_PIPELINE_OFF)
-		conn->asyncStatus = PGASYNC_BUSY;
+	conn->asyncStatus = PGASYNC_BUSY;
 
 	/*
 	 * Give the data a push (in pipeline mode, only if we're past the size
@@ -1817,8 +1815,7 @@ PQsendQueryGuts(PGconn *conn,
 
 	/* OK, it's launched! */
 	pqAppendCmdQueueEntry(conn, entry);
-	if (conn->pipelineStatus == PQ_PIPELINE_OFF)
-		conn->asyncStatus = PGASYNC_BUSY;
+	conn->asyncStatus = PGASYNC_BUSY;
 	return 1;
 
 sendFailed:
@@ -2448,8 +2445,7 @@ PQsendDescribe(PGconn *conn, char desc_type, const char *desc_target)
 
 	/* OK, it's launched! */
 	pqAppendCmdQueueEntry(conn, entry);
-	if (conn->pipelineStatus == PQ_PIPELINE_OFF)
-		conn->asyncStatus = PGASYNC_BUSY;
+	conn->asyncStatus = PGASYNC_BUSY;
 	return 1;
 
 sendFailed:
@@ -3084,12 +3080,7 @@ PQpipelineSync(PGconn *conn)
 	 */
 	if (PQflush(conn) < 0)
 		goto sendFailed;
-
-	/*
-	 * Call pqPipelineProcessQueue so the user can call start calling
-	 * PQgetResult.
-	 */
-	pqPipelineProcessQueue(conn);
+	conn->asyncStatus = PGASYNC_BUSY;
 
 	return 1;
 
-- 
2.30.2


--iq42jpxio57kw75o--