On Sat, Aug 20, 2016 at 10:44 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:
> On 20/08/16 02:13, Michael Paquier wrote:
>> On Fri, Aug 19, 2016 at 10:47 PM, Adrien Nayrat
>> <adrien.nayrat@dalibo.com> wrote:
>> Using a PG_TRY/CATCH block the way you do to show to user a different
>> error message while the original one is actually correct does not
>> sound like a good idea to me. It complicates the code and the original
>> pattern matches what is already done for timestamps, where in case of
>> error you'd get that:
>> FATAL: invalid input syntax for type timestamp with time zone: "aa"
>>
>> I think that a better solution would be to make clearer in the docs
>> that pg_lsn is used here. First, in recovery.conf.sample, let's use
>> pg_lsn and not LSN. Then, in the sgml docs, let's add a reference to
>> the page of pg_lsn as well:
>> https://www.postgresql.org/docs/devel/static/datatype-pg-lsn.html
>>
>> Now we have that:
>> + <para>
>> + This parameter specifies the LSN of the write-ahead log stream up
>> to
>> + which recovery will proceed. The precise stopping point is also
>> + influenced by <xref linkend="recovery-target-inclusive">.
>> + </para>
>> Perhaps we could just add an additional sentence: "This parameter
>> value is parsed using the system datatype <link=pg_lsn>". Or something
>> like that. Thoughts?
>>
>
> +1
So here is the patch resulting in that. After thinking more about that
I have arrived at the conclusion to not use LSN in recovery.conf, but
"WAL position (LSN)", and also mention in the docs that pg_lsn is used
to parse the parameter value. A couple of log messages are changed
similarly. It definitely makes it more understandable for users to
speak of WAL positions.
I also noticed that the top block of "Recovery Target Settings" needs
to mention recovery_target_lsn, aka when name, xid, lsn and time are
combined in the same recovery.conf, only the last value will be used.
--
Michael
