On Wed, Jun 13, 2012 at 1:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.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