From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Stephen Frost <sfrost(at)snowman(dot)net> |
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 03:12:00 |
Message-ID: | 26144.1374721920@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers pgsql-hackers |
Stephen Frost <sfrost(at)snowman(dot)net> writes:
> On Wednesday, July 24, 2013, Tom Lane wrote:
>> I don't find it to be a terribly good idea that GetErrorContextStack
>> does FlushErrorState().
> It's not intended (nor allowed, if I got it right) in an error context. I
> will admit that it's not a wonderful situation to have it using the error
> handling's internal components like this, but that's also where it has to
> go for the callbacks to get the information needed.
Sure. What I'm complaining about is that the function has side-effects
on the state of the error handler, which is surprising, undocumented,
and IMHO unsafe.
I think it should do its work in the caller's context, never mind
whether the errcontext callbacks leak a bit of memory; and it should
absolutely NOT be calling FlushErrorState afterwards. That should be
done by the caller when it's done with error processing.
> My concerns and thoughts around this were what may happen if a callback
> throws an error and it still makes me a bit nervous but It seems like it
> should work.
Right. In particular, what if we run out of memory while trying to
build the context string? That's not going to be a good situation in
any case, but I think this design turns it into a worst-case scenario.
It'd be better to be building the string outside the error handler's
reserved space.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Stephen Frost | 2013-07-25 03:13:31 | Re: pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL |
Previous Message | Stephen Frost | 2013-07-25 03:03:41 | Re: pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL |
From | Date | Subject | |
---|---|---|---|
Next Message | Stephen Frost | 2013-07-25 03:13:31 | Re: pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL |
Previous Message | Stephen Frost | 2013-07-25 03:03:41 | Re: pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL |