public inbox for [email protected]
help / color / mirror / Atom feedFrom: Alvaro Herrera <[email protected]>
To: Peter Eisentraut <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: dynamic result sets support in extended query protocol
Date: Mon, 30 Jan 2023 14:06:09 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
On 2022-Nov-22, Peter Eisentraut wrote:
> I added tests using the new psql \bind command to test this functionality in
> the extended query protocol, which showed that this got broken since I first
> wrote this patch. This "blame" is on the pipeline mode in libpq patch
> (acb7e4eb6b1c614c68a62fb3a6a5bba1af0a2659). I need to spend more time on
> this and figure out how to repair it.
Applying this patch, your test queries seem to work correctly.
This is quite WIP, especially because there's a couple of scenarios
uncovered by tests that I'd like to ensure correctness about, but if you
would like to continue adding tests for extended query and dynamic
result sets, it may be helpful.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"How strange it is to find the words "Perl" and "saner" in such close
proximity, with no apparent sense of irony. I doubt that Larry himself
could have managed it." (ncm, http://lwn.net/Articles/174769/)
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index ec62550e38..b530c51ccd 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -2109,19 +2109,19 @@ PQgetResult(PGconn *conn)
break;
case PGASYNC_READY:
+ res = pqPrepareAsyncResult(conn);
/*
- * For any query type other than simple query protocol, we advance
- * the command queue here. This is because for simple query
- * protocol we can get the READY state multiple times before the
- * command is actually complete, since the command string can
- * contain many queries. In simple query protocol, the queue
- * advance is done by fe-protocol3 when it receives ReadyForQuery.
+ * When an error has occurred, we consume one command from the
+ * queue for each result we return. (Normally, the command would
+ * be consumed as each result is read from the server.)
*/
if (conn->cmd_queue_head &&
- conn->cmd_queue_head->queryclass != PGQUERY_SIMPLE)
+ (conn->error_result ||
+ (conn->result != NULL &&
+ conn->result->resultStatus == PGRES_FATAL_ERROR)))
pqCommandQueueAdvance(conn);
- res = pqPrepareAsyncResult(conn);
+
if (conn->pipelineStatus != PQ_PIPELINE_OFF)
{
/*
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 8ab6a88416..2ed74aa0f1 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -205,6 +205,10 @@ pqParseInput3(PGconn *conn)
pqSaveErrorResult(conn);
}
}
+ if (conn->cmd_queue_head &&
+ conn->cmd_queue_head->queryclass != PGQUERY_SIMPLE)
+ pqCommandQueueAdvance(conn);
+
if (conn->result)
strlcpy(conn->result->cmdStatus, conn->workBuffer.data,
CMDSTATUS_LEN);
@@ -231,6 +235,7 @@ pqParseInput3(PGconn *conn)
else
{
conn->pipelineStatus = PQ_PIPELINE_ON;
+ pqCommandQueueAdvance(conn);
conn->asyncStatus = PGASYNC_READY;
}
}
@@ -257,6 +262,7 @@ pqParseInput3(PGconn *conn)
pqSaveErrorResult(conn);
}
}
+ /* XXX should we advance the command queue here? */
conn->asyncStatus = PGASYNC_READY;
break;
case '1': /* Parse Complete */
@@ -274,6 +280,7 @@ pqParseInput3(PGconn *conn)
pqSaveErrorResult(conn);
}
}
+ pqCommandQueueAdvance(conn);
conn->asyncStatus = PGASYNC_READY;
}
break;
@@ -324,6 +331,10 @@ pqParseInput3(PGconn *conn)
* really possible with the current backend.) We stop
* parsing until the application accepts the current
* result.
+ *
+ * Note that we must have already read one 'T' message
+ * previously for the same command, so we do not
+ * advance the command queue here.
*/
conn->asyncStatus = PGASYNC_READY;
return;
@@ -355,6 +366,7 @@ pqParseInput3(PGconn *conn)
}
}
conn->asyncStatus = PGASYNC_READY;
+ pqCommandQueueAdvance(conn);
}
break;
case 't': /* Parameter Description */
@@ -593,14 +605,20 @@ getRowDescriptions(PGconn *conn, int msgLength)
/*
* If we're doing a Describe, we're done, and ready to pass the result
* back to the client.
+ *
+ * XXX this coding here is a bit suspicious ...
*/
- if ((!conn->cmd_queue_head) ||
- (conn->cmd_queue_head &&
- conn->cmd_queue_head->queryclass == PGQUERY_DESCRIBE))
+ if (!conn->cmd_queue_head)
{
conn->asyncStatus = PGASYNC_READY;
return 0;
}
+ else if (conn->cmd_queue_head->queryclass == PGQUERY_DESCRIBE)
+ {
+ conn->asyncStatus = PGASYNC_READY;
+ pqCommandQueueAdvance(conn);
+ return 0;
+ }
/*
* We could perform additional setup for the new result set here, but for
Attachments:
[text/plain] libpq-fix.patch.txt (3.7K, ../[email protected]/2-libpq-fix.patch.txt)
download | inline diff:
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index ec62550e38..b530c51ccd 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -2109,19 +2109,19 @@ PQgetResult(PGconn *conn)
break;
case PGASYNC_READY:
+ res = pqPrepareAsyncResult(conn);
/*
- * For any query type other than simple query protocol, we advance
- * the command queue here. This is because for simple query
- * protocol we can get the READY state multiple times before the
- * command is actually complete, since the command string can
- * contain many queries. In simple query protocol, the queue
- * advance is done by fe-protocol3 when it receives ReadyForQuery.
+ * When an error has occurred, we consume one command from the
+ * queue for each result we return. (Normally, the command would
+ * be consumed as each result is read from the server.)
*/
if (conn->cmd_queue_head &&
- conn->cmd_queue_head->queryclass != PGQUERY_SIMPLE)
+ (conn->error_result ||
+ (conn->result != NULL &&
+ conn->result->resultStatus == PGRES_FATAL_ERROR)))
pqCommandQueueAdvance(conn);
- res = pqPrepareAsyncResult(conn);
+
if (conn->pipelineStatus != PQ_PIPELINE_OFF)
{
/*
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 8ab6a88416..2ed74aa0f1 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -205,6 +205,10 @@ pqParseInput3(PGconn *conn)
pqSaveErrorResult(conn);
}
}
+ if (conn->cmd_queue_head &&
+ conn->cmd_queue_head->queryclass != PGQUERY_SIMPLE)
+ pqCommandQueueAdvance(conn);
+
if (conn->result)
strlcpy(conn->result->cmdStatus, conn->workBuffer.data,
CMDSTATUS_LEN);
@@ -231,6 +235,7 @@ pqParseInput3(PGconn *conn)
else
{
conn->pipelineStatus = PQ_PIPELINE_ON;
+ pqCommandQueueAdvance(conn);
conn->asyncStatus = PGASYNC_READY;
}
}
@@ -257,6 +262,7 @@ pqParseInput3(PGconn *conn)
pqSaveErrorResult(conn);
}
}
+ /* XXX should we advance the command queue here? */
conn->asyncStatus = PGASYNC_READY;
break;
case '1': /* Parse Complete */
@@ -274,6 +280,7 @@ pqParseInput3(PGconn *conn)
pqSaveErrorResult(conn);
}
}
+ pqCommandQueueAdvance(conn);
conn->asyncStatus = PGASYNC_READY;
}
break;
@@ -324,6 +331,10 @@ pqParseInput3(PGconn *conn)
* really possible with the current backend.) We stop
* parsing until the application accepts the current
* result.
+ *
+ * Note that we must have already read one 'T' message
+ * previously for the same command, so we do not
+ * advance the command queue here.
*/
conn->asyncStatus = PGASYNC_READY;
return;
@@ -355,6 +366,7 @@ pqParseInput3(PGconn *conn)
}
}
conn->asyncStatus = PGASYNC_READY;
+ pqCommandQueueAdvance(conn);
}
break;
case 't': /* Parameter Description */
@@ -593,14 +605,20 @@ getRowDescriptions(PGconn *conn, int msgLength)
/*
* If we're doing a Describe, we're done, and ready to pass the result
* back to the client.
+ *
+ * XXX this coding here is a bit suspicious ...
*/
- if ((!conn->cmd_queue_head) ||
- (conn->cmd_queue_head &&
- conn->cmd_queue_head->queryclass == PGQUERY_DESCRIBE))
+ if (!conn->cmd_queue_head)
{
conn->asyncStatus = PGASYNC_READY;
return 0;
}
+ else if (conn->cmd_queue_head->queryclass == PGQUERY_DESCRIBE)
+ {
+ conn->asyncStatus = PGASYNC_READY;
+ pqCommandQueueAdvance(conn);
+ return 0;
+ }
/*
* We could perform additional setup for the new result set here, but for
view thread (5+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: dynamic result sets support in extended query protocol
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox