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 8198882b252ed3559d2ac0cae1eb6de393765057..621ad8ac940717a04275711ac4a4ebe1aba401ec 100644 (file)
--- 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 c0eab4bf0db894dce689bfcabbde4d7e281b7714..cb64cfc857a379d42b20510d8387eec058174e60 100644 (file)
--- 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 */