Horton
Menu

Re: Add Pipelining support in psql

Michael Paquier <michael@paquier.xyz>

View thread
From: Michael Paquier <michael@paquier.xyz>
To: "a.kozhemyakin" <a.kozhemyakin@postgrespro.ru>
Cc: Daniel Verite <daniel@manitou-mail.org>, Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>, Jelte Fennema-Nio <postgres@jeltef.nl>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2025-04-21T06:22:28Z
Lists: pgsql-hackers

Attachments

On Wed, Apr 16, 2025 at 09:18:01AM -0700, Michael Paquier wrote:
> On Wed, Apr 16, 2025 at 09:31:59PM +0700, a.kozhemyakin wrote:
>> After commit 2cce0fe on master
>> 
>> ERROR:  unexpected message type 0x50 during COPY from stdin
>> CONTEXT:  COPY psql_pipeline, line 1
>> Pipeline aborted, command did not run
>> psql: common.c:1510: discardAbortedPipelineResults: Assertion `res == ((void
>> *)0) || result_status == PGRES_PIPELINE_ABORTED' failed.
>> Aborted (core dumped)
> 
> Reproduced here, thanks for the report.  I'll look at that at the
> beginning of next week, adding an open item for now.

The failure is not related to 2cce0fe.  The following sequence fails
as well, as long as we have one SELECT after the COPY to mess up with
the \getresults that causes a PGRES_FATAL_ERROR combined with a
"terminating connection because protocol synchronization was lost" on
the backend side, because the server expects some data while the
client does not send it but psql is not able to cope with this state:
\startpipeline
COPY psql_pipeline FROM STDIN \bind \sendpipeline
SELECT $1 \bind 'val1' \sendpipeline
\syncpipeline
\getresults
\endpipeline

It's actually nice that we are able to emulate such query patterns
with psql using all these meta-commands, I don't think we have
any coverage for the backend synchronization loss case yet like this
one?

2cce0fe makes that easier to reach by allowing more command patterns,
but it's the mix of COPY followed by a SELECT that causes psql to be
confused.  All the tests that we have don't check this kind of
scenarios, for COPY TO/FROM, with always use a flush or a sync
followed quickly by \getresults, but we don't have tests where we mix
things.

Anyway, I don't think that there is much we can do under a
PGRES_FATAL_ERROR in this code path when discarding the pipe results.
As far as I can tell, the server has failed the query suddenly and the
whole pipeline flow is borked.  The best thing that I can think of is
to discard all the results while decrementing the counters, then let
psql complain about that like in the attached.  I've added two tests
in TAP, as these trigger a FATAL in the backend so we cannot use the
normal SQL route, so as we have some coverage.

@Anthonin: Any thoughts or comments, perhaps?  A second opinion would
be welcome here.
--
Michael

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. psql: Fix assertion failures with pipeline mode

  2. psql: Rework TAP routine psql_fails_like() to define WAL sender context

  3. psql: Split extended query protocol meta-commands in --help=commands

  4. psql: Improve descriptions of \\flush[request] in --help

  5. psql: Fix incorrect status code returned by \getresults

  6. psql: Allow queries terminated by semicolons while in pipeline mode

  7. psql: Add \sendpipeline to send query buffers while in a pipeline

  8. psql: Fix memory leak with \gx used within a pipeline

  9. psql: Add pipeline status to prompt and some state variables

  10. Add more tests for utility commands in pipelines

  11. psql: Add support for pipelines

  12. Add braces for if block with large comment in psql's common.c