Tom,
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> I'm not following your reasoning here. This *has* to be called in an
> error case, before you're outside the error processing context.
> Otherwise there would be no data available to be printed.
>
> In short: FlushErrorState, by definition, destroys the information that
> GetErrorContextStack looks at. So in the current implementation,
> GetErrorContextStack is burning its bridges behind it. That's at the
> very least a surprising behavior. I am betting that it will have
> unpleasant consequences for any sort of nested-error scenario.
I've just pushed up some much needed improvements to the comments which
hopefully clarify that this function is using error_context_stack and
calling the callbacks set up there by callers above on the PG call
stack. Also, hopefully this makes it clear that errrordata is required
to be empty when this function is called and therefore it can be cleaned
up when exiting with FlushErrorState.
Perhaps it would be better to try and work out a way for this to be
reentrant safe and be callable from an exception handler, but it
certainly wasn't part of the original intent and being able to support
either being called under an exception handler or not would essentially
require checking if anything is above us on the stack and, if not,
cleaning things up anyway, or trusting the above caller to handle it and
skipping it.
I'm happy to rework this or even revert it if this use of the
error_context_stack is just too grotty, but this certainly looks like a
useful capability to have.
Thanks!
Stephen
