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 
Reviewed-by: Japin Li 
Discussion: https://www.postgresql.org/message-id/flat/CALDaNm21RwsDzs4xj14ApteAF7auyyomHNnp+NEL-sH8m-jMvQ@mail.gmail.com

diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index 367ac814f4b678bdaf368917652a7d303ccbe015..b3d173179f4cbb79ef6c13c4d35f1b4e6178bd2b 100644 (file)
--- 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:
 
       
@@ -129,7 +129,8 @@ ALTER SUBSCRIPTION name RENAME TO <
       
 
       Additionally, refresh options as described
-      under REFRESH PUBLICATION may be specified.
+      under REFRESH PUBLICATION may be specified,
+      except in the case of DROP PUBLICATION.
      
     
    

diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 75e195f286e8c30cb61f6493316b6c70a39ddbc1..e9a97db9a55797b2e2f20556ac6e3ad3bbf28a6e 100644 (file)
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -953,8 +953,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" */
@@ -967,6 +965,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;
@@ -1676,7 +1676,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 from a subscription")));
 
    return oldpublist;
 }

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 38af5682f2db485a0c4b0eb47b88202a54fc3c3a..0ebd5aa41a1ada838a3deb145f0afbae09f28d9d 100644 (file)
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1675,10 +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))

diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 09576c176b6fda4f3ec686191764f8453b9bcc79..57f7dd9b0a7351dedc5ac6c69b8471775a981515 100644 (file)
--- 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 from a subscription
 -- 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"