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.

3 files changed, 2 insertions, 27 deletions

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 20b147ce94..d52ebd0ea1 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -1929,10 +1929,6 @@ preprocess_rowmarks(PlannerInfo *root)
 			newrc->markType = ROW_MARK_SHARE;
 		newrc->noWait = rc->noWait;
 		newrc->isParent = false;
-		/* attnos will be assigned in preprocess_targetlist */
-		newrc->ctidAttNo = InvalidAttrNumber;
-		newrc->toidAttNo = InvalidAttrNumber;
-		newrc->wholeAttNo = InvalidAttrNumber;
 
 		prowmarks = lappend(prowmarks, newrc);
 	}
@@ -1959,10 +1955,6 @@ preprocess_rowmarks(PlannerInfo *root)
 			newrc->markType = ROW_MARK_COPY;
 		newrc->noWait = false;	/* doesn't matter */
 		newrc->isParent = false;
-		/* attnos will be assigned in preprocess_targetlist */
-		newrc->ctidAttNo = InvalidAttrNumber;
-		newrc->toidAttNo = InvalidAttrNumber;
-		newrc->wholeAttNo = InvalidAttrNumber;
 
 		prowmarks = lappend(prowmarks, newrc);
 	}
diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c
index 2a04e2562d..36c19438c0 100644
--- a/src/backend/optimizer/prep/preptlist.c
+++ b/src/backend/optimizer/prep/preptlist.c
@@ -80,8 +80,7 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
 	/*
 	 * Add necessary junk columns for rowmarked rels.  These values are needed
 	 * for locking of rels selected FOR UPDATE/SHARE, and to do EvalPlanQual
-	 * rechecking.	While we are at it, store these junk attnos in the
-	 * PlanRowMark list so that we don't have to redetermine them at runtime.
+	 * rechecking.  See comments for PlanRowMark in plannodes.h.
 	 */
 	foreach(lc, root->rowMarks)
 	{
@@ -90,18 +89,9 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
 		char		resname[32];
 		TargetEntry *tle;
 
-		/* child rels should just use the same junk attrs as their parents */
+		/* child rels use the same junk attrs as their parents */
 		if (rc->rti != rc->prti)
-		{
-			PlanRowMark *prc = get_plan_rowmark(root->rowMarks, rc->prti);
-
-			/* parent should have appeared earlier in list */
-			if (prc == NULL || prc->toidAttNo == InvalidAttrNumber)
-				elog(ERROR, "parent PlanRowMark not processed yet");
-			rc->ctidAttNo = prc->ctidAttNo;
-			rc->toidAttNo = prc->toidAttNo;
 			continue;
-		}
 
 		if (rc->markType != ROW_MARK_COPY)
 		{
@@ -117,7 +107,6 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
 								  pstrdup(resname),
 								  true);
 			tlist = lappend(tlist, tle);
-			rc->ctidAttNo = tle->resno;
 
 			/* if parent of inheritance tree, need the tableoid too */
 			if (rc->isParent)
@@ -133,7 +122,6 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
 									  pstrdup(resname),
 									  true);
 				tlist = lappend(tlist, tle);
-				rc->toidAttNo = tle->resno;
 			}
 		}
 		else
@@ -148,7 +136,6 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
 								  pstrdup(resname),
 								  true);
 			tlist = lappend(tlist, tle);
-			rc->wholeAttNo = tle->resno;
 		}
 	}
 
diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c
index dd5513bb57..449c8dab50 100644
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -1292,10 +1292,6 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
 			newrc->markType = oldrc->markType;
 			newrc->noWait = oldrc->noWait;
 			newrc->isParent = false;
-			/* junk attrs for children are not identified yet */
-			newrc->ctidAttNo = InvalidAttrNumber;
-			newrc->toidAttNo = InvalidAttrNumber;
-			newrc->wholeAttNo = InvalidAttrNumber;
 
 			root->rowMarks = lappend(root->rowMarks, newrc);
 		}