From 82f6c494524954ba4cfd669b97944398e2b3b888 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Wed, 12 Apr 2017 22:12:30 -0400
Subject: [PATCH 2/2] pg_dump: Always dump subscriptions NOCONNECT

This removes the pg_dump option --no-subscription-connect and makes it
the default.  Dumping a subscription so that it activates right away
when restored is not very useful, because the state of the publication
server is unclear.

Discussion: https://www.postgresql.org/message-id/e4fbfad5-c6ac-fd50-6777-18c84b34eb2f@2ndquadrant.com
---
 doc/src/sgml/ref/pg_dump.sgml    | 26 +++++++++++++-------------
 src/bin/pg_dump/pg_backup.h      |  1 -
 src/bin/pg_dump/pg_dump.c        | 22 ++--------------------
 src/bin/pg_dump/pg_dump.h        |  1 -
 src/bin/pg_dump/t/002_pg_dump.pl |  4 ++--
 5 files changed, 17 insertions(+), 37 deletions(-)

diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 53b5dd5239..6cf7e570ef 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -799,19 +799,6 @@ <title>Options</title>
      </varlistentry>
 
      <varlistentry>
-      <term><option>--no-subscription-connect</option></term>
-      <listitem>
-       <para>
-        When dumping logical replication subscriptions,
-        generate <command>CREATE SUBSCRIPTION</command> commands that do not
-        make remote connections for creating replication slot or initial table
-        copy.  That way, the dump can be restored without requiring network
-        access to the remote servers.
-       </para>
-      </listitem>
-     </varlistentry>
-
-     <varlistentry>
       <term><option>--no-synchronized-snapshots</></term>
       <listitem>
        <para>
@@ -1235,6 +1222,19 @@ <title>Notes</title>
    in cross-version cases, as it can prevent problems arising from varying
    reserved-word lists in different <productname>PostgreSQL</> versions.
   </para>
+
+  <para>
+   When dumping logical replication subscriptions,
+   <application>pg_dump</application> will generate <command>CREATE
+   SUBSCRIPTION</command> commands that use the <literal>NOCONNECT</literal>
+   option, so that restoring the subscription does not make remote connections
+   for creating a replication slot or for initial table copy.  That way, the
+   dump can be restored without requiring network access to the remote
+   servers.  It is then up to the user to reactivate the subscriptions in a
+   suitable way.  If the involved hosts have changed, the connection
+   information might have to be changed.  It might also be appropriate to
+   truncate the target tables before initiating a new full table copy.
+  </para>
  </refsect1>
 
  <refsect1 id="pg-dump-examples">
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 1d14b68983..08b883efb0 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -153,7 +153,6 @@ typedef struct _dumpOptions
 	int			outputNoTablespaces;
 	int			use_setsessauth;
 	int			enable_row_security;
-	int			no_subscription_connect;
 
 	/* default, if no "inclusion" switches appear, is to dump everything */
 	bool		include_everything;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index dcefe975d8..14dc1b2423 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -353,7 +353,6 @@ main(int argc, char **argv)
 		{"strict-names", no_argument, &strict_names, 1},
 		{"use-set-session-authorization", no_argument, &dopt.use_setsessauth, 1},
 		{"no-security-labels", no_argument, &dopt.no_security_labels, 1},
-		{"no-subscription-connect", no_argument, &dopt.no_subscription_connect, 1},
 		{"no-synchronized-snapshots", no_argument, &dopt.no_synchronized_snapshots, 1},
 		{"no-unlogged-table-data", no_argument, &dopt.no_unlogged_table_data, 1},
 		{"no-sync", no_argument, NULL, 7},
@@ -951,7 +950,6 @@ help(const char *progname)
 	printf(_("  --if-exists                  use IF EXISTS when dropping objects\n"));
 	printf(_("  --inserts                    dump data as INSERT commands, rather than COPY\n"));
 	printf(_("  --no-security-labels         do not dump security label assignments\n"));
-	printf(_("  --no-subscription-connect    dump subscriptions so they don't connect on restore\n"));
 	printf(_("  --no-synchronized-snapshots  do not use synchronized snapshots in parallel jobs\n"));
 	printf(_("  --no-tablespaces             do not dump tablespace assignments\n"));
 	printf(_("  --no-unlogged-table-data     do not dump unlogged table data\n"));
@@ -3669,7 +3667,6 @@ getSubscriptions(Archive *fout)
 	int			i_oid;
 	int			i_subname;
 	int			i_rolname;
-	int			i_subenabled;
 	int			i_subconninfo;
 	int			i_subslotname;
 	int			i_subpublications;
@@ -3702,7 +3699,7 @@ getSubscriptions(Archive *fout)
 	/* Get the subscriptions in current database. */
 	appendPQExpBuffer(query,
 					  "SELECT s.tableoid, s.oid, s.subname,"
-					  "(%s s.subowner) AS rolname, s.subenabled, "
+					  "(%s s.subowner) AS rolname, "
 					  " s.subconninfo, s.subslotname, s.subpublications "
 					  "FROM pg_catalog.pg_subscription s "
 					  "WHERE s.subdbid = (SELECT oid FROM pg_catalog.pg_database"
@@ -3716,7 +3713,6 @@ getSubscriptions(Archive *fout)
 	i_oid = PQfnumber(res, "oid");
 	i_subname = PQfnumber(res, "subname");
 	i_rolname = PQfnumber(res, "rolname");
-	i_subenabled = PQfnumber(res, "subenabled");
 	i_subconninfo = PQfnumber(res, "subconninfo");
 	i_subslotname = PQfnumber(res, "subslotname");
 	i_subpublications = PQfnumber(res, "subpublications");
@@ -3732,8 +3728,6 @@ getSubscriptions(Archive *fout)
 		AssignDumpId(&subinfo[i].dobj);
 		subinfo[i].dobj.name = pg_strdup(PQgetvalue(res, i, i_subname));
 		subinfo[i].rolname = pg_strdup(PQgetvalue(res, i, i_rolname));
-		subinfo[i].subenabled =
-			(strcmp(PQgetvalue(res, i, i_subenabled), "t") == 0);
 		subinfo[i].subconninfo = pg_strdup(PQgetvalue(res, i, i_subconninfo));
 		subinfo[i].subslotname = pg_strdup(PQgetvalue(res, i, i_subslotname));
 		subinfo[i].subpublications =
@@ -3758,7 +3752,6 @@ getSubscriptions(Archive *fout)
 static void
 dumpSubscription(Archive *fout, SubscriptionInfo *subinfo)
 {
-	DumpOptions *dopt = fout->dopt;
 	PQExpBuffer delq;
 	PQExpBuffer query;
 	PQExpBuffer publications;
@@ -3799,19 +3792,8 @@ dumpSubscription(Archive *fout, SubscriptionInfo *subinfo)
 		appendPQExpBufferStr(publications, fmtId(pubnames[i]));
 	}
 
-	appendPQExpBuffer(query, " PUBLICATION %s WITH (", publications->data);
-
-	if (subinfo->subenabled)
-		appendPQExpBufferStr(query, "ENABLED");
-	else
-		appendPQExpBufferStr(query, "DISABLED");
-
-	appendPQExpBufferStr(query, ", SLOT NAME = ");
+	appendPQExpBuffer(query, " PUBLICATION %s WITH (NOCONNECT, SLOT NAME = ", publications->data);
 	appendStringLiteralAH(query, subinfo->subslotname, fout);
-
-	if (dopt->no_subscription_connect)
-		appendPQExpBufferStr(query, ", NOCONNECT");
-
 	appendPQExpBufferStr(query, ");\n");
 
 	ArchiveEntry(fout, subinfo->dobj.catId, subinfo->dobj.dumpId,
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 61097e6d99..ba85392f11 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -614,7 +614,6 @@ typedef struct _SubscriptionInfo
 {
 	DumpableObject dobj;
 	char	   *rolname;
-	bool		subenabled;
 	char	   *subconninfo;
 	char	   *subslotname;
 	char	   *subpublications;
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 1db3767f46..e0d1ce6232 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -4303,9 +4303,9 @@
 		create_order => 50,
 		create_sql   => 'CREATE SUBSCRIPTION sub1
 						 CONNECTION \'dbname=doesnotexist\' PUBLICATION pub1
-						 WITH (DISABLED, NOCONNECT);',
+						 WITH (NOCONNECT);',
 		regexp       => qr/^
-			\QCREATE SUBSCRIPTION sub1 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (DISABLED, SLOT NAME = 'sub1');\E
+			\QCREATE SUBSCRIPTION sub1 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (NOCONNECT, SLOT NAME = 'sub1');\E
 			/xm,
 		like => {
 			binary_upgrade           => 1,
-- 
2.12.2

