Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 0b1e177

Browse files
committed
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.
1 parent 7c016e3 commit 0b1e177

File tree

2 files changed

+19
-20
lines changed

2 files changed

+19
-20
lines changed

src/backend/executor/execMain.c

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2274,24 +2274,15 @@ EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree)
22742274
* ExecInitSubPlan expects to be able to find these entries. Some of the
22752275
* SubPlans might not be used in the part of the plan tree we intend to
22762276
* run, but since it's not easy to tell which, we just initialize them
2277-
* all. (However, if the subplan is headed by a ModifyTable node, then it
2278-
* must be a data-modifying CTE, which we will certainly not need to
2279-
* re-run, so we can skip initializing it. This is just an efficiency
2280-
* hack; it won't skip data-modifying CTEs for which the ModifyTable node
2281-
* is not at the top.)
2277+
* all.
22822278
*/
22832279
Assert(estate->es_subplanstates == NIL);
22842280
foreach(l, parentestate->es_plannedstmt->subplans)
22852281
{
22862282
Plan *subplan = (Plan *) lfirst(l);
22872283
PlanState *subplanstate;
22882284

2289-
/* Don't initialize ModifyTable subplans, per comment above */
2290-
if (IsA(subplan, ModifyTable))
2291-
subplanstate = NULL;
2292-
else
2293-
subplanstate = ExecInitNode(subplan, estate, 0);
2294-
2285+
subplanstate = ExecInitNode(subplan, estate, 0);
22952286
estate->es_subplanstates = lappend(estate->es_subplanstates,
22962287
subplanstate);
22972288
}

src/backend/executor/nodeModifyTable.c

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -714,6 +714,18 @@ ExecModifyTable(ModifyTableState *node)
714714
ItemPointerData tuple_ctid;
715715
HeapTupleHeader oldtuple = NULL;
716716

717+
/*
718+
* This should NOT get called during EvalPlanQual; we should have passed a
719+
* subplan tree to EvalPlanQual, instead. Use a runtime test not just
720+
* Assert because this condition is easy to miss in testing. (Note:
721+
* although ModifyTable should not get executed within an EvalPlanQual
722+
* operation, we do have to allow it to be initialized and shut down in
723+
* case it is within a CTE subplan. Hence this test must be here, not in
724+
* ExecInitModifyTable.)
725+
*/
726+
if (estate->es_epqTuple != NULL)
727+
elog(ERROR, "ModifyTable should not be called during EvalPlanQual");
728+
717729
/*
718730
* If we've already completed processing, don't try to do more. We need
719731
* this test because ExecPostprocessPlan might call us an extra time, and
@@ -891,14 +903,6 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
891903
/* check for unsupported flags */
892904
Assert(!(eflags & (EXEC_FLAG_BACKWARD | EXEC_FLAG_MARK)));
893905

894-
/*
895-
* This should NOT get called during EvalPlanQual; we should have passed a
896-
* subplan tree to EvalPlanQual, instead. Use a runtime test not just
897-
* Assert because this condition is easy to miss in testing ...
898-
*/
899-
if (estate->es_epqTuple != NULL)
900-
elog(ERROR, "ModifyTable should not be called during EvalPlanQual");
901-
902906
/*
903907
* create state structure
904908
*/
@@ -946,9 +950,13 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
946950
* descriptors in the result relation info, so that we can add new
947951
* index entries for the tuples we add/update. We need not do this
948952
* for a DELETE, however, since deletion doesn't affect indexes.
953+
* Also, inside an EvalPlanQual operation, the indexes might be open
954+
* already, since we share the resultrel state with the original
955+
* query.
949956
*/
950957
if (resultRelInfo->ri_RelationDesc->rd_rel->relhasindex &&
951-
operation != CMD_DELETE)
958+
operation != CMD_DELETE &&
959+
resultRelInfo->ri_IndexRelationDescs == NULL)
952960
ExecOpenIndices(resultRelInfo);
953961

954962
/* Now init the plan for this result rel */

0 commit comments

Comments
 (0)