Re: Handle infinite recursion in logical replication setup

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?

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?

-- 
With Regards,
Amit Kapila.