Re: [PATCH] minor bugfix for pg_basebackup (9.6 ~ )

On 7/23/19 5:10 PM, Michael Paquier wrote:
> On Mon, Jul 22, 2019 at 12:58:40PM -0400, Alvaro Herrera wrote:
>> Maybe it's just me, but it seems weird to try to forestall a problem
>> that cannot occur by definition. I would rather remove the escaping,
>> and add a one-line comment explaining why we don't do it?
>
> No objections with doing that either, really. Perhaps you would
> prefer pushing a patch among those lines by yourself?
>
> One argument that I got in mind to justify the escaping would be if we
> add a new feature in pg_basebackup to write a new set of recovery
> options on an existing data folder, which does not require an option.
> In this case, if the escaping does not exist, starting the server
> would fail with a confusing parsing error if a quote is added to the
> slot name. But if the escaping is done, then we would get a correct
> error that the replication slot value includes an incorrect character.
> If such an hypothetical option is added, most likely this would be
> noticed anyway, so that's mainly nannyism from my side.

It'd be better if such a hypothetical option validated the provided
slot name anwyay, to prevent later surprises.

Revised patch attached, which as Alvaro suggests removes the escaping
and adds a comment explaining why the raw value can be passed as-is.

Regards

Ian Barwick

--
Ian Barwick https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services