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
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 |
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 |