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

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 17:56:56
Message-ID: 22178.1374775016@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:
> * 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.

Yeah, I was just noticing that. We'd have to change that ...

> 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.

Yeah, that'd be a nonstarter. I thought about simply not doing
MemoryContextSwitchTo in errcontext_msg(), which would work for ordinary
usage in error context callbacks, because those are run in ErrorContext
anyway. However, there are some call sites that use errcontext() like a
regular ereport helper, for instance in dblink, and we'd end up with the
context string in the wrong memory context in that case. What seems
like a better idea is to add a memory-context-to-use field to struct
ErrorData. We'd initialize it to ErrorContext when pushing a stack
entry for normal error processing, but GetErrorContextStack could
do something different. This would eliminate most of the explicit
references to ErrorContext in elog.c.

If you want I'll draft something up.

regards, tom lane

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Pavel Stehule 2013-07-25 18:02:30 Re: pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL
Previous Message Stephen Frost 2013-07-25 17:54:15 Re: pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2013-07-25 18:02:30 Re: pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL
Previous Message Stephen Frost 2013-07-25 17:54:15 Re: pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL