On Wed, Jun 13, 2012 at 12:27 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Wed, Jun 13, 2012 at 11:55 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> In any case, the proposed scheme for providing context requires that
>>> you know where the error is before you can identify the context. I
>>> considered schemes that would keep track of the last N characters or
>>> line breaks in case one of them proved to be the one we need. That
>>> would add enough cycles to non-error cases to almost certainly not be
>>> desirable. I also considered trying to back up, but that doesn't look
>>> promising either for arbitrary multibyte encodings.
>
>> Oh, I see. :-(
>
> Attached is a complete proposed patch for this, with some further
> adjustments to make the output look a bit more like what we use for
> SQL error context printouts (in particular, "..." at both ends of the
> excerpt when appropriate).
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.
! 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.
! DETAIL: "\u" must be followed by four hexadecimal digits.
! CONTEXT: JSON data, line 1: "\u000g...
Why does this end in ... even though there's nothing further in the
input string?
> One thing I did not touch, but am less than happy with, is the wording
> of this detail message:
>
> errdetail("Expected string, number, object, array, true, false, or null, but found \"%s\".",
> token),
>
> This seems uselessly verbose to me. It could be argued that enumerating
> all the options is helpful to rank JSON novices ... but unless you know
> exactly what an "object" is and why that's different from the other
> options, it's not all that helpful. I'm inclined to think that
> something like this would be better:
>
> errdetail("Expected JSON value, but found \"%s\".",
> token),
>
> Thoughts?
Good idea.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company