Fix connection string handling in src/bin/scripts/ programs.

When told to process all databases, clusterdb, reindexdb, and vacuumdb
would reconnect by replacing their --maintenance-db parameter with the
name of the target database.  If that parameter is a connstring (which
has been allowed for a long time, though we failed to document that
before this patch), we'd lose any other options it might specify, for
example SSL or GSS parameters, possibly resulting in failure to connect.
Thus, this is the same bug as commit a45bc8a4f fixed in pg_dump and
pg_restore.  We can fix it in the same way, by using libpq's rules for
handling multiple "dbname" parameters to add the target database name
separately.  I chose to apply the same refactoring approach as in that
patch, with a struct to handle the command line parameters that need to
be passed through to connectDatabase.  (Maybe someday we can unify the
very similar functions here and in pg_dump/pg_restore.)

Per Peter Eisentraut's comments on bug #16604.  Back-patch to all
supported branches.

Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org

Author: tglsfdc

doc/src/sgml/ref/clusterdb.sgml

@@ -90,9 +90,9 @@ PostgreSQL documentation
       <term><option><optional>--dbname=</optional><replaceable class="parameter">dbname</replaceable></option></term>
       <listitem>
        <para>
-        Specifies the name of the database to be clustered.
-        If this is not specified and <option>-a</option> (or
-        <option>--all</option>) is not used, the database name is read
+        Specifies the name of the database to be clustered,
+        when <option>-a</option>/<option>--all</option> is not used.
+        If this is not specified, the database name is read
         from the environment variable <envar>PGDATABASE</envar>.  If
         that is not set, the user name specified for the connection is
         used.  The <replaceable>dbname</replaceable> can be a <link
@@ -249,10 +249,16 @@ PostgreSQL documentation
       <term><option>--maintenance-db=<replaceable class="parameter">dbname</replaceable></option></term>
       <listitem>
        <para>
-         Specifies the name of the database to connect to discover what other
-         databases should be clustered. If not specified, the
-         <literal>postgres</literal> database will be used,
-         and if that does not exist, <literal>template1</literal> will be used.
+        Specifies the name of the database to connect to to discover which
+        databases should be clustered,
+        when <option>-a</option>/<option>--all</option> is used.
+        If not specified, the <literal>postgres</literal> database will be used,
+        or if that does not exist, <literal>template1</literal> will be used.
+        This can be a <link linkend="libpq-connstring">connection
+        string</link>.  If so, connection string parameters will override any
+        conflicting command line options.  Also, connection string parameters
+        other than the database name itself will be re-used when connecting
+        to other databases.
        </para>
       </listitem>
      </varlistentry>

doc/src/sgml/ref/createdb.sgml

@@ -284,6 +284,9 @@ PostgreSQL documentation
          database will be used; if that does not exist (or if it is the name
          of the new database being created), <literal>template1</literal> will
          be used.
+         This can be a <link linkend="libpq-connstring">connection
+         string</link>.  If so, connection string parameters will override any
+         conflicting command line options.
        </para>
       </listitem>
      </varlistentry>

doc/src/sgml/ref/dropdb.sgml

@@ -217,6 +217,9 @@ PostgreSQL documentation
          target database. If not specified, the <literal>postgres</literal>
          database will be used; if that does not exist (or is the database
          being dropped), <literal>template1</literal> will be used.
+         This can be a <link linkend="libpq-connstring">connection
+         string</link>.  If so, connection string parameters will override any
+         conflicting command line options.
        </para>
       </listitem>
      </varlistentry>

doc/src/sgml/ref/reindexdb.sgml

@@ -134,9 +134,9 @@ PostgreSQL documentation
       <term><option><optional>--dbname=</optional><replaceable class="parameter">dbname</replaceable></option></term>
       <listitem>
        <para>
-        Specifies the name of the database to be reindexed.
-        If this is not specified and <option>-a</option> (or
-        <option>--all</option>) is not used, the database name is read
+        Specifies the name of the database to be reindexed,
+        when <option>-a</option>/<option>--all</option> is not used.
+        If this is not specified, the database name is read
         from the environment variable <envar>PGDATABASE</envar>.  If
         that is not set, the user name specified for the connection is
         used.  The <replaceable>dbname</replaceable> can be a <link
@@ -351,10 +351,16 @@ PostgreSQL documentation
       <term><option>--maintenance-db=<replaceable class="parameter">dbname</replaceable></option></term>
       <listitem>
        <para>
-         Specifies the name of the database to connect to discover what other
-         databases should be reindexed. If not specified, the
-         <literal>postgres</literal> database will be used,
-         and if that does not exist, <literal>template1</literal> will be used.
+        Specifies the name of the database to connect to to discover which
+        databases should be reindexed,
+        when <option>-a</option>/<option>--all</option> is used.
+        If not specified, the <literal>postgres</literal> database will be used,
+        or if that does not exist, <literal>template1</literal> will be used.
+        This can be a <link linkend="libpq-connstring">connection
+        string</link>.  If so, connection string parameters will override any
+        conflicting command line options.  Also, connection string parameters
+        other than the database name itself will be re-used when connecting
+        to other databases.
        </para>
       </listitem>
      </varlistentry>

doc/src/sgml/ref/vacuumdb.sgml

@@ -92,9 +92,9 @@ PostgreSQL documentation
       <term><option><optional>--dbname=</optional><replaceable class="parameter">dbname</replaceable></option></term>
       <listitem>
        <para>
-        Specifies the name of the database to be cleaned or analyzed.
-        If this is not specified and <option>-a</option> (or
-        <option>--all</option>) is not used, the database name is read
+        Specifies the name of the database to be cleaned or analyzed,
+        when <option>-a</option>/<option>--all</option> is not used.
+        If this is not specified, the database name is read
         from the environment variable <envar>PGDATABASE</envar>.  If
         that is not set, the user name specified for the connection is
         used.  The <replaceable>dbname</replaceable> can be a <link
@@ -444,10 +444,16 @@ PostgreSQL documentation
       <term><option>--maintenance-db=<replaceable class="parameter">dbname</replaceable></option></term>
       <listitem>
        <para>
-         Specifies the name of the database to connect to discover what other
-         databases should be vacuumed. If not specified, the
-         <literal>postgres</literal> database will be used,
-         and if that does not exist, <literal>template1</literal> will be used.
+        Specifies the name of the database to connect to to discover which
+        databases should be vacuumed,
+        when <option>-a</option>/<option>--all</option> is used.
+        If not specified, the <literal>postgres</literal> database will be used,
+        or if that does not exist, <literal>template1</literal> will be used.
+        This can be a <link linkend="libpq-connstring">connection
+        string</link>.  If so, connection string parameters will override any
+        conflicting command line options.  Also, connection string parameters
+        other than the database name itself will be re-used when connecting
+        to other databases.
        </para>
       </listitem>
      </varlistentry>

src/bin/scripts/clusterdb.c

@@ -17,15 +17,10 @@
 #include "fe_utils/string_utils.h"
 
 
-static void cluster_one_database(const char *dbname, bool verbose, const char *table,
-								 const char *host, const char *port,
-								 const char *username, enum trivalue prompt_password,
-								 const char *progname, bool echo);
-static void cluster_all_databases(bool verbose, const char *maintenance_db,
-								  const char *host, const char *port,
-								  const char *username, enum trivalue prompt_password,
-								  const char *progname, bool echo, bool quiet);
-
+static void cluster_one_database(const ConnParams *cparams, const char *table,
+								 const char *progname, bool verbose, bool echo);
+static void cluster_all_databases(ConnParams *cparams, const char *progname,
+								  bool verbose, bool echo, bool quiet);
 static void help(const char *progname);
 
 
@@ -58,6 +53,7 @@ main(int argc, char *argv[])
 	char	   *port = NULL;
 	char	   *username = NULL;
 	enum trivalue prompt_password = TRI_DEFAULT;
+	ConnParams	cparams;
 	bool		echo = false;
 	bool		quiet = false;
 	bool		alldb = false;
@@ -134,6 +130,13 @@ main(int argc, char *argv[])
 		exit(1);
 	}
 
+	/* fill cparams except for dbname, which is set below */
+	cparams.pghost = host;
+	cparams.pgport = port;
+	cparams.pguser = username;
+	cparams.prompt_password = prompt_password;
+	cparams.override_dbname = NULL;
+
 	setup_cancel_handler(NULL);
 
 	if (alldb)
@@ -150,8 +153,9 @@ main(int argc, char *argv[])
 			exit(1);
 		}
 
-		cluster_all_databases(verbose, maintenance_db, host, port, username, prompt_password,
-							  progname, echo, quiet);
+		cparams.dbname = maintenance_db;
+
+		cluster_all_databases(&cparams, progname, verbose, echo, quiet);
 	}
 	else
 	{
@@ -165,39 +169,36 @@ main(int argc, char *argv[])
 				dbname = get_user_name_or_exit(progname);
 		}
 
+		cparams.dbname = dbname;
+
 		if (tables.head != NULL)
 		{
 			SimpleStringListCell *cell;
 
 			for (cell = tables.head; cell; cell = cell->next)
 			{
-				cluster_one_database(dbname, verbose, cell->val,
-									 host, port, username, prompt_password,
-									 progname, echo);
+				cluster_one_database(&cparams, cell->val,
+									 progname, verbose, echo);
 			}
 		}
 		else
-			cluster_one_database(dbname, verbose, NULL,
-								 host, port, username, prompt_password,
-								 progname, echo);
+			cluster_one_database(&cparams, NULL,
+								 progname, verbose, echo);
 	}
 
 	exit(0);
 }
 
 
 static void
-cluster_one_database(const char *dbname, bool verbose, const char *table,
-					 const char *host, const char *port,
-					 const char *username, enum trivalue prompt_password,
-					 const char *progname, bool echo)
+cluster_one_database(const ConnParams *cparams, const char *table,
+					 const char *progname, bool verbose, bool echo)
 {
 	PQExpBufferData sql;
 
 	PGconn	   *conn;
 
-	conn = connectDatabase(dbname, host, port, username, prompt_password,
-						   progname, echo, false, false);
+	conn = connectDatabase(cparams, progname, echo, false, false);
 
 	initPQExpBuffer(&sql);
 
@@ -228,22 +229,17 @@ cluster_one_database(const char *dbname, bool verbose, const char *table,
 
 
 static void
-cluster_all_databases(bool verbose, const char *maintenance_db,
-					  const char *host, const char *port,
-					  const char *username, enum trivalue prompt_password,
-					  const char *progname, bool echo, bool quiet)
+cluster_all_databases(ConnParams *cparams, const char *progname,
+					  bool verbose, bool echo, bool quiet)
 {
 	PGconn	   *conn;
 	PGresult   *result;
-	PQExpBufferData connstr;
 	int			i;
 
-	conn = connectMaintenanceDatabase(maintenance_db, host, port, username,
-									  prompt_password, progname, echo);
+	conn = connectMaintenanceDatabase(cparams, progname, echo);
 	result = executeQuery(conn, "SELECT datname FROM pg_database WHERE datallowconn ORDER BY 1;", echo);
 	PQfinish(conn);
 
-	initPQExpBuffer(&connstr);
 	for (i = 0; i < PQntuples(result); i++)
 	{
 		char	   *dbname = PQgetvalue(result, i, 0);
@@ -254,15 +250,10 @@ cluster_all_databases(bool verbose, const char *maintenance_db,
 			fflush(stdout);
 		}
 
-		resetPQExpBuffer(&connstr);
-		appendPQExpBufferStr(&connstr, "dbname=");
-		appendConnStrVal(&connstr, dbname);
+		cparams->override_dbname = dbname;
 
-		cluster_one_database(connstr.data, verbose, NULL,
-							 host, port, username, prompt_password,
-							 progname, echo);
+		cluster_one_database(cparams, NULL, progname, verbose, echo);
 	}
-	termPQExpBuffer(&connstr);
 
 	PQclear(result);
 }