Re: Sketch of extending error handling for subtransactions - Mailing list pgsql-hackers

Are you suggesting these changes for 7.5?

---------------------------------------------------------------------------

Tom Lane wrote:
> Currently, the way that ereport/elog processing works is:
> 
> 1. Collect up all the error parameter information into an ErrorData
> structure.  (This is somewhat nontrivial in itself, at least in the
> ereport case, but I'll gloss over that here.)
> 
> 2. Construct a log message and/or a client Error or Notice message
> and send it off to the right places.
> 
> 3. Discard the ErrorData structure.
> 
> 4. If severity level < ERROR, return to caller; if = ERROR,
> longjmp(Warn_restart) to return control to postgres.c; if > ERROR,
> bail out via proc_exit.
> 
> This needs some refactoring if we are going to be able to use nested
> transactions for trapping errors in functions.  In particular, if
> the error is to be trapped then it should not be reported to the client
> (and probably not to the log either, though I suppose that might be
> debatable).  Also control had better not bounce all the way out to
> postgres.c.
> 
> What I propose is that we leave the non-ERROR cases (both NOTICE and
> FATAL branches) acting as above.  In the ERROR case, though, I think
> elog.c should longjmp to Warn_restart just after step 1 is complete,
> without having reported the error anywhere.  In the simple case where
> there is no error trapping involved, the longjmp will still lead back
> to postgres.c, and we will add code there that calls back into elog.c
> to print out and release the ErrorData info (ie, steps 2 and 3 get
> factored out as subroutines that are called from PostgresMain, instead
> of being done directly in errfinish).
> 
> To implement error trapping, the PL languages will change Warn_restart
> to point at their own handlers whenever a TRY-block is running.
> What such a handler would normally do is:
>     * Restore the outer value of Warn_restart.  (This must be
>       done first, to avoid infinite loop if any error happens
>       in the next steps.)
>     * Save off a copy of whatever information it wants about
>       the error.  (Probably we should just provide a function
>       to copy the whole ErrorData structure out of elog.c's
>       private memory context.)
>     * Release elog.c's ErrorData structure (step 3 above).
>       At this point we are out of the critical error-handling
>       code and able to support a new error.
>     * Roll back to the savepoint that was established at entry to
>       the TRY block.  This cleans up the rest of the backend's
>       state to finish recovering from the error.
>     * Execute whatever error-trapping code the user has provided.
>       The copied error information would be available for
>       inspection here.
>     * Release the copied error information when no longer needed.
> 
> This doesn't seem to require any great amount of new code, just
> refactoring of code that exists now.
> 
> One issue is that it may break existing PLs that override Warn_restart,
> since the semantics of doing that will have changed a bit.  We can
> easily fix the PLs that are in our own CVS, but what are the
> implications for other PLs such as PL/R and PL/SH?  Joe, Peter, any
> comments?
> 
> I am somewhat tempted to rename the setjmp variable Warn_restart to
> something else, so as to catch any code that is still expecting the
> old behavior (besides, it was never a very good name anyway).  On the
> other hand, there may be cases where a PL's code doesn't actually need
> to change, and if so a rename would just break it unnecessarily.  Any
> votes which way to jump?
> 
>             regards, tom lane
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 9: the planner will ignore your desire to choose an index scan if your
>       joining column's datatypes do not match
> 

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073