Re: pipelining in psql, commit 41625ab - Mailing list pgsql-hackers

On Tue, Apr 15, 2025 at 02:34:50PM -0700, Noah Misch wrote:
> On Fri, Feb 21, 2025 at 11:33:41AM +0900, Michael Paquier wrote:
> commit 41625ab wrote:
>>     * \syncpipeline queues a synchronisation request, without flushing the
>>     commands to the server, equivalent of PQsendPipelineSync().
>>
> libpq has both PQpipelineSync() and PQsendPipelineSync(), so I find it odd
> that the psql command for PQsendPipelineSync() is \syncpipeline.  I would have
> expected the word "send" somewhere in its name.  That said, maybe having
> PQpipelineSync() was a mistake, since I think it's just PQsendPipelineSync() +
> PQflush().  In that light, it's reasonable not to spread the extra "send" word
> into psql.  \syncpipeline is fine with me, but it's worth others taking a
> second look.

At this stage, PQpipelineSync() is just around for compatibility
purposes, we cannot remove it and I don't see recommending it either.
It is true that \syncpipeline does not map clearly with the fact that
we're just calling PQsendPipelineSync() under the hood.

>
>> +                pg_log_error("\\getresults: invalid number of requested results");
>> +                return PSQL_CMD_SKIP_LINE;
>
> This should be PSQL_CMD_ERROR.  That matters under ON_ERROR_STOP=1.

Yes, good catch.  Will fix.

>> --- a/src/bin/psql/help.c
>> +++ b/src/bin/psql/help.c
>> @@ -167,15 +167,22 @@ slashUsage(unsigned short int pager)
>>      HELP0("  \\close STMT_NAME       close an existing prepared statement\n");
>>      HELP0("  \\copyright             show PostgreSQL usage and distribution terms\n");
>>      HELP0("  \\crosstabview [COLUMNS] execute query and display result in crosstab\n");
>> +    HELP0("  \\endpipeline           exit pipeline mode\n");
>>      HELP0("  \\errverbose            show most recent error message at maximum verbosity\n");
>> +    HELP0("  \\flush                 push unsent data to the server\n");
>> +    HELP0("  \\flushrequest          send a flushrequest command\n");
>
> protocol.sgml calls it a "Flush command".

For \flushrequest, how about "send a request to the server to flush
its output buffer" and for \flush "flush output data to the server",
mapping more with the libpq desctiptions?

> General
>   \bind [PARAM]...       set query parameters
>   \copyright             show PostgreSQL usage and distribution terms
>   \crosstabview [COLUMNS] execute query and display result in crosstab
>   \errverbose            show most recent error message at maximum verbosity
>   \g [(OPTIONS)] [FILE]  execute query (and send result to file or |pipe);
>                          \g with no arguments is equivalent to a semicolon
>   \gdesc                 describe result of query, without executing it
>   \gexec                 execute query, then execute each value in its result
>   \gset [PREFIX]         execute query and store result in psql variables
>   \gx [(OPTIONS)] [FILE] as \g, but forces expanded output mode
>   \q                     quit psql
>   \watch [[i=]SEC] [c=N] [m=MIN]
>                          execute query every SEC seconds, up to N times,
>                          stop if less than MIN rows are returned
>
> v18 has raised that to 26 lines via $SUBJECT and other additions.  I think a
> new "Extended Query Protocol" section should house \bind and all the v18
> additions.  Beginners should ignore the new section.  The section may as well
> appear last.  What do you think?

At this stage, most of the bloat of the general section is caused by
the extended protocol commands, so I like your suggestion of a new
section with a split from General.
--
Michael