vacuumdb, clusterdb, reindexdb: Add --continue With --all, these commands connect 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. Managed PostgreSQL services commonly reserve a database for the provider that ordinary users cannot connect to, which defeats --all on those systems. Add --continue, which reports such a database as a warning and moves on to the next one. Without the option the previous behavior is unchanged, and the option is rejected unless --all is also given, so a database named on the command line is never silently skipped. cluster_one_database() and reindex_one_database() now return a bool indicating whether the database was processed, since both are called more than once for a single database and only the first call should report it. 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/clusterdb.sgml b/doc/src/sgml/ref/clusterdb.sgml index 0d2051bf6f1..0c97067526c 100644 --- a/doc/src/sgml/ref/clusterdb.sgml +++ b/doc/src/sgml/ref/clusterdb.sgml @@ -84,6 +84,26 @@ PostgreSQL documentation + + + + + When used with , report a database that cannot + be connected to as a warning and continue with the remaining + databases, instead of terminating with an error. This option can + only be used together with . + + + This is chiefly of use with managed + PostgreSQL services, which commonly + include a database reserved for the provider that other users are not + permitted to connect to; without this option such a database prevents + from processing any of the databases that + follow it. + + + + diff --git a/doc/src/sgml/ref/reindexdb.sgml b/doc/src/sgml/ref/reindexdb.sgml index a90e48ea86b..5d5fda88500 100644 --- a/doc/src/sgml/ref/reindexdb.sgml +++ b/doc/src/sgml/ref/reindexdb.sgml @@ -121,6 +121,26 @@ PostgreSQL documentation + + + + + When used with , report a database that cannot + be connected to as a warning and continue with the remaining + databases, instead of terminating with an error. This option can + only be used together with . + + + This is chiefly of use with managed + PostgreSQL services, which commonly + include a database reserved for the provider that other users are not + permitted to connect to; without this option such a database prevents + from processing any of the databases that + follow it. + + + + diff --git a/doc/src/sgml/ref/vacuumdb.sgml b/doc/src/sgml/ref/vacuumdb.sgml index 508c8dfa146..efe9a531d1a 100644 --- a/doc/src/sgml/ref/vacuumdb.sgml +++ b/doc/src/sgml/ref/vacuumdb.sgml @@ -144,6 +144,26 @@ PostgreSQL documentation + + + + + When used with , report a database that cannot + be connected to as a warning and continue with the remaining + databases, instead of terminating with an error. This option can + only be used together with . + + + This is chiefly of use with managed + PostgreSQL services, which commonly + include a database reserved for the provider that other users are not + permitted to connect to; without this option such a database prevents + from processing any of the databases that + follow it. + + + + diff --git a/src/bin/scripts/clusterdb.c b/src/bin/scripts/clusterdb.c index 53bbb42c883..24a99baf9bf 100644 --- a/src/bin/scripts/clusterdb.c +++ b/src/bin/scripts/clusterdb.c @@ -18,11 +18,12 @@ #include "fe_utils/simple_list.h" -static void cluster_one_database(const ConnParams *cparams, const char *table, - const char *progname, bool verbose, bool echo); +static bool cluster_one_database(const ConnParams *cparams, const char *table, + const char *progname, bool verbose, bool echo, + bool conn_fail_ok); static void cluster_all_databases(ConnParams *cparams, SimpleStringList *tables, const char *progname, bool verbose, bool echo, - bool quiet); + bool quiet, bool conn_fail_ok); static void help(const char *progname); @@ -42,6 +43,7 @@ main(int argc, char *argv[]) {"table", required_argument, NULL, 't'}, {"verbose", no_argument, NULL, 'v'}, {"maintenance-db", required_argument, NULL, 2}, + {"continue", no_argument, NULL, 3}, {NULL, 0, NULL, 0} }; @@ -60,6 +62,7 @@ main(int argc, char *argv[]) bool quiet = false; bool alldb = false; bool verbose = false; + bool conn_fail_ok = false; SimpleStringList tables = {NULL, NULL}; pg_logging_init(argv[0]); @@ -108,6 +111,9 @@ main(int argc, char *argv[]) case 2: maintenance_db = pg_strdup(optarg); break; + case 3: + conn_fail_ok = true; + break; default: /* getopt_long already emitted a complaint */ pg_log_error_hint("Try \"%s --help\" for more information.", progname); @@ -142,6 +148,11 @@ main(int argc, char *argv[]) setup_cancel_handler(NULL); + /* --continue only governs the all-databases loop. */ + if (conn_fail_ok && !alldb) + pg_fatal("cannot use the \"%s\" option without \"%s\"", + "continue", "all"); + if (alldb) { if (dbname) @@ -150,7 +161,7 @@ main(int argc, char *argv[]) cparams.dbname = maintenance_db; cluster_all_databases(&cparams, &tables, - progname, verbose, echo, quiet); + progname, verbose, echo, quiet, conn_fail_ok); } else { @@ -173,27 +184,40 @@ main(int argc, char *argv[]) for (cell = tables.head; cell; cell = cell->next) { cluster_one_database(&cparams, cell->val, - progname, verbose, echo); + progname, verbose, echo, false); } } else cluster_one_database(&cparams, NULL, - progname, verbose, echo); + progname, verbose, echo, false); } exit(0); } -static void +/* + * Cluster one database (or one table in it). Returns false if the database + * had to be skipped because it could not be connected to, which is only + * possible when conn_fail_ok is set; see cluster_all_databases(). + */ +static bool cluster_one_database(const ConnParams *cparams, const char *table, - const char *progname, bool verbose, bool echo) + const char *progname, bool verbose, bool echo, + bool conn_fail_ok) { PQExpBufferData sql; PGconn *conn; - conn = connectDatabase(cparams, progname, echo, false, true); + conn = connectDatabase(cparams, progname, echo, conn_fail_ok, true); + if (PQstatus(conn) == CONNECTION_BAD) + { + pg_log_warning("skipping database \"%s\": %s", + PQdb(conn), PQerrorMessage(conn)); + PQfinish(conn); + return false; + } initPQExpBuffer(&sql); @@ -220,13 +244,15 @@ cluster_one_database(const ConnParams *cparams, const char *table, } PQfinish(conn); termPQExpBuffer(&sql); + + return true; } static void cluster_all_databases(ConnParams *cparams, SimpleStringList *tables, const char *progname, bool verbose, bool echo, - bool quiet) + bool quiet, bool conn_fail_ok) { PGconn *conn; PGresult *result; @@ -255,12 +281,17 @@ cluster_all_databases(ConnParams *cparams, SimpleStringList *tables, SimpleStringListCell *cell; for (cell = tables->head; cell; cell = cell->next) - cluster_one_database(cparams, cell->val, - progname, verbose, echo); + { + /* one warning per skipped database, not per table */ + if (!cluster_one_database(cparams, cell->val, + progname, verbose, echo, + conn_fail_ok)) + break; + } } else cluster_one_database(cparams, NULL, - progname, verbose, echo); + progname, verbose, echo, conn_fail_ok); } PQclear(result); @@ -275,6 +306,8 @@ help(const char *progname) printf(_(" %s [OPTION]... [DBNAME]\n"), progname); printf(_("\nOptions:\n")); printf(_(" -a, --all cluster all databases\n")); + printf(_(" --continue with --all, skip databases that cannot be\n" + " connected to instead of exiting\n")); printf(_(" -d, --dbname=DBNAME database to cluster\n")); printf(_(" -e, --echo show the commands being sent to the server\n")); printf(_(" -q, --quiet don't write any messages\n")); diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c index d7fb16d3c85..e0e0f47657d 100644 --- a/src/bin/scripts/reindexdb.c +++ b/src/bin/scripts/reindexdb.c @@ -42,18 +42,20 @@ static void get_parallel_tabidx_list(PGconn *conn, SimpleStringList *index_list, SimpleOidList **table_list, bool echo); -static void reindex_one_database(ConnParams *cparams, ReindexType type, +static bool reindex_one_database(ConnParams *cparams, ReindexType type, SimpleStringList *user_list, const char *progname, bool echo, bool verbose, bool concurrently, - int concurrentCons, const char *tablespace); + int concurrentCons, const char *tablespace, + bool conn_fail_ok); static void reindex_all_databases(ConnParams *cparams, const char *progname, bool echo, bool quiet, bool verbose, bool concurrently, int concurrentCons, const char *tablespace, bool syscatalog, SimpleStringList *schemas, SimpleStringList *tables, - SimpleStringList *indexes); + SimpleStringList *indexes, + bool conn_fail_ok); static void gen_reindex_command(PGconn *conn, ReindexType type, const char *name, bool echo, bool verbose, bool concurrently, const char *tablespace, @@ -86,6 +88,7 @@ main(int argc, char *argv[]) {"concurrently", no_argument, NULL, 1}, {"maintenance-db", required_argument, NULL, 2}, {"tablespace", required_argument, NULL, 3}, + {"continue", no_argument, NULL, 4}, {NULL, 0, NULL, 0} }; @@ -107,6 +110,7 @@ main(int argc, char *argv[]) bool quiet = false; bool verbose = false; bool concurrently = false; + bool conn_fail_ok = false; SimpleStringList indexes = {NULL, NULL}; SimpleStringList tables = {NULL, NULL}; SimpleStringList schemas = {NULL, NULL}; @@ -179,6 +183,9 @@ main(int argc, char *argv[]) case 3: tablespace = pg_strdup(optarg); break; + case 4: + conn_fail_ok = true; + break; default: /* getopt_long already emitted a complaint */ pg_log_error_hint("Try \"%s --help\" for more information.", progname); @@ -216,6 +223,11 @@ main(int argc, char *argv[]) if (concurrentCons > 1 && syscatalog) pg_fatal("cannot use multiple jobs to reindex system catalogs"); + /* --continue only governs the all-databases loop. */ + if (conn_fail_ok && !alldb) + pg_fatal("cannot use the \"%s\" option without \"%s\"", + "continue", "all"); + if (alldb) { if (dbname) @@ -225,7 +237,8 @@ main(int argc, char *argv[]) reindex_all_databases(&cparams, progname, echo, quiet, verbose, concurrently, concurrentCons, tablespace, - syscatalog, &schemas, &tables, &indexes); + syscatalog, &schemas, &tables, &indexes, + conn_fail_ok); } else { @@ -244,22 +257,25 @@ main(int argc, char *argv[]) if (syscatalog) reindex_one_database(&cparams, REINDEX_SYSTEM, NULL, progname, echo, verbose, - concurrently, 1, tablespace); + concurrently, 1, tablespace, false); if (schemas.head != NULL) reindex_one_database(&cparams, REINDEX_SCHEMA, &schemas, progname, echo, verbose, - concurrently, concurrentCons, tablespace); + concurrently, concurrentCons, tablespace, + false); if (indexes.head != NULL) reindex_one_database(&cparams, REINDEX_INDEX, &indexes, progname, echo, verbose, - concurrently, concurrentCons, tablespace); + concurrently, concurrentCons, tablespace, + false); if (tables.head != NULL) reindex_one_database(&cparams, REINDEX_TABLE, &tables, progname, echo, verbose, - concurrently, concurrentCons, tablespace); + concurrently, concurrentCons, tablespace, + false); /* * reindex database only if neither index nor table nor schema nor @@ -269,18 +285,24 @@ main(int argc, char *argv[]) tables.head == NULL && schemas.head == NULL) reindex_one_database(&cparams, REINDEX_DATABASE, NULL, progname, echo, verbose, - concurrently, concurrentCons, tablespace); + concurrently, concurrentCons, tablespace, + false); } exit(0); } -static void +/* + * Reindex one database. Returns false if the database had to be skipped + * because it could not be connected to, which is only possible when + * conn_fail_ok is set; see reindex_all_databases(). + */ +static bool reindex_one_database(ConnParams *cparams, ReindexType type, SimpleStringList *user_list, const char *progname, bool echo, bool verbose, bool concurrently, int concurrentCons, - const char *tablespace) + const char *tablespace, bool conn_fail_ok) { PGconn *conn; SimpleStringListCell *cell; @@ -294,7 +316,14 @@ reindex_one_database(ConnParams *cparams, ReindexType type, int items_count = 0; ParallelSlot *free_slot = NULL; - conn = connectDatabase(cparams, progname, echo, false, true); + conn = connectDatabase(cparams, progname, echo, conn_fail_ok, true); + if (PQstatus(conn) == CONNECTION_BAD) + { + pg_log_warning("skipping database \"%s\": %s", + PQdb(conn), PQerrorMessage(conn)); + PQfinish(conn); + return false; + } if (concurrently && PQserverVersion(conn) < 120000) { @@ -353,7 +382,7 @@ reindex_one_database(ConnParams *cparams, ReindexType type, if (process_list == NULL) { PQfinish(conn); - return; + return true; } break; @@ -371,7 +400,7 @@ reindex_one_database(ConnParams *cparams, ReindexType type, if (tableoid_list == NULL) { PQfinish(conn); - return; + return true; } indices_tables_cell = tableoid_list->head; @@ -489,6 +518,8 @@ finish: if (failed) exit(1); + + return true; } /* @@ -833,7 +864,7 @@ reindex_all_databases(ConnParams *cparams, bool concurrently, int concurrentCons, const char *tablespace, bool syscatalog, SimpleStringList *schemas, SimpleStringList *tables, - SimpleStringList *indexes) + SimpleStringList *indexes, bool conn_fail_ok) { PGconn *conn; PGresult *result; @@ -857,25 +888,35 @@ reindex_all_databases(ConnParams *cparams, cparams->override_dbname = dbname; - if (syscatalog) - reindex_one_database(cparams, REINDEX_SYSTEM, NULL, - progname, echo, verbose, - concurrently, 1, tablespace); + /* + * A skipped database is reported once, by whichever call below runs + * first for it; the remaining work for that database is dropped. + */ + if (syscatalog && + !reindex_one_database(cparams, REINDEX_SYSTEM, NULL, + progname, echo, verbose, + concurrently, 1, tablespace, conn_fail_ok)) + continue; - if (schemas->head != NULL) - reindex_one_database(cparams, REINDEX_SCHEMA, schemas, - progname, echo, verbose, - concurrently, concurrentCons, tablespace); + if (schemas->head != NULL && + !reindex_one_database(cparams, REINDEX_SCHEMA, schemas, + progname, echo, verbose, + concurrently, concurrentCons, tablespace, + conn_fail_ok)) + continue; - if (indexes->head != NULL) - reindex_one_database(cparams, REINDEX_INDEX, indexes, - progname, echo, verbose, - concurrently, 1, tablespace); + if (indexes->head != NULL && + !reindex_one_database(cparams, REINDEX_INDEX, indexes, + progname, echo, verbose, + concurrently, 1, tablespace, conn_fail_ok)) + continue; - if (tables->head != NULL) - reindex_one_database(cparams, REINDEX_TABLE, tables, - progname, echo, verbose, - concurrently, concurrentCons, tablespace); + if (tables->head != NULL && + !reindex_one_database(cparams, REINDEX_TABLE, tables, + progname, echo, verbose, + concurrently, concurrentCons, tablespace, + conn_fail_ok)) + continue; /* * reindex database only if neither index nor table nor schema nor @@ -885,7 +926,8 @@ reindex_all_databases(ConnParams *cparams, tables->head == NULL && schemas->head == NULL) reindex_one_database(cparams, REINDEX_DATABASE, NULL, progname, echo, verbose, - concurrently, concurrentCons, tablespace); + concurrently, concurrentCons, tablespace, + conn_fail_ok); } PQclear(result); @@ -900,6 +942,8 @@ help(const char *progname) printf(_("\nOptions:\n")); printf(_(" -a, --all reindex all databases\n")); printf(_(" --concurrently reindex concurrently\n")); + printf(_(" --continue with --all, skip databases that cannot be\n" + " connected to instead of exiting\n")); printf(_(" -d, --dbname=DBNAME database to reindex\n")); printf(_(" -e, --echo show the commands being sent to the server\n")); printf(_(" -i, --index=INDEX recreate specific index(es) only\n")); diff --git a/src/bin/scripts/t/010_clusterdb.pl b/src/bin/scripts/t/010_clusterdb.pl index 56e6881244b..cf5e90ef5ba 100644 --- a/src/bin/scripts/t/010_clusterdb.pl +++ b/src/bin/scripts/t/010_clusterdb.pl @@ -37,4 +37,31 @@ $node->issues_sql_like( $node->command_ok([qw(clusterdb --echo --verbose dbname=template1)], 'clusterdb with connection string'); +# A database that cannot be connected to terminates --all, unless --continue +# is given. This needs its own cluster, since connections can only be refused +# to a role that is not a superuser. +my $cnode = PostgreSQL::Test::Cluster->new('continue'); +$cnode->init(auth_extra => [ '--create-role' => 'regress_continue' ]); +$cnode->start; +$cnode->safe_psql('postgres', + 'CREATE ROLE regress_continue LOGIN IN ROLE pg_maintain'); +$cnode->safe_psql('postgres', 'CREATE DATABASE regress_noconn'); +$cnode->safe_psql('postgres', + 'REVOKE CONNECT ON DATABASE regress_noconn FROM PUBLIC'); + +$cnode->command_fails_like( + [ 'clusterdb', '--all', '--username' => 'regress_continue' ], + qr/permission denied for database "regress_noconn"/, + '--all fails on a database that cannot be connected to'); +$cnode->command_fails_like( + [ 'clusterdb', '--continue', 'postgres' ], + qr/cannot use the "continue" option without "all"/, + '--continue requires --all'); +$cnode->command_checks_all( + [ 'clusterdb', '--all', '--continue', '--username' => 'regress_continue' ], + 0, + [qr/clustering database "template1"/], + [qr/warning: skipping database "regress_noconn": /], + '--all --continue skips databases that cannot be connected to'); + done_testing(); diff --git a/src/bin/scripts/t/090_reindexdb.pl b/src/bin/scripts/t/090_reindexdb.pl index ae7d3724464..81a5a307e27 100644 --- a/src/bin/scripts/t/090_reindexdb.pl +++ b/src/bin/scripts/t/090_reindexdb.pl @@ -323,4 +323,31 @@ $node->issues_sql_like( qr/statement:\ REINDEX SCHEMA pg_catalog;/, 'specify both --system and --schema'); +# A database that cannot be connected to terminates --all, unless --continue +# is given. This needs its own cluster, since connections can only be refused +# to a role that is not a superuser. +my $cnode = PostgreSQL::Test::Cluster->new('continue'); +$cnode->init(auth_extra => [ '--create-role' => 'regress_continue' ]); +$cnode->start; +$cnode->safe_psql('postgres', + 'CREATE ROLE regress_continue LOGIN IN ROLE pg_maintain'); +$cnode->safe_psql('postgres', 'CREATE DATABASE regress_noconn'); +$cnode->safe_psql('postgres', + 'REVOKE CONNECT ON DATABASE regress_noconn FROM PUBLIC'); + +$cnode->command_fails_like( + [ 'reindexdb', '--all', '--username' => 'regress_continue' ], + qr/permission denied for database "regress_noconn"/, + '--all fails on a database that cannot be connected to'); +$cnode->command_fails_like( + [ 'reindexdb', '--continue', 'postgres' ], + qr/cannot use the "continue" option without "all"/, + '--continue requires --all'); +$cnode->command_checks_all( + [ 'reindexdb', '--all', '--continue', '--username' => 'regress_continue' ], + 0, + [qr/reindexing database "template1"/], + [qr/warning: skipping database "regress_noconn": /], + '--all --continue skips databases that cannot be connected to'); + done_testing(); diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl index 58e38971b3d..feeb4cb51c2 100644 --- a/src/bin/scripts/t/100_vacuumdb.pl +++ b/src/bin/scripts/t/100_vacuumdb.pl @@ -373,4 +373,32 @@ $node->issues_sql_unlike( qr/statement:\ VACUUM/sx, '--analyze-only does not run vacuum'); +# A database that cannot be connected to terminates --all, unless --continue +# is given. This needs its own cluster: connections can only be refused to a +# non-superuser, and the tests above leave behind objects that such a role +# cannot process (pg_maintain covers the relations, but not schema USAGE). +my $cnode = PostgreSQL::Test::Cluster->new('continue'); +$cnode->init(auth_extra => [ '--create-role' => 'regress_continue' ]); +$cnode->start; +$cnode->safe_psql('postgres', + 'CREATE ROLE regress_continue LOGIN IN ROLE pg_maintain'); +$cnode->safe_psql('postgres', 'CREATE DATABASE regress_noconn'); +$cnode->safe_psql('postgres', + 'REVOKE CONNECT ON DATABASE regress_noconn FROM PUBLIC'); + +$cnode->command_fails_like( + [ 'vacuumdb', '--all', '--username' => 'regress_continue' ], + qr/permission denied for database "regress_noconn"/, + '--all fails on a database that cannot be connected to'); +$cnode->command_fails_like( + [ 'vacuumdb', '--continue', 'postgres' ], + qr/cannot use the "continue" option without "all"/, + '--continue requires --all'); +$cnode->command_checks_all( + [ 'vacuumdb', '--all', '--continue', '--username' => 'regress_continue' ], + 0, + [qr/vacuuming database "template1"/], + [qr/warning: skipping database "regress_noconn": /], + '--all --continue skips databases that cannot be connected to'); + done_testing(); diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c index f8158ca6a78..30fc584fdf2 100644 --- a/src/bin/scripts/vacuumdb.c +++ b/src/bin/scripts/vacuumdb.c @@ -60,6 +60,7 @@ main(int argc, char *argv[]) {"buffer-usage-limit", required_argument, NULL, 13}, {"missing-stats-only", no_argument, NULL, 14}, {"dry-run", no_argument, NULL, 15}, + {"continue", no_argument, NULL, 16}, {NULL, 0, NULL, 0} }; @@ -211,6 +212,9 @@ main(int argc, char *argv[]) case 15: vacopts.dry_run = true; break; + case 16: + vacopts.conn_fail_ok = true; + break; default: /* getopt_long already emitted a complaint */ pg_log_error_hint("Try \"%s --help\" for more information.", progname); @@ -307,6 +311,11 @@ main(int argc, char *argv[]) pg_fatal("cannot use the \"%s\" option without \"%s\" or \"%s\"", "missing-stats-only", "analyze-only", "analyze-in-stages"); + /* Prohibit --continue without --all: it only governs the database loop. */ + if (vacopts.conn_fail_ok && !(vacopts.objfilter & OBJFILTER_ALL_DBS)) + pg_fatal("cannot use the \"%s\" option without \"%s\"", + "continue", "all"); + if (vacopts.dry_run && !vacopts.quiet) { pg_log_info("executing in dry-run mode"); @@ -353,6 +362,8 @@ help(const char *progname) printf(_("\nOptions:\n")); printf(_(" -a, --all vacuum all databases\n")); printf(_(" --buffer-usage-limit=SIZE size of ring buffer used for vacuum\n")); + printf(_(" --continue with --all, skip databases that cannot be\n" + " connected to instead of exiting\n")); printf(_(" -d, --dbname=DBNAME database to vacuum\n")); printf(_(" --disable-page-skipping disable all page-skipping behavior\n")); printf(_(" --dry-run show the commands that would be sent to the server\n")); diff --git a/src/bin/scripts/vacuuming.c b/src/bin/scripts/vacuuming.c index 855a5754c98..9ae2267e114 100644 --- a/src/bin/scripts/vacuuming.c +++ b/src/bin/scripts/vacuuming.c @@ -192,7 +192,24 @@ vacuum_one_database(ConnParams *cparams, Assert(stage == ANALYZE_NO_STAGE || (stage >= 0 && stage < ANALYZE_NUM_STAGES)); - conn = connectDatabase(cparams, progname, vacopts->echo, false, true); + /* + * With --continue, a database 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. The option is rejected unless --all was given, so + * this cannot quietly skip a database named on the command line. + */ + conn = connectDatabase(cparams, progname, vacopts->echo, + vacopts->conn_fail_ok, 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/bin/scripts/vacuuming.h b/src/bin/scripts/vacuuming.h index 5a491db2526..1ac7645823c 100644 --- a/src/bin/scripts/vacuuming.h +++ b/src/bin/scripts/vacuuming.h @@ -54,6 +54,8 @@ typedef struct vacuumingOptions bool echo; bool quiet; bool dry_run; + bool conn_fail_ok; /* --continue: skip databases that cannot be + * connected to (only valid with --all) */ } vacuumingOptions; /* Valid values for vacuumingOptions->objfilter */ 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); }