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. ;-) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers