Re: [COMMITTERS] pgsql: Add STRICT to PL/pgSQL SELECT INTO, so exceptions are thrown if

Log Message:
-----------
Add STRICT to PL/pgSQL SELECT INTO, so exceptions are thrown if more or
less than one row is returned by the SELECT, for Oracle PL/SQL
compatibility.

Improve SELECT INTO documentation.

Matt Miller

Modified Files:
--------------
pgsql/doc/src/sgml:
plpgsql.sgml (r1.95 -> r1.96)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/doc/src/sgml/plpgsql.sgml.diff?r1=1.95&r2=1.96)
pgsql/src/pl/plpgsql/src:
gram.y (r1.91 -> r1.92)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/pl/plpgsql/src/gram.y.diff?r1=1.91&r2=1.92)
pl_exec.c (r1.170 -> r1.171)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/pl/plpgsql/src/pl_exec.c.diff?r1=1.170&r2=1.171)
plerrcodes.h (r1.7 -> r1.8)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/pl/plpgsql/src/plerrcodes.h.diff?r1=1.7&r2=1.8)
plpgsql.h (r1.75 -> r1.76)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/pl/plpgsql/src/plpgsql.h.diff?r1=1.75&r2=1.76)
scan.l (r1.50 -> r1.51)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/pl/plpgsql/src/scan.l.diff?r1=1.50&r2=1.51)

momjian(at)postgresql(dot)org (Bruce Momjian) writes:
> Add STRICT to PL/pgSQL SELECT INTO, so exceptions are thrown if more or
> less than one row is returned by the SELECT, for Oracle PL/SQL
> compatibility.

I've got a couple of problems with the error codes used by this patch.
In the first place, you can't arbitrarily assign names to error
conditions that are different from the standard spelling (see
errcodes.sgml for why not: the standard spellings are what are
documented). In the second place, the spec clearly says that class 02
is warning conditions, not errors, so using ERRCODE_NO_DATA for an error
is inappropriate.

Where did you get those names from ... were they picked out of the air,
or were they intended to be Oracle-compatible, or what? Surely we
aren't trying to be Oracle-compatible at that level of detail (else
we've doubtless got a huge number of other cases where we throw a
different error than they do).

Do we actually need different error codes for too few and too many rows?
It looks to me like the only relevant standard error condition is
CARDINALITY_VIOLATION, so either we throw CARDINALITY_VIOLATION for both
cases or we invent nonstandard codes.

regards, tom lane

Tom Lane wrote:
> momjian(at)postgresql(dot)org (Bruce Momjian) writes:
> > Add STRICT to PL/pgSQL SELECT INTO, so exceptions are thrown if more or
> > less than one row is returned by the SELECT, for Oracle PL/SQL
> > compatibility.
>
> I've got a couple of problems with the error codes used by this patch.
> In the first place, you can't arbitrarily assign names to error
> conditions that are different from the standard spelling (see
> errcodes.sgml for why not: the standard spellings are what are
> documented). In the second place, the spec clearly says that class 02

I saw this at the top of plerrcodes.h:

* Eventually this header file should be auto-generated from errcodes.h
* with some sort of sed hackery, but no time for that now. It's likely
* that an exact mapping will not be what's wanted anyhow ...

so I figured we were supposed to map them.

> is warning conditions, not errors, so using ERRCODE_NO_DATA for an error
> is inappropriate.

Oh, I see that now:

/* Class 02 - No Data --- this is also a warning class per SQL99 */
/* (do not use this class for failure conditions!) */
#define ERRCODE_NO_DATA MAKE_SQLSTATE('0','2', '0','0','0')

> Where did you get those names from ... were they picked out of the air,
> or were they intended to be Oracle-compatible, or what? Surely we

I pulled this from the Oracle documentation that I quoted earlier:

> > When you use a SELECT INTO statement without the BULK COLLECT clause, it
> > should return only one row. If it returns more than one row, PL/SQL
> > raises the predefined exception TOO_MANY_ROWS.
> >
> > However, if no rows are returned, PL/SQL raises NO_DATA_FOUND unless the
> > SELECT statement called a SQL aggregate function such as AVG or SUM.
> > (SQL aggregate functions always return a value or a null. So, a SELECT
> > INTO statement that calls an aggregate function never raises
> > NO_DATA_FOUND.)

Are those both errors in Oracle? I assumed so.

> aren't trying to be Oracle-compatible at that level of detail (else
> we've doubtless got a huge number of other cases where we throw a
> different error than they do).

I thought it was nice to get as close as possible, but using a warning
code is clearly bad.

> Do we actually need different error codes for too few and too many rows?
> It looks to me like the only relevant standard error condition is
> CARDINALITY_VIOLATION, so either we throw CARDINALITY_VIOLATION for both
> cases or we invent nonstandard codes.

We could, and then suggest using ROW_COUNT to determine if there were
too few rows, or too many.

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Tom Lane wrote:
>> Do we actually need different error codes for too few and too many rows?
>> It looks to me like the only relevant standard error condition is
>> CARDINALITY_VIOLATION, so either we throw CARDINALITY_VIOLATION for both
>> cases or we invent nonstandard codes.

> We could, and then suggest using ROW_COUNT to determine if there were
> too few rows, or too many.

SELECT INTO doesn't set ROW_COUNT ... but if we change the code to set
FOUND before throwing the error, it'd work to tell people to check
FOUND.

(Thinks a bit...) Actually not, because if the exception catcher isn't
in the same function as the SELECT INTO, it'll be looking at the wrong
FOUND variable. ROW_COUNT same problem, even if we were setting it.

Plan B is to invent new errcodes to match the Oracle spellings. If
that's what we want to do, it's not that hard.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > Tom Lane wrote:
> >> Do we actually need different error codes for too few and too many rows?
> >> It looks to me like the only relevant standard error condition is
> >> CARDINALITY_VIOLATION, so either we throw CARDINALITY_VIOLATION for both
> >> cases or we invent nonstandard codes.
>
> > We could, and then suggest using ROW_COUNT to determine if there were
> > too few rows, or too many.
>
> SELECT INTO doesn't set ROW_COUNT ... but if we change the code to set
> FOUND before throwing the error, it'd work to tell people to check
> FOUND.
>
> (Thinks a bit...) Actually not, because if the exception catcher isn't
> in the same function as the SELECT INTO, it'll be looking at the wrong
> FOUND variable. ROW_COUNT same problem, even if we were setting it.
>
> Plan B is to invent new errcodes to match the Oracle spellings. If
> that's what we want to do, it's not that hard.

We could use:

#define ERRCODE_DATA_EXCEPTION MAKE_SQLSTATE('2','2',
or
#define ERRCODE_ERROR_IN_ASSIGNMENT MAKE_SQLSTATE('2','2',

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Tom Lane wrote:
>> Plan B is to invent new errcodes to match the Oracle spellings. If
>> that's what we want to do, it's not that hard.

> We could use:
> #define ERRCODE_DATA_EXCEPTION MAKE_SQLSTATE('2','2',
> or
> #define ERRCODE_ERROR_IN_ASSIGNMENT MAKE_SQLSTATE('2','2',

Those are both mighty generic (in fact DATA_EXCEPTION is a class code
not a specific error). If we want to stick to existing errcodes I think
CARDINALITY_VIOLATION is the only reasonable choice.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > Tom Lane wrote:
> >> Plan B is to invent new errcodes to match the Oracle spellings. If
> >> that's what we want to do, it's not that hard.
>
> > We could use:
> > #define ERRCODE_DATA_EXCEPTION MAKE_SQLSTATE('2','2',
> > or
> > #define ERRCODE_ERROR_IN_ASSIGNMENT MAKE_SQLSTATE('2','2',
>
> Those are both mighty generic (in fact DATA_EXCEPTION is a class code
> not a specific error). If we want to stick to existing errcodes I think
> CARDINALITY_VIOLATION is the only reasonable choice.

If we go with that how does the caller check between not found and too
many? And if we go with Oracle names, I need different codes to match
with the two Oracle names.

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> If we go with that how does the caller check between not found and too
> many? And if we go with Oracle names, I need different codes to match
> with the two Oracle names.

I think we should just go with two new codes and use the Oracle names
for them. One remaining question: shall we assign codes in class 21
(Cardinality Violation) or class P0 (PL/pgSQL Error)? If you think
these are likely to be used in other places then class 21 seems
reasonable, but if we are thinking of them as being Oracle compatibility
hacks then I'd lean to class P0.

Actually ... does Oracle have SQLSTATEs assigned to these errors?
If so, maybe we should use theirs. I had the idea they were still
stuck on non-spec-compatible error numbers, though.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > If we go with that how does the caller check between not found and too
> > many? And if we go with Oracle names, I need different codes to match
> > with the two Oracle names.
>
> I think we should just go with two new codes and use the Oracle names
> for them. One remaining question: shall we assign codes in class 21
> (Cardinality Violation) or class P0 (PL/pgSQL Error)? If you think
> these are likely to be used in other places then class 21 seems
> reasonable, but if we are thinking of them as being Oracle compatibility
> hacks then I'd lean to class P0.

Oracle-only, I would think, but I am no Oracle expert (never used it,
actually (a badge of honor?)).

> Actually ... does Oracle have SQLSTATEs assigned to these errors?
> If so, maybe we should use theirs. I had the idea they were still
> stuck on non-spec-compatible error numbers, though.

Anyone?

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Tom Lane wrote:
>> Actually ... does Oracle have SQLSTATEs assigned to these errors?
>> If so, maybe we should use theirs. I had the idea they were still
>> stuck on non-spec-compatible error numbers, though.

> Anyone?

I did some googling and found one page that says no_data_found is
ORA-01403 and too_many_rows is ORA-01422. Another page said that
ORA-01403 maps to SQLSTATE 02000 (which is exactly what we need to
avoid to be spec-compliant) and they don't give a mapping at all
for ORA-01422 :-(. So once again, Oracle is totally useless as a
precedent for SQLSTATE choices.

I'll go with a couple of codes in the P0 class for the moment.
If anyone has a better idea, it won't be hard to change.

regards, tom lane