Re: Patch for Improved Syntax Error Reporting - Mailing list pgsql-patches
From | Tom Lane |
---|---|
Subject | Re: Patch for Improved Syntax Error Reporting |
Date | |
Msg-id | 28577.996712729@sss.pgh.pa.us Whole thread Raw |
In response to | Re: Patch for Improved Syntax Error Reporting (Neil Padgett <npadgett@redhat.com>) |
List | pgsql-patches |
Neil Padgett <npadgett@redhat.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
pgsql-patches by date: