Hi, here is another patch related to using CALL statement inside PL/pgSQL code.
A repeated using of CALL statement is expensive. How much? I wrote synthetic test: CREATE TABLE foo(a int, b int, c int); CREATE OR REPLACE PROCEDURE public.simple_proc3(a integer, b integer, c integer, cnt int, OUT r boolean) AS $$ BEGIN INSERT INTO foo VALUES(a, b, c); IF cnt % 10000 = 0 THEN COMMIT; r := true; ELSE r := false; END IF; END; $$ LANGUAGE plpgsql; DO $$ DECLARE a int; b int; c int; r boolean; BEGIN TRUNCATE foo; FOR i IN 1..10000000 LOOP a := (random() * 100)::int; b := (random() * 100)::int; c := (random() * 100)::int; CALL simple_proc3(a, b, c, i, r); IF r THEN RAISE NOTICE 'committed at % row', i; END IF; END LOOP; END; $$; I try to insert 10M rows with commit after inserting 10K rows. Execution time on master is ~ 6 minutes 368251,691 ms (06:08,252) DO $$ DECLARE a int; b int; c int; r boolean; BEGIN TRUNCATE foo; FOR i IN 1..10000000 LOOP a := (random() * 100)::int; b := (random() * 100)::int; c := (random() * 100)::int; INSERT INTO foo VALUES(a, b, c); IF i % 10000 = 0 THEN COMMIT; r := true; ELSE r := false; END IF; IF r THEN RAISE NOTICE 'committed at % row', i; END IF; END LOOP; END; $$; When I try to remove CALL statement then same work needs less to 2 minutes 99109,511 ms (01:39,110). So this code is three times slower with calling one procedure. There are two significant parts of overhead: a) internal implementation of CALL statement that doesn't use plan cache well, and it does lot of expensive operations over pg_proc catalogue, b) wrapper in PL/pgSQL that repeatedly repearse expression string. Overhead of PL/pgSQL can be reduced by using plan cache after fixing issue with resource owner. I did it, and I introduced "local_resowner" for holding references of plans for CALL statement expressions. After patching the execution time is reduced to 4 minutes Time: 245852,846 ms (04:05,853). Still the overhead is significant, but it is 30% speedup. The best case for this patch is about 2x times better performance CREATE OR REPLACE PROCEDURE public.simple_proc2(a integer, b integer, c integer, cnt int, OUT r boolean) AS $$ BEGIN END; $$ LANGUAGE plpgsql; DO $$ DECLARE a int; r boolean; BEGIN FOR i IN 1..10000000 LOOP CALL simple_proc2((random()*100)::int, (random()*100)::int, (random()*100)::int, i, r); END LOOP; END; $$; Time: 184667,970 ms (03:04,668), master: Time: 417463,457 ms (06:57,463) On second hand, the worst case is about 10% (probably this overhead can be reduced by creating "local_resowner" only when it is necessary) CREATE OR REPLACE FUNCTION simple_fx2(a int) RETURNS int AS $$ BEGIN RETURN a + a; END; $$ LANGUAGE plpgsql IMMUTABLE STRICT; DO $$ DECLARE a int; BEGIN FOR i IN 1..10000000 LOOP a := simple_fx2(i); END LOOP; END; $$; Time: 5434,808 ms (00:05,435) , master: Time: 4632,762 ms (00:04,633) Comments, notes, ideas? Regards Pavel
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c index 4b18be5b27..c4011a5b58 100644 --- a/src/backend/commands/prepare.c +++ b/src/backend/commands/prepare.c @@ -230,7 +230,7 @@ ExecuteQuery(ParseState *pstate, entry->plansource->query_string); /* Replan if needed, and increment plan refcount for portal */ - cplan = GetCachedPlan(entry->plansource, paramLI, false, NULL); + cplan = GetCachedPlan(entry->plansource, paramLI, false, NULL, NULL); plan_list = cplan->stmt_list; /* @@ -653,7 +653,7 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es, } /* Replan if needed, and acquire a transient refcount */ - cplan = GetCachedPlan(entry->plansource, paramLI, true, queryEnv); + cplan = GetCachedPlan(entry->plansource, paramLI, true, CurrentResourceOwner, queryEnv); INSTR_TIME_SET_CURRENT(planduration); INSTR_TIME_SUBTRACT(planduration, planstart); @@ -689,7 +689,7 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es, if (estate) FreeExecutorState(estate); - ReleaseCachedPlan(cplan, true); + ReleaseCachedPlan(cplan, true, CurrentResourceOwner); } /* diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index 055ebb77ae..39c19f3b40 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -61,7 +61,7 @@ static void _SPI_prepare_oneshot_plan(const char *src, SPIPlanPtr plan); static int _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, Snapshot snapshot, Snapshot crosscheck_snapshot, bool read_only, bool fire_triggers, uint64 tcount, - DestReceiver *caller_dest); + DestReceiver *caller_dest, ResourceOwner owner); static ParamListInfo _SPI_convert_params(int nargs, Oid *argtypes, Datum *Values, const char *Nulls); @@ -514,7 +514,8 @@ SPI_execute(const char *src, bool read_only, long tcount) res = _SPI_execute_plan(&plan, NULL, InvalidSnapshot, InvalidSnapshot, - read_only, true, tcount, NULL); + read_only, true, tcount, NULL, + CurrentResourceOwner); _SPI_end_call(true); return res; @@ -548,7 +549,8 @@ SPI_execute_plan(SPIPlanPtr plan, Datum *Values, const char *Nulls, _SPI_convert_params(plan->nargs, plan->argtypes, Values, Nulls), InvalidSnapshot, InvalidSnapshot, - read_only, true, tcount, NULL); + read_only, true, tcount, NULL, + CurrentResourceOwner); _SPI_end_call(true); return res; @@ -577,7 +579,8 @@ SPI_execute_plan_with_paramlist(SPIPlanPtr plan, ParamListInfo params, res = _SPI_execute_plan(plan, params, InvalidSnapshot, InvalidSnapshot, - read_only, true, tcount, NULL); + read_only, true, tcount, NULL, + CurrentResourceOwner); _SPI_end_call(true); return res; @@ -606,7 +609,37 @@ SPI_execute_plan_with_receiver(SPIPlanPtr plan, res = _SPI_execute_plan(plan, params, InvalidSnapshot, InvalidSnapshot, - read_only, true, tcount, dest); + read_only, true, tcount, dest, + CurrentResourceOwner); + + _SPI_end_call(true); + return res; +} + + +/* + * Execute a previously prepared plan with possibility to + * specify resource owner + */ +int +SPI_execute_plan_with_resowner(SPIPlanPtr plan, + ParamListInfo params, + bool read_only, long tcount, + ResourceOwner owner) +{ + int res; + + if (plan == NULL || plan->magic != _SPI_PLAN_MAGIC || tcount < 0) + return SPI_ERROR_ARGUMENT; + + res = _SPI_begin_call(true); + if (res < 0) + return res; + + res = _SPI_execute_plan(plan, params, + InvalidSnapshot, InvalidSnapshot, + read_only, true, tcount, NULL, + owner); _SPI_end_call(true); return res; @@ -647,7 +680,8 @@ SPI_execute_snapshot(SPIPlanPtr plan, _SPI_convert_params(plan->nargs, plan->argtypes, Values, Nulls), snapshot, crosscheck_snapshot, - read_only, fire_triggers, tcount, NULL); + read_only, fire_triggers, tcount, NULL, + CurrentResourceOwner); _SPI_end_call(true); return res; @@ -694,7 +728,8 @@ SPI_execute_with_args(const char *src, res = _SPI_execute_plan(&plan, paramLI, InvalidSnapshot, InvalidSnapshot, - read_only, true, tcount, NULL); + read_only, true, tcount, NULL, + CurrentResourceOwner); _SPI_end_call(true); return res; @@ -737,7 +772,8 @@ SPI_execute_with_receiver(const char *src, res = _SPI_execute_plan(&plan, params, InvalidSnapshot, InvalidSnapshot, - read_only, true, tcount, dest); + read_only, true, tcount, dest, + CurrentResourceOwner); _SPI_end_call(true); return res; @@ -1502,7 +1538,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan, */ /* Replan if needed, and increment plan refcount for portal */ - cplan = GetCachedPlan(plansource, paramLI, false, _SPI_current->queryEnv); + cplan = GetCachedPlan(plansource, paramLI, false, NULL, _SPI_current->queryEnv); stmt_list = cplan->stmt_list; if (!plan->saved) @@ -1516,7 +1552,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan, oldcontext = MemoryContextSwitchTo(portal->portalContext); stmt_list = copyObject(stmt_list); MemoryContextSwitchTo(oldcontext); - ReleaseCachedPlan(cplan, false); + ReleaseCachedPlan(cplan, false, NULL); cplan = NULL; /* portal shouldn't depend on cplan */ } @@ -1930,6 +1966,7 @@ SPI_plan_get_cached_plan(SPIPlanPtr plan) /* Get the generic plan for the query */ cplan = GetCachedPlan(plansource, NULL, plan->saved, + CurrentResourceOwner, _SPI_current->queryEnv); Assert(cplan == plansource->gplan); @@ -2212,7 +2249,7 @@ static int _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, Snapshot snapshot, Snapshot crosscheck_snapshot, bool read_only, bool fire_triggers, uint64 tcount, - DestReceiver *caller_dest) + DestReceiver *caller_dest, ResourceOwner owner) { int my_res = 0; uint64 my_processed = 0; @@ -2323,9 +2360,14 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, /* * Replan if needed, and increment plan refcount. If it's a saved - * plan, the refcount must be backed by the CurrentResourceOwner. + * plan, the refcount must be backed by the owner. */ - cplan = GetCachedPlan(plansource, paramLI, plan->saved, _SPI_current->queryEnv); + cplan = GetCachedPlan(plansource, + paramLI, + plan->saved, + owner, + _SPI_current->queryEnv); + stmt_list = cplan->stmt_list; /* @@ -2521,7 +2563,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, } /* Done with this plan, so release refcount */ - ReleaseCachedPlan(cplan, plan->saved); + ReleaseCachedPlan(cplan, plan->saved, owner); cplan = NULL; /* @@ -2541,7 +2583,7 @@ fail: /* We no longer need the cached plan refcount, if any */ if (cplan) - ReleaseCachedPlan(cplan, plan->saved); + ReleaseCachedPlan(cplan, plan->saved, owner); /* * Pop the error context stack diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 411cfadbff..536843b795 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -1962,7 +1962,7 @@ exec_bind_message(StringInfo input_message) * will be generated in MessageContext. The plan refcount will be * assigned to the Portal, so it will be released at portal destruction. */ - cplan = GetCachedPlan(psrc, params, false, NULL); + cplan = GetCachedPlan(psrc, params, false, NULL, NULL); /* * Now we can define the portal. diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c index 50d6ad28b4..54b6e91b82 100644 --- a/src/backend/utils/cache/plancache.c +++ b/src/backend/utils/cache/plancache.c @@ -533,7 +533,7 @@ ReleaseGenericPlan(CachedPlanSource *plansource) Assert(plan->magic == CACHEDPLAN_MAGIC); plansource->gplan = NULL; - ReleaseCachedPlan(plan, false); + ReleaseCachedPlan(plan, false, NULL); } } @@ -1139,7 +1139,8 @@ cached_plan_cost(CachedPlan *plan, bool include_planner) */ CachedPlan * GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams, - bool useResOwner, QueryEnvironment *queryEnv) + bool useResOwner, ResourceOwner owner, + QueryEnvironment *queryEnv) { CachedPlan *plan = NULL; List *qlist; @@ -1229,10 +1230,10 @@ GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams, /* Flag the plan as in use by caller */ if (useResOwner) - ResourceOwnerEnlargePlanCacheRefs(CurrentResourceOwner); + ResourceOwnerEnlargePlanCacheRefs(owner); plan->refcount++; if (useResOwner) - ResourceOwnerRememberPlanCacheRef(CurrentResourceOwner, plan); + ResourceOwnerRememberPlanCacheRef(owner, plan); /* * Saved plans should be under CacheMemoryContext so they will not go away @@ -1261,13 +1262,13 @@ GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams, * Portal. Transient references should be protected by a resource owner. */ void -ReleaseCachedPlan(CachedPlan *plan, bool useResOwner) +ReleaseCachedPlan(CachedPlan *plan, bool useResOwner, ResourceOwner owner) { Assert(plan->magic == CACHEDPLAN_MAGIC); if (useResOwner) { Assert(plan->is_saved); - ResourceOwnerForgetPlanCacheRef(CurrentResourceOwner, plan); + ResourceOwnerForgetPlanCacheRef(owner, plan); } Assert(plan->refcount > 0); plan->refcount--; diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c index ec6f80ee99..3db634d128 100644 --- a/src/backend/utils/mmgr/portalmem.c +++ b/src/backend/utils/mmgr/portalmem.c @@ -310,7 +310,7 @@ PortalReleaseCachedPlan(Portal portal) { if (portal->cplan) { - ReleaseCachedPlan(portal->cplan, false); + ReleaseCachedPlan(portal->cplan, false, NULL); portal->cplan = NULL; /* diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c index 8bc2c4e9ea..2d4ab6afca 100644 --- a/src/backend/utils/resowner/resowner.c +++ b/src/backend/utils/resowner/resowner.c @@ -637,7 +637,7 @@ ResourceOwnerReleaseInternal(ResourceOwner owner, if (isCommit) PrintPlanCacheLeakWarning(res); - ReleaseCachedPlan(res, true); + ReleaseCachedPlan(res, true, owner); } /* Ditto for tupdesc references */ @@ -688,18 +688,14 @@ ResourceOwnerReleaseInternal(ResourceOwner owner, void ResourceOwnerReleaseAllPlanCacheRefs(ResourceOwner owner) { - ResourceOwner save; Datum foundres; - save = CurrentResourceOwner; - CurrentResourceOwner = owner; while (ResourceArrayGetAny(&(owner->planrefarr), &foundres)) { CachedPlan *res = (CachedPlan *) DatumGetPointer(foundres); - ReleaseCachedPlan(res, true); + ReleaseCachedPlan(res, true, owner); } - CurrentResourceOwner = save; } /* diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h index 896ec0a2ad..e055f1b9b5 100644 --- a/src/include/executor/spi.h +++ b/src/include/executor/spi.h @@ -94,6 +94,10 @@ extern int SPI_execute_plan_with_receiver(SPIPlanPtr plan, ParamListInfo params, bool read_only, long tcount, DestReceiver *dest); +extern int SPI_execute_plan_with_resowner(SPIPlanPtr plan, + ParamListInfo params, + bool read_only, long tcount, + ResourceOwner owner); extern int SPI_exec(const char *src, long tcount); extern int SPI_execp(SPIPlanPtr plan, Datum *Values, const char *Nulls, long tcount); diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h index 4901568553..303fdfcdee 100644 --- a/src/include/utils/plancache.h +++ b/src/include/utils/plancache.h @@ -220,8 +220,9 @@ extern List *CachedPlanGetTargetList(CachedPlanSource *plansource, extern CachedPlan *GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams, bool useResOwner, + ResourceOwner owner, QueryEnvironment *queryEnv); -extern void ReleaseCachedPlan(CachedPlan *plan, bool useResOwner); +extern void ReleaseCachedPlan(CachedPlan *plan, bool useResOwner, ResourceOwner owner); extern bool CachedPlanAllowsSimpleValidityCheck(CachedPlanSource *plansource, CachedPlan *plan, diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index ccbc50fc45..7df44f0306 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -329,8 +329,7 @@ static void plpgsql_estate_setup(PLpgSQL_execstate *estate, static void exec_eval_cleanup(PLpgSQL_execstate *estate); static void exec_prepare_plan(PLpgSQL_execstate *estate, - PLpgSQL_expr *expr, int cursorOptions, - bool keepplan); + PLpgSQL_expr *expr, int cursorOptions); static void exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr); static void exec_save_simple_expr(PLpgSQL_expr *expr, CachedPlan *cplan); static void exec_check_rw_parameter(PLpgSQL_expr *expr, int target_dno); @@ -1601,7 +1600,28 @@ exec_toplevel_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block) CHECK_FOR_INTERRUPTS(); - rc = exec_stmt_block(estate, block); + PG_TRY(); + { + /* + * This resource owner is top resource owner. It is not assigned + * to CurrentResourceOwner, that is assigned to current transaction. + */ + estate->local_resowner = ResourceOwnerCreate(NULL, + "PL/pgSQL local resource owner"); + rc = exec_stmt_block(estate, block); + } + PG_CATCH(); + { + ResourceOwnerReleaseAllPlanCacheRefs(estate->local_resowner); + ResourceOwnerDelete(estate->local_resowner); + + /* And propagate the error */ + PG_RE_THROW(); + } + PG_END_TRY(); + + ResourceOwnerReleaseAllPlanCacheRefs(estate->local_resowner); + ResourceOwnerDelete(estate->local_resowner); /* Let the plugin know that we have finished executing this statement */ if (*plpgsql_plugin_ptr && (*plpgsql_plugin_ptr)->stmt_end) @@ -2144,238 +2164,194 @@ exec_stmt_perform(PLpgSQL_execstate *estate, PLpgSQL_stmt_perform *stmt) } /* - * exec_stmt_call - * - * NOTE: this is used for both CALL and DO statements. + * We construct a DTYPE_ROW datum representing the plpgsql variables + * associated with the procedure's output arguments. Then we can use + * exec_move_row() to do the assignments. */ -static int -exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt) +static PLpgSQL_variable * +make_callstmt_target(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) { - PLpgSQL_expr *expr = stmt->expr; - SPIPlanPtr orig_plan = expr->plan; - bool local_plan; - PLpgSQL_variable *volatile cur_target = stmt->target; - volatile LocalTransactionId before_lxid; - LocalTransactionId after_lxid; - volatile bool pushed_active_snap = false; - volatile int rc; + Node *node; + FuncExpr *funcexpr; + HeapTuple func_tuple; + List *funcargs; + Oid *argtypes; + char **argnames; + char *argmodes; + MemoryContext oldcontext; + PLpgSQL_row *row; + int nfields; + int i; + ListCell *lc; + + /* Use eval_mcontext for any cruft accumulated here */ + oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate)); /* - * If not in atomic context, we make a local plan that we'll just use for - * this invocation, and will free at the end. Otherwise, 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. + * Get the parsed CallStmt, and look up the called procedure */ - local_plan = !estate->atomic; - - /* PG_TRY to ensure we clear the plan link, if needed, on failure */ - PG_TRY(); - { - SPIPlanPtr plan = expr->plan; - ParamListInfo paramLI; + node = linitial_node(Query, + ((CachedPlanSource *) linitial(expr->plan->plancache_list))->query_list)->utilityStmt; + if (node == NULL || !IsA(node, CallStmt)) + elog(ERROR, "query for CALL statement is not a CallStmt"); - /* - * Make a plan if we don't have one, or if we need a local one. Note - * that we'll overwrite expr->plan either way; the PG_TRY block will - * ensure we undo that on the way out, if the plan is local. - */ - if (plan == NULL || local_plan) - { - /* Don't let SPI save the plan if it's going to be local */ - exec_prepare_plan(estate, expr, 0, !local_plan); - plan = expr->plan; + funcexpr = ((CallStmt *) node)->funcexpr; - /* - * A CALL or DO can never be a simple expression. (If it could - * be, we'd have to worry about saving/restoring the previous - * values of the related expr fields, not just expr->plan.) - */ - Assert(!expr->expr_simple_expr); + func_tuple = SearchSysCache1(PROCOID, + ObjectIdGetDatum(funcexpr->funcid)); + if (!HeapTupleIsValid(func_tuple)) + elog(ERROR, "cache lookup failed for function %u", + funcexpr->funcid); - /* - * The procedure call could end transactions, which would upset - * the snapshot management in SPI_execute*, so don't let it do it. - * Instead, we set the snapshots ourselves below. - */ - plan->no_snapshots = true; - - /* - * Force target to be recalculated whenever the plan changes, in - * case the procedure's argument list has changed. - */ - stmt->target = NULL; - cur_target = NULL; - } - - /* - * We construct a DTYPE_ROW datum representing the plpgsql variables - * associated with the procedure's output arguments. Then we can use - * exec_move_row() to do the assignments. - * - * If we're using a local plan, also make a local target; otherwise, - * since the above code will force a new plan each time through, we'd - * repeatedly leak the memory for the target. (Note: we also leak the - * target when a plan change is forced, but that isn't so likely to - * cause excessive memory leaks.) - */ - if (stmt->is_call && cur_target == NULL) - { - Node *node; - FuncExpr *funcexpr; - HeapTuple func_tuple; - List *funcargs; - Oid *argtypes; - char **argnames; - char *argmodes; - MemoryContext oldcontext; - PLpgSQL_row *row; - int nfields; - int i; - ListCell *lc; - - /* Use eval_mcontext for any cruft accumulated here */ - oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate)); - - /* - * Get the parsed CallStmt, and look up the called procedure - */ - node = linitial_node(Query, - ((CachedPlanSource *) linitial(plan->plancache_list))->query_list)->utilityStmt; - if (node == NULL || !IsA(node, CallStmt)) - elog(ERROR, "query for CALL statement is not a CallStmt"); - - funcexpr = ((CallStmt *) node)->funcexpr; - - func_tuple = SearchSysCache1(PROCOID, - ObjectIdGetDatum(funcexpr->funcid)); - if (!HeapTupleIsValid(func_tuple)) - elog(ERROR, "cache lookup failed for function %u", - funcexpr->funcid); + /* + * Extract function arguments, and expand any named-arg notation + */ + funcargs = expand_function_arguments(funcexpr->args, + funcexpr->funcresulttype, + func_tuple); - /* - * Extract function arguments, and expand any named-arg notation - */ - funcargs = expand_function_arguments(funcexpr->args, - funcexpr->funcresulttype, - func_tuple); + /* + * Get the argument names and modes, too + */ + get_func_arg_info(func_tuple, &argtypes, &argnames, &argmodes); - /* - * Get the argument names and modes, too - */ - get_func_arg_info(func_tuple, &argtypes, &argnames, &argmodes); + ReleaseSysCache(func_tuple); - ReleaseSysCache(func_tuple); + /* + * Begin constructing row Datum; keep it in fn_cxt if it's to be + * long-lived. + */ + MemoryContextSwitchTo(estate->func->fn_cxt); - /* - * Begin constructing row Datum; keep it in fn_cxt if it's to be - * long-lived. - */ - if (!local_plan) - MemoryContextSwitchTo(estate->func->fn_cxt); + row = (PLpgSQL_row *) palloc0(sizeof(PLpgSQL_row)); + row->dtype = PLPGSQL_DTYPE_ROW; + row->refname = "(unnamed row)"; + row->lineno = -1; + row->varnos = (int *) palloc(sizeof(int) * list_length(funcargs)); - row = (PLpgSQL_row *) palloc0(sizeof(PLpgSQL_row)); - row->dtype = PLPGSQL_DTYPE_ROW; - row->refname = "(unnamed row)"; - row->lineno = -1; - row->varnos = (int *) palloc(sizeof(int) * list_length(funcargs)); + MemoryContextSwitchTo(get_eval_mcontext(estate)); - if (!local_plan) - MemoryContextSwitchTo(get_eval_mcontext(estate)); + /* + * Examine procedure's argument list. Each output arg position + * should be an unadorned plpgsql variable (Datum), which we can + * insert into the row Datum. + */ + nfields = 0; + i = 0; + foreach(lc, funcargs) + { + Node *n = lfirst(lc); - /* - * Examine procedure's argument list. Each output arg position - * should be an unadorned plpgsql variable (Datum), which we can - * insert into the row Datum. - */ - nfields = 0; - i = 0; - foreach(lc, funcargs) + if (argmodes && + (argmodes[i] == PROARGMODE_INOUT || + argmodes[i] == PROARGMODE_OUT)) + { + if (IsA(n, Param)) { - Node *n = lfirst(lc); - - if (argmodes && - (argmodes[i] == PROARGMODE_INOUT || - argmodes[i] == PROARGMODE_OUT)) - { - if (IsA(n, Param)) - { - Param *param = (Param *) n; + Param *param = (Param *) n; - /* paramid is offset by 1 (see make_datum_param()) */ - row->varnos[nfields++] = param->paramid - 1; - } - else - { - /* report error using parameter name, if available */ - if (argnames && argnames[i] && argnames[i][0]) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("procedure parameter \"%s\" is an output parameter but corresponding argument is not writable", - argnames[i]))); - else - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("procedure parameter %d is an output parameter but corresponding argument is not writable", - i + 1))); - } - } - i++; + /* paramid is offset by 1 (see make_datum_param()) */ + row->varnos[nfields++] = param->paramid - 1; } + else + { + /* report error using parameter name, if available */ + if (argnames && argnames[i] && argnames[i][0]) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("procedure parameter \"%s\" is an output parameter but corresponding argument is not writable", + argnames[i]))); + else + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("procedure parameter %d is an output parameter but corresponding argument is not writable", + i + 1))); + } + } + i++; + } - row->nfields = nfields; + row->nfields = nfields; - cur_target = (PLpgSQL_variable *) row; + MemoryContextSwitchTo(oldcontext); - /* We can save and re-use the target datum, if it's not local */ - if (!local_plan) - stmt->target = cur_target; + return (PLpgSQL_variable *) row; +} - MemoryContextSwitchTo(oldcontext); - } +/* + * exec_stmt_call + * + * NOTE: this is used for both CALL and DO statements. + */ +static int +exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt) +{ + PLpgSQL_expr *expr = stmt->expr; + LocalTransactionId before_lxid; + LocalTransactionId after_lxid; + bool pushed_active_snap = false; + volatile int rc; + ParamListInfo paramLI; - paramLI = setup_param_list(estate, expr); + /* + * Make a plan if we don't have one, or if we need a local one. Note + * that we'll overwrite expr->plan either way; the PG_TRY block will + * ensure we undo that on the way out, if the plan is local. + */ + if (expr->plan == NULL) + { + /* Don't let SPI save the plan if it's going to be local */ + exec_prepare_plan(estate, expr, 0); - before_lxid = MyProc->lxid; + /* + * A CALL or DO can never be a simple expression. (If it could + * be, we'd have to worry about saving/restoring the previous + * values of the related expr fields, not just expr->plan.) + */ + Assert(!expr->expr_simple_expr); /* - * Set snapshot only for non-read-only procedures, similar to SPI - * behavior. + * The procedure call could end transactions, which would upset + * the snapshot management in SPI_execute*, so don't let it do it. + * Instead, we set the snapshots ourselves below. */ - if (!estate->readonly_func) - { - PushActiveSnapshot(GetTransactionSnapshot()); - pushed_active_snap = true; - } + expr->plan->no_snapshots = true; - rc = SPI_execute_plan_with_paramlist(expr->plan, paramLI, - estate->readonly_func, 0); - } - PG_CATCH(); - { /* - * If we are using a local plan, restore the old plan link. + * We construct a DTYPE_ROW datum representing the plpgsql variables + * associated with the procedure's output arguments. Then we can use + * exec_move_row() to do the assignments. */ - if (local_plan) - expr->plan = orig_plan; - PG_RE_THROW(); + if (stmt->is_call) + stmt->target = make_callstmt_target(estate, expr); } - PG_END_TRY(); + + paramLI = setup_param_list(estate, expr); + + before_lxid = MyProc->lxid; /* - * If we are using a local plan, restore the old plan link; then free the - * local plan to avoid memory leaks. (Note that the error exit path above - * just clears the link without risking calling SPI_freeplan; we expect - * that xact cleanup will take care of the mess in that case.) + * Set snapshot only for non-read-only procedures, similar to SPI + * behavior. */ - if (local_plan) + if (!estate->readonly_func) { - SPIPlanPtr plan = expr->plan; + PushActiveSnapshot(GetTransactionSnapshot()); + pushed_active_snap = true; + } - expr->plan = orig_plan; - SPI_freeplan(plan); + PG_TRY(); + { + rc = SPI_execute_plan_with_resowner(expr->plan, paramLI, + estate->readonly_func, 0, + estate->local_resowner); + } + PG_CATCH(); + { + ResourceOwnerReleaseAllPlanCacheRefs(estate->local_resowner); + PG_RE_THROW(); } + PG_END_TRY(); if (rc < 0) elog(ERROR, "SPI_execute_plan_with_paramlist failed executing query \"%s\": %s", @@ -2412,10 +2388,10 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt) { SPITupleTable *tuptab = SPI_tuptable; - if (!cur_target) + if (!stmt->is_call) elog(ERROR, "DO statement returned a row"); - exec_move_row(estate, cur_target, tuptab->vals[0], tuptab->tupdesc); + exec_move_row(estate, stmt->target, tuptab->vals[0], tuptab->tupdesc); } else if (SPI_processed > 1) elog(ERROR, "procedure call returned more than one row"); @@ -2962,7 +2938,7 @@ exec_stmt_forc(PLpgSQL_execstate *estate, PLpgSQL_stmt_forc *stmt) Assert(query); if (query->plan == NULL) - exec_prepare_plan(estate, query, curvar->cursor_options, true); + exec_prepare_plan(estate, query, curvar->cursor_options); /* * Set up ParamListInfo for this query @@ -3614,7 +3590,7 @@ exec_stmt_return_query(PLpgSQL_execstate *estate, * On the first call for this expression generate the plan. */ if (expr->plan == NULL) - exec_prepare_plan(estate, expr, CURSOR_OPT_PARALLEL_OK, true); + exec_prepare_plan(estate, expr, CURSOR_OPT_PARALLEL_OK); /* * Set up ParamListInfo to pass to executor @@ -4166,8 +4142,7 @@ exec_eval_cleanup(PLpgSQL_execstate *estate) */ static void exec_prepare_plan(PLpgSQL_execstate *estate, - PLpgSQL_expr *expr, int cursorOptions, - bool keepplan) + PLpgSQL_expr *expr, int cursorOptions) { SPIPlanPtr plan; @@ -4187,8 +4162,8 @@ exec_prepare_plan(PLpgSQL_execstate *estate, if (plan == NULL) elog(ERROR, "SPI_prepare_params failed for \"%s\": %s", expr->query, SPI_result_code_string(SPI_result)); - if (keepplan) - SPI_keepplan(plan); + + SPI_keepplan(plan); expr->plan = plan; /* Check to see if it's a simple expression */ @@ -4233,7 +4208,7 @@ exec_stmt_execsql(PLpgSQL_execstate *estate, { ListCell *l; - exec_prepare_plan(estate, expr, CURSOR_OPT_PARALLEL_OK, true); + exec_prepare_plan(estate, expr, CURSOR_OPT_PARALLEL_OK); stmt->mod_stmt = false; foreach(l, SPI_plan_get_plan_sources(expr->plan)) { @@ -4692,7 +4667,7 @@ exec_stmt_open(PLpgSQL_execstate *estate, PLpgSQL_stmt_open *stmt) */ query = stmt->query; if (query->plan == NULL) - exec_prepare_plan(estate, query, stmt->cursor_options, true); + exec_prepare_plan(estate, query, stmt->cursor_options); } else if (stmt->dynquery != NULL) { @@ -4763,7 +4738,7 @@ exec_stmt_open(PLpgSQL_execstate *estate, PLpgSQL_stmt_open *stmt) query = curvar->cursor_explicit_expr; if (query->plan == NULL) - exec_prepare_plan(estate, query, curvar->cursor_options, true); + exec_prepare_plan(estate, query, curvar->cursor_options); } /* @@ -5000,7 +4975,7 @@ exec_stmt_set(PLpgSQL_execstate *estate, PLpgSQL_stmt_set *stmt) if (expr->plan == NULL) { - exec_prepare_plan(estate, expr, 0, true); + exec_prepare_plan(estate, expr, 0); expr->plan->no_snapshots = true; } @@ -5034,7 +5009,7 @@ exec_assign_expr(PLpgSQL_execstate *estate, PLpgSQL_datum *target, */ if (expr->plan == NULL) { - exec_prepare_plan(estate, expr, 0, true); + exec_prepare_plan(estate, expr, 0); if (target->dtype == PLPGSQL_DTYPE_VAR) exec_check_rw_parameter(expr, target->dno); } @@ -5893,7 +5868,7 @@ exec_eval_expr(PLpgSQL_execstate *estate, * If first time through, create a plan for this expression. */ if (expr->plan == NULL) - exec_prepare_plan(estate, expr, CURSOR_OPT_PARALLEL_OK, true); + exec_prepare_plan(estate, expr, CURSOR_OPT_PARALLEL_OK); /* * If this is a simple expression, bypass SPI and use the executor @@ -5979,7 +5954,7 @@ exec_run_select(PLpgSQL_execstate *estate, */ if (expr->plan == NULL) exec_prepare_plan(estate, expr, - portalP == NULL ? CURSOR_OPT_PARALLEL_OK : 0, true); + portalP == NULL ? CURSOR_OPT_PARALLEL_OK : 0); /* * Set up ParamListInfo to pass to executor @@ -6252,11 +6227,7 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, */ if (expr->expr_simple_plan_lxid == curlxid) { - ResourceOwner saveResourceOwner = CurrentResourceOwner; - - CurrentResourceOwner = estate->simple_eval_resowner; - ReleaseCachedPlan(expr->expr_simple_plan, true); - CurrentResourceOwner = saveResourceOwner; + ReleaseCachedPlan(expr->expr_simple_plan, true, estate->simple_eval_resowner); expr->expr_simple_plan = NULL; expr->expr_simple_plan_lxid = InvalidLocalTransactionId; } @@ -6290,7 +6261,7 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, else { /* Release SPI_plan_get_cached_plan's refcount */ - ReleaseCachedPlan(cplan, true); + ReleaseCachedPlan(cplan, true, CurrentResourceOwner); /* Mark expression as non-simple, and fail */ expr->expr_simple_expr = NULL; return false; @@ -6300,7 +6271,7 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, * SPI_plan_get_cached_plan acquired a plan refcount stored in the * active resowner. We don't need that anymore, so release it. */ - ReleaseCachedPlan(cplan, true); + ReleaseCachedPlan(cplan, true, CurrentResourceOwner); /* Extract desired scalar expression from cached plan */ exec_save_simple_expr(expr, cplan); @@ -8218,7 +8189,7 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) * Release the plan refcount obtained by SPI_plan_get_cached_plan. (This * refcount is held by the wrong resowner, so we can't just repurpose it.) */ - ReleaseCachedPlan(cplan, true); + ReleaseCachedPlan(cplan, true, CurrentResourceOwner); } /* diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h index 0c3d30fb13..75894519d8 100644 --- a/src/pl/plpgsql/src/plpgsql.h +++ b/src/pl/plpgsql/src/plpgsql.h @@ -1095,6 +1095,9 @@ typedef struct PLpgSQL_execstate EState *simple_eval_estate; ResourceOwner simple_eval_resowner; + /* routine execution resource owner */ + ResourceOwner local_resowner; + /* lookup table to use for executing type casts */ HTAB *cast_hash; MemoryContext cast_hash_context;