elog/ereport syntax difficulties - Mailing list pgsql-hackers

From Tom Lane
Subject elog/ereport syntax difficulties
Date
Msg-id 19836.1051042872@sss.pgh.pa.us
Whole thread Raw
List pgsql-hackers
Awhile ago I proposed that we update elog() calls into a syntax like
   ereport(ERROR, errcode(ERRCODE_INVALID_CURSOR_NAME),           errmsg("portal \"%s\" not found", stmt->portalname),
        ... other errxxx() fields as needed ...);
 

implemented by a macro and some support routines, in the style

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

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

While working on implementing this, I realized that it's not really
going to fly, because the error level is only available to errfinish(),
not to errstart(), errmsg(), etc.  This has one little problem and one
big problem.  The little problem is that errmsg() can't skip formatting
of messages that aren't going to get displayed, which will cost us some
performance for elog(DEBUG) cases.  The big problem is that there's a
risk of losing a fatal error.  Suppose we haveereport(PANIC, errmsg(....))
and during the processing of errmsg() some ERROR condition comes up
(out of memory, say).  The ERROR will get processed, then we'll longjmp
out to the outer loop, and the PANIC condition never gets reported at
all.  Now we have a backend that thinks it's functional but has unknown
corruption in internal state (whatever the PANIC wanted to whinge about).
Very uncool.

We can work around these problems if the elevel is known during
errstart() --- for example, the error-level-degradation problem can be
fixed by looking in the error stack to see if there's a pending FATAL or
PANIC condition.  But the nice clean macro syntax exhibited above
doesn't lend itself to that.

I am thinking that there's little choice but to put an extra set of
parentheses into the ereport usage syntax.  It would look something
like
   ereport(ERROR,           (errcode(ERRCODE_INVALID_CURSOR_NAME),            errmsg("portal \"%s\" not found",
stmt->portalname),           ... other errxxx() fields as needed ...));
 

with an implementation along the lines of

#define ereport(elevel,rest)    \   (errstart(elevel, __FILE__, __LINE__, __func__) ? errfinish rest : 0)

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

(The value of having errstart return a bool is that we can completely
skip the rest of the work if errstart detects that no message need be
displayed.  This is actually better than the current elog
implementation, wherein any functions used in the arguments to elog
must be evaluated even if the message is then discarded.)

The extra parens in the ereport call seem pretty ugly though.  Can
anyone think of a nicer way?
        regards, tom lane



pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: There are advantages to having an odd name
Next
From: "scott.marlowe"
Date:
Subject: Re: Are we losing momentum?