On Wednesday, August 4, 2021 1:47 PM Masahiko Sawada <sawada.m...@gmail.com> > On Mon, Aug 2, 2021 at 10:52 PM houzj.f...@fujitsu.com > <houzj.f...@fujitsu.com> wrote: > > > > > > Hi hackers, > > > > When testing some other logical replication related patches, I found > > two unexpected behaviours about ALTER SUBSCRIPTION ADD/DROP > PUBLICATION. > > > > (1) > > when I execute the following sqls[1], the data of tables that > > registered to 'pub' wasn't copied to the subscriber side which means > > tablesync worker didn't start. > > > > -----sub originally had 2 pub nodes(pub,pub2) ALTER SUBSCRIPTION sub > > drop PUBLICATION pub; ALTER SUBSCRIPTION sub add PUBLICATION pub; > > ----- > > > > (2) > > And when I execute the following sqls, the data of table registered to > > 'pub2' > > are synced again. > > > > -----sub originally had 2 pub nodes(pub,pub2) ALTER SUBSCRIPTION sub > > drop PUBLICATION pub; ALTER SUBSCRIPTION sub REFRESH PUBLICATION; > > ----- > > I could reproduce this issue by the above steps.
Thanks for looking into this problem. > > I've not looked at the patch deeply yet but I think that the following one > line > change seems to fix the cases you reported, although I migit be missing > something: > > diff --git a/src/backend/commands/subscriptioncmds.c > b/src/backend/commands/subscriptioncmds.c > index 22ae982328..c74d6d51d6 100644 > --- a/src/backend/commands/subscriptioncmds.c > +++ b/src/backend/commands/subscriptioncmds.c > @@ -1068,7 +1068,7 @@ AlterSubscription(ParseState *pstate, > AlterSubscriptionStmt *stmt, > PreventInTransactionBlock(isTopLevel, "ALTER > SUBSCRIPTION with refresh"); > > /* Only refresh the added/dropped list of publications. */ > - sub->publications = stmt->publication; > + sub->publications = publist; > > AlterSubscription_refresh(sub, opts.copy_data); > } I considered the same fix before, but with the above fix, it seems refresh both existsing publications and new publication, which most of people didn't like in previous discussion[1]. From the doc[2], IIRC, Currently, the ADD/DROP PUBLICATION is supposed to only refresh the new publication. [1] https://www.postgresql.org/message-id/MEYP282MB166921C8622675A5C0701C31B6BB0%40MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM [2] https://www.postgresql.org/docs/14/sql-altersubscription.html By default, this command will also act like REFRESH PUBLICATION, except that in case of ADD or DROP, only the added or dropped publications are refreshed. > Also, I think we need to add some regression tests for this. > Currently, subscription.sql has tests for ADD/DROP PUBLICATION but they don't > check pg_subscription_rel. Currently, the subscription.sql doesn't have actual tables to be subscribed which means the pg_subscription_rel is empty. I am not sure where to place the testcases, temporarily placed in 001_rep_changes.pl. Best regards, houzj
v2-0001-fix-ALTER-SUB-ADD-DROP-PUBLICATION.patch
Description: v2-0001-fix-ALTER-SUB-ADD-DROP-PUBLICATION.patch