Here is what I have staged for commit, which I'm planning to do soon. Thinking ahead to the automatic deletion project, I wonder if we should make DROP DATABASE work more like CHECKPOINT. That is, it would mark the database for deletion, but another process would do the actual work. That way, dropping a database follows the same path every time, regardless of whether the DROP command was interrupted. This would also open the door for a DROP DATABASE option to initiate the drop without waiting for it to complete, although I'm admittedly not sure that has many use-cases.
-- nathan
>From dc74fe4c7e1e5ddbd05e5d632575f4902b8bbe99 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Thu, 5 Nov 2026 08:48:42 -0600 Subject: [PATCH v4 1/1] Teach pg_upgrade to skip invalid databases. If DROP DATABASE is interrupted at the wrong moment, it leaves the pg_database row behind, but it is marked as invalid, i.e., datconnlimit is set to -2. Such databases cannot be connected to, and there is no supported recovery mechanism (besides of course restoring a backup to a time before the DROP DATABASE command). The only thing that a user can do is run DROP DATABASE on them again. Currently, pg_upgrade checks for invalid databases and refuses to proceed if any exist. This commit teaches pg_upgrade to instead simply skip them. The pg_database row and any associated files won't be transferred; the invalid database will just cease to exist on the newly upgraded cluster. In the future, we should probably try to automatically drop invalid databases during normal operation, but that is left as a future exercise. Author: Bharath Rupireddy <[email protected]> Discussion: https://postgr.es/m/CALj2ACV3jSPaMCSRQ8XZTN_meZezNyySpMJVhVBZy_qDJK2mmQ%40mail.gmail.com --- src/bin/pg_upgrade/check.c | 46 +++++++++++++------------- src/bin/pg_upgrade/info.c | 5 +-- src/bin/pg_upgrade/t/002_pg_upgrade.pl | 45 ++++--------------------- 3 files changed, 32 insertions(+), 64 deletions(-) diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index 150c4fc34b9..4c0d040d855 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -1117,7 +1117,6 @@ check_for_connection_status(ClusterInfo *cluster) int ntups; int i_datname; int i_datallowconn; - int i_datconnlimit; FILE *script = NULL; char output_path[MAXPGPATH]; @@ -1129,21 +1128,23 @@ check_for_connection_status(ClusterInfo *cluster) conn_template1 = connectToServer(cluster, "template1"); - /* get database names */ + /* + * Get database names. We skip invalid databases since they can only be + * dropped, and can't be upgraded. + */ dbres = executeQueryOrDie(conn_template1, "SELECT datname, datallowconn, datconnlimit " - "FROM pg_catalog.pg_database"); + "FROM pg_catalog.pg_database " + "WHERE datconnlimit <> -2"); i_datname = PQfnumber(dbres, "datname"); i_datallowconn = PQfnumber(dbres, "datallowconn"); - i_datconnlimit = PQfnumber(dbres, "datconnlimit"); ntups = PQntuples(dbres); for (dbnum = 0; dbnum < ntups; dbnum++) { char *datname = PQgetvalue(dbres, dbnum, i_datname); char *datallowconn = PQgetvalue(dbres, dbnum, i_datallowconn); - char *datconnlimit = PQgetvalue(dbres, dbnum, i_datconnlimit); if (strcmp(datname, "template0") == 0) { @@ -1152,20 +1153,16 @@ check_for_connection_status(ClusterInfo *cluster) pg_fatal("template0 must not allow connections, " "i.e. its pg_database.datallowconn must be false"); } - else + else if (strcmp(datallowconn, "f") == 0) { /* - * Avoid datallowconn == false databases from being skipped on - * restore, and ensure that no databases are marked invalid with - * datconnlimit == -2. + * avoid datallowconn == false databases from being skipped on + * restore */ - if ((strcmp(datallowconn, "f") == 0) || strcmp(datconnlimit, "-2") == 0) - { - if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL) - pg_fatal("could not open file \"%s\": %m", output_path); + if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL) + pg_fatal("could not open file \"%s\": %m", output_path); - fprintf(script, "%s\n", datname); - } + fprintf(script, "%s\n", datname); } } @@ -1178,11 +1175,11 @@ check_for_connection_status(ClusterInfo *cluster) fclose(script); pg_log(PG_REPORT, "fatal"); pg_fatal("All non-template0 databases must allow connections, i.e. their\n" - "pg_database.datallowconn must be true and pg_database.datconnlimit\n" - "must not be -2. Your installation contains non-template0 databases\n" - "which cannot be connected to. Consider allowing connection for all\n" - "non-template0 databases or drop the databases which do not allow\n" - "connections. A list of databases with the problem is in the file:\n" + "pg_database.datallowconn must be true. Your installation contains\n" + "non-template0 databases with their pg_database.datallowconn set to\n" + "false. Consider allowing connection for all non-template0 databases\n" + "or drop the databases which do not allow connections. A list of\n" + "databases with the problem is in the file:\n" " %s", output_path); } else @@ -1214,7 +1211,8 @@ check_for_unsupported_encodings(ClusterInfo *cluster) res = executeQueryOrDie(conn, "SELECT datname, encoding " - "FROM pg_catalog.pg_database"); + "FROM pg_catalog.pg_database " + "WHERE datconnlimit <> -2"); ntups = PQntuples(res); i_datname = PQfnumber(res, "datname"); i_encoding = PQfnumber(res, "encoding"); @@ -2570,7 +2568,8 @@ check_old_cluster_global_names(ClusterInfo *cluster) */ res = executeQueryOrDie(conn_template1, "SELECT datname AS objname, 'database' AS objtype " - "FROM pg_catalog.pg_database UNION ALL " + "FROM pg_catalog.pg_database " + "WHERE datconnlimit <> -2 UNION ALL " "SELECT rolname AS objname, 'role' AS objtype " "FROM pg_catalog.pg_roles UNION ALL " "SELECT spcname AS objname, 'tablespace' AS objtype " @@ -2645,7 +2644,8 @@ check_for_oldestxid_consistency(ClusterInfo *cluster) dbres = executeQueryOrDie(conn_template1, "SELECT datname, datfrozenxid, datminmxid " - "FROM pg_catalog.pg_database"); + "FROM pg_catalog.pg_database " + "WHERE datconnlimit <> -2"); i_datname = PQfnumber(dbres, "datname"); i_datfrozenxid = PQfnumber(dbres, "datfrozenxid"); diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c index 5c59cfb32e8..38e2416e5ba 100644 --- a/src/bin/pg_upgrade/info.c +++ b/src/bin/pg_upgrade/info.c @@ -412,8 +412,9 @@ get_db_infos(ClusterInfo *cluster) "FROM pg_catalog.pg_database d " " LEFT OUTER JOIN pg_catalog.pg_tablespace t " " ON d.dattablespace = t.oid " - "WHERE d.datallowconn = true " - "ORDER BY 1"); + "WHERE d.datallowconn = true %s" + "ORDER BY 1", + cluster == &old_cluster ? "AND d.datconnlimit <> -2 " : ""); res = executeQueryOrDie(conn, "%s", query); diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl index be57b2d838c..2f19a10533b 100644 --- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl +++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl @@ -551,7 +551,7 @@ SKIP: 'run of pg_upgrade --check with old instance running'); } -# Create an invalid database, will be deleted below +# Create an invalid database $oldnode->safe_psql( 'postgres', qq( CREATE DATABASE regression_invalid; @@ -561,44 +561,6 @@ $oldnode->safe_psql( # Upgrade the instance. $oldnode->stop; -# Check that pg_upgrade aborts when encountering an invalid database -# (However, versions that were out of support by commit c66a7d75e652 don't -# know how to do this, so skip this test there.) -SKIP: -{ - skip "database invalidation not implemented", 4 - if $oldnode->pg_version < 11; - - command_checks_all( - [ - 'pg_upgrade', '--no-sync', - '--old-datadir' => $oldnode->data_dir, - '--new-datadir' => $newnode->data_dir, - '--old-bindir' => $oldbindir, - '--new-bindir' => $newbindir, - '--socketdir' => $newnode->host, - '--old-port' => $oldnode->port, - '--new-port' => $newnode->port, - $mode, '--check', - ], - 1, - [qr/datconnlimit/], - [qr/^$/], - 'invalid database causes failure'); - - # Check that the failure creates the logging directory pg_upgrade_output.d - # and leaves it around - ok(-d $newnode->data_dir . "/pg_upgrade_output.d", - "pg_upgrade_output.d/ not removed after pg_upgrade failure"); - - rmtree($newnode->data_dir . "/pg_upgrade_output.d"); -} - -# And drop it, so we can continue -$oldnode->start; -$oldnode->safe_psql('postgres', 'DROP DATABASE regression_invalid'); -$oldnode->stop; - # --check command works here, cleans up pg_upgrade_output.d. command_ok( [ @@ -680,6 +642,11 @@ is( $result, "$original_encoding|$original_provider|$original_datcollate|$original_datctype|$original_datlocale", "check that locales in new cluster match original cluster"); +# Test that the invalid database was not upgraded. +$result = $newnode->safe_psql('postgres', + "SELECT count(*) FROM pg_database WHERE datname='regression_invalid'"); +is($result, "0", "check that the invalid database is not upgraded"); + # Second dump from the upgraded instance. @dump_command = ( 'pg_dumpall', '--no-sync', -- 2.55.0
