Re: [PATCH] Relocation of tablespaces in pg_basebackup

New patch attached with the following changes:

On Thu, Jan 23, 2014 at 11:01 AM, Steeve Lennmark <steevel(at)handeldsbanken(dot)se
> wrote:

> On Thu, Jan 23, 2014 at 2:06 AM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
>
>> 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".
>>
>
> This design choice I made about -T (--tablespace) and colon as
> separator was copied from pg_barman, which is the way I back up my
> clusters at the moment. Renaming the option to --tablespace-mapping and
> changing the syntax to something like "=" is totally fine by me.
>

I changed the directory separator from ":" to "=" but made it
configurable in the code.

> 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.
>>
>
> Accepting a relative path should be fine, I made a failed attempt using
> realpath(3) initially but I guess checking for [0] != '/' and
> prepending getcwd(3) would suffice.
>

Relative paths are now accepted. This code will probably not work on
windows though. I tried setting up Windows 8 with the git version of
postgres but was unsuccessful, so I can't really test any of this.
Help on this subject (Windows) would be much appreciated.

> 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.
>>
>
> I like that a lot, but I'm afraid the code would have to get a bit more
> complex to add that functionality. It would be an easier rewrite if we
> added a hint character, something like -T '/mnt/*:mnt'.
>

This is not implemented as suggested by Peter in his previous comment.
-T /mnt:mnt now relocates all tablespaces under /mnt to the relative
path mnt.

Review style guide for error messages:
>> http://www.postgresql.org/docs/devel/static/error-style-guide.html
>
>
I've updated error messages according to the style guide.

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.
>>
>
> I hope someone with experience with those kind of systems can come up
> with suggestions on how to solve that. I only run postgres on Linux.
>

I would still love some input on this.

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. ;-)
>>
>
> Done.
>

//Steeve