Re: Handle infinite recursion in logical replication setup

On Sat, Jul 16, 2022 at 9:05 AM vignesh C <vignesh21@gmail.com> wrote:
>
> On Thu, Jul 14, 2022 at 6:42 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Thu, 14 Jul 2022 at 6:34 PM, vignesh C <vignesh21@gmail.com> wrote:
> >>
> >> On Thu, Jul 14, 2022 at 11:26 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >> >
> >> > On Wed, Jul 13, 2022 at 4:49 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >> > >
> >> > > On Tue, Jul 12, 2022 at 2:58 PM vignesh C <vignesh21@gmail.com> wrote:
> >> > > >
> >> > > > On Tue, Jul 12, 2022 at 9:51 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >> > >
> >> > > I find one thing  confusing about this patch.  Basically, this has two
> >> > > option 'local' and 'any', so I would assume that all the local server
> >> > > changes should be covered under the 'local' but now if we set some
> >> > > origin using 'select pg_replication_origin_session_setup('aa');' then
> >> > > changes from that session will be ignored because it has an origin id.
> >> > > I think actually the name is creating confusion, because by local it
> >> > > seems like a change which originated locally and the document is also
> >> > > specifying the same.
> >> > >
> >> > > +       If <literal>local</literal>, the subscription will request the publisher
> >> > > +       to only send changes that originated locally. If <literal>any</literal>,
> >> > >
> >> > > I think if we want to keep the option local then we should look up all
> >> > > the origin in the replication origin catalog and identify whether it
> >> > > is a local origin id or remote origin id and based on that filter out
> >> > > the changes.
> >> >
> >> > On the other hand if we are interested in receiving the changes which
> >> > are generated without any origin then I think we should change 'local'
> >> > to 'none' and then in future we can provide a new option which can
> >> > send the changes generated by all the local origin?  I think other
> >> > than this the patch LGTM.
> >>
> >> Thanks for the comment. The attached v33 patch has the changes to
> >> specify origin as 'none' instead of 'local' which will not publish the
> >> data having any origin.
> >
> >
> > I think the ‘none’ might have problem from expand ability pov?  what if in future we support the actual origin name
andthan what none mean? no origin or origin name none?  Should we just give origin name empty name ‘’?  Or is there
someother issue? 
>
> Currently there is no restriction in the name we can specify for
> origin, ex any, none, local, etc all are allowed as origin name. How
> about extending it with another parameter "origin_name" when we
> support filtering of a particular origin like:
> 1) origin = name, origin_name = 'orig1' -- Means that the publisher
> will filter the changes having origin name as 'orig1' and send the
> other changes.
> 2) origin = any -- Means that the publisher sends all changes
> regardless of their origin.
> 3) origin = none -- Means that the subscription will request the
> publisher to only send changes that have no origin associated.
> When we need to specify filtering of a particular origin name we will
> have to use both origin and origin_name option, like origin = name,
> origin_name = 'orig1' as in the first example.
>
> I'm not sure if there is a simpler way to do this with only a single
> option as both 'none' and 'any' can be specified as the origin name.
> Thoughts?

I think giving two options would be really confusing from the
usability perspective.  I think what we should be doing here is to
keep these three names 'none', 'any' and 'local' as reserved names for
the origin name so that those are not allowed to be set by the user
and they have some internal meaning.  And I don't think this is going
to create too much trouble for anyone because those are not really the
names someone wants to use for their replication origin.  So I think
pg_replication_origin_create() we can also check for the reserved
names just for the replication origin.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com