From 0b1e1779533081f9d48c110a5e91ba122fc54461 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 28 Jan 2012 17:44:03 -0500 Subject: [PATCH] Fix handling of data-modifying CTE subplans in EvalPlanQual. We can't just skip initializing such subplans, because the referencing CTE node will expect to find the subplan available when it initializes. That in turn means that ExecInitModifyTable must allow the case (which actually it needed to do anyway, since there's no guarantee that ModifyTable is exactly at the top of the CTE plan tree). So move the complaint about not being allowed in EvalPlanQual mode to execution instead of initialization. Testing turned up yet another problem, which is that we'd try to re-initialize the result relation's index list, leading to leaks and dangling pointers. Per report from Phil Sorber. Back-patch to 9.1 where data-modifying CTEs were introduced. --- src/backend/executor/execMain.c | 13 ++----------- src/backend/executor/nodeModifyTable.c | 26 +++++++++++++++++--------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 8198882b25..621ad8ac94 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -2274,11 +2274,7 @@ EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree) * ExecInitSubPlan expects to be able to find these entries. Some of the * SubPlans might not be used in the part of the plan tree we intend to * run, but since it's not easy to tell which, we just initialize them - * all. (However, if the subplan is headed by a ModifyTable node, then it - * must be a data-modifying CTE, which we will certainly not need to - * re-run, so we can skip initializing it. This is just an efficiency - * hack; it won't skip data-modifying CTEs for which the ModifyTable node - * is not at the top.) + * all. */ Assert(estate->es_subplanstates == NIL); foreach(l, parentestate->es_plannedstmt->subplans) @@ -2286,12 +2282,7 @@ EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree) Plan *subplan = (Plan *) lfirst(l); PlanState *subplanstate; - /* Don't initialize ModifyTable subplans, per comment above */ - if (IsA(subplan, ModifyTable)) - subplanstate = NULL; - else - subplanstate = ExecInitNode(subplan, estate, 0); - + subplanstate = ExecInitNode(subplan, estate, 0); estate->es_subplanstates = lappend(estate->es_subplanstates, subplanstate); } diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index c0eab4bf0d..cb64cfc857 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -714,6 +714,18 @@ ExecModifyTable(ModifyTableState *node) ItemPointerData tuple_ctid; HeapTupleHeader oldtuple = NULL; + /* + * This should NOT get called during EvalPlanQual; we should have passed a + * subplan tree to EvalPlanQual, instead. Use a runtime test not just + * Assert because this condition is easy to miss in testing. (Note: + * although ModifyTable should not get executed within an EvalPlanQual + * operation, we do have to allow it to be initialized and shut down in + * case it is within a CTE subplan. Hence this test must be here, not in + * ExecInitModifyTable.) + */ + if (estate->es_epqTuple != NULL) + elog(ERROR, "ModifyTable should not be called during EvalPlanQual"); + /* * If we've already completed processing, don't try to do more. We need * this test because ExecPostprocessPlan might call us an extra time, and @@ -891,14 +903,6 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) /* check for unsupported flags */ Assert(!(eflags & (EXEC_FLAG_BACKWARD | EXEC_FLAG_MARK))); - /* - * This should NOT get called during EvalPlanQual; we should have passed a - * subplan tree to EvalPlanQual, instead. Use a runtime test not just - * Assert because this condition is easy to miss in testing ... - */ - if (estate->es_epqTuple != NULL) - elog(ERROR, "ModifyTable should not be called during EvalPlanQual"); - /* * create state structure */ @@ -946,9 +950,13 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) * descriptors in the result relation info, so that we can add new * index entries for the tuples we add/update. We need not do this * for a DELETE, however, since deletion doesn't affect indexes. + * Also, inside an EvalPlanQual operation, the indexes might be open + * already, since we share the resultrel state with the original + * query. */ if (resultRelInfo->ri_RelationDesc->rd_rel->relhasindex && - operation != CMD_DELETE) + operation != CMD_DELETE && + resultRelInfo->ri_IndexRelationDescs == NULL) ExecOpenIndices(resultRelInfo); /* Now init the plan for this result rel */ -- 2.40.0