pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feed[PATCH] Reset single-row processing mode at end of pipeline commands queue
2+ messages / 2 participants
[nested] [flat]
Commits from this thread:
2022-10-14 db1b931a4e 27ca0bce5f b8af4166ff libpq: Reset singlerow flag correctly in pipeline mode
* [PATCH] Reset single-row processing mode at end of pipeline commands queue
@ 2022-10-07 13:08 Denis Laxalde <denis.laxalde@dalibo.com>
2022-10-14 17:34 ` Re: [PATCH] Reset single-row processing mode at end of pipeline commands queue Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 1 reply; 2+ messages in thread
From: Denis Laxalde @ 2022-10-07 13:08 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org
Hello,
I'm trying to make single-row mode and pipeline mode work together in
Psycopg using libpq. I think there is something wrong with respect to
the single-row mode flag, not being correctly reset, in some situations.
The minimal case I'm considering is (in a pipeline):
* send query 1,
* get its results in single-row mode,
* send query 2,
* get its results *not* in single-row mode.
It seems that, as the command queue in the pipeline is empty after
getting the results of query 1, the single-row mode flag is not reset
and is still active for query 2, thus leading to an unexpected
PGRES_SINGLE_TUPLE status.
The attached patch demonstrates this in the test suite. It also suggests
to move the statement resetting single-row mode up in
pqPipelineProcessQueue(), before exiting the function when the command
queue is empty in particular.
Thanks for considering,
Denis
Attachments:
0001-Reset-single-row-processing-mode-at-end-of-pipeline-.patch (text/x-patch, 5.3K) download
From de28d9ac1e541c3b3a0279dc58fb5a7f23f775f8 Mon Sep 17 00:00:00 2001
From: Denis Laxalde <denis.laxalde@dalibo.com>
Date: Fri, 7 Oct 2022 14:00:24 +0200
Subject: [PATCH] Reset single-row processing mode at end of pipeline commands
queue
Previously, we'd reset the single-row mode only when processing results
of the next command in a pipeline and not if there's no more commands in
the queue. But if the client emits a query, retrieves its results in
single-row mode (thus setting single-row mode flag), then emits another
query and wants to retrieve its result in "normal" mode, the single-row
mode was not reset in pqPipelineProcessQueue(); this lead to an
erroneous PGRES_SINGLE_TUPLE instead of, e.g., PGRES_TUPLES_OK.
We fix this by resetting the single-row mode *even* if the command queue
is empty in pqPipelineProcessQueue().
---
src/interfaces/libpq/fe-exec.c | 12 +++---
.../modules/libpq_pipeline/libpq_pipeline.c | 43 ++++++++++++++++++-
.../libpq_pipeline/traces/singlerow.trace | 20 +++++++++
3 files changed, 68 insertions(+), 7 deletions(-)
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 0274c1b156..64701b562b 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -3096,6 +3096,12 @@ pqPipelineProcessQueue(PGconn *conn)
break;
}
+ /*
+ * Reset single-row processing mode. (Client has to set it up for each
+ * query, if desired.)
+ */
+ conn->singleRowMode = false;
+
/*
* If there are no further commands to process in the queue, get us in
* "real idle" mode now.
@@ -3115,12 +3121,6 @@ pqPipelineProcessQueue(PGconn *conn)
/* Initialize async result-accumulation state */
pqClearAsyncResult(conn);
- /*
- * Reset single-row processing mode. (Client has to set it up for each
- * query, if desired.)
- */
- conn->singleRowMode = false;
-
if (conn->pipelineStatus == PQ_PIPELINE_ABORTED &&
conn->cmd_queue_head->queryclass != PGQUERY_SYNC)
{
diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/test/modules/libpq_pipeline/libpq_pipeline.c
index c609f42258..4b9ccbfb4c 100644
--- a/src/test/modules/libpq_pipeline/libpq_pipeline.c
+++ b/src/test/modules/libpq_pipeline/libpq_pipeline.c
@@ -1153,11 +1153,11 @@ test_singlerowmode(PGconn *conn)
int i;
bool pipeline_ended = false;
- /* 1 pipeline, 3 queries in it */
if (PQenterPipelineMode(conn) != 1)
pg_fatal("failed to enter pipeline mode: %s",
PQerrorMessage(conn));
+ /* One series of three commands, using single-row mode for the first two. */
for (i = 0; i < 3; i++)
{
char *param[1];
@@ -1249,6 +1249,47 @@ test_singlerowmode(PGconn *conn)
pg_fatal("didn't get expected terminating TUPLES_OK");
}
+ /*
+ * Now issue one command, get its results in with single-row mode, then
+ * issue another command, and get its results in normal mode; make sure
+ * the single-row mode flag is reset as expected.
+ */
+ if (PQsendQueryParams(conn, "SELECT generate_series(0, 0)", 0, NULL, NULL, NULL, NULL, 0) != 1)
+ pg_fatal("failed to send query: %s",
+ PQerrorMessage(conn));
+ if (PQsendFlushRequest(conn) != 1)
+ pg_fatal("failed to send flush request");
+ if (PQsetSingleRowMode(conn) != 1)
+ pg_fatal("PQsetSingleRowMode() failed");
+ res = PQgetResult(conn);
+ if (res == NULL)
+ pg_fatal("unexpected NULL");
+ if (PQresultStatus(res) != PGRES_SINGLE_TUPLE)
+ pg_fatal("Expected PGRES_SINGLE_TUPLE, got %s",
+ PQresStatus(PQresultStatus(res)));
+ res = PQgetResult(conn);
+ if (res == NULL)
+ pg_fatal("unexpected NULL");
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ pg_fatal("Expected PGRES_TUPLES_OK, got %s",
+ PQresStatus(PQresultStatus(res)));
+ if (PQgetResult(conn) != NULL)
+ pg_fatal("expected NULL result");
+
+ if (PQsendQueryParams(conn, "SELECT 1", 0, NULL, NULL, NULL, NULL, 0) != 1)
+ pg_fatal("failed to send query: %s",
+ PQerrorMessage(conn));
+ if (PQsendFlushRequest(conn) != 1)
+ pg_fatal("failed to send flush request");
+ res = PQgetResult(conn);
+ if (res == NULL)
+ pg_fatal("unexpected NULL");
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ pg_fatal("Expected PGRES_TUPLES_OK, got %s",
+ PQresStatus(PQresultStatus(res)));
+ if (PQgetResult(conn) != NULL)
+ pg_fatal("expected NULL result");
+
if (PQexitPipelineMode(conn) != 1)
pg_fatal("failed to end pipeline mode: %s", PQerrorMessage(conn));
diff --git a/src/test/modules/libpq_pipeline/traces/singlerow.trace b/src/test/modules/libpq_pipeline/traces/singlerow.trace
index 9de99befcc..83043e1407 100644
--- a/src/test/modules/libpq_pipeline/traces/singlerow.trace
+++ b/src/test/modules/libpq_pipeline/traces/singlerow.trace
@@ -36,4 +36,24 @@ B 12 DataRow 1 2 '45'
B 12 DataRow 1 2 '46'
B 13 CommandComplete "SELECT 5"
B 5 ReadyForQuery I
+F 36 Parse "" "SELECT generate_series(0, 0)" 0
+F 14 Bind "" "" 0 0 1 0
+F 6 Describe P ""
+F 9 Execute "" 0
+F 4 Flush
+B 4 ParseComplete
+B 4 BindComplete
+B 40 RowDescription 1 "generate_series" NNNN 0 NNNN 4 -1 0
+B 11 DataRow 1 1 '0'
+B 13 CommandComplete "SELECT 1"
+F 16 Parse "" "SELECT 1" 0
+F 14 Bind "" "" 0 0 1 0
+F 6 Describe P ""
+F 9 Execute "" 0
+F 4 Flush
+B 4 ParseComplete
+B 4 BindComplete
+B 33 RowDescription 1 "?column?" NNNN 0 NNNN 4 -1 0
+B 11 DataRow 1 1 '1'
+B 13 CommandComplete "SELECT 1"
F 4 Terminate
--
2.30.2
* Re: [PATCH] Reset single-row processing mode at end of pipeline commands queue
2022-10-07 13:08 [PATCH] Reset single-row processing mode at end of pipeline commands queue Denis Laxalde <denis.laxalde@dalibo.com>
@ 2022-10-14 17:34 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 2+ messages in thread
From: Alvaro Herrera @ 2022-10-14 17:34 UTC (permalink / raw)
To: Denis Laxalde <denis.laxalde@dalibo.com>; +Cc: pgsql-hackers@lists.postgresql.org
Hello Denis,
On 2022-Oct-07, Denis Laxalde wrote:
> I'm trying to make single-row mode and pipeline mode work together in
> Psycopg using libpq. I think there is something wrong with respect to the
> single-row mode flag, not being correctly reset, in some situations.
>
> The minimal case I'm considering is (in a pipeline):
> * send query 1,
> * get its results in single-row mode,
> * send query 2,
> * get its results *not* in single-row mode.
>
> It seems that, as the command queue in the pipeline is empty after getting
> the results of query 1, the single-row mode flag is not reset and is still
> active for query 2, thus leading to an unexpected PGRES_SINGLE_TUPLE status.
>
> The attached patch demonstrates this in the test suite. It also suggests to
> move the statement resetting single-row mode up in pqPipelineProcessQueue(),
> before exiting the function when the command queue is empty in particular.
Your suggestion to move the code up seems correct to me. Therefore, I
have pushed this including the added test code. Thanks for an excellent
report and patch.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2022-10-14 17:34 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-10-07 13:08 [PATCH] Reset single-row processing mode at end of pipeline commands queue Denis Laxalde <denis.laxalde@dalibo.com>
2022-10-14 17:34 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox