psql: Abort connection when using \syncpipeline after COPY TO/FROM

When the backend reads COPY data, it ignores all sync messages, as per
c01641f8aed0.  With psql pipelines, it is possible to manually send sync
messages with \sendpipeline which leaves the frontend in an
unrecoverable state as the backend will not send the necessary
ReadyForQuery message that is expected to feed psql result consumption
logic.

It could be possible to artificially reduce the piped_syncs and
requested_results, however libpq's state would still have queued sync
messages in its command queue, and the only way to consume those without
directly calling pqCommandQueueAdvance() is to process ReadyForQuery
messages that won't be sent since the backend ignores these.  Perhaps
this could be improved in the future, but I am not really excited about
introducing this amount of complications in libpq to manipulate the
message queues without a better use case to support it.

Hence, this patch aborts the connection if we detect excessive sync
messages after a COPY in a pipeline to avoid staying in an inconsistent
protocol state, which is the best thing we can do with pipelines in
psql for now.  Note that this change does not prevent wrapping a set
of queries inside a block made of \startpipeline and \endpipeline, only
the use of \syncpipeline for a COPY.

Reported-by: Nikita Kalinin <n.kalinin@postgrespro.ru>
Author: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>
Discussion: https://postgr.es/m/18944-8a926c30f68387dd@postgresql.org

diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 3e4e444f3fd93d1d014fc9f53d45fb48794008ef..47352b7faed2c8605130dc2fe706584eea8d9144 100644 (file)
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -1867,6 +1867,21 @@ ExecQueryAndProcessResults(const char *query,
        {
            FILE       *copy_stream = NULL;
 
+           if (pset.piped_syncs > 1)
+           {
+               /*
+                * When reading COPY data, the backend ignores sync messages
+                * and will not send a matching ReadyForQuery response.  Even
+                * if we adjust piped_syncs and requested_results, it is not
+                * possible to salvage this as the sync message would still be
+                * in libpq's command queue and we would be stuck in a busy
+                * pipeline state.  Thus, we abort the connection to avoid
+                * this state.
+                */
+               pg_log_info("\\syncpipeline after COPY is not supported, aborting connection");
+               exit(EXIT_BADCONN);
+           }
+
            /*
             * For COPY OUT, direct the output to the default place (probably
             * a pager pipe) for \watch, or to pset.copyStream for \copy,

diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl
index 4050f9a5e3e117b4ab9c917b3d22010651c5088b..ae5c1d664051996da4a4cf64f23b4af7fa76e2e0 100644 (file)
--- a/src/bin/psql/t/001_basic.pl
+++ b/src/bin/psql/t/001_basic.pl
@@ -513,15 +513,33 @@ SELECT 'val1' \\bind \\sendpipeline
    qr/server closed the connection unexpectedly/,
    'protocol sync loss in pipeline: bind COPY, SELECT, sync and getresult');
 
-# This time, test without the \getresults.
+# This time, test without the \getresults and \syncpipeline.
 psql_fails_like(
    $node,
    qq{\\startpipeline
 COPY psql_pipeline FROM STDIN;
 SELECT 'val1';
-\\syncpipeline
 \\endpipeline},
    qr/server closed the connection unexpectedly/,
    'protocol sync loss in pipeline: COPY, SELECT and sync');
 
+# Tests sending a sync after a COPY TO/FROM.  These abort the connection
+# from the frontend.
+psql_fails_like(
+   $node,
+   qq{\\startpipeline
+COPY psql_pipeline FROM STDIN;
+\\syncpipeline
+\\endpipeline},
+   qr/\\syncpipeline after COPY is not supported, aborting connection/,
+   'sending sync after COPY FROM');
+psql_fails_like(
+   $node,
+   qq{\\startpipeline
+COPY psql_pipeline TO STDOUT;
+\\syncpipeline
+\\endpipeline},
+   qr/\\syncpipeline after COPY is not supported, aborting connection/,
+   'sending sync after COPY TO');
+
 done_testing();