Re: [COMMITTERS] pgsql: Mark JSON error detail messages for translation. - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [COMMITTERS] pgsql: Mark JSON error detail messages for translation.
Date
Msg-id 20494.1339526452@sss.pgh.pa.us
Whole thread Raw
Responses Re: [COMMITTERS] pgsql: Mark JSON error detail messages for translation.  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
Robert Haas <rhaas@postgresql.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
syntaxfor 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


pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: Re: [COMMITTERS] pgsql: Run pgindent on 9.2 source tree in preparation for first 9.3
Next
From: Robert Haas
Date:
Subject: Re: WIP patch for Todo Item : Provide fallback_application_name in contrib/pgbench, oid2name, and dblink