On Thu, May 13, 2021 at 8:13 PM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > > On Thu, May 13, 2021 at 7:43 PM vignesh C <vignes...@gmail.com> wrote: > > I have separated the Drop publication documentation contents. There > > are some duplicate contents but the readability is slightly better. > > Thoughts? > > -ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> > DROP PUBLICATION <replaceable > class="parameter">publication_name</replaceable> [, ...] [ WITH ( > <replaceable class="parameter">set_publication_option</replaceable> [= > <replaceable class="parameter">value</replaceable>] [, ... ] ) ] > +ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> > DROP PUBLICATION <replaceable > class="parameter">publication_name</replaceable> [, ...] [ WITH ( > refresh [= <replaceable class="parameter">value</replaceable>] ) ] > > IMO, let's not list the "refresh" option directly here. If we don't > want to add a new list of operations "drop_publication_opition", you > could just mention a note "Except for DROP PUBLICATION, the refresh > options as described under REFRESH PUBLICATION may be specified." or > "Additionally, refresh options as described under REFRESH PUBLICATION > may be specified, except for DROP PUBLICATION."
Thanks for the comment, the attached v3 patch has the changes for the same. I also made another change to change set_publication_option to publication_option as it is common for SET/ADD & DROP. Regards, Vignesh
From 3b477122f909d5e1df35c32a1775665bc92ec86b Mon Sep 17 00:00:00 2001 From: vignesh <vignes...@gmail.com> Date: Fri, 14 May 2021 19:30:40 +0530 Subject: [PATCH v3] Fixes in alter subscription drop publication. Fixes in alter subscription drop publication. --- doc/src/sgml/ref/alter_subscription.sgml | 13 +++++++------ src/backend/commands/subscriptioncmds.c | 6 +++--- src/bin/psql/tab-complete.c | 9 ++++++--- src/test/regress/expected/subscription.out | 2 +- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml index 367ac814f4..faf785760c 100644 --- a/doc/src/sgml/ref/alter_subscription.sgml +++ b/doc/src/sgml/ref/alter_subscription.sgml @@ -22,9 +22,9 @@ PostgreSQL documentation <refsynopsisdiv> <synopsis> ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> CONNECTION '<replaceable>conninfo</replaceable>' -ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> SET PUBLICATION <replaceable class="parameter">publication_name</replaceable> [, ...] [ WITH ( <replaceable class="parameter">set_publication_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] ) ] -ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> ADD PUBLICATION <replaceable class="parameter">publication_name</replaceable> [, ...] [ WITH ( <replaceable class="parameter">set_publication_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] ) ] -ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> DROP PUBLICATION <replaceable class="parameter">publication_name</replaceable> [, ...] [ WITH ( <replaceable class="parameter">set_publication_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] ) ] +ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> SET PUBLICATION <replaceable class="parameter">publication_name</replaceable> [, ...] [ WITH ( <replaceable class="parameter">publication_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] ) ] +ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> ADD PUBLICATION <replaceable class="parameter">publication_name</replaceable> [, ...] [ WITH ( <replaceable class="parameter">publication_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] ) ] +ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> DROP PUBLICATION <replaceable class="parameter">publication_name</replaceable> [, ...] [ WITH ( <replaceable class="parameter">publication_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] ) ] ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> REFRESH PUBLICATION [ WITH ( <replaceable class="parameter">refresh_option</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] ) ] ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> ENABLE ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> DISABLE @@ -103,7 +103,7 @@ ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> RENAME TO < Changes the list of subscribed publications. <literal>SET</literal> replaces the entire list of publications with a new list, <literal>ADD</literal> adds additional publications, - <literal>DROP</literal> removes publications from the list of + <literal>DROP</literal> removes publications to/from the list of publications. See <xref linkend="sql-createsubscription"/> for more information. By default, this command will also act like <literal>REFRESH PUBLICATION</literal>, except that in case of @@ -112,7 +112,7 @@ ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> RENAME TO < </para> <para> - <replaceable>set_publication_option</replaceable> specifies additional + <replaceable>publication_option</replaceable> specifies additional options for this operation. The supported options are: <variablelist> @@ -129,7 +129,8 @@ ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> RENAME TO < </variablelist> Additionally, refresh options as described - under <literal>REFRESH PUBLICATION</literal> may be specified. + under <literal>REFRESH PUBLICATION</literal> may be specified, except for + <literal>DROP PUBLICATION</literal>. </para> </listitem> </varlistentry> diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 8aa6de1785..e93fee6b99 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -951,8 +951,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 +963,8 @@ 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; @@ -1671,7 +1671,7 @@ 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 <name> ADD|DROP|SET PUBLICATION <name> WITH ( */ + /* ALTER SUBSCRIPTION <name> ADD|SET PUBLICATION <name> 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 <name> DROP PUBLICATION <name> WITH ( */ + else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && + TailMatches("DROP", "PUBLICATION", MatchAny, "WITH", "(")) + COMPLETE_WITH("refresh"); /* ALTER SCHEMA <name> */ 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