MIME-Version: 1.0
References: <CAO6_XqqFVQjLjZQiL7xdwLpzZEy1ghO_JWvCFPM_OmwF9s7XdA@mail.gmail.com>
 <d67b9c19-d009-4a50-8020-1a0ea92366a1@manitou-mail.org>
In-Reply-To: <d67b9c19-d009-4a50-8020-1a0ea92366a1@manitou-mail.org>
From: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>
Date: Tue, 4 Mar 2025 18:37:09 +0100
Message-ID: <CAO6_Xqroaof=Gu_NCg4bZfpdgwOjNW9fg753cuuts-FBnv9UGw@mail.gmail.com>
Subject: Re: Add Pipelining support in psql
To: Daniel Verite <daniel@manitou-mail.org>
Cc: Michael Paquier <michael@paquier.xyz>, Jelte Fennema-Nio <postgres@jeltef.nl>, 
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAO6_Xqroaof%3DGu_NCg4bZfpdgwOjNW9fg753cuuts-FBnv9UGw%40mail.gmail.com>
Precedence: bulk

On Tue, Mar 4, 2025 at 1:32=E2=80=AFPM Daniel Verite <daniel@manitou-mail.o=
rg> wrote:
> But if the code triggered the use of the extended query protocol
> if \bind is in effect *or* a pipeline is active, then the first sequence
> would  just push "select 1" into the pipeline.
>
> This would have the advantage that, to submit into a pipeline
> a pre-existing file with SQL commands separated with ";"  you don't have
> to pre-process it to inject metacommands. Adding a \startpipeline at
> the beginning and an \endpipeline at the end would be sufficient in the
> cases that the user does not need the results before the end.
>
> The \sendpipeline is not mandatory when ";" can be used to terminate
> the queries. But it makes it clearer that the script wants
> specifically to push into a pipeline, and it might accept specific
> options in the future, whereas obviously ";" cannot.

So if I understand correctly, you want to automatically convert a
simple query into an extended query when we're within a pipeline. That
would be doable with:

--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -1668,7 +1668,16 @@ ExecQueryAndProcessResults(const char *query,
                        }
                        break;
                case PSQL_SEND_QUERY:
-                       success =3D PQsendQuery(pset.db, query);
+                       if (PQpipelineStatus(pset.db) !=3D PQ_PIPELINE_OFF)=
 {
+                               success =3D PQsendQueryParams(pset.db, quer=
y,
+
pset.bind_nparams, NULL,
+                                                               (const
char *const *) pset.bind_params,
+                                                               NULL, NULL,=
 0);
+                               if (success)
+                                       pset.piped_commands++;
+                       }
+                       else
+                               success =3D PQsendQuery(pset.db, query);
                        break;
        }

I do see the idea to make it easier to convert existing scripts into
using pipelining. The main focus of the initial implementation was
more on protocol regression tests with psql, so that's not necessarily
something I had in mind. I have some reservation as it will push all
parameters in the query string which may not be the desired behaviour.
But on the other hand, if it is to convert existing psql scripts, then
everything was already pushed as simple queries. Plus, this is similar
to what pgbench is doing when using -Mextended or -Mprepared.