On Fri, Nov 12, 2021 at 3:49 PM Ajin Cherian <itsajin@gmail.com> wrote:
>
> Attaching version 39-
>
> V39 fixes the following review comments:
>
> On Fri, Nov 5, 2021 at 7:49 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > CheckObjSchemaNotAlreadyInPublication(rels, schemaidlist,
> > PUBLICATIONOBJ_TABLE);
> >
> >I think for the correct merge you need to just call
> >CheckObjSchemaNotAlreadyInPublication() before this for loop. BTW, I
> >have a question regarding this implementation. Here, it has been
> >assumed that the new rel will always be specified with a different
> >qual, what if there is no qual or if the qual is the same?
>
> Actually with this code, no qual or a different qual does not matter,
> it recreates everything as specified by the ALTER SET command.
> I have added CheckObjSchemaNotAlreadyInPublication as you specified since this
> is required to match the schema patch behaviour. I've also added
> a test case that tests this particular case.
>
What I meant was that with this new code we have regressed the old
behavior. Basically, imagine a case where no filter was given for any
of the tables. Then after the patch, we will remove all the old tables
whereas before the patch it will remove the oldrels only when they are
not specified as part of new rels. If you agree with this, then we can
retain the old behavior and for the new tables, we can always override
the where clause for a SET variant of command.
--
With Regards,
Amit Kapila.
