Re: pg_decode_message vs skip_empty_xacts and xact_wrote_changes - Mailing list pgsql-hackers
From | vignesh C |
---|---|
Subject | Re: pg_decode_message vs skip_empty_xacts and xact_wrote_changes |
Date | |
Msg-id | CALDaNm14bBNi60JaveK-WDgjYsDxa5ArRWzj2JrjQgc9CBzYiw@mail.gmail.com Whole thread Raw |
In response to | RE: pg_decode_message vs skip_empty_xacts and xact_wrote_changes ("Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>) |
Responses |
Re: pg_decode_message vs skip_empty_xacts and xact_wrote_changes
|
List | pgsql-hackers |
On Thu, 29 Jun 2023 at 09:58, Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> wrote: > > On Thursday, June 29, 2023 12:06 PM vignesh C <vignesh21@gmail.com> wrote: > > > > On Wed, 28 Jun 2023 at 19:26, Ashutosh Bapat > > <ashutosh.bapat.oss@gmail.com> wrote: > > > > > > Hi Vignesh, > > > Thanks for working on this. > > > > > > On Wed, Jun 28, 2023 at 4:52 PM vignesh C <vignesh21@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? last_write is used to indicate if it is begin/"begin prepare"(last_write is true) or change/truncate/message(last_write is false). We have specified logical XNOR which will be true for the following conditions: Condition1: last_write && data->skip_empty_xacts -> If it is begin/begin prepare and user has specified skip empty transactions, we will return from here, so that the begin message can be appended at the point where the first operation is received for this transaction. Condition2: !last_write && !data->skip_empty_xacts -> If it is change/truncate or message and user has not specified skip empty transactions, we will return from here as we would have appended the begin earlier itself. The txndata->xact_w6rote_changes will be set after the first operation is received for this transaction during which we would have outputted the begin message, this condition is to skip outputting begin message if the begin message was already outputted. Regards, Vignesh
pgsql-hackers by date: