User-Agent: Cyrus-JMAP/3.5.0-alpha0-4526-gbc24f4957e-fm-20220105.001-gbc24f495
Mime-Version: 1.0
Message-Id: <ef6295fa-678b-41bd-b2a8-50497dc95cb9@www.fastmail.com>
In-Reply-To: 
 <CAA4eK1KyiBPVOzvzpB9LEMnD2fPAGyU_SXaup0WG=mxuGV=Hnw@mail.gmail.com>
References: 
 <CAHut+Pv4os_ZtB4uJE6hi_Kp4px6e3WtPTX-7eTK9YJicqq1Cw@mail.gmail.com>
 <CAHut+PsUe3+crzQF_Ljivp5UgxkuEMBM76K=M+8fp41rXb-46Q@mail.gmail.com>
 <OS0PR01MB5716A7ADC776818C5C6F334494429@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <OS0PR01MB571620801232D197AFC1F2C394429@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <TYAPR01MB57240C054D95F0AE55618D2C94439@TYAPR01MB5724.jpnprd01.prod.outlook.com>
 <CAA4eK1J=G4t=e+4a=+qFx5QHpF41ce=2tD_ACMhtMJPzqBknUQ@mail.gmail.com>
 <CAA4eK1Ky0z=+UznCUHOs--L=Es_EMmZ_rxNo8FH73=758sahsQ@mail.gmail.com>
 <CAHut+PsjMh1fkYsAWqJsd_Ks=d0-CwUyp5rMYQf6kzOfyZUYhw@mail.gmail.com>
 <CAA4eK1KyiBPVOzvzpB9LEMnD2fPAGyU_SXaup0WG=mxuGV=Hnw@mail.gmail.com>
Date: Thu, 06 Jan 2022 10:11:56 -0300
From: "Euler Taveira" <euler@eulerto.com>
To: "Amit Kapila" <amit.kapila16@gmail.com>,
 "Peter Smith" <smithpb2250@gmail.com>
Cc: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>,
 "Greg Nancarrow" <gregn4422@gmail.com>, "vignesh C" <vignesh21@gmail.com>,
 "Ajin Cherian" <itsajin@gmail.com>,
 "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>,
 "Dilip Kumar" <dilipbalaut@gmail.com>,
 "Rahila Syed" <rahilasyed90@gmail.com>,
 "Peter Eisentraut" <peter.eisentraut@enterprisedb.com>,
 =?UTF-8?Q?=C3=96nder_Kalac=C4=B1?= <onderkalaci@gmail.com>,
 japin <japinli@hotmail.com>, "Michael Paquier" <michael@paquier.xyz>,
 "David Steele" <david@pgmasters.net>, "Craig Ringer" <craig@2ndquadrant.com>,
 "Amit Langote" <amitlangote09@gmail.com>,
 "PostgreSQL Hackers" <pgsql-hackers@lists.postgresql.org>
Subject: Re: row filtering for logical replication
Content-Type: multipart/alternative;
 boundary=9419105c109443bcb0c2a03f0e76a8ba
Archived-At: <https://www.postgresql.org/message-id/ef6295fa-678b-41bd-b2a8-50497dc95cb9%40www.fastmail.com>
Precedence: bulk

--9419105c109443bcb0c2a03f0e76a8ba
Content-Type: text/plain

On Thu, Jan 6, 2022, at 1:18 AM, Amit Kapila wrote:
> On Thu, Jan 6, 2022 at 8:43 AM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > On Wed, Jan 5, 2022 at 9:52 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > ...
> >
> > > Another minor comment:
> > > +static bool pgoutput_row_filter(enum ReorderBufferChangeType changetype,
> > >
> > > Do we need to specify the 'enum' type before changetype parameter?
> > >
> >
> > That is because there is currently no typedef for the enum
> > ReorderBufferChangeType.
> >
> 
> But I see that the 0002 patch is already adding the required typedef.
IMO we shouldn't reuse ReorderBufferChangeType. For a long-term solution, it is
fragile. ReorderBufferChangeType has values that do not matter for row filter
and it relies on the fact that REORDER_BUFFER_CHANGE_INSERT,
REORDER_BUFFER_CHANGE_UPDATE and REORDER_BUFFER_CHANGE_DELETE are the first 3
values from the enum, otherwise, it breaks rfnodes and no_filters in
pgoutput_row_filter(). I suggest a separate enum that contains only these 3
values.

enum RowFilterPublishAction {
   PUBLISH_ACTION_INSERT,
   PUBLISH_ACTION_UPDATE,
   PUBLISH_ACTION_DELETE
};


--
Euler Taveira
EDB   https://www.enterprisedb.com/

--9419105c109443bcb0c2a03f0e76a8ba
Content-Type: text/html
Content-Transfer-Encoding: quoted-printable

<!DOCTYPE html><html><head><title></title><style type=3D"text/css">p.Mso=
Normal,p.MsoNoSpacing{margin:0}</style></head><body><div>On Thu, Jan 6, =
2022, at 1:18 AM, Amit Kapila wrote:<br></div><blockquote type=3D"cite" =
id=3D"qt" style=3D""><div>On Thu, Jan 6, 2022 at 8:43 AM Peter Smith &lt=
;<a href=3D"mailto:smithpb2250@gmail.com">smithpb2250@gmail.com</a>&gt; =
wrote:<br></div><div>&gt;<br></div><div>&gt; On Wed, Jan 5, 2022 at 9:52=
 PM Amit Kapila &lt;<a href=3D"mailto:amit.kapila16@gmail.com">amit.kapi=
la16@gmail.com</a>&gt; wrote:<br></div><div>&gt; &gt;<br></div><div>&gt;=
 ...<br></div><div>&gt;<br></div><div>&gt; &gt; Another minor comment:<b=
r></div><div>&gt; &gt; +static bool pgoutput_row_filter(enum ReorderBuff=
erChangeType changetype,<br></div><div>&gt; &gt;<br></div><div>&gt; &gt;=
 Do we need to specify the 'enum' type before changetype parameter?<br><=
/div><div>&gt; &gt;<br></div><div>&gt;<br></div><div>&gt; That is becaus=
e there is currently no typedef for the enum<br></div><div>&gt; ReorderB=
ufferChangeType.<br></div><div>&gt;<br></div><div><br></div><div>But I s=
ee that the 0002 patch is already adding the required typedef.<br></div>=
</blockquote><div>IMO we shouldn't reuse ReorderBufferChangeType. For a =
long-term solution, it is<br></div><div>fragile. ReorderBufferChangeType=
 has values that do not matter for row filter<br></div><div>and it relie=
s on the fact that REORDER_BUFFER_CHANGE_INSERT,<br></div><div>REORDER_B=
UFFER_CHANGE_UPDATE and REORDER_BUFFER_CHANGE_DELETE are the first 3<br>=
</div><div>values from the enum, otherwise, it breaks rfnodes and no_fil=
ters in<br></div><div>pgoutput_row_filter(). I suggest a separate enum t=
hat contains only these 3<br></div><div>values.<br></div><div><br></div>=
<div>enum RowFilterPublishAction {<br></div><div>&nbsp;&nbsp; PUBLISH_AC=
TION_INSERT,<br></div><div>&nbsp;&nbsp; PUBLISH_ACTION_UPDATE,<br></div>=
<div>&nbsp;&nbsp; PUBLISH_ACTION_DELETE<br></div><div>};<br></div><div><=
br></div><div><br></div><div id=3D"sig112409080"><div class=3D"signature=
">--<br></div><div class=3D"signature">Euler Taveira<br></div><div class=
=3D"signature">EDB&nbsp;&nbsp; <a href=3D"https://www.enterprisedb.com/"=
>https://www.enterprisedb.com/</a><br></div></div><div><br></div></body>=
</html>
--9419105c109443bcb0c2a03f0e76a8ba--