diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 2d1ede46be0..526f9d789e5 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -3920,7 +3920,14 @@ exec_eval_expr(PLpgSQL_execstate *estate, * directly */ if (expr->expr_simple_expr != NULL) - return exec_eval_simple_expr(estate, expr, isNull, rettype); + { + /* + * If expression is in use in current xact, don't touch it. + */ + if (!(expr->expr_simple_in_use && + expr->expr_simple_id == estate->eval_estate_simple_id)) + return exec_eval_simple_expr(estate, expr, isNull, rettype); + } rc = exec_run_select(estate, expr, 2, NULL); if (rc != SPI_OK_SELECT) @@ -4071,6 +4078,7 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, { expr->expr_simple_state = ExecPrepareExpr(expr->expr_simple_expr, estate->eval_estate); + expr->expr_simple_in_use = false; expr->expr_simple_id = estate->eval_estate_simple_id; } @@ -4130,6 +4138,11 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, ActiveSnapshot = CopySnapshot(GetTransactionSnapshot()); } + /* + * Mark expression as busy for the duration of the ExecEvalExpr call. + */ + expr->expr_simple_in_use = true; + /* * Finally we can call the executor to evaluate the expression */ @@ -4137,11 +4150,15 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, econtext, isNull, NULL); + + /* Assorted cleanup */ + expr->expr_simple_in_use = false; MemoryContextSwitchTo(oldcontext); } PG_CATCH(); { /* Restore global vars and propagate error */ + /* note we intentionally don't reset expr_simple_in_use here */ ActiveSnapshot = saveActiveSnapshot; PG_RE_THROW(); } @@ -4741,6 +4758,7 @@ exec_simple_check_plan(PLpgSQL_expr *expr) */ expr->expr_simple_expr = tle->expr; expr->expr_simple_state = NULL; + expr->expr_simple_in_use = false; expr->expr_simple_id = -1; /* Also stash away the expression result type */ expr->expr_simple_type = exprType((Node *) tle->expr); diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h index c48a66a1912..12790e9aceb 100644 --- a/src/pl/plpgsql/src/plpgsql.h +++ b/src/pl/plpgsql/src/plpgsql.h @@ -181,10 +181,11 @@ typedef struct PLpgSQL_expr /* * if expr is simple AND prepared in current eval_estate, - * expr_simple_state is valid. Test validity by seeing if expr_simple_id - * matches eval_estate_simple_id. + * expr_simple_state and expr_simple_in_use are valid. Test validity by + * seeing if expr_simple_id matches eval_estate_simple_id. */ - ExprState *expr_simple_state; + ExprState *expr_simple_state; /* eval tree for expr_simple_expr */ + bool expr_simple_in_use; /* true if eval tree is active */ long int expr_simple_id; /* params to pass to expr */ diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index f2826f10625..ff1a9604df7 100644 --- a/src/test/regress/expected/plpgsql.out +++ b/src/test/regress/expected/plpgsql.out @@ -2977,3 +2977,50 @@ SELECT nonsimple_expr_test(); (1 row) DROP FUNCTION nonsimple_expr_test(); +-- +-- Test cases involving recursion and error recovery in simple expressions +-- (bugs in all versions before October 2010). The problems are most +-- easily exposed by mutual recursion between plpgsql and sql functions. +-- +create function recurse(float8) returns float8 as +$$ +begin + if ($1 < 10) then + return sql_recurse($1 + 1); + else + return $1; + end if; +end; +$$ language plpgsql; +-- "limit" is to prevent this from being inlined +create function sql_recurse(float8) returns float8 as +$$ select recurse($1) limit 1; $$ language sql; +select recurse(0); + recurse +--------- + 10 +(1 row) + +create function error1(text) returns text language sql as +$$ SELECT relname::text FROM pg_class c WHERE c.oid = $1::regclass $$; +create function error2(p_name_table text) returns text language plpgsql as $$ +begin + return error1(p_name_table); +end$$; +BEGIN; +create table public.stuffs (stuff text); +SAVEPOINT a; +select error2('nonexistent.stuffs'); +ERROR: schema "nonexistent" does not exist +CONTEXT: SQL function "error1" statement 1 +PL/pgSQL function "error2" line 2 at return +ROLLBACK TO a; +select error2('public.stuffs'); + error2 +-------- + stuffs +(1 row) + +rollback; +drop function error2(p_name_table text); +drop function error1(text); diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql index 27ab5afbdff..37bdb72fb7e 100644 --- a/src/test/regress/sql/plpgsql.sql +++ b/src/test/regress/sql/plpgsql.sql @@ -2480,3 +2480,45 @@ $$ LANGUAGE plpgsql; SELECT nonsimple_expr_test(); DROP FUNCTION nonsimple_expr_test(); + +-- +-- Test cases involving recursion and error recovery in simple expressions +-- (bugs in all versions before October 2010). The problems are most +-- easily exposed by mutual recursion between plpgsql and sql functions. +-- + +create function recurse(float8) returns float8 as +$$ +begin + if ($1 < 10) then + return sql_recurse($1 + 1); + else + return $1; + end if; +end; +$$ language plpgsql; + +-- "limit" is to prevent this from being inlined +create function sql_recurse(float8) returns float8 as +$$ select recurse($1) limit 1; $$ language sql; + +select recurse(0); + +create function error1(text) returns text language sql as +$$ SELECT relname::text FROM pg_class c WHERE c.oid = $1::regclass $$; + +create function error2(p_name_table text) returns text language plpgsql as $$ +begin + return error1(p_name_table); +end$$; + +BEGIN; +create table public.stuffs (stuff text); +SAVEPOINT a; +select error2('nonexistent.stuffs'); +ROLLBACK TO a; +select error2('public.stuffs'); +rollback; + +drop function error2(p_name_table text); +drop function error1(text);