Re: BUG #19616: pgoutput sends stream abort ('A') to clients that did not enable streaming

сб, 15 авг. 2026 г. в 03:15, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>:

> While the fix
> looks good to me, I have a few review comments on the regression test
> part:
>
> +-- bug #19616
> +-- pgoutput protocol compatibility could be broken for an aborted xact
> +-- discarded at spill eviction while a subxact remained in memory.
> +-- Stream Abort ('A'), valid only since protocol version 2, could be seen
> +-- with protocol version 1.
> ...
> +SELECT chr(get_byte(data, 0)) AS msgtype, count(*)
> +FROM pg_logical_slot_peek_binary_changes('regression_slot_pgoutput',
> NULL, NULL,
> + 'proto_version', '1', 'publication_names', 'spill_pub')
> +GROUP BY 1 ORDER BY 1;
>
> I'm not sure we should test the case against proto_version=1 for the
> reason I mentioned above. I think we can use proto_version=4 and
> streaming=0 instead. Also, the comment needs to be adjusted.
>
> stream.sql would be more suitable to put this test.
>
> Dear Sawada-san,

You are right, and these remarks have been addressed in v5 of the patch.

The spurious Stream Abort is not really about the protocol version.
So the real invariant is that a client which did not enable streaming
must not receive streaming messages. proto_version=1 only tested a
special case. proto_version=4 with streaming=0 exercises the
invariant directly. I confirmed the reformulated test still catches the bug.
I moved the test to stream.sql as you suggested, keeping the structure of
the already-reviewed comment.

--
Regards,
Rachitskiy Andrey