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

On Fri, Aug 14, 2026 at 7:35 AM Andrey Rachitskiy <pl0h0yp1(at)gmail(dot)com> wrote:
>
>
> пт, 14 авг. 2026 г. в 17:33, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
>>
>> One minor comment: ReorderBufferMaybeMarkTXNStreamed() seems to rely on
>> the assumption that it is never called for a top-level transaction when
>> streaming is disabled. If that's correct, how about documenting this
>> assumption in the function comment and/or adding an assertion such as:
>>
>> if (rbtxn_is_toptxn(txn))
>> {
>> Assert(ReorderBufferCanStream(rb));
>> txn->txn_flags |= RBTXN_IS_STREAMED;
>>
> Dear Fujii-san,
>
> Thanks for the review.
> v4 with assert and comment, in attachment.

Thank you for creating the patch.

IIUC it's not only a protocol compatibility issue but also an issue
that streaming messages are sent even with the streaming mode being
disabled. Therefore, it can be hit even with the latest protocol
version if subscribers disable the streaming option. 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.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com