diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 9929e04e57bbc7d08938765b7f506c05c07b772b..1f40f3cf69234ff650ece2ccb09791f8e075ec1a 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -4491,7 +4491,18 @@ loop_exit:
* a Datum by directly calling ExecEvalExpr().
*
* If successful, store results into *result, *isNull, *rettype and return
- * TRUE. If the expression is not simple (any more), return FALSE.
+ * TRUE. If the expression cannot be handled by simple evaluation,
+ * return FALSE.
+ *
+ * Because we only store one execution tree for a simple expression, we
+ * can't handle recursion cases. So, if we see the tree is already busy
+ * with an evaluation in the current xact, we just return FALSE and let the
+ * caller run the expression the hard way. (Other alternatives such as
+ * creating a new tree for a recursive call either introduce memory leaks,
+ * or add enough bookkeeping to be doubtful wins anyway.) Another case that
+ * is covered by the expr_simple_in_use test is where a previous execution
+ * of the tree was aborted by an error: the tree may contain bogus state
+ * so we dare not re-use it.
*
* It is possible though unlikely for a simple expression to become non-simple
* (consider for example redefining a trivial view). We must handle that for
@@ -4527,6 +4538,12 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
if (expr->expr_simple_expr == NULL)
return false;
+ /*
+ * If expression is in use in current xact, don't touch it.
+ */
+ if (expr->expr_simple_in_use && expr->expr_simple_lxid == curlxid)
+ return false;
+
/*
* Revalidate cached plan, so that we will notice if it became stale. (We
* also need to hold a refcount while using the plan.) Note that even if
@@ -4562,6 +4579,7 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
{
expr->expr_simple_state = ExecPrepareExpr(expr->expr_simple_expr,
simple_eval_estate);
+ expr->expr_simple_in_use = false;
expr->expr_simple_lxid = curlxid;
}
@@ -4596,6 +4614,11 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
paramLI = setup_param_list(estate, expr);
econtext->ecxt_param_list_info = paramLI;
+ /*
+ * 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
*/
@@ -4605,6 +4628,8 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
NULL);
/* Assorted cleanup */
+ expr->expr_simple_in_use = false;
+
estate->cur_expr = save_cur_expr;
if (!estate->readonly_func)
@@ -5341,6 +5366,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_lxid = InvalidLocalTransactionId;
/* 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 5b174c49b8bcac4658ffd54fc4493e43221533e7..155796be0f5b2d1ad7850f5734fc6363d3c58c15 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -214,10 +214,12 @@ typedef struct PLpgSQL_expr
/*
* if expr is simple AND prepared in current transaction,
- * expr_simple_state is valid. Test validity by seeing if expr_simple_lxid
- * matches current LXID.
+ * expr_simple_state and expr_simple_in_use are valid. Test validity by
+ * seeing if expr_simple_lxid matches current LXID. (If not,
+ * expr_simple_state probably points at garbage!)
*/
- 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 */
LocalTransactionId expr_simple_lxid;
} PLpgSQL_expr;
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index b454e63e58ac886a778309ef6f3f9326355bcb26..cceae0357210def7172a374e4aef45f02969e40a 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -3988,6 +3988,53 @@ 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 3 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);
-- Test handling of string literals.
set standard_conforming_strings = off;
create or replace function strtest() returns text as $$
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index 4811558c4a978126473317debe42a3d76d201e1c..52735b718fc6353e9865624ce741a3a2b46283e2 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -3190,6 +3190,48 @@ 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);
+
-- Test handling of string literals.
set standard_conforming_strings = off;