Thread: pl/pgsql: END verbosity [patch]
Hello this patch allows optional using label with END and END LOOP. Ending label has only informational value, but can enhance readability large block and enhance likeness with Oracle. <<main>>LOOP ... ... END LOOP<<main>>; Regards Pavel Stehule
Attachment
Hello
Per small recent discussion I corrected patch user's exception.
diff: User can choise any sqlstate (without class U0, which I reserve as
range for default values sqlstates - if user don't spec sqlstate, is
used value from this range). There is only basic changes in documentation
and needs enhancing. I am not able to do (I am sorry, my english is poor).
Note: patch don't create deep changes in plpgsql core. Only enhance stmts
DECLARE, RAISE and EXCEPTION condition.
Next ToDo (needs discussion):
+ Optional message in raise stmt for user's or system exception
raise exception division_by_zero;
+ Possibility rethrown exception
raise;
Regards
Pavel Stehule
Attachment
Pavel Stehule wrote:
> this patch allows optional using label with END and END LOOP. Ending label
> has only informational value, but can enhance readability large block and
> enhance likeness with Oracle.
>
> <<main>>LOOP
> ...
> ...
> END LOOP<<main>>;
Attached is a revised version of this patch. Changes / comments:
- AFAICS Oracle's syntax is actually
<<label>> LOOP
...
END LOOP label;
i.e. the ending block label isn't enclosed in <<>>. I've adjusted the
patch accordingly.
- your patch broke EXIT and CONTINUE, as running the regression tests
would have made clear.
- yyerror() will set plpgsql_error_lineno, so you needn't do it
yourself. I changed it to use ereport(ERROR) anyway, as it seems a bit
more appropriate. I'm not quite happy with the error message text:
ERROR: end label "outer_label" differs from block's label "inner_label"
CONTEXT: compile of PL/pgSQL function "end_label3" near line 6
ERROR: end label "outer_label" specified for unlabelled block
CONTEXT: compile of PL/pgSQL function "end_label4" near line 5
suggestions for improvement are welcome.
BTW, I notice that some but not all the call sites of ereport(ERROR) in
PL/PgSQL's gram.y set plpgsql_error_lineno. Is there a reason for this?
Barring any objections, I'll apply the attached patch to CVS tomorrow.
-Neil
Index: doc/src/sgml/plpgsql.sgml
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql/doc/src/sgml/plpgsql.sgml,v
retrieving revision 1.74
diff -c -r1.74 plpgsql.sgml
*** doc/src/sgml/plpgsql.sgml 22 Jun 2005 01:35:02 -0000 1.74
--- doc/src/sgml/plpgsql.sgml 1 Jul 2005 11:43:36 -0000
***************
*** 456,462 ****
<replaceable>declarations</replaceable> </optional>
BEGIN
<replaceable>statements</replaceable>
! END;
</synopsis>
</para>
--- 456,462 ----
<replaceable>declarations</replaceable> </optional>
BEGIN
<replaceable>statements</replaceable>
! END <optional> <replaceable>label</replaceable> </optional>;
</synopsis>
</para>
***************
*** 1789,1806 ****
<title><literal>LOOP</></title>
<synopsis>
! <optional><<<replaceable>label</replaceable>>></optional>
LOOP
<replaceable>statements</replaceable>
! END LOOP;
</synopsis>
<para>
! <literal>LOOP</> defines an unconditional loop that is repeated indefinitely
! until terminated by an <literal>EXIT</> or <command>RETURN</command>
! statement. The optional label can be used by <literal>EXIT</> statements in
! nested loops to specify which level of nesting should be
! terminated.
</para>
</sect3>
--- 1789,1807 ----
<title><literal>LOOP</></title>
<synopsis>
! <optional> <<<replaceable>label</replaceable>>> </optional>
LOOP
<replaceable>statements</replaceable>
! END LOOP <optional> <replaceable>label</replaceable> </optional>;
</synopsis>
<para>
! <literal>LOOP</> defines an unconditional loop that is repeated
! indefinitely until terminated by an <literal>EXIT</> or
! <command>RETURN</command> statement. The optional
! <replaceable>label</replaceable> can be used by <literal>EXIT</>
! and <literal>CONTINUE</literal> statements in nested loops to
! specify which loop the statement should be applied to.
</para>
</sect3>
***************
*** 1920,1929 ****
</indexterm>
<synopsis>
! <optional><<<replaceable>label</replaceable>>></optional>
WHILE <replaceable>expression</replaceable> LOOP
<replaceable>statements</replaceable>
! END LOOP;
</synopsis>
<para>
--- 1921,1930 ----
</indexterm>
<synopsis>
! <optional> <<<replaceable>label</replaceable>>> </optional>
WHILE <replaceable>expression</replaceable> LOOP
<replaceable>statements</replaceable>
! END LOOP <optional> <replaceable>label</replaceable> </optional>;
</synopsis>
<para>
***************
*** 1951,1960 ****
<title><literal>FOR</> (integer variant)</title>
<synopsis>
! <optional><<<replaceable>label</replaceable>>></optional>
FOR <replaceable>name</replaceable> IN <optional> REVERSE </optional> <replaceable>expression</replaceable> ..
<replaceable>expression</replaceable>LOOP
<replaceable>statements</replaceable>
! END LOOP;
</synopsis>
<para>
--- 1952,1961 ----
<title><literal>FOR</> (integer variant)</title>
<synopsis>
! <optional> <<<replaceable>label</replaceable>>> </optional>
FOR <replaceable>name</replaceable> IN <optional> REVERSE </optional> <replaceable>expression</replaceable> ..
<replaceable>expression</replaceable>LOOP
<replaceable>statements</replaceable>
! END LOOP <optional> <replaceable>labal</replaceable> </optional>;
</synopsis>
<para>
***************
*** 1997,2006 ****
the results of a query and manipulate that data
accordingly. The syntax is:
<synopsis>
! <optional><<<replaceable>label</replaceable>>></optional>
FOR <replaceable>record_or_row</replaceable> IN <replaceable>query</replaceable> LOOP
<replaceable>statements</replaceable>
! END LOOP;
</synopsis>
The record or row variable is successively assigned each row
resulting from the <replaceable>query</replaceable> (which must be a
--- 1998,2007 ----
the results of a query and manipulate that data
accordingly. The syntax is:
<synopsis>
! <optional> <<<replaceable>label</replaceable>>> </optional>
FOR <replaceable>record_or_row</replaceable> IN <replaceable>query</replaceable> LOOP
<replaceable>statements</replaceable>
! END LOOP <optional> <replaceable>label</replaceable> </optional>;
</synopsis>
The record or row variable is successively assigned each row
resulting from the <replaceable>query</replaceable> (which must be a
***************
*** 2036,2045 ****
The <literal>FOR-IN-EXECUTE</> statement is another way to iterate over
rows:
<synopsis>
! <optional><<<replaceable>label</replaceable>>></optional>
FOR <replaceable>record_or_row</replaceable> IN EXECUTE <replaceable>text_expression</replaceable> LOOP
<replaceable>statements</replaceable>
! END LOOP;
</synopsis>
This is like the previous form, except that the source
<command>SELECT</command> statement is specified as a string
--- 2037,2046 ----
The <literal>FOR-IN-EXECUTE</> statement is another way to iterate over
rows:
<synopsis>
! <optional> <<<replaceable>label</replaceable>>> </optional>
FOR <replaceable>record_or_row</replaceable> IN EXECUTE <replaceable>text_expression</replaceable> LOOP
<replaceable>statements</replaceable>
! END LOOP <optional> <replaceable>label</replaceable> </optional>;
</synopsis>
This is like the previous form, except that the source
<command>SELECT</command> statement is specified as a string
Index: src/pl/plpgsql/src/gram.y
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql/src/pl/plpgsql/src/gram.y,v
retrieving revision 1.77
diff -c -r1.77 gram.y
*** src/pl/plpgsql/src/gram.y 22 Jun 2005 01:35:02 -0000 1.77
--- src/pl/plpgsql/src/gram.y 1 Jul 2005 12:08:45 -0000
***************
*** 56,61 ****
--- 56,63 ----
PLpgSQL_datum *initial_datum);
static void check_sql_expr(const char *stmt);
static void plpgsql_sql_error_callback(void *arg);
+ static void check_labels(const char *start_label,
+ const char *end_label);
%}
***************
*** 69,75 ****
int lineno;
} varname;
struct
! {
char *name;
int lineno;
PLpgSQL_rec *rec;
--- 71,77 ----
int lineno;
} varname;
struct
! {
char *name;
int lineno;
PLpgSQL_rec *rec;
***************
*** 81,86 ****
--- 83,93 ----
int n_initvars;
int *initvarnos;
} declhdr;
+ struct
+ {
+ char *end_label;
+ List *stmts;
+ } loop_body;
List *list;
PLpgSQL_type *dtype;
PLpgSQL_datum *scalar; /* a VAR, RECFIELD, or TRIGARG */
***************
*** 119,129 ****
%type <forvariable> for_variable
%type <stmt> for_control
! %type <str> opt_lblname opt_label
! %type <str> opt_exitlabel
%type <str> execsql_start
! %type <list> proc_sect proc_stmts stmt_else loop_body
%type <stmt> proc_stmt pl_block
%type <stmt> stmt_assign stmt_if stmt_loop stmt_while stmt_exit
%type <stmt> stmt_return stmt_return_next stmt_raise stmt_execsql
--- 126,136 ----
%type <forvariable> for_variable
%type <stmt> for_control
! %type <str> opt_lblname opt_block_label opt_label
%type <str> execsql_start
! %type <list> proc_sect proc_stmts stmt_else
! %type <loop_body> loop_body
%type <stmt> proc_stmt pl_block
%type <stmt> stmt_assign stmt_if stmt_loop stmt_while stmt_exit
%type <stmt> stmt_return stmt_return_next stmt_raise stmt_execsql
***************
*** 248,257 ****
| ';'
;
! pl_block : decl_sect K_BEGIN lno proc_sect exception_sect K_END
{
PLpgSQL_stmt_block *new;
new = palloc0(sizeof(PLpgSQL_stmt_block));
new->cmd_type = PLPGSQL_STMT_BLOCK;
--- 255,266 ----
| ';'
;
! pl_block : decl_sect K_BEGIN lno proc_sect exception_sect K_END opt_label
{
PLpgSQL_stmt_block *new;
+ check_labels($1.label, $7);
+
new = palloc0(sizeof(PLpgSQL_stmt_block));
new->cmd_type = PLPGSQL_STMT_BLOCK;
***************
*** 269,275 ****
;
! decl_sect : opt_label
{
plpgsql_ns_setlocal(false);
$$.label = $1;
--- 278,284 ----
;
! decl_sect : opt_block_label
{
plpgsql_ns_setlocal(false);
$$.label = $1;
***************
*** 277,283 ****
$$.initvarnos = NULL;
plpgsql_add_initdatums(NULL);
}
! | opt_label decl_start
{
plpgsql_ns_setlocal(false);
$$.label = $1;
--- 286,292 ----
$$.initvarnos = NULL;
plpgsql_add_initdatums(NULL);
}
! | opt_block_label decl_start
{
plpgsql_ns_setlocal(false);
$$.label = $1;
***************
*** 285,291 ****
$$.initvarnos = NULL;
plpgsql_add_initdatums(NULL);
}
! | opt_label decl_start decl_stmts
{
plpgsql_ns_setlocal(false);
if ($3 != NULL)
--- 294,300 ----
$$.initvarnos = NULL;
plpgsql_add_initdatums(NULL);
}
! | opt_block_label decl_start decl_stmts
{
plpgsql_ns_setlocal(false);
if ($3 != NULL)
***************
*** 409,415 ****
plpgsql_ns_setlocal(false);
query = read_sql_stmt("");
plpgsql_ns_setlocal(true);
!
$$ = query;
}
;
--- 418,424 ----
plpgsql_ns_setlocal(false);
query = read_sql_stmt("");
plpgsql_ns_setlocal(true);
!
$$ = query;
}
;
***************
*** 757,763 ****
* ... ...
* ELSE ELSE
* ... ...
! * END IF END IF
* END IF
*/
PLpgSQL_stmt_if *new_if;
--- 766,772 ----
* ... ...
* ELSE ELSE
* ... ...
! * END IF END IF
* END IF
*/
PLpgSQL_stmt_if *new_if;
***************
*** 776,786 ****
| K_ELSE proc_sect
{
! $$ = $2;
}
;
! stmt_loop : opt_label K_LOOP lno loop_body
{
PLpgSQL_stmt_loop *new;
--- 785,795 ----
| K_ELSE proc_sect
{
! $$ = $2;
}
;
! stmt_loop : opt_block_label K_LOOP lno loop_body
{
PLpgSQL_stmt_loop *new;
***************
*** 788,802 ****
new->cmd_type = PLPGSQL_STMT_LOOP;
new->lineno = $3;
new->label = $1;
! new->body = $4;
plpgsql_ns_pop();
$$ = (PLpgSQL_stmt *)new;
}
;
! stmt_while : opt_label K_WHILE lno expr_until_loop loop_body
{
PLpgSQL_stmt_while *new;
--- 797,812 ----
new->cmd_type = PLPGSQL_STMT_LOOP;
new->lineno = $3;
new->label = $1;
! new->body = $4.stmts;
+ check_labels($1, $4.end_label);
plpgsql_ns_pop();
$$ = (PLpgSQL_stmt *)new;
}
;
! stmt_while : opt_block_label K_WHILE lno expr_until_loop loop_body
{
PLpgSQL_stmt_while *new;
***************
*** 805,819 ****
new->lineno = $3;
new->label = $1;
new->cond = $4;
! new->body = $5;
plpgsql_ns_pop();
$$ = (PLpgSQL_stmt *)new;
}
;
! stmt_for : opt_label K_FOR for_control loop_body
{
/* This runs after we've scanned the loop body */
if ($3->cmd_type == PLPGSQL_STMT_FORI)
--- 815,830 ----
new->lineno = $3;
new->label = $1;
new->cond = $4;
! new->body = $5.stmts;
+ check_labels($1, $5.end_label);
plpgsql_ns_pop();
$$ = (PLpgSQL_stmt *)new;
}
;
! stmt_for : opt_block_label K_FOR for_control loop_body
{
/* This runs after we've scanned the loop body */
if ($3->cmd_type == PLPGSQL_STMT_FORI)
***************
*** 822,828 ****
new = (PLpgSQL_stmt_fori *) $3;
new->label = $1;
! new->body = $4;
$$ = (PLpgSQL_stmt *) new;
}
else if ($3->cmd_type == PLPGSQL_STMT_FORS)
--- 833,839 ----
new = (PLpgSQL_stmt_fori *) $3;
new->label = $1;
! new->body = $4.stmts;
$$ = (PLpgSQL_stmt *) new;
}
else if ($3->cmd_type == PLPGSQL_STMT_FORS)
***************
*** 831,837 ****
new = (PLpgSQL_stmt_fors *) $3;
new->label = $1;
! new->body = $4;
$$ = (PLpgSQL_stmt *) new;
}
else
--- 842,848 ----
new = (PLpgSQL_stmt_fors *) $3;
new->label = $1;
! new->body = $4.stmts;
$$ = (PLpgSQL_stmt *) new;
}
else
***************
*** 841,850 ****
Assert($3->cmd_type == PLPGSQL_STMT_DYNFORS);
new = (PLpgSQL_stmt_dynfors *) $3;
new->label = $1;
! new->body = $4;
$$ = (PLpgSQL_stmt *) new;
}
/* close namespace started in opt_label */
plpgsql_ns_pop();
}
--- 852,862 ----
Assert($3->cmd_type == PLPGSQL_STMT_DYNFORS);
new = (PLpgSQL_stmt_dynfors *) $3;
new->label = $1;
! new->body = $4.stmts;
$$ = (PLpgSQL_stmt *) new;
}
+ check_labels($1, $4.end_label);
/* close namespace started in opt_label */
plpgsql_ns_pop();
}
***************
*** 1037,1043 ****
}
;
! stmt_exit : exit_type lno opt_exitlabel opt_exitcond
{
PLpgSQL_stmt_exit *new;
--- 1049,1055 ----
}
;
! stmt_exit : exit_type lno opt_label opt_exitcond
{
PLpgSQL_stmt_exit *new;
***************
*** 1245,1252 ****
}
;
! loop_body : proc_sect K_END K_LOOP ';'
! { $$ = $1; }
;
stmt_execsql : execsql_start lno
--- 1257,1267 ----
}
;
! loop_body : proc_sect K_END K_LOOP opt_label ';'
! {
! $$.stmts = $1;
! $$.end_label = $4;
! }
;
stmt_execsql : execsql_start lno
***************
*** 1262,1268 ****
}
;
! stmt_dynexecute : K_EXECUTE lno
{
PLpgSQL_stmt_dynexecute *new;
PLpgSQL_expr *expr;
--- 1277,1283 ----
}
;
! stmt_dynexecute : K_EXECUTE lno
{
PLpgSQL_stmt_dynexecute *new;
PLpgSQL_expr *expr;
***************
*** 1418,1424 ****
errmsg("cursor \"%s\" has no arguments",
$3->refname)));
}
!
if (tok != ';')
{
plpgsql_error_lineno = plpgsql_scanner_lineno();
--- 1433,1439 ----
errmsg("cursor \"%s\" has no arguments",
$3->refname)));
}
!
if (tok != ';')
{
plpgsql_error_lineno = plpgsql_scanner_lineno();
***************
*** 1596,1602 ****
{ $$ = plpgsql_read_expression(K_LOOP, "LOOP"); }
;
! opt_label :
{
plpgsql_ns_push(NULL);
$$ = NULL;
--- 1611,1617 ----
{ $$ = plpgsql_read_expression(K_LOOP, "LOOP"); }
;
! opt_block_label :
{
plpgsql_ns_push(NULL);
$$ = NULL;
***************
*** 1608,1621 ****
}
;
! opt_exitlabel :
! { $$ = NULL; }
| T_LABEL
{
! char *name;
!
! plpgsql_convert_ident(yytext, &name, 1);
! $$ = name;
}
| T_WORD
{
--- 1623,1637 ----
}
;
! opt_label :
! {
! $$ = NULL;
! }
| T_LABEL
{
! char *label_name;
! plpgsql_convert_ident(yytext, &label_name, 1);
! $$ = label_name;
}
| T_WORD
{
***************
*** 2210,2213 ****
--- 2226,2254 ----
errposition(0);
}
+ static void
+ check_labels(const char *start_label, const char *end_label)
+ {
+ if (end_label)
+ {
+ if (!start_label)
+ {
+ plpgsql_error_lineno = plpgsql_scanner_lineno();
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("end label \"%s\" specified for unlabelled block",
+ end_label)));
+ }
+
+ if (strcmp(start_label, end_label) != 0)
+ {
+ plpgsql_error_lineno = plpgsql_scanner_lineno();
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("end label \"%s\" differs from block's label \"%s\"",
+ end_label, start_label)));
+ }
+ }
+ }
+
#include "pl_scan.c"
Index: src/test/regress/expected/plpgsql.out
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql/src/test/regress/expected/plpgsql.out,v
retrieving revision 1.36
diff -c -r1.36 plpgsql.out
*** src/test/regress/expected/plpgsql.out 22 Jun 2005 07:28:47 -0000 1.36
--- src/test/regress/expected/plpgsql.out 1 Jul 2005 11:53:55 -0000
***************
*** 2491,2497 ****
(1 row)
drop function raise_exprs();
! -- continue statement
create table conttesttbl(idx serial, v integer);
NOTICE: CREATE TABLE will create implicit sequence "conttesttbl_idx_seq" for serial column "conttesttbl.idx"
insert into conttesttbl(v) values(10);
--- 2491,2497 ----
(1 row)
drop function raise_exprs();
! -- continue statement
create table conttesttbl(idx serial, v integer);
NOTICE: CREATE TABLE will create implicit sequence "conttesttbl_idx_seq" for serial column "conttesttbl.idx"
insert into conttesttbl(v) values(10);
***************
*** 2532,2538 ****
for _i in 1..10 loop
begin
-- applies to outer loop, not the nested begin block
! continue when _i < 5;
raise notice '%', _i;
end;
end loop;
--- 2532,2538 ----
for _i in 1..10 loop
begin
-- applies to outer loop, not the nested begin block
! continue when _i < 5;
raise notice '%', _i;
end;
end loop;
***************
*** 2666,2668 ****
--- 2666,2723 ----
drop function continue_test2();
drop function continue_test3();
drop table conttesttbl;
+ -- verbose end block and end loop
+ create function end_label1() returns void as $$
+ <<blbl>>
+ begin
+ <<flbl1>>
+ for _i in 1 .. 10 loop
+ exit flbl1;
+ end loop flbl1;
+ <<flbl2>>
+ for _i in 1 .. 10 loop
+ exit flbl2;
+ end loop;
+ end blbl;
+ $$ language plpgsql;
+ select end_label1();
+ end_label1
+ ------------
+
+ (1 row)
+
+ drop function end_label1();
+ -- should fail: undefined end label
+ create function end_label2() returns void as $$
+ begin
+ for _i in 1 .. 10 loop
+ exit;
+ end loop flbl1;
+ end;
+ $$ language plpgsql;
+ ERROR: no such label at or near "flbl1" at character 101
+ LINE 5: end loop flbl1;
+ ^
+ -- should fail: end label does not match start label
+ create function end_label3() returns void as $$
+ <<outer_label>>
+ begin
+ <<inner_label>>
+ for _i in 1 .. 10 loop
+ exit;
+ end loop outer_label;
+ end;
+ $$ language plpgsql;
+ ERROR: end label "outer_label" differs from block's label "inner_label"
+ CONTEXT: compile of PL/pgSQL function "end_label3" near line 6
+ -- should fail: end label on a block without a start label
+ create function end_label4() returns void as $$
+ <<outer_label>>
+ begin
+ for _i in 1 .. 10 loop
+ exit;
+ end loop outer_label;
+ end;
+ $$ language plpgsql;
+ ERROR: end label "outer_label" specified for unlabelled block
+ CONTEXT: compile of PL/pgSQL function "end_label4" near line 5
Index: src/test/regress/sql/plpgsql.sql
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql/src/test/regress/sql/plpgsql.sql,v
retrieving revision 1.31
diff -c -r1.31 plpgsql.sql
*** src/test/regress/sql/plpgsql.sql 22 Jun 2005 07:28:47 -0000 1.31
--- src/test/regress/sql/plpgsql.sql 1 Jul 2005 11:43:36 -0000
***************
*** 2113,2119 ****
select raise_exprs();
drop function raise_exprs();
! -- continue statement
create table conttesttbl(idx serial, v integer);
insert into conttesttbl(v) values(10);
insert into conttesttbl(v) values(20);
--- 2113,2119 ----
select raise_exprs();
drop function raise_exprs();
! -- continue statement
create table conttesttbl(idx serial, v integer);
insert into conttesttbl(v) values(10);
insert into conttesttbl(v) values(20);
***************
*** 2154,2160 ****
for _i in 1..10 loop
begin
-- applies to outer loop, not the nested begin block
! continue when _i < 5;
raise notice '%', _i;
end;
end loop;
--- 2154,2160 ----
for _i in 1..10 loop
begin
-- applies to outer loop, not the nested begin block
! continue when _i < 5;
raise notice '%', _i;
end;
end loop;
***************
*** 2232,2234 ****
--- 2232,2282 ----
drop function continue_test2();
drop function continue_test3();
drop table conttesttbl;
+
+ -- verbose end block and end loop
+ create function end_label1() returns void as $$
+ <<blbl>>
+ begin
+ <<flbl1>>
+ for _i in 1 .. 10 loop
+ exit flbl1;
+ end loop flbl1;
+ <<flbl2>>
+ for _i in 1 .. 10 loop
+ exit flbl2;
+ end loop;
+ end blbl;
+ $$ language plpgsql;
+
+ select end_label1();
+ drop function end_label1();
+
+ -- should fail: undefined end label
+ create function end_label2() returns void as $$
+ begin
+ for _i in 1 .. 10 loop
+ exit;
+ end loop flbl1;
+ end;
+ $$ language plpgsql;
+
+ -- should fail: end label does not match start label
+ create function end_label3() returns void as $$
+ <<outer_label>>
+ begin
+ <<inner_label>>
+ for _i in 1 .. 10 loop
+ exit;
+ end loop outer_label;
+ end;
+ $$ language plpgsql;
+
+ -- should fail: end label on a block without a start label
+ create function end_label4() returns void as $$
+ <<outer_label>>
+ begin
+ for _i in 1 .. 10 loop
+ exit;
+ end loop outer_label;
+ end;
+ $$ language plpgsql;
Neil Conway <neilc@samurai.com> writes:
> BTW, I notice that some but not all the call sites of ereport(ERROR) in
> PL/PgSQL's gram.y set plpgsql_error_lineno. Is there a reason for this?
Without looking at the code, I think it may be that you only need to set
the variable if you want the error to point someplace different than the
last "lno" nonterminal.
regards, tom lane
Pavel Stehule wrote: > this patch allows optional using label with END and END LOOP. Ending label > has only informational value, but can enhance readability large block and > enhance likeness with Oracle. Reviewed and applied -- thanks for the patch. -Neil
Pavel Stehule wrote: > Per small recent discussion I corrected patch user's exception. I'll review and apply this in the next day or so. > Next ToDo (needs discussion): > + Optional message in raise stmt for user's or system exception > raise exception division_by_zero; > + Possibility rethrown exception > raise; Both sound pretty reasonable to me. -Neil
Neil Conway <neilc@samurai.com> writes:
> Pavel Stehule wrote:
>> Per small recent discussion I corrected patch user's exception.
> I'll review and apply this in the next day or so.
Have we got a consensus yet on the behavior? There seemed to be no
meeting of the minds at all the last time I paid attention to this
thread ...
regards, tom lane
On Mon, 4 Jul 2005, Tom Lane wrote: > Neil Conway <neilc@samurai.com> writes: > > Pavel Stehule wrote: > >> Per small recent discussion I corrected patch user's exception. > > > I'll review and apply this in the next day or so. > > Have we got a consensus yet on the behavior? There seemed to be no > meeting of the minds at all the last time I paid attention to this > thread ... > There was only one objection against - requirement of uniqueness, and I corrected it. regards Pavel
Pavel Stehule wrote:
> Per small recent discussion I corrected patch user's exception.
Attached is a revised patch. I haven't looked at the documentation
changes yet (more work is needed I believe) or some of the error message
text.
I was originally hoping to make "exception variables" a little more
full-featured -- it seems silly to DECLARE something that cannot be
initialized with the value of another expression, for example. I can
also see how it would be useful to evaluate an expression variable (e.g.
to print it out for debugging purposes). It would be possible extend the
operations allowed upon exception variables, thinking about this
further, I wonder if there is any point introducing the concept of an
"exception variable" in the first place. What does it buy us over simply
using a string? In other words, if we allowed the syntax:
RAISE LEVEL [ opt_sqlstate ] 'fmt' [, expr ... ]
where `opt_sqlstate' is either empty, a T_WORD we find in the table of
predefined condition names, or an expression that evaluates to a text
value. The text value must be of a certain form (e.g. 5 characters in
length, begins with a "U" and so on).
It might be slightly more difficult to parse this (especially if we
allow 'fmt' to be an expression yielding a string, not just a string
literal), but I don't think it is ambiguous and can be sorted out via
yylex().
-Neil
Index: doc/src/sgml/plpgsql.sgml
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql/doc/src/sgml/plpgsql.sgml,v
retrieving revision 1.75
diff -c -r1.75 plpgsql.sgml
*** doc/src/sgml/plpgsql.sgml 2 Jul 2005 08:59:47 -0000 1.75
--- doc/src/sgml/plpgsql.sgml 6 Jul 2005 13:26:22 -0000
***************
*** 2117,2123 ****
<para>
The <replaceable>condition</replaceable> names can be any of those
shown in <xref linkend="errcodes-appendix">. A category name matches
! any error within its category.
The special condition name <literal>OTHERS</>
matches every error type except <literal>QUERY_CANCELED</>.
(It is possible, but often unwise, to trap
--- 2117,2125 ----
<para>
The <replaceable>condition</replaceable> names can be any of those
shown in <xref linkend="errcodes-appendix">. A category name matches
! any error within its category. You can use exception variable as
! condition name. Exception variable is declared with type
! <literal>EXCEPTION</literal>
The special condition name <literal>OTHERS</>
matches every error type except <literal>QUERY_CANCELED</>.
(It is possible, but often unwise, to trap
***************
*** 2571,2577 ****
raise errors.
<synopsis>
! RAISE <replaceable class="parameter">level</replaceable> '<replaceable class="parameter">format</replaceable>'
<optional>,<replaceable class="parameter">expression</replaceable> <optional>, ...</optional></optional>;
</synopsis>
Possible levels are <literal>DEBUG</literal>,
--- 2573,2580 ----
raise errors.
<synopsis>
! RAISE <replaceable class="parameter">level</replaceable>
! <optional>system exception|exception variable</optional> '<replaceable class="parameter">format</replaceable>'
<optional>,<replaceable class="parameter">expression</replaceable> <optional>, ...</optional></optional>;
</synopsis>
Possible levels are <literal>DEBUG</literal>,
***************
*** 2588,2593 ****
--- 2591,2600 ----
variables. See <xref linkend="runtime-config"> for more
information.
</para>
+
+ <para>
+ You can specify any system exception or any user exception.
+ </para>
<para>
Inside the format string, <literal>%</literal> is replaced by the
Index: src/include/utils/elog.h
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql/src/include/utils/elog.h,v
retrieving revision 1.79
diff -c -r1.79 elog.h
*** src/include/utils/elog.h 10 Jun 2005 16:23:10 -0000 1.79
--- src/include/utils/elog.h 6 Jul 2005 13:26:22 -0000
***************
*** 61,66 ****
--- 61,72 ----
(PGSIXBIT(ch1) + (PGSIXBIT(ch2) << 6) + (PGSIXBIT(ch3) << 12) + \
(PGSIXBIT(ch4) << 18) + (PGSIXBIT(ch5) << 24))
+ #define MAKE_SQLSTATE_STR(str) \
+ ( \
+ AssertMacro(strlen(str) == 5), \
+ MAKE_SQLSTATE(str[0], str[1], str[2], str[3], str[4]) \
+ )
+
/* These macros depend on the fact that '0' becomes a zero in SIXBIT */
#define ERRCODE_TO_CATEGORY(ec) ((ec) & ((1 << 12) - 1))
#define ERRCODE_IS_CATEGORY(ec) (((ec) & ~((1 << 12) - 1)) == 0)
Index: src/pl/plpgsql/src/gram.y
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql/src/pl/plpgsql/src/gram.y,v
retrieving revision 1.80
diff -c -r1.80 gram.y
*** src/pl/plpgsql/src/gram.y 2 Jul 2005 17:01:59 -0000 1.80
--- src/pl/plpgsql/src/gram.y 6 Jul 2005 13:38:35 -0000
***************
*** 103,108 ****
--- 103,109 ----
PLpgSQL_exception_block *exception_block;
PLpgSQL_nsitem *nsitem;
PLpgSQL_diag_item *diagitem;
+ PLpgSQL_user_exc *user_exc;
}
%type <declhdr> decl_sect
***************
*** 142,150 ****
%type <exception_block> exception_sect
%type <exception> proc_exception
%type <condition> proc_conditions
!
! %type <ival> raise_level
%type <str> raise_msg
%type <list> getdiag_list
--- 143,152 ----
%type <exception_block> exception_sect
%type <exception> proc_exception
%type <condition> proc_conditions
+ %type <str> exception_name
+ %type <ival> decl_sqlstate sqlstate_defn
! %type <ival> raise_level opt_raise_exc
%type <str> raise_msg
%type <list> getdiag_list
***************
*** 221,226 ****
--- 223,229 ----
%token T_LABEL
%token T_WORD
%token T_ERROR
+ %token T_EXCEPTION
%token O_OPTION
%token O_DUMP
***************
*** 348,354 ****
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("row or record variable cannot be NOT NULL")));
}
! if ($5 != NULL)
{
if (var->dtype == PLPGSQL_DTYPE_VAR)
((PLpgSQL_var *) var)->default_val = $5;
--- 351,357 ----
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("row or record variable cannot be NOT NULL")));
}
! if ($5)
{
if (var->dtype == PLPGSQL_DTYPE_VAR)
((PLpgSQL_var *) var)->default_val = $5;
***************
*** 358,363 ****
--- 361,378 ----
errmsg("default value for row or record variable is not supported")));
}
}
+ | decl_varname K_EXCEPTION decl_sqlstate
+ {
+ PLpgSQL_user_exc *exc_var;
+ PLpgSQL_type dtype;
+
+ dtype.typname = "exception";
+ dtype.ttype = PLPGSQL_TTYPE_EXCEPTION;
+
+ exc_var = (PLpgSQL_user_exc *) plpgsql_build_variable($1.name, $1.lineno,
+ &dtype, true);
+ exc_var->sqlstate = $3;
+ }
| decl_varname K_ALIAS K_FOR decl_aliasitem ';'
{
plpgsql_ns_additem($4->itemtype,
***************
*** 563,572 ****
| K_DEFAULT
;
! proc_sect :
{
! $$ = NIL;
}
| proc_stmts
{ $$ = $1; }
;
--- 578,615 ----
| K_DEFAULT
;
! decl_sqlstate : ';'
! { $$ = plpgsql_new_user_sqlstate(); }
! | decl_defkey sqlstate_defn ';'
{
! $$ = $2;
}
+ ;
+
+ sqlstate_defn : T_STRING
+ {
+ char *state_str = plpgsql_get_string_value();
+
+ if (strlen(state_str) != 5)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid SQLSTATE value \"%s\" for "
+ "user-defined exception", state_str),
+ errdetail("SQLSTATE values must be 5 characters in length")));
+
+ if (strncmp(state_str, "U0", 2) == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid SQLSTATE value \"%s\" for "
+ "user-defined exception", state_str),
+ errhint("Class \"U0\" is reserved for default values user's exceptions.")));
+
+ $$ = MAKE_SQLSTATE_STR(state_str);
+ }
+ ;
+
+ proc_sect :
+ { $$ = NIL; }
| proc_stmts
{ $$ = $1; }
;
***************
*** 1184,1190 ****
}
;
! stmt_raise : K_RAISE lno raise_level raise_msg
{
PLpgSQL_stmt_raise *new;
int tok;
--- 1227,1233 ----
}
;
! stmt_raise : K_RAISE lno raise_level opt_raise_exc raise_msg
{
PLpgSQL_stmt_raise *new;
int tok;
***************
*** 1194,1202 ****
new->cmd_type = PLPGSQL_STMT_RAISE;
new->lineno = $2;
new->elog_level = $3;
! new->message = $4;
new->params = NIL;
tok = yylex();
/*
--- 1237,1256 ----
new->cmd_type = PLPGSQL_STMT_RAISE;
new->lineno = $2;
new->elog_level = $3;
! new->message = $5;
new->params = NIL;
+ /* No exception variable or SQLSTATE value specified? */
+ if ($4 == -1)
+ {
+ if (new->elog_level >= ERROR)
+ new->sqlstate = ERRCODE_RAISE_EXCEPTION;
+ else
+ new->sqlstate = 0;
+ }
+ else
+ new->sqlstate = $4;
+
tok = yylex();
/*
***************
*** 1266,1271 ****
--- 1320,1344 ----
}
;
+ opt_raise_exc : T_EXCEPTION
+ {
+ $$ = yylval.user_exc->sqlstate;
+ }
+ | T_WORD
+ {
+ PLpgSQL_condition *c = plpgsql_parse_err_condition(yytext);
+ /* Don't allow "OTHERS" to be thrown via RAISE */
+ if (c->sqlerrstate == 0)
+ yyerror("illegal SQLSTATE value");
+ $$ = c->sqlerrstate;
+ pfree(c);
+ }
+ | /* EMPTY */
+ {
+ $$ = -1;
+ }
+ ;
+
stmt_execsql : execsql_start lno
{
PLpgSQL_stmt_execsql *new;
***************
*** 1285,1292 ****
PLpgSQL_expr *expr;
int endtoken;
! expr = read_sql_construct(K_INTO, ';', "INTO|;", "SELECT ",
! true, true, &endtoken);
new = palloc(sizeof(PLpgSQL_stmt_dynexecute));
new->cmd_type = PLPGSQL_STMT_DYNEXECUTE;
--- 1358,1366 ----
PLpgSQL_expr *expr;
int endtoken;
! expr = read_sql_construct(K_INTO, ';', "INTO or ;",
! "SELECT ", true, true,
! &endtoken);
new = palloc(sizeof(PLpgSQL_stmt_dynexecute));
new->cmd_type = PLPGSQL_STMT_DYNEXECUTE;
***************
*** 1581,1600 ****
}
;
! proc_conditions : proc_conditions K_OR opt_lblname
! {
! PLpgSQL_condition *old;
! for (old = $1; old->next != NULL; old = old->next)
! /* skip */ ;
! old->next = plpgsql_parse_err_condition($3);
! $$ = $1;
! }
! | opt_lblname
! {
! $$ = plpgsql_parse_err_condition($1);
! }
;
expr_until_semi :
--- 1655,1686 ----
}
;
! proc_conditions : proc_conditions K_OR exception_name
! {
! PLpgSQL_condition *old;
!
! for (old = $1; old->next != NULL; old = old->next)
! /* skip */ ;
! old->next = plpgsql_parse_err_condition($3);
! $$ = $1;
! }
! | exception_name
! {
! $$ = plpgsql_parse_err_condition($1);
! }
! ;
! exception_name : T_WORD
! {
! char *name;
! plpgsql_convert_ident(yytext, &name, 1);
! $$ = name;
! }
! | T_EXCEPTION
! {
! $$ = yylval.user_exc->refname;
! }
;
expr_until_semi :
Index: src/pl/plpgsql/src/pl_comp.c
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql/src/pl/plpgsql/src/pl_comp.c,v
retrieving revision 1.91
diff -c -r1.91 pl_comp.c
*** src/pl/plpgsql/src/pl_comp.c 10 Jun 2005 16:23:11 -0000 1.91
--- src/pl/plpgsql/src/pl_comp.c 6 Jul 2005 13:26:22 -0000
***************
*** 80,85 ****
--- 80,87 ----
bool plpgsql_DumpExecTree = false;
bool plpgsql_check_syntax = false;
+ static int plpgsql_user_exc_counter;
+
PLpgSQL_function *plpgsql_curr_compile;
/* A context appropriate for short-term allocs during compilation */
***************
*** 316,321 ****
--- 318,325 ----
plpgsql_Datums = palloc(sizeof(PLpgSQL_datum *) * datums_alloc);
datums_last = 0;
+ plpgsql_user_exc_counter = 0;
+
/*
* Do extra syntax checks when validating the function
* definition. We skip this when actually compiling functions for
***************
*** 905,910 ****
--- 909,918 ----
plpgsql_yylval.row = (PLpgSQL_row *) (plpgsql_Datums[nse->itemno]);
return T_ROW;
+ case PLPGSQL_NSTYPE_EXCEPTION:
+ plpgsql_yylval.user_exc = (PLpgSQL_user_exc *) (plpgsql_Datums[nse->itemno]);
+ return T_EXCEPTION;
+
default:
return T_ERROR;
}
***************
*** 1626,1631 ****
--- 1634,1658 ----
result = (PLpgSQL_variable *) rec;
break;
}
+ case PLPGSQL_TTYPE_EXCEPTION:
+ {
+ /* The exception type */
+ PLpgSQL_user_exc *exception;
+
+ exception = palloc0(sizeof(PLpgSQL_user_exc));
+ exception->dtype = PLPGSQL_DTYPE_EXCEPTION;
+ exception->refname = pstrdup(refname);
+ exception->lineno = lineno;
+ /* caller should define sqlstate! */
+
+ plpgsql_adddatum((PLpgSQL_datum *) exception);
+ if (add2namespace)
+ plpgsql_ns_additem(PLPGSQL_NSTYPE_EXCEPTION,
+ exception->dno,
+ refname);
+ result = (PLpgSQL_variable *) exception;
+ break;
+ }
case PLPGSQL_TTYPE_PSEUDO:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
***************
*** 1883,1890 ****
* plpgsql_parse_err_condition
* Generate PLpgSQL_condition entry(s) for an exception condition name
*
! * This has to be able to return a list because there are some duplicate
! * names in the table of error code names.
*/
PLpgSQL_condition *
plpgsql_parse_err_condition(char *condname)
--- 1910,1919 ----
* plpgsql_parse_err_condition
* Generate PLpgSQL_condition entry(s) for an exception condition name
*
! * This has to be able to return a list because there are some
! * duplicate names in the table of error code names. Note that the
! * exception name should already be normalized (e.g. via
! * plpgsql_convert_ident).
*/
PLpgSQL_condition *
plpgsql_parse_err_condition(char *condname)
***************
*** 1892,1902 ****
int i;
PLpgSQL_condition *new;
PLpgSQL_condition *prev;
!
! /*
! * XXX Eventually we will want to look for user-defined exception
! * names here.
! */
/*
* OTHERS is represented as code 0 (which would map to '00000', but we
--- 1921,1927 ----
int i;
PLpgSQL_condition *new;
PLpgSQL_condition *prev;
! PLpgSQL_nsitem *nse;
/*
* OTHERS is represented as code 0 (which would map to '00000', but we
***************
*** 1924,1929 ****
--- 1949,1981 ----
}
}
+ /*
+ * If "condname" is not the name of any builtin exceptions, look
+ * for a user-defined exception variable with that name.
+ */
+ if (!prev)
+ {
+ /*
+ * Do a lookup on the compiler's namestack
+ */
+ nse = plpgsql_ns_lookup(condname, NULL);
+
+ if (nse != NULL)
+ {
+ PLpgSQL_user_exc *exc_var;
+
+ exc_var = (PLpgSQL_user_exc *) (plpgsql_Datums[nse->itemno]);
+ if (nse->itemtype == PLPGSQL_NSTYPE_EXCEPTION)
+ {
+ new = palloc(sizeof(PLpgSQL_condition));
+ new->sqlerrstate = exc_var->sqlstate;
+ new->condname = condname;
+ new->next = prev;
+ prev = new;
+ }
+ }
+ }
+
if (!prev)
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
***************
*** 2171,2179 ****
plpgsql_HashEnt *hentry;
hentry = (plpgsql_HashEnt *) hash_search(plpgsql_HashTable,
! (void *) function->fn_hashkey,
HASH_REMOVE,
NULL);
if (hentry == NULL)
elog(WARNING, "trying to delete function that does not exist");
}
--- 2223,2246 ----
plpgsql_HashEnt *hentry;
hentry = (plpgsql_HashEnt *) hash_search(plpgsql_HashTable,
! (void *) function->fn_hashkey,
HASH_REMOVE,
NULL);
if (hentry == NULL)
elog(WARNING, "trying to delete function that does not exist");
}
+
+ #define MAX_USER_EXCPT 999
+
+ int
+ plpgsql_new_user_sqlstate(void)
+ {
+ char str[6];
+
+ if (plpgsql_user_exc_counter >= MAX_USER_EXCPT)
+ ereport(ERROR,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("Too many user-defined exceptions")));
+ snprintf(str, sizeof(str), "U0%03d", ++plpgsql_user_exc_counter);
+ return MAKE_SQLSTATE_STR(str);
+ }
Index: src/pl/plpgsql/src/pl_exec.c
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.149
diff -c -r1.149 pl_exec.c
*** src/pl/plpgsql/src/pl_exec.c 26 Jun 2005 22:05:42 -0000 1.149
--- src/pl/plpgsql/src/pl_exec.c 6 Jul 2005 13:56:49 -0000
***************
*** 723,728 ****
--- 723,733 ----
result = datum;
break;
+ case PLPGSQL_DTYPE_EXCEPTION:
+ /* XXX: this is read-only at runtime -- just copy as well? */
+ result = NULL;
+ break;
+
default:
elog(ERROR, "unrecognized dtype: %d", datum->dtype);
result = NULL; /* keep compiler quiet */
***************
*** 825,830 ****
--- 830,836 ----
case PLPGSQL_DTYPE_RECFIELD:
case PLPGSQL_DTYPE_ARRAYELEM:
+ case PLPGSQL_DTYPE_EXCEPTION:
break;
default:
***************
*** 2062,2069 ****
estate->err_text = raise_skip_msg; /* suppress traceback of raise */
ereport(stmt->elog_level,
! ((stmt->elog_level >= ERROR) ? errcode(ERRCODE_RAISE_EXCEPTION) : 0,
! errmsg_internal("%s", plpgsql_dstring_get(&ds))));
estate->err_text = NULL; /* un-suppress... */
--- 2068,2075 ----
estate->err_text = raise_skip_msg; /* suppress traceback of raise */
ereport(stmt->elog_level,
! (errcode(stmt->sqlstate),
! errmsg_internal("%s", plpgsql_dstring_get(&ds))));
estate->err_text = NULL; /* un-suppress... */
Index: src/pl/plpgsql/src/plpgsql.h
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql/src/pl/plpgsql/src/plpgsql.h,v
retrieving revision 1.64
diff -c -r1.64 plpgsql.h
*** src/pl/plpgsql/src/plpgsql.h 22 Jun 2005 01:35:02 -0000 1.64
--- src/pl/plpgsql/src/plpgsql.h 6 Jul 2005 13:26:22 -0000
***************
*** 58,64 ****
PLPGSQL_NSTYPE_LABEL,
PLPGSQL_NSTYPE_VAR,
PLPGSQL_NSTYPE_ROW,
! PLPGSQL_NSTYPE_REC
};
/* ----------
--- 58,65 ----
PLPGSQL_NSTYPE_LABEL,
PLPGSQL_NSTYPE_VAR,
PLPGSQL_NSTYPE_ROW,
! PLPGSQL_NSTYPE_REC,
! PLPGSQL_NSTYPE_EXCEPTION
};
/* ----------
***************
*** 73,79 ****
PLPGSQL_DTYPE_RECFIELD,
PLPGSQL_DTYPE_ARRAYELEM,
PLPGSQL_DTYPE_EXPR,
! PLPGSQL_DTYPE_TRIGARG
};
/* ----------
--- 74,81 ----
PLPGSQL_DTYPE_RECFIELD,
PLPGSQL_DTYPE_ARRAYELEM,
PLPGSQL_DTYPE_EXPR,
! PLPGSQL_DTYPE_TRIGARG,
! PLPGSQL_DTYPE_EXCEPTION
};
/* ----------
***************
*** 85,91 ****
PLPGSQL_TTYPE_SCALAR, /* scalar types and domains */
PLPGSQL_TTYPE_ROW, /* composite types */
PLPGSQL_TTYPE_REC, /* RECORD pseudotype */
! PLPGSQL_TTYPE_PSEUDO /* other pseudotypes */
};
/* ----------
--- 87,94 ----
PLPGSQL_TTYPE_SCALAR, /* scalar types and domains */
PLPGSQL_TTYPE_ROW, /* composite types */
PLPGSQL_TTYPE_REC, /* RECORD pseudotype */
! PLPGSQL_TTYPE_PSEUDO, /* other pseudotypes */
! PLPGSQL_TTYPE_EXCEPTION /* user-defined exception variables */
};
/* ----------
***************
*** 173,179 ****
* PLpgSQL_trigarg
*/
typedef struct
! { /* Generic datum array item */
int dtype;
int dno;
} PLpgSQL_datum;
--- 176,182 ----
* PLpgSQL_trigarg
*/
typedef struct
! { /* Generic datum array item */
int dtype;
int dno;
} PLpgSQL_datum;
***************
*** 190,197 ****
int lineno;
} PLpgSQL_variable;
typedef struct PLpgSQL_expr
! { /* SQL Query to plan and execute */
int dtype;
int exprno;
char *query;
--- 193,209 ----
int lineno;
} PLpgSQL_variable;
+ typedef struct
+ { /* User-defined exception variable */
+ int dtype;
+ int dno;
+ char *refname;
+ int lineno;
+ int sqlstate;
+ } PLpgSQL_user_exc;
+
typedef struct PLpgSQL_expr
! { /* SQL Query to plan and execute */
int dtype;
int exprno;
char *query;
***************
*** 292,298 ****
typedef struct
! { /* Item in the compilers namestack */
int itemtype;
int itemno;
char name[1];
--- 304,310 ----
typedef struct
! { /* Item in the compiler's namestack */
int itemtype;
int itemno;
char name[1];
***************
*** 516,521 ****
--- 528,534 ----
int cmd_type;
int lineno;
int elog_level;
+ int sqlstate;
char *message;
List *params; /* list of expressions */
} PLpgSQL_stmt_raise;
***************
*** 688,693 ****
--- 701,708 ----
extern int plpgsql_add_initdatums(int **varnos);
extern void plpgsql_HashTableInit(void);
extern void plpgsql_compile_error_callback(void *arg);
+ extern int plpgsql_new_user_sqlstate(void);
+
/* ----------
* Functions in pl_handler.c
Index: src/test/regress/expected/plpgsql.out
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql/src/test/regress/expected/plpgsql.out,v
retrieving revision 1.38
diff -c -r1.38 plpgsql.out
*** src/test/regress/expected/plpgsql.out 2 Jul 2005 08:59:48 -0000 1.38
--- src/test/regress/expected/plpgsql.out 6 Jul 2005 15:03:56 -0000
***************
*** 2721,2723 ****
--- 2721,2778 ----
$$ language plpgsql;
ERROR: end label "outer_label" specified for unlabelled block
CONTEXT: compile of PL/pgSQL function "end_label4" near line 5
+ -- user-defined exceptions
+ -- should fail: illegal sqlstate for exception
+ create function innerfx() returns void as $$
+ declare
+ my_exc exception = 'U0001';
+ begin -- using msgtext as one param of exception
+ raise exception my_exc '%', CURRENT_TIMESTAMP;
+ end;
+ $$ language plpgsql;
+ ERROR: invalid SQLSTATE value "U0001" for user-defined exception
+ HINT: Class "U0" is reserved for default values user's exceptions.
+ CONTEXT: compile of PL/pgSQL function "innerfx" near line 2
+ create function innerfx() returns integer as $$
+ declare
+ my_exc1 exception = 'U1001';
+ begin -- using msgtext as one param of exception
+ raise exception my_exc1 '%', 100.34::numeric;
+ return 1;
+ end $$ language plpgsql;
+ create function outerfx() returns integer as $$
+ declare
+ my_excpt exception = 'U1001';
+ alias_div_by_zero exception = '22012';
+ def_sqlstate exception;
+ begin
+ begin
+ raise exception def_sqlstate 'foo';
+ exception when def_sqlstate then
+ raise notice '01 catch: %, %', sqlstate, sqlerrm;
+ end;
+ begin
+ raise notice '%', innerfx();
+ exception when my_excpt then
+ raise notice '02 catch: %, %', sqlstate, sqlerrm::numeric;
+ end;
+ begin
+ raise exception alias_div_by_zero 'testing';
+ exception when division_by_zero then
+ raise notice 'Divison by zero: %, %', sqlstate, sqlerrm;
+ end;
+ return 1;
+ end; $$ language plpgsql;
+ select innerfx();
+ ERROR: 100.34
+ select outerfx();
+ NOTICE: 01 catch: U0001, foo
+ NOTICE: 02 catch: U1001, 100.34
+ NOTICE: Divison by zero: 22012, testing
+ outerfx
+ ---------
+ 1
+ (1 row)
+
+ drop function outerfx();
+ drop function innerfx();
Index: src/test/regress/sql/plpgsql.sql
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql/src/test/regress/sql/plpgsql.sql,v
retrieving revision 1.33
diff -c -r1.33 plpgsql.sql
*** src/test/regress/sql/plpgsql.sql 2 Jul 2005 08:59:48 -0000 1.33
--- src/test/regress/sql/plpgsql.sql 6 Jul 2005 13:57:44 -0000
***************
*** 2280,2282 ****
--- 2280,2331 ----
end loop outer_label;
end;
$$ language plpgsql;
+
+ -- user-defined exceptions
+
+ -- should fail: illegal sqlstate for exception
+ create function innerfx() returns void as $$
+ declare
+ my_exc exception = 'U0001';
+ begin -- using msgtext as one param of exception
+ raise exception my_exc '%', CURRENT_TIMESTAMP;
+ end;
+ $$ language plpgsql;
+
+ create function innerfx() returns integer as $$
+ declare
+ my_exc1 exception = 'U1001';
+ begin -- using msgtext as one param of exception
+ raise exception my_exc1 '%', 100.34::numeric;
+ return 1;
+ end $$ language plpgsql;
+
+ create function outerfx() returns integer as $$
+ declare
+ my_excpt exception = 'U1001';
+ alias_div_by_zero exception = '22012';
+ def_sqlstate exception;
+ begin
+ begin
+ raise exception def_sqlstate 'foo';
+ exception when def_sqlstate then
+ raise notice '01 catch: %, %', sqlstate, sqlerrm;
+ end;
+ begin
+ raise notice '%', innerfx();
+ exception when my_excpt then
+ raise notice '02 catch: %, %', sqlstate, sqlerrm::numeric;
+ end;
+ begin
+ raise exception alias_div_by_zero 'testing';
+ exception when division_by_zero then
+ raise notice 'Divison by zero: %, %', sqlstate, sqlerrm;
+ end;
+ return 1;
+ end; $$ language plpgsql;
+
+ select innerfx();
+ select outerfx();
+
+ drop function outerfx();
+ drop function innerfx();
Neil Conway <neilc@samurai.com> writes:
> I wonder if there is any point introducing the concept of an
> "exception variable" in the first place. What does it buy us over simply
> using a string?
Not a lot really, except for keeping things similar to the Oracle way of
doing it ... but that's a nontrivial consideration.
> RAISE LEVEL [ opt_sqlstate ] 'fmt' [, expr ... ]
> It might be slightly more difficult to parse this (especially if we
> allow 'fmt' to be an expression yielding a string, not just a string
> literal), but I don't think it is ambiguous and can be sorted out via
> yylex().
I think it is a bad idea, if not actually impossible, to have an
expression for sqlstate with no separating syntax before the 'fmt';
especially not if you'd like to also allow an expression for the 'fmt'.
At one point we had talked about
RAISE LEVEL [ opt_sqlstate, ] 'fmt' [, expr ... ]
The hard part here is that there isn't any very easy way to tell whether
you have a sqlstate, a fmt, and N exprs, or a fmt and N+1 exprs. The
saving grace of the declared-exception approach for this is that you
can tell by the datatype of the first argument expression which case you
have: if the expression yields text, it's a fmt, if it yields "exception"
(which we assume is an actual datatype) then it's a sqlstate.
We could handle "undeclared exceptions" in such a design by having a
function that converts text to an exception value:
RAISE LEVEL SQLSTATE('12345'), 'format here', ...
and maybe the short-term cheesy thing to do is special-case exactly this
syntax:
RAISE LEVEL [ SQLSTATE(text_expr), ] text_expr [, ... ]
which would give us the minimum functionality with a clear path to
expansion later.
regards, tom lane
Tom Lane wrote:
> I think it is a bad idea, if not actually impossible, to have an
> expression for sqlstate with no separating syntax before the 'fmt';
> especially not if you'd like to also allow an expression for the 'fmt'.
I don't actually see much of a need to allow 'fmt' to be an expression,
especially now that RAISE parameters can be expressions. The ratio of
constant printf() format strings to variable format strings is probably
100:1, for good reason...
> The hard part here is that there isn't any very easy way to tell whether
> you have a sqlstate, a fmt, and N exprs, or a fmt and N+1 exprs. The
> saving grace of the declared-exception approach for this is that you
> can tell by the datatype of the first argument expression which case you
> have
I really don't like the idea of introducing a new concept into the
language ("exception variables") to resolve some ambiguous syntax. It
would be another matter if exception variables actually provided
something that strings do not...
Another solution might be varying the syntax slightly, such as:
RAISE [ opt_sqlstate ] LEVEL 'fmt' [ , expr ... ]
-Neil
> "exception variable" in the first place. What does it buy us over simply > using a string? In other words, if we allowed the syntax: > > RAISE LEVEL [ opt_sqlstate ] 'fmt' [, expr ... ] > > where `opt_sqlstate' is either empty, a T_WORD we find in the table of > predefined condition names, or an expression that evaluates to a text > value. The text value must be of a certain form (e.g. 5 characters in > length, begins with a "U" and so on). I unlike this syntax. Yes, it's easy and clear, but not readable. Exception variables are better and an way for future. SQL state can be only one value wich can hold exception variable. And more it's more in oracle style (I don't wont to copy all Oracle ware into PostgreSQL) Pavel p.s. I have patch for rethrow exception which isn't related to user's exception (but need's finished plpgsql code). Syntax is easy, I hope RAISE;
Neil Conway <neilc@samurai.com> writes:
> Tom Lane wrote:
>> I think it is a bad idea, if not actually impossible, to have an
>> expression for sqlstate with no separating syntax before the 'fmt';
>> especially not if you'd like to also allow an expression for the 'fmt'.
> I don't actually see much of a need to allow 'fmt' to be an expression,
Well, in any case we have a problem if there's no comma. Consider
RAISE NOTICE '12' !! '345', ...
Is !! an infix operator (using both strings as arguments) or a postfix
operator (in which case '345' is the format)?
> Another solution might be varying the syntax slightly, such as:
> RAISE [ opt_sqlstate ] LEVEL 'fmt' [ , expr ... ]
This would require promoting all the options for LEVEL into fully
reserved words. You really can't get around the fact that you need
something pretty identifiable to terminate the expression.
It might work to require parentheses:
RAISE LEVEL ( sqlstate expression ), 'fmt' [, ...]
The comma after the right paren is optional from a formal point of view,
but I'd still consider it better design to use one than not. (For one
reason, it would make it much easier to catch mismatched-parens problems.)
regards, tom lane
> and maybe the short-term cheesy thing to do is special-case exactly this
> syntax:
>
> RAISE LEVEL [ SQLSTATE(text_expr), ] text_expr [, ... ]
>
> which would give us the minimum functionality with a clear path to
> expansion later.
>
or only RAISE LEVEL SQLSTATE(text_expr)|text_expr [, ...]
if I use registered sqlstate, plpgsql knows text message. But I think this
syntax has more questions than exception's variables. It's really problem
declare one exceptio's variable? It's similar like using constant
variables or magic values.
Pavel
DECLARE not_money EXCEPTION=SQLSTATE('U1101');
BEGIN
IF account < 0 THEN
RAISE EXCEPTION not_money;
...
or
BEGIN
IF account < 0 THEN
RAISE SQLSTATE ('U1101') 'Not money';
Pavel Stehule <stehule@kix.fsv.cvut.cz> writes:
> if I use registered sqlstate, plpgsql knows text message.
No, it does not. I already pointed out that tying a single error
message to a SQLSTATE is unreasonable, because that's not how the
SQL committee intended SQLSTATEs to be used. I haven't looked at
this patch yet, but if it's doing things that way it is wrong.
regards, tom lane
>
> This would require promoting all the options for LEVEL into fully
> reserved words. You really can't get around the fact that you need
> something pretty identifiable to terminate the expression.
>
> It might work to require parentheses:
>
> RAISE LEVEL ( sqlstate expression ), 'fmt' [, ...]
>
? what sense has sqlstate expression? like any expression returns sqlstate
type?
SQLSTATE('')|user exception|system exception
Pavel
On Wed, 6 Jul 2005, Tom Lane wrote: > Pavel Stehule <stehule@kix.fsv.cvut.cz> writes: > > if I use registered sqlstate, plpgsql knows text message. > > No, it does not. I already pointed out that tying a single error > message to a SQLSTATE is unreasonable, because that's not how the > SQL committee intended SQLSTATEs to be used. I haven't looked at > this patch yet, but if it's doing things that way it is wrong. > no, raise stmt still needs message text (patch) > regards, tom lane > What I wont. Maybe I going in wrong direction. Please, correct me. User's exception needs and will needs message text. I don't wont to introduce wrong programming style. But if I use exception variable and have to use its, then there is not only SQLSTATE but there exist name of exception too. But I wont to simplify using system's exception. The system knows all what need: name, text, sqlstate. And in mostly time I don't wont to substitute text of system message. But if I wont to show it, I have to copy it. example: raise exception div_by_zero; -- I wont to use system message, why not? but now, I have to do raise exception div_by_zero, 'division by zero ...' Regards Pavel
>
> I really don't like the idea of introducing a new concept into the
> language ("exception variables") to resolve some ambiguous syntax. It
> would be another matter if exception variables actually provided
> something that strings do not...
>
In this time e.variables does it - only holds sqlstate and name.
You see only raise stmt. But there is part of begin exception block too.
without e.v. you have to catch users exception only via OTHERS or you have
to change syntax.
EXCEPTION WHEN SQLSTATE('0000') THEN
e.v. solve this problem. And I hope so can hold others info in future
Pavel
Tom Lane wrote: > RAISE NOTICE '12' !! '345', ... > > Is !! an infix operator (using both strings as arguments) or a postfix > operator (in which case '345' is the format)? Ah, I see. I would be content to allow opt_sqlstate to be either a string literal, a T_WORD (predefined error condition), or a TEXT variable. If users need to throw a sqlstate that is derived from a SQL expression, they can always assign to a TEXT variable and then specify that variable to RAISE. >> RAISE [ opt_sqlstate ] LEVEL 'fmt' [ , expr ... ] This syntax might be slightly better anyway, as allowing two string literals without any intervening tokens is a bit ugly. We would still need to restrict opt_sqlstate as described above, though. -Neil
Neil Conway wrote: > Ah, I see. I would be content to allow opt_sqlstate to be either a > string literal, a T_WORD (predefined error condition), or a TEXT > variable. If users need to throw a sqlstate that is derived from a SQL > expression, they can always assign to a TEXT variable and then specify > that variable to RAISE. >>> RAISE [ opt_sqlstate ] LEVEL 'fmt' [ , expr ... ] BTW, do have we reached a consensus on this? -Neil
Neil Conway <neilc@samurai.com> writes:
> BTW, do have we reached a consensus on this?
Doesn't look that way --- I tend to agree with you that we could avoid
inventing declared exceptions at all, but Pavel is definitely not happy
with it, and AFAIR no one else has weighed in. Maybe we need to take
the discussion back to pghackers to draw a wider audience.
regards, tom lane
On Fri, 8 Jul 2005, Neil Conway wrote: > Neil Conway wrote: > > Ah, I see. I would be content to allow opt_sqlstate to be either a > > string literal, a T_WORD (predefined error condition), or a TEXT > > variable. If users need to throw a sqlstate that is derived from a SQL > > expression, they can always assign to a TEXT variable and then specify > > that variable to RAISE. > > >>> RAISE [ opt_sqlstate ] LEVEL 'fmt' [ , expr ... ] > > BTW, do have we reached a consensus on this? > > -Neil > ok, but don't forget, please, on exception part. Pavel
Pavel Stehule wrote: > ok, but don't forget, please, on exception part. What do you mean? -Neil
On Fri, 8 Jul 2005, Tom Lane wrote: > Neil Conway <neilc@samurai.com> writes: > > BTW, do have we reached a consensus on this? > > Doesn't look that way --- I tend to agree with you that we could avoid > inventing declared exceptions at all, but Pavel is definitely not happy > with it, and AFAIR no one else has weighed in. Maybe we need to take > the discussion back to pghackers to draw a wider audience. > I am not happy (this is only half of step), but I don't expect better discussion. My opinion is so exception variable has more possibilities, but this solution is usefull and funkcional too. And we can introduce exception variables later without problems if will be good time. Discussion on pghackers was, but not too much people contributed. And more, I don't see user's exception as big qustion this days. > regards, tom lane >
On Fri, 8 Jul 2005, Neil Conway wrote: > Pavel Stehule wrote: > > ok, but don't forget, please, on exception part. > > What do you mean? > > -Neil > BEGIN EXCEPTION WHEN * THEN equvalent rules for raise have to be in * is true? Pavel
Pavel Stehule wrote: > BEGIN > EXCEPTION WHEN * THEN > > equvalent rules for raise have to be in * > > is true? I'm sorry, but I'm still not sure what you mean. -Neil
Where are we on this patch? Is there something to apply? --------------------------------------------------------------------------- Pavel Stehule wrote: > On Fri, 8 Jul 2005, Neil Conway wrote: > > > Pavel Stehule wrote: > > > ok, but don't forget, please, on exception part. > > > > What do you mean? > > > > -Neil > > > > BEGIN > EXCEPTION WHEN * THEN > > equvalent rules for raise have to be in * > > is true? > > Pavel > > > ---------------------------(end of broadcast)--------------------------- > TIP 5: don't forget to increase your free space map settings > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian wrote: > Where are we on this patch? Is there something to apply? Not at the moment. I believe we have agreed that it would be better to remove the concept of "exception variables" and just use strings, but I haven't implemented this yet. I'm happy to do that, but I might not get a chance till Wednesday. -Neil
Neil Conway wrote: > Not at the moment. I believe we have agreed that it would be better to > remove the concept of "exception variables" and just use strings, but I > haven't implemented this yet. BTW, one minor annoyance I noticed: a builtin condition name can actually map to multiple SQLSTATE values. If we allow a builtin condition name to be specified to RAISE, this means we'll actually need to pass around a list of SQLSTATE values that are thrown by the RAISE, rather than a single SQLSTATE. This seems pretty ugly, though -- especially considering that only a handful of the builtin condition names actually do map to multiple SQLSTATEs. Does anyone have a better suggestion? -Neil
On Tue, 26 Jul 2005, Neil Conway wrote: > Neil Conway wrote: > > Not at the moment. I believe we have agreed that it would be better to > > remove the concept of "exception variables" and just use strings, but I > > haven't implemented this yet. > > BTW, one minor annoyance I noticed: a builtin condition name can > actually map to multiple SQLSTATE values. can you show sample, please? If we allow a builtin > condition name to be specified to RAISE, this means we'll actually need > to pass around a list of SQLSTATE values that are thrown by the RAISE, > rather than a single SQLSTATE. This seems pretty ugly, though -- > especially considering that only a handful of the builtin condition > names actually do map to multiple SQLSTATEs. Does anyone have a better > suggestion? > Exception variables can solve it, but its dead concept. We can have list of prohibited condition names and for its throw compile error condition name is ambigous Pavel > -Neil >
hello, sorry, exception variables don't solve this problem too. But we can detect it in compile-time. I don't wont to complicate raise syntax. best regards Pavel
Pavel Stehule wrote: > can you show sample, please? modifying_sql_data_not_permitted, null_value_not_allowed, prohibited_sql_statement_attempted and reading_sql_data_not_permitted are the examples I can see from scanning plerrcodes.h. If we had this to do over again, I'm not sure I see the point in mapping a single condition names to multiple SQLSTATEs, but it's probably too late to undo that now. > Exception variables can solve it, but its dead concept. We can have list > of prohibited condition names and for its throw compile error condition > name is ambigous Yeah, that's possible, but it doesn't seem any nicer :-( -Neil
Neil Conway <neilc@samurai.com> writes:
> Pavel Stehule wrote:
>> can you show sample, please?
> modifying_sql_data_not_permitted, null_value_not_allowed,
> prohibited_sql_statement_attempted and reading_sql_data_not_permitted
> are the examples I can see from scanning plerrcodes.h. If we had this to
> do over again, I'm not sure I see the point in mapping a single
> condition names to multiple SQLSTATEs, but it's probably too late to
> undo that now.
Those cases are for places where the spec defines similar cases under
the error classes "SQL Routine Exception" and "External Routine Exception".
You can blame me for having assumed that plpgsql didn't need to
distinguish these cases.
A quick grep says that the only one of these codes being generated today
is
contrib/dblink/dblink.c: (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
and that's for a "you should not do that" case, which it's very unlikely
anyone is specifically trapping for. So I see no
backwards-compatibility argument that we can't change this. How would
you want to do it better?
regards, tom lane
Tom Lane wrote: > Those cases are for places where the spec defines similar cases under > the error classes "SQL Routine Exception" and "External Routine Exception". > You can blame me for having assumed that plpgsql didn't need to > distinguish these cases. Well, in and of itself, I agree it is probably better to combine similar SQLSTATEs into a single logical condition. However, considering the problem it poses for implementing RAISE with builtin condition names, IMHO it would be a net win to get rid of it, if we can't find a better solution. > So I see no backwards-compatibility argument that we can't change > this. How would you want to do it better? I would just change the mapping from condition names to SQLSTATEs to be one-to-one. If a client application does need to trap multiple SQLSTATEs for a logically similar condition, they can always specify "WHEN x OR y OR ..." -Neil