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.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 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;
char query[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));
curr->toastrelid = atooid(PQgetvalue(res, relnum,
i_reltoastrelid));
! tblspace = PQgetvalue(res, relnum, i_spclocation);
! /* if no table tablespace, use the database tablespace */
! if (strlen(tblspace) == 0)
tblspace = dbinfo->db_tblspace;
strlcpy(curr->tablespace, tblspace, sizeof(curr->tablespace));
}
PQclear(res);
--- 381,393 ----
curr->relfilenode = atooid(PQgetvalue(res, relnum,
i_relfilenode));
curr->toastrelid = atooid(PQgetvalue(res, relnum,
i_reltoastrelid));
! if (atooid(PQgetvalue(res, relnum, i_reltablespace)) != 0)
! /* Might be "", meaning the cluster default location. */
! tblspace = PQgetvalue(res, relnum, i_spclocation);
! else
! /* A zero reltablespace indicates the database
tablespace. */
tblspace = dbinfo->db_tblspace;
+
strlcpy(curr->tablespace, tblspace, sizeof(curr->tablespace));
}
PQclear(res);
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
new file mode 100644
index 7a02fa1..bfcabb3
*** a/contrib/pg_upgrade/pg_upgrade.h
--- b/contrib/pg_upgrade/pg_upgrade.h
*************** typedef struct
*** 69,75 ****
Oid reloid; /* relation oid
*/
Oid relfilenode; /* relation relfile node
*/
Oid toastrelid; /* oid of the toast
relation */
! char tablespace[MAXPGPATH]; /* relations tablespace path */
} RelInfo;
typedef struct
--- 69,76 ----
Oid reloid; /* relation oid
*/
Oid relfilenode; /* relation relfile node
*/
Oid toastrelid; /* oid of the toast
relation */
! /* relation tablespace path, or "" for the cluster default */
! char tablespace[MAXPGPATH];
} RelInfo;
typedef struct
diff --git a/contrib/pg_upgrade/info.c b/contrib/pg_upgrade/info.c
new file mode 100644
index 7e177d4..dad0117
*** a/contrib/pg_upgrade/info.c
--- b/contrib/pg_upgrade/info.c
*************** get_rel_infos(ClusterInfo *cluster, DbIn
*** 250,256 ****
i_nspname,
i_relname,
i_oid,
! i_relfilenode;
char query[QUERY_ALLOC];
/*
--- 250,257 ----
i_nspname,
i_relname,
i_oid,
! i_relfilenode,
! i_reltablespace;
char query[QUERY_ALLOC];
/*
*************** get_rel_infos(ClusterInfo *cluster, DbIn
*** 263,269 ****
snprintf(query, sizeof(query),
"SELECT c.oid, n.nspname, c.relname, "
! " c.relfilenode, t.spclocation "
"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 "
--- 264,270 ----
snprintf(query, sizeof(query),
"SELECT c.oid, n.nspname, c.relname, "
! " c.relfilenode, c.reltablespace, t.spclocation "
"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 "
*************** get_rel_infos(ClusterInfo *cluster, DbIn
*** 297,302 ****
--- 298,304 ----
i_nspname = PQfnumber(res, "nspname");
i_relname = PQfnumber(res, "relname");
i_relfilenode = PQfnumber(res, "relfilenode");
+ i_reltablespace = PQfnumber(res, "reltablespace");
i_spclocation = PQfnumber(res, "spclocation");
for (relnum = 0; relnum < ntups; relnum++)
*************** get_rel_infos(ClusterInfo *cluster, DbIn
*** 314,323 ****
curr->relfilenode = atooid(PQgetvalue(res, relnum,
i_relfilenode));
! tblspace = PQgetvalue(res, relnum, i_spclocation);
! /* if no table tablespace, use the database tablespace */
! if (strlen(tblspace) == 0)
tblspace = dbinfo->db_tblspace;
strlcpy(curr->tablespace, tblspace, sizeof(curr->tablespace));
}
PQclear(res);
--- 316,328 ----
curr->relfilenode = atooid(PQgetvalue(res, relnum,
i_relfilenode));
! if (atooid(PQgetvalue(res, relnum, i_reltablespace)) != 0)
! /* Might be "", meaning the cluster default location. */
! tblspace = PQgetvalue(res, relnum, i_spclocation);
! else
! /* A zero reltablespace indicates the database
tablespace. */
tblspace = dbinfo->db_tblspace;
+
strlcpy(curr->tablespace, tblspace, sizeof(curr->tablespace));
}
PQclear(res);
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
new file mode 100644
index 898541b..204749b
*** a/contrib/pg_upgrade/pg_upgrade.h
--- b/contrib/pg_upgrade/pg_upgrade.h
*************** typedef struct
*** 71,77 ****
char relname[NAMEDATALEN]; /* relation name */
Oid reloid; /* relation oid */
Oid relfilenode; /* relation relfile node */
! char tablespace[MAXPGPATH]; /* relations tablespace path */
} RelInfo;
typedef struct
--- 71,78 ----
char relname[NAMEDATALEN]; /* relation name */
Oid reloid; /* relation oid */
Oid relfilenode; /* relation relfile node */
! /* relation tablespace path, or "" for the cluster default */
! char tablespace[MAXPGPATH];
} RelInfo;
typedef struct
diff --git a/contrib/pg_upgrade/info.c b/contrib/pg_upgrade/info.c
new file mode 100644
index 36683fa..e0e3a9f
*** a/contrib/pg_upgrade/info.c
--- b/contrib/pg_upgrade/info.c
*************** get_rel_infos(ClusterInfo *cluster, DbIn
*** 256,262 ****
i_nspname,
i_relname,
i_oid,
! i_relfilenode;
char query[QUERY_ALLOC];
/*
--- 256,263 ----
i_nspname,
i_relname,
i_oid,
! i_relfilenode,
! i_reltablespace;
char query[QUERY_ALLOC];
/*
*************** get_rel_infos(ClusterInfo *cluster, DbIn
*** 269,275 ****
snprintf(query, sizeof(query),
"SELECT c.oid, n.nspname, c.relname, "
! " c.relfilenode, %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 "
--- 270,276 ----
snprintf(query, sizeof(query),
"SELECT c.oid, n.nspname, c.relname, "
! " c.relfilenode, c.reltablespace, %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 "
*************** get_rel_infos(ClusterInfo *cluster, DbIn
*** 306,311 ****
--- 307,313 ----
i_nspname = PQfnumber(res, "nspname");
i_relname = PQfnumber(res, "relname");
i_relfilenode = PQfnumber(res, "relfilenode");
+ i_reltablespace = PQfnumber(res, "reltablespace");
i_spclocation = PQfnumber(res, "spclocation");
for (relnum = 0; relnum < ntups; relnum++)
*************** get_rel_infos(ClusterInfo *cluster, DbIn
*** 323,332 ****
curr->relfilenode = atooid(PQgetvalue(res, relnum,
i_relfilenode));
! tblspace = PQgetvalue(res, relnum, i_spclocation);
! /* if no table tablespace, use the database tablespace */
! if (strlen(tblspace) == 0)
tblspace = dbinfo->db_tblspace;
strlcpy(curr->tablespace, tblspace, sizeof(curr->tablespace));
}
PQclear(res);
--- 325,337 ----
curr->relfilenode = atooid(PQgetvalue(res, relnum,
i_relfilenode));
! if (atooid(PQgetvalue(res, relnum, i_reltablespace)) != 0)
! /* Might be "", meaning the cluster default location. */
! tblspace = PQgetvalue(res, relnum, i_spclocation);
! else
! /* A zero reltablespace indicates the database
tablespace. */
tblspace = dbinfo->db_tblspace;
+
strlcpy(curr->tablespace, tblspace, sizeof(curr->tablespace));
}
PQclear(res);
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
new file mode 100644
index c1925cf..6dcb1a5
*** a/contrib/pg_upgrade/pg_upgrade.h
--- b/contrib/pg_upgrade/pg_upgrade.h
*************** typedef struct
*** 109,115 ****
char relname[NAMEDATALEN]; /* relation name */
Oid reloid; /* relation oid */
Oid relfilenode; /* relation relfile node */
! char tablespace[MAXPGPATH]; /* relations tablespace path */
} RelInfo;
typedef struct
--- 109,116 ----
char relname[NAMEDATALEN]; /* relation name */
Oid reloid; /* relation oid */
Oid relfilenode; /* relation relfile node */
! /* relation tablespace path, or "" for the cluster 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
