Thread: Re: [COMMITTERS] pgsql: Add STRICT to PL/pgSQL SELECT INTO, so exceptions are thrown if
Re: [COMMITTERS] pgsql: Add STRICT to PL/pgSQL SELECT INTO, so exceptions are thrown if
From
Tom Lane
Date:
momjian@postgresql.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@postgresql.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 somesort 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 failureconditions!) */#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. +
Re: [COMMITTERS] pgsql: Add STRICT to PL/pgSQL SELECT INTO, so exceptions are thrown if
From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.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@candle.pha.pa.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. +
Re: [COMMITTERS] pgsql: Add STRICT to PL/pgSQL SELECT INTO, so exceptions are thrown if
From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.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@candle.pha.pa.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. +
Re: [COMMITTERS] pgsql: Add STRICT to PL/pgSQL SELECT INTO, so exceptions are thrown if
From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.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@candle.pha.pa.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. +
Re: [COMMITTERS] pgsql: Add STRICT to PL/pgSQL SELECT INTO, so exceptions are thrown if
From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.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