Re: Tablespaces

Gavin Sherry wrote:
> Attached is my latest patch implementing tablespaces. This has all the
> functionality I was planning for 7.5.

A few minor points I happened to notice while reading through the patch:

+  * To simply initialisation and XLog activity, have create and
maintain
+  * a symbolic link map in data/pg_tablespaces.

Grammar errors.

+ void
+ TblspcCreateDbspace(Oid tbloid)
+ {
+ #ifndef HAVE_SYMLINK
+     return;
+ #endif
+     struct stat st;
+     char        *dir;

If HAVE_SYMLINK is undefined, this is a syntax error (at least in
C89, which is what we ought to limit ourselves to). Similar problems
elsewhere in the same file (tablespc.c)

+     dir = (char *) palloc(strlen(DataDir) + 14 + 10 + 10 + 10 + 3 + 1);
+     sprintf(dir, "%s/pg_tablespaces/%u/%u", DataDir, tbloid,
+                 MyDatabaseId);

Is the length of that buffer right? At the least the addition is a
little weird (why are you adding 10 three times for two numeric
variables?) I noticed another buffer allocation (linkloc) that
looked dubious at first glance as well.

+     char        realnewpath[MAXPGPATH];

This is somewhat pedantic, but how do we know that MAXPGPATH >=
PATH_MAX (the minimum safe size of the second argument to
realpath(), at least on my local system)?

-Neil