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 <br...@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 <br...@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers