On Thursday, June 29, 2023 12:06 PM vignesh C <vignes...@gmail.com> wrote: > > On Wed, 28 Jun 2023 at 19:26, Ashutosh Bapat > <ashutosh.bapat....@gmail.com> wrote: > > > > Hi Vignesh, > > Thanks for working on this. > > > > On Wed, Jun 28, 2023 at 4:52 PM vignesh C <vignes...@gmail.com> wrote: > > > > > > Here is a patch having the fix for the same. I have not added any > > > tests as the existing tests cover this scenario. The same issue is > > > present in back branches too. > > > > Interesting, we have a test for this scenario and it accepts erroneous > > output :). > > > > > v1-0001-Call-pg_output_begin-in-pg_decode_message-if-it-i_master.pat > > > ch can be applied on master, PG15 and PG14, > > > v1-0001-Call-pg_output_begin-in-pg_decode_message-if-it-i_PG13.patch > > > patch can be applied on PG13, PG12 and PG11. > > > Thoughts? > > > > I noticed this when looking at Tomas's patches for logical decoding of > > sequences. The code block you have added is repeated in > > pg_decode_change() and pg_decode_truncate(). It might be better to > > push the conditions in pg_output_begin() itself so that any future > > callsite of pg_output_begin() automatically takes care of these > > conditions. > > Thanks for the comments, here is an updated patch handling the above issue.
Thanks for the patches. I tried to understand the following check: /* * If asked to skip empty transactions, we'll emit BEGIN at the point * where the first operation is received for this transaction. */ - if (data->skip_empty_xacts) + if (!(last_write ^ data->skip_empty_xacts) || txndata->xact_wrote_changes) return; I might miss something, but would you mind elaborating on why we use "last_write" in this check? Best Regard, Hou zj