v20-0009-Remove-S-option-to-force-unix-domain-connection.patch

From 19e845327134f499cacb01779b9f6debe2db95f6 Mon Sep 17 00:00:00 2001
From: Shlok Kyal <shlok.kyal.oss@gmail.com>
Date: Tue, 6 Feb 2024 14:45:03 +0530
Subject: [PATCH v20 09/12] Remove -S option to force unix domain connection

With this patch removed -S option and added option for username(-u), port(-p)
and socket directory(-s) for standby. This helps to force standby to use
unix domain connection.
---
 doc/src/sgml/ref/pg_createsubscriber.sgml     | 36 ++++++--
 src/bin/pg_basebackup/pg_createsubscriber.c   | 91 ++++++++++++++-----
 .../t/041_pg_createsubscriber_standby.pl      | 21 +++--
 3 files changed, 109 insertions(+), 39 deletions(-)

diff --git a/doc/src/sgml/ref/pg_createsubscriber.sgml b/doc/src/sgml/ref/pg_createsubscriber.sgml
index 7cdd047d67..63086a8e98 100644
--- a/doc/src/sgml/ref/pg_createsubscriber.sgml
+++ b/doc/src/sgml/ref/pg_createsubscriber.sgml
@@ -34,11 +34,6 @@ PostgreSQL documentation
      <arg choice="plain"><option>--publisher-server</option></arg>
     </group>
     <replaceable>connstr</replaceable>
-    <group choice="req">
-     <arg choice="plain"><option>-S</option></arg>
-     <arg choice="plain"><option>--subscriber-server</option></arg>
-    </group>
-    <replaceable>connstr</replaceable>
     <group choice="req">
      <arg choice="plain"><option>-d</option></arg>
      <arg choice="plain"><option>--database</option></arg>
@@ -173,11 +168,36 @@ PostgreSQL documentation
      </varlistentry>
 
      <varlistentry>
-      <term><option>-S <replaceable class="parameter">connstr</replaceable></option></term>
-      <term><option>--subscriber-server=<replaceable class="parameter">connstr</replaceable></option></term>
+      <term><option>-p <replaceable class="parameter">port</replaceable></option></term>
+      <term><option>--port=<replaceable class="parameter">port</replaceable></option></term>
+      <listitem>
+       <para>
+        A port number on which the target server is listening for connections.
+        Defaults to the <envar>PGPORT</envar> environment variable, if set, or
+        a compiled-in default.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
+      <term><option>-U <replaceable>username</replaceable></option></term>
+      <term><option>--username=<replaceable class="parameter">username</replaceable></option></term>
+      <listitem>
+       <para>
+        Target's user name. Defaults to the <envar>PGUSER</envar> environment
+        variable.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
+      <term><option>-s</option> <replaceable>dir</replaceable></term>
+      <term><option>--socketdir=</option><replaceable>dir</replaceable></term>
       <listitem>
        <para>
-        The connection string to the subscriber. For details see <xref linkend="libpq-connstring"/>.
+        A directory which locales a temporary Unix socket files. If not
+        specified, <application>pg_createsubscriber</application> tries to
+        connect via TCP/IP to <literal>localhost</literal>.
        </para>
       </listitem>
      </varlistentry>
diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c
index 5ccab80032..0a70f00252 100644
--- a/src/bin/pg_basebackup/pg_createsubscriber.c
+++ b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -34,7 +34,9 @@ typedef struct CreateSubscriberOptions
 {
 	char	   *subscriber_dir; 		/* standby/subscriber data directory */
 	char	   *pub_conninfo_str;		/* publisher connection string */
-	char	   *sub_conninfo_str;		/* subscriber connection string */
+	unsigned short subport;				/* port number listen()'d by the standby */
+	char	   *subuser;				/* database user of the standby */
+	char	   *socketdir;				/* socket directory */
 	SimpleStringList database_names;	/* list of database names */
 	bool		retain;					/* retain log file? */
 	int			recovery_timeout;		/* stop recovery after this time */
@@ -57,7 +59,9 @@ typedef struct LogicalRepInfo
 
 static void cleanup_objects_atexit(void);
 static void usage();
-static char *get_base_conninfo(char *conninfo, char **dbname);
+static char *get_pub_base_conninfo(char *conninfo, char **dbname);
+static char *construct_sub_conninfo(char *username, unsigned short subport,
+									char *socketdir);
 static char *get_bin_directory(const char *path);
 static bool check_data_directory(const char *datadir);
 static char *concat_conninfo_dbname(const char *conninfo, const char *dbname);
@@ -170,7 +174,10 @@ usage(void)
 	printf(_("\nOptions:\n"));
 	printf(_(" -D, --pgdata=DATADIR                location for the subscriber data directory\n"));
 	printf(_(" -P, --publisher-server=CONNSTR      publisher connection string\n"));
-	printf(_(" -S, --subscriber-server=CONNSTR     subscriber connection string\n"));
+	printf(_(" -p, --port=PORT                     subscriber port number\n"));
+	printf(_(" -U, --username=NAME                 subscriber user\n"));
+	printf(_(" -s, --socketdir=DIR                 socket directory to use\n"));
+	printf(_("                                     If not specified, localhost would be used\n"));
 	printf(_(" -d, --database=DBNAME               database to create a subscription\n"));
 	printf(_(" -n, --dry-run                       check clusters only, don't change target server\n"));
 	printf(_(" -t, --recovery-timeout=SECS         seconds to wait for recovery to end\n"));
@@ -183,8 +190,8 @@ usage(void)
 }
 
 /*
- * Validate a connection string. Returns a base connection string that is a
- * connection string without a database name.
+ * Validate a connection string for the publisher. Returns a base connection
+ * string that is a connection string without a database name.
  *
  * Since we might process multiple databases, each database name will be
  * appended to this base connection string to provide a final connection
@@ -196,7 +203,7 @@ usage(void)
  * dbname.
  */
 static char *
-get_base_conninfo(char *conninfo, char **dbname)
+get_pub_base_conninfo(char *conninfo, char **dbname)
 {
 	PQExpBuffer buf = createPQExpBuffer();
 	PQconninfoOption *conn_opts = NULL;
@@ -1540,6 +1547,40 @@ enable_subscription(PGconn *conn, LogicalRepInfo *dbinfo)
 	destroyPQExpBuffer(str);
 }
 
+/*
+ * Construct a connection string toward a target server, from argument options.
+ *
+ * If inputs are the zero, default value would be used.
+ * - username: PGUSER environment value (it would not be parsed)
+ * - port: PGPORT environment value (it would not be parsed)
+ * - socketdir: localhost connection (unix-domain would not be used)
+ */
+static char *
+construct_sub_conninfo(char *username, unsigned short subport, char *sockdir)
+{
+	PQExpBuffer buf = createPQExpBuffer();
+	char	   *ret;
+
+	if (username)
+		appendPQExpBuffer(buf, "user=%s ", username);
+
+	if (subport != 0)
+		appendPQExpBuffer(buf, "port=%u ", subport);
+
+	if (sockdir)
+		appendPQExpBuffer(buf, "host=%s ", sockdir);
+	else
+		appendPQExpBuffer(buf, "host=localhost ");
+
+	appendPQExpBuffer(buf, "fallback_application_name=%s", progname);
+
+	ret = pg_strdup(buf->data);
+
+	destroyPQExpBuffer(buf);
+
+	return ret;
+}
+
 int
 main(int argc, char **argv)
 {
@@ -1549,7 +1590,9 @@ main(int argc, char **argv)
 		{"version", no_argument, NULL, 'V'},
 		{"pgdata", required_argument, NULL, 'D'},
 		{"publisher-server", required_argument, NULL, 'P'},
-		{"subscriber-server", required_argument, NULL, 'S'},
+		{"port", required_argument, NULL, 'p'},
+		{"username", required_argument, NULL, 'U'},
+		{"socketdir", required_argument, NULL, 's'},
 		{"database", required_argument, NULL, 'd'},
 		{"dry-run", no_argument, NULL, 'n'},
 		{"recovery-timeout", required_argument, NULL, 't'},
@@ -1605,7 +1648,9 @@ main(int argc, char **argv)
 	/* Default settings */
 	opt.subscriber_dir = NULL;
 	opt.pub_conninfo_str = NULL;
-	opt.sub_conninfo_str = NULL;
+	opt.subport = 0;
+	opt.subuser = NULL;
+	opt.socketdir = NULL;
 	opt.database_names = (SimpleStringList)
 	{
 		NULL, NULL
@@ -1629,7 +1674,7 @@ main(int argc, char **argv)
 
 	get_restricted_token();
 
-	while ((c = getopt_long(argc, argv, "D:P:S:d:nrt:v",
+	while ((c = getopt_long(argc, argv, "D:P:p:U:s:S:d:nrt:v",
 							long_options, &option_index)) != -1)
 	{
 		switch (c)
@@ -1640,8 +1685,17 @@ main(int argc, char **argv)
 			case 'P':
 				opt.pub_conninfo_str = pg_strdup(optarg);
 				break;
-			case 'S':
-				opt.sub_conninfo_str = pg_strdup(optarg);
+			case 'p':
+				if ((opt.subport = atoi(optarg)) <= 0)
+					pg_fatal("invalid old port number");
+				break;
+			case 'U':
+				pfree(opt.subuser);
+				opt.subuser = pg_strdup(optarg);
+				break;
+			case 's':
+				pfree(opt.socketdir);
+				opt.socketdir = pg_strdup(optarg);
 				break;
 			case 'd':
 				/* Ignore duplicated database names */
@@ -1709,21 +1763,12 @@ main(int argc, char **argv)
 		exit(1);
 	}
 	pg_log_info("validating connection string on publisher");
-	pub_base_conninfo = get_base_conninfo(opt.pub_conninfo_str,
-										  &dbname_conninfo);
+	pub_base_conninfo = get_pub_base_conninfo(opt.pub_conninfo_str,
+											  &dbname_conninfo);
 	if (pub_base_conninfo == NULL)
 		exit(1);
 
-	if (opt.sub_conninfo_str == NULL)
-	{
-		pg_log_error("no subscriber connection string specified");
-		pg_log_error_hint("Try \"%s --help\" for more information.", progname);
-		exit(1);
-	}
-	pg_log_info("validating connection string on subscriber");
-	sub_base_conninfo = get_base_conninfo(opt.sub_conninfo_str, NULL);
-	if (sub_base_conninfo == NULL)
-		exit(1);
+	sub_base_conninfo = construct_sub_conninfo(opt.subuser, opt.subport, opt.socketdir);
 
 	if (opt.database_names.head == NULL)
 	{
diff --git a/src/bin/pg_basebackup/t/041_pg_createsubscriber_standby.pl b/src/bin/pg_basebackup/t/041_pg_createsubscriber_standby.pl
index d7567ef8e9..55781423cf 100644
--- a/src/bin/pg_basebackup/t/041_pg_createsubscriber_standby.pl
+++ b/src/bin/pg_basebackup/t/041_pg_createsubscriber_standby.pl
@@ -60,7 +60,8 @@ command_fails(
 		'pg_createsubscriber', '--verbose',
 		'--pgdata', $node_f->data_dir,
 		'--publisher-server', $node_p->connstr('pg1'),
-		'--subscriber-server', $node_f->connstr('pg1'),
+		'--port', $node_f->port,
+		'--host', $node_f->host,
 		'--database', 'pg1',
 		'--database', 'pg2'
 	],
@@ -72,8 +73,9 @@ command_fails(
 		'pg_createsubscriber', '--verbose',
 		'--dry-run', '--pgdata',
 		$node_s->data_dir, '--publisher-server',
-		$node_p->connstr('pg1'), '--subscriber-server',
-		$node_s->connstr('pg1'), '--database',
+		$node_p->connstr('pg1'), '--port',
+		$node_s->port, '--host',
+		$node_s->host, '--database',
 		'pg1', '--database',
 		'pg2'
 	],
@@ -91,8 +93,9 @@ command_ok(
 		'pg_createsubscriber', '--verbose',
 		'--dry-run', '--pgdata',
 		$node_s->data_dir, '--publisher-server',
-		$node_p->connstr('pg1'), '--subscriber-server',
-		$node_s->connstr('pg1'), '--database',
+		$node_p->connstr('pg1'), '--port',
+		$node_s->port, '--socketdir',
+		$node_s->host, '--database',
 		'pg1', '--database',
 		'pg2'
 	],
@@ -108,8 +111,9 @@ command_ok(
 		'pg_createsubscriber', '--verbose',
 		'--dry-run', '--pgdata',
 		$node_s->data_dir, '--publisher-server',
-		$node_p->connstr('pg1'), '--subscriber-server',
-		$node_s->connstr('pg1')
+		$node_p->connstr('pg1'), '--port',
+		$node_s->port, '--socketdir',
+		$node_s->host,
 	],
 	'run pg_createsubscriber without --databases');
 
@@ -119,7 +123,8 @@ command_ok(
 		'pg_createsubscriber', '--verbose',
 		'--pgdata', $node_s->data_dir,
 		'--publisher-server', $node_p->connstr('pg1'),
-		'--subscriber-server', $node_s->connstr('pg1'),
+		'--port', $node_s->port,
+		'--socketdir', $node_s->host,
 		'--database', 'pg1',
 		'--database', 'pg2'
 	],
-- 
2.43.0