public inbox for [email protected]  
help / color / mirror / Atom feed
From: vignesh C <[email protected]>
To: Amit Kapila <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Ashutosh Bapat <[email protected]>
Subject: Re: pg_decode_message vs skip_empty_xacts and xact_wrote_changes
Date: Mon, 3 Jul 2023 16:49:40 +0530
Message-ID: <CALDaNm3Hj72XtEcn_nRm4t4c59gaYCJk=-aeaJT47ktoAmBUNQ@mail.gmail.com> (raw)
In-Reply-To: <CAA4eK1J-_g8ruuOKTXA7sGjRASBG7XsXpHzpz324ygEd+HmsvA@mail.gmail.com>
References: <CAExHW5ujRhbOz6_aTq_jQA8NjeFqq9d_8G9viShWvXx8gdSXiQ@mail.gmail.com>
	<CAA4eK1JLRiW_Mkps+oGu_a1DZHJqhoXV9Ru33B4hV5UYUtLPJg@mail.gmail.com>
	<CALDaNm1bfwt=hmZD7V17t-giJLw3kndLBxLzSD+oSXNfMmGVsw@mail.gmail.com>
	<CAExHW5sqY8AS5WqyTD1v=V+pAZvtHXcZ5++2z1fn7NcpBmhj_g@mail.gmail.com>
	<CALDaNm0fWoS7eX9o3iVq8NwynpvAKKFK0YPEXWDTgBEOOjvFkw@mail.gmail.com>
	<OS0PR01MB57161E8D61ACCF34D505FBDA9425A@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<CALDaNm14bBNi60JaveK-WDgjYsDxa5ArRWzj2JrjQgc9CBzYiw@mail.gmail.com>
	<CAA4eK1J-_g8ruuOKTXA7sGjRASBG7XsXpHzpz324ygEd+HmsvA@mail.gmail.com>

On Fri, 30 Jun 2023 at 09:55, Amit Kapila <[email protected]> wrote:
>
> On Thu, Jun 29, 2023 at 9:40 PM vignesh C <[email protected]> wrote:
> >
> > On Thu, 29 Jun 2023 at 09:58, Zhijie Hou (Fujitsu)
> > <[email protected]> wrote:
> > >
> > > On Thursday, June 29, 2023 12:06 PM vignesh C <[email protected]> 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






view thread (7+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: pg_decode_message vs skip_empty_xacts and xact_wrote_changes
  In-Reply-To: <CALDaNm3Hj72XtEcn_nRm4t4c59gaYCJk=-aeaJT47ktoAmBUNQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox