Fix plan cache issue in PL/pgSQL CALL
authorPeter Eisentraut <peter_e@gmx.net>
Thu, 5 Apr 2018 18:51:56 +0000 (14:51 -0400)
committerPeter Eisentraut <peter_e@gmx.net>
Thu, 5 Apr 2018 18:51:56 +0000 (14:51 -0400)
If we are not going to save the plan, then we need to unset expr->plan
after we are done, also in error cases.  Otherwise, we get a dangling
pointer next time around.

This is not the ideal solution.  It would be better if we could convince
SPI not to associate a cached plan with a resource owner, and then we
could just save the plan in all cases.  But that would require bigger
surgery.

Reported-by: Pavel Stehule <pavel.stehule@gmail.com>
src/pl/plpgsql/src/expected/plpgsql_call.out
src/pl/plpgsql/src/pl_exec.c
src/pl/plpgsql/src/sql/plpgsql_call.sql

index 1e94a44f2bb238a2a2dda37618e4d386ddbfeaf3..ab9d3bbc701fe41f9b9c8f4b750a127b11ea7c2d 100644 (file)
@@ -53,6 +53,16 @@ SELECT * FROM test1;
  66
 (2 rows)
 
+CALL test_proc4(66);
+SELECT * FROM test1;
+ a  
+----
+ 66
+ 66
+ 66
+ 66
+(4 rows)
+
 -- output arguments
 CREATE PROCEDURE test_proc5(INOUT a text)
 LANGUAGE plpgsql
index 67123f85c95abb1be260e69b871fbae624e60709..255bdbf2c8e597d64e8a37150edb99bdb57175be 100644 (file)
@@ -2060,6 +2060,7 @@ static int
 exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
 {
        PLpgSQL_expr *expr = stmt->expr;
+       SPIPlanPtr      plan;
        ParamListInfo paramLI;
        LocalTransactionId before_lxid;
        LocalTransactionId after_lxid;
@@ -2069,7 +2070,9 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
        {
                /*
                 * Don't save the plan if not in atomic context.  Otherwise,
-                * transaction ends would cause warnings about plan leaks.
+                * transaction ends would cause errors about plancache leaks.  XXX
+                * This would be fixable with some plancache/resowner surgery
+                * elsewhere, but for now we'll just work around this here.
                 */
                exec_prepare_plan(estate, expr, 0, estate->atomic);
 
@@ -2084,8 +2087,27 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
 
        before_lxid = MyProc->lxid;
 
-       rc = SPI_execute_plan_with_paramlist(expr->plan, paramLI,
-                                                                                estate->readonly_func, 0);
+       PG_TRY();
+       {
+               rc = SPI_execute_plan_with_paramlist(expr->plan, paramLI,
+                                                                                        estate->readonly_func, 0);
+       }
+       PG_CATCH();
+       {
+               /*
+                * If we aren't saving the plan, unset the pointer.  Note that it
+                * could have been unset already, in case of a recursive call.
+                */
+               if (expr->plan && !expr->plan->saved)
+                       expr->plan = NULL;
+               PG_RE_THROW();
+       }
+       PG_END_TRY();
+
+       plan = expr->plan;
+
+       if (expr->plan && !expr->plan->saved)
+               expr->plan = NULL;
 
        after_lxid = MyProc->lxid;
 
@@ -2129,7 +2151,7 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
                        /*
                         * Get the original CallStmt
                         */
-                       node = linitial_node(Query, ((CachedPlanSource *) linitial(expr->plan->plancache_list))->query_list)->utilityStmt;
+                       node = linitial_node(Query, ((CachedPlanSource *) linitial(plan->plancache_list))->query_list)->utilityStmt;
                        if (!IsA(node, CallStmt))
                                elog(ERROR, "returned row from not a CallStmt");
 
index f1eed9975a1acfb0a81951e5494a813dd219dbcb..551bb77c7034193acbaf354370cfa8957a1d823d 100644 (file)
@@ -54,6 +54,10 @@ CALL test_proc4(66);
 
 SELECT * FROM test1;
 
+CALL test_proc4(66);
+
+SELECT * FROM test1;
+
 
 -- output arguments