public inbox for [email protected]  
help / color / mirror / Atom feed
From: vignesh C <[email protected]>
To: Zhijie Hou (Fujitsu) <[email protected]>
Cc: Amit Kapila <[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: Thu, 29 Jun 2023 21:39:46 +0530
Message-ID: <CALDaNm14bBNi60JaveK-WDgjYsDxa5ArRWzj2JrjQgc9CBzYiw@mail.gmail.com> (raw)
In-Reply-To: <OS0PR01MB57161E8D61ACCF34D505FBDA9425A@OS0PR01MB5716.jpnprd01.prod.outlook.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>

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:
> >
> > On Wed, 28 Jun 2023 at 19:26, Ashutosh Bapat
> > <[email protected]> wrote:
> > >
> > > Hi Vignesh,
> > > Thanks for working on this.
> > >
> > > On Wed, Jun 28, 2023 at 4:52 PM vignesh C <[email protected]> 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






view thread (8+ 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: <CALDaNm14bBNi60JaveK-WDgjYsDxa5ArRWzj2JrjQgc9CBzYiw@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