My review: Clearly, everyone likes this feature.
I'm tempted to think it should be mandatory to specify this option in
plain mode when tablespaces are present. Otherwise, creating a base
backup is liable to create random files all over the place. Obviously,
there would be backward compatibility concerns.
I'm not totally happy with the choice of ":" as the mapping separator,
because that would always require escaping on Windows. We could make it
analogous to the path handling and make ";" the separator on Windows.
Then again, this is not a path, so maybe it should look like one. We
pick something else altogether, like "=".
The option name "--tablespace" isn't very clear. It ought to be
something like "--tablespace-mapping".
I don't think we should require the new directory to be an absolute
path. It should be relative to the current directory, just like the -D
option does it.
I'm not so worried about the atooid() and linked-list duplication. That
can be addressed at some later point.
I would try to write this patch without using MAXPGPATH. I know
existing code uses it, but we should try to use it less, because it
overallocates memory and requires handling additional error conditions.
For example, you catch overflow in tablespace_list_append() but later
report that as invalid tablespace format. We now have psprintf() to
make coding with dynamic memory allocation easier.
It looks like when you ignore an escaped ":" as a separator, you don't
actually unescape it for use as a directory.
OLDDIR and NEWDIR should be capitalized in the help message.
Somehow, I had the subconscious assumption that this feature would do
prefix matching on the directories, not only complete string matching.
So if I had tablespaces in /mnt/data1, /mnt/data2, /mnt/data3, I could
map them all with -T /mnt:mnt and be done. Not sure if that is useful
to many, but it's worth a thought.
Review style guide for error messages:
http://www.postgresql.org/docs/devel/static/error-style-guide.html
We need to think about how to handle this on platforms without symlinks.
I don't like just printing an error message and moving on. It should be
either pass or fail or an option to choose between them.
Please put the options in the getopt handling in alphabetical order.
It's not very alphabetical now, but between D and F is probably not the
best place. ;-)
