Re: pgsql: Add PL/pgSQL SQLSTATE and SQLERRM support which sets these values

Log Message:
-----------
Add PL/pgSQL SQLSTATE and SQLERRM support which sets these values on
error.

Pavel Stehule

Modified Files:
--------------
pgsql/doc/src/sgml:
plpgsql.sgml (r1.67 -> r1.68)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/doc/src/sgml/plpgsql.sgml.diff?r1=1.67&r2=1.68)
pgsql/src/pl/plpgsql/src:
gram.y (r1.69 -> r1.70)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/pl/plpgsql/src/gram.y.diff?r1=1.69&r2=1.70)
pl_exec.c (r1.138 -> r1.139)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/pl/plpgsql/src/pl_exec.c.diff?r1=1.138&r2=1.139)
plpgsql.h (r1.58 -> r1.59)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/pl/plpgsql/src/plpgsql.h.diff?r1=1.58&r2=1.59)
pgsql/src/test/regress/expected:
plpgsql.out (r1.28 -> r1.29)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/test/regress/expected/plpgsql.out.diff?r1=1.28&r2=1.29)
pgsql/src/test/regress/sql:
plpgsql.sql (r1.23 -> r1.24)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/test/regress/sql/plpgsql.sql.diff?r1=1.23&r2=1.24)

momjian(at)svr1(dot)postgresql(dot)org (Bruce Momjian) writes:
> Add PL/pgSQL SQLSTATE and SQLERRM support which sets these values on
> error.

I had not taken the time to review this patch before, but now that I
have looked at it I'm pretty unhappy with it. It creates new local
variables SQLSTATE and SQLERRM in *every* plpgsql block, whether the
block has any exception handlers or not (to say nothing of whether
the exception handlers actually use the values). That is rather a lot
of overhead for a feature that not everyone needs.

The reasoning is evidently to try to emulate the Oracle definition.
According to some quick googling, in Oracle SQLCODE and SQLERRM are
functions (not variables) which inside an exception handler return
data about the error that triggered the handler, and elsewhere return
000/Successful completion. However, the patch as-is is a pretty poor
emulation of the Oracle definition, considering that:

1. Variables are not functions (in particular, you can assign to a
variable).

2. We don't even support SQLCODE; it's SQLSTATE.

3. If one tries to put a BEGIN block inside an exception handler,
one suddenly can't see the error values anymore within that block.

Point 1 is minor, and point 2 is already agreed to --- but it already
means we've lost exact compatibility with Oracle, and so a slavish
attempt to emulate exactly what they do seems a tad pointless. But
point 3 is an out-and-out bug, and a pretty serious one IMHO.

I suggest that what we should do is define SQLSTATE and SQLERRM
similarly to FOUND: they are procedure-local variables that are
assigned to by an occurrence of an error. I'd be inclined to make them
start out NULL, too, not 00000/"Successful completion".

Alternatively we could make them local to any block that contains an
EXCEPTION clause, which would fix point 3 and also go a long way towards
addressing the unnecessary-overhead gripe. However that would mean that
an attempt to reference them from outside an exception handler would
probably fail outright, rather than deliver either NULLs or
00000/"Successful completion". That doesn't bother me a whole lot since
it seems unlikely that any real-world code would do that, considering
the complete uselessness of the Oracle definition for code outside an
exception handler.

In the meantime, though, this patch is not ready for prime time ...
and the documentation is certainly inadequate since it gives no hint of
just how special these variables are.

regards, tom lane

Tom Lane wrote:
> Alternatively we could make them local to any block that contains an
> EXCEPTION clause, which would fix point 3 and also go a long way towards
> addressing the unnecessary-overhead gripe. However that would mean that
> an attempt to reference them from outside an exception handler would
> probably fail outright, rather than deliver either NULLs or
> 00000/"Successful completion".

This behavior sounds fine to me.

-Neil

Neil Conway <neilc(at)samurai(dot)com> writes:
> Tom Lane wrote:
>> Alternatively we could make them local to any block that contains an
>> EXCEPTION clause, which would fix point 3 and also go a long way towards
>> addressing the unnecessary-overhead gripe. However that would mean that
>> an attempt to reference them from outside an exception handler would
>> probably fail outright, rather than deliver either NULLs or
>> 00000/"Successful completion".

> This behavior sounds fine to me.

I think the key distinction between this proposal and my other one
(that SQLSTATE/SQLERRM be procedure-local) is whether you want the error
status to be available to code that immediately follows the BEGIN block
containing the exception handler. That is, consider code like

BEGIN
-- do something perilous
EXCEPTION
WHEN OTHERS THEN -- nothing much
END;
IF SQLSTATE = '42000' THEN ...

At the moment I don't have a strong opinion about this. It seems
closely analogous to the question whether a loop iteration variable
should remain defined after the loop exits --- you can find cases
where that's handy, but you can also argue it shouldn't be used.
plpgsql itself is schizophrenic on the point (see integer versus
record FOR-loops), which means we don't have a solid precedent to go by.

regards, tom lane

Patch backed out, and new combined version attached.

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

Tom Lane wrote:
> Neil Conway <neilc(at)samurai(dot)com> writes:
> > Tom Lane wrote:
> >> Alternatively we could make them local to any block that contains an
> >> EXCEPTION clause, which would fix point 3 and also go a long way towards
> >> addressing the unnecessary-overhead gripe. However that would mean that
> >> an attempt to reference them from outside an exception handler would
> >> probably fail outright, rather than deliver either NULLs or
> >> 00000/"Successful completion".
>
> > This behavior sounds fine to me.
>
> I think the key distinction between this proposal and my other one
> (that SQLSTATE/SQLERRM be procedure-local) is whether you want the error
> status to be available to code that immediately follows the BEGIN block
> containing the exception handler. That is, consider code like
>
> BEGIN
> -- do something perilous
> EXCEPTION
> WHEN OTHERS THEN -- nothing much
> END;
> IF SQLSTATE = '42000' THEN ...
>
> At the moment I don't have a strong opinion about this. It seems
> closely analogous to the question whether a loop iteration variable
> should remain defined after the loop exits --- you can find cases
> where that's handy, but you can also argue it shouldn't be used.
> plpgsql itself is schizophrenic on the point (see integer versus
> record FOR-loops), which means we don't have a solid precedent to go by.
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly
>

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

Tom Lane wrote:
> I suggest that what we should do is define SQLSTATE and SQLERRM
> similarly to FOUND: they are procedure-local variables that are
> assigned to by an occurrence of an error. I'd be inclined to make
> them start out NULL, too, not 00000/"Successful completion".

Does Oracle support GET DIAGNOSTICS? If so, couldn't we just use that?
I can't see what good will become of making any slightly useful
information become available as magic variables of some kind.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

On Thu, 26 May 2005, Neil Conway wrote:

> Tom Lane wrote:
> > Alternatively we could make them local to any block that contains an
> > EXCEPTION clause, which would fix point 3 and also go a long way towards
> > addressing the unnecessary-overhead gripe. However that would mean that
> > an attempt to reference them from outside an exception handler would
> > probably fail outright, rather than deliver either NULLs or
> > 00000/"Successful completion".
>
> This behavior sounds fine to me.
>
true, there is not any reason use this variables outside exception
handler.

Pavel

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> Does Oracle support GET DIAGNOSTICS? If so, couldn't we just use that?
> I can't see what good will become of making any slightly useful
> information become available as magic variables of some kind.

Oracle actually defines these things as parameterless functions that are
called without parentheses (like CURRENT_USER). "Magic variables" are
about as close as we can get to matching that.

If we go with the idea that they should be local to blocks containing
EXCEPTION, then the easiest implementation would involve pushing them
into the namespace at the beginning of processing the EXCEPTION clause;
which'd mean they are actually physically inaccessible anywhere outside
EXCEPTION. That seems like a suitably narrow API --- in fact it
completely gets rid of the question of what their initial/default values
should be. I think we can mark 'em CONST, too, so that they are truly
semantically indistinguishable from functions.

regards, tom lane

> BEGIN
> -- do something perilous
> EXCEPTION
> WHEN OTHERS THEN -- nothing much
> END;
> IF SQLSTATE = '42000' THEN ...

I understand. My idea was detect local exception for local block, I can't
to see exception's information outside block and I cant get exception's
info from inner block. Your idea is easy for implementation, but oracle

http://www.unix.org.ua/orelly/oracle/prog2/ch13_03.htm

In Oracle doc:

If no exception has been raised, SQLCODE returns zero and SQLERRM returns
the message: ORA-0000: normal, successful completion.

If you reference SQLCODE outside of an exception section, it always
returns 0, which means normal, successful completion.

I tested it on Oracle 10g

return integer as
begin
begin
dbms_output.put_line('1: '||SQLCODE||' -> '||SQLERRM);
raise_application_error(-20001, 'First exception');
exception when others then
dbms_output.put_line('2: '||SQLCODE||' -> '||SQLERRM);
begin
dbms_output.put_line('3: '||SQLCODE||' -> '||SQLERRM);
raise_application_error(-20002, 'Second exception');
exception when others then
dbms_output.put_line('4: '||SQLCODE||' -> '||SQLERRM);
end;
dbms_output.put_line('5: '||SQLCODE||' -> '||SQLERRM);
end;
dbms_output.put_line('6: '||SQLCODE||' -> '||SQLERRM);
return 1;
end;

select foo from dual

1: 0 -> ORA-0000: normal, successful completion
2: -20001 -> ORA-20001: First exception
3: -20001 -> ORA-20001: First exception
4: -20002 -> ORA-20002: Second exception
5: 0 -> ORA-0000: normal, successful completion
6: 0 -> ORA-0000: normal, successful completion

What it is mean?

So we can have only one procedure level scope variable, which is
initialized on start of exception and zeroized on the end of exception
block. This behavior is different from my patch, but is better for Oracle
compatibility and I prefere its.

I'll change patch, I can simplify it, if there will be agreement.

Best regards
Pavel Stehule

Pavel Stehule <stehule(at)kix(dot)fsv(dot)cvut(dot)cz> writes:
> What it is mean?

I think it means Oracle is broken ;-)

> So we can have only one procedure level scope variable, which is
> initialized on start of exception and zeroized on the end of exception
> block. This behavior is different from my patch, but is better for Oracle
> compatibility and I prefere its.

That might be taking the notion of bug-compatibility with PL/SQL
a bit too far. For that matter, did you check whether Oracle
actually treats it as a procedure-scope variable? Try having the
exception block call another function and trap an error inside that.
Does SQLCODE change in the calling function?

regards, tom lane

>
> That might be taking the notion of bug-compatibility with PL/SQL
> a bit too far. For that matter, did you check whether Oracle
> actually treats it as a procedure-scope variable? Try having the
> exception block call another function and trap an error inside that.
> Does SQLCODE change in the calling function?
>

good shot. Its more like session (global) variable.

create function foo2
return integer
as
begin
dbms_output.put_line('10: '||SQLCODE||' -> '||SQLERRM);
raise_application_error(-20003, 'Third exception');
end;

change of foo

dbms_output.put_line('3: '||SQLCODE||' -> '||SQLERRM);
--raise_application_error(-20002, 'Second exception');
f := foo2;
exception when others then
dbms_output.put_line('4: '||SQLCODE||' -> '||SQLERRM);
end;

and result:

1: 0 -> ORA-0000: normal, successful completion
2: -20001 -> ORA-20001: First exception
3: -20001 -> ORA-20001: First exception
10: -20001 -> ORA-20001: First exception
4: -20003 -> ORA-20003: Third exception
5: 0 -> ORA-0000: normal, successful completion
6: 0 -> ORA-0000: normal, successful completion

foo2 knows first exception. Is it bug? Maybe. The exception was outside
function, not in function and expected value of SQLCODE is zero.

Pavel

Tom Lane wrote:
> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> > Does Oracle support GET DIAGNOSTICS? If so, couldn't we just use
> > that? I can't see what good will become of making any slightly
> > useful information become available as magic variables of some
> > kind.
>
> Oracle actually defines these things as parameterless functions that
> are called without parentheses (like CURRENT_USER). "Magic
> variables" are about as close as we can get to matching that.

What I was aiming for is this: If Oracle supported GET DIAGNOSTICS, then
we could ask people to switch to that instead of using the magic
variables and they'd still have code that works both ways.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Pavel Stehule wrote:
> So we can have only one procedure level scope variable, which is
> initialized on start of exception and zeroized on the end of exception
> block. This behavior is different from my patch, but is better for Oracle
> compatibility and I prefere its.

I should have commented on this earlier: I don't think exact Oracle
compatibility is _at all_ important. This feature won't be bug-for-bug
compatible with Oracle in any case (e.g. SQLSTATE vs. SQLERRM) -- I
think we should implement what makes the most sense, as long as it
provides functionality more or less equivalent to what Oracle does.

As for "what makes the most sense", I like Tom's proposal here:

http://archives.postgresql.org/pgsql-committers/2005-05/msg00311.php

i.e. make SQLSTATE and SQLERRM const variables that are local to
EXCEPTION blocks, and make accessing them outside an EXCEPTION block
yield an error.

Does this sound reasonable to people?

-Neil

Neil Conway <neilc(at)samurai(dot)com> writes:
> I should have commented on this earlier: I don't think exact Oracle
> compatibility is _at all_ important.

The results of Pavel's experiments prove that Oracle's behavior is
pretty random --- it looks to me like the chance results of whatever
quick-and-dirty implementation someone chose long ago, rather than
behavior that was thought out and agreed to. I concur that we shouldn't
feel a compulsion to match it exactly, especially seeing that we
aren't going to match it exactly in terms of the contents of the
variables, let alone fine details of what they may contain at different
times.

> As for "what makes the most sense", I like Tom's proposal here:
> http://archives.postgresql.org/pgsql-committers/2005-05/msg00311.php

> Does this sound reasonable to people?

Sounds good to me of course ;-)

regards, tom lane

On Fri, 3 Jun 2005, Neil Conway wrote:

> Pavel Stehule wrote:
> > So we can have only one procedure level scope variable, which is
> > initialized on start of exception and zeroized on the end of exception
> > block. This behavior is different from my patch, but is better for Oracle
> > compatibility and I prefere its.
>
> I should have commented on this earlier: I don't think exact Oracle
> compatibility is _at all_ important. This feature won't be bug-for-bug
> compatible with Oracle in any case (e.g. SQLSTATE vs. SQLERRM) -- I
> think we should implement what makes the most sense, as long as it
> provides functionality more or less equivalent to what Oracle does.
>

Oracle behavior ~ when control go out from any exception protected block
then reset SQLSTATE. Tom's proposal is more logic, but needs much more
changes in parser. And there is one possible incompatibility - Oracle
documentation clearly speeks so SQLSTATE is outside EXCEPTION BLOCK
visible and has value 00000.

I din't find easy way how append variable only when block contains
EXCEPTION part. I wilcome any advice

Pavel

Pavel Stehule <stehule(at)kix(dot)fsv(dot)cvut(dot)cz> writes:
> I din't find easy way how append variable only when block contains
> EXCEPTION part. I wilcome any advice

I was envisioning an action in the beginning of the EXCEPTION clause,
viz

exception_sect :
{ $$ = NIL; }
| K_EXCEPTION
{ push vars into namespace here }
proc_exceptions
{ $$ = $2; }
;

regards, tom lane

On Thu, 2 Jun 2005, Tom Lane wrote:

> Neil Conway <neilc(at)samurai(dot)com> writes:
> > I should have commented on this earlier: I don't think exact Oracle
> > compatibility is _at all_ important.
>
> The results of Pavel's experiments prove that Oracle's behavior is
> pretty random --- it looks to me like the chance results of whatever

What I can speek. Oracle has session variable for saving err code of last
exception. On the start of session is zero. After executing any EXCEPTION
BLOCK is this session zeroed. I can't to see so it's buggy behavior.

We have more possibilities, because wont to implement it only for PL/pgSQL
and can do more complicated solutions. Maybe on general level, Oracle's
developers had to use only solution on session variable.

> quick-and-dirty implementation someone chose long ago, rather than
> behavior that was thought out and agreed to. I concur that we shouldn't
> feel a compulsion to match it exactly, especially seeing that we

true. Compatibility isn't tabu, but sometimes is very usefull ;-), and
motivation for future.

> aren't going to match it exactly in terms of the contents of the
> variables, let alone fine details of what they may contain at different
> times.
>

Pavel

Pavel Stehule <stehule(at)kix(dot)fsv(dot)cvut(dot)cz> writes:
> On Thu, 2 Jun 2005, Tom Lane wrote:
>> The results of Pavel's experiments prove that Oracle's behavior is
>> pretty random --- it looks to me like the chance results of whatever

> What I can speek. Oracle has session variable for saving err code of last
> exception. On the start of session is zero. After executing any EXCEPTION
> BLOCK is this session zeroed. I can't to see so it's buggy behavior.

Well, it may not be strictly a "bug", but it's certainly a very poor
design, since it fails to cope at all nicely with nested-exception cases.

regards, tom lane

On Thu, 2 Jun 2005, Tom Lane wrote:

> Pavel Stehule <stehule(at)kix(dot)fsv(dot)cvut(dot)cz> writes:
> > I din't find easy way how append variable only when block contains
> > EXCEPTION part. I wilcome any advice
>
> I was envisioning an action in the beginning of the EXCEPTION clause,
> viz
>
> exception_sect :
> { $$ = NIL; }
> | K_EXCEPTION
> { push vars into namespace here }
> proc_exceptions
> { $$ = $2; }
> ;

True. It's can work. I'll change it.

Pavel Stehule