Re: Patch for Improved Syntax Error Reporting

Neil Padgett <npadgett(at)redhat(dot)com> writes:
> This is exactly what I want. If you don't have a new client, it looks
> like a message with some funk on the end. If you have a new, friendly
> client, it will strip out the field/value list at the end. Exactly the
> same as the multi-line list, really. And the translation complaint is
> equally applicable to either format.

Agreed --- as long as the *only* thing you want to add is a syntax error
location, that way would be better. But it doesn't scale...

>> --- distinguishing the actual error message from tips/hints about what
>> to do about it. There are a fair number of these already, and right
>> now there's just a very weak formatting convention that hints
>> appear on a separate line.

> I didn't know that such a convention exists already -- how would these
> hints look under your proposed new format?

Well, it's a pretty weak convention, but here are a couple of examples
of what I'm talking about:

elog(ERROR, "_bt_getstackbuf: my bits moved right off the end of the world!"
"\n\tRecreate index %s.", RelationGetRelationName(rel));

elog(ERROR, "Left hand side of operator '%s' has an unknown type"
"\n\tProbably a bad attribute name", op);

elog(ERROR, "Unable to identify a %s operator '%s' for type '%s'"
"\n\tYou may need to add parentheses or an explicit cast",
(is_left_op ? "left" : "right"),
op, typeidTypeName(arg));

In all these cases, I'd call the added-on lines hints --- they're not
part of the basic error message, and the hint may not be applicable
to your particular situation. Without wanting to start an argument
as to the validity of these particular hints, I do think that it'd
be a good idea to distinguish them from the primary error message.
In the first example, where I'd like to see us end up is something
like:

ERROR: Internal error, please report to pgsql-bugs
DETAIL: my bits moved right off the end of the world!
HINT: Recreate index foo
CODELOCATION: _bt_getstackbuf: src/backend/access/nbtree/nbtinsert.c, line 551

I'm not wedded to these particular keywords, but hopefully this will
serve as an illustration that we're cramming a lot of stuff into an
error message already. Breaking it out into fields could render it
more intelligible, not less so --- and with an updated client, a user
could choose not to look at the info he doesn't want. Right now he
has no real prospect of suppressing unwanted info. An example is the
source routine name that we cram into many messages, as a poor man's
substitute for accurate error location info. That's pretty much useless
to non-hackers, and ought to be moved out to a secondary field.

> Why aren't we using numerics to do this?

Why, thanks for reminding me. Adding a standardized error code (not
message) that client programs could interpret is another thing that
is on the TODO list. Seems like another application for a separable
field of an error report. I think we should keep such codes separate
from the (localizable) message text, however. Peter E. made some cogent
arguments that trying to drive localization off error numbers would be
a losing proposition, as opposed to using gettext().

> Would the elog call be changed to support passing in a list of
> arguments?

That hadn't really been decided, but clearly some change of the elog
API will be needed. I think there is more about this in the archives
than you will find in TODO.detail/elog.

> We should probably introduce
> a new call, say, eelog (for enhanced error log) that takes such a list,
> and then we could define elog as a macro which calls eelog with suitable
> defaults for use with "legacy" messages. Then, we wouldn't need to go
> after every error message right away.

Yeah, the $64 question is how to avoid needing a "big bang" changeover
of all the elog calls. Even if we wanted to try that, it'd be a
continuing headache for user-added datatypes and such. I'd like to be
able to leave the existing elog() API in place for a few releases, if
possible.

> The question this brings up is whether a logging change can / should be
> tackled in this release. Specifically, with the current state of
> internationalization work, is it best to do it now, or later?

I'm still pointing towards 7.2 beta near the end of the month, which
would be a mighty tight schedule for anything ambitious in elog rework.
On the other hand, there's no harm in working out a design now with
the intention of starting to implement it in the 7.3 cycle.

regards, tom lane