pg_rewind: Add dbname to primary_conninfo when using --write-recovery-conf.

This commit enhances pg_rewind's --write-recovery-conf option to
include the dbname in the generated primary_conninfo value when
specified in the --source-server option. With this modification, the
rewound server can connect to the primary server without manual
configuration file modifications when sync_replication_slots is
enabled.

Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Discussion: https://postgr.es/m/CAD21AoAkW=Ht0k9dVoBTCcqLiiZ2MXhVr+d=j2T_EZMerGrLWQ@mail.gmail.com

Author: MasahikoSawada

doc/src/sgml/ref/pg_rewind.sgml

@@ -186,8 +186,10 @@ PostgreSQL documentation
        <para>
         Create <filename>standby.signal</filename> and append connection
         settings to <filename>postgresql.auto.conf</filename> in the output
-        directory.  <literal>--source-server</literal> is mandatory with
-        this option.
+        directory.  The dbname will be recorded only if the dbname was
+        specified explicitly in the connection string or <link linkend="libpq-envars">
+        environment variable</link>. <literal>--source-server</literal> is
+        mandatory with this option.
        </para>
       </listitem>
      </varlistentry>

src/bin/pg_basebackup/pg_basebackup.c

@@ -1818,7 +1818,7 @@ BaseBackup(char *compression_algorithm, char *compression_detail,
 	if (writerecoveryconf)
 		recoveryconfcontents = GenerateRecoveryConfig(conn,
 													  replication_slot,
-													  GetDbnameFromConnectionOptions());
+													  GetDbnameFromConnectionOptions(connection_string));
 
 	/*
 	 * Run IDENTIFY_SYSTEM so we can get the timeline

src/bin/pg_basebackup/streamutil.c

@@ -32,7 +32,6 @@
 int			WalSegSz;
 
 static bool RetrieveDataDirCreatePerm(PGconn *conn);
-static char *FindDbnameInConnParams(PQconninfoOption *conn_opts);
 
 /* SHOW command for replication connection was introduced in version 10 */
 #define MINIMUM_VERSION_FOR_SHOW_CMD 100000
@@ -269,74 +268,6 @@ GetConnection(void)
 	return tmpconn;
 }
 
-/*
- * FindDbnameInConnParams
- *
- * This is a helper function for GetDbnameFromConnectionOptions(). Extract
- * the value of dbname from PQconninfoOption parameters, if it's present.
- * Returns a strdup'd result or NULL.
- */
-static char *
-FindDbnameInConnParams(PQconninfoOption *conn_opts)
-{
-	PQconninfoOption *conn_opt;
-
-	for (conn_opt = conn_opts; conn_opt->keyword != NULL; conn_opt++)
-	{
-		if (strcmp(conn_opt->keyword, "dbname") == 0 &&
-			conn_opt->val != NULL && conn_opt->val[0] != '\0')
-			return pg_strdup(conn_opt->val);
-	}
-	return NULL;
-}
-
-/*
- * GetDbnameFromConnectionOptions
- *
- * This is a special purpose function to retrieve the dbname from either the
- * connection_string specified by the user or from the environment variables.
- *
- * We follow GetConnection() to fetch the dbname from various connection
- * options.
- *
- * Returns NULL, if dbname is not specified by the user in the above
- * mentioned connection options.
- */
-char *
-GetDbnameFromConnectionOptions(void)
-{
-	PQconninfoOption *conn_opts;
-	char	   *err_msg = NULL;
-	char	   *dbname;
-
-	/* First try to get the dbname from connection string. */
-	if (connection_string)
-	{
-		conn_opts = PQconninfoParse(connection_string, &err_msg);
-		if (conn_opts == NULL)
-			pg_fatal("%s", err_msg);
-
-		dbname = FindDbnameInConnParams(conn_opts);
-
-		PQconninfoFree(conn_opts);
-		if (dbname)
-			return dbname;
-	}
-
-	/*
-	 * Next try to get the dbname from default values that are available from
-	 * the environment.
-	 */
-	conn_opts = PQconndefaults();
-	if (conn_opts == NULL)
-		pg_fatal("out of memory");
-
-	dbname = FindDbnameInConnParams(conn_opts);
-
-	PQconninfoFree(conn_opts);
-	return dbname;
-}
-
 /*
  * From version 10, explicitly set wal segment size using SHOW wal_segment_size
  * since ControlFile is not accessible here.

src/bin/pg_basebackup/streamutil.h

@@ -31,8 +31,6 @@ extern PGconn *conn;
 
 extern PGconn *GetConnection(void);
 
-extern char *GetDbnameFromConnectionOptions(void);
-
 /* Replication commands */
 extern bool CreateReplicationSlot(PGconn *conn, const char *slot_name,
 								  const char *plugin, bool is_temporary,

src/bin/pg_rewind/pg_rewind.c

@@ -451,7 +451,8 @@ main(int argc, char **argv)
 		pg_log_info("no rewind required");
 		if (writerecoveryconf && !dry_run)
 			WriteRecoveryConfig(conn, datadir_target,
-								GenerateRecoveryConfig(conn, NULL, NULL));
+								GenerateRecoveryConfig(conn, NULL,
+													   GetDbnameFromConnectionOptions(connstr_source)));
 		exit(0);
 	}
 
@@ -528,7 +529,8 @@ main(int argc, char **argv)
 	/* Also update the standby configuration, if requested. */
 	if (writerecoveryconf && !dry_run)
 		WriteRecoveryConfig(conn, datadir_target,
-							GenerateRecoveryConfig(conn, NULL, NULL));
+							GenerateRecoveryConfig(conn, NULL,
+												   GetDbnameFromConnectionOptions(connstr_source)));
 
 	/* don't need the source connection anymore */
 	source->destroy(source);

src/bin/pg_rewind/t/RewindTest.pm

@@ -279,6 +279,11 @@ sub run_pg_rewind
 			],
 			'pg_rewind remote');
 
+		# Check that pg_rewind with dbname and --write-recovery-conf
+		# wrote the dbname in the generated primary_conninfo value.
+		like(slurp_file("$primary_pgdata/postgresql.auto.conf"),
+		     qr/dbname=postgres/m, 'recovery conf file sets dbname');
+
 		# Check that standby.signal is here as recovery configuration
 		# was requested.
 		ok( -e "$primary_pgdata/standby.signal",

src/fe_utils/recovery_gen.c

@@ -14,6 +14,7 @@
 #include "fe_utils/string_utils.h"
 
 static char *escape_quotes(const char *src);
+static char *FindDbnameInConnOpts(PQconninfoOption *conn_opts);
 
 /*
  * Write recovery configuration contents into a fresh PQExpBuffer, and
@@ -168,3 +169,68 @@ escape_quotes(const char *src)
 		pg_fatal("out of memory");
 	return result;
 }
+
+/*
+ * FindDbnameInConnOpts
+ *
+ * This is a helper function for GetDbnameFromConnectionOptions(). Extract
+ * the value of dbname from PQconninfoOption parameters, if it's present.
+ * Returns a strdup'd result or NULL.
+ */
+static char *
+FindDbnameInConnOpts(PQconninfoOption *conn_opts)
+{
+	for (PQconninfoOption *conn_opt = conn_opts;
+		 conn_opt->keyword != NULL;
+		 conn_opt++)
+	{
+		if (strcmp(conn_opt->keyword, "dbname") == 0 &&
+			conn_opt->val != NULL && conn_opt->val[0] != '\0')
+			return pg_strdup(conn_opt->val);
+	}
+	return NULL;
+}
+
+/*
+ * GetDbnameFromConnectionOptions
+ *
+ * This is a special purpose function to retrieve the dbname from either the
+ * 'connstr' specified by the caller or from the environment variables.
+ *
+ * Returns NULL, if dbname is not specified by the user in the given
+ * connection options.
+ */
+char *
+GetDbnameFromConnectionOptions(const char *connstr)
+{
+	PQconninfoOption *conn_opts;
+	char	   *err_msg = NULL;
+	char	   *dbname;
+
+	/* First try to get the dbname from connection string. */
+	if (connstr)
+	{
+		conn_opts = PQconninfoParse(connstr, &err_msg);
+		if (conn_opts == NULL)
+			pg_fatal("%s", err_msg);
+
+		dbname = FindDbnameInConnOpts(conn_opts);
+
+		PQconninfoFree(conn_opts);
+		if (dbname)
+			return dbname;
+	}
+
+	/*
+	 * Next try to get the dbname from default values that are available from
+	 * the environment.
+	 */
+	conn_opts = PQconndefaults();
+	if (conn_opts == NULL)
+		pg_fatal("out of memory");
+
+	dbname = FindDbnameInConnOpts(conn_opts);
+
+	PQconninfoFree(conn_opts);
+	return dbname;
+}

src/include/fe_utils/recovery_gen.h

@@ -25,5 +25,6 @@ extern PQExpBuffer GenerateRecoveryConfig(PGconn *pgconn,
 										  char *dbname);
 extern void WriteRecoveryConfig(PGconn *pgconn, const char *target_dir,
 								PQExpBuffer contents);
+extern char *GetDbnameFromConnectionOptions(const char *connstr);
 
 #endif							/* RECOVERY_GEN_H */