From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Subject: [PATCH] Remove useless Close message from PQsendQuery() in pipleline mode
Date: Fri, 10 Jun 2022 15:03:52 +0900
PQsendQuery() switch to the extended query protocol while in pipeline
mode. The message sequence mistakenly contained a Close message for
unnamed portal, which may lead to an out-of-sync CloseComplete
message. That Close message is useless and removing it resolves the
issue. A test for this case is added.
Still PQsendQuery() in pipeline-mode prompts ParseComplete and
BindComplete messages that are not seen while non-pipeline mode but
they are in-sync and correctly ignored.
---
src/interfaces/libpq/fe-exec.c | 5 --
.../modules/libpq_pipeline/libpq_pipeline.c | 56 +++++++++++++++++++
.../traces/pipeline_abort.trace | 2 -
.../traces/simple_pipeline.trace | 12 ++++
4 files changed, 68 insertions(+), 7 deletions(-)
inline patch 1: src/interfaces/libpq/fe-exec.c, src/test/modules/libpq_pipeline/libpq_pipeline.c, src/test/modules/libpq_pipeline/traces/pipeline_abort.trace and 1 more (+68/-7 lines) download inline-1.patchdiff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.cindex 919cf5741d..321579319e 100644--- a/src/interfaces/libpq/fe-exec.c+++ b/src/interfaces/libpq/fe-exec.c@@ -1501,11 +1501,6 @@ PQsendQueryInternal(PGconn *conn, const char *query, bool newQuery)
pqPutInt(0, 4, conn) < 0 ||
pqPutMsgEnd(conn) < 0)
goto sendFailed;
- if (pqPutMsgStart('C', conn) < 0 ||- pqPutc('P', conn) < 0 ||- pqPuts("", conn) < 0 ||- pqPutMsgEnd(conn) < 0)- goto sendFailed;
entry->queryclass = PGQUERY_EXTENDED;
entry->query = strdup(query);
diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/test/modules/libpq_pipeline/libpq_pipeline.cindex 0ff563f59a..0b443a611a 100644--- a/src/test/modules/libpq_pipeline/libpq_pipeline.c+++ b/src/test/modules/libpq_pipeline/libpq_pipeline.c@@ -968,15 +968,27 @@ test_prepared(PGconn *conn)
fprintf(stderr, "ok\n");
}
+static int n_notice;+static void+notice_processor(void *arg, const char *message)+{+ n_notice++;+ fprintf(stderr, "%s", message);+}+
static void
test_simple_pipeline(PGconn *conn)
{
PGresult *res = NULL;
const char *dummy_params[1] = {"1"};
Oid dummy_param_oids[1] = {INT4OID};
+ PQnoticeProcessor oldproc;
fprintf(stderr, "simple pipeline... ");
+ n_notice = 0;+ oldproc = PQsetNoticeProcessor(conn, notice_processor, NULL);+
/*
* Enter pipeline mode and dispatch a set of operations, which we'll then
* process the results of as they come in.
@@ -1052,6 +1064,50 @@ test_simple_pipeline(PGconn *conn)
if (PQpipelineStatus(conn) != PQ_PIPELINE_OFF)
pg_fatal("Exiting pipeline mode didn't seem to work");
+ if (n_notice > 0)+ pg_fatal("unexpected notice");++ /* Try the same thing with PQsendQuery */+ if (PQenterPipelineMode(conn) != 1)+ pg_fatal("failed to enter pipeline mode: %s", PQerrorMessage(conn));++ if (PQsendQuery(conn, "SELECT 1;") != 1)+ pg_fatal("failed to send query: %s", PQerrorMessage(conn));+ PQsendFlushRequest(conn);+ res = PQgetResult(conn);+ if (res == NULL)+ pg_fatal("PQgetResult returned null when there's a pipeline item: %s",+ PQerrorMessage(conn));+ if (PQresultStatus(res) != PGRES_TUPLES_OK)+ pg_fatal("Unexpected result code %s from first pipeline item",+ PQresStatus(PQresultStatus(res)));+ PQclear(res);++ res = PQgetResult(conn);+ if (res != NULL)+ pg_fatal("expected NULL result");++ if (PQpipelineSync(conn) != 1)+ pg_fatal("pipeline sync failed: %s", PQerrorMessage(conn));+ res = PQgetResult(conn);+ if (res == NULL)+ pg_fatal("PQgetResult returned null when there's a pipeline item: %s",+ PQerrorMessage(conn));+ if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)+ pg_fatal("Unexpected result code %s instead of PGRES_PIPELINE_SYNC, error: %s",+ PQresStatus(PQresultStatus(res)), PQerrorMessage(conn));+ PQclear(res);+ res =NULL;++ if (PQexitPipelineMode(conn) != 1)+ pg_fatal("attempt to exit pipeline mode failed when it should've succeeded: %s",+ PQerrorMessage(conn));++ if (n_notice > 0)+ pg_fatal("unexpected notice");++ PQsetNoticeProcessor(conn, oldproc, NULL);+
fprintf(stderr, "ok\n");
}
diff --git a/src/test/modules/libpq_pipeline/traces/pipeline_abort.trace b/src/test/modules/libpq_pipeline/traces/pipeline_abort.traceindex 3fce548b99..254e485997 100644--- a/src/test/modules/libpq_pipeline/traces/pipeline_abort.trace+++ b/src/test/modules/libpq_pipeline/traces/pipeline_abort.trace@@ -38,7 +38,6 @@ F 26 Parse "" "SELECT 1; SELECT 2" 0
F 12 Bind "" "" 0 0 0
F 6 Describe P ""
F 9 Execute "" 0
-F 6 Close P ""
F 4 Sync
B NN ErrorResponse S "ERROR" V "ERROR" C "42601" M "cannot insert multiple commands into a prepared statement" F "SSSS" L "SSSS" R "SSSS" \x00
B 5 ReadyForQuery I
@@ -46,7 +45,6 @@ F 54 Parse "" "SELECT 1.0/g FROM generate_series(3, -1, -1) g" 0
F 12 Bind "" "" 0 0 0
F 6 Describe P ""
F 9 Execute "" 0
-F 6 Close P ""
F 4 Sync
B 4 ParseComplete
B 4 BindComplete
diff --git a/src/test/modules/libpq_pipeline/traces/simple_pipeline.trace b/src/test/modules/libpq_pipeline/traces/simple_pipeline.traceindex 5c94749bc1..00710bc6bc 100644--- a/src/test/modules/libpq_pipeline/traces/simple_pipeline.trace+++ b/src/test/modules/libpq_pipeline/traces/simple_pipeline.trace@@ -9,4 +9,16 @@ B 33 RowDescription 1 "?column?" NNNN 0 NNNN 4 -1 0
B 11 DataRow 1 1 '1'
B 13 CommandComplete "SELECT 1"
B 5 ReadyForQuery I
+F 17 Parse "" "SELECT 1;" 0+F 12 Bind "" "" 0 0 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 Sync+B 5 ReadyForQuery I
F 4 Terminate
--
2.31.1
----Next_Part(Fri_Jun_10_15_25_44_2022_200)----
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: pgsql-bugs@postgresql.org
Cc: horikyota.ntt@gmail.com
Subject: Re: [PATCH] Remove useless Close message from PQsendQuery() in pipleline mode
In-Reply-To: <no-message-id-29789@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
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