Re: pg_upgrade if 'postgres' database is dropped

Bruce Momjian wrote:
> > What I would prefer is to have the upgrade succeed, and just ignore
> > the existence of a postgres database in the new cluster.  Maybe give
> > the user a notice and let them decide whether they wish to take any
> > action.  I understand that failing is probably less code, but IMHO one
> > of the biggest problems with pg_upgrade is that it's too fragile:
> > there are too many seemingly innocent things that can make it croak
> > (which isn't good, when you consider that anyone using pg_upgrade is
> > probably in a hurry to get the upgrade done and the database back
> > on-line).  It seems like this is an opportunity to get rid of one of
> > those unnecessary failure cases.
>
> OK, then the simplest fix, once you modify pg_dumpall, would be to
> modify pg_upgrade to remove reference to the postgres database in the
> new cluster if it doesn't exist in the old one.  That would allow
> pg_upgrade to maintain a 1-1 matching of databases in the old and new
> cluster --- it allows the change to be locallized without affecting much
> code.

I fixed this a different way.  I originally thought I could skip over
the 'postgres' database in the new cluster if it didn't exist in the old
cluster, but we have do things like check it is empty, so that was going
to be awkward.

It turns out there was only one place that expected a 1-1 mapping of old
and new databases (file transfer), so I just modified that code to allow
skipping a database in the new cluster that didn't exist in the old
cluster.

Attached patch applied.  This allows an upgrade if the 'postgres'
database is missing from the old cluster.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c
new file mode 100644
index e400814..d32a84c
*** a/contrib/pg_upgrade/check.c
--- b/contrib/pg_upgrade/check.c
***************
*** 14,20 ****

  static void set_locale_and_encoding(ClusterInfo *cluster);
  static void check_new_cluster_is_empty(void);
- static void check_old_cluster_has_new_cluster_dbs(void);
  static void check_locale_and_encoding(ControlData *oldctrl,
                            ControlData *newctrl);
  static void check_is_super_user(ClusterInfo *cluster);
--- 14,19 ----
*************** check_new_cluster(void)
*** 127,133 ****

      check_new_cluster_is_empty();
      check_for_prepared_transactions(&new_cluster);
-     check_old_cluster_has_new_cluster_dbs();

      check_loadable_libraries();

--- 126,131 ----
*************** check_new_cluster_is_empty(void)
*** 382,420 ****


  /*
-  *    If someone removes the 'postgres' database from the old cluster and
-  *    the new cluster has a 'postgres' database, the number of databases
-  *    will not match.  We actually could upgrade such a setup, but it would
-  *    violate the 1-to-1 mapping of database counts, so we throw an error
-  *    instead.  We would detect this as a database count mismatch during
-  *    upgrade, but we want to detect it during the check phase and report
-  *    the database name.
-  */
- static void
- check_old_cluster_has_new_cluster_dbs(void)
- {
-     int            old_dbnum,
-                 new_dbnum;
-
-     for (new_dbnum = 0; new_dbnum < new_cluster.dbarr.ndbs; new_dbnum++)
-     {
-         for (old_dbnum = 0; old_dbnum < old_cluster.dbarr.ndbs; old_dbnum++)
-             if (strcmp(old_cluster.dbarr.dbs[old_dbnum].db_name,
-                        new_cluster.dbarr.dbs[new_dbnum].db_name) == 0)
-                 break;
-         if (old_dbnum == old_cluster.dbarr.ndbs)
-         {
-             if (strcmp(new_cluster.dbarr.dbs[new_dbnum].db_name, "postgres") == 0)
-                 pg_log(PG_FATAL, "The \"postgres\" database must exist in the old cluster\n");
-             else
-                 pg_log(PG_FATAL, "New cluster database \"%s\" does not exist in the old cluster\n",
-                        new_cluster.dbarr.dbs[new_dbnum].db_name);
-         }
-     }
- }
-
-
- /*
   * create_script_for_old_cluster_deletion()
   *
   *    This is particularly useful for tablespace deletion.
--- 380,385 ----
*************** create_script_for_old_cluster_deletion(c
*** 462,468 ****
                  fprintf(script, RM_CMD " %s%s/PG_VERSION\n",
                   os_info.tablespaces[tblnum], old_cluster.tablespace_suffix);

!             for (dbnum = 0; dbnum < new_cluster.dbarr.ndbs; dbnum++)
              {
                  fprintf(script, RMDIR_CMD " %s%s/%d\n",
                    os_info.tablespaces[tblnum], old_cluster.tablespace_suffix,
--- 427,433 ----
                  fprintf(script, RM_CMD " %s%s/PG_VERSION\n",
                   os_info.tablespaces[tblnum], old_cluster.tablespace_suffix);

!             for (dbnum = 0; dbnum < old_cluster.dbarr.ndbs; dbnum++)
              {
                  fprintf(script, RMDIR_CMD " %s%s/%d\n",
                    os_info.tablespaces[tblnum], old_cluster.tablespace_suffix,
diff --git a/contrib/pg_upgrade/function.c b/contrib/pg_upgrade/function.c
new file mode 100644
index 0f80089..b154f03
*** a/contrib/pg_upgrade/function.c
--- b/contrib/pg_upgrade/function.c
*************** get_loadable_libraries(void)
*** 132,139 ****
      int            totaltups;
      int            dbnum;

!     ress = (PGresult **)
!         pg_malloc(old_cluster.dbarr.ndbs * sizeof(PGresult *));
      totaltups = 0;

      /* Fetch all library names, removing duplicates within each DB */
--- 132,138 ----
      int            totaltups;
      int            dbnum;

!     ress = (PGresult **) pg_malloc(old_cluster.dbarr.ndbs * sizeof(PGresult *));
      totaltups = 0;

      /* Fetch all library names, removing duplicates within each DB */
diff --git a/contrib/pg_upgrade/relfilenode.c b/contrib/pg_upgrade/relfilenode.c
new file mode 100644
index 1aefd33..ad893be
*** a/contrib/pg_upgrade/relfilenode.c
--- b/contrib/pg_upgrade/relfilenode.c
*************** const char *
*** 34,55 ****
  transfer_all_new_dbs(DbInfoArr *old_db_arr,
                     DbInfoArr *new_db_arr, char *old_pgdata, char *new_pgdata)
  {
!     int            dbnum;
      const char *msg = NULL;

      prep_status("Restoring user relation files\n");

!     if (old_db_arr->ndbs != new_db_arr->ndbs)
!         pg_log(PG_FATAL, "old and new clusters have a different number of databases\n");
!
!     for (dbnum = 0; dbnum < old_db_arr->ndbs; dbnum++)
      {
!         DbInfo       *old_db = &old_db_arr->dbs[dbnum];
!         DbInfo       *new_db = &new_db_arr->dbs[dbnum];
          FileNameMap *mappings;
          int            n_maps;
          pageCnvCtx *pageConverter = NULL;

          if (strcmp(old_db->db_name, new_db->db_name) != 0)
              pg_log(PG_FATAL, "old and new databases have different names: old \"%s\", new \"%s\"\n",
                     old_db->db_name, new_db->db_name);
--- 34,63 ----
  transfer_all_new_dbs(DbInfoArr *old_db_arr,
                     DbInfoArr *new_db_arr, char *old_pgdata, char *new_pgdata)
  {
!     int            old_dbnum, new_dbnum;
      const char *msg = NULL;

      prep_status("Restoring user relation files\n");

!     /* Scan the old cluster databases and transfer their files */
!     for (old_dbnum = new_dbnum = 0;
!          old_dbnum < old_db_arr->ndbs && new_dbnum < new_db_arr->ndbs;
!          old_dbnum++, new_dbnum++)
      {
!         DbInfo       *old_db = &old_db_arr->dbs[old_dbnum];
!         DbInfo       *new_db = &new_db_arr->dbs[new_dbnum];
          FileNameMap *mappings;
          int            n_maps;
          pageCnvCtx *pageConverter = NULL;

+         /*
+          *    Advance past any databases that exist in the new cluster
+          *    but not in the old, e.g. "postgres".
+          */
+         while (strcmp(old_db->db_name, new_db->db_name) != 0 &&
+                new_dbnum < new_db_arr->ndbs)
+             new_db = &new_db_arr->dbs[++new_dbnum];
+
          if (strcmp(old_db->db_name, new_db->db_name) != 0)
              pg_log(PG_FATAL, "old and new databases have different names: old \"%s\", new \"%s\"\n",
                     old_db->db_name, new_db->db_name);