From 6719b238e8f0ea83c39d2b1728c7670cdaa34e06 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 21 Dec 2017 12:57:41 -0500 Subject: [PATCH] Rearrange execution of PARAM_EXTERN Params for plpgsql's benefit. This patch does three interrelated things: * Create a new expression execution step type EEOP_PARAM_CALLBACK and add the infrastructure needed for add-on modules to generate that. As discussed, the best control mechanism for that seems to be to add another hook function to ParamListInfo, which will be called by ExecInitExpr if it's supplied and a PARAM_EXTERN Param is found. For stand-alone expressions, we add a new entry point to allow the ParamListInfo to be specified directly, since it can't be retrieved from the parent plan node's EState. * Redesign the API for the ParamListInfo paramFetch hook so that the ParamExternData array can be entirely virtual. This also lets us get rid of ParamListInfo.paramMask, instead leaving it to the paramFetch hook to decide which param IDs should be accessible or not. plpgsql_param_fetch was already doing the identical masking check, so having callers do it too seemed redundant. While I was at it, I added a "speculative" flag to paramFetch that the planner can specify as TRUE to avoid unwanted failures. This solves an ancient problem for plpgsql that it couldn't provide values of non-DTYPE_VAR variables to the planner for fear of triggering premature "record not assigned yet" or "field not found" errors during planning. * Rework plpgsql to get rid of the need for "unshared" parameter lists, by dint of turning the single ParamListInfo per estate into a nearly read-only data structure that doesn't instantiate any per-variable data. Instead, the paramFetch hook controls access to per-variable data and can make the right decisions on the fly, replacing the cases that we used to need multiple ParamListInfos for. This might perhaps have been a performance loss on its own, but by using a paramCompile hook we can bypass plpgsql_param_fetch entirely during normal query execution. (It's now only called when, eg, we copy the ParamListInfo into a cursor portal. copyParamList() or SerializeParamList() effectively instantiate the virtual parameter array as a simple physical array without a paramFetch hook, which is what we want in those cases.) This allows reverting most of commit 6c82d8d1f, though I kept the cosmetic code-consolidation aspects of that (eg the assign_simple_var function). Performance testing shows this to be at worst a break-even change, and it can provide wins ranging up to 20% in test cases involving accesses to fields of "record" variables. The fact that values of such variables can now be exposed to the planner might produce wins in some situations, too, but I've not pursued that angle. In passing, remove the "parent" pointer from the arguments to ExecInitExprRec and related functions, instead storing that pointer in a transient field in ExprState. The ParamListInfo pointer for a stand-alone expression is handled the same way; we'd otherwise have had to add yet another recursively-passed-down argument in expression compilation. Discussion: https://postgr.es/m/32589.1513706441@sss.pgh.pa.us --- src/backend/commands/prepare.c | 3 +- src/backend/executor/execCurrent.c | 9 +- src/backend/executor/execExpr.c | 231 ++++++++---- src/backend/executor/execExprInterp.c | 17 +- src/backend/executor/functions.c | 3 +- src/backend/executor/spi.c | 3 +- src/backend/nodes/params.c | 64 ++-- src/backend/optimizer/util/clauses.c | 19 +- src/backend/tcop/postgres.c | 6 +- src/include/executor/execExpr.h | 23 +- src/include/executor/executor.h | 1 + src/include/nodes/execnodes.h | 21 +- src/include/nodes/params.h | 84 ++++- src/pl/plpgsql/src/pl_comp.c | 18 - src/pl/plpgsql/src/pl_exec.c | 508 ++++++++++++++------------ src/pl/plpgsql/src/plpgsql.h | 9 +- 16 files changed, 613 insertions(+), 406 deletions(-) diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c index be7222f003..6e90912ae1 100644 --- a/src/backend/commands/prepare.c +++ b/src/backend/commands/prepare.c @@ -399,10 +399,11 @@ EvaluateParams(PreparedStatement *pstmt, List *params, /* we have static list of params, so no hooks needed */ paramLI->paramFetch = NULL; paramLI->paramFetchArg = NULL; + paramLI->paramCompile = NULL; + paramLI->paramCompileArg = NULL; paramLI->parserSetup = NULL; paramLI->parserSetupArg = NULL; paramLI->numParams = num_params; - paramLI->paramMask = NULL; i = 0; foreach(l, exprstates) diff --git a/src/backend/executor/execCurrent.c b/src/backend/executor/execCurrent.c index a3e962ee67..eaeb3a2836 100644 --- a/src/backend/executor/execCurrent.c +++ b/src/backend/executor/execCurrent.c @@ -216,11 +216,14 @@ fetch_cursor_param_value(ExprContext *econtext, int paramId) if (paramInfo && paramId > 0 && paramId <= paramInfo->numParams) { - ParamExternData *prm = ¶mInfo->params[paramId - 1]; + ParamExternData *prm; + ParamExternData prmdata; /* give hook a chance in case parameter is dynamic */ - if (!OidIsValid(prm->ptype) && paramInfo->paramFetch != NULL) - paramInfo->paramFetch(paramInfo, paramId); + if (paramInfo->paramFetch != NULL) + prm = paramInfo->paramFetch(paramInfo, paramId, false, &prmdata); + else + prm = ¶mInfo->params[paramId - 1]; if (OidIsValid(prm->ptype) && !prm->isnull) { diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index e0839616e1..55bb925191 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -55,22 +55,21 @@ typedef struct LastAttnumInfo } LastAttnumInfo; static void ExecReadyExpr(ExprState *state); -static void ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, +static void ExecInitExprRec(Expr *node, ExprState *state, Datum *resv, bool *resnull); -static void ExprEvalPushStep(ExprState *es, const ExprEvalStep *s); static void ExecInitFunc(ExprEvalStep *scratch, Expr *node, List *args, - Oid funcid, Oid inputcollid, PlanState *parent, + Oid funcid, Oid inputcollid, ExprState *state); static void ExecInitExprSlots(ExprState *state, Node *node); static bool get_last_attnums_walker(Node *node, LastAttnumInfo *info); static void ExecInitWholeRowVar(ExprEvalStep *scratch, Var *variable, - PlanState *parent); + ExprState *state); static void ExecInitArrayRef(ExprEvalStep *scratch, ArrayRef *aref, - PlanState *parent, ExprState *state, + ExprState *state, Datum *resv, bool *resnull); static bool isAssignmentIndirectionExpr(Expr *expr); static void ExecInitCoerceToDomain(ExprEvalStep *scratch, CoerceToDomain *ctest, - PlanState *parent, ExprState *state, + ExprState *state, Datum *resv, bool *resnull); @@ -122,12 +121,51 @@ ExecInitExpr(Expr *node, PlanState *parent) /* Initialize ExprState with empty step list */ state = makeNode(ExprState); state->expr = node; + state->parent = parent; + state->ext_params = NULL; /* Insert EEOP_*_FETCHSOME steps as needed */ ExecInitExprSlots(state, (Node *) node); /* Compile the expression proper */ - ExecInitExprRec(node, parent, state, &state->resvalue, &state->resnull); + ExecInitExprRec(node, state, &state->resvalue, &state->resnull); + + /* Finally, append a DONE step */ + scratch.opcode = EEOP_DONE; + ExprEvalPushStep(state, &scratch); + + ExecReadyExpr(state); + + return state; +} + +/* + * ExecInitExprWithParams: prepare a standalone expression tree for execution + * + * This is the same as ExecInitExpr, except that there is no parent PlanState, + * and instead we may have a ParamListInfo describing PARAM_EXTERN Params. + */ +ExprState * +ExecInitExprWithParams(Expr *node, ParamListInfo ext_params) +{ + ExprState *state; + ExprEvalStep scratch; + + /* Special case: NULL expression produces a NULL ExprState pointer */ + if (node == NULL) + return NULL; + + /* Initialize ExprState with empty step list */ + state = makeNode(ExprState); + state->expr = node; + state->parent = NULL; + state->ext_params = ext_params; + + /* Insert EEOP_*_FETCHSOME steps as needed */ + ExecInitExprSlots(state, (Node *) node); + + /* Compile the expression proper */ + ExecInitExprRec(node, state, &state->resvalue, &state->resnull); /* Finally, append a DONE step */ scratch.opcode = EEOP_DONE; @@ -172,6 +210,9 @@ ExecInitQual(List *qual, PlanState *parent) state = makeNode(ExprState); state->expr = (Expr *) qual; + state->parent = parent; + state->ext_params = NULL; + /* mark expression as to be used with ExecQual() */ state->flags = EEO_FLAG_IS_QUAL; @@ -198,7 +239,7 @@ ExecInitQual(List *qual, PlanState *parent) Expr *node = (Expr *) lfirst(lc); /* first evaluate expression */ - ExecInitExprRec(node, parent, state, &state->resvalue, &state->resnull); + ExecInitExprRec(node, state, &state->resvalue, &state->resnull); /* then emit EEOP_QUAL to detect if it's false (or null) */ scratch.d.qualexpr.jumpdone = -1; @@ -314,6 +355,9 @@ ExecBuildProjectionInfo(List *targetList, projInfo->pi_state.tag.type = T_ExprState; state = &projInfo->pi_state; state->expr = (Expr *) targetList; + state->parent = parent; + state->ext_params = NULL; + state->resultslot = slot; /* Insert EEOP_*_FETCHSOME steps as needed */ @@ -398,7 +442,7 @@ ExecBuildProjectionInfo(List *targetList, * matter) can change between executions. We instead evaluate * into the ExprState's resvalue/resnull and then move. */ - ExecInitExprRec(tle->expr, parent, state, + ExecInitExprRec(tle->expr, state, &state->resvalue, &state->resnull); /* @@ -581,12 +625,11 @@ ExecReadyExpr(ExprState *state) * possibly recursing into sub-expressions of node. * * node - expression to evaluate - * parent - parent executor node (or NULL if a standalone expression) * state - ExprState to whose ->steps to append the necessary operations * resv / resnull - where to store the result of the node into */ static void -ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, +ExecInitExprRec(Expr *node, ExprState *state, Datum *resv, bool *resnull) { ExprEvalStep scratch; @@ -609,7 +652,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, if (variable->varattno == InvalidAttrNumber) { /* whole-row Var */ - ExecInitWholeRowVar(&scratch, variable, parent); + ExecInitWholeRowVar(&scratch, variable, state); } else if (variable->varattno <= 0) { @@ -674,6 +717,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, case T_Param: { Param *param = (Param *) node; + ParamListInfo params; switch (param->paramkind) { @@ -681,19 +725,41 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, scratch.opcode = EEOP_PARAM_EXEC; scratch.d.param.paramid = param->paramid; scratch.d.param.paramtype = param->paramtype; + ExprEvalPushStep(state, &scratch); break; case PARAM_EXTERN: - scratch.opcode = EEOP_PARAM_EXTERN; - scratch.d.param.paramid = param->paramid; - scratch.d.param.paramtype = param->paramtype; + + /* + * If we have a relevant ParamCompileHook, use it; + * otherwise compile a standard EEOP_PARAM_EXTERN + * step. ext_params, if supplied, takes precedence + * over info from the parent node's EState (if any). + */ + if (state->ext_params) + params = state->ext_params; + else if (state->parent && + state->parent->state) + params = state->parent->state->es_param_list_info; + else + params = NULL; + if (params && params->paramCompile) + { + params->paramCompile(params, param, state, + resv, resnull); + } + else + { + scratch.opcode = EEOP_PARAM_EXTERN; + scratch.d.param.paramid = param->paramid; + scratch.d.param.paramtype = param->paramtype; + ExprEvalPushStep(state, &scratch); + } break; default: elog(ERROR, "unrecognized paramkind: %d", (int) param->paramkind); break; } - - ExprEvalPushStep(state, &scratch); break; } @@ -706,9 +772,9 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, scratch.d.aggref.astate = astate; astate->aggref = aggref; - if (parent && IsA(parent, AggState)) + if (state->parent && IsA(state->parent, AggState)) { - AggState *aggstate = (AggState *) parent; + AggState *aggstate = (AggState *) state->parent; aggstate->aggs = lcons(astate, aggstate->aggs); aggstate->numaggs++; @@ -728,14 +794,14 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, GroupingFunc *grp_node = (GroupingFunc *) node; Agg *agg; - if (!parent || !IsA(parent, AggState) || - !IsA(parent->plan, Agg)) + if (!state->parent || !IsA(state->parent, AggState) || + !IsA(state->parent->plan, Agg)) elog(ERROR, "GroupingFunc found in non-Agg plan node"); scratch.opcode = EEOP_GROUPING_FUNC; - scratch.d.grouping_func.parent = (AggState *) parent; + scratch.d.grouping_func.parent = (AggState *) state->parent; - agg = (Agg *) (parent->plan); + agg = (Agg *) (state->parent->plan); if (agg->groupingSets) scratch.d.grouping_func.clauses = grp_node->cols; @@ -753,9 +819,9 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, wfstate->wfunc = wfunc; - if (parent && IsA(parent, WindowAggState)) + if (state->parent && IsA(state->parent, WindowAggState)) { - WindowAggState *winstate = (WindowAggState *) parent; + WindowAggState *winstate = (WindowAggState *) state->parent; int nfuncs; winstate->funcs = lcons(wfstate, winstate->funcs); @@ -764,9 +830,10 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, winstate->numaggs++; /* for now initialize agg using old style expressions */ - wfstate->args = ExecInitExprList(wfunc->args, parent); + wfstate->args = ExecInitExprList(wfunc->args, + state->parent); wfstate->aggfilter = ExecInitExpr(wfunc->aggfilter, - parent); + state->parent); /* * Complain if the windowfunc's arguments contain any @@ -795,7 +862,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, { ArrayRef *aref = (ArrayRef *) node; - ExecInitArrayRef(&scratch, aref, parent, state, resv, resnull); + ExecInitArrayRef(&scratch, aref, state, resv, resnull); break; } @@ -805,7 +872,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, ExecInitFunc(&scratch, node, func->args, func->funcid, func->inputcollid, - parent, state); + state); ExprEvalPushStep(state, &scratch); break; } @@ -816,7 +883,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, ExecInitFunc(&scratch, node, op->args, op->opfuncid, op->inputcollid, - parent, state); + state); ExprEvalPushStep(state, &scratch); break; } @@ -827,7 +894,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, ExecInitFunc(&scratch, node, op->args, op->opfuncid, op->inputcollid, - parent, state); + state); /* * Change opcode of call instruction to EEOP_DISTINCT. @@ -849,7 +916,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, ExecInitFunc(&scratch, node, op->args, op->opfuncid, op->inputcollid, - parent, state); + state); /* * Change opcode of call instruction to EEOP_NULLIF. @@ -896,7 +963,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, opexpr->inputcollid, NULL, NULL); /* Evaluate scalar directly into left function argument */ - ExecInitExprRec(scalararg, parent, state, + ExecInitExprRec(scalararg, state, &fcinfo->arg[0], &fcinfo->argnull[0]); /* @@ -905,7 +972,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, * be overwritten by EEOP_SCALARARRAYOP, and will not be * passed to any other expression. */ - ExecInitExprRec(arrayarg, parent, state, resv, resnull); + ExecInitExprRec(arrayarg, state, resv, resnull); /* And perform the operation */ scratch.opcode = EEOP_SCALARARRAYOP; @@ -949,7 +1016,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, Expr *arg = (Expr *) lfirst(lc); /* Evaluate argument into our output variable */ - ExecInitExprRec(arg, parent, state, resv, resnull); + ExecInitExprRec(arg, state, resv, resnull); /* Perform the appropriate step type */ switch (boolexpr->boolop) @@ -1009,13 +1076,14 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, SubPlan *subplan = (SubPlan *) node; SubPlanState *sstate; - if (!parent) + if (!state->parent) elog(ERROR, "SubPlan found with no parent plan"); - sstate = ExecInitSubPlan(subplan, parent); + sstate = ExecInitSubPlan(subplan, state->parent); - /* add SubPlanState nodes to parent->subPlan */ - parent->subPlan = lappend(parent->subPlan, sstate); + /* add SubPlanState nodes to state->parent->subPlan */ + state->parent->subPlan = lappend(state->parent->subPlan, + sstate); scratch.opcode = EEOP_SUBPLAN; scratch.d.subplan.sstate = sstate; @@ -1029,10 +1097,10 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, AlternativeSubPlan *asplan = (AlternativeSubPlan *) node; AlternativeSubPlanState *asstate; - if (!parent) + if (!state->parent) elog(ERROR, "AlternativeSubPlan found with no parent plan"); - asstate = ExecInitAlternativeSubPlan(asplan, parent); + asstate = ExecInitAlternativeSubPlan(asplan, state->parent); scratch.opcode = EEOP_ALTERNATIVE_SUBPLAN; scratch.d.alternative_subplan.asstate = asstate; @@ -1046,7 +1114,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, FieldSelect *fselect = (FieldSelect *) node; /* evaluate row/record argument into result area */ - ExecInitExprRec(fselect->arg, parent, state, resv, resnull); + ExecInitExprRec(fselect->arg, state, resv, resnull); /* and extract field */ scratch.opcode = EEOP_FIELDSELECT; @@ -1083,7 +1151,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, *descp = NULL; /* emit code to evaluate the composite input value */ - ExecInitExprRec(fstore->arg, parent, state, resv, resnull); + ExecInitExprRec(fstore->arg, state, resv, resnull); /* next, deform the input tuple into our workspace */ scratch.opcode = EEOP_FIELDSTORE_DEFORM; @@ -1134,7 +1202,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, state->innermost_caseval = &values[fieldnum - 1]; state->innermost_casenull = &nulls[fieldnum - 1]; - ExecInitExprRec(e, parent, state, + ExecInitExprRec(e, state, &values[fieldnum - 1], &nulls[fieldnum - 1]); @@ -1158,7 +1226,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, /* relabel doesn't need to do anything at runtime */ RelabelType *relabel = (RelabelType *) node; - ExecInitExprRec(relabel->arg, parent, state, resv, resnull); + ExecInitExprRec(relabel->arg, state, resv, resnull); break; } @@ -1171,7 +1239,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, FunctionCallInfo fcinfo_in; /* evaluate argument into step's result area */ - ExecInitExprRec(iocoerce->arg, parent, state, resv, resnull); + ExecInitExprRec(iocoerce->arg, state, resv, resnull); /* * Prepare both output and input function calls, to be @@ -1228,7 +1296,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, ExprState *elemstate; /* evaluate argument into step's result area */ - ExecInitExprRec(acoerce->arg, parent, state, resv, resnull); + ExecInitExprRec(acoerce->arg, state, resv, resnull); resultelemtype = get_element_type(acoerce->resulttype); if (!OidIsValid(resultelemtype)) @@ -1244,10 +1312,13 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, */ elemstate = makeNode(ExprState); elemstate->expr = acoerce->elemexpr; + elemstate->parent = state->parent; + elemstate->ext_params = state->ext_params; + elemstate->innermost_caseval = (Datum *) palloc(sizeof(Datum)); elemstate->innermost_casenull = (bool *) palloc(sizeof(bool)); - ExecInitExprRec(acoerce->elemexpr, parent, elemstate, + ExecInitExprRec(acoerce->elemexpr, elemstate, &elemstate->resvalue, &elemstate->resnull); if (elemstate->steps_len == 1 && @@ -1290,7 +1361,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, ConvertRowtypeExpr *convert = (ConvertRowtypeExpr *) node; /* evaluate argument into step's result area */ - ExecInitExprRec(convert->arg, parent, state, resv, resnull); + ExecInitExprRec(convert->arg, state, resv, resnull); /* and push conversion step */ scratch.opcode = EEOP_CONVERT_ROWTYPE; @@ -1324,7 +1395,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, caseval = palloc(sizeof(Datum)); casenull = palloc(sizeof(bool)); - ExecInitExprRec(caseExpr->arg, parent, state, + ExecInitExprRec(caseExpr->arg, state, caseval, casenull); /* @@ -1375,7 +1446,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, state->innermost_casenull = casenull; /* evaluate condition into CASE's result variables */ - ExecInitExprRec(when->expr, parent, state, resv, resnull); + ExecInitExprRec(when->expr, state, resv, resnull); state->innermost_caseval = save_innermost_caseval; state->innermost_casenull = save_innermost_casenull; @@ -1390,7 +1461,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, * If WHEN result is true, evaluate THEN result, storing * it into the CASE's result variables. */ - ExecInitExprRec(when->result, parent, state, resv, resnull); + ExecInitExprRec(when->result, state, resv, resnull); /* Emit JUMP step to jump to end of CASE's code */ scratch.opcode = EEOP_JUMP; @@ -1415,7 +1486,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, Assert(caseExpr->defresult); /* evaluate ELSE expr into CASE's result variables */ - ExecInitExprRec(caseExpr->defresult, parent, state, + ExecInitExprRec(caseExpr->defresult, state, resv, resnull); /* adjust jump targets */ @@ -1484,7 +1555,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, { Expr *e = (Expr *) lfirst(lc); - ExecInitExprRec(e, parent, state, + ExecInitExprRec(e, state, &scratch.d.arrayexpr.elemvalues[elemoff], &scratch.d.arrayexpr.elemnulls[elemoff]); elemoff++; @@ -1578,7 +1649,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, } /* Evaluate column expr into appropriate workspace slot */ - ExecInitExprRec(e, parent, state, + ExecInitExprRec(e, state, &scratch.d.row.elemvalues[i], &scratch.d.row.elemnulls[i]); i++; @@ -1667,9 +1738,9 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, */ /* evaluate left and right args directly into fcinfo */ - ExecInitExprRec(left_expr, parent, state, + ExecInitExprRec(left_expr, state, &fcinfo->arg[0], &fcinfo->argnull[0]); - ExecInitExprRec(right_expr, parent, state, + ExecInitExprRec(right_expr, state, &fcinfo->arg[1], &fcinfo->argnull[1]); scratch.opcode = EEOP_ROWCOMPARE_STEP; @@ -1738,7 +1809,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, Expr *e = (Expr *) lfirst(lc); /* evaluate argument, directly into result datum */ - ExecInitExprRec(e, parent, state, resv, resnull); + ExecInitExprRec(e, state, resv, resnull); /* if it's not null, skip to end of COALESCE expr */ scratch.opcode = EEOP_JUMP_IF_NOT_NULL; @@ -1820,7 +1891,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, { Expr *e = (Expr *) lfirst(lc); - ExecInitExprRec(e, parent, state, + ExecInitExprRec(e, state, &scratch.d.minmax.values[off], &scratch.d.minmax.nulls[off]); off++; @@ -1886,7 +1957,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, { Expr *e = (Expr *) lfirst(arg); - ExecInitExprRec(e, parent, state, + ExecInitExprRec(e, state, &scratch.d.xmlexpr.named_argvalue[off], &scratch.d.xmlexpr.named_argnull[off]); off++; @@ -1897,7 +1968,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, { Expr *e = (Expr *) lfirst(arg); - ExecInitExprRec(e, parent, state, + ExecInitExprRec(e, state, &scratch.d.xmlexpr.argvalue[off], &scratch.d.xmlexpr.argnull[off]); off++; @@ -1935,7 +2006,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, scratch.d.nulltest_row.argdesc = NULL; /* first evaluate argument into result variable */ - ExecInitExprRec(ntest->arg, parent, state, + ExecInitExprRec(ntest->arg, state, resv, resnull); /* then push the test of that argument */ @@ -1953,7 +2024,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, * and will get overwritten by the below EEOP_BOOLTEST_IS_* * step. */ - ExecInitExprRec(btest->arg, parent, state, resv, resnull); + ExecInitExprRec(btest->arg, state, resv, resnull); switch (btest->booltesttype) { @@ -1990,7 +2061,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, { CoerceToDomain *ctest = (CoerceToDomain *) node; - ExecInitCoerceToDomain(&scratch, ctest, parent, state, + ExecInitCoerceToDomain(&scratch, ctest, state, resv, resnull); break; } @@ -2046,7 +2117,7 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, * Note that this potentially re-allocates es->steps, therefore no pointer * into that array may be used while the expression is still being built. */ -static void +void ExprEvalPushStep(ExprState *es, const ExprEvalStep *s) { if (es->steps_alloc == 0) @@ -2074,7 +2145,7 @@ ExprEvalPushStep(ExprState *es, const ExprEvalStep *s) */ static void ExecInitFunc(ExprEvalStep *scratch, Expr *node, List *args, Oid funcid, - Oid inputcollid, PlanState *parent, ExprState *state) + Oid inputcollid, ExprState *state) { int nargs = list_length(args); AclResult aclresult; @@ -2126,8 +2197,9 @@ ExecInitFunc(ExprEvalStep *scratch, Expr *node, List *args, Oid funcid, ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("set-valued function called in context that cannot accept a set"), - parent ? executor_errposition(parent->state, - exprLocation((Node *) node)) : 0)); + state->parent ? + executor_errposition(state->parent->state, + exprLocation((Node *) node)) : 0)); /* Build code to evaluate arguments directly into the fcinfo struct */ argno = 0; @@ -2148,7 +2220,7 @@ ExecInitFunc(ExprEvalStep *scratch, Expr *node, List *args, Oid funcid, } else { - ExecInitExprRec(arg, parent, state, + ExecInitExprRec(arg, state, &fcinfo->arg[argno], &fcinfo->argnull[argno]); } argno++; @@ -2260,8 +2332,10 @@ get_last_attnums_walker(Node *node, LastAttnumInfo *info) * The caller still has to push the step. */ static void -ExecInitWholeRowVar(ExprEvalStep *scratch, Var *variable, PlanState *parent) +ExecInitWholeRowVar(ExprEvalStep *scratch, Var *variable, ExprState *state) { + PlanState *parent = state->parent; + /* fill in all but the target */ scratch->opcode = EEOP_WHOLEROW; scratch->d.wholerow.var = variable; @@ -2331,7 +2405,7 @@ ExecInitWholeRowVar(ExprEvalStep *scratch, Var *variable, PlanState *parent) * Prepare evaluation of an ArrayRef expression. */ static void -ExecInitArrayRef(ExprEvalStep *scratch, ArrayRef *aref, PlanState *parent, +ExecInitArrayRef(ExprEvalStep *scratch, ArrayRef *aref, ExprState *state, Datum *resv, bool *resnull) { bool isAssignment = (aref->refassgnexpr != NULL); @@ -2355,7 +2429,7 @@ ExecInitArrayRef(ExprEvalStep *scratch, ArrayRef *aref, PlanState *parent, * be overwritten by the final EEOP_ARRAYREF_FETCH/ASSIGN step, which is * pushed last. */ - ExecInitExprRec(aref->refexpr, parent, state, resv, resnull); + ExecInitExprRec(aref->refexpr, state, resv, resnull); /* * If refexpr yields NULL, and it's a fetch, then result is NULL. We can @@ -2401,7 +2475,7 @@ ExecInitArrayRef(ExprEvalStep *scratch, ArrayRef *aref, PlanState *parent, arefstate->upperprovided[i] = true; /* Each subscript is evaluated into subscriptvalue/subscriptnull */ - ExecInitExprRec(e, parent, state, + ExecInitExprRec(e, state, &arefstate->subscriptvalue, &arefstate->subscriptnull); /* ... and then ARRAYREF_SUBSCRIPT saves it into step's workspace */ @@ -2434,7 +2508,7 @@ ExecInitArrayRef(ExprEvalStep *scratch, ArrayRef *aref, PlanState *parent, arefstate->lowerprovided[i] = true; /* Each subscript is evaluated into subscriptvalue/subscriptnull */ - ExecInitExprRec(e, parent, state, + ExecInitExprRec(e, state, &arefstate->subscriptvalue, &arefstate->subscriptnull); /* ... and then ARRAYREF_SUBSCRIPT saves it into step's workspace */ @@ -2488,7 +2562,7 @@ ExecInitArrayRef(ExprEvalStep *scratch, ArrayRef *aref, PlanState *parent, state->innermost_casenull = &arefstate->prevnull; /* evaluate replacement value into replacevalue/replacenull */ - ExecInitExprRec(aref->refassgnexpr, parent, state, + ExecInitExprRec(aref->refassgnexpr, state, &arefstate->replacevalue, &arefstate->replacenull); state->innermost_caseval = save_innermost_caseval; @@ -2566,8 +2640,7 @@ isAssignmentIndirectionExpr(Expr *expr) */ static void ExecInitCoerceToDomain(ExprEvalStep *scratch, CoerceToDomain *ctest, - PlanState *parent, ExprState *state, - Datum *resv, bool *resnull) + ExprState *state, Datum *resv, bool *resnull) { ExprEvalStep scratch2; DomainConstraintRef *constraint_ref; @@ -2587,7 +2660,7 @@ ExecInitCoerceToDomain(ExprEvalStep *scratch, CoerceToDomain *ctest, * if there's constraint failures there'll be errors, otherwise it's what * needs to be returned. */ - ExecInitExprRec(ctest->arg, parent, state, resv, resnull); + ExecInitExprRec(ctest->arg, state, resv, resnull); /* * Note: if the argument is of varlena type, it could be a R/W expanded @@ -2684,7 +2757,7 @@ ExecInitCoerceToDomain(ExprEvalStep *scratch, CoerceToDomain *ctest, state->innermost_domainnull = domainnull; /* evaluate check expression value */ - ExecInitExprRec(con->check_expr, parent, state, + ExecInitExprRec(con->check_expr, state, scratch->d.domaincheck.checkvalue, scratch->d.domaincheck.checknull); diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 6c4612dad4..0c3f66803f 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -335,6 +335,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) &&CASE_EEOP_BOOLTEST_IS_NOT_FALSE, &&CASE_EEOP_PARAM_EXEC, &&CASE_EEOP_PARAM_EXTERN, + &&CASE_EEOP_PARAM_CALLBACK, &&CASE_EEOP_CASE_TESTVAL, &&CASE_EEOP_MAKE_READONLY, &&CASE_EEOP_IOCOERCE, @@ -1047,6 +1048,13 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) EEO_NEXT(); } + EEO_CASE(EEOP_PARAM_CALLBACK) + { + /* allow an extension module to supply a PARAM_EXTERN value */ + op->d.cparam.paramfunc(state, op, econtext); + EEO_NEXT(); + } + EEO_CASE(EEOP_CASE_TESTVAL) { /* @@ -1967,11 +1975,14 @@ ExecEvalParamExtern(ExprState *state, ExprEvalStep *op, ExprContext *econtext) if (likely(paramInfo && paramId > 0 && paramId <= paramInfo->numParams)) { - ParamExternData *prm = ¶mInfo->params[paramId - 1]; + ParamExternData *prm; + ParamExternData prmdata; /* give hook a chance in case parameter is dynamic */ - if (!OidIsValid(prm->ptype) && paramInfo->paramFetch != NULL) - paramInfo->paramFetch(paramInfo, paramId); + if (paramInfo->paramFetch != NULL) + prm = paramInfo->paramFetch(paramInfo, paramId, false, &prmdata); + else + prm = ¶mInfo->params[paramId - 1]; if (likely(OidIsValid(prm->ptype))) { diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c index 3caa343723..527f7d810f 100644 --- a/src/backend/executor/functions.c +++ b/src/backend/executor/functions.c @@ -914,10 +914,11 @@ postquel_sub_params(SQLFunctionCachePtr fcache, /* we have static list of params, so no hooks needed */ paramLI->paramFetch = NULL; paramLI->paramFetchArg = NULL; + paramLI->paramCompile = NULL; + paramLI->paramCompileArg = NULL; paramLI->parserSetup = NULL; paramLI->parserSetupArg = NULL; paramLI->numParams = nargs; - paramLI->paramMask = NULL; fcache->paramLI = paramLI; } else diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index f3da2ddd08..977f317420 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -2259,10 +2259,11 @@ _SPI_convert_params(int nargs, Oid *argtypes, /* we have static list of params, so no hooks needed */ paramLI->paramFetch = NULL; paramLI->paramFetchArg = NULL; + paramLI->paramCompile = NULL; + paramLI->paramCompileArg = NULL; paramLI->parserSetup = NULL; paramLI->parserSetupArg = NULL; paramLI->numParams = nargs; - paramLI->paramMask = NULL; for (i = 0; i < nargs; i++) { diff --git a/src/backend/nodes/params.c b/src/backend/nodes/params.c index 51429af1e3..94acdf4e7b 100644 --- a/src/backend/nodes/params.c +++ b/src/backend/nodes/params.c @@ -48,32 +48,25 @@ copyParamList(ParamListInfo from) retval = (ParamListInfo) palloc(size); retval->paramFetch = NULL; retval->paramFetchArg = NULL; + retval->paramCompile = NULL; + retval->paramCompileArg = NULL; retval->parserSetup = NULL; retval->parserSetupArg = NULL; retval->numParams = from->numParams; - retval->paramMask = NULL; for (i = 0; i < from->numParams; i++) { - ParamExternData *oprm = &from->params[i]; + ParamExternData *oprm; ParamExternData *nprm = &retval->params[i]; + ParamExternData prmdata; int16 typLen; bool typByVal; - /* Ignore parameters we don't need, to save cycles and space. */ - if (from->paramMask != NULL && - !bms_is_member(i, from->paramMask)) - { - nprm->value = (Datum) 0; - nprm->isnull = true; - nprm->pflags = 0; - nprm->ptype = InvalidOid; - continue; - } - /* give hook a chance in case parameter is dynamic */ - if (!OidIsValid(oprm->ptype) && from->paramFetch != NULL) - from->paramFetch(from, i + 1); + if (from->paramFetch != NULL) + oprm = from->paramFetch(from, i + 1, false, &prmdata); + else + oprm = &from->params[i]; /* flat-copy the parameter info */ *nprm = *oprm; @@ -102,22 +95,19 @@ EstimateParamListSpace(ParamListInfo paramLI) for (i = 0; i < paramLI->numParams; i++) { - ParamExternData *prm = ¶mLI->params[i]; + ParamExternData *prm; + ParamExternData prmdata; Oid typeOid; int16 typLen; bool typByVal; - /* Ignore parameters we don't need, to save cycles and space. */ - if (paramLI->paramMask != NULL && - !bms_is_member(i, paramLI->paramMask)) - typeOid = InvalidOid; + /* give hook a chance in case parameter is dynamic */ + if (paramLI->paramFetch != NULL) + prm = paramLI->paramFetch(paramLI, i + 1, false, &prmdata); else - { - /* give hook a chance in case parameter is dynamic */ - if (!OidIsValid(prm->ptype) && paramLI->paramFetch != NULL) - paramLI->paramFetch(paramLI, i + 1); - typeOid = prm->ptype; - } + prm = ¶mLI->params[i]; + + typeOid = prm->ptype; sz = add_size(sz, sizeof(Oid)); /* space for type OID */ sz = add_size(sz, sizeof(uint16)); /* space for pflags */ @@ -171,22 +161,19 @@ SerializeParamList(ParamListInfo paramLI, char **start_address) /* Write each parameter in turn. */ for (i = 0; i < nparams; i++) { - ParamExternData *prm = ¶mLI->params[i]; + ParamExternData *prm; + ParamExternData prmdata; Oid typeOid; int16 typLen; bool typByVal; - /* Ignore parameters we don't need, to save cycles and space. */ - if (paramLI->paramMask != NULL && - !bms_is_member(i, paramLI->paramMask)) - typeOid = InvalidOid; + /* give hook a chance in case parameter is dynamic */ + if (paramLI->paramFetch != NULL) + prm = paramLI->paramFetch(paramLI, i + 1, false, &prmdata); else - { - /* give hook a chance in case parameter is dynamic */ - if (!OidIsValid(prm->ptype) && paramLI->paramFetch != NULL) - paramLI->paramFetch(paramLI, i + 1); - typeOid = prm->ptype; - } + prm = ¶mLI->params[i]; + + typeOid = prm->ptype; /* Write type OID. */ memcpy(*start_address, &typeOid, sizeof(Oid)); @@ -237,10 +224,11 @@ RestoreParamList(char **start_address) paramLI = (ParamListInfo) palloc(size); paramLI->paramFetch = NULL; paramLI->paramFetchArg = NULL; + paramLI->paramCompile = NULL; + paramLI->paramCompileArg = NULL; paramLI->parserSetup = NULL; paramLI->parserSetupArg = NULL; paramLI->numParams = nparams; - paramLI->paramMask = NULL; for (i = 0; i < nparams; i++) { diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 6a2d5ad760..9ca384db51 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -2513,14 +2513,27 @@ eval_const_expressions_mutator(Node *node, case T_Param: { Param *param = (Param *) node; + ParamListInfo paramLI = context->boundParams; /* Look to see if we've been given a value for this Param */ if (param->paramkind == PARAM_EXTERN && - context->boundParams != NULL && + paramLI != NULL && param->paramid > 0 && - param->paramid <= context->boundParams->numParams) + param->paramid <= paramLI->numParams) { - ParamExternData *prm = &context->boundParams->params[param->paramid - 1]; + ParamExternData *prm; + ParamExternData prmdata; + + /* + * Give hook a chance in case parameter is dynamic. Tell + * it that this fetch is speculative, so it should avoid + * erroring out if parameter is unavailable. + */ + if (paramLI->paramFetch != NULL) + prm = paramLI->paramFetch(paramLI, param->paramid, + true, &prmdata); + else + prm = ¶mLI->params[param->paramid - 1]; if (OidIsValid(prm->ptype)) { diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 1ae9ac2d57..1b24dddbce 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -1646,10 +1646,11 @@ exec_bind_message(StringInfo input_message) /* we have static list of params, so no hooks needed */ params->paramFetch = NULL; params->paramFetchArg = NULL; + params->paramCompile = NULL; + params->paramCompileArg = NULL; params->parserSetup = NULL; params->parserSetupArg = NULL; params->numParams = numParams; - params->paramMask = NULL; for (paramno = 0; paramno < numParams; paramno++) { @@ -2211,6 +2212,9 @@ errdetail_params(ParamListInfo params) MemoryContext oldcontext; int paramno; + /* This code doesn't support dynamic param lists */ + Assert(params->paramFetch == NULL); + /* Make sure any trash is generated in MessageContext */ oldcontext = MemoryContextSwitchTo(MessageContext); diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h index 5bbb63a9d8..080252fad6 100644 --- a/src/include/executor/execExpr.h +++ b/src/include/executor/execExpr.h @@ -16,7 +16,8 @@ #include "nodes/execnodes.h" -/* forward reference to avoid circularity */ +/* forward references to avoid circularity */ +struct ExprEvalStep; struct ArrayRefState; /* Bits in ExprState->flags (see also execnodes.h for public flag bits): */ @@ -25,6 +26,11 @@ struct ArrayRefState; /* jump-threading is in use */ #define EEO_FLAG_DIRECT_THREADED (1 << 2) +/* Typical API for out-of-line evaluation subroutines */ +typedef void (*ExecEvalSubroutine) (ExprState *state, + struct ExprEvalStep *op, + ExprContext *econtext); + /* * Discriminator for ExprEvalSteps. * @@ -131,6 +137,7 @@ typedef enum ExprEvalOp /* evaluate PARAM_EXEC/EXTERN parameters */ EEOP_PARAM_EXEC, EEOP_PARAM_EXTERN, + EEOP_PARAM_CALLBACK, /* return CaseTestExpr value */ EEOP_CASE_TESTVAL, @@ -331,6 +338,15 @@ typedef struct ExprEvalStep Oid paramtype; /* OID of parameter's datatype */ } param; + /* for EEOP_PARAM_CALLBACK */ + struct + { + ExecEvalSubroutine paramfunc; /* add-on evaluation subroutine */ + void *paramarg; /* private data for same */ + int paramid; /* numeric ID for parameter */ + Oid paramtype; /* OID of parameter's datatype */ + } cparam; + /* for EEOP_CASE_TESTVAL/DOMAIN_TESTVAL */ struct { @@ -598,8 +614,11 @@ typedef struct ArrayRefState } ArrayRefState; -extern void ExecReadyInterpretedExpr(ExprState *state); +/* functions in execExpr.c */ +extern void ExprEvalPushStep(ExprState *es, const ExprEvalStep *s); +/* functions in execExprInterp.c */ +extern void ExecReadyInterpretedExpr(ExprState *state); extern ExprEvalOp ExecEvalStepOp(ExprState *state, ExprEvalStep *op); /* diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index dea9216fd6..2cc74da0ba 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -247,6 +247,7 @@ ExecProcNode(PlanState *node) * prototypes from functions in execExpr.c */ extern ExprState *ExecInitExpr(Expr *node, PlanState *parent); +extern ExprState *ExecInitExprWithParams(Expr *node, ParamListInfo ext_params); extern ExprState *ExecInitQual(List *qual, PlanState *parent); extern ExprState *ExecInitCheck(List *qual, PlanState *parent); extern List *ExecInitExprList(List *nodes, PlanState *parent); diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 44d8c47d2c..c9a5279dc5 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -33,6 +33,13 @@ #include "storage/condition_variable.h" +struct PlanState; /* forward references in this file */ +struct ParallelHashJoinState; +struct ExprState; +struct ExprContext; +struct ExprEvalStep; /* avoid including execExpr.h everywhere */ + + /* ---------------- * ExprState node * @@ -40,12 +47,6 @@ * It contains instructions (in ->steps) to evaluate the expression. * ---------------- */ -struct ExprState; /* forward references in this file */ -struct ExprContext; -struct ExprEvalStep; /* avoid including execExpr.h everywhere */ - -struct ParallelHashJoinState; - typedef Datum (*ExprStateEvalFunc) (struct ExprState *expression, struct ExprContext *econtext, bool *isNull); @@ -87,12 +88,16 @@ typedef struct ExprState Expr *expr; /* - * XXX: following only needed during "compilation", could be thrown away. + * XXX: following fields only needed during "compilation" (ExecInitExpr); + * could be thrown away afterwards. */ int steps_len; /* number of steps currently */ int steps_alloc; /* allocated length of steps array */ + struct PlanState *parent; /* parent PlanState node, if any */ + ParamListInfo ext_params; /* for compiling PARAM_EXTERN nodes */ + Datum *innermost_caseval; bool *innermost_casenull; @@ -827,8 +832,6 @@ typedef struct DomainConstraintState * ---------------------------------------------------------------- */ -struct PlanState; - /* ---------------- * ExecProcNodeMtd * diff --git a/src/include/nodes/params.h b/src/include/nodes/params.h index 55219dab6e..b198db5ee4 100644 --- a/src/include/nodes/params.h +++ b/src/include/nodes/params.h @@ -16,16 +16,23 @@ /* Forward declarations, to avoid including other headers */ struct Bitmapset; +struct ExprState; +struct Param; struct ParseState; -/* ---------------- +/* * ParamListInfo * - * ParamListInfo arrays are used to pass parameters into the executor - * for parameterized plans. Each entry in the array defines the value - * to be substituted for a PARAM_EXTERN parameter. The "paramid" - * of a PARAM_EXTERN Param can range from 1 to numParams. + * ParamListInfo structures are used to pass parameters into the executor + * for parameterized plans. We support two basic approaches to supplying + * parameter values, the "static" way and the "dynamic" way. + * + * In the static approach, per-parameter data is stored in an array of + * ParamExternData structs appended to the ParamListInfo struct. + * Each entry in the array defines the value to be substituted for a + * PARAM_EXTERN parameter. The "paramid" of a PARAM_EXTERN Param + * can range from 1 to numParams. * * Although parameter numbers are normally consecutive, we allow * ptype == InvalidOid to signal an unused array entry. @@ -35,18 +42,47 @@ struct ParseState; * as a constant (i.e., generate a plan that works only for this value * of the parameter). * - * There are two hook functions that can be associated with a ParamListInfo - * array to support dynamic parameter handling. First, if paramFetch - * isn't null and the executor requires a value for an invalid parameter - * (one with ptype == InvalidOid), the paramFetch hook is called to give - * it a chance to fill in the parameter value. Second, a parserSetup - * hook can be supplied to re-instantiate the original parsing hooks if - * a query needs to be re-parsed/planned (as a substitute for supposing - * that the current ptype values represent a fixed set of parameter types). - + * In the dynamic approach, all access to parameter values is done through + * hook functions found in the ParamListInfo struct. In this case, + * the ParamExternData array is typically unused and not allocated; + * but the legal range of paramid is still 1 to numParams. + * * Although the data structure is really an array, not a list, we keep * the old typedef name to avoid unnecessary code changes. - * ---------------- + * + * There are 3 hook functions that can be associated with a ParamListInfo + * structure: + * + * If paramFetch isn't null, it is called to fetch the ParamExternData + * for a particular param ID, rather than accessing the relevant element + * of the ParamExternData array. This supports the case where the array + * isn't there at all, as well as cases where the data in the array + * might be obsolete or lazily evaluated. paramFetch must return the + * address of a ParamExternData struct describing the specified param ID; + * the convention above about ptype == InvalidOid signaling an invalid + * param ID still applies. The returned struct can either be placed in + * the "workspace" supplied by the caller, or it can be in storage + * controlled by the paramFetch hook if that's more convenient. + * (In either case, the struct is not expected to be long-lived.) + * If "speculative" is true, the paramFetch hook should not risk errors + * in trying to fetch the parameter value, and should report an invalid + * parameter instead. + * + * If paramCompile isn't null, then it controls what execExpr.c compiles + * for PARAM_EXTERN Param nodes --- typically, this hook would emit a + * EEOP_PARAM_CALLBACK step. This allows unnecessary work to be + * optimized away in compiled expressions. + * + * If parserSetup isn't null, then it is called to re-instantiate the + * original parsing hooks when a query needs to be re-parsed/planned. + * This is especially useful if the types of parameters might change + * from time to time, since it can replace the need to supply a fixed + * list of parameter types to the parser. + * + * Notice that the paramFetch and paramCompile hooks are actually passed + * the ParamListInfo struct's address; they can therefore access all + * three of the "arg" fields, and the distinction between paramFetchArg + * and paramCompileArg is rather arbitrary. */ #define PARAM_FLAG_CONST 0x0001 /* parameter is constant */ @@ -61,7 +97,13 @@ typedef struct ParamExternData typedef struct ParamListInfoData *ParamListInfo; -typedef void (*ParamFetchHook) (ParamListInfo params, int paramid); +typedef ParamExternData *(*ParamFetchHook) (ParamListInfo params, + int paramid, bool speculative, + ParamExternData *workspace); + +typedef void (*ParamCompileHook) (ParamListInfo params, struct Param *param, + struct ExprState *state, + Datum *resv, bool *resnull); typedef void (*ParserSetupHook) (struct ParseState *pstate, void *arg); @@ -69,10 +111,16 @@ typedef struct ParamListInfoData { ParamFetchHook paramFetch; /* parameter fetch hook */ void *paramFetchArg; + ParamCompileHook paramCompile; /* parameter compile hook */ + void *paramCompileArg; ParserSetupHook parserSetup; /* parser setup hook */ void *parserSetupArg; - int numParams; /* number of ParamExternDatas following */ - struct Bitmapset *paramMask; /* if non-NULL, can ignore omitted params */ + int numParams; /* nominal/maximum # of Params represented */ + + /* + * params[] may be of length zero if paramFetch is supplied; otherwise it + * must be of length numParams. + */ ParamExternData params[FLEXIBLE_ARRAY_MEMBER]; } ParamListInfoData; diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c index 2d7844bd9d..e9eab17338 100644 --- a/src/pl/plpgsql/src/pl_comp.c +++ b/src/pl/plpgsql/src/pl_comp.c @@ -2350,35 +2350,17 @@ plpgsql_adddatum(PLpgSQL_datum *new) /* ---------- * plpgsql_finish_datums Copy completed datum info into function struct. - * - * This is also responsible for building resettable_datums, a bitmapset - * of the dnos of all ROW, REC, and RECFIELD datums in the function. * ---------- */ static void plpgsql_finish_datums(PLpgSQL_function *function) { - Bitmapset *resettable_datums = NULL; int i; function->ndatums = plpgsql_nDatums; function->datums = palloc(sizeof(PLpgSQL_datum *) * plpgsql_nDatums); for (i = 0; i < plpgsql_nDatums; i++) - { function->datums[i] = plpgsql_Datums[i]; - switch (function->datums[i]->dtype) - { - case PLPGSQL_DTYPE_ROW: - case PLPGSQL_DTYPE_REC: - case PLPGSQL_DTYPE_RECFIELD: - resettable_datums = bms_add_member(resettable_datums, i); - break; - - default: - break; - } - } - function->resettable_datums = resettable_datums; } diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index fa4d573e50..dd575e73f4 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -22,6 +22,7 @@ #include "access/tupconvert.h" #include "catalog/pg_proc.h" #include "catalog/pg_type.h" +#include "executor/execExpr.h" #include "executor/spi.h" #include "funcapi.h" #include "miscadmin.h" @@ -268,9 +269,18 @@ static int exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt, Portal portal, bool prefetch_ok); static ParamListInfo setup_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr); -static ParamListInfo setup_unshared_param_list(PLpgSQL_execstate *estate, - PLpgSQL_expr *expr); -static void plpgsql_param_fetch(ParamListInfo params, int paramid); +static ParamExternData *plpgsql_param_fetch(ParamListInfo params, + int paramid, bool speculative, + ParamExternData *prm); +static void plpgsql_param_compile(ParamListInfo params, Param *param, + ExprState *state, + Datum *resv, bool *resnull); +static void plpgsql_param_eval_var(ExprState *state, ExprEvalStep *op, + ExprContext *econtext); +static void plpgsql_param_eval_var_ro(ExprState *state, ExprEvalStep *op, + ExprContext *econtext); +static void plpgsql_param_eval_non_var(ExprState *state, ExprEvalStep *op, + ExprContext *econtext); static void exec_move_row(PLpgSQL_execstate *estate, PLpgSQL_variable *target, HeapTuple tup, TupleDesc tupdesc); @@ -2346,9 +2356,9 @@ exec_stmt_forc(PLpgSQL_execstate *estate, PLpgSQL_stmt_forc *stmt) exec_prepare_plan(estate, query, curvar->cursor_options); /* - * Set up short-lived ParamListInfo + * Set up ParamListInfo for this query */ - paramLI = setup_unshared_param_list(estate, query); + paramLI = setup_param_list(estate, query); /* * Open the cursor (the paramlist will get copied into the portal) @@ -3440,17 +3450,16 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate, estate->datums = palloc(sizeof(PLpgSQL_datum *) * estate->ndatums); /* caller is expected to fill the datums array */ - /* initialize ParamListInfo with one entry per datum, all invalid */ + /* initialize our ParamListInfo with appropriate hook functions */ estate->paramLI = (ParamListInfo) - palloc0(offsetof(ParamListInfoData, params) + - estate->ndatums * sizeof(ParamExternData)); + palloc(offsetof(ParamListInfoData, params)); estate->paramLI->paramFetch = plpgsql_param_fetch; estate->paramLI->paramFetchArg = (void *) estate; + estate->paramLI->paramCompile = plpgsql_param_compile; + estate->paramLI->paramCompileArg = NULL; /* not needed */ estate->paramLI->parserSetup = (ParserSetupHook) plpgsql_parser_setup; estate->paramLI->parserSetupArg = NULL; /* filled during use */ estate->paramLI->numParams = estate->ndatums; - estate->paramLI->paramMask = NULL; - estate->params_dirty = false; /* set up for use of appropriate simple-expression EState and cast hash */ if (simple_eval_estate) @@ -4169,12 +4178,12 @@ exec_stmt_open(PLpgSQL_execstate *estate, PLpgSQL_stmt_open *stmt) } /* - * Set up short-lived ParamListInfo + * Set up ParamListInfo for this query */ - paramLI = setup_unshared_param_list(estate, query); + paramLI = setup_param_list(estate, query); /* - * Open the cursor + * Open the cursor (the paramlist will get copied into the portal) */ portal = SPI_cursor_open_with_paramlist(curname, query->plan, paramLI, @@ -5268,15 +5277,15 @@ exec_run_select(PLpgSQL_execstate *estate, portalP == NULL ? CURSOR_OPT_PARALLEL_OK : 0); /* - * If a portal was requested, put the query into the portal + * Set up ParamListInfo to pass to executor + */ + paramLI = setup_param_list(estate, expr); + + /* + * If a portal was requested, put the query and paramlist into the portal */ if (portalP != NULL) { - /* - * Set up short-lived ParamListInfo - */ - paramLI = setup_unshared_param_list(estate, expr); - *portalP = SPI_cursor_open_with_paramlist(NULL, expr->plan, paramLI, estate->readonly_func); @@ -5287,11 +5296,6 @@ exec_run_select(PLpgSQL_execstate *estate, return SPI_OK_CURSOR; } - /* - * Set up ParamListInfo to pass to executor - */ - paramLI = setup_param_list(estate, expr); - /* * Execute the query */ @@ -5504,7 +5508,6 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, ExprContext *econtext = estate->eval_econtext; LocalTransactionId curlxid = MyProc->lxid; CachedPlan *cplan; - ParamListInfo paramLI; void *save_setup_arg; MemoryContext oldcontext; @@ -5551,6 +5554,14 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, *rettype = expr->expr_simple_type; *rettypmod = expr->expr_simple_typmod; + /* + * Set up ParamListInfo to pass to executor. For safety, save and restore + * estate->paramLI->parserSetupArg around our use of the param list. + */ + save_setup_arg = estate->paramLI->parserSetupArg; + + econtext->ecxt_param_list_info = setup_param_list(estate, expr); + /* * Prepare the expression for execution, if it's not been done already in * the current transaction. (This will be forced to happen if we called @@ -5559,7 +5570,9 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, if (expr->expr_simple_lxid != curlxid) { oldcontext = MemoryContextSwitchTo(estate->simple_eval_estate->es_query_cxt); - expr->expr_simple_state = ExecInitExpr(expr->expr_simple_expr, NULL); + expr->expr_simple_state = + ExecInitExprWithParams(expr->expr_simple_expr, + econtext->ecxt_param_list_info); expr->expr_simple_in_use = false; expr->expr_simple_lxid = curlxid; MemoryContextSwitchTo(oldcontext); @@ -5578,21 +5591,6 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, PushActiveSnapshot(GetTransactionSnapshot()); } - /* - * Set up ParamListInfo to pass to executor. We need an unshared list if - * it's going to include any R/W expanded-object pointer. For safety, - * save and restore estate->paramLI->parserSetupArg around our use of the - * param list. - */ - save_setup_arg = estate->paramLI->parserSetupArg; - - if (expr->rwparam >= 0) - paramLI = setup_unshared_param_list(estate, expr); - else - paramLI = setup_param_list(estate, expr); - - econtext->ecxt_param_list_info = paramLI; - /* * Mark expression as busy for the duration of the ExecEvalExpr call. */ @@ -5632,35 +5630,17 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, /* * Create a ParamListInfo to pass to SPI * - * We share a single ParamListInfo array across all SPI calls made from this - * estate, except calls creating cursors, which use setup_unshared_param_list - * (see its comments for reasons why), and calls that pass a R/W expanded - * object pointer. A shared array is generally OK since any given slot in - * the array would need to contain the same current datum value no matter - * which query or expression we're evaluating; but of course that doesn't - * hold when a specific variable is being passed as a R/W pointer, because - * other expressions in the same function probably don't want to do that. - * - * Note that paramLI->parserSetupArg points to the specific PLpgSQL_expr - * being evaluated. This is not an issue for statement-level callers, but - * lower-level callers must save and restore estate->paramLI->parserSetupArg - * just in case there's an active evaluation at an outer call level. + * We use a single ParamListInfo struct for all SPI calls made from this + * estate; it contains no per-param data, just hook functions, so it's + * effectively read-only for SPI. * - * The general plan for passing parameters to SPI is that plain VAR datums - * always have valid images in the shared param list. This is ensured by - * assign_simple_var(), which also marks those params as PARAM_FLAG_CONST, - * allowing the planner to use those values in custom plans. However, non-VAR - * datums cannot conveniently be managed that way. For one thing, they could - * throw errors (for example "no such record field") and we do not want that - * to happen in a part of the expression that might never be evaluated at - * runtime. For another thing, exec_eval_datum() may return short-lived - * values stored in the estate's eval_mcontext, which will not necessarily - * survive to the next SPI operation. And for a third thing, ROW - * and RECFIELD datums' values depend on other datums, and we don't have a - * cheap way to track that. Therefore, param slots for non-VAR datum types - * are always reset here and then filled on-demand by plpgsql_param_fetch(). - * We can save a few cycles by not bothering with the reset loop unless at - * least one such param has actually been filled by plpgsql_param_fetch(). + * An exception from pure read-only-ness is that the parserSetupArg points + * to the specific PLpgSQL_expr being evaluated. This is not an issue for + * statement-level callers, but lower-level callers must save and restore + * estate->paramLI->parserSetupArg just in case there's an active evaluation + * at an outer call level. (A plausible alternative design would be to + * create a ParamListInfo struct for each PLpgSQL_expr, but for the moment + * that seems like a waste of memory.) */ static ParamListInfo setup_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) @@ -5673,11 +5653,6 @@ setup_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) */ Assert(expr->plan != NULL); - /* - * Expressions with R/W parameters can't use the shared param list. - */ - Assert(expr->rwparam == -1); - /* * We only need a ParamListInfo if the expression has parameters. In * principle we should test with bms_is_empty(), but we use a not-null @@ -5689,25 +5664,6 @@ setup_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) /* Use the common ParamListInfo */ paramLI = estate->paramLI; - /* - * If any resettable parameters have been passed to the executor since - * last time, we need to reset those param slots to "invalid", for the - * reasons mentioned in the comment above. - */ - if (estate->params_dirty) - { - Bitmapset *resettable_datums = estate->func->resettable_datums; - int dno = -1; - - while ((dno = bms_next_member(resettable_datums, dno)) >= 0) - { - ParamExternData *prm = ¶mLI->params[dno]; - - prm->ptype = InvalidOid; - } - estate->params_dirty = false; - } - /* * Set up link to active expr where the hook functions can find it. * Callers must save and restore parserSetupArg if there is any chance @@ -5715,12 +5671,6 @@ setup_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) */ paramLI->parserSetupArg = (void *) expr; - /* - * Allow parameters that aren't needed by this expression to be - * ignored. - */ - paramLI->paramMask = expr->paramnos; - /* * Also make sure this is set before parser hooks need it. There is * no need to save and restore, since the value is always correct once @@ -5741,115 +5691,25 @@ setup_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) } /* - * Create an unshared, short-lived ParamListInfo to pass to SPI - * - * When creating a cursor, we do not use the shared ParamListInfo array - * but create a short-lived one that will contain only params actually - * referenced by the query. The reason for this is that copyParamList() will - * be used to copy the parameters into cursor-lifespan storage, and we don't - * want it to copy anything that's not used by the specific cursor; that - * could result in uselessly copying some large values. - * - * We also use this for expressions that are passing a R/W object pointer - * to some trusted function. We don't want the R/W pointer to get into the - * shared param list, where it could get passed to some less-trusted function. + * plpgsql_param_fetch paramFetch callback for dynamic parameter fetch * - * The result, if not NULL, is in the estate's eval_mcontext. + * We always use the caller's workspace to construct the returned struct. * - * XXX. Could we use ParamListInfo's new paramMask to avoid creating unshared - * parameter lists? - */ -static ParamListInfo -setup_unshared_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) -{ - ParamListInfo paramLI; - - /* - * We must have created the SPIPlan already (hence, query text has been - * parsed/analyzed at least once); else we cannot rely on expr->paramnos. - */ - Assert(expr->plan != NULL); - - /* - * We only need a ParamListInfo if the expression has parameters. In - * principle we should test with bms_is_empty(), but we use a not-null - * test because it's faster. In current usage bits are never removed from - * expr->paramnos, only added, so this test is correct anyway. - */ - if (expr->paramnos) - { - int dno; - - /* initialize ParamListInfo with one entry per datum, all invalid */ - paramLI = (ParamListInfo) - eval_mcontext_alloc0(estate, - offsetof(ParamListInfoData, params) + - estate->ndatums * sizeof(ParamExternData)); - paramLI->paramFetch = plpgsql_param_fetch; - paramLI->paramFetchArg = (void *) estate; - paramLI->parserSetup = (ParserSetupHook) plpgsql_parser_setup; - paramLI->parserSetupArg = (void *) expr; - paramLI->numParams = estate->ndatums; - paramLI->paramMask = NULL; - - /* - * Instantiate values for "safe" parameters of the expression. We - * could skip this and leave them to be filled by plpgsql_param_fetch; - * but then the values would not be available for query planning, - * since the planner doesn't call the paramFetch hook. - */ - dno = -1; - while ((dno = bms_next_member(expr->paramnos, dno)) >= 0) - { - PLpgSQL_datum *datum = estate->datums[dno]; - - if (datum->dtype == PLPGSQL_DTYPE_VAR) - { - PLpgSQL_var *var = (PLpgSQL_var *) datum; - ParamExternData *prm = ¶mLI->params[dno]; - - if (dno == expr->rwparam) - prm->value = var->value; - else - prm->value = MakeExpandedObjectReadOnly(var->value, - var->isnull, - var->datatype->typlen); - prm->isnull = var->isnull; - prm->pflags = PARAM_FLAG_CONST; - prm->ptype = var->datatype->typoid; - } - } - - /* - * Also make sure this is set before parser hooks need it. There is - * no need to save and restore, since the value is always correct once - * set. (Should be set already, but let's be sure.) - */ - expr->func = estate->func; - } - else - { - /* - * Expression requires no parameters. Be sure we represent this case - * as a NULL ParamListInfo, so that plancache.c knows there is no - * point in a custom plan. - */ - paramLI = NULL; - } - return paramLI; -} - -/* - * plpgsql_param_fetch paramFetch callback for dynamic parameter fetch + * Note: this is no longer used during query execution. It is used during + * planning (with speculative == true) and when the ParamListInfo we supply + * to the executor is copied into a cursor portal or transferred to a + * parallel child process. */ -static void -plpgsql_param_fetch(ParamListInfo params, int paramid) +static ParamExternData * +plpgsql_param_fetch(ParamListInfo params, + int paramid, bool speculative, + ParamExternData *prm) { int dno; PLpgSQL_execstate *estate; PLpgSQL_expr *expr; PLpgSQL_datum *datum; - ParamExternData *prm; + bool ok = true; int32 prmtypmod; /* paramid's are 1-based, but dnos are 0-based */ @@ -5866,35 +5726,74 @@ plpgsql_param_fetch(ParamListInfo params, int paramid) /* * Since copyParamList() or SerializeParamList() will try to materialize - * every single parameter slot, it's important to do nothing when asked - * for a datum that's not supposed to be used by this SQL expression. - * Otherwise we risk failures in exec_eval_datum(), or copying a lot more - * data than necessary. + * every single parameter slot, it's important to return a dummy param + * when asked for a datum that's not supposed to be used by this SQL + * expression. Otherwise we risk failures in exec_eval_datum(), or + * copying a lot more data than necessary. */ if (!bms_is_member(dno, expr->paramnos)) - return; + ok = false; - if (params == estate->paramLI) + /* + * If the access is speculative, we prefer to return no data rather than + * to fail in exec_eval_datum(). Check the likely failure cases. + */ + else if (speculative) { - /* - * We need to mark the shared params array dirty if we're about to - * evaluate a resettable datum. - */ switch (datum->dtype) { + case PLPGSQL_DTYPE_VAR: + /* always safe */ + break; + case PLPGSQL_DTYPE_ROW: + /* should be safe in all interesting cases */ + break; + case PLPGSQL_DTYPE_REC: + { + PLpgSQL_rec *rec = (PLpgSQL_rec *) datum; + + if (!HeapTupleIsValid(rec->tup)) + ok = false; + break; + } + case PLPGSQL_DTYPE_RECFIELD: - estate->params_dirty = true; - break; + { + PLpgSQL_recfield *recfield = (PLpgSQL_recfield *) datum; + PLpgSQL_rec *rec; + int fno; + + rec = (PLpgSQL_rec *) (estate->datums[recfield->recparentno]); + if (!HeapTupleIsValid(rec->tup)) + ok = false; + else + { + fno = SPI_fnumber(rec->tupdesc, recfield->fieldname); + if (fno == SPI_ERROR_NOATTRIBUTE) + ok = false; + } + break; + } default: + ok = false; break; } } - /* OK, evaluate the value and store into the appropriate paramlist slot */ - prm = ¶ms->params[dno]; + /* Return "no such parameter" if not ok */ + if (!ok) + { + prm->value = (Datum) 0; + prm->isnull = true; + prm->pflags = 0; + prm->ptype = InvalidOid; + return prm; + } + + /* OK, evaluate the value and store into the return struct */ exec_eval_datum(estate, datum, &prm->ptype, &prmtypmod, &prm->value, &prm->isnull); @@ -5909,6 +5808,174 @@ plpgsql_param_fetch(ParamListInfo params, int paramid) prm->value = MakeExpandedObjectReadOnly(prm->value, prm->isnull, ((PLpgSQL_var *) datum)->datatype->typlen); + + return prm; +} + +/* + * plpgsql_param_compile paramCompile callback for plpgsql parameters + */ +static void +plpgsql_param_compile(ParamListInfo params, Param *param, + ExprState *state, + Datum *resv, bool *resnull) +{ + PLpgSQL_execstate *estate; + PLpgSQL_expr *expr; + int dno; + PLpgSQL_datum *datum; + ExprEvalStep scratch; + + /* fetch back the hook data */ + estate = (PLpgSQL_execstate *) params->paramFetchArg; + expr = (PLpgSQL_expr *) params->parserSetupArg; + + /* paramid's are 1-based, but dnos are 0-based */ + dno = param->paramid - 1; + Assert(dno >= 0 && dno < estate->ndatums); + + /* now we can access the target datum */ + datum = estate->datums[dno]; + + scratch.opcode = EEOP_PARAM_CALLBACK; + scratch.resvalue = resv; + scratch.resnull = resnull; + + /* Select appropriate eval function */ + if (datum->dtype == PLPGSQL_DTYPE_VAR) + { + if (dno != expr->rwparam && + ((PLpgSQL_var *) datum)->datatype->typlen == -1) + scratch.d.cparam.paramfunc = plpgsql_param_eval_var_ro; + else + scratch.d.cparam.paramfunc = plpgsql_param_eval_var; + } + else + scratch.d.cparam.paramfunc = plpgsql_param_eval_non_var; + + /* + * Note: it's tempting to use paramarg to store the estate pointer and + * thereby save an indirection or two in the eval functions. But that + * doesn't work because the compiled expression might be used with + * different estates for the same PL/pgSQL function. + */ + scratch.d.cparam.paramarg = NULL; + scratch.d.cparam.paramid = param->paramid; + scratch.d.cparam.paramtype = param->paramtype; + ExprEvalPushStep(state, &scratch); +} + +/* + * plpgsql_param_eval_var evaluation of EEOP_PARAM_CALLBACK step + * + * This is specialized to the case of DTYPE_VAR variables for which + * we do not need to invoke MakeExpandedObjectReadOnly. + */ +static void +plpgsql_param_eval_var(ExprState *state, ExprEvalStep *op, + ExprContext *econtext) +{ + ParamListInfo params; + PLpgSQL_execstate *estate; + int dno = op->d.cparam.paramid - 1; + PLpgSQL_var *var; + + /* fetch back the hook data */ + params = econtext->ecxt_param_list_info; + estate = (PLpgSQL_execstate *) params->paramFetchArg; + Assert(dno >= 0 && dno < estate->ndatums); + + /* now we can access the target datum */ + var = (PLpgSQL_var *) estate->datums[dno]; + Assert(var->dtype == PLPGSQL_DTYPE_VAR); + + /* inlined version of exec_eval_datum() */ + *op->resvalue = var->value; + *op->resnull = var->isnull; + + /* safety check -- an assertion should be sufficient */ + Assert(var->datatype->typoid == op->d.cparam.paramtype); +} + +/* + * plpgsql_param_eval_var_ro evaluation of EEOP_PARAM_CALLBACK step + * + * This is specialized to the case of DTYPE_VAR variables for which + * we need to invoke MakeExpandedObjectReadOnly. + */ +static void +plpgsql_param_eval_var_ro(ExprState *state, ExprEvalStep *op, + ExprContext *econtext) +{ + ParamListInfo params; + PLpgSQL_execstate *estate; + int dno = op->d.cparam.paramid - 1; + PLpgSQL_var *var; + + /* fetch back the hook data */ + params = econtext->ecxt_param_list_info; + estate = (PLpgSQL_execstate *) params->paramFetchArg; + Assert(dno >= 0 && dno < estate->ndatums); + + /* now we can access the target datum */ + var = (PLpgSQL_var *) estate->datums[dno]; + Assert(var->dtype == PLPGSQL_DTYPE_VAR); + + /* + * Inlined version of exec_eval_datum() ... and while we're at it, force + * expanded datums to read-only. + */ + *op->resvalue = MakeExpandedObjectReadOnly(var->value, + var->isnull, + -1); + *op->resnull = var->isnull; + + /* safety check -- an assertion should be sufficient */ + Assert(var->datatype->typoid == op->d.cparam.paramtype); +} + +/* + * plpgsql_param_eval_non_var evaluation of EEOP_PARAM_CALLBACK step + * + * This handles all variable types except DTYPE_VAR. + */ +static void +plpgsql_param_eval_non_var(ExprState *state, ExprEvalStep *op, + ExprContext *econtext) +{ + ParamListInfo params; + PLpgSQL_execstate *estate; + int dno = op->d.cparam.paramid - 1; + PLpgSQL_datum *datum; + Oid datumtype; + int32 datumtypmod; + + /* fetch back the hook data */ + params = econtext->ecxt_param_list_info; + estate = (PLpgSQL_execstate *) params->paramFetchArg; + Assert(dno >= 0 && dno < estate->ndatums); + + /* now we can access the target datum */ + datum = estate->datums[dno]; + Assert(datum->dtype != PLPGSQL_DTYPE_VAR); + + exec_eval_datum(estate, datum, + &datumtype, &datumtypmod, + op->resvalue, op->resnull); + + /* safety check -- needed for, eg, record fields */ + if (unlikely(datumtype != op->d.cparam.paramtype)) + ereport(ERROR, + (errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("type of parameter %d (%s) does not match that when preparing the plan (%s)", + op->d.cparam.paramid, + format_type_be(datumtype), + format_type_be(op->d.cparam.paramtype)))); + + /* + * Currently, if the dtype isn't VAR, the value couldn't be a read/write + * expanded datum. + */ } @@ -6875,14 +6942,12 @@ plpgsql_subxact_cb(SubXactEvent event, SubTransactionId mySubid, * assign_simple_var --- assign a new value to any VAR datum. * * This should be the only mechanism for assignment to simple variables, - * lest we forget to update the paramLI image. + * lest we do the release of the old value incorrectly. */ static void assign_simple_var(PLpgSQL_execstate *estate, PLpgSQL_var *var, Datum newvalue, bool isnull, bool freeable) { - ParamExternData *prm; - Assert(var->dtype == PLPGSQL_DTYPE_VAR); /* Free the old value if needed */ if (var->freeval) @@ -6898,15 +6963,6 @@ assign_simple_var(PLpgSQL_execstate *estate, PLpgSQL_var *var, var->value = newvalue; var->isnull = isnull; var->freeval = freeable; - /* And update the image in the common parameter list */ - prm = &estate->paramLI->params[var->dno]; - prm->value = MakeExpandedObjectReadOnly(newvalue, - isnull, - var->datatype->typlen); - prm->isnull = isnull; - /* these might be set already, but let's be sure */ - prm->pflags = PARAM_FLAG_CONST; - prm->ptype = var->datatype->typoid; } /* diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h index 39bd82acd1..43d7d7db36 100644 --- a/src/pl/plpgsql/src/plpgsql.h +++ b/src/pl/plpgsql/src/plpgsql.h @@ -857,7 +857,6 @@ typedef struct PLpgSQL_function /* the datums representing the function's local variables */ int ndatums; PLpgSQL_datum **datums; - Bitmapset *resettable_datums; /* dnos of non-simple vars */ /* function body parsetree */ PLpgSQL_stmt_block *action; @@ -899,9 +898,13 @@ typedef struct PLpgSQL_execstate int ndatums; PLpgSQL_datum **datums; - /* we pass datums[i] to the executor, when needed, in paramLI->params[i] */ + /* + * paramLI is what we use to pass local variable values to the executor. + * It does not have a ParamExternData array; we just dynamically + * instantiate parameter data as needed. By convention, PARAM_EXTERN + * Params have paramid equal to the dno of the referenced local variable. + */ ParamListInfo paramLI; - bool params_dirty; /* T if any resettable datum has been passed */ /* EState to use for "simple" expression evaluation */ EState *simple_eval_estate; -- 2.40.0