On Thu, Mar 17, 2022 at 10:43 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> Review comments/suggestions:
> =========================
> 1. Isn't it sufficient to call pgoutput_send_begin from
> maybe_send_schema as that is commonplace for all others and is always
> the first message we send? If so, I think we can remove it from other
> places?
I've done the other way, I've removed it from maybe_send_schema as we
always call this
prior to calling maybe_send_schema.
> 2. Can we write some comments to explain why we don't skip streaming
> or prepared empty transactions and some possible solutions (the
> protocol change and additional subscription parameter as discussed
> [1]) as discussed in this thread pgoutput.c?
I've added comment in the header of pgoutput_begin_prepare_txn() and
pgoutput_stream_start()
> 3. Can we add a simple test for it in one of the existing test
> files(say in 001_rep_changes.pl)?
added a simple test.
> 4. I think we can drop the skip streaming patch as we can't do that for now.
Dropped,
In addition, I have also added a few more comments explaining why the begin send
is delayed in pgoutput_change till row_filter is checked and also ran pgindent.
regards,
Ajin Cherian
Fujitsu Australia