public inbox for [email protected]  

help / color / mirror / Atom feed

From: Zhijie Hou (Fujitsu) <[email protected]>
To: vignesh C <[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 04:28:52 +0000
Message-ID: <OS0PR01MB57161E8D61ACCF34D505FBDA9425A@OS0PR01MB5716.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CALDaNm0fWoS7eX9o3iVq8NwynpvAKKFK0YPEXWDTgBEOOjvFkw@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>

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?

Best Regard,
Hou zj

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: <OS0PR01MB57161E8D61ACCF34D505FBDA9425A@OS0PR01MB5716.jpnprd01.prod.outlook.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