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.

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 422f737e82dcb1202d3f9e927fe87f06f6b33c7b..36dcc8e4b5d2e24abca9f07b8dcd673200ca745d 100644 (file)
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -2347,11 +2347,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)
@@ -2359,12 +2355,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 37b70b88d53f6f8589c2fbb3036e5f1dc27f95ad..dfdcb20b1d1e06cc154f74275ae93c3539aac2a2 100644 (file)
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -715,6 +715,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
@@ -892,14 +904,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
         */
@@ -947,9 +951,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 */