Re: Possible Bug in pg_upgrade - Mailing list pgsql-hackers
| From | Bruce Momjian |
|---|---|
| Subject | Re: Possible Bug in pg_upgrade |
| Date | |
| Msg-id | 201108150327.p7F3RtM21294@momjian.us Whole thread Raw |
| In response to | Possible Bug in pg_upgrade (Dave Byrne <dbyrne@mdb.com>) |
| Responses |
Re: Possible Bug in pg_upgrade
|
| List | pgsql-hackers |
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. +
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);
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);
pgsql-hackers by date: