v23-0004-Remove-S-option-to-force-unix-domain-connection.patch

From 27203fdfb0dbae37768428d7dbe01ebb738c3bbb 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 v23 04/13] 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      | 33 ++++---
 3 files changed, 115 insertions(+), 45 deletions(-)

diff --git a/doc/src/sgml/ref/pg_createsubscriber.sgml b/doc/src/sgml/ref/pg_createsubscriber.sgml
index 9d0c6c764c..579e50a0a0 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>
@@ -179,11 +174,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 b15769c75b..1ad7de9190 100644
--- a/src/bin/pg_basebackup/pg_createsubscriber.c
+++ b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -35,7 +35,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_exec_path(const char *argv0, const char *progname);
 static bool check_data_directory(const char *datadir);
 static char *concat_conninfo_dbname(const char *conninfo, const char *dbname);
@@ -180,7 +184,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                       dry run, just show what would be done\n"));
 	printf(_(" -t, --recovery-timeout=SECS         seconds to wait for recovery to end\n"));
@@ -193,8 +200,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
@@ -206,7 +213,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;
@@ -1593,6 +1600,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)
 {
@@ -1602,7 +1643,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'},
@@ -1659,7 +1702,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
@@ -1683,7 +1728,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)
@@ -1695,8 +1740,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 */
@@ -1763,21 +1817,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 e2807d3fac..93148417db 100644
--- a/src/bin/pg_basebackup/t/041_pg_createsubscriber_standby.pl
+++ b/src/bin/pg_basebackup/t/041_pg_createsubscriber_standby.pl
@@ -66,7 +66,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'
 	],
@@ -78,8 +79,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'
 	],
@@ -104,10 +106,11 @@ command_fails(
 		'pg_createsubscriber', '--verbose',
 		'--dry-run', '--pgdata',
 		$node_c->data_dir, '--publisher-server',
-		$node_s->connstr('pg1'), '--subscriber-server',
-		$node_c->connstr('pg1'), '--database',
-		'pg1', '--database',
-		'pg2'
+		$node_s->connstr('pg1'),
+		'--port', $node_c->port,
+		'--socketdir', $node_c->host,
+		'--database', 'pg1',
+		'--database', 'pg2'
 	],
 	'primary server is in recovery');
 
@@ -124,8 +127,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'
 	],
@@ -141,8 +145,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');
 
@@ -152,9 +157,9 @@ command_ok(
 		'pg_createsubscriber', '--verbose',
 		'--verbose', '--pgdata',
 		$node_s->data_dir, '--publisher-server',
-		$node_p->connstr('pg1'), '--subscriber-server',
-		$node_s->connstr('pg1'), '--database',
-		'pg1', '--database',
+		$node_p->connstr('pg1'), '--port', $node_s->port,
+		'--socketdir', $node_s->host,
+		'--database', 'pg1', '--database',
 		'pg2'
 	],
 	'run pg_createsubscriber on node S');
-- 
2.41.0.windows.3