On 23/08/16 09:33, Michael Paquier wrote:
> On Tue, Aug 23, 2016 at 12:49 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>> On Mon, Aug 22, 2016 at 8:28 AM, Michael Paquier
>> <michael.paquier@gmail.com> wrote:
>>> On Mon, Aug 22, 2016 at 7:12 PM, Adrien Nayrat <adrien.nayrat@dalibo.com> wrote:
>>>> As Julien said, there is nothing to notice that error comes from
>>>> recovery.conf.
>>>> My fear would be that an user encounters an error like this. Il will be
>>>> difficult to link to the recovery.conf.
>>>
>>> Thinking a bit wider than that, we may want to know such context for
>>> normal GUC parameters as well, and that's not the case now. Perhaps
>>> there is actually a reason why that's not done for GUCs, but it seems
>>> that it would be useful there as well. That would give another reason
>>> to move all that under the GUC umbrella.
>>
>> Maybe so, but that's been tried multiple times without success. If
>> you think an error context is useful here, and I bet it is, I'd say
>> just add it and be done with it.
>
> This has finished by being less ugly than I thought, so I implemented
> it as attached. Patch 0001 introduces recovery_target_lsn, and patch
> 0002 sets up an error context callback generating things like that on
> failure:
> FATAL: invalid input syntax for type pg_lsn: "popo"
> CONTEXT: line 11 of configuration file "recovery.conf", parameter
> "recovery_target_lsn"
>
Looks very reasonable to me (both patches). Thanks for doing that.
I am inclined to mark this as ready for committer.
-- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services
