On Fri, Aug 19, 2016 at 10:47 PM, Adrien Nayrat <adrien.nay...@dalibo.com> wrote: > I reviewed this patch rebased to deal with > f6ced51f9188ad5806219471a0b40a91dde923aa, and minor adjustment (see below)
Thanks! > It do the job. However if you use an incorrect recovery_target_lsn you > get this message : > "FATAL: invalid input syntax for type pg_lsn: "0RT5/4"" > > I added a PG_TRY/PG_CATCH section in order to get a more explicit message : > FATAL: wrong recovery_target_lsn (must be pg_lsn type) > > I am not sure if it is the best solution? 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? I'll think about that a bit more and send a new patch. I have switched the patch to "waiting on author" for now, just to not forget about that. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers