Remove PQsendQuery support in pipeline mode

The extended query protocol implementation I added in commit
acb7e4eb6b1c has bugs when used in pipeline mode.  Rather than spend
more time trying to fix it, remove that code and make the function rely
on simple query protocol only, meaning it can no longer be used in
pipeline mode.

Users can easily change their applications to use PQsendQueryParams
instead.  We leave PQsendQuery in place for Postgres 14, just in case
somebody is using it and has not hit the mentioned bugs; but we should
recommend that it not be used.

Backpatch to 15.

Per bug report from Gabriele Varrazzo.
Discussion: https://postgr.es/m/CA+mi_8ZGSQNmW6-mk_iSR4JZB_LJ4ww3suOF+1vGNs3MrLsv4g@mail.gmail.com

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 8a1a9e9932c24403d60a92ebb8bec3d0a0f75cf8..57bfc8fc714d59f9be7918744b81071d449a1d1d 100644 (file)
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -4601,8 +4601,7 @@ int PQsendQuery(PGconn *conn, const char *command);
       </para>
 
       <para>
-       In pipeline mode, command strings containing more than one SQL command
-       are disallowed.
+       In pipeline mode, this function is disallowed.
       </para>
      </listitem>
     </varlistentry>
@@ -5056,6 +5055,7 @@ int PQflush(PGconn *conn);
     <xref linkend="libpq-PQpipelineStatus"/> can be used
     to test whether pipeline mode is active.
     In pipeline mode, only <link linkend="libpq-async">asynchronous operations</link>
+    that utilize the extended query protocol
     are permitted, command strings containing multiple SQL commands are
     disallowed, and so is <literal>COPY</literal>.
     Using synchronous command execution functions
@@ -5067,6 +5067,8 @@ int PQflush(PGconn *conn);
     <function>PQdescribePrepared</function>,
     <function>PQdescribePortal</function>,
     is an error condition.
+    <function>PQsendQuery</function> is
+    also disallowed, because it uses the simple query protocol.
     Once all dispatched commands have had their results processed, and
     the end pipeline result has been consumed, the application may return
     to non-pipelined mode with <xref linkend="libpq-PQexitPipelineMode"/>.
@@ -5095,8 +5097,7 @@ int PQflush(PGconn *conn);
 
     <para>
      After entering pipeline mode, the application dispatches requests using
-     <xref linkend="libpq-PQsendQuery"/>,
-     <xref linkend="libpq-PQsendQueryParams"/>,
+     <xref linkend="libpq-PQsendQueryParams"/>
      or its prepared-query sibling
      <xref linkend="libpq-PQsendQueryPrepared"/>.
      These requests are queued on the client-side until flushed to the server;

diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 97f6894244dce6ca73b5be971912d485fd25d745..0274c1b156c69be6bbfb5e9bb7101468327295fc 100644 (file)
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -1433,7 +1433,6 @@ static int
 PQsendQueryInternal(PGconn *conn, const char *query, bool newQuery)
 {
    PGcmdQueueEntry *entry = NULL;
-   PGcmdQueueEntry *entry2 = NULL;
 
    if (!PQsendQueryStart(conn, newQuery))
        return 0;
@@ -1446,103 +1445,48 @@ PQsendQueryInternal(PGconn *conn, const char *query, bool newQuery)
        return 0;
    }
 
-   entry = pqAllocCmdQueueEntry(conn);
-   if (entry == NULL)
-       return 0;               /* error msg already set */
    if (conn->pipelineStatus != PQ_PIPELINE_OFF)
    {
-       entry2 = pqAllocCmdQueueEntry(conn);
-       if (entry2 == NULL)
-           goto sendFailed;
+       appendPQExpBuffer(&conn->errorMessage,
+                         libpq_gettext("%s not allowed in pipeline mode\n"),
+                         "PQsendQuery");
+       return 0;
    }
 
+   entry = pqAllocCmdQueueEntry(conn);
+   if (entry == NULL)
+       return 0;               /* error msg already set */
+
    /* Send the query message(s) */
-   if (conn->pipelineStatus == PQ_PIPELINE_OFF)
+   /* construct the outgoing Query message */
+   if (pqPutMsgStart('Q', conn) < 0 ||
+       pqPuts(query, conn) < 0 ||
+       pqPutMsgEnd(conn) < 0)
    {
-       /* construct the outgoing Query message */
-       if (pqPutMsgStart('Q', conn) < 0 ||
-           pqPuts(query, conn) < 0 ||
-           pqPutMsgEnd(conn) < 0)
-       {
-           /* error message should be set up already */
-           pqRecycleCmdQueueEntry(conn, entry);
-           return 0;
-       }
-
-       /* remember we are using simple query protocol */
-       entry->queryclass = PGQUERY_SIMPLE;
-       /* and remember the query text too, if possible */
-       entry->query = strdup(query);
+       /* error message should be set up already */
+       pqRecycleCmdQueueEntry(conn, entry);
+       return 0;
    }
-   else
-   {
-       /*
-        * In pipeline mode we cannot use the simple protocol, so we send
-        * Parse, Bind, Describe Portal, Execute, Close Portal (with the
-        * unnamed portal).
-        */
-       if (pqPutMsgStart('P', conn) < 0 ||
-           pqPuts("", conn) < 0 ||
-           pqPuts(query, conn) < 0 ||
-           pqPutInt(0, 2, conn) < 0 ||
-           pqPutMsgEnd(conn) < 0)
-           goto sendFailed;
-       if (pqPutMsgStart('B', conn) < 0 ||
-           pqPuts("", conn) < 0 ||
-           pqPuts("", conn) < 0 ||
-           pqPutInt(0, 2, conn) < 0 ||
-           pqPutInt(0, 2, conn) < 0 ||
-           pqPutInt(0, 2, conn) < 0 ||
-           pqPutMsgEnd(conn) < 0)
-           goto sendFailed;
-       if (pqPutMsgStart('D', conn) < 0 ||
-           pqPutc('P', conn) < 0 ||
-           pqPuts("", conn) < 0 ||
-           pqPutMsgEnd(conn) < 0)
-           goto sendFailed;
-       if (pqPutMsgStart('E', conn) < 0 ||
-           pqPuts("", conn) < 0 ||
-           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);
-   }
+   /* remember we are using simple query protocol */
+   entry->queryclass = PGQUERY_SIMPLE;
+   /* and remember the query text too, if possible */
+   entry->query = strdup(query);
 
    /*
     * Give the data a push.  In nonblock mode, don't complain if we're unable
     * to send it all; PQgetResult() will do any additional flushing needed.
     */
-   if (pqPipelineFlush(conn) < 0)
+   if (pqFlush(conn) < 0)
        goto sendFailed;
 
    /* OK, it's launched! */
    pqAppendCmdQueueEntry(conn, entry);
 
-   /*
-    * When pipeline mode is in use, we need a second entry in the command
-    * queue to represent Close Portal message.  This allows us later to wait
-    * for the CloseComplete message to be received before getting in IDLE
-    * state.
-    */
-   if (conn->pipelineStatus != PQ_PIPELINE_OFF)
-   {
-       entry2->queryclass = PGQUERY_CLOSE;
-       entry2->query = NULL;
-       pqAppendCmdQueueEntry(conn, entry2);
-   }
-
    return 1;
 
 sendFailed:
    pqRecycleCmdQueueEntry(conn, entry);
-   pqRecycleCmdQueueEntry(conn, entry2);
    /* error message should be set up already */
    return 0;
 }
@@ -2246,22 +2190,6 @@ PQgetResult(PGconn *conn)
            break;
    }
 
-   /* If the next command we expect is CLOSE, read and consume it */
-   if (conn->asyncStatus == PGASYNC_PIPELINE_IDLE &&
-       conn->cmd_queue_head &&
-       conn->cmd_queue_head->queryclass == PGQUERY_CLOSE)
-   {
-       if (res && res->resultStatus != PGRES_FATAL_ERROR)
-       {
-           conn->asyncStatus = PGASYNC_BUSY;
-           parseInput(conn);
-           conn->asyncStatus = PGASYNC_PIPELINE_IDLE;
-       }
-       else
-           /* we won't ever see the Close */
-           pqCommandQueueAdvance(conn);
-   }
-
    /* Time to fire PGEVT_RESULTCREATE events, if there are any */
    if (res && res->nEvents > 0)
        (void) PQfireResultCreateEvents(conn, res);
@@ -2977,8 +2905,9 @@ PQfn(PGconn *conn,
 
    if (conn->pipelineStatus != PQ_PIPELINE_OFF)
    {
-       appendPQExpBufferStr(&conn->errorMessage,
-                            libpq_gettext("PQfn not allowed in pipeline mode\n"));
+       appendPQExpBuffer(&conn->errorMessage,
+                         libpq_gettext("%s not allowed in pipeline mode\n"),
+                         "PQfn");
        return NULL;
    }
 

diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index bbfb55542df8d1395eb2f919d2bca14ff61d8b53..f001137b7692ff0d82571a96f414229be8b56273 100644 (file)
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -282,24 +282,8 @@ pqParseInput3(PGconn *conn)
                    }
                    break;
                case '2':       /* Bind Complete */
-                   /* Nothing to do for this message type */
-                   break;
                case '3':       /* Close Complete */
-                   /*
-                    * If we get CloseComplete when waiting for it, consume
-                    * the queue element and keep going.  A result is not
-                    * expected from this message; it is just there so that
-                    * we know to wait for it when PQsendQuery is used in
-                    * pipeline mode, before going in IDLE state.  Failing to
-                    * do this makes us receive CloseComplete when IDLE, which
-                    * creates problems.
-                    */
-                   if (conn->cmd_queue_head &&
-                       conn->cmd_queue_head->queryclass == PGQUERY_CLOSE)
-                   {
-                       pqCommandQueueAdvance(conn);
-                   }
-
+                   /* Nothing to do for these message types */
                    break;
                case 'S':       /* parameter status */
                    if (getParameterStatus(conn))

diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/test/modules/libpq_pipeline/libpq_pipeline.c
index de4d75cfa82a129a00973e1cdb3926d172bf84d8..c609f42258984e175f1538165f3435c3ccdebbda 100644 (file)
--- a/src/test/modules/libpq_pipeline/libpq_pipeline.c
+++ b/src/test/modules/libpq_pipeline/libpq_pipeline.c
@@ -106,6 +106,18 @@ test_disallowed_in_pipeline(PGconn *conn)
    res = PQexec(conn, "SELECT 1");
    if (PQresultStatus(res) != PGRES_FATAL_ERROR)
        pg_fatal("PQexec should fail in pipeline mode but succeeded");
+   if (strcmp(PQerrorMessage(conn),
+              "synchronous command execution functions are not allowed in pipeline mode\n") != 0)
+       pg_fatal("did not get expected error message; got: \"%s\"",
+                PQerrorMessage(conn));
+
+   /* PQsendQuery should fail in pipeline mode */
+   if (PQsendQuery(conn, "SELECT 1") != 0)
+       pg_fatal("PQsendQuery should fail in pipeline mode but succeeded");
+   if (strcmp(PQerrorMessage(conn),
+              "PQsendQuery not allowed in pipeline mode\n") != 0)
+       pg_fatal("did not get expected error message; got: \"%s\"",
+                PQerrorMessage(conn));
 
    /* Entering pipeline mode when already in pipeline mode is OK */
    if (PQenterPipelineMode(conn) != 1)