Re: Handle infinite recursion in logical replication setup

I didn't find this in https://commitfest.postgresql.org/37/. Is this
somewhere in the commitfest?

On Fri, Apr 1, 2022 at 12:46 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Thu, Mar 31, 2022 at 2:52 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Thu, Mar 31, 2022 at 9:14 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Wed, Mar 30, 2022 at 7:40 PM Ashutosh Bapat
> > > <ashutosh.bapat.oss@gmail.com> wrote:
> > > > >
> > > > > The changes for the same are available int the v5 patch available at [1].
> > > > > [1] - https://www.postgresql.org/message-id/CALDaNm3wCf0YcvVo%2BgHMGpupk9K6WKJxCyLUvhPC2GkPKRZUWA%40mail.gmail.com
> > > > >
> > > >
> > > >      cb->truncate_cb = pg_decode_truncate;
> > > >      cb->commit_cb = pg_decode_commit_txn;
> > > >      cb->filter_by_origin_cb = pg_decode_filter;
> > > > +    cb->filter_remote_origin_cb = pg_decode_filter_remote_origin;
> > > >
> > > > Why do we need a new hook? Can we use filter_by_origin_cb?
> > > >
> > >
> > > I also think there is no need for a new hook in this case but I might
> > > also be missing something that Vignesh has in his mind.
> > >
> > > > Also it looks like
> > > > implementation of pg_decode_filter and pg_decode_filter_remote_origin is same,
> > > > unless my eyes are deceiving me.
> > > >
> > > > -      <literal>binary</literal>, <literal>streaming</literal>, and
> > > > -      <literal>disable_on_error</literal>.
> > > > +      <literal>binary</literal>, <literal>streaming</literal>,
> > > > +      <literal>disable_on_error</literal> and
> > > > +      <literal>publish_local_only</literal>.
> > > >
> > > > "publish_local_only" as a "subscription" option looks odd. Should it be
> > > > "subscribe_local_only"?
> > > >
> > >
> > > I also think "subscribe_local_only" fits better here.
> > >
> >
> > About 0002 patch,
> > Commit message:
> > ------
> > If a subscription is created to Node1 from Node3 with publish_local_only
> > and copy_data as ON, then throw an error so that user can handle
> > creation of subscription with table having consistent data.
> > ------
> > Do you want to refer to Node2 instead of Node3 here as Node3 doesn't
> > make sense in the description?
>
> It should be Node3, I have added more details in the commit message
> mentioning about the scenario.
>
> > Also, you haven't explained anywhere in the patch why
> > 'publish_local_only' (or whatever we call it) won't work for initial
> > sync? IIUC, it is because we can identify origin changes only based on
> > WAL and initial sync directly copies data from the heap. Am, I missing
> > something here?
>
> I have added the explanation where we are throwing the error.
>
> The v6 patch at [1] has the changes for the same.
> [1] - https://www.postgresql.org/message-id/CALDaNm16eBx2BjLFjfFHSU4pb25HmgV--M692OPgH_91Dwn%3D2g%40mail.gmail.com
>
> Regards,
> Vignesh



-- 
Best Wishes,
Ashutosh Bapat