Thread: pg_upgrade and toasted pg_largeobject

pg_upgrade and toasted pg_largeobject

From
Alvaro Herrera
Date:
Hi,

A customer of ours was unable to pg_upgrade a database, with this error:
 old and new databases "postgres" have a mismatched number of relations Failure, exiting

After some research, it turned out that pg_largeobject had acquired a
toast table.  After some more research, we determined that it was
because right after initdb of the old database (months or years prior)
they moved pg_largeobject to another, slower tablespace, because for
their case it is very bulky and not used as much as the other data.
(This requires restarting postmaster with the -O parameter).

While I understand that manual system catalog modifications are frowned
upon, it seems to me that we should handle this better.  The failure is
very confusing and hard to diagnose, and hard to fix also.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: pg_upgrade and toasted pg_largeobject

From
Robert Haas
Date:
On Mon, May 2, 2016 at 12:30 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> A customer of ours was unable to pg_upgrade a database, with this error:
>
>   old and new databases "postgres" have a mismatched number of relations
>   Failure, exiting
>
> After some research, it turned out that pg_largeobject had acquired a
> toast table.  After some more research, we determined that it was
> because right after initdb of the old database (months or years prior)
> they moved pg_largeobject to another, slower tablespace, because for
> their case it is very bulky and not used as much as the other data.
> (This requires restarting postmaster with the -O parameter).
>
> While I understand that manual system catalog modifications are frowned
> upon, it seems to me that we should handle this better.  The failure is
> very confusing and hard to diagnose, and hard to fix also.

I think that if you use -O, and it breaks something, you get to keep
both pieces.  pg_largeobject is a big problem, and we should replace
it with something better.  And maybe in the meantime we should support
moving it to a different tablespace.  But if it's not officially
supported and you do it anyway, I don't think it's pg_upgrade's job to
cope.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: pg_upgrade and toasted pg_largeobject

From
Alvaro Herrera
Date:
Robert Haas wrote:
> On Mon, May 2, 2016 at 12:30 PM, Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:
> > A customer of ours was unable to pg_upgrade a database, with this error:
> >
> >   old and new databases "postgres" have a mismatched number of relations
> >   Failure, exiting
> >
> > After some research, it turned out that pg_largeobject had acquired a
> > toast table.  After some more research, we determined that it was
> > because right after initdb of the old database (months or years prior)
> > they moved pg_largeobject to another, slower tablespace, because for
> > their case it is very bulky and not used as much as the other data.
> > (This requires restarting postmaster with the -O parameter).

> I think that if you use -O, and it breaks something, you get to keep
> both pieces.  pg_largeobject is a big problem, and we should replace
> it with something better.  And maybe in the meantime we should support
> moving it to a different tablespace.  But if it's not officially
> supported and you do it anyway, I don't think it's pg_upgrade's job to
> cope.

I'm happy with the solution that pg_upgrade has a step in the check
stage that says "catalog XYZ has a toast table but shouldn't, aborting
the upgrade".  (Well, not _happy_, but at least it's a lot easier to
diagnose).

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: pg_upgrade and toasted pg_largeobject

From
Tom Lane
Date:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Robert Haas wrote:
>> On Mon, May 2, 2016 at 12:30 PM, Alvaro Herrera
>> <alvherre@2ndquadrant.com> wrote:
>>> A customer of ours was unable to pg_upgrade a database, with this error:
>>>> old and new databases "postgres" have a mismatched number of relations
>>> After some research, it turned out that pg_largeobject had acquired a
>>> toast table.

>> I think that if you use -O, and it breaks something, you get to keep
>> both pieces.

> I'm happy with the solution that pg_upgrade has a step in the check
> stage that says "catalog XYZ has a toast table but shouldn't, aborting
> the upgrade".  (Well, not _happy_, but at least it's a lot easier to
> diagnose).

I agree with Robert that this is not something we should expend a huge
amount of effort on, but I also agree that that error message is
inadequate if the situation is not a nobody-could-ever-hit-this case.

I think though that you're defining the problem too narrowly by putting
forward a solution that would result in an error message like that.
If we're going to do anything here at all, I think the code should emit
a list of the names of relations that it was unable to match up, rather
than trying (and likely failing) to be smart about why.  To take just
one reason why, the issue might be that there were too many rels in
the new installation, not too few.

The matching probably does need to be smart about toast tables to the
extent of regarding them as "toast table belonging to relation FOO",
since just printing their actual names would be unhelpful in most cases.
        regards, tom lane



Re: pg_upgrade and toasted pg_largeobject

From
Tom Lane
Date:
I wrote:
> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
>> I'm happy with the solution that pg_upgrade has a step in the check
>> stage that says "catalog XYZ has a toast table but shouldn't, aborting
>> the upgrade".  (Well, not _happy_, but at least it's a lot easier to
>> diagnose).

> I think though that you're defining the problem too narrowly by putting
> forward a solution that would result in an error message like that.
> If we're going to do anything here at all, I think the code should emit
> a list of the names of relations that it was unable to match up, rather
> than trying (and likely failing) to be smart about why.  To take just
> one reason why, the issue might be that there were too many rels in
> the new installation, not too few.

I took a break from release-note writing and hacked up something for
this; see attached patch series.

pg_upgrade-fix-0001.patch attempts to turn get_rel_infos()'s data
collection query into less of an unmaintainable mess.  In the current
code, the "regular_heap" CTE doesn't fetch heap OIDs: it collects some
index OIDs as well.  The "all_indexes" CTE doesn't fetch all index OIDs:
it only fetches OIDs for toast-table indexes (although this is far from
obvious to the naked eye, since it reads both the regular_heap and
toast_heap CTEs; it's only when you notice that it's subsequently ignoring
tables with zero reltoastrelid, and therefore that having read the
toast_heap CTE was totally useless, that you realize this).  Also it
forgets to check indisready, so it's fortunate that it doesn't fetch
anything but toast-table indexes, which are unlikely to be in !indisready
state.  Only the toast_heap CTE actually does what it says, though rather
inefficiently since it's uselessly looking for toast tables attached to
indexes.  The comments that aren't outright wrong are variously
misleading, repetitive, badly placed, and/or ungrammatical.  I'd like to
commit this even if we don't use the rest, because what's there now is not
a fit basis for further development.

pg_upgrade-fix-0002.patch expands the data collection query to collect
the OID of the table associated with each index, and the OID of the base
table for each toast table.  This isn't needed for pg_upgrade's other
processing but we need it in order to usefully identify mismatched tables.

pg_upgrade-fix-0003.patch actually changes gen_db_file_maps() so that
it prints identifying information about each unmatched relation.

pg_upgrade-fix-0004.patch isn't meant to get committed; it's for testing
the previous patches.  It hacks the pg_upgrade test script to cause
pg_largeobject in the source DB to acquire a toast table, thereby
replicating the situation Alvaro complained of.  With that hack in place,
I get

...
Copying user relation files
No match found in new cluster for old relation with OID 13270 in database "postgres": "pg_toast.pg_toast_2613", toast
tablefor "pg_catalog.pg_largeobject" 
No match found in new cluster for old relation with OID 13272 in database "postgres": "pg_toast.pg_toast_2613_index",
indexon "pg_toast.pg_toast_2613", toast table for "pg_catalog.pg_largeobject" 

Failed to match up old and new tables in database "postgres"
Failure, exiting
+ rm -rf /tmp/pg_upgrade_check-Lui8zH
make: *** [check] Error 1

which illustrates the output this will produce for a mismatch.  (If anyone
wants to bikeshed the output wording, feel free.)

Any thoughts what to do with this?  We could decide that it's a bug fix
and backpatch, or decide that it's a new feature and delay till 9.7,
or decide that it's a minor bug fix and add it to 9.6 only.  I kinda lean
towards the last alternative.

            regards, tom lane

diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
index 5d83455..0653ff1 100644
*** a/src/bin/pg_upgrade/info.c
--- b/src/bin/pg_upgrade/info.c
*************** get_db_infos(ClusterInfo *cluster)
*** 301,311 ****
  /*
   * get_rel_infos()
   *
!  * gets the relinfos for all the user tables of the database referred
!  * by "db".
   *
!  * NOTE: we assume that relations/entities with oids greater than
!  * FirstNormalObjectId belongs to the user
   */
  static void
  get_rel_infos(ClusterInfo *cluster, DbInfo *dbinfo)
--- 301,311 ----
  /*
   * get_rel_infos()
   *
!  * gets the relinfos for all the user tables and indexes of the database
!  * referred to by "dbinfo".
   *
!  * Note: the resulting RelInfo array is assumed to be sorted by OID.
!  * This allows later processing to match up old and new databases efficiently.
   */
  static void
  get_rel_infos(ClusterInfo *cluster, DbInfo *dbinfo)
*************** get_rel_infos(ClusterInfo *cluster, DbIn
*** 330,415 ****
      char       *last_namespace = NULL,
                 *last_tablespace = NULL;

      /*
       * pg_largeobject contains user data that does not appear in pg_dump
!      * --schema-only output, so we have to copy that system table heap and
!      * index.  We could grab the pg_largeobject oids from template1, but it is
!      * easy to treat it as a normal table. Order by oid so we can join old/new
!      * structures efficiently.
       */
!
!     snprintf(query, sizeof(query),
!     /* get regular heap */
               "WITH regular_heap (reloid) AS ( "
!              "    SELECT c.oid "
!              "    FROM pg_catalog.pg_class c JOIN pg_catalog.pg_namespace n "
!              "           ON c.relnamespace = n.oid "
!              "    LEFT OUTER JOIN pg_catalog.pg_index i "
!              "           ON c.oid = i.indexrelid "
!              "    WHERE relkind IN ('r', 'm', 'i', 'S') AND "

      /*
!      * pg_dump only dumps valid indexes;  testing indisready is necessary in
!      * 9.2, and harmless in earlier/later versions.
       */
!              "        i.indisvalid IS DISTINCT FROM false AND "
!              "        i.indisready IS DISTINCT FROM false AND "
!     /* exclude possible orphaned temp tables */
!              "      ((n.nspname !~ '^pg_temp_' AND "
!              "        n.nspname !~ '^pg_toast_temp_' AND "
!     /* skip pg_toast because toast index have relkind == 'i', not 't' */
!              "        n.nspname NOT IN ('pg_catalog', 'information_schema', "
!              "                            'binary_upgrade', 'pg_toast') AND "
!              "          c.oid >= %u) OR "
!              "      (n.nspname = 'pg_catalog' AND "
!              "    relname IN ('pg_largeobject', 'pg_largeobject_loid_pn_index'%s) ))), "

      /*
!      * We have to gather the TOAST tables in later steps because we can't
!      * schema-qualify TOAST tables.
       */
!     /* get TOAST heap */
!              "    toast_heap (reloid) AS ( "
!              "    SELECT reltoastrelid "
!              "    FROM regular_heap JOIN pg_catalog.pg_class c "
!              "        ON regular_heap.reloid = c.oid "
!              "        AND c.reltoastrelid != %u), "
!     /* get indexes on regular and TOAST heap */
!              "    all_index (reloid) AS ( "
!              "    SELECT indexrelid "
!              "    FROM pg_index "
!              "    WHERE indisvalid "
!              "    AND indrelid IN (SELECT reltoastrelid "
!              "        FROM (SELECT reloid FROM regular_heap "
!              "               UNION ALL "
!              "               SELECT reloid FROM toast_heap) all_heap "
!              "            JOIN pg_catalog.pg_class c "
!              "            ON all_heap.reloid = c.oid "
!              "            AND c.reltoastrelid != %u)) "
!     /* get all rels */
               "SELECT c.oid, n.nspname, c.relname, "
!              "    c.relfilenode, c.reltablespace, %s "
               "FROM (SELECT reloid FROM regular_heap "
!              "       UNION ALL "
!              "       SELECT reloid FROM toast_heap  "
!              "       UNION ALL "
!              "       SELECT reloid FROM all_index) all_rels "
               "  JOIN pg_catalog.pg_class c "
!              "        ON all_rels.reloid = c.oid "
               "  JOIN pg_catalog.pg_namespace n "
!              "       ON c.relnamespace = n.oid "
               "  LEFT OUTER JOIN pg_catalog.pg_tablespace t "
!              "       ON c.reltablespace = t.oid "
!     /* we preserve pg_class.oid so we sort by it to match old/new */
               "ORDER BY 1;",
!              FirstNormalObjectId,
!     /* does pg_largeobject_metadata need to be migrated? */
!              (GET_MAJOR_VERSION(old_cluster.major_version) <= 804) ?
!      "" : ", 'pg_largeobject_metadata', 'pg_largeobject_metadata_oid_index'",
!              InvalidOid, InvalidOid,
!     /* 9.2 removed the spclocation column */
!              (GET_MAJOR_VERSION(cluster->major_version) <= 901) ?
!              "t.spclocation" : "pg_catalog.pg_tablespace_location(t.oid) AS spclocation");

      res = executeQueryOrDie(conn, "%s", query);

--- 330,418 ----
      char       *last_namespace = NULL,
                 *last_tablespace = NULL;

+     query[0] = '\0';            /* initialize query string to empty */
+
      /*
+      * Create a CTE that collects OIDs of regular user tables, including
+      * matviews and sequences, but excluding toast tables and indexes.  We
+      * assume that relations with OIDs >= FirstNormalObjectId belong to the
+      * user.  (That's probably redundant with the namespace-name exclusions,
+      * but let's be safe.)
+      *
       * pg_largeobject contains user data that does not appear in pg_dump
!      * output, so we have to copy that system table.  It's easiest to do that
!      * by treating it as a user table.  Likewise for pg_largeobject_metadata,
!      * if it exists.
       */
!     snprintf(query + strlen(query), sizeof(query) - strlen(query),
               "WITH regular_heap (reloid) AS ( "
!              "  SELECT c.oid "
!              "  FROM pg_catalog.pg_class c JOIN pg_catalog.pg_namespace n "
!              "         ON c.relnamespace = n.oid "
!              "  WHERE relkind IN ('r', 'm', 'S') AND "
!     /* exclude possible orphaned temp tables */
!              "    ((n.nspname !~ '^pg_temp_' AND "
!              "      n.nspname !~ '^pg_toast_temp_' AND "
!              "      n.nspname NOT IN ('pg_catalog', 'information_schema', "
!              "                        'binary_upgrade', 'pg_toast') AND "
!              "      c.oid >= %u::pg_catalog.oid) OR "
!              "     (n.nspname = 'pg_catalog' AND "
!              "      relname IN ('pg_largeobject'%s) ))), ",
!              FirstNormalObjectId,
!              (GET_MAJOR_VERSION(old_cluster.major_version) >= 900) ?
!              ", 'pg_largeobject_metadata'" : "");

      /*
!      * Add a CTE that collects OIDs of toast tables belonging to the tables
!      * selected by the regular_heap CTE.  (We have to do this separately
!      * because the namespace-name rules above don't work for toast tables.)
       */
!     snprintf(query + strlen(query), sizeof(query) - strlen(query),
!              "  toast_heap (reloid) AS ( "
!              "  SELECT c.reltoastrelid "
!              "  FROM regular_heap JOIN pg_catalog.pg_class c "
!              "      ON regular_heap.reloid = c.oid "
!              "  WHERE c.reltoastrelid != 0), ");

      /*
!      * Add a CTE that collects OIDs of all valid indexes on the previously
!      * selected tables.  We can ignore invalid indexes since pg_dump does.
!      * Testing indisready is necessary in 9.2, and harmless in earlier/later
!      * versions.
       */
!     snprintf(query + strlen(query), sizeof(query) - strlen(query),
!              "  all_index (reloid) AS ( "
!              "  SELECT indexrelid "
!              "  FROM pg_catalog.pg_index "
!              "  WHERE indisvalid AND indisready "
!              "    AND indrelid IN "
!              "        (SELECT reloid FROM regular_heap "
!              "         UNION ALL "
!              "         SELECT reloid FROM toast_heap)) ");
!
!     /*
!      * And now we can write the query that retrieves the data we want for each
!      * heap and index relation.  Make sure result is sorted by OID.
!      */
!     snprintf(query + strlen(query), sizeof(query) - strlen(query),
               "SELECT c.oid, n.nspname, c.relname, "
!              "  c.relfilenode, c.reltablespace, %s "
               "FROM (SELECT reloid FROM regular_heap "
!              "      UNION ALL "
!              "      SELECT reloid FROM toast_heap "
!              "      UNION ALL "
!              "      SELECT reloid FROM all_index) all_rels "
               "  JOIN pg_catalog.pg_class c "
!              "      ON all_rels.reloid = c.oid "
               "  JOIN pg_catalog.pg_namespace n "
!              "     ON c.relnamespace = n.oid "
               "  LEFT OUTER JOIN pg_catalog.pg_tablespace t "
!              "     ON c.reltablespace = t.oid "
               "ORDER BY 1;",
!     /* 9.2 removed the pg_tablespace.spclocation column */
!              (GET_MAJOR_VERSION(cluster->major_version) >= 902) ?
!              "pg_catalog.pg_tablespace_location(t.oid) AS spclocation" :
!              "t.spclocation");

      res = executeQueryOrDie(conn, "%s", query);

*************** get_rel_infos(ClusterInfo *cluster, DbIn
*** 453,459 ****
          curr->relfilenode = atooid(PQgetvalue(res, relnum, i_relfilenode));
          curr->tblsp_alloc = false;

!         /* Is the tablespace oid non-zero? */
          if (atooid(PQgetvalue(res, relnum, i_reltablespace)) != 0)
          {
              /*
--- 456,462 ----
          curr->relfilenode = atooid(PQgetvalue(res, relnum, i_relfilenode));
          curr->tblsp_alloc = false;

!         /* Is the tablespace oid non-default? */
          if (atooid(PQgetvalue(res, relnum, i_reltablespace)) != 0)
          {
              /*
diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
index 0653ff1..85af773 100644
*** a/src/bin/pg_upgrade/info.c
--- b/src/bin/pg_upgrade/info.c
*************** get_rel_infos(ClusterInfo *cluster, DbIn
*** 323,329 ****
      int            i_spclocation,
                  i_nspname,
                  i_relname,
!                 i_oid,
                  i_relfilenode,
                  i_reltablespace;
      char        query[QUERY_ALLOC];
--- 323,331 ----
      int            i_spclocation,
                  i_nspname,
                  i_relname,
!                 i_reloid,
!                 i_indtable,
!                 i_toastheap,
                  i_relfilenode,
                  i_reltablespace;
      char        query[QUERY_ALLOC];
*************** get_rel_infos(ClusterInfo *cluster, DbIn
*** 345,352 ****
       * if it exists.
       */
      snprintf(query + strlen(query), sizeof(query) - strlen(query),
!              "WITH regular_heap (reloid) AS ( "
!              "  SELECT c.oid "
               "  FROM pg_catalog.pg_class c JOIN pg_catalog.pg_namespace n "
               "         ON c.relnamespace = n.oid "
               "  WHERE relkind IN ('r', 'm', 'S') AND "
--- 347,354 ----
       * if it exists.
       */
      snprintf(query + strlen(query), sizeof(query) - strlen(query),
!              "WITH regular_heap (reloid, indtable, toastheap) AS ( "
!              "  SELECT c.oid, 0::oid, 0::oid "
               "  FROM pg_catalog.pg_class c JOIN pg_catalog.pg_namespace n "
               "         ON c.relnamespace = n.oid "
               "  WHERE relkind IN ('r', 'm', 'S') AND "
*************** get_rel_infos(ClusterInfo *cluster, DbIn
*** 368,375 ****
       * because the namespace-name rules above don't work for toast tables.)
       */
      snprintf(query + strlen(query), sizeof(query) - strlen(query),
!              "  toast_heap (reloid) AS ( "
!              "  SELECT c.reltoastrelid "
               "  FROM regular_heap JOIN pg_catalog.pg_class c "
               "      ON regular_heap.reloid = c.oid "
               "  WHERE c.reltoastrelid != 0), ");
--- 370,377 ----
       * because the namespace-name rules above don't work for toast tables.)
       */
      snprintf(query + strlen(query), sizeof(query) - strlen(query),
!              "  toast_heap (reloid, indtable, toastheap) AS ( "
!              "  SELECT c.reltoastrelid, 0::oid, c.oid "
               "  FROM regular_heap JOIN pg_catalog.pg_class c "
               "      ON regular_heap.reloid = c.oid "
               "  WHERE c.reltoastrelid != 0), ");
*************** get_rel_infos(ClusterInfo *cluster, DbIn
*** 381,388 ****
       * versions.
       */
      snprintf(query + strlen(query), sizeof(query) - strlen(query),
!              "  all_index (reloid) AS ( "
!              "  SELECT indexrelid "
               "  FROM pg_catalog.pg_index "
               "  WHERE indisvalid AND indisready "
               "    AND indrelid IN "
--- 383,390 ----
       * versions.
       */
      snprintf(query + strlen(query), sizeof(query) - strlen(query),
!              "  all_index (reloid, indtable, toastheap) AS ( "
!              "  SELECT indexrelid, indrelid, 0::oid "
               "  FROM pg_catalog.pg_index "
               "  WHERE indisvalid AND indisready "
               "    AND indrelid IN "
*************** get_rel_infos(ClusterInfo *cluster, DbIn
*** 395,407 ****
       * heap and index relation.  Make sure result is sorted by OID.
       */
      snprintf(query + strlen(query), sizeof(query) - strlen(query),
!              "SELECT c.oid, n.nspname, c.relname, "
               "  c.relfilenode, c.reltablespace, %s "
!              "FROM (SELECT reloid FROM regular_heap "
               "      UNION ALL "
!              "      SELECT reloid FROM toast_heap "
               "      UNION ALL "
!              "      SELECT reloid FROM all_index) all_rels "
               "  JOIN pg_catalog.pg_class c "
               "      ON all_rels.reloid = c.oid "
               "  JOIN pg_catalog.pg_namespace n "
--- 397,409 ----
       * heap and index relation.  Make sure result is sorted by OID.
       */
      snprintf(query + strlen(query), sizeof(query) - strlen(query),
!              "SELECT all_rels.*, n.nspname, c.relname, "
               "  c.relfilenode, c.reltablespace, %s "
!              "FROM (SELECT * FROM regular_heap "
               "      UNION ALL "
!              "      SELECT * FROM toast_heap "
               "      UNION ALL "
!              "      SELECT * FROM all_index) all_rels "
               "  JOIN pg_catalog.pg_class c "
               "      ON all_rels.reloid = c.oid "
               "  JOIN pg_catalog.pg_namespace n "
*************** get_rel_infos(ClusterInfo *cluster, DbIn
*** 420,426 ****

      relinfos = (RelInfo *) pg_malloc(sizeof(RelInfo) * ntups);

!     i_oid = PQfnumber(res, "oid");
      i_nspname = PQfnumber(res, "nspname");
      i_relname = PQfnumber(res, "relname");
      i_relfilenode = PQfnumber(res, "relfilenode");
--- 422,430 ----

      relinfos = (RelInfo *) pg_malloc(sizeof(RelInfo) * ntups);

!     i_reloid = PQfnumber(res, "reloid");
!     i_indtable = PQfnumber(res, "indtable");
!     i_toastheap = PQfnumber(res, "toastheap");
      i_nspname = PQfnumber(res, "nspname");
      i_relname = PQfnumber(res, "relname");
      i_relfilenode = PQfnumber(res, "relfilenode");
*************** get_rel_infos(ClusterInfo *cluster, DbIn
*** 431,437 ****
      {
          RelInfo    *curr = &relinfos[num_rels++];

!         curr->reloid = atooid(PQgetvalue(res, relnum, i_oid));

          nspname = PQgetvalue(res, relnum, i_nspname);
          curr->nsp_alloc = false;
--- 435,443 ----
      {
          RelInfo    *curr = &relinfos[num_rels++];

!         curr->reloid = atooid(PQgetvalue(res, relnum, i_reloid));
!         curr->indtable = atooid(PQgetvalue(res, relnum, i_indtable));
!         curr->toastheap = atooid(PQgetvalue(res, relnum, i_toastheap));

          nspname = PQgetvalue(res, relnum, i_nspname);
          curr->nsp_alloc = false;
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 89beb73..389d463 100644
*** a/src/bin/pg_upgrade/pg_upgrade.h
--- b/src/bin/pg_upgrade/pg_upgrade.h
*************** extern char *output_files[];
*** 137,151 ****
   */
  typedef struct
  {
!     /* Can't use NAMEDATALEN;  not guaranteed to fit on client */
      char       *nspname;        /* namespace name */
      char       *relname;        /* relation name */
!     Oid            reloid;            /* relation oid */
      Oid            relfilenode;    /* relation relfile node */
!     /* relation tablespace path, or "" for the cluster default */
!     char       *tablespace;
!     bool        nsp_alloc;
!     bool        tblsp_alloc;
  } RelInfo;

  typedef struct
--- 137,152 ----
   */
  typedef struct
  {
!     /* Can't use NAMEDATALEN; not guaranteed to be same on client */
      char       *nspname;        /* namespace name */
      char       *relname;        /* relation name */
!     Oid            reloid;            /* relation OID */
      Oid            relfilenode;    /* relation relfile node */
!     Oid            indtable;        /* if index, OID of its table, else 0 */
!     Oid            toastheap;        /* if toast table, OID of base table, else 0 */
!     char       *tablespace;        /* tablespace path; "" for cluster default */
!     bool        nsp_alloc;        /* should nspname be freed? */
!     bool        tblsp_alloc;    /* should tablespace be freed? */
  } RelInfo;

  typedef struct
diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
index 85af773..731eb43 100644
*** a/src/bin/pg_upgrade/info.c
--- b/src/bin/pg_upgrade/info.c
*************** static void create_rel_filename_map(cons
*** 18,23 ****
--- 18,25 ----
                          const DbInfo *old_db, const DbInfo *new_db,
                          const RelInfo *old_rel, const RelInfo *new_rel,
                          FileNameMap *map);
+ static void report_unmatched_relation(const RelInfo *rel, const DbInfo *db,
+                           bool is_new_db);
  static void free_db_and_rel_infos(DbInfoArr *db_arr);
  static void get_db_infos(ClusterInfo *cluster);
  static void get_rel_infos(ClusterInfo *cluster, DbInfo *dbinfo);
*************** static void print_rel_infos(RelInfoArr *
*** 29,127 ****
  /*
   * gen_db_file_maps()
   *
!  * generates database mappings for "old_db" and "new_db". Returns a malloc'ed
!  * array of mappings. nmaps is a return parameter which refers to the number
!  * mappings.
   */
  FileNameMap *
  gen_db_file_maps(DbInfo *old_db, DbInfo *new_db,
!                  int *nmaps, const char *old_pgdata, const char *new_pgdata)
  {
      FileNameMap *maps;
      int            old_relnum,
                  new_relnum;
      int            num_maps = 0;

      maps = (FileNameMap *) pg_malloc(sizeof(FileNameMap) *
                                       old_db->rel_arr.nrels);

      /*
!      * The old database shouldn't have more relations than the new one. We
!      * force the new cluster to have a TOAST table if the old table had one.
       */
!     if (old_db->rel_arr.nrels > new_db->rel_arr.nrels)
!         pg_fatal("old and new databases \"%s\" have a mismatched number of relations\n",
!                  old_db->db_name);
!
!     /* Drive the loop using new_relnum, which might be higher. */
!     for (old_relnum = new_relnum = 0; new_relnum < new_db->rel_arr.nrels;
!          new_relnum++)
      {
!         RelInfo    *old_rel;
!         RelInfo    *new_rel = &new_db->rel_arr.rels[new_relnum];

!         /*
!          * It is possible that the new cluster has a TOAST table for a table
!          * that didn't need one in the old cluster, e.g. 9.0 to 9.1 changed
!          * the NUMERIC length computation.  Therefore, if we have a TOAST
!          * table in the new cluster that doesn't match, skip over it and
!          * continue processing.  It is possible this TOAST table used an OID
!          * that was reserved in the old cluster, but we have no way of testing
!          * that, and we would have already gotten an error at the new cluster
!          * schema creation stage.  Fortunately, since we only restore the OID
!          * counter after schema restore, and restore in OID order via pg_dump,
!          * a conflict would only happen if the new TOAST table had a very low
!          * OID.  However, TOAST tables created long after initial table
!          * creation can have any OID, particularly after OID wraparound.
!          */
!         if (old_relnum == old_db->rel_arr.nrels)
          {
!             if (strcmp(new_rel->nspname, "pg_toast") == 0)
!                 continue;
!             else
!                 pg_fatal("Extra non-TOAST relation found in database \"%s\": new OID %d\n",
!                          old_db->db_name, new_rel->reloid);
          }

!         old_rel = &old_db->rel_arr.rels[old_relnum];
!
!         if (old_rel->reloid != new_rel->reloid)
          {
!             if (strcmp(new_rel->nspname, "pg_toast") == 0)
!                 continue;
!             else
!                 pg_fatal("Mismatch of relation OID in database \"%s\": old OID %d, new OID %d\n",
!                          old_db->db_name, old_rel->reloid, new_rel->reloid);
          }

          /*
!          * TOAST table names initially match the heap pg_class oid. In
!          * pre-8.4, TOAST table names change during CLUSTER; in pre-9.0, TOAST
!          * table names change during ALTER TABLE ALTER COLUMN SET TYPE. In >=
!          * 9.0, TOAST relation names always use heap table oids, hence we
!          * cannot check relation names when upgrading from pre-9.0. Clusters
!          * upgraded to 9.0 will get matching TOAST names. If index names don't
!          * match primary key constraint names, this will fail because pg_dump
!          * dumps constraint names and pg_upgrade checks index names.
           */
          if (strcmp(old_rel->nspname, new_rel->nspname) != 0 ||
!             ((GET_MAJOR_VERSION(old_cluster.major_version) >= 900 ||
!               strcmp(old_rel->nspname, "pg_toast") != 0) &&
!              strcmp(old_rel->relname, new_rel->relname) != 0))
!             pg_fatal("Mismatch of relation names in database \"%s\": "
!                      "old name \"%s.%s\", new name \"%s.%s\"\n",
!                      old_db->db_name, old_rel->nspname, old_rel->relname,
!                      new_rel->nspname, new_rel->relname);

          create_rel_filename_map(old_pgdata, new_pgdata, old_db, new_db,
                                  old_rel, new_rel, maps + num_maps);
          num_maps++;
          old_relnum++;
      }

!     /* Did we fail to exhaust the old array? */
!     if (old_relnum != old_db->rel_arr.nrels)
!         pg_fatal("old and new databases \"%s\" have a mismatched number of relations\n",
                   old_db->db_name);

      *nmaps = num_maps;
--- 31,155 ----
  /*
   * gen_db_file_maps()
   *
!  * generates a database mapping from "old_db" to "new_db".
!  *
!  * Returns a malloc'ed array of mappings.  The length of the array
!  * is returned into *nmaps.
   */
  FileNameMap *
  gen_db_file_maps(DbInfo *old_db, DbInfo *new_db,
!                  int *nmaps,
!                  const char *old_pgdata, const char *new_pgdata)
  {
      FileNameMap *maps;
      int            old_relnum,
                  new_relnum;
      int            num_maps = 0;
+     bool        all_matched = true;

+     /* There will certainly not be more mappings than there are old rels */
      maps = (FileNameMap *) pg_malloc(sizeof(FileNameMap) *
                                       old_db->rel_arr.nrels);

      /*
!      * Each of the RelInfo arrays should be sorted by OID.  Scan through them
!      * and match them up.  If we fail to match everything, we'll abort, but
!      * first print as much info as we can about mismatches.
       */
!     old_relnum = new_relnum = 0;
!     while (old_relnum < old_db->rel_arr.nrels ||
!            new_relnum < new_db->rel_arr.nrels)
      {
!         RelInfo    *old_rel = (old_relnum < old_db->rel_arr.nrels) ?
!         &old_db->rel_arr.rels[old_relnum] : NULL;
!         RelInfo    *new_rel = (new_relnum < new_db->rel_arr.nrels) ?
!         &new_db->rel_arr.rels[new_relnum] : NULL;

!         /* handle running off one array before the other */
!         if (!new_rel)
          {
!             /*
!              * old_rel is unmatched.  This should never happen, because we
!              * force new rels to have TOAST tables if the old one did.
!              */
!             report_unmatched_relation(old_rel, old_db, false);
!             all_matched = false;
!             old_relnum++;
!             continue;
!         }
!         if (!old_rel)
!         {
!             /*
!              * new_rel is unmatched.  This shouldn't really happen either, but
!              * if it's a TOAST table, we can ignore it and continue
!              * processing, assuming that the new server made a TOAST table
!              * that wasn't needed.
!              */
!             if (strcmp(new_rel->nspname, "pg_toast") != 0)
!             {
!                 report_unmatched_relation(new_rel, new_db, true);
!                 all_matched = false;
!             }
!             new_relnum++;
!             continue;
          }

!         /* check for mismatched OID */
!         if (old_rel->reloid < new_rel->reloid)
          {
!             /* old_rel is unmatched, see comment above */
!             report_unmatched_relation(old_rel, old_db, false);
!             all_matched = false;
!             old_relnum++;
!             continue;
!         }
!         else if (old_rel->reloid > new_rel->reloid)
!         {
!             /* new_rel is unmatched, see comment above */
!             if (strcmp(new_rel->nspname, "pg_toast") != 0)
!             {
!                 report_unmatched_relation(new_rel, new_db, true);
!                 all_matched = false;
!             }
!             new_relnum++;
!             continue;
          }

          /*
!          * Verify that rels of same OID have same name.  The namespace name
!          * should always match, but the relname might not match for TOAST
!          * tables (and, therefore, their indexes).
!          *
!          * TOAST table names initially match the heap pg_class oid, but
!          * pre-9.0 they can change during certain commands such as CLUSTER, so
!          * don't insist on a match if old cluster is < 9.0.
           */
          if (strcmp(old_rel->nspname, new_rel->nspname) != 0 ||
!             (strcmp(old_rel->relname, new_rel->relname) != 0 &&
!              (GET_MAJOR_VERSION(old_cluster.major_version) >= 900 ||
!               strcmp(old_rel->nspname, "pg_toast") != 0)))
!         {
!             pg_log(PG_WARNING, "Relation names for OID %u in database \"%s\" do not match: "
!                    "old name \"%s.%s\", new name \"%s.%s\"\n",
!                    old_rel->reloid, old_db->db_name,
!                    old_rel->nspname, old_rel->relname,
!                    new_rel->nspname, new_rel->relname);
!             all_matched = false;
!             old_relnum++;
!             new_relnum++;
!             continue;
!         }

+         /* OK, create a mapping entry */
          create_rel_filename_map(old_pgdata, new_pgdata, old_db, new_db,
                                  old_rel, new_rel, maps + num_maps);
          num_maps++;
          old_relnum++;
+         new_relnum++;
      }

!     if (!all_matched)
!         pg_fatal("Failed to match up old and new tables in database \"%s\"\n",
                   old_db->db_name);

      *nmaps = num_maps;
*************** create_rel_filename_map(const char *old_
*** 187,192 ****
--- 215,285 ----
  }


+ /*
+  * Complain about a relation we couldn't match to the other database,
+  * identifying it as best we can.
+  */
+ static void
+ report_unmatched_relation(const RelInfo *rel, const DbInfo *db, bool is_new_db)
+ {
+     Oid            reloid = rel->reloid;    /* we might change rel below */
+     char        reldesc[1000];
+     int            i;
+
+     snprintf(reldesc, sizeof(reldesc), "\"%s.%s\"",
+              rel->nspname, rel->relname);
+     if (rel->indtable)
+     {
+         for (i = 0; i < db->rel_arr.nrels; i++)
+         {
+             const RelInfo *hrel = &db->rel_arr.rels[i];
+
+             if (hrel->reloid == rel->indtable)
+             {
+                 snprintf(reldesc + strlen(reldesc),
+                          sizeof(reldesc) - strlen(reldesc),
+                          ", index on \"%s.%s\"",
+                          hrel->nspname, hrel->relname);
+                 /* Shift attention to index's table for toast check */
+                 rel = hrel;
+                 break;
+             }
+         }
+         if (i >= db->rel_arr.nrels)
+             snprintf(reldesc + strlen(reldesc),
+                      sizeof(reldesc) - strlen(reldesc),
+                      ", index on OID %u", rel->indtable);
+     }
+     if (rel->toastheap)
+     {
+         for (i = 0; i < db->rel_arr.nrels; i++)
+         {
+             const RelInfo *brel = &db->rel_arr.rels[i];
+
+             if (brel->reloid == rel->toastheap)
+             {
+                 snprintf(reldesc + strlen(reldesc),
+                          sizeof(reldesc) - strlen(reldesc),
+                          ", toast table for \"%s.%s\"",
+                          brel->nspname, brel->relname);
+                 break;
+             }
+         }
+         if (i >= db->rel_arr.nrels)
+             snprintf(reldesc + strlen(reldesc),
+                      sizeof(reldesc) - strlen(reldesc),
+                      ", toast table for OID %u", rel->toastheap);
+     }
+
+     if (is_new_db)
+         pg_log(PG_WARNING, "No match found in old cluster for new relation with OID %u in database \"%s\": %s\n",
+                reloid, db->db_name, reldesc);
+     else
+         pg_log(PG_WARNING, "No match found in new cluster for old relation with OID %u in database \"%s\": %s\n",
+                reloid, db->db_name, reldesc);
+ }
+
+
  void
  print_maps(FileNameMap *maps, int n_maps, const char *db_name)
  {
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index ba79fb3..414183d 100644
*** a/src/bin/pg_upgrade/test.sh
--- b/src/bin/pg_upgrade/test.sh
*************** export EXTRA_REGRESS_OPTS
*** 152,157 ****
--- 152,162 ----
  set -x

  standard_initdb "$oldbindir"/initdb
+
+ # make pg_largeobject have a toast table in postgres DB
+ echo "alter table pg_largeobject reset (user_catalog_table)" | \
+   "$oldbindir"/postgres --single -O postgres
+
  "$oldbindir"/pg_ctl start -l "$logdir/postmaster1.log" -o "$POSTMASTER_OPTS" -w
  if "$MAKE" -C "$oldsrc" installcheck; then
      pg_dumpall -f "$temp_root"/dump1.sql || pg_dumpall1_status=$?

Re: pg_upgrade and toasted pg_largeobject

From
Robert Haas
Date:
On Tue, May 3, 2016 at 1:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Any thoughts what to do with this?  We could decide that it's a bug fix
> and backpatch, or decide that it's a new feature and delay till 9.7,
> or decide that it's a minor bug fix and add it to 9.6 only.  I kinda lean
> towards the last alternative.

I'm fine with that.

(But I haven't reviewed the code.)

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: pg_upgrade and toasted pg_largeobject

From
Alvaro Herrera
Date:
Tom Lane wrote:

> Any thoughts what to do with this?  We could decide that it's a bug fix
> and backpatch, or decide that it's a new feature and delay till 9.7,
> or decide that it's a minor bug fix and add it to 9.6 only.  I kinda lean
> towards the last alternative.

How about backpatching patch 1 all the way back, and putting the others
in 9.6?


-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: pg_upgrade and toasted pg_largeobject

From
Robert Haas
Date:
On Tue, May 3, 2016 at 2:09 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> Tom Lane wrote:
>> Any thoughts what to do with this?  We could decide that it's a bug fix
>> and backpatch, or decide that it's a new feature and delay till 9.7,
>> or decide that it's a minor bug fix and add it to 9.6 only.  I kinda lean
>> towards the last alternative.
>
> How about backpatching patch 1 all the way back, and putting the others
> in 9.6?

Why would we do that?  It seems very odd to back-patch a pure
refactoring - isn't that taking a risk for no benefit?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: pg_upgrade and toasted pg_largeobject

From
Alvaro Herrera
Date:
Robert Haas wrote:
> On Tue, May 3, 2016 at 2:09 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> > Tom Lane wrote:
> >> Any thoughts what to do with this?  We could decide that it's a bug fix
> >> and backpatch, or decide that it's a new feature and delay till 9.7,
> >> or decide that it's a minor bug fix and add it to 9.6 only.  I kinda lean
> >> towards the last alternative.
> >
> > How about backpatching patch 1 all the way back, and putting the others
> > in 9.6?
> 
> Why would we do that?  It seems very odd to back-patch a pure
> refactoring - isn't that taking a risk for no benefit?

From Tom's description, what is there works by chance only, and maybe
not even in all cases.  The rest of the patches are to fix one
particular problem, which perhaps is not overly serious, but maybe some
other future problem will be discovered and we will want to have patch 1
installed.

My inclination is actually to put the whole series back to 9.2, but if
we don't want to do that, then backpatching just the first one seems to
make pg_upgrade more amenable to future bugfixes.

(I say "back to 9.2" instead of "back to 9.1" because surely we don't
care all that much about upgrades *to* 9.1, since it's going unsupported
soon.)

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: pg_upgrade and toasted pg_largeobject

From
Tom Lane
Date:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Robert Haas wrote:
>> On Tue, May 3, 2016 at 2:09 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>>> How about backpatching patch 1 all the way back, and putting the others
>>> in 9.6?

>> Why would we do that?  It seems very odd to back-patch a pure
>> refactoring - isn't that taking a risk for no benefit?

Yeah, I don't see the point of that either.

> My inclination is actually to put the whole series back to 9.2, but if
> we don't want to do that, then backpatching just the first one seems to
> make pg_upgrade more amenable to future bugfixes.

I checked, and found that patch 1 doesn't apply cleanly before 9.5.
I've not looked into exactly why not, but it would possibly take some
work to adapt these patches to older branches.
        regards, tom lane