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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>, Pavel Stehule <stehule(at)kix(dot)fsv(dot)cvut(dot)cz>
Cc: pgsql-committers(at)postgresql(dot)org
Subject: Re: pgsql: Add PL/pgSQL SQLSTATE and SQLERRM support which sets these values
Date: 2005-05-26 03:26:00
Message-ID: 22915.1117077960@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-patches

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

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2005-05-26 03:36:59 Re: pgsql: Minor cleanup for recent SQLSTATE / SQLERRM patch: spell
Previous Message Neil Conway 2005-05-26 03:18:53 pgsql: Minor cleanup for recent SQLSTATE / SQLERRM patch: spell

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2005-05-26 03:30:00 Re: Implementation of SQLSTATE and SQLERRM variables
Previous Message Neil Conway 2005-05-26 03:05:41 Re: Implementation of SQLSTATE and SQLERRM variables