Re: Possible Bug in pg_upgrade

Applied.

---------------------------------------------------------------------------

Bruce Momjian wrote:
> Dave Byrne wrote:
> > Beginning with commit 002c105a0706bd1c1e939fe0f47ecdceeae6c52d
> > pg_upgrade will fail if there are orphaned temp tables in the current
> > database with the message 'old and new databases "postgres" have a
> > different number of relations'
> > 
> > On line 41 of pg_upgrade/info.c pg_upgrade checks that the number of
> > relations are the same but includes orphaned temp tables in the comparison.
> > 
> > Is this expected behavior?  If so is there a more detailed error message
> > that can be added explain the cause of the failure? It wasn't evident
> > until modified pg_upgrade to show the missing oid's and recognized them
> > as temp tables with oid2name.
> 
> Thanks for your report and patch.
> 
> Let me give some background on pg_upgrade to explain what is happening. 
> Pg_upgrade uses two C arrays to store information about tables and
> indexes for the old and new clusters.  It is not possible to store this
> information in a database because both clusters are down when pg_upgrade
> needs to use this information.
> 
> In pre-9.1 pg_upgrade, pg_upgrade did a sequential scan of the arrays
> looking for a match between old and new cluster objects.  This was
> reported as slow for databases with many objects, and I could reproduce
> the slowness.  I added some caching in 9.0 but the real solution for 9.1
> was to assume a one-to-one mapping between the old and new C arrays,
> i.e. the 5th entry in the old cluster array is the same as the 5th
> element in the new cluster array.
> 
> I knew this was risky but was the right solution so it doesn't surprise
> me you found a failure.  pg_upgrade checks that the size of the two
> arrays in the same and also checks that each element matches --- the
> former is what generated your error.
> 
> Now, about the cause.  I had not anticipated that orphaned temp objects
> could exist in either cluster.  In fact, this case would have generated
> an error in 9.0 as well, but with a different error message.
> 
> Looking futher, pg_upgrade has to use the same object filter as
> pg_dump, and pg_dump uses this C test:
> 
>     pg_dump.c:      else if (strncmp(nsinfo->dobj.name, "pg_", 3) == 0 ||
> 
> pg_dumpall uses this filter:
> 
>         "WHERE spcname !~ '^pg_' "
> 
> The problem is that the filter used by pg_upgrade only excluded
> pg_catalog, not pg_temp* as well.
> 
> I have developed the attached two patches, one for 9.0, and the second
> for 9.1 and 9.2 which will make pg_upgrade now match the pg_dump
> filtering and produce proper results for orphaned temp tables by
> filtering them.
> 
> As far as unlogged tables, those are dumped by 9.1/9.2, so there is no
> need to check relpersistence in this patch.  pg_dump doesn't check
> relistemp either.
> 
> -- 
>   Bruce Momjian  <bruce@momjian.us>        http://momjian.us
>   EnterpriseDB                             http://enterprisedb.com
> 
>   + It's impossible for everything to be true. +

[ text/x-diff is unsupported, treating like TEXT/PLAIN ]

> diff --git a/contrib/pg_upgrade/info.c b/contrib/pg_upgrade/info.c
> new file mode 100644
> index 567c64e..ca357e7
> *** a/contrib/pg_upgrade/info.c
> --- b/contrib/pg_upgrade/info.c
> *************** get_rel_infos(migratorContext *ctx, cons
> *** 326,332 ****
>                "    ON c.relnamespace = n.oid "
>                "   LEFT OUTER JOIN pg_catalog.pg_tablespace t "
>                "    ON c.reltablespace = t.oid "
> !              "WHERE (( n.nspname NOT IN ('pg_catalog', 'information_schema') "
>                "    AND c.oid >= %u "
>                "    ) OR ( "
>                "    n.nspname = 'pg_catalog' "
> --- 326,335 ----
>                "    ON c.relnamespace = n.oid "
>                "   LEFT OUTER JOIN pg_catalog.pg_tablespace t "
>                "    ON c.reltablespace = t.oid "
> !              "WHERE (( "
> !              /* exclude pg_catalog and pg_temp_ (could be orphaned tables) */
> !              "    n.nspname !~ '^pg_' "
> !              "    AND n.nspname != 'information_schema' "
>                "    AND c.oid >= %u "
>                "    ) OR ( "
>                "    n.nspname = 'pg_catalog' "
> diff --git a/contrib/pg_upgrade/version_old_8_3.c b/contrib/pg_upgrade/version_old_8_3.c
> new file mode 100644
> index 6ca266c..930f76d
> *** a/contrib/pg_upgrade/version_old_8_3.c
> --- b/contrib/pg_upgrade/version_old_8_3.c
> *************** old_8_3_check_for_name_data_type_usage(m
> *** 61,67 ****
>                                   "        NOT a.attisdropped AND "
>                                   "        a.atttypid = 'pg_catalog.name'::pg_catalog.regtype AND "
>                                   "        c.relnamespace = n.oid AND "
> !                               "        n.nspname != 'pg_catalog' AND "
>                            "        n.nspname != 'information_schema'");
>   
>           ntups = PQntuples(res);
> --- 61,68 ----
>                                   "        NOT a.attisdropped AND "
>                                   "        a.atttypid = 'pg_catalog.name'::pg_catalog.regtype AND "
>                                   "        c.relnamespace = n.oid AND "
> !                              /* exclude pg_catalog and pg_temp_ (could be orphaned tables) */
> !                                 "   n.nspname !~ '^pg_' AND "
>                            "        n.nspname != 'information_schema'");
>   
>           ntups = PQntuples(res);
> *************** old_8_3_check_for_tsquery_usage(migrator
> *** 151,157 ****
>                                   "        NOT a.attisdropped AND "
>                                   "        a.atttypid = 'pg_catalog.tsquery'::pg_catalog.regtype AND "
>                                   "        c.relnamespace = n.oid AND "
> !                               "        n.nspname != 'pg_catalog' AND "
>                            "        n.nspname != 'information_schema'");
>   
>           ntups = PQntuples(res);
> --- 152,159 ----
>                                   "        NOT a.attisdropped AND "
>                                   "        a.atttypid = 'pg_catalog.tsquery'::pg_catalog.regtype AND "
>                                   "        c.relnamespace = n.oid AND "
> !                              /* exclude pg_catalog and pg_temp_ (could be orphaned tables) */
> !                                 "   n.nspname !~ '^pg_' AND "
>                            "        n.nspname != 'information_schema'");
>   
>           ntups = PQntuples(res);
> *************** old_8_3_rebuild_tsvector_tables(migrator
> *** 250,256 ****
>                                   "        NOT a.attisdropped AND "
>                                   "        a.atttypid = 'pg_catalog.tsvector'::pg_catalog.regtype AND "
>                                   "        c.relnamespace = n.oid AND "
> !                               "        n.nspname != 'pg_catalog' AND "
>                            "        n.nspname != 'information_schema'");
>   
>   /*
> --- 252,259 ----
>                                   "        NOT a.attisdropped AND "
>                                   "        a.atttypid = 'pg_catalog.tsvector'::pg_catalog.regtype AND "
>                                   "        c.relnamespace = n.oid AND "
> !                              /* exclude pg_catalog and pg_temp_ (could be orphaned tables) */
> !                                 "   n.nspname !~ '^pg_' AND "
>                            "        n.nspname != 'information_schema'");
>   
>   /*
> *************** old_8_3_rebuild_tsvector_tables(migrator
> *** 268,274 ****
>                                   "        NOT a.attisdropped AND "        \
>                                   "        a.atttypid = 'pg_catalog.tsvector'::pg_catalog.regtype AND " \
>                                   "        c.relnamespace = n.oid AND "    \
> !                                 "        n.nspname != 'pg_catalog' AND " \
>                                   "        n.nspname != 'information_schema') "
>   
>           ntups = PQntuples(res);
> --- 271,277 ----
>                                   "        NOT a.attisdropped AND "        \
>                                   "        a.atttypid = 'pg_catalog.tsvector'::pg_catalog.regtype AND " \
>                                   "        c.relnamespace = n.oid AND "    \
> !                                 "       n.nspname !~ '^pg_' AND "        \
>                                   "        n.nspname != 'information_schema') "
>   
>           ntups = PQntuples(res);
> *************** old_8_3_create_sequence_script(migratorC
> *** 638,644 ****
>                                   "        pg_catalog.pg_namespace n "
>                                   "WHERE    c.relkind = 'S' AND "
>                                   "        c.relnamespace = n.oid AND "
> !                               "        n.nspname != 'pg_catalog' AND "
>                            "        n.nspname != 'information_schema'");
>   
>           ntups = PQntuples(res);
> --- 641,648 ----
>                                   "        pg_catalog.pg_namespace n "
>                                   "WHERE    c.relkind = 'S' AND "
>                                   "        c.relnamespace = n.oid AND "
> !                              /* exclude pg_catalog and pg_temp_ (could be orphaned tables) */
> !                                 "   n.nspname !~ '^pg_' AND "
>                            "        n.nspname != 'information_schema'");
>   
>           ntups = PQntuples(res);

[ text/x-diff is unsupported, treating like TEXT/PLAIN ]

> diff --git a/contrib/pg_upgrade/info.c b/contrib/pg_upgrade/info.c
> new file mode 100644
> index 72bc489..3ef3429
> *** a/contrib/pg_upgrade/info.c
> --- b/contrib/pg_upgrade/info.c
> *************** get_rel_infos(ClusterInfo *cluster, DbIn
> *** 266,272 ****
>                "  LEFT OUTER JOIN pg_catalog.pg_tablespace t "
>                "       ON c.reltablespace = t.oid "
>                "WHERE relkind IN ('r','t', 'i'%s) AND "
> !              "  ((n.nspname NOT IN ('pg_catalog', 'information_schema', 'binary_upgrade') AND "
>                "      c.oid >= %u) "
>                "  OR (n.nspname = 'pg_catalog' AND "
>       "    relname IN ('pg_largeobject', 'pg_largeobject_loid_pn_index'%s) )) "
> --- 266,274 ----
>                "  LEFT OUTER JOIN pg_catalog.pg_tablespace t "
>                "       ON c.reltablespace = t.oid "
>                "WHERE relkind IN ('r','t', 'i'%s) AND "
> !              /* exclude pg_catalog and pg_temp_ (could be orphaned tables) */
> !              "  ((n.nspname !~ '^pg_' AND "
> !             "     n.nspname NOT IN ('information_schema', 'binary_upgrade') AND "
>                "      c.oid >= %u) "
>                "  OR (n.nspname = 'pg_catalog' AND "
>       "    relname IN ('pg_largeobject', 'pg_largeobject_loid_pn_index'%s) )) "
> diff --git a/contrib/pg_upgrade/version_old_8_3.c b/contrib/pg_upgrade/version_old_8_3.c
> new file mode 100644
> index 43bfdc1..1c736d2
> *** a/contrib/pg_upgrade/version_old_8_3.c
> --- b/contrib/pg_upgrade/version_old_8_3.c
> *************** old_8_3_check_for_name_data_type_usage(C
> *** 59,65 ****
>                                   "        NOT a.attisdropped AND "
>                                   "        a.atttypid = 'pg_catalog.name'::pg_catalog.regtype AND "
>                                   "        c.relnamespace = n.oid AND "
> !                               "        n.nspname != 'pg_catalog' AND "
>                            "        n.nspname != 'information_schema'");
>   
>           ntups = PQntuples(res);
> --- 59,66 ----
>                                   "        NOT a.attisdropped AND "
>                                   "        a.atttypid = 'pg_catalog.name'::pg_catalog.regtype AND "
>                                   "        c.relnamespace = n.oid AND "
> !                              /* exclude pg_catalog and pg_temp_ (could be orphaned tables) */
> !                                 "   n.nspname !~ '^pg_' AND "
>                            "        n.nspname != 'information_schema'");
>   
>           ntups = PQntuples(res);
> *************** old_8_3_check_for_tsquery_usage(ClusterI
> *** 148,154 ****
>                                   "        NOT a.attisdropped AND "
>                                   "        a.atttypid = 'pg_catalog.tsquery'::pg_catalog.regtype AND "
>                                   "        c.relnamespace = n.oid AND "
> !                               "        n.nspname != 'pg_catalog' AND "
>                            "        n.nspname != 'information_schema'");
>   
>           ntups = PQntuples(res);
> --- 149,156 ----
>                                   "        NOT a.attisdropped AND "
>                                   "        a.atttypid = 'pg_catalog.tsquery'::pg_catalog.regtype AND "
>                                   "        c.relnamespace = n.oid AND "
> !                              /* exclude pg_catalog and pg_temp_ (could be orphaned tables) */
> !                                 "   n.nspname !~ '^pg_' AND "
>                            "        n.nspname != 'information_schema'");
>   
>           ntups = PQntuples(res);
> *************** old_8_3_rebuild_tsvector_tables(ClusterI
> *** 245,251 ****
>                                   "        NOT a.attisdropped AND "
>                                   "        a.atttypid = 'pg_catalog.tsvector'::pg_catalog.regtype AND "
>                                   "        c.relnamespace = n.oid AND "
> !                               "        n.nspname != 'pg_catalog' AND "
>                            "        n.nspname != 'information_schema'");
>   
>   /*
> --- 247,254 ----
>                                   "        NOT a.attisdropped AND "
>                                   "        a.atttypid = 'pg_catalog.tsvector'::pg_catalog.regtype AND "
>                                   "        c.relnamespace = n.oid AND "
> !                              /* exclude pg_catalog and pg_temp_ (could be orphaned tables) */
> !                                 "   n.nspname !~ '^pg_' AND "
>                            "        n.nspname != 'information_schema'");
>   
>   /*
> *************** old_8_3_rebuild_tsvector_tables(ClusterI
> *** 263,269 ****
>                                   "        NOT a.attisdropped AND "        \
>                                   "        a.atttypid = 'pg_catalog.tsvector'::pg_catalog.regtype AND " \
>                                   "        c.relnamespace = n.oid AND "    \
> !                                 "        n.nspname != 'pg_catalog' AND " \
>                                   "        n.nspname != 'information_schema') "
>   
>           ntups = PQntuples(res);
> --- 266,272 ----
>                                   "        NOT a.attisdropped AND "        \
>                                   "        a.atttypid = 'pg_catalog.tsvector'::pg_catalog.regtype AND " \
>                                   "        c.relnamespace = n.oid AND "    \
> !                                 "       n.nspname !~ '^pg_' AND "        \
>                                   "        n.nspname != 'information_schema') "
>   
>           ntups = PQntuples(res);
> *************** old_8_3_create_sequence_script(ClusterIn
> *** 616,622 ****
>                                   "        pg_catalog.pg_namespace n "
>                                   "WHERE    c.relkind = 'S' AND "
>                                   "        c.relnamespace = n.oid AND "
> !                               "        n.nspname != 'pg_catalog' AND "
>                            "        n.nspname != 'information_schema'");
>   
>           ntups = PQntuples(res);
> --- 619,626 ----
>                                   "        pg_catalog.pg_namespace n "
>                                   "WHERE    c.relkind = 'S' AND "
>                                   "        c.relnamespace = n.oid AND "
> !                              /* exclude pg_catalog and pg_temp_ (could be orphaned tables) */
> !                                 "   n.nspname !~ '^pg_' AND "
>                            "        n.nspname != 'information_schema'");
>   
>           ntups = PQntuples(res);

> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + It's impossible for everything to be true. +