Re: [BUG] wrong refresh when ALTER SUBSCRIPTION ADD/DROP PUBLICATION

On Mon, Aug 23, 2021 at 11:05 PM houzj(dot)fnst(at)fujitsu(dot)com
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Mon, Aug 23, 2021 8:01 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > On Mon, Aug 23, 2021 at 2:45 PM houzj(dot)fnst(at)fujitsu(dot)com<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > >
> > > On Mon, Aug 23, 2021 12:59 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > >
> > > > On Sat, Aug 7, 2021 at 6:53 PM houzj(dot)fnst(at)fujitsu(dot)com <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > > > >
> > > > > Personally, I also think it will be better to make the behavior consistent.
> > > > > Attach the new version patch make both ADD and DROP behave the
> > > > > same as SET PUBLICATION which refresh all the publications.
> > > > >
> > > >
> > > > I think we can have tests in the separate test file
> > > > (alter_sub_pub.pl) like you earlier had in one of the versions. Use
> > > > some meaningful names for tables instead of temp1, temp2 as you had in the
> > previous version.
> > > > Otherwise, the code changes look good to me.
> > >
> > > Thanks for the comment.
> > > Attach new version patch which did the following changes.
> > >
> > > * move the tests to a separate test file (024_alter_sub_pub.pl)
> > > * adjust the document of copy_data option
> > > * add tab-complete for copy_data option when ALTER DROP publication.
> > >
> >
> > Thanks, the patch looks good to me. I have made some cosmetic changes in the
> > attached version. It makes the test cases easier to understand.
> > I am planning to push this tomorrow unless there are more comments or
> > suggestions.
>
> Thanks ! The patch looks good to me.
>
> I noticed that the patch cannot be applied to PG14-stable,
> so I attach a separate patch for back-patch(I didn’t change the patch for HEAD branch).

Thanks. The patch for HEAD looks good to me. Regarding the patch for
PG14, I think we need to update the comment as well:

@@ -987,7 +986,7 @@ AlterSubscription(AlterSubscriptionStmt *stmt,
bool isTopLevel)
PreventInTransactionBlock(isTopLevel, "ALTER SUBSCRIPTION with refresh");

/* Only refresh the added/dropped list of publications. */
- sub->publications = stmt->publication;
+ sub->publications = publist;

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/