From Thurs, Sep 23, 2021 12:09 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Wed, Sep 22, 2021 at 5:03 PM Hou Zhijie <houzj.fnst@fujitsu.com> wrote:
> >
> > Personally, I think we'd better move the code about changing publication's
> > tablelist into AlterPublicationTables and the code about changing
> publication's
> > schemalist into AlterPublicationSchemas. It's similar to what the
> v29-patchset
> > did, the difference is the SET action, I suggest we drop all the tables in
> > function AlterPublicationTables when user only set schemas and drop all the
> > schema in AlterPublicationSchemas when user only set tables. In this
> approach,
> > we can keep schema and relation code separate and don't need to worry
> > about the locking order.
> >
> > Attach a top-up patch which refactor the code like above.
> >
>
> Good suggestion. I think it would still be better if we can move the
> checks related to superuser and puballtables into a separate function
> that gets called before taking a lock on publication.
I agreed.
I noticed v30-0001 has been committed with some minor changes, and the V30-0002
patchset need to be rebased accordingly. Attach a rebased version patch set to
make cfbot happy. Also Attach the two top-up patches which refactor the code as
suggested. (top-up patch 1 is to keep schema and table code separate, top-up
patch 2 is to move some cheap check into a function and invoke it before
locking.)
Best regards,
Hou zj