From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Subject: [PATCH v2] PQsendQuery: Don't send Close in pipeline mode
Date: Wed, 15 Jun 2022 19:56:41 +0200
In commit 4efcf47053ea, we modified PQsendQuery to send a Close message
when in pipeline mode. But now we discover that that's not a good
thing: under certain circumstances, it causes the server to deliver a
CloseComplete message at a time when the client is not expecting it. We
failed to noticed it because the tests don't have any scenario where the
problem is hit. Remove the offending Close, and add a test case that
tickles the problematic scenario.
Co-authored-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reported-by: Daniele Varrazzo <daniele.varrazzo@gmail.com>
Discussion: https://postgr.es/m/CA+mi_8bvD0_CW3sumgwPvWdNzXY32itoG_16tDYRu_1S2gV2iw@mail.gmail.com
---
src/interfaces/libpq/fe-exec.c | 5 --
.../modules/libpq_pipeline/libpq_pipeline.c | 62 +++++++++++++++++++
.../traces/pipeline_abort.trace | 2 -
.../traces/simple_pipeline.trace | 12 ++++
4 files changed, 74 insertions(+), 7 deletions(-)
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.cindex 4180683194..e2df3a3480 100644--- a/src/interfaces/libpq/fe-exec.c+++ b/src/interfaces/libpq/fe-exec.c@@ -1403,11 +1403,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 c27c4e0ada..e24fbfe1cc 100644--- a/src/test/modules/libpq_pipeline/libpq_pipeline.c+++ b/src/test/modules/libpq_pipeline/libpq_pipeline.c@@ -968,15 +968,29 @@ test_prepared(PGconn *conn)
fprintf(stderr, "ok\n");
}
+/* Notice processor: print them, and count how many we got */+static void+notice_processor(void *arg, const char *message)+{+ int *n_notices = (int *) arg;++ (*n_notices)++;+ fprintf(stderr, "NOTICE %d: %s", *n_notices, 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;+ int n_notices = 0;
fprintf(stderr, "simple pipeline... ");
+ oldproc = PQsetNoticeProcessor(conn, notice_processor, &n_notices);+
/*
* Enter pipeline mode and dispatch a set of operations, which we'll then
* process the results of as they come in.
@@ -1052,6 +1066,54 @@ test_simple_pipeline(PGconn *conn)
if (PQpipelineStatus(conn) != PQ_PIPELINE_OFF)
pg_fatal("Exiting pipeline mode didn't seem to work");
+ if (n_notices > 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));++ /*+ * Must not have got any notices here; note bug as described in+ * https://postgr.es/m/CA+mi_8bvD0_CW3sumgwPvWdNzXY32itoG_16tDYRu_1S2gV2iw@mail.gmail.com+ */+ if (n_notices > 0)+ pg_fatal("got %d notice(s)", n_notices);++ 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..349034dda6 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 16 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.30.2
--eq2sy33kqql2cp67--
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: alvherre@alvh.no-ip.org
Subject: Re: [PATCH v2] PQsendQuery: Don't send Close in pipeline mode
In-Reply-To: <no-message-id-29827@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