From 9beda5d8a3189d0c189434eac6e5572d8b2f1eac Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Sat, 15 May 2021 14:49:22 +0530 Subject: [PATCH v5] Fixes in ALTER SUBSCRIPTION DROP PUBLICATION code ALTER SUBSCRIPTION DROP PUBLICATION does not actually support copy_data option, so remove it from tab completion. Also, reword the error message that is thrown when all the publications from a subscription are specified to be dropped. Also, made few doc and cosmetic adjustments. Author: Vignesh C Reviewed-by: Bharath Rupireddy, Japin Li, Tom Lane --- doc/src/sgml/ref/alter_subscription.sgml | 20 ++++++++++---------- src/backend/commands/subscriptioncmds.c | 16 +++++++++++----- src/bin/psql/tab-complete.c | 9 ++++++--- src/test/regress/expected/subscription.out | 2 +- 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml index 367ac814f4..177bb372aa 100644 --- a/doc/src/sgml/ref/alter_subscription.sgml +++ b/doc/src/sgml/ref/alter_subscription.sgml @@ -22,9 +22,9 @@ PostgreSQL documentation ALTER SUBSCRIPTION name CONNECTION 'conninfo' -ALTER SUBSCRIPTION name SET PUBLICATION publication_name [, ...] [ WITH ( set_publication_option [= value] [, ... ] ) ] -ALTER SUBSCRIPTION name ADD PUBLICATION publication_name [, ...] [ WITH ( set_publication_option [= value] [, ... ] ) ] -ALTER SUBSCRIPTION name DROP PUBLICATION publication_name [, ...] [ WITH ( set_publication_option [= value] [, ... ] ) ] +ALTER SUBSCRIPTION name SET PUBLICATION publication_name [, ...] [ WITH ( publication_option [= value] [, ... ] ) ] +ALTER SUBSCRIPTION name ADD PUBLICATION publication_name [, ...] [ WITH ( publication_option [= value] [, ... ] ) ] +ALTER SUBSCRIPTION name DROP PUBLICATION publication_name [, ...] [ WITH ( publication_option [= value] [, ... ] ) ] ALTER SUBSCRIPTION name REFRESH PUBLICATION [ WITH ( refresh_option [= value] [, ... ] ) ] ALTER SUBSCRIPTION name ENABLE ALTER SUBSCRIPTION name DISABLE @@ -102,17 +102,17 @@ ALTER SUBSCRIPTION name RENAME TO < Changes the list of subscribed publications. SET replaces the entire list of publications with a new list, - ADD adds additional publications, - DROP removes publications from the list of - publications. See for more - information. By default, this command will also act like + ADD adds additional publications to the list of + publications and DROP removes the publications from + the list of publications. See + for more information. 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. - set_publication_option specifies additional + publication_option specifies additional options for this operation. The supported options are: @@ -128,8 +128,8 @@ ALTER SUBSCRIPTION name RENAME TO < - Additionally, refresh options as described - under REFRESH PUBLICATION may be specified. + Additionally, refresh options as described under REFRESH PUBLICATION + may be specified, except for DROP PUBLICATION. diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 8aa6de1785..c9c03d8fa3 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -48,7 +48,8 @@ static List *fetch_table_list(WalReceiverConn *wrconn, List *publications); static void check_duplicates_in_publist(List *publist, Datum *datums); -static List *merge_publications(List *oldpublist, List *newpublist, bool addpub, const char *subname); +static List *merge_publications(List *oldpublist, List *newpublist, + bool addpub, const char *subname); static void ReportSlotConnectionError(List *rstates, Oid subid, char *slotname, char *err); @@ -951,8 +952,6 @@ AlterSubscription(AlterSubscriptionStmt *stmt, bool isTopLevel) bool refresh; List *publist; - publist = merge_publications(sub->publications, stmt->publication, isadd, stmt->subname); - parse_subscription_options(stmt->options, NULL, /* no "connect" */ NULL, NULL, /* no "enabled" */ @@ -965,6 +964,11 @@ AlterSubscription(AlterSubscriptionStmt *stmt, bool isTopLevel) NULL, NULL, /* no "binary" */ NULL, NULL); /* no "streaming" */ + publist = merge_publications(sub->publications, + stmt->publication, + isadd, + stmt->subname); + values[Anum_pg_subscription_subpublications - 1] = publicationListToArray(publist); replaces[Anum_pg_subscription_subpublications - 1] = true; @@ -1622,7 +1626,8 @@ check_duplicates_in_publist(List *publist, Datum *datums) * subname is the subscription name, for error messages. */ static List * -merge_publications(List *oldpublist, List *newpublist, bool addpub, const char *subname) +merge_publications(List *oldpublist, List *newpublist, bool addpub, + const char *subname) { ListCell *lc; @@ -1671,7 +1676,8 @@ merge_publications(List *oldpublist, List *newpublist, bool addpub, const char * if (!oldpublist) ereport(ERROR, (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), - errmsg("subscription must contain at least one publication"))); + errmsg("cannot drop all the publications of the subscriber \"%s\"", + subname))); return oldpublist; } diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 6598c5369a..2595941408 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -1675,11 +1675,14 @@ psql_completion(const char *text, int start, int end) else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && TailMatches("ADD|DROP|SET", "PUBLICATION", MatchAny)) COMPLETE_WITH("WITH ("); - /* ALTER SUBSCRIPTION ADD|DROP|SET PUBLICATION WITH ( */ + /* ALTER SUBSCRIPTION ADD|SET PUBLICATION WITH ( */ else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && - TailMatches("ADD|DROP|SET", "PUBLICATION", MatchAny, "WITH", "(")) + TailMatches("ADD|SET", "PUBLICATION", MatchAny, "WITH", "(")) COMPLETE_WITH("copy_data", "refresh"); - + /* ALTER SUBSCRIPTION DROP PUBLICATION WITH ( */ + else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && + TailMatches("DROP", "PUBLICATION", MatchAny, "WITH", "(")) + COMPLETE_WITH("refresh"); /* ALTER SCHEMA */ else if (Matches("ALTER", "SCHEMA", MatchAny)) COMPLETE_WITH("OWNER TO", "RENAME TO"); diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out index 09576c176b..76de317830 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -223,7 +223,7 @@ ALTER SUBSCRIPTION regress_testsub DROP PUBLICATION testpub1, testpub1 WITH (ref ERROR: publication name "testpub1" used more than once -- fail - all publications are deleted ALTER SUBSCRIPTION regress_testsub DROP PUBLICATION testpub, testpub1, testpub2 WITH (refresh = false); -ERROR: subscription must contain at least one publication +ERROR: cannot drop all the publications of the subscriber "regress_testsub" -- fail - publication does not exist in subscription ALTER SUBSCRIPTION regress_testsub DROP PUBLICATION testpub3 WITH (refresh = false); ERROR: publication "testpub3" is not in subscription "regress_testsub" -- 2.25.1