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 17:49:09 |
Message-ID: | 20130725174909.GO15510@tamriel.snowman.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers pgsql-hackers |
* 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
From | Date | Subject | |
---|---|---|---|
Next Message | Stephen Frost | 2013-07-25 17:54:15 | Re: pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL |
Previous Message | Pavel Stehule | 2013-07-25 17:07:01 | Re: pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL |
From | Date | Subject | |
---|---|---|---|
Next Message | Stephen Frost | 2013-07-25 17:54:15 | Re: pgsql: Add GET DIAGNOSTICS ... PG_CONTEXT in PL/PgSQL |
Previous Message | David Fetter | 2013-07-25 17:33:54 | Re: Review: UNNEST (and other functions) WITH ORDINALITY |