Re: LSN as a recovery target

On Sun, Sep 4, 2016 at 3:02 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On 4 September 2016 at 04:50, Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote:
>> On Sun, Sep 4, 2016 at 8:05 AM, Michael Paquier
>> <michael(dot)paquier(at)gmail(dot)com> wrote:
>>> On Sun, Sep 4, 2016 at 1:57 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>>>> On 24 August 2016 at 05:50, Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote:
>>>>
>>>>>>> Everything else looks in good order.
>>>>
>>>> Committed. Thanks.
>>>
>>> Thanks for the commit!
>
> No problem, it was a good patch. Since I moan to others about lack of
> docs, tests etc, I'll do the same here and compliment you on providing
> a well rounded patch with docs, tests that does what it says in a
> clean way.

Thanks a lot! That's really motivating! I don't think I deserve that.

>> By the way, what has been committed does not include the patch adding
>> the parsing context in case of an error as wanted upthread. Perhaps
>> that's not worth adding now as there is the GUC refactoring
>> potentially happening for the recovery parameters, so I don't mind
>> much. Just that's worth mentioning.
>
> Hmm, that was unintentional. If something stalls the recovery
> parameter project, please remind me to commit that as well.

That's what is here as 0002:
https://www.postgresql.org/message-id/CAB7nPqSG5adk7%3DnBgKqy8uQ1tXkeX212jboO_hJbZDVehD3w8Q%40mail.gmail.com
This makes produce a context message should an error occur, like that:
FATAL: invalid input syntax for type pg_lsn: "popo"
CONTEXT: line 11 of configuration file "recovery.conf", parameter
"recovery_target_lsn"

And as outlined by reviewers upthread this is particularly useful for
recovery_target_time and recovery_target_lsn because those would bump
on a parsing error message from the data type, letting users no
information that it came from recovery.conf :(
--
Michael