On Sun, May 10, 2015 at 6:01 AM, Andrew Dunstan <
andrew@dunslane.net> wrote:
>
>
>
> This generally looks good, but I have a couple of questions before I commit it.
>
> First, why is the new option for the BASE_BACKUP command of the Streaming Replication protcol "TAR"? It seems rather misleading. Shouldn't it be something like "TABLESPACEMAP"?
>
The reason to keep new option's name as TAR was that tablespace_map
was generated for that format type, but I agree with you that something
like "TABLESPACEMAP" suits better, so I have changed it to
"TABLESPACE_MAP". Putting '_' in name makes it somewhat consistent
with other names and filename it generates with this new option.
> Second, these lines in xlog.c seem wrong:
>
> else if ((ch == '\n' || ch == '\r') && prev_ch == '\\')
> str[i-1] = '\n';
>
> It looks to me like we should be putting ch in the string, not arbitrarily transforming \r into \n.
>
You are right, I have changed it as per your suggestion.