On 20/08/16 02:13, Michael Paquier wrote:
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)


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:

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?


If we want to specifically name the recovery_target_lsn in the message, we could probably do it using context.

  Petr Jelinek                  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to