Thread: plpgsql raise - parameters can be expressions
Hello,
I did trivial patch which eliminate limit raise command. Using
expressions instead of variables are a little bit expensive, but little.
Type: \copyright for distribution terms
\h for help with SQL commands
\? for help with psql commands
\g or terminate with semicolon to execute query
\q to quit
pokus=# create or replace function x() returns void as $$
declare c integer[] = '{10,20,30}'; a integer = 3;b record;
begin b := row(1,2);
raise notice 'sss % % % % % % % % %', interval '23 hour',
1, current_user, c,now(), c[1],
(select * from fx where 1 = 0 limit 1),
null,current_timestamp::timestamp(0);
end; $$ language plpgsql;
CREATE FUNCTION
pokus=# select x();
NOTICE: sss 23:00:00 1 root {10,20,30} 2005-06-13 07:06:07.43569+02 10
<NULL> <NULL> 2005-06-13 07:
06:07
Regards
Pavel Stehule
Attachment
> pokus=# create or replace function x() returns void as $$
> declare c integer[] = '{10,20,30}'; a integer = 3;b record;
> begin b := row(1,2);
> raise notice 'sss % % % % % % % % %', interval '23 hour',
> 1, current_user, c,now(), c[1],
> (select * from fx where 1 = 0 limit 1),
> null,current_timestamp::timestamp(0);
> end; $$ language plpgsql;
> CREATE FUNCTION
> pokus=# select x();
> NOTICE: sss 23:00:00 1 root {10,20,30} 2005-06-13 07:06:07.43569+02 10
> <NULL> <NULL> 2005-06-13 07:
> 06:07
>
> Regards
> Pavel Stehule
I like :)
+1
On Jun 13, 2005, at 2:07 PM, Pavel Stehule wrote: > I did trivial patch which eliminate limit raise command. Using > expressions instead of variables are a little bit expensive, but > little. I'm right in thinking this essentially does the same thing as first assigning the value of the expression to a variable and then using the variable in the RAISE statement, correct? This patch just eliminates the step of assigning the value of the expression to the variable? If so, I'd expect the cost to be in line with the cost of explicitly assigning to a variable. Definitely nifty! Michael Glaesemann grzm myrealbox com
On Mon, 13 Jun 2005, Michael Glaesemann wrote: > > On Jun 13, 2005, at 2:07 PM, Pavel Stehule wrote: > > > I did trivial patch which eliminate limit raise command. Using > > expressions instead of variables are a little bit expensive, but > > little. > > I'm right in thinking this essentially does the same thing as first > assigning the value of the expression to a variable and then using > the variable in the RAISE statement, correct? This patch just > eliminates the step of assigning the value of the expression to the > variable? If so, I'd expect the cost to be in line with the cost of > explicitly assigning to a variable. > true, total cost is less Pavel
Pavel Stehule wrote:
> I did trivial patch which eliminate limit raise command.
Looks pretty good. Attached is a cleaned-up version that I'll apply to
HEAD tomorrow, barring any objections.
BTW, one regression is that an undefined variable in the RAISE list is
no longer a compile-time error:
create function foo() returns void as '
begin
raise notice ''hello, world: %'', baz;
end;' language plpgsql;
neilc=# select foo();
ERROR: column "baz" does not exist
I don't see an easy way to get around this, though, and it's not too
concerning. Amusingly it does completely break the SQLSTATE and SQLERRM
tests we added a few days ago :)
BTW, another easy improvement in this area is changing the RAISE format
string to allow it to be an expression, rather than only a string literal.
-Neil
Index: doc/src/sgml/plpgsql.sgml
===================================================================
RCS file: /var/lib/cvs/pgsql/doc/src/sgml/plpgsql.sgml,v
retrieving revision 1.71
diff -c -r1.71 plpgsql.sgml
*** doc/src/sgml/plpgsql.sgml 10 Jun 2005 16:23:09 -0000 1.71
--- doc/src/sgml/plpgsql.sgml 13 Jun 2005 05:38:55 -0000
***************
*** 2533,2541 ****
<para>
Inside the format string, <literal>%</literal> is replaced by the
next optional argument's string representation. Write
! <literal>%%</literal> to emit a literal <literal>%</literal>. Note
! that the optional arguments must presently be simple variables,
! not expressions, and the format must be a simple string literal.
</para>
<!--
--- 2533,2541 ----
<para>
Inside the format string, <literal>%</literal> is replaced by the
next optional argument's string representation. Write
! <literal>%%</literal> to emit a literal <literal>%</literal>.
! Arguments can be simple variables or expressions,
! and the format must be a simple string literal.
</para>
<!--
Index: src/pl/plpgsql/src/gram.y
===================================================================
RCS file: /var/lib/cvs/pgsql/src/pl/plpgsql/src/gram.y,v
retrieving revision 1.75
diff -c -r1.75 gram.y
*** src/pl/plpgsql/src/gram.y 10 Jun 2005 16:23:11 -0000 1.75
--- src/pl/plpgsql/src/gram.y 13 Jun 2005 05:44:02 -0000
***************
*** 135,142 ****
%type <exception> proc_exception
%type <condition> proc_conditions
! %type <list> raise_params
! %type <ival> raise_level raise_param
%type <str> raise_msg
%type <list> getdiag_list
--- 135,142 ----
%type <exception> proc_exception
%type <condition> proc_conditions
!
! %type <ival> raise_level
%type <str> raise_msg
%type <list> getdiag_list
***************
*** 1157,1163 ****
}
;
! stmt_raise : K_RAISE lno raise_level raise_msg raise_params ';'
{
PLpgSQL_stmt_raise *new;
--- 1157,1163 ----
}
;
! stmt_raise : K_RAISE lno raise_level raise_msg
{
PLpgSQL_stmt_raise *new;
***************
*** 1167,1187 ****
new->lineno = $2;
new->elog_level = $3;
new->message = $4;
- new->params = $5;
! $$ = (PLpgSQL_stmt *)new;
! }
! | K_RAISE lno raise_level raise_msg ';'
! {
! PLpgSQL_stmt_raise *new;
! new = palloc(sizeof(PLpgSQL_stmt_raise));
! new->cmd_type = PLPGSQL_STMT_RAISE;
! new->lineno = $2;
! new->elog_level = $3;
! new->message = $4;
! new->params = NIL;
$$ = (PLpgSQL_stmt *)new;
}
--- 1167,1186 ----
new->lineno = $2;
new->elog_level = $3;
new->message = $4;
! switch (yylex())
! {
! case ';':
! new->params_expr = NULL;
! break;
! case ',':
! new->params_expr = plpgsql_read_expression(';', ";");
! break;
! default:
! yyerror("syntax error");
! }
$$ = (PLpgSQL_stmt *)new;
}
***************
*** 1219,1240 ****
}
;
- raise_params : raise_params raise_param
- {
- $$ = lappend_int($1, $2);
- }
- | raise_param
- {
- $$ = list_make1_int($1);
- }
- ;
-
- raise_param : ',' T_SCALAR
- {
- $$ = yylval.scalar->dno;
- }
- ;
-
loop_body : proc_sect K_END K_LOOP ';'
{ $$ = $1; }
;
--- 1218,1223 ----
Index: src/pl/plpgsql/src/pl_exec.c
===================================================================
RCS file: /var/lib/cvs/pgsql/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.143
diff -c -r1.143 pl_exec.c
*** src/pl/plpgsql/src/pl_exec.c 10 Jun 2005 16:23:11 -0000 1.143
--- src/pl/plpgsql/src/pl_exec.c 13 Jun 2005 06:04:21 -0000
***************
*** 1911,1920 ****
{
char *cp;
PLpgSQL_dstring ds;
! ListCell *current_param;
plpgsql_dstring_init(&ds);
! current_param = list_head(stmt->params);
for (cp = stmt->message; *cp; cp++)
{
--- 1911,1939 ----
{
char *cp;
PLpgSQL_dstring ds;
! int param_idx = 0;
! int params_count = 0;
plpgsql_dstring_init(&ds);
!
! if (stmt->params_expr)
! {
! int rc = exec_run_select(estate, stmt->params_expr, 2, NULL);
!
! if (rc != SPI_OK_SELECT || estate->eval_processed == 0)
! ereport(ERROR,
! (errcode(ERRCODE_WRONG_OBJECT_TYPE),
! errmsg("query \"%s\" did not return data", stmt->params_expr->query)));
!
! /* Check that the expression returned a single Datum */
! if (estate->eval_processed > 1)
! ereport(ERROR,
! (errcode(ERRCODE_CARDINALITY_VIOLATION),
! errmsg("query \"%s\" returned more than one row",
! stmt->params_expr->query)));
!
! params_count = estate->eval_tuptable->tupdesc->natts;
! }
for (cp = stmt->message; *cp; cp++)
{
***************
*** 1936,1966 ****
continue;
}
! if (current_param == NULL)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("too few parameters specified for RAISE")));
! exec_eval_datum(estate, estate->datums[lfirst_int(current_param)],
! InvalidOid,
! ¶mtypeid, ¶mvalue, ¶misnull);
if (paramisnull)
extval = "<NULL>";
else
extval = convert_value_to_string(paramvalue, paramtypeid);
plpgsql_dstring_append(&ds, extval);
! current_param = lnext(current_param);
continue;
}
plpgsql_dstring_append_char(&ds, cp[0]);
}
/*
* If more parameters were specified than were required to process
* the format string, throw an error
*/
! if (current_param != NULL)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("too many parameters specified for RAISE")));
--- 1955,1989 ----
continue;
}
! if (param_idx == params_count)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("too few parameters specified for RAISE")));
! paramvalue = SPI_getbinval(estate->eval_tuptable->vals[0],
! estate->eval_tuptable->tupdesc,
! param_idx + 1, ¶misnull);
! paramtypeid = SPI_gettypeid(estate->eval_tuptable->tupdesc, param_idx + 1);
!
if (paramisnull)
extval = "<NULL>";
else
extval = convert_value_to_string(paramvalue, paramtypeid);
plpgsql_dstring_append(&ds, extval);
! param_idx++;
continue;
}
plpgsql_dstring_append_char(&ds, cp[0]);
}
+ exec_eval_cleanup(estate);
+
/*
* If more parameters were specified than were required to process
* the format string, throw an error
*/
! if (param_idx != params_count)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("too many parameters specified for RAISE")));
Index: src/pl/plpgsql/src/pl_funcs.c
===================================================================
RCS file: /var/lib/cvs/pgsql/src/pl/plpgsql/src/pl_funcs.c,v
retrieving revision 1.41
diff -c -r1.41 pl_funcs.c
*** src/pl/plpgsql/src/pl_funcs.c 10 Jun 2005 16:23:11 -0000 1.41
--- src/pl/plpgsql/src/pl_funcs.c 13 Jun 2005 05:57:17 -0000
***************
*** 883,894 ****
static void
dump_raise(PLpgSQL_stmt_raise *stmt)
{
- ListCell *l;
-
dump_ind();
printf("RAISE '%s'", stmt->message);
! foreach (l, stmt->params)
! printf(" %d", lfirst_int(l));
printf("\n");
}
--- 883,891 ----
static void
dump_raise(PLpgSQL_stmt_raise *stmt)
{
dump_ind();
printf("RAISE '%s'", stmt->message);
! dump_expr(stmt->params_expr);
printf("\n");
}
Index: src/pl/plpgsql/src/plpgsql.h
===================================================================
RCS file: /var/lib/cvs/pgsql/src/pl/plpgsql/src/plpgsql.h,v
retrieving revision 1.62
diff -c -r1.62 plpgsql.h
*** src/pl/plpgsql/src/plpgsql.h 10 Jun 2005 16:23:11 -0000 1.62
--- src/pl/plpgsql/src/plpgsql.h 13 Jun 2005 05:38:55 -0000
***************
*** 515,521 ****
int lineno;
int elog_level;
char *message;
! List *params;
} PLpgSQL_stmt_raise;
--- 515,521 ----
int lineno;
int elog_level;
char *message;
! PLpgSQL_expr *params_expr;
} PLpgSQL_stmt_raise;
Index: src/test/regress/expected/plpgsql.out
===================================================================
RCS file: /var/lib/cvs/pgsql/src/test/regress/expected/plpgsql.out,v
retrieving revision 1.33
diff -c -r1.33 plpgsql.out
*** src/test/regress/expected/plpgsql.out 10 Jun 2005 16:23:11 -0000 1.33
--- src/test/regress/expected/plpgsql.out 13 Jun 2005 06:28:26 -0000
***************
*** 2418,2444 ****
--
-- SQLSTATE and SQLERRM test
--
- -- should fail: SQLSTATE and SQLERRM are only in defined EXCEPTION
- -- blocks
- create function excpt_test() returns void as $$
- begin
- raise notice '% %', sqlstate, sqlerrm;
- end; $$ language plpgsql;
- ERROR: syntax error at or near "sqlstate" at character 79
- LINE 3: raise notice '% %', sqlstate, sqlerrm;
- ^
- -- should fail
- create function excpt_test() returns void as $$
- begin
- begin
- begin
- raise notice '% %', sqlstate, sqlerrm;
- end;
- end;
- end; $$ language plpgsql;
- ERROR: syntax error at or near "sqlstate" at character 108
- LINE 5: raise notice '% %', sqlstate, sqlerrm;
- ^
create function excpt_test() returns void as $$
begin
begin
--- 2418,2423 ----
***************
*** 2469,2471 ****
--- 2448,2468 ----
(1 row)
drop function excpt_test();
+ -- parameters of raise stmt can be expressions
+ create function raise_exprs() returns void as $$
+ declare
+ a integer[] = '{10,20,30}';
+ c varchar = 'xyz';
+ i integer;
+ begin
+ i := 2;
+ raise notice '%; %; %; %; %', a, a[i], c, (select c || 'abc'), row(10,'aaa',30);
+ end;$$ language plpgsql;
+ select raise_exprs();
+ NOTICE: {10,20,30}; 20; xyz; xyzabc; (10,aaa,30)
+ raise_exprs
+ -------------
+
+ (1 row)
+
+ drop function raise_exprs();
Index: src/test/regress/sql/plpgsql.sql
===================================================================
RCS file: /var/lib/cvs/pgsql/src/test/regress/sql/plpgsql.sql,v
retrieving revision 1.28
diff -c -r1.28 plpgsql.sql
*** src/test/regress/sql/plpgsql.sql 10 Jun 2005 16:23:11 -0000 1.28
--- src/test/regress/sql/plpgsql.sql 13 Jun 2005 06:25:17 -0000
***************
*** 2054,2077 ****
--
-- SQLSTATE and SQLERRM test
--
-
- -- should fail: SQLSTATE and SQLERRM are only in defined EXCEPTION
- -- blocks
- create function excpt_test() returns void as $$
- begin
- raise notice '% %', sqlstate, sqlerrm;
- end; $$ language plpgsql;
-
- -- should fail
- create function excpt_test() returns void as $$
- begin
- begin
- begin
- raise notice '% %', sqlstate, sqlerrm;
- end;
- end;
- end; $$ language plpgsql;
-
create function excpt_test() returns void as $$
begin
begin
--- 2054,2059 ----
***************
*** 2094,2096 ****
--- 2076,2092 ----
select excpt_test();
drop function excpt_test();
+
+ -- parameters of raise stmt can be expressions
+ create function raise_exprs() returns void as $$
+ declare
+ a integer[] = '{10,20,30}';
+ c varchar = 'xyz';
+ i integer;
+ begin
+ i := 2;
+ raise notice '%; %; %; %; %', a, a[i], c, (select c || 'abc'), row(10,'aaa',30);
+ end;$$ language plpgsql;
+
+ select raise_exprs();
+ drop function raise_exprs();
> I don't see an easy way to get around this, though, and it's not too > concerning. Amusingly it does completely break the SQLSTATE and SQLERRM > tests we added a few days ago :) Yes, it's true. But with simple plpgsql parser isn't possible expect miracles :). I think so there is some space for modifications - read_sql_construct - add control for availability sql params. You can solve all errors where is expr_until_semi. This is exactly same situation like create function ... begin execute 'select '||a; end; $$ it generate runtime error > > BTW, another easy improvement in this area is changing the RAISE format > string to allow it to be an expression, rather than only a string literal. > No problem. But it's maybe big change. And I don't see using format string too much limiting. Pavel
Pavel Stehule wrote: > I did trivial patch which eliminate limit raise command. In thinking about this some more, it would be nice to be able to use exec_eval_expr() to reduce expression evaluation overhead for simple RAISE parameters. It is easy enough to refactor the current exec_eval_expr() code so this is possible, but it will be of only limited use: PL/PgSQL currently only considers expressions that result in a single attribute as sufficiently simple that they can be evaluated via ExecEvalExpr(). I wonder how much work it would take to lift this restriction... -Neil
Neil Conway <neilc@samurai.com> writes:
> BTW, one regression is that an undefined variable in the RAISE list is
> no longer a compile-time error:
> ...
> I don't see an easy way to get around this, though, and it's not too
> concerning. Amusingly it does completely break the SQLSTATE and SQLERRM
> tests we added a few days ago :)
That doesn't bother me either, seeing that an undefined variable isn't
detected at compile time anywhere else. However, fixing the SQLSTATE
tests by removing them doesn't seem like a great solution ...
> BTW, another easy improvement in this area is changing the RAISE format
> string to allow it to be an expression, rather than only a string literal.
I would sort of have expected this to get done at the same time.
Actually, the reason I didn't do something about RAISE in 8.0 was that
I thought we should reconsider the whole design of the statement: it
desperately needs to be fixed so that you can specify the SQLSTATE to
be thrown, and so that you can re-throw the same exception you caught.
(Note that SQLERRM is not really a solution to that: you might think
something like "RAISE EXCEPTION SQLSTATE, '%', SQLERRM" would do,
but it loses information, namely all the auxiliary fields.)
Ideas?
regards, tom lane
> That doesn't bother me either, seeing that an undefined variable isn't
> detected at compile time anywhere else. However, fixing the SQLSTATE
> tests by removing them doesn't seem like a great solution ...
>
> > BTW, another easy improvement in this area is changing the RAISE format
> > string to allow it to be an expression, rather than only a string literal.
>
> I would sort of have expected this to get done at the same time.
>
> Actually, the reason I didn't do something about RAISE in 8.0 was that
> I thought we should reconsider the whole design of the statement: it
> desperately needs to be fixed so that you can specify the SQLSTATE to
> be thrown, and so that you can re-throw the same exception you caught.
> (Note that SQLERRM is not really a solution to that: you might think
> something like "RAISE EXCEPTION SQLSTATE, '%', SQLERRM" would do,
> but it loses information, namely all the auxiliary fields.)
>
> Ideas?
only RAISE? Without parameters can be used only in block. It's same scope
like SQLERRM and SQLSTATE.
Oracle can define variables with type EXCEPTION. This all what we need -
identificator. For value of exception Oracle use PRAGMA EXCEPTION_INIT -
for SQLCODE value. PostgreSQL don't suport SQLCODE, then PRAGMA is
irelevant, but what:
DECLARE my_exception EXCEPTION = '22012'; -- division by zero
BEGIN
RAISE my_exception; -- named exception; -- no params
EXCEPTION WHEN division_by_zero THEN
my_exception ~ division by zero
END;
----------or----------------
EXCEPTIO WHEN my_exception THEN
...
END
all variants are legal. I can use user's exception
with default unique value from predefined interval too.
DECLARE my_exception EXCEPTION;
regards
Pavel Stehule
p.s.
>
> regards, tom lane
>
Pavel Stehule <stehule@kix.fsv.cvut.cz> writes:
>> Ideas?
> only RAISE? Without parameters can be used only in block. It's same scope
> like SQLERRM and SQLSTATE.
OK, but what about the other problem --- throwing a non-default SQLSTATE
along with your custom error message?
> RAISE my_exception; -- named exception; -- no params
Definitely not adequate. Maybe we could do
RAISE my_exception, 'format', params;
I think it would be better to do something like
RAISE ERROR my_exception, 'format', params;
since this won't be ambiguous with the existing variants of RAISE;
without the ERROR keyword it'd be necessary to forbid exception names
from being DEBUG, LOG, etc.
I like your DECLARE syntax better than Oracle's incredibly ugly "pragma"
notation, and we can't expect to be exactly compatible with the pragma
anyway since the pragma is associating the name with an integer code,
which is not what we are going to use. So that part seems good.
I assume you intend to allow "RAISE ERROR division_by_zero ..." and
"EXCEPTION WHEN my_exception THEN ...", right? That is, declared
exception names are interchangeable with the predefined ones for
both throwing and catching errors.
> I can use user's exception with default unique value from predefined
> interval too.
I see zero value in that. The only reason for declaring a specific
user-defined exception code is so you can identify it from elsewhere,
like in your application --- so not knowing which code it's getting
makes it useless.
regards, tom lane
> I would sort of have expected this to get done at the same time.
>
> Actually, the reason I didn't do something about RAISE in 8.0 was that
> I thought we should reconsider the whole design of the statement: it
> desperately needs to be fixed so that you can specify the SQLSTATE to
> be thrown, and so that you can re-throw the same exception you caught.
> (Note that SQLERRM is not really a solution to that: you might think
> something like "RAISE EXCEPTION SQLSTATE, '%', SQLERRM" would do,
> but it loses information, namely all the auxiliary fields.)
There is space for future version. I think for todo for plpgsql
o rethrow exception in exception handler block
o user defined exceptions
o mapping user's exception on system's exception
My previous mail isn't correct. I need somewhere specify errmsg text, and
ofcourse params.
???
declare myexcept exception = 'xxxxx';
begin;
assoc(myexcept, 'my message % % %');
raise myexcept, param1, param2, ...
default level of user exception is exception.
regards
Pavel Stehule
Pavel Stehule <stehule@kix.fsv.cvut.cz> writes:
> declare myexcept exception = 'xxxxx';
> begin;
> assoc(myexcept, 'my message % % %');
> raise myexcept, param1, param2, ...
Our experience in writing the backend is that hard-wiring a single error
message text for a SQLSTATE code is the wrong thing. The SQLSTATE codes
are relatively coarse-grained (which is usually the right thing from the
perspective of an application trying to match a SQLSTATE) and there is
very often room for the text message to give more detail than the
SQLSTATE alone conveys.
Also, the above seems highly error prone to me since it decouples the
format string from the parameters. This is even worse for built-in
exception codes since the writer of a plpgsql function shouldn't assume
that he knows exactly what % parameters a built-in exception's format
string would use.
So for all these reasons, I think that only the SQLSTATE should be
associated with the exception name. The format string should be part
of the RAISE command.
regards, tom lane
On Mon, 13 Jun 2005, Tom Lane wrote: > Pavel Stehule <stehule@kix.fsv.cvut.cz> writes: > >> Ideas? > > > only RAISE? Without parameters can be used only in block. It's same scope > > like SQLERRM and SQLSTATE. > > OK, but what about the other problem --- throwing a non-default SQLSTATE > along with your custom error message? > I understand. Its not problem when I have SQLERRM non paramized ~ constant string - I can get custom error message without changes. Problem is when error message is parametrized. What is better, rebuild this string or use old. Maybe its reason why isn't possible easy change errstr in Oracle. Next problem, visibility custom exceptions. When I define exception variable I can't rethrown exceptions outside block when is defined. What is outside - some custom exception? > I think it would be better to do something like > > RAISE ERROR my_exception, 'format', params; disadvantage - I have to define format string everywhere where I wont to raise exception. Idea is similar MsSQL. Then is necessary define only interval for user's exception. But it's not too much readable. > I like your DECLARE syntax better than Oracle's incredibly ugly "pragma" > notation, and we can't expect to be exactly compatible with the pragma > anyway since the pragma is associating the name with an integer code, > which is not what we are going to use. So that part seems good. From OOP view exception is object. But I need define more properties than one. SQLSTATE is only first, second message, level, meybe next DECLARE myexception EXCEPTION VALUE 'xxxx' MESSAGE 'xxx % % %'; or DECLARE myexception EXCEPTION MESSAGE 'xxxx % %' = '3333'; -- optional DECLARE myexception EXCEPTION = '2222'; -- short no message I think so user's exception must not have errmsg. When I define own exception I expect own exception handler. This is only more readable style. I can have one global variable and one exception. It's can be short way to exception handler I think so we need more then one exception level. I can use user's exception for easy way of write to log. Every user's exception can by handled - this is differnt then RAISE NOTICE, any user's exception have to be handled - equal RAISE EXCEPTION (default have to be handled), any user's excp. can be logged. This is big theory :-), pardon DECLARE myex EXCEPTION TOLOG NOERROR CATCHED MESSAGE 'aaaa' VALUE '10'; short myex EXCEPTION = '10'; ~ ERROR CATCHED MESSAGE '' > > I assume you intend to allow "RAISE ERROR division_by_zero ..." and > "EXCEPTION WHEN my_exception THEN ...", right? That is, declared > exception names are interchangeable with the predefined ones for > both throwing and catching errors. Yes, I wont it. When I use SQLSTATE from system's SQLSTATEs, I speek my exception can by handled by any general handler. But I have possibility logging, level, ... this is way for throwing system's exception. I think will be better prohibit throwing system's exceptions directly. But I can associate my exception with system's exception, and throw my exception. > > > I can use user's exception with default unique value from predefined > > interval too. > > I see zero value in that. The only reason for declaring a specific > user-defined exception code is so you can identify it from elsewhere, > like in your application --- so not knowing which code it's getting > makes it useless. There are two questions? Have Every user's exception unique SQLSTATE value? Will be exist any interval for user's exception SQLSTATES? My answer is true for all. If it's true, then I can check one SQLSTATE exception handler in block. In my example is clean nonsens DECLARE e EXCEPTION = '22....'; -- div_by_zero BEGIN RAISE ERROR e; EXCEPTION WHEN e THEN .. EXCEPTION WHEN div_by_zero THEN .. NONSENS e and div_by_zero are synonyms Regards Pavel Stehule
> Our experience in writing the backend is that hard-wiring a single error > message text for a SQLSTATE code is the wrong thing. The SQLSTATE codes > are relatively coarse-grained (which is usually the right thing from the > perspective of an application trying to match a SQLSTATE) and there is > very often room for the text message to give more detail than the > SQLSTATE alone conveys. I can define more user's exceptions. And every can have own message text. It's personal preference. I prefere one exception, one parametrized message text. It's not important on procedure level. But If can be possible define exceptions for schema, then I can share message texts. > > Also, the above seems highly error prone to me since it decouples the > format string from the parameters. This is even worse for built-in > exception codes since the writer of a plpgsql function shouldn't assume > that he knows exactly what % parameters a built-in exception's format > string would use. Yes, it's can be source of errors. But I can check it in compile time (not now, or after apply patch (if will be). > > So for all these reasons, I think that only the SQLSTATE should be > associated with the exception name. The format string should be part > of the RAISE command. > my ideas are only proposal. Nothing more. What I think is important o using expr in raise params o set SQLSTATE for better diagnostic o raising system's exceptions all next is unnecessary luxus. But the idea EXCEPTION's variable is good, and can be easy evolved. Regards Pavel Stehule
Pavel Stehule <stehule@kix.fsv.cvut.cz> writes:
> Next problem, visibility custom exceptions. When I define exception
> variable I can't rethrown exceptions outside block when is defined. What
> is outside - some custom exception?
I don't think this is an issue. A custom exception is really just a
name for a SQLSTATE value, so you can throw it in any case. An outer
block that does not know that name can only catch it as WHEN OTHERS,
but so what?
I would also expect that matching is by SQLSTATE, that is if I write
DECLARE foo EXCEPTION = '12345';
...
RAISE ERROR foo, ...;
then some outer block written as
DECLARE bar EXCEPTION = '12345';
...
EXCEPTION WHEN bar THEN ...
would catch this error.
> disadvantage - I have to define format string everywhere where I wont to
> raise exception.
Why is that a disadvantage? You should not be able to throw an error
without providing a useful message --- that's just basic good
programming.
> From OOP view exception is object. But I need define more properties than
> one. SQLSTATE is only first, second message, level, meybe next
I think we are better off defining exception names as SQLSTATEs and
nothing else. That's essentially how we have done it in the backend
code and it has worked well over a very large body of code. You are
arguing for a less flexible definition on the basis of nothing more
than a vague appeal to "OOP principles". You have neither stated
exactly which principles nor exactly why they dictate this choice,
so I see nothing convincing in your argument.
> I think so we need more then one exception level. I can use
> user's exception for easy way of write to log.
Well, can we get away with making the syntax be
RAISE level [ exception_name , ] format_string [ , parameters ] ;
where "level" is one of the already-defined level keywords? Normally
I would think that this would be unacceptably ambiguous, but as long as
the exception_name is constrained to be either a built-in or previously
defined exception name, this is probably workable from a syntactic point
of view.
regards, tom lane
> > I would also expect that matching is by SQLSTATE, that is if I write > > DECLARE foo EXCEPTION = '12345'; > ... > RAISE ERROR foo, ...; > > then some outer block written as > > DECLARE bar EXCEPTION = '12345'; > ... > EXCEPTION WHEN bar THEN ... > if we accept exception is like variable, then there is rules for variables. I don't see problem there. EXCEPTION WHEN bar is equivalent of EXCEPTION SQLSTATE = 12345 THEN where I see bar, I can see bar. But isn't possible two exception handlers in one block with one SQLSTATE. > would catch this error. > > > disadvantage - I have to define format string everywhere where I wont to > > raise exception. > > Why is that a disadvantage? You should not be able to throw an error > without providing a useful message --- that's just basic good > programming. > it's ok. Exception without error message is wrong. One type of exception with different error messages (parametrized message is ok) is wrong too. But it's my personal opinion. Maybe one message string is not sufficient, better is message string and hint string (like now). > > From OOP view exception is object. But I need define more properties than > > one. SQLSTATE is only first, second message, level, meybe next > > I think we are better off defining exception names as SQLSTATEs and > nothing else. That's essentially how we have done it in the backend > code and it has worked well over a very large body of code. You are > arguing for a less flexible definition on the basis of nothing more > than a vague appeal to "OOP principles". You have neither stated > exactly which principles nor exactly why they dictate this choice, > so I see nothing convincing in your argument. I have less. All is only my ideas. I don't wont PLPGSQL like full OOP language. I speak only about possible ways now. I see usefull global definition of exception on package (or schema, database) level - like others db objects (sequences). Packages not exists and all is in future. On procedural level I have to agree with you. Your syntax don't exude "my" syntax. If I choise level, format_string,.. in raise stmt, then are used this params. If not, then are used default parames (in future). > > I think so we need more then one exception level. I can use > > user's exception for easy way of write to log. > > Well, can we get away with making the syntax be > > RAISE level [ exception_name , ] format_string [ , parameters ] ; > I agree. I unlike big steps. About level: I think so already defined levels are good. I have idea about somethink between levels NOTICE and EXCEPTION. I can't catch NOTICE and I have to catch EXCEPTION, maybe RAISE EVENT. I can catch it, if I wont. And this level don't rollback transaction, and should be return back from exception handler. It's more like calling subrutine. Can be usefull. Question: What do you think about I specify minimal level of event in log. But when I have user's exception I can specify list of user's exception for log. like log_level NOTICE log_exceptions myexception1, myexception2, ... > where "level" is one of the already-defined level keywords? Normally > I would think that this would be unacceptably ambiguous, but as long as > the exception_name is constrained to be either a built-in or previously > defined exception name, this is probably workable from a syntactic point > of view. Pavel Stehule
Tom Lane wrote: > That doesn't bother me either, seeing that an undefined variable isn't > detected at compile time anywhere else. However, fixing the SQLSTATE > tests by removing them doesn't seem like a great solution ... Yeah, true, I can just invoke the function to trigger the undefined variable error. > Actually, the reason I didn't do something about RAISE in 8.0 was that > I thought we should reconsider the whole design of the statement The ensuing discussion on this sounds good to me; should I apply Pavel's RAISE patch now, or wait for the subsequent work on specifying a particular SQLSTATE? -Neil
Neil Conway <neilc@samurai.com> writes:
> Tom Lane wrote:
>> Actually, the reason I didn't do something about RAISE in 8.0 was that
>> I thought we should reconsider the whole design of the statement
> The ensuing discussion on this sounds good to me; should I apply Pavel's
> RAISE patch now, or wait for the subsequent work on specifying a
> particular SQLSTATE?
The patch seems to me to be OK as far as it goes. I brought up the
other points only because I wanted to be sure that it wouldn't be
inconsistent with the next step; but it seems we're pretty well agreed
that we aren't going to do anything that would break this. So I have no
problem with applying as-is, rather than waiting for an all-inclusive
patch.
But you had mentioned wanting to look at reducing overhead by using
exec_eval_expr(); were you intending to do that before committing?
As far as the subsequent discussion itself goes, Pavel and I seem to
be pretty unsuccessful at convincing each other of our respective
visions of what an exception ought to be. Any opinions? Should
we be taking this thread to -hackers for a wider audience?
regards, tom lane
Tom Lane wrote: > But you had mentioned wanting to look at reducing overhead by using > exec_eval_expr(); were you intending to do that before committing? I'm a bit busy with other matters at the moment, so I'll probably look at it later. One question is whether we should make use of exec_eval_expr() by representing the RAISE parameter list as a list of expressions (each of which would likely be simple enough to evaluate via ExecEvalExpr()), or whether we want to extend exec_eval_expr() to handle expressions that yield multiple attributes. -Neil
Neil Conway <neilc@samurai.com> writes:
> ... One question is whether we should make use of
> exec_eval_expr() by representing the RAISE parameter list as a list of
> expressions (each of which would likely be simple enough to evaluate via
> ExecEvalExpr()), or whether we want to extend exec_eval_expr() to handle
> expressions that yield multiple attributes.
I'd lean to the former myself --- which actually does suggest that this
patch is not ready for application yet, because it banks on the
assumption that "x,y,z" should be treated as a single expression.
Now that I think about it, the amount of overhead in that assumption is
pretty high: there's tuple construction and deconstruction involved,
no matter how simple the individual datatypes are. So I'd definitely
prefer to see it changed.
regards, tom lane
Tom Lane wrote:
> I'd lean to the former myself --- which actually does suggest that this
> patch is not ready for application yet, because it banks on the
> assumption that "x,y,z" should be treated as a single expression.
Attached is a revised patch that stores the RAISE parameters as a list
of expressions, and evaluates that list via repeated application of
exec_eval_expr(). I also updated the regression tests to not remove the
SQLERRM/SQLSTATE tests. Applied to HEAD.
-Neil
Index: doc/src/sgml/plpgsql.sgml
===================================================================
RCS file: /var/lib/cvs/pgsql/doc/src/sgml/plpgsql.sgml,v
retrieving revision 1.71
diff -c -r1.71 plpgsql.sgml
*** doc/src/sgml/plpgsql.sgml 10 Jun 2005 16:23:09 -0000 1.71
--- doc/src/sgml/plpgsql.sgml 14 Jun 2005 04:57:37 -0000
***************
*** 2533,2541 ****
<para>
Inside the format string, <literal>%</literal> is replaced by the
next optional argument's string representation. Write
! <literal>%%</literal> to emit a literal <literal>%</literal>. Note
! that the optional arguments must presently be simple variables,
! not expressions, and the format must be a simple string literal.
</para>
<!--
--- 2533,2541 ----
<para>
Inside the format string, <literal>%</literal> is replaced by the
next optional argument's string representation. Write
! <literal>%%</literal> to emit a literal <literal>%</literal>.
! Arguments can be simple variables or expressions,
! and the format must be a simple string literal.
</para>
<!--
Index: src/pl/plpgsql/src/gram.y
===================================================================
RCS file: /var/lib/cvs/pgsql/src/pl/plpgsql/src/gram.y,v
retrieving revision 1.75
diff -c -r1.75 gram.y
*** src/pl/plpgsql/src/gram.y 10 Jun 2005 16:23:11 -0000 1.75
--- src/pl/plpgsql/src/gram.y 14 Jun 2005 06:24:36 -0000
***************
*** 135,142 ****
%type <exception> proc_exception
%type <condition> proc_conditions
! %type <list> raise_params
! %type <ival> raise_level raise_param
%type <str> raise_msg
%type <list> getdiag_list
--- 135,142 ----
%type <exception> proc_exception
%type <condition> proc_conditions
!
! %type <ival> raise_level
%type <str> raise_msg
%type <list> getdiag_list
***************
*** 1157,1165 ****
}
;
! stmt_raise : K_RAISE lno raise_level raise_msg raise_params ';'
{
PLpgSQL_stmt_raise *new;
new = palloc(sizeof(PLpgSQL_stmt_raise));
--- 1157,1166 ----
}
;
! stmt_raise : K_RAISE lno raise_level raise_msg
{
PLpgSQL_stmt_raise *new;
+ int tok;
new = palloc(sizeof(PLpgSQL_stmt_raise));
***************
*** 1167,1187 ****
new->lineno = $2;
new->elog_level = $3;
new->message = $4;
! new->params = $5;
! $$ = (PLpgSQL_stmt *)new;
! }
! | K_RAISE lno raise_level raise_msg ';'
! {
! PLpgSQL_stmt_raise *new;
! new = palloc(sizeof(PLpgSQL_stmt_raise));
! new->cmd_type = PLPGSQL_STMT_RAISE;
! new->lineno = $2;
! new->elog_level = $3;
! new->message = $4;
! new->params = NIL;
$$ = (PLpgSQL_stmt *)new;
}
--- 1168,1200 ----
new->lineno = $2;
new->elog_level = $3;
new->message = $4;
! new->params = NIL;
! tok = yylex();
! /*
! * We expect either a semi-colon, which
! * indicates no parameters, or a comma that
! * begins the list of parameter expressions
! */
! if (tok != ',' && tok != ';')
! yyerror("syntax error");
! if (tok == ',')
! {
! PLpgSQL_expr *expr;
! int term;
!
! for (;;)
! {
! expr = read_sql_construct(',', ';', ", or ;",
! "SELECT ",
! true, true, &term);
! new->params = lappend(new->params, expr);
! if (term == ';')
! break;
! }
! }
$$ = (PLpgSQL_stmt *)new;
}
***************
*** 1219,1240 ****
}
;
- raise_params : raise_params raise_param
- {
- $$ = lappend_int($1, $2);
- }
- | raise_param
- {
- $$ = list_make1_int($1);
- }
- ;
-
- raise_param : ',' T_SCALAR
- {
- $$ = yylval.scalar->dno;
- }
- ;
-
loop_body : proc_sect K_END K_LOOP ';'
{ $$ = $1; }
;
--- 1232,1237 ----
***************
*** 1658,1664 ****
* expected: text to use in complaining that terminator was not found
* sqlstart: text to prefix to the accumulated SQL text
* isexpression: whether to say we're reading an "expression" or a "statement"
! * valid_sql: whether to check the syntax of the expression (plus sqlstart)
* endtoken: if not NULL, ending token is stored at *endtoken
* (this is only interesting if until2 isn't zero)
*/
--- 1655,1661 ----
* expected: text to use in complaining that terminator was not found
* sqlstart: text to prefix to the accumulated SQL text
* isexpression: whether to say we're reading an "expression" or a "statement"
! * valid_sql: whether to check the syntax of the expr (prefixed with sqlstart)
* endtoken: if not NULL, ending token is stored at *endtoken
* (this is only interesting if until2 isn't zero)
*/
Index: src/pl/plpgsql/src/pl_exec.c
===================================================================
RCS file: /var/lib/cvs/pgsql/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.143
diff -c -r1.143 pl_exec.c
*** src/pl/plpgsql/src/pl_exec.c 10 Jun 2005 16:23:11 -0000 1.143
--- src/pl/plpgsql/src/pl_exec.c 14 Jun 2005 06:25:50 -0000
***************
*** 594,600 ****
error_context_stack = plerrcontext.previous;
/*
! * Return the triggers result
*/
return rettup;
}
--- 594,600 ----
error_context_stack = plerrcontext.previous;
/*
! * Return the trigger's result
*/
return rettup;
}
***************
*** 1095,1116 ****
exec_stmt_perform(PLpgSQL_execstate *estate, PLpgSQL_stmt_perform *stmt)
{
PLpgSQL_expr *expr = stmt->expr;
- int rc;
-
- /*
- * If not already done create a plan for this expression
- */
- if (expr->plan == NULL)
- exec_prepare_plan(estate, expr);
-
- rc = exec_run_select(estate, expr, 0, NULL);
- if (rc != SPI_OK_SELECT)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("query \"%s\" did not return data", expr->query)));
exec_set_found(estate, (estate->eval_processed != 0));
-
exec_eval_cleanup(estate);
return PLPGSQL_RC_OK;
--- 1095,1103 ----
exec_stmt_perform(PLpgSQL_execstate *estate, PLpgSQL_stmt_perform *stmt)
{
PLpgSQL_expr *expr = stmt->expr;
+ (void) exec_run_select(estate, expr, 0, NULL);
exec_set_found(estate, (estate->eval_processed != 0));
exec_eval_cleanup(estate);
return PLPGSQL_RC_OK;
***************
*** 1941,1955 ****
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("too few parameters specified for RAISE")));
! exec_eval_datum(estate, estate->datums[lfirst_int(current_param)],
! InvalidOid,
! ¶mtypeid, ¶mvalue, ¶misnull);
if (paramisnull)
extval = "<NULL>";
else
extval = convert_value_to_string(paramvalue, paramtypeid);
plpgsql_dstring_append(&ds, extval);
current_param = lnext(current_param);
continue;
}
--- 1928,1945 ----
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("too few parameters specified for RAISE")));
! paramvalue = exec_eval_expr(estate,
! (PLpgSQL_expr *) lfirst(current_param),
! ¶misnull,
! ¶mtypeid);
!
if (paramisnull)
extval = "<NULL>";
else
extval = convert_value_to_string(paramvalue, paramtypeid);
plpgsql_dstring_append(&ds, extval);
current_param = lnext(current_param);
+ exec_eval_cleanup(estate);
continue;
}
Index: src/pl/plpgsql/src/pl_funcs.c
===================================================================
RCS file: /var/lib/cvs/pgsql/src/pl/plpgsql/src/pl_funcs.c,v
retrieving revision 1.42
diff -c -r1.42 pl_funcs.c
*** src/pl/plpgsql/src/pl_funcs.c 14 Jun 2005 00:10:02 -0000 1.42
--- src/pl/plpgsql/src/pl_funcs.c 14 Jun 2005 06:23:23 -0000
***************
*** 885,897 ****
static void
dump_raise(PLpgSQL_stmt_raise *stmt)
{
! ListCell *l;
dump_ind();
! printf("RAISE '%s'", stmt->message);
! foreach (l, stmt->params)
! printf(" %d", lfirst_int(l));
! printf("\n");
}
static void
--- 885,904 ----
static void
dump_raise(PLpgSQL_stmt_raise *stmt)
{
! ListCell *lc;
! int i = 0;
dump_ind();
! printf("RAISE '%s'\n", stmt->message);
! dump_indent += 2;
! foreach (lc, stmt->params)
! {
! dump_ind();
! printf(" parameter %d: ", i++);
! dump_expr((PLpgSQL_expr *) lfirst(lc));
! printf("\n");
! }
! dump_indent -= 2;
}
static void
***************
*** 916,922 ****
{
dump_ind();
printf(" target = %d %s\n", stmt->rec->recno, stmt->rec->refname);
! } else if (stmt->row != NULL)
{
dump_ind();
printf(" target = %d %s\n", stmt->row->rowno, stmt->row->refname);
--- 923,930 ----
{
dump_ind();
printf(" target = %d %s\n", stmt->rec->recno, stmt->rec->refname);
! }
! else if (stmt->row != NULL)
{
dump_ind();
printf(" target = %d %s\n", stmt->row->rowno, stmt->row->refname);
Index: src/pl/plpgsql/src/plpgsql.h
===================================================================
RCS file: /var/lib/cvs/pgsql/src/pl/plpgsql/src/plpgsql.h,v
retrieving revision 1.62
diff -c -r1.62 plpgsql.h
*** src/pl/plpgsql/src/plpgsql.h 10 Jun 2005 16:23:11 -0000 1.62
--- src/pl/plpgsql/src/plpgsql.h 14 Jun 2005 06:33:24 -0000
***************
*** 515,521 ****
int lineno;
int elog_level;
char *message;
! List *params;
} PLpgSQL_stmt_raise;
--- 515,521 ----
int lineno;
int elog_level;
char *message;
! List *params; /* list of expressions */
} PLpgSQL_stmt_raise;
Index: src/test/regress/expected/plpgsql.out
===================================================================
RCS file: /var/lib/cvs/pgsql/src/test/regress/expected/plpgsql.out,v
retrieving revision 1.33
diff -c -r1.33 plpgsql.out
*** src/test/regress/expected/plpgsql.out 10 Jun 2005 16:23:11 -0000 1.33
--- src/test/regress/expected/plpgsql.out 14 Jun 2005 06:30:31 -0000
***************
*** 2418,2434 ****
--
-- SQLSTATE and SQLERRM test
--
! -- should fail: SQLSTATE and SQLERRM are only in defined EXCEPTION
! -- blocks
! create function excpt_test() returns void as $$
begin
raise notice '% %', sqlstate, sqlerrm;
end; $$ language plpgsql;
! ERROR: syntax error at or near "sqlstate" at character 79
! LINE 3: raise notice '% %', sqlstate, sqlerrm;
! ^
! -- should fail
! create function excpt_test() returns void as $$
begin
begin
begin
--- 2418,2434 ----
--
-- SQLSTATE and SQLERRM test
--
! create function excpt_test1() returns void as $$
begin
raise notice '% %', sqlstate, sqlerrm;
end; $$ language plpgsql;
! -- should fail: SQLSTATE and SQLERRM are only in defined EXCEPTION
! -- blocks
! select excpt_test1();
! ERROR: column "sqlstate" does not exist
! CONTEXT: SQL statement "SELECT sqlstate"
! PL/pgSQL function "excpt_test1" line 2 at raise
! create function excpt_test2() returns void as $$
begin
begin
begin
***************
*** 2436,2445 ****
end;
end;
end; $$ language plpgsql;
! ERROR: syntax error at or near "sqlstate" at character 108
! LINE 5: raise notice '% %', sqlstate, sqlerrm;
! ^
! create function excpt_test() returns void as $$
begin
begin
raise exception 'user exception';
--- 2436,2447 ----
end;
end;
end; $$ language plpgsql;
! -- should fail
! select excpt_test2();
! ERROR: column "sqlstate" does not exist
! CONTEXT: SQL statement "SELECT sqlstate"
! PL/pgSQL function "excpt_test2" line 4 at raise
! create function excpt_test3() returns void as $$
begin
begin
raise exception 'user exception';
***************
*** 2458,2471 ****
raise notice '% %', sqlstate, sqlerrm;
end;
end; $$ language plpgsql;
! select excpt_test();
NOTICE: caught exception P0001 user exception
NOTICE: P0001 user exception
NOTICE: caught exception 22012 division by zero
NOTICE: P0001 user exception
! excpt_test
! ------------
(1 row)
! drop function excpt_test();
--- 2460,2493 ----
raise notice '% %', sqlstate, sqlerrm;
end;
end; $$ language plpgsql;
! select excpt_test3();
NOTICE: caught exception P0001 user exception
NOTICE: P0001 user exception
NOTICE: caught exception 22012 division by zero
NOTICE: P0001 user exception
! excpt_test3
! -------------
!
! (1 row)
!
! drop function excpt_test1();
! drop function excpt_test2();
! drop function excpt_test3();
! -- parameters of raise stmt can be expressions
! create function raise_exprs() returns void as $$
! declare
! a integer[] = '{10,20,30}';
! c varchar = 'xyz';
! i integer;
! begin
! i := 2;
! raise notice '%; %; %; %; %; %', a, a[i], c, (select c || 'abc'), row(10,'aaa',NULL,30), NULL;
! end;$$ language plpgsql;
! select raise_exprs();
! NOTICE: {10,20,30}; 20; xyz; xyzabc; (10,aaa,,30); <NULL>
! raise_exprs
! -------------
(1 row)
! drop function raise_exprs();
Index: src/test/regress/sql/plpgsql.sql
===================================================================
RCS file: /var/lib/cvs/pgsql/src/test/regress/sql/plpgsql.sql,v
retrieving revision 1.28
diff -c -r1.28 plpgsql.sql
*** src/test/regress/sql/plpgsql.sql 10 Jun 2005 16:23:11 -0000 1.28
--- src/test/regress/sql/plpgsql.sql 14 Jun 2005 06:26:52 -0000
***************
*** 2055,2069 ****
-- SQLSTATE and SQLERRM test
--
! -- should fail: SQLSTATE and SQLERRM are only in defined EXCEPTION
! -- blocks
! create function excpt_test() returns void as $$
begin
raise notice '% %', sqlstate, sqlerrm;
end; $$ language plpgsql;
! -- should fail
! create function excpt_test() returns void as $$
begin
begin
begin
--- 2055,2069 ----
-- SQLSTATE and SQLERRM test
--
! create function excpt_test1() returns void as $$
begin
raise notice '% %', sqlstate, sqlerrm;
end; $$ language plpgsql;
+ -- should fail: SQLSTATE and SQLERRM are only in defined EXCEPTION
+ -- blocks
+ select excpt_test1();
! create function excpt_test2() returns void as $$
begin
begin
begin
***************
*** 2071,2078 ****
end;
end;
end; $$ language plpgsql;
! create function excpt_test() returns void as $$
begin
begin
raise exception 'user exception';
--- 2071,2080 ----
end;
end;
end; $$ language plpgsql;
+ -- should fail
+ select excpt_test2();
! create function excpt_test3() returns void as $$
begin
begin
raise exception 'user exception';
***************
*** 2092,2096 ****
end;
end; $$ language plpgsql;
! select excpt_test();
! drop function excpt_test();
--- 2094,2114 ----
end;
end; $$ language plpgsql;
! select excpt_test3();
! drop function excpt_test1();
! drop function excpt_test2();
! drop function excpt_test3();
!
! -- parameters of raise stmt can be expressions
! create function raise_exprs() returns void as $$
! declare
! a integer[] = '{10,20,30}';
! c varchar = 'xyz';
! i integer;
! begin
! i := 2;
! raise notice '%; %; %; %; %; %', a, a[i], c, (select c || 'abc'), row(10,'aaa',NULL,30), NULL;
! end;$$ language plpgsql;
!
! select raise_exprs();
! drop function raise_exprs();
Tom Lane wrote: > I would also expect that matching is by SQLSTATE, that is if I write > > DECLARE foo EXCEPTION = '12345'; > ... > RAISE ERROR foo, ...; BTW, is there any value in a separate "EXCEPTION" type? ISTM that an exception is just a SQLSTATE, which is in turn just a string. A separate exception type does simplify the parsing of RAISE, but I wonder if it would be useful to be able to also allow specifying the SQLSTATE code as a string literal. > I think we are better off defining exception names as SQLSTATEs and > nothing else. That seems the right way to go to me. > Well, can we get away with making the syntax be > > RAISE level [ exception_name , ] format_string [ , parameters ] ; > > where "level" is one of the already-defined level keywords? I think we can. I don't see the point of inventing a new RAISE level for exceptions with a custom SQLSTATE. For one thing, it would be useful to be able to specify a custom SQLSTATE for a RAISE WARNING. -Neil
Hello,
the name of exception's variable I use as exception's name. Attached patch
work, but miss documentations, regress, ..
>BTW, is there any value in a separate "EXCEPTION" type? ISTM that an
>exception is just a SQLSTATE, which is in turn just a string. A separate
>exception type does simplify the parsing of RAISE, but I wonder if it
>would be useful to be able to also allow specifying the SQLSTATE code as
>a string literal.
Definition new attributes for exception isn't problem: level, errmsg.
Parsing raise stmt will be little bit more complicete. But why now?
CREATE OR REPLACE FUNCTION foo() RETURNS void AS $$
DECLARE my_own_exception EXCEPTION;
BEGIN
RAISE EXCEPTION my_own_exception 'some text';
END; $$ LANGUAGE plpgsql;
pokus=# select foo();
ERROR: some text
DETAIL: User's exception sqlstate: U0001, name: my_own_exception
HINT: Unhandled user's exception, from RAISE stmt on line 3
pokus=#
CREATE OR REPLACE FUNCTION foo() RETURNS void AS $$
DECLARE div_by_zero_test EXCEPTION = '22012';
BEGIN
RAISE EXCEPTION div_by_zero_test 'some text';
EXCEPTION WHEN division_by_zero THEN
RAISE NOTICE 'foo text';
END; $$ LANGUAGE plpgsql;
pokus=# select foo();
NOTICE: foo text
foo
-----
(1 row)
CREATE OR REPLACE FUNCTION foo() RETURNS void AS $$
DECLARE uexcpt01 EXCEPTION;
BEGIN
RAISE EXCEPTION uexcpt01 'aaaa';
EXCEPTION WHEN uexcpt01 THEN
RAISE NOTICE 'hello';
END; $$ LANGUAGE plpgsql;
pokus=# select foo();
NOTICE: hello
The patch isn't in production state (no from me:), but maybe is usefull
for test. The moust important is posibility handling own exception without
parsing SQLERRMS, I think. Setting SQLSTATE is usefull for
interoperatibility between procedures and throwing system errors.
Regards
Pavel Stehule
Attachment
Neil Conway <neilc@samurai.com> writes:
> BTW, is there any value in a separate "EXCEPTION" type? ISTM that an
> exception is just a SQLSTATE, which is in turn just a string. A separate
> exception type does simplify the parsing of RAISE, but I wonder if it
> would be useful to be able to also allow specifying the SQLSTATE code as
> a string literal.
It would save some typing, but I do not think we can make the proposed
syntax work if we do it that way:
>> RAISE level [ exception_name , ] format_string [ , parameters ] ;
>>
>> where "level" is one of the already-defined level keywords?
How will you tell whether the string literal just after "level" is meant
to be a SQLSTATE or a format? Maybe with some ad-hoc tests, but ugh ...
regards, tom lane