On Sun, May 10, 2015 at 6:01 AM, Andrew Dunstan <and...@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.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Attachment: extend_basebackup_to_include_symlink_v7.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to