Re: Added schema level support for publication.

On Mon, Sep 20, 2021 at 4:20 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Mon, Sep 20, 2021 at 3:57 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Fri, Sep 17, 2021 at 5:40 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > >
> > > On Thu, Sep 16, 2021 at 9:54 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)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.

I have made the changes for the above at the v30 patch attached at [1].
[1] - https://www.postgresql.org/message-id/CALDaNm3aFtXpkD4m28-ENG9F4faBEVdGNUrEhgKV0pHr2S_C2g%40mail.gmail.com

Regards,
Vignesh