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

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

From
momjian@svr1.postgresql.org (Bruce Momjian)
Date:
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@svr1.postgresql.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

Re: pgsql: Add PL/pgSQL SQLSTATE and SQLERRM support

From
Neil Conway
Date:
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

Re: pgsql: Add PL/pgSQL SQLSTATE and SQLERRM support

From
Tom Lane
Date:
Neil Conway <neilc@samurai.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

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

From
Peter Eisentraut
Date:
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/

Peter Eisentraut <peter_e@gmx.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

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

From
Peter Eisentraut
Date:
Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.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/

Re: pgsql: Add PL/pgSQL SQLSTATE and SQLERRM support

From
Pavel Stehule
Date:
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


Re: pgsql: Add PL/pgSQL SQLSTATE and SQLERRM support

From
Pavel Stehule
Date:
>     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



Re: pgsql: Add PL/pgSQL SQLSTATE and SQLERRM support

From
Neil Conway
Date:
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

Re: pgsql: Add PL/pgSQL SQLSTATE and SQLERRM support

From
Tom Lane
Date:
Neil Conway <neilc@samurai.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

Re: pgsql: Add PL/pgSQL SQLSTATE and SQLERRM support

From
Pavel Stehule
Date:
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


Re: pgsql: Add PL/pgSQL SQLSTATE and SQLERRM support

From
Pavel Stehule
Date:
On Thu, 2 Jun 2005, Tom Lane wrote:

> Neil Conway <neilc@samurai.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