diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 812ead95bc..23f2dbfef3 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -133,6 +133,7 @@ ExecInitExpr(Expr *node, PlanState *parent) /* Initialize ExprState with empty step list */ state = makeNode(ExprState); state->expr = node; + state->last_param = -1; state->parent = parent; state->ext_params = NULL; @@ -170,6 +171,7 @@ ExecInitExprWithParams(Expr *node, ParamListInfo ext_params) /* Initialize ExprState with empty step list */ state = makeNode(ExprState); state->expr = node; + state->last_param = -1; state->parent = NULL; state->ext_params = ext_params; @@ -222,6 +224,7 @@ ExecInitQual(List *qual, PlanState *parent) state = makeNode(ExprState); state->expr = (Expr *) qual; + state->last_param = -1; state->parent = parent; state->ext_params = NULL; @@ -367,6 +370,7 @@ ExecBuildProjectionInfo(List *targetList, projInfo->pi_state.type = T_ExprState; state = &projInfo->pi_state; state->expr = (Expr *) targetList; + state->last_param = -1; state->parent = parent; state->ext_params = NULL; @@ -538,6 +542,7 @@ ExecBuildUpdateProjection(List *targetList, state->expr = (Expr *) targetList; else state->expr = NULL; /* not used */ + state->last_param = -1; state->parent = parent; state->ext_params = NULL; @@ -874,18 +879,46 @@ ExecCheck(ExprState *state, ExprContext *econtext) /* * Prepare a compiled expression for execution. This has to be called for * every ExprState before it can be executed. - * - * NB: While this currently only calls ExecReadyInterpretedExpr(), - * this will likely get extended to further expression evaluation methods. - * Therefore this should be used instead of directly calling - * ExecReadyInterpretedExpr(). */ static void ExecReadyExpr(ExprState *state) { + /* + * We need one last fixup in the steps list: if there are any subplan + * nodes with private output arrays, their associated PARAM_EXEC params + * need to be given pointers to those arrays. + */ + if (state->parent != NULL) + { + ListCell *lc; + + foreach(lc, state->parent->subPlan) + { + SubPlanState *sstate = lfirst_node(SubPlanState, lc); + List *setParam; + int idx; + + if (sstate->private_exec_vals == NULL) + continue; /* nothing to do here */ + + setParam = sstate->subplan->setParam; + idx = state->last_param; + while (idx >= 0) + { + ExprEvalStep *as = &state->steps[idx]; + + if (list_member_int(setParam, as->d.param.paramid)) + as->d.param.private_exec_vals = sstate->private_exec_vals; + idx = as->d.param.prev_param; + } + } + } + + /* Now see if JIT wants to compile it */ if (jit_compile_expr(state)) return; + /* Nope, use the default interpreted-expression implementation */ ExecReadyInterpretedExpr(state); } @@ -993,7 +1026,12 @@ ExecInitExprRec(Expr *node, ExprState *state, scratch.opcode = EEOP_PARAM_EXEC; scratch.d.param.paramid = param->paramid; scratch.d.param.paramtype = param->paramtype; + /* For now, assume it's not a MULTIEXPR output Param */ + scratch.d.param.private_exec_vals = NULL; + /* ...but link it into a list so we can fix it later */ + scratch.d.param.prev_param = state->last_param; ExprEvalPushStep(state, &scratch); + state->last_param = state->steps_len - 1; break; case PARAM_EXTERN: @@ -1622,6 +1660,7 @@ ExecInitExprRec(Expr *node, ExprState *state, */ elemstate = makeNode(ExprState); elemstate->expr = acoerce->elemexpr; + elemstate->last_param = -1; elemstate->parent = state->parent; elemstate->ext_params = state->ext_params; @@ -3270,6 +3309,7 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase, LastAttnumInfo deform = {0, 0, 0}; state->expr = (Expr *) aggstate; + state->last_param = -1; state->parent = parent; scratch.resvalue = &state->resvalue; @@ -3739,6 +3779,7 @@ ExecBuildGroupingEqual(TupleDesc ldesc, TupleDesc rdesc, state->expr = NULL; state->flags = EEO_FLAG_IS_QUAL; + state->last_param = -1; state->parent = parent; scratch.resvalue = &state->resvalue; @@ -3889,6 +3930,7 @@ ExecBuildParamSetEqual(TupleDesc desc, state->expr = NULL; state->flags = EEO_FLAG_IS_QUAL; + state->last_param = -1; state->parent = parent; scratch.resvalue = &state->resvalue; diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 19351fe34b..268c7fde70 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -2413,15 +2413,19 @@ ExecEvalFuncExprStrictFusage(ExprState *state, ExprEvalStep *op, /* * Evaluate a PARAM_EXEC parameter. * - * PARAM_EXEC params (internal executor parameters) are stored in the - * ecxt_param_exec_vals array, and can be accessed by array index. + * Most PARAM_EXEC params (internal executor parameters) are stored in the + * ecxt_param_exec_vals array, but ones representing MULTIEXPR subplan outputs + * may be in a private array. In either case the paramid is the array index. */ void ExecEvalParamExec(ExprState *state, ExprEvalStep *op, ExprContext *econtext) { + ParamExecData *param_array = op->d.param.private_exec_vals; ParamExecData *prm; - prm = &(econtext->ecxt_param_exec_vals[op->d.param.paramid]); + if (param_array == NULL) + param_array = econtext->ecxt_param_exec_vals; + prm = &(param_array[op->d.param.paramid]); if (unlikely(prm->execPlan != NULL)) { /* Parameter not evaluated yet, so go do it */ diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c index 64af36a187..79d8f1a33e 100644 --- a/src/backend/executor/nodeSubplan.c +++ b/src/backend/executor/nodeSubplan.c @@ -255,20 +255,30 @@ ExecScanSubPlan(SubPlanState *node, * same underlying subplan. However, that fails to hold for MULTIEXPRs * because they can have non-empty args lists, and the "same" args might * have mutated into different forms in different parts of a plan tree. - * There is currently no problem because MULTIEXPR can appear only in an - * UPDATE's top-level target list, so it won't get duplicated anyplace. - * Postgres versions before v14 had to make concrete efforts to avoid - * sharing output parameters across different clones of a MULTIEXPR, and - * the problem could recur someday. + * Therefore, MULTIEXPR SubPlans that are not initplans (i.e., have + * non-empty args lists) are given private output arrays that are + * allocated when the containing targetlist expression is compiled. This + * solution works because the referencing Params must be part of the same + * targetlist expression, so we can fix them up to find the private output + * array during expression compilation. */ if (subLinkType == MULTIEXPR_SUBLINK) { - EState *estate = node->parent->state; + ParamExecData *param_array; + + /* + * If subplan has a private output array, use that; otherwise use + * es_param_exec_vals. + */ + if (node->private_exec_vals) + param_array = node->private_exec_vals; + else + param_array = node->parent->state->es_param_exec_vals; foreach(l, subplan->setParam) { int paramid = lfirst_int(l); - ParamExecData *prm = &(estate->es_param_exec_vals[paramid]); + ParamExecData *prm = &(param_array[paramid]); prm->execPlan = node; } @@ -830,6 +840,7 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent) /* * initialize my state */ + sstate->private_exec_vals = NULL; sstate->curTuple = NULL; sstate->curArray = PointerGetDatum(NULL); sstate->projLeft = NULL; @@ -861,12 +872,36 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent) */ if (subplan->setParam != NIL && subplan->subLinkType != CTE_SUBLINK) { + ParamExecData *param_array = estate->es_param_exec_vals; ListCell *lst; + /* + * If this is a MULTIEXPR subplan (and not an initplan), it needs a + * private ParamExecData output array. We index this array with the + * same Param IDs used for the query-global es_param_exec_vals array, + * so there will be some wasted space, but not enough to sweat over. + */ + if (subplan->subLinkType == MULTIEXPR_SUBLINK && + subplan->parParam != NIL) + { + int max_paramid = 0; + + foreach(lst, subplan->setParam) + { + int paramid = lfirst_int(lst); + + max_paramid = Max(max_paramid, paramid); + } + + param_array = palloc0_array(ParamExecData, max_paramid + 1); + sstate->private_exec_vals = param_array; + } + + /* Now set the execPlan links within the correct output array. */ foreach(lst, subplan->setParam) { int paramid = lfirst_int(lst); - ParamExecData *prm = &(estate->es_param_exec_vals[paramid]); + ParamExecData *prm = &(param_array[paramid]); prm->execPlan = sstate; } @@ -1057,8 +1092,8 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent) * Note that this routine MUST clear the execPlan fields of the plan's * output parameters after evaluating them! * - * The results of this function are stored in the EState associated with the - * ExprContext (particularly, its ecxt_param_exec_vals); any pass-by-ref + * The results of this function are usually stored in the es_param_exec_vals + * array of the EState associated with the ExprContext; any pass-by-ref * result Datums are allocated in the EState's per-query memory. The passed * econtext can be any ExprContext belonging to that EState; which one is * important only to the extent that the ExprContext's per-tuple memory @@ -1067,6 +1102,11 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent) * that data isn't needed after we return. In practice, because initplan * parameters are never more complex than Vars, Aggrefs, etc, evaluating them * currently never leaks any memory anyway.) + * + * However, for MULTIEXPR subplans we instead store the output in a private + * output array. This avoids confusion when several "mutated" versions of + * the same MULTIEXPR subplan appear in a query, which is possible in an + * UPDATE on a partitioned table or inheritance tree. * ---------------------------------------------------------------- */ void @@ -1077,6 +1117,7 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext) SubLinkType subLinkType = subplan->subLinkType; EState *estate = planstate->state; ScanDirection dir = estate->es_direction; + ParamExecData *output_array; MemoryContext oldcontext; TupleTableSlot *slot; ListCell *pvar; @@ -1126,6 +1167,17 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext) planstate->chgParam = bms_add_member(planstate->chgParam, paramid); } + /* + * Decide which ParamExecData array receives the output. Typically it's + * the ecxt_param_exec_vals array, but not if the subplan has a private + * output array. (That's currently only possible for MULTIEXPR subplans, + * but it's simplest to support it for all cases.) + */ + if (node->private_exec_vals) + output_array = node->private_exec_vals; + else + output_array = econtext->ecxt_param_exec_vals; + /* * Run the plan. (If it needs to be rescanned, the first ExecProcNode * call will take care of that.) @@ -1141,7 +1193,7 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext) { /* There can be only one setParam... */ int paramid = linitial_int(subplan->setParam); - ParamExecData *prm = &(econtext->ecxt_param_exec_vals[paramid]); + ParamExecData *prm = &(output_array[paramid]); prm->execPlan = NULL; prm->value = BoolGetDatum(true); @@ -1191,7 +1243,7 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext) foreach(l, subplan->setParam) { int paramid = lfirst_int(l); - ParamExecData *prm = &(econtext->ecxt_param_exec_vals[paramid]); + ParamExecData *prm = &(output_array[paramid]); prm->execPlan = NULL; prm->value = heap_getattr(node->curTuple, i, tdesc, @@ -1204,7 +1256,7 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext) { /* There can be only one setParam... */ int paramid = linitial_int(subplan->setParam); - ParamExecData *prm = &(econtext->ecxt_param_exec_vals[paramid]); + ParamExecData *prm = &(output_array[paramid]); /* * We build the result array in query context so it won't disappear; @@ -1226,7 +1278,7 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext) { /* There can be only one setParam... */ int paramid = linitial_int(subplan->setParam); - ParamExecData *prm = &(econtext->ecxt_param_exec_vals[paramid]); + ParamExecData *prm = &(output_array[paramid]); prm->execPlan = NULL; prm->value = BoolGetDatum(false); @@ -1238,7 +1290,7 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext) foreach(l, subplan->setParam) { int paramid = lfirst_int(l); - ParamExecData *prm = &(econtext->ecxt_param_exec_vals[paramid]); + ParamExecData *prm = &(output_array[paramid]); prm->execPlan = NULL; prm->value = (Datum) 0; @@ -1302,6 +1354,8 @@ ExecReScanSetParamPlan(SubPlanState *node, PlanState *parent) elog(ERROR, "setParam list of initplan is empty"); if (bms_is_empty(planstate->plan->extParam)) elog(ERROR, "extParam set of initplan is empty"); + if (node->private_exec_vals != NULL) + elog(ERROR, "initplan should not have private output area"); /* * Don't actually re-scan: it'll happen inside ExecSetParamPlan if needed. diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h index 06c3adc0a1..c43bdcea2b 100644 --- a/src/include/executor/execExpr.h +++ b/src/include/executor/execExpr.h @@ -379,6 +379,11 @@ typedef struct ExprEvalStep { int paramid; /* numeric ID for parameter */ Oid paramtype; /* OID of parameter's datatype */ + /* These fields are used for EEOP_PARAM_EXEC only: */ + /* Param's value is to be sought at private_exec_vals[paramid], */ + /* or ecxt_param_exec_vals[paramid] if private_exec_vals is NULL */ + ParamExecData *private_exec_vals; + int prev_param; /* previous PARAM_EXEC's index, or -1 */ } param; /* for EEOP_PARAM_CALLBACK */ diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 20f4c8b35f..2ae9cc702e 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -119,8 +119,9 @@ typedef struct ExprState int steps_len; /* number of steps currently */ int steps_alloc; /* allocated length of steps array */ + int last_param; /* index of last EEOP_PARAM_EXEC step, or -1 */ -#define FIELDNO_EXPRSTATE_PARENT 11 +#define FIELDNO_EXPRSTATE_PARENT 12 struct PlanState *parent; /* parent PlanState node, if any */ ParamListInfo ext_params; /* for compiling PARAM_EXTERN nodes */ @@ -948,6 +949,7 @@ typedef struct SubPlanState struct PlanState *parent; /* parent plan node's state tree */ ExprState *testexpr; /* state of combining expression */ List *args; /* states of argument expression(s) */ + ParamExecData *private_exec_vals; /* if not NULL, output values go here */ HeapTuple curTuple; /* copy of most recent tuple from subplan */ Datum curArray; /* most recent array from ARRAY() subplan */ /* these are used when hashing the subselect's output: */ diff --git a/src/include/nodes/params.h b/src/include/nodes/params.h index ad23113a61..90ddbaffed 100644 --- a/src/include/nodes/params.h +++ b/src/include/nodes/params.h @@ -132,14 +132,16 @@ typedef struct ParamListInfoData * ParamExecData entries are used for executor internal parameters * (that is, values being passed into or out of a sub-query). The * paramid of a PARAM_EXEC Param is a (zero-based) index into an - * array of ParamExecData records, which is referenced through - * es_param_exec_vals or ecxt_param_exec_vals. + * array of ParamExecData records, which is typically found via + * es_param_exec_vals or ecxt_param_exec_vals. But Params that + * reference the output of a MULTIEXPR subplan will reference a + * private ParamExecData array for that specific subplan. * * If execPlan is not NULL, it points to a SubPlanState node that needs * to be executed to produce the value. (This is done so that we can have * lazy evaluation of InitPlans: they aren't executed until/unless a - * result value is needed.) Otherwise the value is assumed to be valid - * when needed. + * result value is needed. MULTIEXPR subplans also use this mechanism.) + * Otherwise the value is assumed to be valid when needed. * ---------------- */