Re: Added schema level support for publication. - Mailing list pgsql-hackers

From vignesh C
Subject Re: Added schema level support for publication.
Date
Msg-id CALDaNm3LaUrgyj=dP0=WZ8yhtRJ3ArmnaYMO-NGNGFW17mKjnA@mail.gmail.com
Whole thread Raw
In response to Re: Added schema level support for publication.  (Amit Kapila <amit.kapila16@gmail.com>)
Responses Re: Added schema level support for publication.
List pgsql-hackers
On Mon, Sep 20, 2021 at 3:57 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Fri, Sep 17, 2021 at 5:40 PM vignesh C <vignesh21@gmail.com> wrote:
> >
> > On Thu, Sep 16, 2021 at 9:54 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > I think there is one more similar locking problem.
> > > AlterPublicationSchemas()
> > > {
> > > ..
> > > + if (stmt->action == DEFELEM_ADD)
> > > + {
> > > + List *rels;
> > > +
> > > + rels = GetPublicationRelations(pubform->oid, PUBLICATION_PART_ROOT);
> > > + RelSchemaIsMemberOfSchemaList(rels, schemaidlist, true);
> > > ...
> > > ...
> > > }
> > >
> > > Here, we don't have a lock on the relation. So, what if the relation
> > > is concurrently dropped after you get the rel list by
> > > GetPublicationRelations?
> >
> > This works fine without locking even after concurrent drop, I felt
> > this works because of MVCC.
> >
>
> Can you share the exact scenario you have tested? I think here it can
> give a wrong error because it might access invalid cache entry, so I
> think a lock is required here. Also, as said before, this might help
> to make the rel list consistent in function
> CheckObjSchemaNotAlreadyInPublication().

This is the scenario I tried:
create schema sch1;
create table sch1.t1 (c1 int);
create publication pub1 for table sch1.t1;
alter publication pub1 add all tables in schema sch1;  -- concurrently
drop table sch1.t1 from another session.

I will add the locking and changing of
CheckObjSchemaNotAlreadyInPublication in the next version.

Regards,
Vignesh



pgsql-hackers by date:

Previous
From: "Drouvot, Bertrand"
Date:
Subject: Re: Patch to avoid orphaned dependencies
Next
From: John Naylor
Date:
Subject: Re: mark the timestamptz variant of date_bin() as stable