psql: Fix assertion failures with pipeline mode
authorMichael Paquier <michael@paquier.xyz>
Thu, 24 Apr 2025 03:22:53 +0000 (12:22 +0900)
committerMichael Paquier <michael@paquier.xyz>
Thu, 24 Apr 2025 03:22:53 +0000 (12:22 +0900)
A correct cocktail of COPY FROM, SELECT and/or DML queries and
\syncpipeline was able to break the logic in charge of discarding
results of a pipeline, done in discardAbortedPipelineResults().  Such
sequence make the backend generate a FATAL, due to a protocol
synchronization loss.

This problem comes down to the fact that we did not consider the case of
libpq returning a PGRES_FATAL_ERROR when discarding the results of an
aborted pipeline.  The discarding code is changed so as this result
status is handled as a special case, with the caller of
discardAbortedPipelineResults() being responsible for consuming the
result.

A couple of tests are added to cover the problems reported, bringing an
interesting gain in coverage as there were no tests in the tree covering
the case of protocol synchronization loss.

Issue introduced by 41625ab8ea3d.

Reported-by: Alexander Kozhemyakin <a.kozhemyakin@postgrespro.ru>
Author: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>
Co-authored-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/ebf6ce77-b180-4d6b-8eab-71f641499ddf@postgrespro.ru

src/bin/psql/common.c
src/bin/psql/t/001_basic.pl

index 21d660a8961a8af24883b5c8cb936ee762313e06..3e4e444f3fd93d1d014fc9f53d45fb48794008ef 100644 (file)
@@ -1478,6 +1478,23 @@ discardAbortedPipelineResults(void)
             */
            return res;
        }
+       else if (res != NULL && result_status == PGRES_FATAL_ERROR)
+       {
+           /*
+            * Found a FATAL error sent by the backend, and we cannot recover
+            * from this state.  Instead, return the last result and let the
+            * outer loop handle it.
+            */
+           PGresult   *fatal_res PG_USED_FOR_ASSERTS_ONLY;
+
+           /*
+            * Fetch result to consume the end of the current query being
+            * processed.
+            */
+           fatal_res = PQgetResult(pset.db);
+           Assert(fatal_res == NULL);
+           return res;
+       }
        else if (res == NULL)
        {
            /* A query was processed, decrement the counters */
index b0e4919d4d71fd1e7af5b8e72098c1c8d8709853..4050f9a5e3e117b4ab9c917b3d22010651c5088b 100644 (file)
@@ -483,4 +483,45 @@ psql_like($node, "copy (values ('foo'),('bar')) to stdout \\g | $pipe_cmd",
 my $c4 = slurp_file($g_file);
 like($c4, qr/foo.*bar/s);
 
+# Tests with pipelines.  These trigger FATAL failures in the backend,
+# so they cannot be tested via SQL.
+$node->safe_psql('postgres', 'CREATE TABLE psql_pipeline()');
+my $log_location = -s $node->logfile;
+psql_fails_like(
+   $node,
+   qq{\\startpipeline
+COPY psql_pipeline FROM STDIN;
+SELECT 'val1';
+\\syncpipeline
+\\getresults
+\\endpipeline},
+   qr/server closed the connection unexpectedly/,
+   'protocol sync loss in pipeline: direct COPY, SELECT, sync and getresult'
+);
+$node->wait_for_log(
+   qr/FATAL: .*terminating connection because protocol synchronization was lost/,
+   $log_location);
+
+psql_fails_like(
+   $node,
+   qq{\\startpipeline
+COPY psql_pipeline FROM STDIN \\bind \\sendpipeline
+SELECT 'val1' \\bind \\sendpipeline
+\\syncpipeline
+\\getresults
+\\endpipeline},
+   qr/server closed the connection unexpectedly/,
+   'protocol sync loss in pipeline: bind COPY, SELECT, sync and getresult');
+
+# This time, test without the \getresults.
+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');
+
 done_testing();