Re: [COMMITTERS] pgsql: Mark JSON error detail messages for translation.

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andres Freund <andres(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org, Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Subject: Re: [COMMITTERS] pgsql: Mark JSON error detail messages for translation.
Date: 2012-06-13 18:09:19
Message-ID: CA+TgmoYPZnPzhW6tjTRz7qYiOXz7thaF049nqjs=cy7FZZhedA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On Wed, Jun 13, 2012 at 1:04 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> I like some of these changes - in particular, the use of errcontext(),
>> but some of them still seem off.
>
>> ! DETAIL:  Token "'" is invalid.
>> ! CONTEXT:  JSON data, line 1: '...
>
>> This doesn't make sense to me.
>
> Well, the input is two single quotes and the complaint is that the first
> one of them doesn't constitute a valid JSON token.  What would you
> expect to see instead?

Oh, I see.

> Another thing that could be done here is to print the rest of the line,
> rather than "...", if there's not very much of it.  I'm not sure that's
> an improvement though.  The advantage of the proposed design is that the
> point where the excerpt ends is exactly where the error was detected;
> lacking an error cursor, I don't see how else to present that info.

OK.

>> ! DETAIL:  Character with value 0x0a must be escaped.
>> ! CONTEXT:  JSON data, line 1: "abc
>> ! ...
>
>> This seems an odd way to present this, especially if the goal is to
>> NOT include the character needing escaping in the log unescaped, which
>> I thought was the point of saying 0x0a.
>
> Do you think it would be better to present something that isn't what the
> user typed?  Again, I don't see an easy improvement here.  If you don't
> want newlines in the logged context, what will we do for something like
>
>        {"foo": {
>                "bar":44
>                }
>        ]
>
> There basically isn't any useful context to present unless we are
> willing to back up several lines, and I don't think it'll be more
> readable if we escape all the newlines.

Hmm. If your plan is to trace back to the opening brace you were
expecting to match, I don't think that's going to work either. What
if there are three pages (or 3MB) of data in between?

> The examples in the regression tests are not really designed to show
> cases where this type of error reporting is an improvement, because
> there's hardly any context around the error sites.

Yeah, true.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2012-06-13 18:55:24 Re: [COMMITTERS] pgsql: Mark JSON error detail messages for translation.
Previous Message Tom Lane 2012-06-13 17:04:31 Re: [COMMITTERS] pgsql: Mark JSON error detail messages for translation.

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2012-06-13 18:42:26 Re: [COMMITTERS] pgsql: Add ERROR msg for GLOBAL/LOCAL TEMP is not yet implemented
Previous Message Andres Freund 2012-06-13 18:01:42 Re: [RFC][PATCH] BDR Prototype startup instructions (not prematurely sent this time)