MIME-Version: 1.0
References: <CALDaNm0gwjY_4HFxvvty01BOT01q_fJLKQ3pWP9=9orqubhjcQ@mail.gmail.com>
 <TYAPR01MB5866C758B1185217950ED13CF5029@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <CALDaNm0WSo5369pr2eN1obTGBeiJU9cQdF6Ju1sC4hMQNy5BfQ@mail.gmail.com>
 <CAHut+PsAWaETh9VMymbBfMrqiE1KuqMq+wpBg0s7eMzwLATr+w@mail.gmail.com>
 <CALDaNm3GQFVGeKg18wUe1-toXd_80=AW+JOW9KQySVbL8Tz4WA@mail.gmail.com>
 <CAHut+PtRxiQR_4UFLNThg-NNRV447FvwtcR-BvqMzjyMJXKwfw@mail.gmail.com>
 <CALDaNm2ttHxORgvtzd3ZFZe7-uLjfrAX3WSM-Wxf5Et_CVm41Q@mail.gmail.com> <CAHut+PvQonJd5epJBM0Yfh1499mL9kTL9a=GrMhvnL6Ok05zqw@mail.gmail.com>
In-Reply-To: <CAHut+PvQonJd5epJBM0Yfh1499mL9kTL9a=GrMhvnL6Ok05zqw@mail.gmail.com>
From: vignesh C <vignesh21@gmail.com>
Date: Mon, 7 Mar 2022 16:23:17 +0530
Message-ID: <CALDaNm3jkotRhKfCqu5CXOf36_yiiW_cYE5=bG=j6N3gOWJkqw@mail.gmail.com>
Subject: Re: Handle infinite recursion in logical replication setup
To: Peter Smith <smithpb2250@gmail.com>
Cc: "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>, 
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Archived-At: <https://www.postgresql.org/message-id/CALDaNm3jkotRhKfCqu5CXOf36_yiiW_cYE5%3DbG%3Dj6N3gOWJkqw%40mail.gmail.com>
Precedence: bulk

On Mon, Mar 7, 2022 at 1:45 PM Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Mon, Mar 7, 2022 at 6:17 PM vignesh C <vignesh21@gmail.com> wrote:
> >
> > On Mon, Mar 7, 2022 at 11:45 AM Peter Smith <smithpb2250@gmail.com> wrote:
> > >
> > > On Mon, Mar 7, 2022 at 4:20 PM vignesh C <vignesh21@gmail.com> wrote:
> > > >
> > > > On Mon, Mar 7, 2022 at 10:26 AM Peter Smith <smithpb2250@gmail.com> wrote:
> > > > >
> > > > > Hi Vignesh, I also have not looked at the patch yet, but I have what
> > > > > seems like a very fundamental (and possibly dumb) question...
> > > > >
> > > > > Basically, I do not understand the choice of syntax for setting things up.
> > > > >
> > > > > IMO that "only-local" option sounds very similar to the other
> > > > > PUBLICATION ("publish") options which decide the kinds of things that
> > > > > will be published. So it feels more natural for me to think of the
> > > > > publisher as being the one to decide what will be published.
> > > > >
> > > > > e.g.
> > > > >
> > > > > option 1:
> > > > > CREATE PUBLICATION p1 FOR TABLE t1;
> > > > > CREATE SUBSCRITION s1 ... FOR PUBLICATION p1 WITH (only_local = true);
> > > > >
> > > > > option 2:
> > > > > CREATE PUBLICATION p1 FOR TABLE t1 WEHRE (publish = 'only_local');
> > > > > CREATE SUBSCRITION s1 ... FOR PUBLICATION p1;
> > > > >
> > > > > ~~
> > > > >
> > > > > IIUC the patch is using option 1. My first impression was it feels
> > > > > back-to-front for the SUBSCRIPTION telling the PUBLICATION what to
> > > > > publish.
> > > > >
> > > > > So, why does the patch use syntax option 1?
> > > >
> > > > I felt the advantage with keeping it at the subscription side is that,
> > > > the subscriber from one node can subscribe with only_local option on
> > > > and a different subscriber from a different node can subscribe with
> > > > only_local option as off. This might not be possible with having the
> > > > option at publisher side. Having it at the subscriber side might give
> > > > more flexibility for the user.
> > > >
> > >
> > > OK.  Option 2 needs two publications for that scenario. IMO it's more
> > > intuitive this way, but maybe you wanted to avoid the extra
> > > publications?
> >
> > Yes, I wanted to avoid the extra publication creation that you pointed
> > out. Option 1 can handle this scenario without creating the extra
> > publications:
> > node0: CREATE PUBLICATION p1 FOR TABLE t1;
> > node1: CREATE SUBSCRIPTION s1 ... FOR PUBLICATION p1 with (only_local = on);
> > node2:  CREATE SUBSCRIPTION s1 ... FOR PUBLICATION p1 with (only_local = off);
> >
> > I'm ok with both the approaches, now that this scenario can be handled
> > by using both the options. i.e providing only_local option as an
> > option while creating publication or providing only_local option as an
> > option while creating subscription as Peter has pointed out at [1].
> > option 1:
> > CREATE PUBLICATION p1 FOR TABLE t1;
> > CREATE SUBSCRITION s1 ... FOR PUBLICATION p1 WITH (only_local = true);
> >
> > option 2:
> > CREATE PUBLICATION p1 FOR TABLE t1 WITH (publish = 'only_local');
> > CREATE SUBSCRITION s1 ... FOR PUBLICATION p1;
> >
> > Shall we get a few opinions on this and take it in that direction?
> >
> > [1] - https://www.postgresql.org/message-id/CAHut%2BPsAWaETh9VMymbBfMrqiE1KuqMq%2BwpBg0s7eMzwLATr%2Bw%40mail.gmail.com
> >
> > Regards,
> > Vignesh
>
> BTW here is a counter-example to your scenario from earlier.
>
> Let's say I have a publication p1 and p2 and want to subscribe to p1
> with only_local=true, and p2 with only_local = false;
>
> Using the current OPtion 1 syntax you cannot do this with a single
> subscription because the option is tied to the subscription.
> But using syntax Option 2 you may be able to do it.
>
> Option 1:
> CREATE PUBLICATION p1 FOR TABLE t1;
> CREATE PUBLICATION p2 FOR TABLE t2;
> CREATE SUBSCRIPTION s1 ... FOR PUBLICATION p1 WITH (local_only = true);
> CREATE SUBSCRIPTION s2 ... FOR PUBLICATION p1 WITH (local_only = false);
>
> Option 2:
> CREATE PUBLICATION p1 FOR TABLE t1 WITH (publish = 'local_only');
> CREATE PUBLICATION p2 FOR TABLE t2;
> CREATE SUBSCRIPTION s1 ... FOR PUBLICATION p1, p2;

I felt having multiple publications will create duplicate entries in
the system table, Amit also has pointed this at [1]. Also enhancing
this approach to support filtering based on replication origin which
is suggested by dilip at [2] is also on the client side and also the
initial check to handle the copy_data specified by Amit at [3] will be
done by the client side. Based on the above I feel the existing
approach is better. I might be missing something here.

[1] - https://www.postgresql.org/message-id/CAA4eK1LgCVv8u-fOsMPbGC96sWXhT3EKOBAeFW3g84otjStztw%40mail.gmail.com
[2] - https://www.postgresql.org/message-id/CAFiTN-tKbjHDjAFNnqRoR8u1B%2Bfs0wunGz%3D3wp0iU-sUaxZJTQ%40mail.gmail.com
[3] - https://www.postgresql.org/message-id/CAA4eK1%2Bco2cd8a6okgUD_pcFEHcc7mVc0k_RE2%3D6ahyv3WPRMg%40mail.gmail.com

Regards,
Vignesh