Обсуждение: Upgrading the backend's error-message infrastructure

(Or, protocol upgrade phase 1...)

After digging through our many past discussions of what to do with error
messages, I have put together the following first-cut proposal.  Fire at
will...


Objective
---------

The basic objective here is to divide error reports into multiple
fields, and in particular to include an "error code" field that gives
applications a stable value to test against when they're trying to find
out what went wrong.  (I am not spending much space in this proposal on
the question of exactly what set of error codes we ought to have, but
that comes soon.)  Peter Eisentraut argued cogently awhile back that the
error codes ought not be hard-wired to specific error message texts,
so this proposal treats them as separate entities.


Wire-protocol changes
---------------------

Error and Notice (maybe also Notify?) msgs will have this structure:
Ex string \0x string \0x string \0\0

where the x's are single-character field identifiers.  A frontend should
simply ignore any unrecognized fields.  Initially defined fields for Error
and Notice are:

S    Severity --- the string is "ERROR", "FATAL", or "PANIC" (if E msg)or "WARNING", "NOTICE", "DEBUG", "INFO", or
"LOG"(if N msg).(Should this string be localizable?  Probably, assuming that theE/N distinction is all the client
libraryreally cares about.)
 
C    Code --- SQLSTATE code for error (a 5-character string per SQLspec).  Not localizable.
M    Message --- the string is the primary error message (localized).
D    Detail --- secondary error message, carrying more detail aboutthe problem (localized).
H    Hint --- a suggestion what to do about the error (localized).
P    Position --- the string is a decimal ASCII integer, indicatingan error cursor position as an index into the
originalquerystring.  First character is index 1.  Q: measure index inbytes, or characters?  Latter seems preferable
consideringthatan encoding conversion may have occurred.
 
F    File --- file name of source-code location where error wasreported (__FILE__)
L    Line # --- line number of source-code location (__LINE__)
R    Routine --- source code routine name reporting error (__func__ or__FUNCTION__)

S,C,M fields will always appear (at least in Error messages; perhaps
Notices might omit C?).  The rest are optional.

Why three textual message fields?  'M' should always appear, 'D' and 'H'
are optional (and relatively rare).  The convention is that the primary
'M' message should be accurate but terse (normally one line); if more info
is needed than can reasonably fit on a line, use the detail message to
carry additional lines.  A "hint" is something that doesn't directly
describe the error, but is a suggestion what to do to get around it.
'M' and 'D' should be factual, whereas 'H' may contain some guesswork, or
advice that might not always apply.  Client interfaces are expected to
report 'M', but might suppress 'D' and/or 'H' depending on factors such as
screen space.  (Preferably they should have a verbose mode that shows all
available info, though.)


Error codes
-----------

The SQL spec defines a set of 5-character status codes (called SQLSTATE
values).  We'll use these as the language-independent identifiers for
error conditions.  There is code space reserved by the spec for
implementation-defined error conditions, which we'll surely need.

Per spec, each of the five characters in a SQLSTATE code must be a digit
'0'-'9' or an upper-case Latin letter 'A'-'Z'.  So it's possible to fit a
SQLSTATE code into a 32-bit integer with some simple encoding conventions.
I propose that we use such a representation in the backend; that is,
instead of passing around strings like "1200D" we pass around integers
formed like ((('1' - '0') << 6) + '2' - '0') << 6 ...  This should save
a useful amount of space per elog call site, and it won't obscure the code
noticeably since all the common values will be represented as macro names
anyway, something like

#define ERRCODE_DIVISION_BY_ZERO   MAKE_SQLSTATE('2','2', '0','1','2')

We need to do some legwork to figure out what set of
implementation-defined error codes we want.  It might make sense to look
and see what other DBMSes are using.


Backend source-code representation for extended error messages
--------------------------------------------------------------

How do we generalize the elog() interface to cope with all this stuff?
I don't think I want a function with a fixed parameter list --- some sort
of open-ended API would be a lot more forward-looking.  After some fooling
around I've come up with the following proposal.

A typical elog() call might be replaced by
ereport(ERROR, ERRCODE_INTERNAL,    errmsg("Big trouble with table %s", name),    errhint("Bail out now, boss"));

ERROR is the severity level, same as before, and ERRCODE_xxx is (a macro
for) the appropriate SQLSTATE code.  The rest is a variable-length list of
optional items, each expressed as a subsidiary function call.  This
representation preserves the single-function-call appearance of elog()
calls, which is convenient for coding purposes, but it gives us something
akin to labeled optional parameters instead of C's usual fixed parameter
list.

How does this work, exactly?  Well, errmsg() and errhint() are indeed
functions, but ereport is actually a macro:

#define ereport    errstart(__FILE__, __LINE__, __FUNCTION__), errfinish

(__FUNCTION__ is only used if we are compiling in gcc).  errstart() pushes
an empty entry onto an error-data-collection stack and fills in the
behind-the-scenes file/line entries.  errmsg() and friends stash values
into the top-level stack entry.  Finally errfinish() assembles and emits
the completed message, then pops the stack.  By using a stack, we can be
assured that things will work correctly if a message is logged by some
subroutine called in the parameters to ereport (not too unlikely when you
think about formatting functions like format_type_be()).

Behind the scenes we have

extern void errstart(const char *filename, int lineno, const char *funcname);
extern void errfinish(int elevel, int sqlerrorcode, ...);

The individual routines for adding optional items to the error report are:

extern int errmsg(const char *fmt, ...);

Primary error message, possibly with parameters interpolated per the
existing elog conventions (sprintf-like format string).  The first
parameter is gettext-ified.  Primary messages should be one line if at
all possible (make it complete but succinct).

extern int errdetail(const char *fmt, ...);

Adds an optional secondary error message, for use when not all the
description of an error condition can be fit into a reasonably terse
primary error message.  Functionality essentially the same as errmsg().
errdetail output can run to multiple lines, but bear in mind that some
client APIs may not show it.

extern int errhint(const char *fmt, ...);

Adds a "hint"; behavior otherwise similar to errdetail().

An example is that the existing       elog(ERROR, "Adding columns with defaults is not implemented."
"\n\tAddthe column, then use ALTER TABLE SET DEFAULT.");
 
becomesereport(ERROR, ERRCODE_something,        errmsg("Adding columns with defaults is not implemented"),
errhint("Addthe column, then use ALTER TABLE SET DEFAULT"));
 
Notice that we got rid of a hard-wired decision about presentation layout.

extern int errmsg_internal(const char *fmt, ...);

Like errmsg() except that the first parameter is not subject to
gettext-ification.  My thought is that this would be used for internal
can't-happen conditions; there's no need to make translators labor over
translating stuff like "eval_const_expressions: unexpected boolop %d",
nor even to make them think about whether they need to.  The only part
of such a message that needs internationalization is the hint "Please
report this problem to pgsql-bugs", which should be added automatically
by errmsg_internal().  The ERRCODE should almost always be "internal
error" if this is used.

extern int errfunction(const char *funcname);

Provides the name of the function reporting the error.  In gcc-compiled
backends, the function name will be provided automatically by errstart,
but there will be some places where we need the name to be available even
in a non-gcc build.  My thought is thatelog(WARNING, "PerformPortalFetch: portal \"%s\" not found",
stmt->portalname);
becomesereport(WARNING, ERRCODE_INVALID_CURSOR_NAME,        errmsg("portal \"%s\" not found", stmt->portalname),
errfunction("PerformPortalFetch"));
This gets us out of the habit of including function name in the primary
error message, while still leaving enough info that we can construct
a backwards-compatible error report for old clients.  (I'm thinking that
if errfunction() is present, the function name and a colon would be
prepended to the primary error message, but only if sending to an
old-protocol client.)

extern int errposition(int cursorpos);

Provides error position info (an offset into the original query text).
For the moment this is probably only going to happen for scanner and
grammar errors.


NOTE: a variant scheme would treat the SQLSTATE code as an optional
parameter too, ie you'd writeereport(ERROR, errcode(ERRCODE_xxx), ...
This would just be excess verbiage if most or all ereport calls specify
error codes --- but for the errmsg_internal case we could leave out
errcode(), expecting it to default to "internal error".  Any thoughts on
which way is better?


Backwards compatibility
-----------------------

When talking to an old-protocol client, the ereport code will assemble the
appropriate elements of the available data to produce an approximately
backward-compatible message, that is, ye oldeERROR:  routine: primary message
(where routine: appears only if errfunction() was called).

elog() will remain available for at least a couple of releases, so as not
to force immediate updates of user-written extension functions.  It will
default to some implementation-defined SQLSTATE value for "unspecified
error".  We can change "elog" into a macro similar to "ereport" so that we
can get file/line number info.  (This means we're only giving source-code
not object-code compatibility with extension functions, but that's
generally the case anyway during PG major version updates.)


Comments?
        regards, tom lane

Larry Rosenman <ler@lerctr.org> writes:
> __FUNCTION__ or an equivalent is MANDATED by C99, and available on 
> UnixWare's native cc.
> You might want to make a configure test for it.

Right, __func__ is the C99 spelling.  I did have a configure test in
mind here: __func__ or __FUNCTION__ or NULL is what would get compiled
in.  One nice thing about this approach is that we need change only one
place to adjust the set of behind-the-scenes error parameters.
        regards, tom lane

Why trade 5 characters for a 4 byte integer -- a saving of 1 byte?
Is this database designed for embedded applications where RAM is scarce?
:)

JLL

Tom Lane wrote:
[...]
> Error codes
> -----------
> 
> The SQL spec defines a set of 5-character status codes (called SQLSTATE
> values).  We'll use these as the language-independent identifiers for
> error conditions.  There is code space reserved by the spec for
> implementation-defined error conditions, which we'll surely need.
> 
> Per spec, each of the five characters in a SQLSTATE code must be a digit
> '0'-'9' or an upper-case Latin letter 'A'-'Z'.  So it's possible to fit a
> SQLSTATE code into a 32-bit integer with some simple encoding conventions.
> I propose that we use such a representation in the backend; that is,
> instead of passing around strings like "1200D" we pass around integers
> formed like ((('1' - '0') << 6) + '2' - '0') << 6 ...  This should save
> a useful amount of space per elog call site, and it won't obscure the code
> noticeably since all the common values will be represented as macro names
> anyway, something like
> 
> #define ERRCODE_DIVISION_BY_ZERO   MAKE_SQLSTATE('2','2', '0','1','2')
> 
> We need to do some legwork to figure out what set of
> implementation-defined error codes we want.  It might make sense to look
> and see what other DBMSes are using.
> 
[...]

Neil Conway <neilc@samurai.com> writes:
> While we're effectively changing every elog call site in the backend,
> would it also be a good idea to adopt a standard for the format of error
> messages? (e.g. capitalization, grammar, etc.)

Yup.  I was planning to bring that up as a separate thread.  I think
Peter has already put some thought into it, but I couldn't find anything
in the archives...

> If we wanted to get fancy, we could make use of the glibc ability to
> generate a back trace programatically:

Hmm ... maybe.  Certainly we all too often ask people to get this info
by hand ... too bad it only works in glibc though.

>> In gcc-compiled
>> backends, the function name will be provided automatically by errstart,
>> but there will be some places where we need the name to be available even
>> in a non-gcc build.

> To be honest, I'd be sceptical whether there are enough platforms
> without *either* gcc or a C99 compiler that it's worthwhile worrying
> about them that much (all that is at stake is some backward
> compatibility, anyway).

I'm only planning to bother with the errfunction hack for messages that
I know are being specifically tested for by existing frontends.  ecpg
looks for "PerformPortalFetch" messages, for example.  If we don't keep
that name in the (old version of the) error message then we have a
compatibility problem.  But I do want to move away from having function
names in the primary error message text.
        regards, tom lane

Jean-Luc Lachance <jllachan@nsd.ca> writes:
> Why trade 5 characters for a 4 byte integer -- a saving of 1 byte?

It's more than that: in one case you have something on the order of
a "load immediate" instruction, whereas in the other case the code
is like "load pointer to global string", plus you need a 6-byte string
literal (maybe costing you 8 bytes depending on alignment
considerations).  Also, depending on your machine's approach to
addressing of global data, that "load pointer" thingy could be multiple
instructions.  So we're talking about at least six, possibly 8-12 bytes
per elog call --- and there are thousands of 'em in the backend.

Admittedly, it's a micro-optimization, but it seems worth doing since it
won't have any direct impact on code legibility.
        regards, tom lane

Tom Lane writes:

> Error and Notice (maybe also Notify?) msgs will have this structure:
>
>     E
>     x string \0
>     x string \0
>     x string \0
>     \0
>
> where the x's are single-character field identifiers.

I think we need more flexible field tags.  The SQL standards has
provisions for more fields accompanying error messages, such as the name
of the affected table.  (See <condition information item name> for a the
list.)  I think it would be nice if applications could easily access, say,
the name of the constraint that was violated.

> NOTE: a variant scheme would treat the SQLSTATE code as an optional
> parameter too, ie you'd write
>     ereport(ERROR, errcode(ERRCODE_xxx), ...
> This would just be excess verbiage if most or all ereport calls specify
> error codes --- but for the errmsg_internal case we could leave out
> errcode(), expecting it to default to "internal error".  Any thoughts on
> which way is better?

I have a feeling that most errors are of the "internal" class, either
those that are really a kind of assertion check (perhaps we should
consider an enhanced API for those in the future) or failed system or
library calls.  We would need to analyze that feeling a little more, but
if it's true then we might save some effort if the default error code
were "internal".

Then again, it might seem better if the default error code were closer in
nature to "default", meaning an unspecified error if the programmer
couldn't think of one (consider loadable modules).

Speaking of loadable modules, one feature that would be useful would be
the ability to select a different message catalog for translations.
Right now an elog(ERROR, "foo") call in a loaded module looks up "foo" in
the message catalog provided by the main server but it probably won't be
there.  This could look like

ereport(ERROR, errmsgdomain("plpgsql"), "...")

or maybe

ereport_domain("plpgsql", ERROR, ...);

-- 
Peter Eisentraut   peter_e@gmx.net

Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane writes:
>> where the x's are single-character field identifiers.

> I think we need more flexible field tags.  The SQL standards has
> provisions for more fields accompanying error messages, such as the name
> of the affected table.

Well, we can certainly add more tags, but do you foresee needing more
than 50 or so?  I'd prefer to stick to single-byte tags for space
reasons, even if they stop being very mnemonic at some point.  As long
as we don't run out of printable ASCII characters, it's easy on both the
sending and receiving sides to cope with single-byte tags.

I had missed the relevance of <condition information item name>, will
go look at it.

>> NOTE: a variant scheme would treat the SQLSTATE code as an optional
>> parameter too, ie you'd write
>> ereport(ERROR, errcode(ERRCODE_xxx), ...

> I have a feeling that most errors are of the "internal" class, either
> those that are really a kind of assertion check (perhaps we should
> consider an enhanced API for those in the future) or failed system or
> library calls.  We would need to analyze that feeling a little more, but
> if it's true then we might save some effort if the default error code
> were "internal".

Yeah, that was in the back of my mind too, but I hadn't got round to
counting to see if it's right.

> Then again, it might seem better if the default error code were closer in
> nature to "default", meaning an unspecified error if the programmer
> couldn't think of one (consider loadable modules).

Unconverted elog's will produce some such SQLSTATE, but if we can't
think of a better SQLSTATE for ones we *have* converted then I think we
need to think harder ;-)

This brings up something I had wanted to start a separate thread for,
which is exactly what SQLSTATEs do we want to define beyond those given
in the spec.  Any thoughts?

> Speaking of loadable modules, one feature that would be useful would be
> the ability to select a different message catalog for translations.
> Right now an elog(ERROR, "foo") call in a loaded module looks up "foo" in
> the message catalog provided by the main server but it probably won't be
> there.  This could look like

> ereport(ERROR, errmsgdomain("plpgsql"), "...")

Rather than cluttering the individual ereport calls with such a thing,
can't a loadable module just do something when it is loaded to add its
message catalog file to the set that will be searched?  (But it is
interesting that we *could* do it as you suggest.  This mechanism is
more flexible than I realized...)
        regards, tom lane

I said:
> I had missed the relevance of <condition information item name>, will
> go look at it.

It looks to me like support of the SQL condition information items would
require adding about two dozen optional fields to my spec for the Error
protocol message, and the same number of optional errFOO(...)
subroutines in the ereport() interface (only two or three of which would
be likely to get invoked in any one ereport instance).  This is a bit
more than I'd been visualizing, but AFAICS the proposed mechanisms would
work perfectly well with it.  I won't bore the list with a detailed spec
for the individual items --- they seem pretty obvious.

Given that we now need order-of-thirty possible field types, do you feel
uncomfortable with a single-byte field identifier in the FE/BE protocol?
I'm still leaning that way on the grounds of compactness and programming
simplicity, but I can see where someone might want to argue it won't do
in the long run.
        regards, tom lane

Tom Lane writes:

> Given that we now need order-of-thirty possible field types, do you feel
> uncomfortable with a single-byte field identifier in the FE/BE protocol?
> I'm still leaning that way on the grounds of compactness and programming
> simplicity, but I can see where someone might want to argue it won't do
> in the long run.

There's a possible solution:  SQL99 part 3 defines numerical codes for
each of these fields (table 12/section 5.14).  The codes are between
around 0 and 40.  (Don't be confused by the negative code numbers in the
table; those are only for use within ODBC.)

-- 
Peter Eisentraut   peter_e@gmx.net

Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane writes:
>> Given that we now need order-of-thirty possible field types, do you feel
>> uncomfortable with a single-byte field identifier in the FE/BE protocol?

> There's a possible solution:  SQL99 part 3 defines numerical codes for
> each of these fields (table 12/section 5.14).  The codes are between
> around 0 and 40.

Hmm.  I can't see any advantage to these over assigning our own codes;
ours would have at least *some* mnemonic value, rather than being chosen
completely at random ...
        regards, tom lane

Tom Lane writes:

> Hmm.  I can't see any advantage to these over assigning our own codes;
> ours would have at least *some* mnemonic value, rather than being chosen
> completely at random ...

One advantage is that interfaces that are required to use these constants
would not need an internal translation table.

-- 
Peter Eisentraut   peter_e@gmx.net

Tom Lane writes:

> M    Message --- the string is the primary error message (localized).
> D    Detail --- secondary error message, carrying more detail about
>     the problem (localized).
> H    Hint --- a suggestion what to do about the error (localized).

Client interfaces for the most part only have the notion of a single
"message text".  (And keep in mind that the definitions of most interfaces
are outside our control: JDBC, ODBC, ECPG, Perl DBI, PHP, etc.)  So what
kind of functionality is needed so that standardized interfaces can get at
any of the provided details and hints?

Maybe this doesn't need to be solved at the protocol layer.  Instead a
server-side switch regulates the detail of the provided messages.

Also, how do we control what amount of detail is written to the server
log?

-- 
Peter Eisentraut   peter_e@gmx.net

Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane writes:
>> M    Message --- the string is the primary error message (localized).
>> D    Detail --- secondary error message, carrying more detail about
>> the problem (localized).
>> H    Hint --- a suggestion what to do about the error (localized).

> Client interfaces for the most part only have the notion of a single
> "message text".  (And keep in mind that the definitions of most interfaces
> are outside our control: JDBC, ODBC, ECPG, Perl DBI, PHP, etc.)  So what
> kind of functionality is needed so that standardized interfaces can get at
> any of the provided details and hints?

I think this is a matter to be solved at the level of the API of each
client library.  For example, libpq's PQerrorMessage would presumably
construct some unified string out of these three fields and the error
severity; plus we'd add new calls to extract the individual fields.
I do not think it's appropriate to try to control this from the server
side of things.

> Also, how do we control what amount of detail is written to the server
> log?

Some GUC variables would do for that, probably, if we think it's a good
idea to be selective (a proposition I'm dubious about).
        regards, tom lane