Re: pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> 1. Just run the whole business in the caller's context and leave it up
> to the caller to worry about whether it needs to clean up memory usage.

I'd certainly be fine with that, and had considered it, but it looks
like errcontext_msg() (which is called by the callbacks through the
errcontext() macro) switches to ErrorContext for its work, meaning we
have to clean up that context anyway. In my initial review I hadn't
considered the possibility of modifying what ErrorContext is pointing
to. As the error system may end up getting called by a callback
function, it seems like changing the global ErrorContext would be a bad
idea. Forgive me if I'm missing something obvious in how we could
simply use the caller's context for this as I'd surely be much happier
with that.

> 2. Create a temporary context underneath CurrentMemoryContext, use that,
> and then delete it when done.

That'd be fine with me also, but runs the same issue as above.

> The only thing that needs to be considered an error condition is if the
> errordata stack is too full to allow creation of the extra entry needed
> by this function, which is an improbable situation. Other than
> temporarily stacking and then unstacking that entry, you don't need to
> have any side-effects on the state of the error subsystem.

If we can address the memory context issue in a simple way then I'll
certainly set this up to be reentrant safe and to handle pushing and
popping itself on the errordata stack.

> One other minor gripe is that the function is documented as returning
> the "call stack", which a C programmer would think means something
> entirely different from what is actually output. I think you need to
> use a different phrase there. "error context stack" would be OK.

Works for me. I had already tried to reword it to address that exact
issue, but "error context stack" does sound better than "PG call stack."

Thanks!

Stephen