Re: pg_decode_message vs skip_empty_xacts and xact_wrote_changes

From: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pg_decode_message vs skip_empty_xacts and xact_wrote_changes
Date: 2023-06-28 13:55:48
Message-ID: CAExHW5sqY8AS5WqyTD1v=V+pAZvtHXcZ5++2z1fn7NcpBmhj_g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Vignesh,
Thanks for working on this.

On Wed, Jun 28, 2023 at 4:52 PM vignesh C <vignesh21(at)gmail(dot)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.patch
> 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.

Otherwise the patches look good to me.

--
Best Wishes,
Ashutosh Bapat

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2023-06-28 14:09:41 Re: Assert !bms_overlap(joinrel->relids, required_outer)
Previous Message Alena Rybakina 2023-06-28 13:53:06 Re: MergeJoin beats HashJoin in the case of multiple hash clauses