From fed8dcdb84d255088d22efa3156a193f3399e792 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 12 Jan 2011 20:47:09 -0500 Subject: 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. --- src/backend/optimizer/plan/planner.c | 8 -------- src/backend/optimizer/prep/preptlist.c | 17 ++--------------- src/backend/optimizer/prep/prepunion.c | 4 ---- 3 files changed, 2 insertions(+), 27 deletions(-) (limited to 'src/backend/optimizer') diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 317a7e87f33..45ba902fbab 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -1858,10 +1858,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); } @@ -1888,10 +1884,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 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); } -- cgit v1.2.3