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(at)momjian(dot)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(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +