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.

2 files changed, 2 insertions, 19 deletions

diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c
index 2ea01a7f08d..7a09d1f8309 100644
--- a/src/backend/optimizer/prep/preptlist.c
+++ b/src/backend/optimizer/prep/preptlist.c
@@ -111,8 +111,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)
 	{
@@ -121,18 +120,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)
 		{
@@ -148,7 +138,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)
@@ -164,7 +153,6 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
 									  pstrdup(resname),
 									  true);
 				tlist = lappend(tlist, tle);
-				rc->toidAttNo = tle->resno;
 			}
 		}
 		else
@@ -179,7 +167,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 3cb5ed977b0..35f9f980e90 100644
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -1291,10 +1291,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);
 		}