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
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. |
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) |