Re: Proposal for changes to recovery.conf API

On Fri, Nov 4, 2016 at 10:17 AM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> wrote:
> At 2016-11-04 10:35:05 +0000, simon(at)2ndquadrant(dot)com wrote:
>> Since the "lots of parameters" approach is almost exactly what we have
>> now, I think we should do that first, get this patch committed and
>> then sit back and discuss an improved API and see what downsides it
>> introduces.
>
> Thanks, I agree strongly with this.
>
> Someone (Michael?) earlier mentioned the potential for introducing bugs
> with this patch (the idea of merging recovery.conf into postgresql.conf
> at all, not this particular incarnation).
>
> I think the current proposed approach with
>
> recovery_target=xid
> recovery_target_xid=xxx
>
> is preferable because (a) it doesn't introduce much new code, e.g., new
> parsing functions, nor (b) need many changes in the documentation—all we
> need is to say that of the various recovery_target_xxx parameters, the
> one that has priority is the one named by recovery_target.
>
> If I were to introduce recovery_target='xid xxx', I would at a minimum
> need to factor out (or duplicate) parsing and error handling code, make
> a type/value union-in-struct to store in the GUC *extra, then make sure
> that we handle the older values in a backwards-compatible way, and move
> a bunch of documentation around.

Just to be clear, the sum total of the additional "parsing" we are
talking about is looking for the first sequence of 1 or more spaces in
the input string and separating the stuff before them from the stuff
after them. I agree that there's some work there, but I'm surprised
to hear that you think it's a lot of work.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company