From 2e35d4f35ca3dd822dbf8dcdcceff822cfb43d78 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 9 Aug 2010 02:25:07 +0000 Subject: [PATCH] Modify the handling of RAISE without parameters so that the error it throws can be caught in the same places that could catch an ordinary RAISE ERROR in the same location. The previous coding insisted on throwing the error from the block containing the active exception handler; which is arguably more surprising, and definitely unlike Oracle's behavior. Not back-patching, since this is a pretty obscure corner case. The risk of breaking somebody's code in a minor version update seems to outweigh any possible benefit. Piyush Newe, reviewed by David Fetter --- doc/src/sgml/plpgsql.sgml | 21 +++++++++--- src/pl/plpgsql/src/pl_exec.c | 46 ++++++++++++++++----------- src/pl/plpgsql/src/plpgsql.h | 6 ++-- src/test/regress/expected/plpgsql.out | 31 +++++++++++++++++- src/test/regress/sql/plpgsql.sql | 25 +++++++++++++++ 5 files changed, 101 insertions(+), 28 deletions(-) diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml index adc56a7a0e..f60273b699 100644 --- a/doc/src/sgml/plpgsql.sgml +++ b/doc/src/sgml/plpgsql.sgml @@ -1,4 +1,4 @@ - + <application>PL/pgSQL</application> - <acronym>SQL</acronym> Procedural Language @@ -2160,7 +2160,7 @@ BEGIN || quote_ident(mviews.mv_name) || ' ...'); EXECUTE 'TRUNCATE TABLE ' || quote_ident(mviews.mv_name); EXECUTE 'INSERT INTO ' - || quote_ident(mviews.mv_name) || ' ' + || quote_ident(mviews.mv_name) || ' ' || mviews.mv_query; END LOOP; @@ -2523,7 +2523,7 @@ OPEN unbound_cursorvar NO An example: -OPEN curs1 FOR EXECUTE 'SELECT * FROM ' || quote_ident(tabname) +OPEN curs1 FOR EXECUTE 'SELECT * FROM ' || quote_ident(tabname) || ' WHERE col1 = $1' USING keyvalue; In this example, the table name is inserted into the query textually, @@ -3012,10 +3012,21 @@ RAISE unique_violation USING MESSAGE = 'Duplicate user ID: ' || user_id; The last variant of RAISE has no parameters at all. This form can only be used inside a BEGIN block's EXCEPTION clause; - it causes the error currently being handled to be re-thrown to the - next enclosing block. + it causes the error currently being handled to be re-thrown. + + + Before PostgreSQL 9.1, RAISE without + parameters was interpreted as re-throwing the error from the block + containing the active exception handler. Thus an EXCEPTION + clause nested within that handler could not catch it, even if the + RAISE was within the nested EXCEPTION clause's + block. This was deemed surprising as well as being incompatible with + Oracle's PL/SQL. + + + If no condition name nor SQLSTATE is specified in a RAISE EXCEPTION command, the default is to use diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index e1f48e3d75..a1ff9120f4 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.261 2010/07/06 19:19:01 momjian Exp $ + * $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.262 2010/08/09 02:25:05 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -327,10 +327,6 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("CONTINUE cannot be used outside a loop"))); - else if (rc == PLPGSQL_RC_RERAISE) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("RAISE without parameters cannot be used outside an exception handler"))); else ereport(ERROR, (errcode(ERRCODE_S_R_E_FUNCTION_EXECUTED_NO_RETURN_STATEMENT), @@ -695,10 +691,6 @@ plpgsql_exec_trigger(PLpgSQL_function *func, ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("CONTINUE cannot be used outside a loop"))); - else if (rc == PLPGSQL_RC_RERAISE) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("RAISE without parameters cannot be used outside an exception handler"))); else ereport(ERROR, (errcode(ERRCODE_S_R_E_FUNCTION_EXECUTED_NO_RETURN_STATEMENT), @@ -1019,6 +1011,7 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block) MemoryContext oldcontext = CurrentMemoryContext; ResourceOwner oldowner = CurrentResourceOwner; ExprContext *old_eval_econtext = estate->eval_econtext; + ErrorData *save_cur_error = estate->cur_error; estate->err_text = gettext_noop("during statement block entry"); @@ -1130,6 +1123,12 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block) unpack_sql_state(edata->sqlerrcode)); assign_text_var(errm_var, edata->message); + /* + * Also set up cur_error so the error data is accessible + * inside the handler. + */ + estate->cur_error = edata; + estate->err_text = NULL; rc = exec_stmts(estate, exception->action); @@ -1141,14 +1140,17 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block) errm_var->value = (Datum) 0; errm_var->isnull = true; - /* re-throw error if requested by handler */ - if (rc == PLPGSQL_RC_RERAISE) - ReThrowError(edata); - break; } } + /* + * Restore previous state of cur_error, whether or not we executed + * a handler. This is needed in case an error got thrown from + * some inner block's exception handler. + */ + estate->cur_error = save_cur_error; + /* If no match found, re-throw the error */ if (e == NULL) ReThrowError(edata); @@ -1156,6 +1158,8 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block) FreeErrorData(edata); } PG_END_TRY(); + + Assert(save_cur_error == estate->cur_error); } else { @@ -1177,7 +1181,6 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block) case PLPGSQL_RC_OK: case PLPGSQL_RC_RETURN: case PLPGSQL_RC_CONTINUE: - case PLPGSQL_RC_RERAISE: return rc; case PLPGSQL_RC_EXIT: @@ -1599,7 +1602,6 @@ exec_stmt_loop(PLpgSQL_execstate *estate, PLpgSQL_stmt_loop *stmt) break; case PLPGSQL_RC_RETURN: - case PLPGSQL_RC_RERAISE: return rc; default: @@ -1663,7 +1665,6 @@ exec_stmt_while(PLpgSQL_execstate *estate, PLpgSQL_stmt_while *stmt) break; case PLPGSQL_RC_RETURN: - case PLPGSQL_RC_RERAISE: return rc; default: @@ -1782,8 +1783,7 @@ exec_stmt_fori(PLpgSQL_execstate *estate, PLpgSQL_stmt_fori *stmt) */ rc = exec_stmts(estate, stmt->body); - if (rc == PLPGSQL_RC_RETURN || - rc == PLPGSQL_RC_RERAISE) + if (rc == PLPGSQL_RC_RETURN) break; /* break out of the loop */ else if (rc == PLPGSQL_RC_EXIT) { @@ -2437,7 +2437,14 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt) /* RAISE with no parameters: re-throw current exception */ if (stmt->condname == NULL && stmt->message == NULL && stmt->options == NIL) - return PLPGSQL_RC_RERAISE; + { + if (estate->cur_error != NULL) + ReThrowError(estate->cur_error); + /* oops, we're not inside a handler */ + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("RAISE without parameters cannot be used outside an exception handler"))); + } if (stmt->condname) { @@ -2637,6 +2644,7 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate, estate->rettupdesc = NULL; estate->exitlabel = NULL; + estate->cur_error = NULL; estate->tuple_store = NULL; if (rsi) diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h index 16e073c210..488c85bd89 100644 --- a/src/pl/plpgsql/src/plpgsql.h +++ b/src/pl/plpgsql/src/plpgsql.h @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/pl/plpgsql/src/plpgsql.h,v 1.130 2010/02/26 02:01:35 momjian Exp $ + * $PostgreSQL: pgsql/src/pl/plpgsql/src/plpgsql.h,v 1.131 2010/08/09 02:25:05 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -115,8 +115,7 @@ enum PLPGSQL_RC_OK, PLPGSQL_RC_EXIT, PLPGSQL_RC_RETURN, - PLPGSQL_RC_CONTINUE, - PLPGSQL_RC_RERAISE + PLPGSQL_RC_CONTINUE }; /* ---------- @@ -699,6 +698,7 @@ typedef struct PLpgSQL_execstate TupleDesc rettupdesc; char *exitlabel; /* the "target" label of the current EXIT or * CONTINUE stmt, if any */ + ErrorData *cur_error; /* current exception handler's error */ Tuplestorestate *tuple_store; /* SRFs accumulate results here */ MemoryContext tuple_store_cxt; diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index f101108aab..34ad4d8651 100644 --- a/src/test/regress/expected/plpgsql.out +++ b/src/test/regress/expected/plpgsql.out @@ -2312,6 +2312,35 @@ $$ language plpgsql; select raise_test2(10); ERROR: too few parameters specified for RAISE CONTEXT: PL/pgSQL function "raise_test2" line 3 at RAISE +-- Test re-RAISE inside a nested exception block. This case is allowed +-- by Oracle's PL/SQL but was handled differently by PG before 9.1. +CREATE FUNCTION reraise_test() RETURNS void AS $$ +BEGIN + BEGIN + RAISE syntax_error; + EXCEPTION + WHEN syntax_error THEN + BEGIN + raise notice 'exception % thrown in inner block, reraising', sqlerrm; + RAISE; + EXCEPTION + WHEN OTHERS THEN + raise notice 'RIGHT - exception % caught in inner block', sqlerrm; + END; + END; +EXCEPTION + WHEN OTHERS THEN + raise notice 'WRONG - exception % caught in outer block', sqlerrm; +END; +$$ LANGUAGE plpgsql; +SELECT reraise_test(); +NOTICE: exception syntax_error thrown in inner block, reraising +NOTICE: RIGHT - exception syntax_error caught in inner block + reraise_test +-------------- + +(1 row) + -- -- reject function definitions that contain malformed SQL queries at -- compile-time, where possible @@ -3577,7 +3606,7 @@ end; $$ language plpgsql; select raise_test(); ERROR: RAISE without parameters cannot be used outside an exception handler -CONTEXT: PL/pgSQL function "raise_test" +CONTEXT: PL/pgSQL function "raise_test" line 3 at RAISE -- check cases where implicit SQLSTATE variable could be confused with -- SQLSTATE as a keyword, cf bug #5524 create or replace function raise_test() returns void as $$ diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql index 612e92d21b..48496f5199 100644 --- a/src/test/regress/sql/plpgsql.sql +++ b/src/test/regress/sql/plpgsql.sql @@ -1966,6 +1966,31 @@ $$ language plpgsql; select raise_test2(10); +-- Test re-RAISE inside a nested exception block. This case is allowed +-- by Oracle's PL/SQL but was handled differently by PG before 9.1. + +CREATE FUNCTION reraise_test() RETURNS void AS $$ +BEGIN + BEGIN + RAISE syntax_error; + EXCEPTION + WHEN syntax_error THEN + BEGIN + raise notice 'exception % thrown in inner block, reraising', sqlerrm; + RAISE; + EXCEPTION + WHEN OTHERS THEN + raise notice 'RIGHT - exception % caught in inner block', sqlerrm; + END; + END; +EXCEPTION + WHEN OTHERS THEN + raise notice 'WRONG - exception % caught in outer block', sqlerrm; +END; +$$ LANGUAGE plpgsql; + +SELECT reraise_test(); + -- -- reject function definitions that contain malformed SQL queries at -- compile-time, where possible -- 2.40.0