From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org, Robert Haas <rhaas(at)postgresql(dot)org> |
Subject: | Re: [COMMITTERS] pgsql: Mark JSON error detail messages for translation. |
Date: | 2012-06-12 18:40:52 |
Message-ID: | 20494.1339526452@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers pgsql-hackers |
Robert Haas <rhaas(at)postgresql(dot)org> writes:
> Mark JSON error detail messages for translation.
> Per gripe from Tom Lane.
OK, that was one generically bad thing about json.c's error messages.
The other thing that was bothering me was the style of the errdetail
strings, eg
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
errmsg("invalid input syntax for type json"),
errdetail("line %d: Invalid escape \"\\%s\".",
lex->line_number, extract_mb_char(s))));
I'm not thrilled with the "line %d:" prefixes. That seems to me to
violate the letter and spirit of the guideline about making errdetail
messages be complete sentences. Furthermore, the line number is not
the most important part of the detail message, if indeed it has any
usefulness at all which is debatable. (It's certainly not going to
be tremendously useful when the error report doesn't show any of the
specific input string being complained of.)
Also, a minority of the errors report the whole input string:
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
errmsg("invalid input syntax for type json: \"%s\"",
lex->input),
errdetail("The input string ended unexpectedly.")));
And then there are some that provide the whole input string AND a line
number, just in case you thought there was any intentional design
decision there.
A minimum change IMO would be to recast the detail messages as complete
sentences, say "The escape sequence \"\\%s\" in line %d is invalid."
But I'd like a better-thought-out solution to identifying the location
of the error.
I notice that there's an unfinished attempt to maintain a line_start
pointer; if that were carried through, we could imagine printing the
current line up to the point of an error, which might provide a
reasonable balance between verbosity and insufficient context.
As an example, instead of
regression=# select '{"x":1,
z "y":"txt1"}'::json;
ERROR: invalid input syntax for type json
LINE 1: select '{"x":1,
^
DETAIL: line 2: Token "z" is invalid.
maybe we could print
regression=# select '{"x":1,
z "y":"txt1"}'::json;
ERROR: invalid input syntax for type json
LINE 1: select '{"x":1,
^
DETAIL: Token "z" is invalid in line 2: "z".
or perhaps better let it run to the end of the line:
regression=# select '{"x":1,
z "y":"txt1"}'::json;
ERROR: invalid input syntax for type json
LINE 1: select '{"x":1,
^
DETAIL: Token "z" is invalid in line 2: "z "y":"txt1"}".
Thoughts?
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2012-06-12 18:49:04 | Re: [COMMITTERS] pgsql: Mark JSON error detail messages for translation. |
Previous Message | Robert Haas | 2012-06-12 18:40:09 | Re: Re: [COMMITTERS] pgsql: Run pgindent on 9.2 source tree in preparation for first 9.3 |
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2012-06-12 18:42:49 | Re: WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink |
Previous Message | Robert Haas | 2012-06-12 18:40:09 | Re: Re: [COMMITTERS] pgsql: Run pgindent on 9.2 source tree in preparation for first 9.3 |