At Tue, 21 Jun 2022 11:42:59 +0900 (JST), Kyotaro Horiguchi
<horikyota....@gmail.com> wrote in
> At Fri, 17 Jun 2022 20:31:50 +0200, Alvaro Herrera <alvhe...@alvh.no-ip.org>
> wrote in
> > Others to think about:
> >
> > PQisBusy (I think no changes are needed),
>
> Agreed.
>
> > PQfn (I think it should accept a call in PGASYNC_PIPELINE_IDLE mode;
> > fully untested in pipeline mode),
>
> Does a PQ_PIPELINE_OFF path need that? Rather I thought that we need
> Assert(!conn->asyncStatus != PGASYNC_PIPELINE_IDLE) there. But anyway
> we might need a test for this path.
In the attached, PQfn() is used while in pipeline mode and
PGASYNC_PIPELINE_IDLE. Both error out and effectivelly no-op.
> > PQexitPipelineMode (I think it needs to return error; needs test case),
>
> Agreed about test case. Currently the function doesn't handle
> PGASYNC_IDLE so I thought that PGASYNC_PIPELINE_IDLE also don't need a
> care. If the function treats PGASYNC_PIPELINE_IDLE state as error,
> the regression test fails (but I haven't examine it furtuer.)
PQexitPipelineMode() is called while PGASYNC_PIPELINE_IDLE.
> > PQsendFlushRequest (I think it should let through; ditto).
>
> Does that mean exit without pushing 'H' message?
I didn't do anything on this in the sttached.
By the way, I noticed that "libpq_pipeline uniqviol" intermittently
fails for uncertain reasons.
> result 574/575: pipeline aborted
> ...........................................................
> done writing
>
> libpq_pipeline:1531: got unexpected NULL
The "...........done writing" is printed too late in the error cases.
This causes the TAP test fail, but I haven't find what's happnening at
the time.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/test/modules/libpq_pipeline/libpq_pipeline.c
index 0ff563f59a..84734f5d41 100644
--- a/src/test/modules/libpq_pipeline/libpq_pipeline.c
+++ b/src/test/modules/libpq_pipeline/libpq_pipeline.c
@@ -53,6 +53,8 @@ static const char *const insert_sql =
static const char *const insert_sql2 =
"INSERT INTO pq_pipeline_demo(itemno,int8filler) VALUES ($1, $2)";
+Oid oid_pg_backend_pid;
+
/* max char length of an int32/64, plus sign and null terminator */
#define MAXINTLEN 12
#define MAXINT8LEN 20
@@ -86,6 +88,24 @@ pg_fatal_impl(int line, const char *fmt,...)
exit(1);
}
+/* returns true if PQfn succeeded */
+static bool
+PQfn_succeed(PGconn *conn)
+{
+ PGresult *res;
+ int res_buf;
+ int res_len;
+
+ /* PQfn should fail in pipeline mode */
+ if ((res = PQfn(conn, oid_pg_backend_pid, &res_buf, &res_len, 1, NULL, 0))
+ != NULL &&
+ PQresultStatus(res) == PGRES_COMMAND_OK)
+ return true;
+
+ return false;
+}
+
+
static void
test_disallowed_in_pipeline(PGconn *conn)
{
@@ -93,6 +113,9 @@ test_disallowed_in_pipeline(PGconn *conn)
fprintf(stderr, "test error cases... ");
+ if (oid_pg_backend_pid == 0)
+ pg_fatal("Failed to obtain functino oid: %s", PQgetvalue(res, 0, 0));
+
if (PQisnonblocking(conn))
pg_fatal("Expected blocking connection mode");
@@ -107,6 +130,10 @@ test_disallowed_in_pipeline(PGconn *conn)
if (PQresultStatus(res) != PGRES_FATAL_ERROR)
pg_fatal("PQexec should fail in pipeline mode but succeeded");
+ /* PQfn should fail in pipeline mode */
+ if (PQfn_succeed(conn))
+ pg_fatal("PQfn succeeded while in pipeline mode");
+
/* Entering pipeline mode when already in pipeline mode is OK */
if (PQenterPipelineMode(conn) != 1)
pg_fatal("re-entering pipeline mode should be a no-op but failed");
@@ -1014,6 +1041,18 @@ test_simple_pipeline(PGconn *conn)
PQclear(res);
res = NULL;
+ /* PQfn shoud be error no-op while PGASYNC_PIPELINE_IDLE */
+ if (PQfn_succeed(conn))
+ pg_fatal("PQfn succeeded while in pipeline mode");
+
+ /*
+ * Trial to getting out of pipeline mode while PGASYNC_PIPELINE_IDLE should
+ * gracefully fail.
+ */
+ if (PQexitPipelineMode(conn) != 0)
+ pg_fatal("exiting pipeline mode after query but before sync succeeded incorrectly");
+
+ /* This PQgetResult moves async status to PGASYNC_BUSY */
if (PQgetResult(conn) != NULL)
pg_fatal("PQgetResult returned something extra after first query result.");
@@ -1627,6 +1666,15 @@ main(int argc, char **argv)
if (PQresultStatus(res) != PGRES_COMMAND_OK)
pg_fatal("failed to set force_parallel_mode: %s", PQerrorMessage(conn));
+ /* obtain function OID of pg_backend_pid for PQfn test */
+ res = PQexec(conn, "SELECT 'pg_backend_pid'::regproc::int");
+ if (!res ||
+ (PQresultStatus(res) != PGRES_COMMAND_OK &&
+ PQresultStatus(res) != PGRES_TUPLES_OK))
+ pg_fatal("Failed to obtain functino oid");
+
+ oid_pg_backend_pid = atoi(PQgetvalue(res, 0, 0));
+
/* Set the trace file, if requested */
if (tracefile != NULL)
{
