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

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "pgsql-committers(at)postgresql(dot)org" <pgsql-committers(at)postgresql(dot)org>
Subject: Re: pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL
Date: 2013-07-25 14:02:38
Message-ID: 20130725140238.GN15510@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

Tom,

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)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

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Bruce Momjian 2013-07-25 15:33:22 pgsql: pg_upgrade: adjust umask() calls
Previous Message Stephen Frost 2013-07-25 13:51:37 pgsql: Improvements to GetErrorContextStack()

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2013-07-25 14:57:28 pg_upgrade -j broken on Windows
Previous Message Tom Lane 2013-07-25 13:13:10 Re: Expression indexes and dependecies