At 2016-11-04 10:35:05 +0000, simon@2ndquadrant.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.
Can it be done? Yes, of course; and I'll do it if that's the consensus.
But it would be a backwards-compatible change to the current approach
anyway, and I think it would be better to put in the simple way now
before discussing the new API further.
Let's get the basic new *functionality* out so that people can play with
it, I'm sure we'll find a few non-API things that need tweaking as a
result of the change.
-- Abhijit