On Fri, May 14, 2021 at 8:56 PM Tom Lane <t...@sss.pgh.pa.us> wrote:
>
> Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> writes:
> > On Fri, May 14, 2021 at 7:58 PM vignesh C <vignes...@gmail.com> wrote:
> >> I have changed it to:
> >> <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
>
> > How about "Publications are added to or dropped from the existing list
> > of publications by <literal>ADD</literal> or <literal>DROP</literal>
> > respectively." ?
>
> We generally prefer to use the active voice, so I don't think
> restructuring the sentence that way is an improvement. The quoted
> bit would be better left alone entirely. Or maybe split it into
> two sentences, but keep the active voice.
I felt changing it to the below was better:
SET replaces the entire list of publications with a new list, ADD adds
additional publications to the list of publications and DROP removes
the publications from the list of publications.
Attached patch has the change for the same.
Thoughts?
Regards,
Vignesh
From 4f887b0b3f338f55d186fa059cfdc255b9783263 Mon Sep 17 00:00:00 2001
From: vignesh <vignes...@gmail.com>
Date: Fri, 14 May 2021 19:30:40 +0530
Subject: [PATCH v4] Fixes in alter subscription drop publication.
Fixes in alter subscription drop publication.
---
doc/src/sgml/ref/alter_subscription.sgml | 19 ++++++++++---------
src/backend/commands/subscriptioncmds.c | 6 +++---
src/bin/psql/tab-complete.c | 9 ++++++---
src/test/regress/expected/subscription.out | 2 +-
4 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index 367ac814f4..6f5a4268e5 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
@@ -102,17 +102,17 @@ ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> RENAME TO <
<para>
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
- publications. See <xref linkend="sql-createsubscription"/> for more
- information. By default, this command will also act like
+ <literal>ADD</literal> adds additional publications to the list of
+ publications and <literal>DROP</literal> removes the publications 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
<literal>ADD</literal> or <literal>DROP</literal>, only the added or
dropped publications are refreshed.
</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
