diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 9c70cb7c17..7d57f8c63c 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -45,6 +45,7 @@ #include "commands/matview.h" #include "commands/trigger.h" #include "executor/execdebug.h" +#include "executor/nodeSubplan.h" #include "foreign/fdwapi.h" #include "mb/pg_wchar.h" #include "miscadmin.h" @@ -1572,8 +1573,8 @@ ExecutePlan(EState *estate, if (TupIsNull(slot)) { /* - * If we know we won't need to back up, we can release - * resources at this point. + * If we know we won't need to back up, we can release resources + * at this point. */ if (!(estate->es_top_eflags & EXEC_FLAG_BACKWARD)) (void) ExecShutdownNode(planstate); @@ -2718,7 +2719,17 @@ EvalPlanQualBegin(EPQState *epqstate, EState *parentestate) /* Recopy current values of parent parameters */ if (parentestate->es_plannedstmt->nParamExec > 0) { - int i = parentestate->es_plannedstmt->nParamExec; + int i; + + /* + * Force evaluation of any InitPlan outputs that could be needed + * by the subplan, just in case they got reset since + * EvalPlanQualStart (see comments therein). + */ + ExecSetParamPlanMulti(planstate->plan->extParam, + GetPerTupleExprContext(parentestate)); + + i = parentestate->es_plannedstmt->nParamExec; while (--i >= 0) { @@ -2808,10 +2819,34 @@ EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree) estate->es_param_list_info = parentestate->es_param_list_info; if (parentestate->es_plannedstmt->nParamExec > 0) { - int i = parentestate->es_plannedstmt->nParamExec; + int i; + /* + * Force evaluation of any InitPlan outputs that could be needed by + * the subplan. (With more complexity, maybe we could postpone this + * till the subplan actually demands them, but it doesn't seem worth + * the trouble; this is a corner case already, since usually the + * InitPlans would have been evaluated before reaching EvalPlanQual.) + * + * This will not touch output params of InitPlans that occur somewhere + * within the subplan tree, only those that are attached to the + * ModifyTable node or above it and are referenced within the subplan. + * That's OK though, because the planner would only attach such + * InitPlans to a lower-level SubqueryScan node, and EPQ execution + * will not descend into a SubqueryScan. + * + * The EState's per-output-tuple econtext is sufficiently short-lived + * for this, since it should get reset before there is any chance of + * doing EvalPlanQual again. + */ + ExecSetParamPlanMulti(planTree->extParam, + GetPerTupleExprContext(parentestate)); + + /* now make the internal param workspace ... */ + i = parentestate->es_plannedstmt->nParamExec; estate->es_param_exec_vals = (ParamExecData *) palloc0(i * sizeof(ParamExecData)); + /* ... and copy down all values, whether really needed or not */ while (--i >= 0) { /* copy value if any, but not execPlan link */ diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c index b4883c4f8d..01518df8b2 100644 --- a/src/backend/executor/nodeSubplan.c +++ b/src/backend/executor/nodeSubplan.c @@ -947,6 +947,17 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent) * of initplans: we don't run the subplan until/unless we need its output. * 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 + * 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 + * context is used to evaluate any parameters passed down to the subplan. + * (Thus in principle, the shorter-lived the ExprContext the better, since + * 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.) * ---------------------------------------------------------------- */ void @@ -1134,6 +1145,37 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext) estate->es_direction = dir; } +/* + * ExecSetParamPlanMulti + * + * Apply ExecSetParamPlan to evaluate any not-yet-evaluated initplan output + * parameters whose ParamIDs are listed in "params". Any listed params that + * are not initplan outputs are ignored. + * + * As with ExecSetParamPlan, any ExprContext belonging to the current EState + * can be used, but in principle a shorter-lived ExprContext is better than a + * longer-lived one. + */ +void +ExecSetParamPlanMulti(const Bitmapset *params, ExprContext *econtext) +{ + int paramid; + + paramid = -1; + while ((paramid = bms_next_member(params, paramid)) >= 0) + { + ParamExecData *prm = &(econtext->ecxt_param_exec_vals[paramid]); + + if (prm->execPlan != NULL) + { + /* Parameter not evaluated yet, so go do it */ + ExecSetParamPlan(prm->execPlan, econtext); + /* ExecSetParamPlan should have processed this param... */ + Assert(prm->execPlan == NULL); + } + } +} + /* * Mark an initplan as needing recalculation */ diff --git a/src/include/executor/nodeSubplan.h b/src/include/executor/nodeSubplan.h index f05a47419d..34b18c1022 100644 --- a/src/include/executor/nodeSubplan.h +++ b/src/include/executor/nodeSubplan.h @@ -24,4 +24,6 @@ extern void ExecReScanSetParamPlan(SubPlanState *node, PlanState *parent); extern void ExecSetParamPlan(SubPlanState *node, ExprContext *econtext); +extern void ExecSetParamPlanMulti(const Bitmapset *params, ExprContext *econtext); + #endif /* NODESUBPLAN_H */ diff --git a/src/test/isolation/expected/eval-plan-qual.out b/src/test/isolation/expected/eval-plan-qual.out index d960e83147..235026cb72 100644 --- a/src/test/isolation/expected/eval-plan-qual.out +++ b/src/test/isolation/expected/eval-plan-qual.out @@ -164,6 +164,37 @@ ta_id ta_value tb_row 1 newTableAValue (1,tableBValue) step c2: COMMIT; +starting permutation: updateforcip updateforcip2 c1 c2 read_a +step updateforcip: + UPDATE table_a SET value = NULL WHERE id = 1; + +step updateforcip2: + UPDATE table_a SET value = COALESCE(value, (SELECT text 'newValue')) WHERE id = 1; + +step c1: COMMIT; +step updateforcip2: <... completed> +step c2: COMMIT; +step read_a: SELECT * FROM table_a ORDER BY id; +id value + +1 newValue + +starting permutation: updateforcip updateforcip3 c1 c2 read_a +step updateforcip: + UPDATE table_a SET value = NULL WHERE id = 1; + +step updateforcip3: + WITH d(val) AS (SELECT text 'newValue' FROM generate_series(1,1)) + UPDATE table_a SET value = COALESCE(value, (SELECT val FROM d)) WHERE id = 1; + +step c1: COMMIT; +step updateforcip3: <... completed> +step c2: COMMIT; +step read_a: SELECT * FROM table_a ORDER BY id; +id value + +1 newValue + starting permutation: wrtwcte readwcte c1 c2 step wrtwcte: UPDATE table_a SET value = 'tableAValue2' WHERE id = 1; step readwcte: diff --git a/src/test/isolation/specs/eval-plan-qual.spec b/src/test/isolation/specs/eval-plan-qual.spec index de8ed39b1e..3825dd22f6 100644 --- a/src/test/isolation/specs/eval-plan-qual.spec +++ b/src/test/isolation/specs/eval-plan-qual.spec @@ -78,6 +78,13 @@ step "updateforss" { UPDATE table_b SET value = 'newTableBValue' WHERE id = 1; } +# these tests exercise EvalPlanQual with conditional InitPlans which +# have not been executed prior to the EPQ + +step "updateforcip" { + UPDATE table_a SET value = NULL WHERE id = 1; +} + session "s2" setup { BEGIN ISOLATION LEVEL READ COMMITTED; } @@ -103,12 +110,20 @@ step "readforss" { FROM table_a ta WHERE ta.id = 1 FOR UPDATE OF ta; } +step "updateforcip2" { + UPDATE table_a SET value = COALESCE(value, (SELECT text 'newValue')) WHERE id = 1; +} +step "updateforcip3" { + WITH d(val) AS (SELECT text 'newValue' FROM generate_series(1,1)) + UPDATE table_a SET value = COALESCE(value, (SELECT val FROM d)) WHERE id = 1; +} step "wrtwcte" { UPDATE table_a SET value = 'tableAValue2' WHERE id = 1; } step "c2" { COMMIT; } session "s3" setup { BEGIN ISOLATION LEVEL READ COMMITTED; } step "read" { SELECT * FROM accounts ORDER BY accountid; } +step "read_a" { SELECT * FROM table_a ORDER BY id; } # this test exercises EvalPlanQual with a CTE, cf bug #14328 step "readwcte" { @@ -142,5 +157,7 @@ permutation "writep2" "returningp1" "c1" "c2" permutation "wx2" "partiallock" "c2" "c1" "read" permutation "wx2" "lockwithvalues" "c2" "c1" "read" permutation "updateforss" "readforss" "c1" "c2" +permutation "updateforcip" "updateforcip2" "c1" "c2" "read_a" +permutation "updateforcip" "updateforcip3" "c1" "c2" "read_a" permutation "wrtwcte" "readwcte" "c1" "c2" permutation "wrtwcte" "multireadwcte" "c1" "c2"