On Fri, 30 Jun 2023 at 09:55, Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Jun 29, 2023 at 9:40 PM vignesh C <vignes...@gmail.com> wrote: > > > > On Thu, 29 Jun 2023 at 09:58, Zhijie Hou (Fujitsu) > > <houzj.f...@fujitsu.com> wrote: > > > > > > On Thursday, June 29, 2023 12:06 PM vignesh C <vignes...@gmail.com> wrote: > > > > > > > > > > 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. > > > > I feel the use of last_write has reduced the readability of this part > of the code. It may be that we can add comments to make it clear but I > feel your previous version was much easier to understand.
+1 for the first version patch, I also felt the first version is easily understandable. Regards, Vignesh