vacuumdb: Don't let an unconnectable database abort --all

vacuumdb --all connects to each database in turn, and a failed
connection was fatal, so a single database that refuses connections
prevented every remaining database from being processed.  Some managed
PostgreSQL services (CloudSQL in particular) reserve a database for the
provider that ordinary users cannot connect to, which defeats --all
entirely on those systems.

Report such a database as a warning and continue with the rest.
Databases named explicitly are unaffected: failing to connect to those
is still an error.

So that the warning can carry the reason the connection failed,
connectDatabase() now hands a failed connection back to the caller when
fail_ok is set, rather than closing it and returning NULL; its only
existing fail_ok caller, connectMaintenanceDatabase(), is adjusted to
match.
---
diff --git a/doc/src/sgml/ref/vacuumdb.sgml b/doc/src/sgml/ref/vacuumdb.sgml
index 508c8dfa146..b96b1aa84cf 100644
--- a/doc/src/sgml/ref/vacuumdb.sgml
+++ b/doc/src/sgml/ref/vacuumdb.sgml
@@ -128,6 +128,13 @@ PostgreSQL documentation
        <para>
         Vacuum all databases.
        </para>
+       <para>
+        Databases that cannot be connected to are skipped with a warning, and
+        the remaining databases are still processed.  This is chiefly of use
+        with managed <productname>PostgreSQL</productname> services, which
+        commonly include a database reserved for the provider that other
+        users are not permitted to connect to.
+       </para>
       </listitem>
      </varlistentry>

diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl
index 58e38971b3d..3559d125cd1 100644
--- a/src/bin/scripts/t/100_vacuumdb.pl
+++ b/src/bin/scripts/t/100_vacuumdb.pl
@@ -13,7 +13,7 @@ program_version_ok('vacuumdb');
 program_options_handling_ok('vacuumdb');

 my $node = PostgreSQL::Test::Cluster->new('main');
-$node->init;
+$node->init(auth_extra => [ '--create-role' => 'regress_vacuumdb_user' ]);
 $node->start;

 $node->issues_sql_like(
@@ -373,4 +373,21 @@ $node->issues_sql_unlike(
 	qr/statement:\ VACUUM/sx,
 	'--analyze-only does not run vacuum');

+# A database that the user cannot connect to must not abandon --all: it is
+# skipped with a warning, and the other databases are still processed.  Run as
+# an unprivileged role, since a superuser is exempt from the CONNECT check.
+$node->safe_psql('postgres', 'CREATE ROLE regress_vacuumdb_user LOGIN');
+$node->safe_psql('postgres', 'CREATE DATABASE regress_vacuumdb_noconn');
+$node->safe_psql('postgres',
+	'REVOKE CONNECT ON DATABASE regress_vacuumdb_noconn FROM PUBLIC');
+$node->command_checks_all(
+	[
+		'vacuumdb', '--all',
+		'--analyze-only', '--username' => 'regress_vacuumdb_user'
+	],
+	0,
+	[qr/^$/],
+	[qr/warning: skipping database "regress_vacuumdb_noconn": /],
+	'--all skips databases that cannot be connected to');
+
 done_testing();
diff --git a/src/bin/scripts/vacuuming.c b/src/bin/scripts/vacuuming.c
index 855a5754c98..fb6db77f761 100644
--- a/src/bin/scripts/vacuuming.c
+++ b/src/bin/scripts/vacuuming.c
@@ -192,7 +192,25 @@ vacuum_one_database(ConnParams *cparams,
 	Assert(stage == ANALYZE_NO_STAGE ||
 		   (stage >= 0 && stage < ANALYZE_NUM_STAGES));

-	conn = connectDatabase(cparams, progname, vacopts->echo, false, true);
+	/*
+	 * When processing every database, one we cannot connect to is not a fatal
+	 * condition: managed environments routinely include a database reserved
+	 * for the hosting provider that ordinary users are refused entry to, and
+	 * treating that as fatal would abandon the run before reaching the
+	 * databases the user actually cares about.  Warn and let the caller move
+	 * on to the next one.  Any other database was named explicitly, so
+	 * failing to connect to it remains an error.
+	 */
+	conn = connectDatabase(cparams, progname, vacopts->echo,
+						   (vacopts->objfilter & OBJFILTER_ALL_DBS) != 0,
+						   true);
+	if (PQstatus(conn) == CONNECTION_BAD)
+	{
+		pg_log_warning("skipping database \"%s\": %s",
+					   PQdb(conn), PQerrorMessage(conn));
+		PQfinish(conn);
+		return EXIT_SUCCESS;
+	}

 	if (vacopts->disable_page_skipping && PQserverVersion(conn) < 90600)
 	{
diff --git a/src/fe_utils/connect_utils.c b/src/fe_utils/connect_utils.c
index b117718513c..6fa81ce404a 100644
--- a/src/fe_utils/connect_utils.c
+++ b/src/fe_utils/connect_utils.c
@@ -27,6 +27,11 @@
  * given during previous calls to this routine.  (Callers should not pass
  * allow_password_reuse=true unless reconnecting to the same host+port+user
  * as before, else we might create password exposure hazards.)
+ *
+ * If fail_ok is true, a connection that could not be established is handed
+ * back to the caller with status CONNECTION_BAD, just as PQconnectdb would
+ * do; the caller must then report PQerrorMessage() as it sees fit and call
+ * PQfinish() on it.  Otherwise, a failure is reported and we exit.
  */
 PGconn *
 connectDatabase(const ConnParams *cparams, const char *progname,
@@ -109,10 +114,7 @@ connectDatabase(const ConnParams *cparams, const char *progname,
 	if (PQstatus(conn) == CONNECTION_BAD)
 	{
 		if (fail_ok)
-		{
-			PQfinish(conn);
-			return NULL;
-		}
+			return conn;		/* caller reports the error and closes it */
 		pg_fatal("%s", PQerrorMessage(conn));
 	}

@@ -143,8 +145,9 @@ connectMaintenanceDatabase(ConnParams *cparams,
 	/* Otherwise, try postgres first and then template1. */
 	cparams->dbname = "postgres";
 	conn = connectDatabase(cparams, progname, echo, true, false);
-	if (!conn)
+	if (PQstatus(conn) == CONNECTION_BAD)
 	{
+		PQfinish(conn);
 		cparams->dbname = "template1";
 		conn = connectDatabase(cparams, progname, echo, false, false);
 	}