Fix PlanRowMark/ExecRowMark structures to handle inheritance correctly.

In an inherited UPDATE/DELETE, each target table has its own subplan,
because it might have a column set different from other targets.  This
means that the resjunk columns we add to support EvalPlanQual might be
at different physical column numbers in each subplan.  The EvalPlanQual
rewrite I did for 9.0 failed to account for this, resulting in possible
misbehavior or even crashes during concurrent updates to the same row,
as seen in a recent report from Gordon Shannon.  Revise the data structure
so that we track resjunk column numbers separately for each subplan.

I also chose to move responsibility for identifying the physical column
numbers back to executor startup, instead of assuming that numbers derived
during preprocess_targetlist would stay valid throughout subsequent
massaging of the plan.  That's a bit slower, so we might want to consider
undoing it someday; but it would complicate the patch considerably and
didn't seem justifiable in a bug fix that has to be back-patched to 9.0.

1 files changed, 82 insertions, 25 deletions

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 17c7ea294dc..600f7e03345 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -744,9 +744,6 @@ InitPlan(QueryDesc *queryDesc, int eflags)
 		erm->prti = rc->prti;
 		erm->markType = rc->markType;
 		erm->noWait = rc->noWait;
-		erm->ctidAttNo = rc->ctidAttNo;
-		erm->toidAttNo = rc->toidAttNo;
-		erm->wholeAttNo = rc->wholeAttNo;
 		ItemPointerSetInvalid(&(erm->curCtid));
 		estate->es_rowMarks = lappend(estate->es_rowMarks, erm);
 	}
@@ -1387,6 +1384,71 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
 
 
 /*
+ * ExecFindRowMark -- find the ExecRowMark struct for given rangetable index
+ */
+ExecRowMark *
+ExecFindRowMark(EState *estate, Index rti)
+{
+	ListCell   *lc;
+
+	foreach(lc, estate->es_rowMarks)
+	{
+		ExecRowMark *erm = (ExecRowMark *) lfirst(lc);
+
+		if (erm->rti == rti)
+			return erm;
+	}
+	elog(ERROR, "failed to find ExecRowMark for rangetable index %u", rti);
+	return NULL;				/* keep compiler quiet */
+}
+
+/*
+ * ExecBuildAuxRowMark -- create an ExecAuxRowMark struct
+ *
+ * Inputs are the underlying ExecRowMark struct and the targetlist of the
+ * input plan node (not planstate node!).  We need the latter to find out
+ * the column numbers of the resjunk columns.
+ */
+ExecAuxRowMark *
+ExecBuildAuxRowMark(ExecRowMark *erm, List *targetlist)
+{
+	ExecAuxRowMark *aerm = (ExecAuxRowMark *) palloc0(sizeof(ExecAuxRowMark));
+	char		resname[32];
+
+	aerm->rowmark = erm;
+
+	/* Look up the resjunk columns associated with this rowmark */
+	if (erm->relation)
+	{
+		Assert(erm->markType != ROW_MARK_COPY);
+
+		/* if child rel, need tableoid */
+		if (erm->rti != erm->prti)
+		{
+			snprintf(resname, sizeof(resname), "tableoid%u", erm->prti);
+			aerm->toidAttNo = ExecFindJunkAttributeInTlist(targetlist,
+														   resname);
+		}
+
+		/* always need ctid for real relations */
+		snprintf(resname, sizeof(resname), "ctid%u", erm->prti);
+		aerm->ctidAttNo = ExecFindJunkAttributeInTlist(targetlist,
+													   resname);
+	}
+	else
+	{
+		Assert(erm->markType == ROW_MARK_COPY);
+
+		snprintf(resname, sizeof(resname), "wholerow%u", erm->prti);
+		aerm->wholeAttNo = ExecFindJunkAttributeInTlist(targetlist,
+														resname);
+	}
+
+	return aerm;
+}
+
+
+/*
  * EvalPlanQual logic --- recheck modified tuple(s) to see if we want to
  * process the updated version under READ COMMITTED rules.
  *
@@ -1676,11 +1738,13 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
 /*
  * EvalPlanQualInit -- initialize during creation of a plan state node
  * that might need to invoke EPQ processing.
- * Note: subplan can be NULL if it will be set later with EvalPlanQualSetPlan.
+ *
+ * Note: subplan/auxrowmarks can be NULL/NIL if they will be set later
+ * with EvalPlanQualSetPlan.
  */
 void
 EvalPlanQualInit(EPQState *epqstate, EState *estate,
-				 Plan *subplan, int epqParam)
+				 Plan *subplan, List *auxrowmarks, int epqParam)
 {
 	/* Mark the EPQ state inactive */
 	epqstate->estate = NULL;
@@ -1688,7 +1752,7 @@ EvalPlanQualInit(EPQState *epqstate, EState *estate,
 	epqstate->origslot = NULL;
 	/* ... and remember data that EvalPlanQualBegin will need */
 	epqstate->plan = subplan;
-	epqstate->rowMarks = NIL;
+	epqstate->arowMarks = auxrowmarks;
 	epqstate->epqParam = epqParam;
 }
 
@@ -1698,25 +1762,14 @@ EvalPlanQualInit(EPQState *epqstate, EState *estate,
  * We need this so that ModifyTuple can deal with multiple subplans.
  */
 void
-EvalPlanQualSetPlan(EPQState *epqstate, Plan *subplan)
+EvalPlanQualSetPlan(EPQState *epqstate, Plan *subplan, List *auxrowmarks)
 {
 	/* If we have a live EPQ query, shut it down */
 	EvalPlanQualEnd(epqstate);
 	/* And set/change the plan pointer */
 	epqstate->plan = subplan;
-}
-
-/*
- * EvalPlanQualAddRowMark -- add an ExecRowMark that EPQ needs to handle.
- *
- * Currently, only non-locking RowMarks are supported.
- */
-void
-EvalPlanQualAddRowMark(EPQState *epqstate, ExecRowMark *erm)
-{
-	if (RowMarkRequiresRowShareLock(erm->markType))
-		elog(ERROR, "EvalPlanQual doesn't support locking rowmarks");
-	epqstate->rowMarks = lappend(epqstate->rowMarks, erm);
+	/* The rowmarks depend on the plan, too */
+	epqstate->arowMarks = auxrowmarks;
 }
 
 /*
@@ -1766,13 +1819,17 @@ EvalPlanQualFetchRowMarks(EPQState *epqstate)
 
 	Assert(epqstate->origslot != NULL);
 
-	foreach(l, epqstate->rowMarks)
+	foreach(l, epqstate->arowMarks)
 	{
-		ExecRowMark *erm = (ExecRowMark *) lfirst(l);
+		ExecAuxRowMark *aerm = (ExecAuxRowMark *) lfirst(l);
+		ExecRowMark *erm = aerm->rowmark;
 		Datum		datum;
 		bool		isNull;
 		HeapTupleData tuple;
 
+		if (RowMarkRequiresRowShareLock(erm->markType))
+			elog(ERROR, "EvalPlanQual doesn't support locking rowmarks");
+
 		/* clear any leftover test tuple for this rel */
 		EvalPlanQualSetTuple(epqstate, erm->rti, NULL);
 
@@ -1788,7 +1845,7 @@ EvalPlanQualFetchRowMarks(EPQState *epqstate)
 				Oid			tableoid;
 
 				datum = ExecGetJunkAttribute(epqstate->origslot,
-											 erm->toidAttNo,
+											 aerm->toidAttNo,
 											 &isNull);
 				/* non-locked rels could be on the inside of outer joins */
 				if (isNull)
@@ -1804,7 +1861,7 @@ EvalPlanQualFetchRowMarks(EPQState *epqstate)
 
 			/* fetch the tuple's ctid */
 			datum = ExecGetJunkAttribute(epqstate->origslot,
-										 erm->ctidAttNo,
+										 aerm->ctidAttNo,
 										 &isNull);
 			/* non-locked rels could be on the inside of outer joins */
 			if (isNull)
@@ -1829,7 +1886,7 @@ EvalPlanQualFetchRowMarks(EPQState *epqstate)
 
 			/* fetch the whole-row Var for the relation */
 			datum = ExecGetJunkAttribute(epqstate->origslot,
-										 erm->wholeAttNo,
+										 aerm->wholeAttNo,
 										 &isNull);
 			/* non-locked rels could be on the inside of outer joins */
 			if (isNull)