8000 Fix PlanRowMark/ExecRowMark structures to handle inheritance correctly. · machack666/postgres@fed8dcd · GitHub
[go: up one dir, main page]
Skip to content

Commit fed8dcd

Browse files
tglsfdctglsfdc
committed
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 parent a1ed4cf commit fed8dcd

File tree

12 files changed

+192
-122
lines changed

12 files changed

+192
-122
lines changed

‎src/backend/executor/execJunk.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,10 +207,22 @@ ExecInitJunkFilterConversion(List *targetList,
207207
*/
208208
AttrNumber
209209
ExecFindJunkAttribute(JunkFilter *junkfilter, const char *attrName)
210+
{
211+
return ExecFindJunkAttributeInTlist(junkfilter->jf_targetList, attrName);
212+
}
213+
214+
/*
215+
* ExecFindJunkAttributeInTlist
216+
*
217+
* Find a junk attribute given a subplan's targetlist (not necessarily
218+
* part of a JunkFilter).
219+
*/
220+
AttrNumber
221+
ExecFindJunkAttributeInTlist(List *targetlist, const char *attrName)
210222
{
211223
ListCell *t;
212224

213-
foreach(t, junkfilter->jf_targetList)
225+
foreach(t, targetlist)
214226
{
215227
TargetEntry *tle = lfirst(t);
216228

‎src/backend/executor/execMain.c

Lines changed: 82 additions & 25 deletions
< 67E6 tr class="diff-line-row">
Original file line numberDiff line numberDiff line change
@@ -728,9 +728,6 @@ InitPlan(QueryDesc *queryDesc, int eflags)
728728
erm->prti = rc->prti;
729729
erm->markType = rc->markType;
730730
erm->noWait = rc->noWait;
731-
erm->ctidAttNo = rc->ctidAttNo;
732-
erm->toidAttNo = rc->toidAttNo;
733-
erm->wholeAttNo = rc->wholeAttNo;
734731
ItemPointerSetInvalid(&(erm->curCtid));
735732
estate->es_rowMarks = lappend(estate->es_rowMarks, erm);
736733
}
@@ -1334,6 +1331,71 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
13341331
}
13351332

13361333

1334+
/*
1335+
* ExecFindRowMark -- find the ExecRowMark struct for given rangetable index
1336+
*/
1337+
ExecRowMark *
1338+
ExecFindRowMark(EState *estate, Index rti)
1339+
{
1340+
ListCell *lc;
1341+
1342+
foreach(lc, estate->es_rowMarks)
1343+
{
1344+
ExecRowMark *erm = (ExecRowMark *) lfirst(lc);
1345+
1346+
if (erm->rti == rti)
1347+
return erm;
1348+
}
1349+
elog(ERROR, "failed to find ExecRowMark for rangetable index %u", rti);
1350+
return NULL; /* keep compiler quiet */
1351+
}
1352+
1353+
/*
1354+
* ExecBuildAuxRowMark -- create an ExecAuxRowMark struct
1355+
*
1356+
* Inputs are the underlying ExecRowMark struct and the targetlist of the
1357+
* input plan node (not planstate node!). We need the latter to find out
1358+
* the column numbers of the resjunk columns.
1359+
*/
1360+
ExecAuxRowMark *
1361+
ExecBuildAuxRowMark(ExecRowMark *erm, List *targetlist)
1362+
{
1363+
ExecAuxRowMark *aerm = (ExecAuxRowMark *) palloc0(sizeof(ExecAuxRowMark));
1364+
char resname[32];
1365+
1366+
aerm->rowmark = erm;
1367+
1368+
/* Look up the resjunk columns associated w F438 ith this rowmark */
1369+
if (erm->relation)
1370+
{
1371+
Assert(erm->markType != ROW_MARK_COPY);
1372+
1373+
/* if child rel, need tableoid */
1374+
if (erm->rti != erm->prti)
1375+
{
1376+
snprintf(resname, sizeof(resname), "tableoid%u", erm->prti);
1377+
aerm->toidAttNo = ExecFindJunkAttributeInTlist(targetlist,
1378+
resname);
1379+
}
1380+
1381+
/* always need ctid for real relations */
1382+
snprintf(resname, sizeof(resname), "ctid%u", erm->prti);
1383+
aerm->ctidAttNo = ExecFindJunkAttributeInTlist(targetlist,
1384+
resname);
1385+
}
1386+
else
1387+
{
1388+
Assert(erm->markType == ROW_MARK_COPY);
1389+
1390+
snprintf(resname, sizeof(resname), "wholerow%u", erm->prti);
1391+
aerm->wholeAttNo = ExecFindJunkAttributeInTlist(targetlist,
1392+
resname);
1393+
}
1394+
1395+
return aerm;
1396+
}
1397+
1398+
13371399
/*
13381400
* EvalPlanQual logic --- recheck modified tuple(s) to see if we want to
13391401
* process the updated version under READ COMMITTED rules.
@@ -1624,19 +1686,21 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
16241686
/*
16251687
* EvalPlanQualInit -- initialize during creation of a plan state node
16261688
* that might need to invoke EPQ processing.
1627-
* Note: subplan can be NULL if it will be set later with EvalPlanQualSetPlan.
1689+
*
1690+
* Note: subplan/auxrowmarks can be NULL/NIL if they will be set later
1691+
* with EvalPlanQualSetPlan.
16281692
*/
16291693
void
16301694
EvalPlanQualInit(EPQState *epqstate, EState *estate,
1631-
Plan *subplan, int epqParam)
1695+
Plan *subplan, List *auxrowmarks, int epqParam)
16321696
{
16331697
/* Mark the EPQ state inactive */
16341698
epqstate->estate = NULL;
16351699
epqstate->planstate = NULL;
16361700
epqstate->origslot = NULL;
16371701
/* ... and remember data that EvalPlanQualBegin will need */
16381702
epqstate->plan = subplan;
1639-
epqstate->rowMarks = NIL;
1703+
epqstate->arowMarks = auxrowmarks;
16401704
epqstate->epqParam = epqParam;
16411705
}
16421706

@@ -1646,25 +1710,14 @@ EvalPlanQualInit(EPQState *epqstate, EState *estate,
16461710
* We need this so that ModifyTuple can deal with multiple subplans.
16471711
*/
16481712
void
1649-
EvalPlanQualSetPlan(EPQState *epqstate, Plan *subplan)
1713+
EvalPlanQualSetPlan(EPQState *epqstate, Plan *subplan, List *auxrowmarks)
16501714
{
16511715
/* If we have a live EPQ query, shut it down */
16521716
EvalPlanQualEnd(epqstate);
16531717
/* And set/change the plan pointer */
16541718
epqstate->plan = subplan;
1655-
}
1656-
1657-
/*
1658-
* EvalPlanQualAddRowMark -- add an ExecRowMark that EPQ needs to handle.
1659-
*
1660-
* Currently, only non-locking RowMarks are supported.
1661-
*/
1662-
void
1663-
EvalPlanQualAddRowMark(EPQState *epqstate, ExecRowMark *erm)
1664-
{
1665-
if (RowMarkRequiresRowShareLock(erm->markType))
1666-
elog(ERROR, "EvalPlanQual doesn't support locking rowmarks");
1667-
epqstate->rowMarks = lappend(epqstate->rowMarks, erm);
1719+
/* The rowmarks depend on the plan, too */
1720+
epqstate->arowMarks = auxrowmarks;
16681721
}
16691722

16701723
/*
@@ -1714,13 +1767,17 @@ EvalPlanQualFetchRowMarks(EPQState *epqstate)
17141767

17151768
Assert(epqstate->origslot != NULL);
17161769

1717-
foreach(l, epqstate->rowMarks)
1770+
foreach(l, epqstate->arowMarks)
17181771
{
1719-
ExecRowMark *erm = (ExecRowMark *) lfirst(l);
1772+
ExecAuxRowMark *aerm = (ExecAuxRowMark *) lfirst(l);
1773+
ExecRowMark *erm = aerm->rowmark;
17201774
Datum datum;
17211775
bool isNull;
17221776
HeapTupleData tuple;
17231777

1778+
if (RowMarkRequiresRowShareLock(erm->markType))
1779+
elog(ERROR, "EvalPlanQual doesn't support locking rowmarks");
1780+
17241781
/* clear any leftover test tuple for this rel */
17251782
EvalPlanQualSetTuple(epqstate, erm->rti, NULL);
17261783

@@ -1736,7 +1793,7 @@ EvalPlanQualFetchRowMarks(EPQState *epqstate)
17361793
Oid tableoid;
17371794

17381795
datum = ExecGetJunkAttribute(epqstate->origslot,
1739-
erm->toidAttNo,
1796+
aerm->toidAttNo,
17401797
&isNull);
17411798
/* non-locked rels could be on the inside of outer joins */
17421799
if (isNull)
@@ -1752,7 +1809,7 @@ EvalPlanQualFetchRowMarks(EPQState *epqstate)
17521809

17531810
/* fetch the tuple's ctid */
17541811
datum = ExecGetJunkAttribute(epqstate->origslot,
1755-
erm->ctidAttNo,
1812+
aerm->ctidAttNo,
17561813
&isNull);
17571814
/* non-locked rels could be on the inside of outer joins */
17581815
if (isNull)
@@ -1777,7 +1834,7 @@ EvalPlanQualFetchRowMarks(EPQState *epqstate)
17771834

17781835
/* fetch the whole-row Var for the relation */
17791836
datum = ExecGetJunkAttribute(epqstate->origslot,
1780-
erm->wholeAttNo,
1837+
aerm->wholeAttNo,
17811838
&isNull);
17821839
/* non-locked rels could be on the inside of outer joins */
17831840
if (isNull)

‎src/backend/executor/nodeLockRows.c

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,13 @@ ExecLockRows(LockRowsState *node)
5858

5959
/*
6060
* Attempt to lock the source tuple(s). (Note we only have locking
61-
* rowmarks in lr_rowMarks.)
61+
* rowmarks in lr_arowMarks.)
6262
*/
6363
epq_started = false;
64-
foreach(lc, node->lr_rowMarks)
64+
foreach(lc, node->lr_arowMarks)
6565
{
66-
ExecRowMark *erm = (ExecRowMark *) lfirst(lc);
66+
ExecAuxRowMark *aerm = (ExecAuxRowMark *) lfirst(lc);
67+
ExecRowMark *erm = aerm->rowmark;
6768
Datum datum;
6869
bool isNull;
6970
HeapTupleData tuple;
@@ -84,7 +85,7 @@ ExecLockRows(LockRowsState *node)
8485
Oid tableoid;
8586

8687
datum = ExecGetJunkAttribute(slot,
87-
erm->toidAttNo,
88+
aerm->toidAttNo,
8889
&isNull);
8990
/* shouldn't ever get a null result... */
9091
if (isNull)
@@ -101,7 +102,7 @@ ExecLockRows(LockRowsState *node)
101102

102103
/* fetch the tuple's ctid */
103104
datum = ExecGetJunkAttribute(slot,
104-
erm->ctidAttNo,
105+
aerm->ctidAttNo,
105106
&isNull);
106107
/* shouldn't ever get a null result... */
107108
if (isNull)
@@ -189,9 +190,10 @@ ExecLockRows(LockRowsState *node)
189190
* so as to avoid overhead in the common case where there are no
190191
* concurrent updates.)
191192
*/
192-
foreach(l 10000 c, node->lr_rowMarks)
193+
foreach(lc, node->lr_arowMarks)
193194
{
194-
ExecRowMark *erm = (ExecRowMark *) lfirst(lc);
195+
ExecAuxRowMark *aerm = (ExecAuxRowMark *) lfirst(lc);
196+
ExecRowMark *erm = aerm->rowmark;
195197
HeapTupleData tuple;
196198
Buffer buffer;
197199

@@ -251,6 +253,7 @@ ExecInitLockRows(LockRows *node, EState *estate, int eflags)
251253
{
252254
LockRowsState *lrstate;
253255
Plan *outerPlan = outerPlan(node);
256+
List *epq_arowmarks;
254257
ListCell *lc;
255258

256259
/* check for unsupported flags */
@@ -262,7 +265,6 @@ ExecInitLockRows(LockRows *node, EState *estate, int eflags)
262265
lrstate = makeNode(LockRowsState);
263266
lrstate->ps.plan = (Plan *) node;
264267
lrstate->ps.state = estate;
265-
EvalPlanQualInit(&lrstate->lr_epqstate, estate, outerPlan, node->epqParam);
266268

267269
/*
268270
* Miscellaneous initialization
@@ -288,32 +290,27 @@ ExecInitLockRows(LockRows *node, EState *estate, int eflags)
288290
lrstate->ps.ps_ProjInfo = NULL;
289291

290292
/*
291-
* Locate the ExecRowMark(s) that this node is responsible for. (InitPlan
292-
* should already have built the global list of ExecRowMarks.)
293+
* Locate the ExecRowMark(s) that this node is responsible for, and
294+
* construct ExecAuxRowMarks for them. (InitPlan should already have
295+
* built the global list of ExecRowMarks.)
293296
*/
294-
lrstate->lr_rowMarks = NIL;
297+
lrstate->lr_arowMarks = NIL;
298+
epq_arowmarks = NIL;
295299
foreach(lc, node->rowMarks)
296300
{
297301
PlanRowMark *rc = (PlanRowMark *) lfirst(lc);
298-
ExecRowMark *erm = NULL;
299-
ListCell *lce;
302+
ExecRowMark *erm;
303+
ExecAuxRowMark *aerm;
300304

301305
Assert(IsA(rc, PlanRowMark));
302306

303307
/* ignore "parent" rowmarks; they are irrelevant at runtime */
304308
if (rc->isParent)
305309
continue;
306310

307-
foreach(lce, estate->es_rowMarks)
308-
{
309-
erm = (ExecRowMark *) lfirst(lce);
310-
if (erm->rti == rc->rti)
311-
break;
312-
erm = NULL;
313-
}
314-
if (erm == NULL)
315-
elog(ERROR, "failed to find ExecRowMark for PlanRowMark %u",
316-
rc->rti);
311+
/* find ExecRowMark and build ExecAuxRowMark */
312+
erm = ExecFindRowMark(estate, rc->rti);
313+
aerm = ExecBuildAuxRowMark(erm, outerPlan->targetlist);
317314

318315
/*
319316
* Only locking rowmarks go into our own list. Non-locking marks are
@@ -322,11 +319,15 @@ ExecInitLockRows(LockRows *node, EState *estate, int eflags)
322319
* do an EPQ recheck.
323320
*/
324321
if (RowMarkRequiresRowShareLock(erm->markType))
325-
lrstate->lr_rowMarks = lappend(lrstate->lr_rowMarks, erm);
322+
lrstate->lr_arowMarks = lappend(lrstate->lr_arowMarks, aerm);
326323
else
327-
EvalPlanQualAddRowMark(&lrstate->lr_epqstate, erm);
324+
epq_arowmarks = lappend(epq_arowmarks, aerm);
328325
}
329326

327+
/* Now we have the info needed to set up EPQ state */
328+
EvalPlanQualInit(&lrstate->lr_epqstate, estate,
329+
outerPlan, epq_arowmarks, node->epqParam);
330+
330331
return lrstate;
331332
}
332333

0 commit comments

Comments
 (0)
0