Thread: A suggestion on PG_TRY() exception handling code.

Hi hackers,<br /><br />    The current usage PG_TRY() looks something like this:<br /><br /><span style="font-family:
couriernew,monospace;">    ... normal code ...<br /><br />    PG_TRY();</span><br style="font-family: courier
new,monospace;"/><span style="font-family: courier new,monospace;">    {</span><br style="font-family: courier
new,monospace;"/><span style="font-family: courier new,monospace;">        ... code that might throw ereport(ERROR)
...</span><brstyle="font-family: courier new,monospace;" /><span style="font-family: courier new,monospace;">   
}</span><brstyle="font-family: courier new,monospace;" /><span style="font-family: courier new,monospace;">   
PG_CATCH();</span><brstyle="font-family: courier new,monospace;" /><span style="font-family: courier
new,monospace;">   {</span><br style="font-family: courier new,monospace;" /><span style="font-family: courier
new,monospace;">       ... error recovery code ...</span><br style="font-family: courier new,monospace;" /><span
style="font-family:courier new,monospace;">        ... plus anything that you wish to do even if an error wasn't thrown
...</span><brstyle="font-family: courier new,monospace;" /><span style="font-family: courier new,monospace;">
           (because of a PG_RE_THROW possibility)</span><br style="font-family: courier new,monospace;" /><span
style="font-family:courier new,monospace;">    }</span><br style="font-family: courier new,monospace;" /><span
style="font-family:courier new,monospace;">     PG_END_TRY();<br /><br />    ... do the same thing over again; since
eitherno ERROR or no RE_THROW() ...<br /></span><br clear="all" /><span style="font-family: courier new,monospace;">   
...normal code ...<br /><br /></span>I propose a new constuct, PG_FINALLY. This will help in eliminating code
duplication(hence lesser possibility of errors), and better modularity. It will also help if someone wishes to call a
non-idempotentfunction in the now-repeating code. <br /><br /><span style="font-family: courier new,monospace;">#define
PG_TRY()                                                       \</span><br style="font-family: courier new,monospace;"
/><spanstyle="font-family: courier new,monospace;">     do
{                                                               \</span><br style="font-family: courier new,monospace;"
/><spanstyle="font-family: courier new,monospace;">        sigjmp_buf *save_exception_stack =
PG_exception_stack;         \ </span><br style="font-family: courier new,monospace;" /><span style="font-family:
couriernew,monospace;">        ErrorContextCallback *save_context_stack = error_context_stack; \</span><br
style="font-family:courier new,monospace;" /><span style="font-family: courier new,monospace;">        bool do_re_throw
=false;                                       \</span><br style="font-family: courier new,monospace;" /><span
style="font-family:courier new,monospace;">         sigjmp_buf local_sigjmp_buf;                                   
\</span><brstyle="font-family: courier new,monospace;" /><span style="font-family: courier new,monospace;">        if
(sigsetjmp(local_sigjmp_buf,0) == 0)                        \ </span><br style="font-family: courier new,monospace;"
/><spanstyle="font-family: courier new,monospace;">       
{                                                              \</span><br style="font-family: courier new,monospace;"
/><spanstyle="font-family: courier new,monospace;">            PG_exception_stack = &local_sigjmp_buf</span><br
style="font-family:courier new,monospace;" /><br style="font-family: courier new,monospace;" /><span
style="font-family:courier new,monospace;"> #define PG_CATCH()                                                     
\</span><brstyle="font-family: courier new,monospace;" /><span style="font-family: courier new,monospace;">       
}                                                              \ </span><br style="font-family: courier new,monospace;"
/><spanstyle="font-family: courier new,monospace;">       
else                                                           \</span><br style="font-family: courier new,monospace;"
/><spanstyle="font-family: courier new,monospace;">       
{                                                              \</span><br style="font-family: courier new,monospace;"
/><brstyle="font-family: courier new,monospace;" /><span style="font-family: courier new,monospace;">#define
PG_FINALLY()                                                   \</span><br style="font-family: courier new,monospace;"
/><spanstyle="font-family: courier new,monospace;">        
}                                                              \</span><br style="font-family: courier new,monospace;"
/><spanstyle="font-family: courier new,monospace;">       
{                                                              \ </span><br style="font-family: courier new,monospace;"
/><spanstyle="font-family: courier new,monospace;">            PG_exception_stack =
save_exception_stack;                 \</span><br style="font-family: courier new,monospace;" /><span
style="font-family:courier new,monospace;">            error_context_stack = save_context_stack</span><br
style="font-family:courier new,monospace;" /><br style="font-family: courier new,monospace;" /><span
style="font-family:courier new,monospace;"> #define PG_END_TRY()                                                   
\</span><brstyle="font-family: courier new,monospace;" /><span style="font-family: courier new,monospace;">       
}                                                              \ </span><br style="font-family: courier new,monospace;"
/><spanstyle="font-family: courier new,monospace;">        if
(do_re_throw)                                               \</span><br style="font-family: courier new,monospace;"
/><spanstyle="font-family: courier new,monospace;">            siglongjmp(*PG_exception_stack,
1)                         \</span><br style="font-family: courier new,monospace;" /><span style="font-family: courier
new,monospace;">    } while (0)</span><br style="font-family: courier new,monospace;" /><br style="font-family: courier
new,monospace;"/><span style="font-family: courier new,monospace;">#define PG_RE_THROW()   do_re_throw = true</span><br
style="font-family:courier new,monospace;" /><br />This would change the semantics to:<br /><br /><span
style="font-family:courier new,monospace;">    ... normal code ...<br /><br />     PG_TRY();</span><br
style="font-family:courier new,monospace;" /><span style="font-family: courier new,monospace;">    {</span><br
style="font-family:courier new,monospace;" /><span style="font-family: courier new,monospace;">        ... code that
mightthrow ereport(ERROR) ...</span><br style="font-family: courier new,monospace;" /><span style="font-family: courier
new,monospace;">   }</span><br style="font-family: courier new,monospace;" /><span style="font-family: courier
new,monospace;">   PG_CATCH();</span><br style="font-family: courier new,monospace;" /><span style="font-family:
couriernew,monospace;">    {</span><br style="font-family: courier new,monospace;" /><span style="font-family: courier
new,monospace;">       ... (optional) error recovery code only *and/or* RE_THROW...</span><br style="font-family:
couriernew,monospace;" /><span style="font-family: courier new,monospace;"></span><span style="font-family: courier
new,monospace;">   }</span><br style="font-family: courier new,monospace;" /><span style="font-family: courier
new,monospace;">   PG_FINALLY();</span><br style="font-family: courier new,monospace;" /><span style="font-family:
couriernew,monospace;">    {</span><br style="font-family: courier new,monospace;" /><span style="font-family: courier
new,monospace;">       ... do something that you wanted done in any case; ERROR or not ...</span><br
style="font-family:courier new,monospace;" /><span style="font-family: courier new,monospace;"></span><span
style="font-family:courier new,monospace;">    }</span><br style="font-family: courier new,monospace;" /><span
style="font-family:courier new,monospace;"></span><span style="font-family: courier new,monospace;">   
PG_END_TRY();<br/><br /></span><span style="font-family: courier new,monospace;">    ... normal code ...<br
/></span><br/>Hope to find buyers.<br /><br />Best regards,<br /><br />-- <br />gurjeet[.singh]@EnterpriseDB.com<br
/>singh.gurjeet@{gmail | hotmail | yahoo }.com  

"Gurjeet Singh" <singh.gurjeet@gmail.com> writes:
> I propose a new constuct, PG_FINALLY.

I took a look through the existing uses of PG_CATCH, and found that in
the places where there is duplicate code between the normal and error
exits, it's usually just one or two lines.  Where it's more than that,
it's intermixed with code that is not duplicate, and so PG_FINALLY
wouldn't help.  I don't care to add code to every use of PG_TRY for
such a limited benefit.

To the extent that the compiler is able to recognize and optimize out
the extra code in blocks where do_re_throw is never changed, that
objection loses force --- but in such cases it seems likely that some
compilers would throw a warning, which we don't want.

Lastly, your proposal silently breaks every existing use of PG_TRY by
changing the existing semantics of PG_CATCH.  That will certainly not
do.  (This is not even counting the bug that the CATCH code would be
executed inside the same setjmp context, so that any elog within the
CATCH code would turn into an infinite loop.)

Possibly it'd make sense to offer PG_FINALLY as a mutually exclusive
alternative to PG_CATCH, rather than trying to allow both to be used
in the same TRY construct.  I doubt you've got reasonable semantics
for the combination anyway --- it seems at least as likely that the
common actions would need to be executed before the special error
actions as after.
        regards, tom lane