RE: Open a streamed block for transactional messages during decoding

On Thursday, October 26, 2023 12:42 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Tue, Oct 24, 2023 at 5:27 PM Zhijie Hou (Fujitsu) <houzj(dot)fnst(at)fujitsu(dot)com>
> wrote:
> >
> > While reviewing the test_decoding code, I noticed that when
> > skip_empty_xacts option is specified, it doesn't open the streaming
> block( e.g.
> > pg_output_stream_start) before streaming the transactional MESSAGE
> > even if it's the first change in a streaming block.
> >
> > It looks inconsistent with what we do when streaming DML changes(e.g.
> > pg_decode_stream_change()).
> >
> > Here is a small patch to open the stream block in this case.
> >
>
> The change looks good to me though I haven't tested it yet. BTW, can we
> change the comment: "Output stream start if we haven't yet, but only for the
> transactional case." to "Output stream start if we haven't yet for transactional
> messages"?

Thanks for the review and I changed this as suggested.

> I think we should backpatch this fix. What do you think?

I think maybe we can improve the code only for HEAD, as skip_empty_xacts is
primarily used to have consistent test results across different runs and this
patch won't help with that. And I saw in 26dd028, we didn't backpatch for the
same reason.

Best Regards,
Hou zj