Re: Consider pipeline implicit transaction as a transaction block

On Mon, Nov 25, 2024 at 02:35:25PM +0100, Anthonin Bonnefoy wrote:
> 0001: Use implicit transaction block for the implicit pipeline
> transaction. I've added tests in pgbench that covered the same checks
> I did in psql. I've avoided using \syncpipeline since it was
> introduced in 17. I've also slightly modified the protocol-flow
> pipelining doc to provide more details on the implicit transaction
> block and how the first message after a sync can be different. I've
> added vacuum as first and second command of a pipeline in the tests
> since it was mentioned in the doc.

That looks acceptable and backpatchable to me. It is a bit sad that
pgbench does not capture the backend errors so as the output of the
tests is more predictable, but this is not new and I'm OK to expand
that with your proposal based on psql in v5-0002 with these new
meta-commands.

The patch provides positive and negative tests for REINDEX, VACUUM,
LOCK and SET LOCAL, which should be plenty enough. There is also a
negative check for subtransactions, which should never happen in
implicit transaction blocks. This last one is a must-have, thanks.

You are right to not use \syncpipeline for the base fix, still I am
tempted to add one extra test with a sequence like this one for only
one of the commands, just to check that the implicit transaction state
is enforced after a sync. Say this one with a WARNING check in the
output:
\startpipeline
SELECT 1;
\syncpipeline
SET LOCAL foo = bar;
\endpipeline

Tweaks of the tests across multiple stable branches happen all the
time, and adding one specific to 17~ is no big issue. I'm in the
middle of it but I'm lacking the steam to do so today. Will likely
finish tomorrow.
--
Michael