At 2016-11-04 10:35:05 +0000, si...@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 -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers