On Thu, Mar 17, 2022 at 10:43 PM Amit Kapila <amit.kapil...@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
v26-0001-Skip-empty-transactions-for-logical-replication.patch
Description: Binary data
