Re: [HACKERS] pg_upgrade incorrectly equates pg_default and database tablespace
On Thu, Mar 22, 2012 at 02:55:32PM +0200, Ants Aasma wrote: Hi, while working on a support case I stumbled upon a bug in pg_upgrade. Upgrade fails with No such file or directory when a database is moved to a non-default tablespace and contains a table that is moved to pg_default. The cause seems to be that the following test incorrectly equates empty spclocation with database tablespace: tblspace = PQgetvalue(res, relnum, i_spclocation); /* if no table tablespace, use the database tablespace */ if (strlen(tblspace) == 0) tblspace = dbinfo-db_tblspace; Patch to fix this is attached. Thank you for the fine bug report, and patch (and the bug confirmation from Jeff Davis). Sorry for the delay in replying. You have certainly found a bug, and one that exists all the way back to pg_upgrade 9.0. I was able to reproduce the bug with this SQL: -- test database in different tablespace with table in cluster -- default tablespace CREATE DATABASE tbltest TABLESPACE tt; \connect tbltest CREATE TABLE t1 (x int); CREATE TABLE t2 (x int) TABLESPACE pg_default; It is exactly as you described --- the database is in a user-defined tablespace, but the table (t2) is in the cluster default location. Not sure how no one else reported this failure before. The crux of the confusion is that pg_class.reltablespace == 0 means the database default tablespace, while a join to pg_tablespace that returns a zero-length string means it is in the cluster data directory. The new code properly looks at reltablespace rather than testing the tablespace location, which was your fix as well. I have applied three different patches very similar to your helpful suggestion, attached. -- Bruce Momjian br...@momjian.ushttp://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 1f5b7ae..02d3e0f *** a/contrib/pg_upgrade/info.c --- b/contrib/pg_upgrade/info.c *** get_rel_infos(migratorContext *ctx, cons *** 306,311 --- 306,312 int i_relname = -1; int i_oid = -1; int i_relfilenode = -1; + int i_reltablespace = -1; int i_reltoastrelid = -1; charquery[QUERY_ALLOC]; *** get_rel_infos(migratorContext *ctx, cons *** 320,326 snprintf(query, sizeof(query), SELECT DISTINCT c.oid, n.nspname, c.relname, ! c.relfilenode, c.reltoastrelid, t.spclocation FROM pg_catalog.pg_class c JOIN pg_catalog.pg_namespace n ON c.relnamespace = n.oid --- 321,327 snprintf(query, sizeof(query), SELECT DISTINCT c.oid, n.nspname, c.relname, ! c.relfilenode, c.reltoastrelid, c.reltablespace, t.spclocation FROM pg_catalog.pg_class c JOIN pg_catalog.pg_namespace n ON c.relnamespace = n.oid *** get_rel_infos(migratorContext *ctx, cons *** 339,345 ('pg_largeobject', 'pg_largeobject_loid_pn_index'%s) )) AND relkind IN ('r','t', 'i'%s) GROUP BY c.oid, n.nspname, c.relname, c.relfilenode, ! c.reltoastrelid, t.spclocation, n.nspname ORDER BY n.nspname, c.relname;, FirstNormalObjectId, --- 340,346 ('pg_largeobject', 'pg_largeobject_loid_pn_index'%s) )) AND relkind IN ('r','t', 'i'%s) GROUP BY c.oid, n.nspname, c.relname, c.relfilenode, ! c.reltoastrelid, c.reltablespace, t.spclocation, n.nspname ORDER BY n.nspname, c.relname;, FirstNormalObjectId, *** get_rel_infos(migratorContext *ctx, cons *** 361,366 --- 362,368 i_relname = PQfnumber(res, relname); i_relfilenode = PQfnumber(res, relfilenode); i_reltoastrelid = PQfnumber(res, reltoastrelid); + i_reltablespace = PQfnumber(res, reltablespace); i_spclocation = PQfnumber(res, spclocation); for (relnum = 0; relnum ntups; relnum++) *** get_rel_infos(migratorContext *ctx, cons *** 379,388 curr-relfilenode = atooid(PQgetvalue(res, relnum, i_relfilenode));
Re: [HACKERS] pg_upgrade incorrectly equates pg_default and database tablespace
On Fri, 2012-03-30 at 13:11 -0700, Jeff Davis wrote: I confirmed this bug upgrading 9.1 to master, and that this patch fixes it. Thank you for the report! Patch looks good to me as well, with one very minor nitpick: the added comment is missing an apostrophe. Bruce, can you take a look at this? Adding this to the next commitfest, just so it doesn't get forgotten. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade incorrectly equates pg_default and database tablespace
On Thu, 2012-03-22 at 14:55 +0200, Ants Aasma wrote: Hi, while working on a support case I stumbled upon a bug in pg_upgrade. Upgrade fails with No such file or directory when a database is moved to a non-default tablespace and contains a table that is moved to pg_default. The cause seems to be that the following test incorrectly equates empty spclocation with database tablespace: tblspace = PQgetvalue(res, relnum, i_spclocation); /* if no table tablespace, use the database tablespace */ if (strlen(tblspace) == 0) tblspace = dbinfo-db_tblspace; Patch to fix this is attached. I confirmed this bug upgrading 9.1 to master, and that this patch fixes it. Thank you for the report! Patch looks good to me as well, with one very minor nitpick: the added comment is missing an apostrophe. Bruce, can you take a look at this? Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg_upgrade incorrectly equates pg_default and database tablespace
Hi, while working on a support case I stumbled upon a bug in pg_upgrade. Upgrade fails with No such file or directory when a database is moved to a non-default tablespace and contains a table that is moved to pg_default. The cause seems to be that the following test incorrectly equates empty spclocation with database tablespace: tblspace = PQgetvalue(res, relnum, i_spclocation); /* if no table tablespace, use the database tablespace */ if (strlen(tblspace) == 0) tblspace = dbinfo-db_tblspace; Patch to fix this is attached. Regards, Ants Aasma -- Cybertec Schönig Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt Web: http://www.postgresql-support.de diff --git a/contrib/pg_upgrade/info.c b/contrib/pg_upgrade/info.c index 36683fa..3914403 100644 --- a/contrib/pg_upgrade/info.c +++ b/contrib/pg_upgrade/info.c @@ -253,6 +253,7 @@ get_rel_infos(ClusterInfo *cluster, DbInfo *dbinfo) char *nspname = NULL; char *relname = NULL; int i_spclocation, +i_spcoid, i_nspname, i_relname, i_oid, @@ -269,7 +270,7 @@ get_rel_infos(ClusterInfo *cluster, DbInfo *dbinfo) snprintf(query, sizeof(query), SELECT c.oid, n.nspname, c.relname, - c.relfilenode, %s + c.relfilenode, t.oid AS spcoid, %s FROM pg_catalog.pg_class c JOIN pg_catalog.pg_namespace n ON c.relnamespace = n.oid LEFT OUTER JOIN pg_catalog.pg_tablespace t @@ -306,6 +307,7 @@ get_rel_infos(ClusterInfo *cluster, DbInfo *dbinfo) i_nspname = PQfnumber(res, nspname); i_relname = PQfnumber(res, relname); i_relfilenode = PQfnumber(res, relfilenode); + i_spcoid = PQfnumber(res, spcoid); i_spclocation = PQfnumber(res, spclocation); for (relnum = 0; relnum ntups; relnum++) @@ -325,7 +327,7 @@ get_rel_infos(ClusterInfo *cluster, DbInfo *dbinfo) tblspace = PQgetvalue(res, relnum, i_spclocation); /* if no table tablespace, use the database tablespace */ - if (strlen(tblspace) == 0) + if (atooid(PQgetvalue(res, relnum, i_spcoid)) == InvalidOid) tblspace = dbinfo-db_tblspace; strlcpy(curr-tablespace, tblspace, sizeof(curr-tablespace)); } diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h index c1925cf..234ca99 100644 --- a/contrib/pg_upgrade/pg_upgrade.h +++ b/contrib/pg_upgrade/pg_upgrade.h @@ -109,7 +109,8 @@ typedef struct char relname[NAMEDATALEN]; /* relation name */ Oid reloid; /* relation oid */ Oid relfilenode; /* relation relfile node */ - char tablespace[MAXPGPATH]; /* relations tablespace path */ + /* relations tablespace path, empty for pg_default */ + char tablespace[MAXPGPATH]; } RelInfo; typedef struct -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers