Greetings,
* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
> On 27/11/2018 13:29, Peter Eisentraut wrote:
> > On 27/11/2018 10:10, Sergei Kornilov wrote:
> >>>> - recovery_target = immediate was replaced with recovery_target_immediate bool GUC
> >>>
> >>> Why?
> >> Due this comment: https://www.postgresql.org/message-id/20181126172118.GY3415%40tamriel.snowman.net
> >>> I've not been following this very closely, but seems like
> >>> recovery_target_string is a bad idea.. Why not just make that
> >>> 'recovery_target_immediate' and make it a boolean? Having a GUC that's
> >>> only got one valid value seems really odd.
> >
> > It is a bit odd, but that's the way it's been, and I don't see a reason
> > to change it as part of this fix. We are attempting to fix the way the
> > GUC parameters are parsed, not change the name and meaning of the
> > parameters themselves.
I disagree quite a bit- we're whacking around how recovery works and
this has been a wart since it went in because the contemplated later
changes to have more than one value be accepted there never
materialized, so we might as well change it while we're changing
everything else and clean it up.
If we really want to change it later we can do that, but we've evidently
decided to go the opposite direction and have multiple GUCs instead, so
let's be consistent.
> The attached seems to be the simplest way to fix this. (Needs
> documentation updates, test updates, error message refinement.)
Sure, this approach seems fine to me and is perhaps a bit cleaner.
Thanks!
Stephen