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

2 files changed, 35 insertions, 2 deletions

diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 3e4e444f3fd..47352b7faed 100644
--- 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 4050f9a5e3e..ae5c1d66405 100644
--- 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();