Skip to content

Commit 0656ed3

Browse files
tglsfdctglsfdc
committed
Make SELECT FOR UPDATE/SHARE work on inheritance trees, by having the plan
return the tableoid as well as the ctid for any FOR UPDATE targets that have child tables. All child tables are listed in the ExecRowMark list, but the executor just skips the ones that didn't produce the current row. Curiously, this longstanding restriction doesn't seem to have been documented anywhere; so no doc changes.
1 parent 07c179a commit 0656ed3

File tree

15 files changed

+150
-41
lines changed

15 files changed

+150
-41
lines changed

‎src/backend/executor/execMain.c

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
*
2727
*
2828
* IDENTIFICATION
29-
* $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.315 2008/11/06 20:51:14 tgl Exp $
29+
* $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.316 2008/11/15 19:43:45 tgl Exp $
3030
*
3131
*-------------------------------------------------------------------------
3232
*/
@@ -590,18 +590,25 @@ InitPlan(QueryDesc *queryDesc, int eflags)
590590
foreach(l, plannedstmt->rowMarks)
591591
{
592592
RowMarkClause *rc = (RowMarkClause *) lfirst(l);
593-
Oid relid = getrelid(rc->rti, rangeTable);
593+
Oid relid;
594594
Relation relation;
595595
ExecRowMark *erm;
596596

597+
/* ignore "parent" rowmarks; they are irrelevant at runtime */
598+
if (rc->isParent)
599+
continue;
600+
601+
relid = getrelid(rc->rti, rangeTable);
597602
relation = heap_open(relid, RowShareLock);
598603
erm = (ExecRowMark *) palloc(sizeof(ExecRowMark));
599604
erm->relation = relation;
600605
erm->rti = rc->rti;
606+
erm->prti = rc->prti;
601607
erm->forUpdate = rc->forUpdate;
602608
erm->noWait = rc->noWait;
603-
/* We'll set up ctidAttno below */
609+
/* We'll locate the junk attrs below */
604610
erm->ctidAttNo = InvalidAttrNumber;
611+
erm->toidAttNo = InvalidAttrNumber;
605612
estate->es_rowMarks = lappend(estate->es_rowMarks, erm);
606613
}
607614

@@ -822,17 +829,29 @@ InitPlan(QueryDesc *queryDesc, int eflags)
822829
elog(ERROR, "could not find junk ctid column");
823830
}
824831

825-
/* For SELECT FOR UPDATE/SHARE, find the ctid attrs now */
832+
/* For SELECT FOR UPDATE/SHARE, find the junk attrs now */
826833
foreach(l, estate->es_rowMarks)
827834
{
828835
ExecRowMark *erm = (ExecRowMark *) lfirst(l);
829836
char resname[32];
830837

831-
snprintf(resname, sizeof(resname), "ctid%u", erm->rti);
838+
/* always need the ctid */
839+
snprintf(resname, sizeof(resname), "ctid%u",
840+
erm->prti);
832841
erm->ctidAttNo = ExecFindJunkAttribute(j, resname);
833842
if (!AttributeNumberIsValid(erm->ctidAttNo))
834843
elog(ERROR, "could not find junk \"%s\" column",
835844
resname);
845+
/* if child relation, need tableoid too */
846+
if (erm->rti != erm->prti)
847+
{
848+
snprintf(resname, sizeof(resname), "tableoid%u",
849+
erm->prti);
850+
erm->toidAttNo = ExecFindJunkAttribute(j, resname);
851+
if (!AttributeNumberIsValid(erm->toidAttNo))
852+
elog(ERROR, "could not find junk \"%s\" column",
853+
resname);
854+
}
836855
}
837856
}
838857
}
@@ -1383,13 +1402,33 @@ lnext: ;
13831402
LockTupleMode lockmode;
13841403
HTSU_Result test;
13851404

1405+
/* if child rel, must check whether it produced this row */
1406+
if (erm->rti != erm->prti)
1407+
{
1408+
Oid tableoid;
1409+
1410+
datum = ExecGetJunkAttribute(slot,
1411+
erm->toidAttNo,
1412+
&isNull);
1413+
/* shouldn't ever get a null result... */
1414+
if (isNull)
1415+
elog(ERROR, "tableoid is NULL");
1416+
tableoid = DatumGetObjectId(datum);
1417+
1418+
if (tableoid != RelationGetRelid(erm->relation))
1419+
{
1420+
/* this child is inactive right now */
1421+
continue;
1422+
}
1423+
}
1424+
1425+
/* okay, fetch the tuple by ctid */
13861426
datum = ExecGetJunkAttribute(slot,
13871427
erm->ctidAttNo,
13881428
&isNull);
13891429
/* shouldn't ever get a null result... */
13901430
if (isNull)
13911431
elog(ERROR, "ctid is NULL");
1392-
13931432
tuple.t_self = *((ItemPointer) DatumGetPointer(datum));
13941433

13951434
if (erm->forUpdate)
@@ -2122,9 +2161,11 @@ EvalPlanQual(EState *estate, Index rti,
21222161
relation = NULL;
21232162
foreach(l, estate->es_rowMarks)
21242163
{
2125-
if (((ExecRowMark *) lfirst(l))->rti == rti)
2164+
ExecRowMark *erm = lfirst(l);
2165+
2166+
if (erm->rti == rti)
21262167
{
2127-
relation = ((ExecRowMark *) lfirst(l))->relation;
2168+
relation = erm->relation;
21282169
break;
21292170
}
21302171
}

‎src/backend/nodes/copyfuncs.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* Portions Copyright (c) 1994, Regents of the University of California
1616
*
1717
* IDENTIFICATION
18-
* $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.411 2008/11/11 18:13:32 tgl Exp $
18+
* $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.412 2008/11/15 19:43:46 tgl Exp $
1919
*
2020
*-------------------------------------------------------------------------
2121
*/
@@ -1735,8 +1735,10 @@ _copyRowMarkClause(RowMarkClause *from)
17351735
RowMarkClause *newnode = makeNode(RowMarkClause);
17361736

17371737
COPY_SCALAR_FIELD(rti);
1738+
COPY_SCALAR_FIELD(prti);
17381739
COPY_SCALAR_FIELD(forUpdate);
17391740
COPY_SCALAR_FIELD(noWait);
1741+
COPY_SCALAR_FIELD(isParent);
17401742

17411743
return newnode;
17421744
}

‎src/backend/nodes/equalfuncs.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
* Portions Copyright (c) 1994, Regents of the University of California
2323
*
2424
* IDENTIFICATION
25-
* $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.336 2008/11/11 18:13:32 tgl Exp $
25+
* $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.337 2008/11/15 19:43:46 tgl Exp $
2626
*
2727
*-------------------------------------------------------------------------
2828
*/
@@ -2005,8 +2005,10 @@ static bool
20052005
_equalRowMarkClause(RowMarkClause *a, RowMarkClause *b)
20062006
{
20072007
COMPARE_SCALAR_FIELD(rti);
2008+
COMPARE_SCALAR_FIELD(prti);
20082009
COMPARE_SCALAR_FIELD(forUpdate);
20092010
COMPARE_SCALAR_FIELD(noWait);
2011+
COMPARE_SCALAR_FIELD(isParent);
20102012

20112013
return true;
20122014
}

‎src/backend/nodes/outfuncs.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.344 2008/11/11 18:13:32 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.345 2008/11/15 19:43:46 tgl Exp $
1212
*
1313
* NOTES
1414
* Every node type that can appear in stored rules' parsetrees *must*
@@ -1900,8 +1900,10 @@ _outRowMarkClause(StringInfo str, RowMarkClause *node)
19001900
WRITE_NODE_TYPE("ROWMARKCLAUSE");
19011901

19021902
WRITE_UINT_FIELD(rti);
1903+
WRITE_UINT_FIELD(prti);
19031904
WRITE_BOOL_FIELD(forUpdate);
19041905
WRITE_BOOL_FIELD(noWait);
1906+
WRITE_BOOL_FIELD(isParent);
19051907
}
19061908

19071909
static void

‎src/backend/nodes/readfuncs.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/nodes/readfuncs.c,v 1.216 2008/10/06 17:39:26 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/nodes/readfuncs.c,v 1.217 2008/11/15 19:43:46 tgl Exp $
1212
*
1313
* NOTES
1414
* Path and Plan nodes do not have any readfuncs support, because we
@@ -226,8 +226,10 @@ _readRowMarkClause(void)
226226
READ_LOCALS(RowMarkClause);
227227

228228
READ_UINT_FIELD(rti);
229+
READ_UINT_FIELD(prti);
229230
READ_BOOL_FIELD(forUpdate);
230231
READ_BOOL_FIELD(noWait);
232+
READ_BOOL_FIELD(isParent);
231233

232234
READ_DONE();
233235
}

‎src/backend/optimizer/path/allpaths.c

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.176 2008/11/11 18:13:32 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.177 2008/11/15 19:43:46 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -283,17 +283,6 @@ set_append_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
283283
int nattrs;
284284
ListCell *l;
285285

286-
/*
287-
* XXX for now, can't handle inherited expansion of FOR UPDATE/SHARE; can
288-
* we do better? (This will take some redesign because the executor
289-
* currently supposes that every rowMark relation is involved in every row
290-
* returned by the query.)
291-
*/
292-
if (get_rowmark(root->parse, parentRTindex))
293-
ereport(ERROR,
294-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
295-
errmsg("SELECT FOR UPDATE/SHARE is not supported for inheritance queries")));
296-
297286
/*
298287
* Initialize to compute size estimates for whole append relation.
299288
*

‎src/backend/optimizer/prep/preptlist.c

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
* Portions Copyright (c) 1994, Regents of the University of California
1717
*
1818
* IDENTIFICATION
19-
* $PostgreSQL: pgsql/src/backend/optimizer/prep/preptlist.c,v 1.93 2008/11/02 01:45:28 tgl Exp $
19+
* $PostgreSQL: pgsql/src/backend/optimizer/prep/preptlist.c,v 1.94 2008/11/15 19:43:46 tgl Exp $
2020
*
2121
*-------------------------------------------------------------------------
2222
*/
@@ -138,6 +138,11 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
138138
char *resname;
139139
TargetEntry *tle;
140140

141+
/* ignore child rels */
142+
if (rc->rti != rc->prti)
143+
continue;
144+
145+
/* always need the ctid */
141146
var = makeVar(rc->rti,
142147
SelfItemPointerAttributeNumber,
143148
TIDOID,
@@ -153,6 +158,26 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
153158
true);
154159

155160
tlist = lappend(tlist, tle);
161+
162+
/* if parent of inheritance tree, need the tableoid too */
163+
if (rc->isParent)
164+
{
165+
var = makeVar(rc->rti,
166+
TableOidAttributeNumber,
167+
OIDOID,
168+
-1,
169+
0);
170+
171+
resname = (char *) palloc(32);
172+
snprintf(resname, 32, "tableoid%u", rc->rti);
173+
174+
tle = makeTargetEntry((Expr *) var,
175+
list_length(tlist) + 1,
176+
resname,
177+
true);
178+
179+
tlist = lappend(tlist, tle);
180+
}
156181
}
157182
}
158183

‎src/backend/optimizer/prep/prepunion.c

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
*
2323
*
2424
* IDENTIFICATION
25-
* $PostgreSQL: pgsql/src/backend/optimizer/prep/prepunion.c,v 1.161 2008/11/11 18:13:32 tgl Exp $
25+
* $PostgreSQL: pgsql/src/backend/optimizer/prep/prepunion.c,v 1.162 2008/11/15 19:43:46 tgl Exp $
2626
*
2727
*-------------------------------------------------------------------------
2828
*/
@@ -1169,6 +1169,7 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
11691169
{
11701170
Query *parse = root->parse;
11711171
Oid parentOID;
1172+
RowMarkClause *oldrc;
11721173
Relation oldrelation;
11731174
LOCKMODE lockmode;
11741175
List *inhOIDs;
@@ -1208,6 +1209,15 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
12081209
return;
12091210
}
12101211

1212+
/*
1213+
* Find out if parent relation is selected FOR UPDATE/SHARE. If so,
1214+
* we need to mark its RowMarkClause as isParent = true, and generate
1215+
* a new RowMarkClause for each child.
1216+
*/
1217+
oldrc = get_rowmark(parse, rti);
1218+
if (oldrc)
1219+
oldrc->isParent = true;
1220+
12111221
/*
12121222
* Must open the parent relation to examine its tupdesc. We need not lock
12131223
* it since the rewriter already obtained at least AccessShareLock on each
@@ -1221,14 +1231,15 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
12211231
* in the parse/rewrite/plan pipeline.
12221232
*
12231233
* If the parent relation is the query's result relation, then we need
1224-
* RowExclusiveLock. Otherwise, check to see if the relation is accessed
1225-
* FOR UPDATE/SHARE or not. We can't just grab AccessShareLock because
1226-
* then the executor would be trying to upgrade the lock, leading to
1227-
* possible deadlocks. (This code should match the parser and rewriter.)
1234+
* RowExclusiveLock. Otherwise, if it's accessed FOR UPDATE/SHARE, we
1235+
* need RowShareLock; otherwise AccessShareLock. We can't just grab
1236+
* AccessShareLock because then the executor would be trying to upgrade
1237+
* the lock, leading to possible deadlocks. (This code should match the
1238+
* parser and rewriter.)
12281239
*/
12291240
if (rti == parse->resultRelation)
12301241
lockmode = RowExclusiveLock;
1231-
else if (get_rowmark(parse, rti))
1242+
else if (oldrc)
12321243
lockmode = RowShareLock;
12331244
else
12341245
lockmode = AccessShareLock;
@@ -1283,6 +1294,22 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
12831294
appinfo->parent_reloid = parentOID;
12841295
appinfos = lappend(appinfos, appinfo);
12851296

1297+
/*
1298+
* Build a RowMarkClause if parent is marked FOR UPDATE/SHARE.
1299+
*/
1300+
if (oldrc)
1301+
{
1302+
RowMarkClause *newrc = makeNode(RowMarkClause);
1303+
1304+
newrc->rti = childRTindex;
1305+
newrc->prti = rti;
1306+
newrc->forUpdate = oldrc->forUpdate;
1307+
newrc->noWait = oldrc->noWait;
1308+
newrc->isParent = false;
1309+
1310+
parse->rowMarks = lappend(parse->rowMarks, newrc);
1311+
}
1312+
12861313
/* Close child relations, but keep locks */
12871314
if (childOID != parentOID)
12881315
heap_close(newrelation, NoLock);

‎src/backend/parser/analyze.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
1818
* Portions Copyright (c) 1994, Regents of the University of California
1919
*
20-
* $PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.382 2008/10/07 01:47:54 tgl Exp $
20+
* $PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.383 2008/11/15 19:43:46 tgl Exp $
2121
*
2222
*-------------------------------------------------------------------------
2323
*/
@@ -2049,8 +2049,10 @@ applyLockingClause(Query *qry, Index rtindex, bool forUpdate, bool noWait)
20492049
/* Make a new RowMarkClause */
20502050
rc = makeNode(RowMarkClause);
20512051
rc->rti = rtindex;
2052+
rc->prti = rtindex;
20522053
rc->forUpdate = forUpdate;
20532054
rc->noWait = noWait;
2055+
rc->isParent = false;
20542056
qry->rowMarks = lappend(qry->rowMarks, rc);
20552057
}
20562058

‎src/backend/rewrite/rewriteManip.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $PostgreSQL: pgsql/src/backend/rewrite/rewriteManip.c,v 1.117 2008/10/22 20:17:52 tgl Exp $
10+
* $PostgreSQL: pgsql/src/backend/rewrite/rewriteManip.c,v 1.118 2008/11/15 19:43:46 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -352,6 +352,7 @@ OffsetVarNodes(Node *node, int offset, int sublevels_up)
352352
RowMarkClause *rc = (RowMarkClause *) lfirst(l);
353353

354354
rc->rti += offset;
355+
rc->prti += offset;
355356
}
356357
}
357358
query_tree_walker(qry, OffsetVarNodes_walker,
@@ -536,6 +537,8 @@ ChangeVarNodes(Node *node, int rt_index, int new_index, int sublevels_up)
536537

537538
if (rc->rti == rt_index)
538539
rc->rti = new_index;
540+
if (rc->prti == rt_index)
541+
rc->prti = new_index;
539542
}
540543
}
541544
query_tree_walker(qry, ChangeVarNodes_walker,

0 commit comments

Comments
 (0)