diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2008-11-15 19:43:47 +0000 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2008-11-15 19:43:47 +0000 |
commit | 0656ed3daa29b00c7c41ad44b407a7165f83d453 (patch) | |
tree | 2167bd78fa501c3ffed1d0de842ded643c32500b /src | |
parent | 07c179a82b39ffbc172175382717706d90c714cd (diff) | |
download | postgresql-0656ed3daa29b00c7c41ad44b407a7165f83d453.tar.gz postgresql-0656ed3daa29b00c7c41ad44b407a7165f83d453.zip |
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.
Diffstat (limited to 'src')
-rw-r--r-- | src/backend/executor/execMain.c | 57 | ||||
-rw-r--r-- | src/backend/nodes/copyfuncs.c | 4 | ||||
-rw-r--r-- | src/backend/nodes/equalfuncs.c | 4 | ||||
-rw-r--r-- | src/backend/nodes/outfuncs.c | 4 | ||||
-rw-r--r-- | src/backend/nodes/readfuncs.c | 4 | ||||
-rw-r--r-- | src/backend/optimizer/path/allpaths.c | 13 | ||||
-rw-r--r-- | src/backend/optimizer/prep/preptlist.c | 27 | ||||
-rw-r--r-- | src/backend/optimizer/prep/prepunion.c | 39 | ||||
-rw-r--r-- | src/backend/parser/analyze.c | 4 | ||||
-rw-r--r-- | src/backend/rewrite/rewriteManip.c | 5 | ||||
-rw-r--r-- | src/include/catalog/catversion.h | 4 | ||||
-rw-r--r-- | src/include/nodes/execnodes.h | 9 | ||||
-rw-r--r-- | src/include/nodes/parsenodes.h | 13 | ||||
-rw-r--r-- | src/test/regress/expected/portals.out | 2 | ||||
-rw-r--r-- | src/test/regress/sql/portals.sql | 2 |
15 files changed, 150 insertions, 41 deletions
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 350381ad4b5..634ca69b4d1 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -26,7 +26,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.315 2008/11/06 20:51:14 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.316 2008/11/15 19:43:45 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -590,18 +590,25 @@ InitPlan(QueryDesc *queryDesc, int eflags) foreach(l, plannedstmt->rowMarks) { RowMarkClause *rc = (RowMarkClause *) lfirst(l); - Oid relid = getrelid(rc->rti, rangeTable); + Oid relid; Relation relation; ExecRowMark *erm; + /* ignore "parent" rowmarks; they are irrelevant at runtime */ + if (rc->isParent) + continue; + + relid = getrelid(rc->rti, rangeTable); relation = heap_open(relid, RowShareLock); erm = (ExecRowMark *) palloc(sizeof(ExecRowMark)); erm->relation = relation; erm->rti = rc->rti; + erm->prti = rc->prti; erm->forUpdate = rc->forUpdate; erm->noWait = rc->noWait; - /* We'll set up ctidAttno below */ + /* We'll locate the junk attrs below */ erm->ctidAttNo = InvalidAttrNumber; + erm->toidAttNo = InvalidAttrNumber; estate->es_rowMarks = lappend(estate->es_rowMarks, erm); } @@ -822,17 +829,29 @@ InitPlan(QueryDesc *queryDesc, int eflags) elog(ERROR, "could not find junk ctid column"); } - /* For SELECT FOR UPDATE/SHARE, find the ctid attrs now */ + /* For SELECT FOR UPDATE/SHARE, find the junk attrs now */ foreach(l, estate->es_rowMarks) { ExecRowMark *erm = (ExecRowMark *) lfirst(l); char resname[32]; - snprintf(resname, sizeof(resname), "ctid%u", erm->rti); + /* always need the ctid */ + snprintf(resname, sizeof(resname), "ctid%u", + erm->prti); erm->ctidAttNo = ExecFindJunkAttribute(j, resname); if (!AttributeNumberIsValid(erm->ctidAttNo)) elog(ERROR, "could not find junk \"%s\" column", resname); + /* if child relation, need tableoid too */ + if (erm->rti != erm->prti) + { + snprintf(resname, sizeof(resname), "tableoid%u", + erm->prti); + erm->toidAttNo = ExecFindJunkAttribute(j, resname); + if (!AttributeNumberIsValid(erm->toidAttNo)) + elog(ERROR, "could not find junk \"%s\" column", + resname); + } } } } @@ -1383,13 +1402,33 @@ lnext: ; LockTupleMode lockmode; HTSU_Result test; + /* if child rel, must check whether it produced this row */ + if (erm->rti != erm->prti) + { + Oid tableoid; + + datum = ExecGetJunkAttribute(slot, + erm->toidAttNo, + &isNull); + /* shouldn't ever get a null result... */ + if (isNull) + elog(ERROR, "tableoid is NULL"); + tableoid = DatumGetObjectId(datum); + + if (tableoid != RelationGetRelid(erm->relation)) + { + /* this child is inactive right now */ + continue; + } + } + + /* okay, fetch the tuple by ctid */ datum = ExecGetJunkAttribute(slot, erm->ctidAttNo, &isNull); /* shouldn't ever get a null result... */ if (isNull) elog(ERROR, "ctid is NULL"); - tuple.t_self = *((ItemPointer) DatumGetPointer(datum)); if (erm->forUpdate) @@ -2122,9 +2161,11 @@ EvalPlanQual(EState *estate, Index rti, relation = NULL; foreach(l, estate->es_rowMarks) { - if (((ExecRowMark *) lfirst(l))->rti == rti) + ExecRowMark *erm = lfirst(l); + + if (erm->rti == rti) { - relation = ((ExecRowMark *) lfirst(l))->relation; + relation = erm->relation; break; } } diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index b26fb827aa2..ec3c591b435 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -15,7 +15,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.411 2008/11/11 18:13:32 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.412 2008/11/15 19:43:46 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1735,8 +1735,10 @@ _copyRowMarkClause(RowMarkClause *from) RowMarkClause *newnode = makeNode(RowMarkClause); COPY_SCALAR_FIELD(rti); + COPY_SCALAR_FIELD(prti); COPY_SCALAR_FIELD(forUpdate); COPY_SCALAR_FIELD(noWait); + COPY_SCALAR_FIELD(isParent); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 01cb7d94c69..a23ef4b03ea 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -22,7 +22,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.336 2008/11/11 18:13:32 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.337 2008/11/15 19:43:46 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -2005,8 +2005,10 @@ static bool _equalRowMarkClause(RowMarkClause *a, RowMarkClause *b) { COMPARE_SCALAR_FIELD(rti); + COMPARE_SCALAR_FIELD(prti); COMPARE_SCALAR_FIELD(forUpdate); COMPARE_SCALAR_FIELD(noWait); + COMPARE_SCALAR_FIELD(isParent); return true; } diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index c8febd273ba..f64553bbf87 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.344 2008/11/11 18:13:32 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.345 2008/11/15 19:43:46 tgl Exp $ * * NOTES * Every node type that can appear in stored rules' parsetrees *must* @@ -1900,8 +1900,10 @@ _outRowMarkClause(StringInfo str, RowMarkClause *node) WRITE_NODE_TYPE("ROWMARKCLAUSE"); WRITE_UINT_FIELD(rti); + WRITE_UINT_FIELD(prti); WRITE_BOOL_FIELD(forUpdate); WRITE_BOOL_FIELD(noWait); + WRITE_BOOL_FIELD(isParent); } static void diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index d48715d36bb..ed5b55fb571 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/nodes/readfuncs.c,v 1.216 2008/10/06 17:39:26 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/nodes/readfuncs.c,v 1.217 2008/11/15 19:43:46 tgl Exp $ * * NOTES * Path and Plan nodes do not have any readfuncs support, because we @@ -226,8 +226,10 @@ _readRowMarkClause(void) READ_LOCALS(RowMarkClause); READ_UINT_FIELD(rti); + READ_UINT_FIELD(prti); READ_BOOL_FIELD(forUpdate); READ_BOOL_FIELD(noWait); + READ_BOOL_FIELD(isParent); READ_DONE(); } diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 4d868570bcb..b0553894c24 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.176 2008/11/11 18:13:32 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.177 2008/11/15 19:43:46 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -284,17 +284,6 @@ set_append_rel_pathlist(PlannerInfo *root, RelOptInfo *rel, ListCell *l; /* - * XXX for now, can't handle inherited expansion of FOR UPDATE/SHARE; can - * we do better? (This will take some redesign because the executor - * currently supposes that every rowMark relation is involved in every row - * returned by the query.) - */ - if (get_rowmark(root->parse, parentRTindex)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("SELECT FOR UPDATE/SHARE is not supported for inheritance queries"))); - - /* * Initialize to compute size estimates for whole append relation. * * We handle width estimates by weighting the widths of different diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c index 2861125212f..e04e7067664 100644 --- a/src/backend/optimizer/prep/preptlist.c +++ b/src/backend/optimizer/prep/preptlist.c @@ -16,7 +16,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/prep/preptlist.c,v 1.93 2008/11/02 01:45:28 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/prep/preptlist.c,v 1.94 2008/11/15 19:43:46 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -138,6 +138,11 @@ preprocess_targetlist(PlannerInfo *root, List *tlist) char *resname; TargetEntry *tle; + /* ignore child rels */ + if (rc->rti != rc->prti) + continue; + + /* always need the ctid */ var = makeVar(rc->rti, SelfItemPointerAttributeNumber, TIDOID, @@ -153,6 +158,26 @@ preprocess_targetlist(PlannerInfo *root, List *tlist) true); tlist = lappend(tlist, tle); + + /* if parent of inheritance tree, need the tableoid too */ + if (rc->isParent) + { + var = makeVar(rc->rti, + TableOidAttributeNumber, + OIDOID, + -1, + 0); + + resname = (char *) palloc(32); + snprintf(resname, 32, "tableoid%u", rc->rti); + + tle = makeTargetEntry((Expr *) var, + list_length(tlist) + 1, + resname, + true); + + tlist = lappend(tlist, tle); + } } } diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index c2113cb2669..bd7c05cc53d 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -22,7 +22,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/prep/prepunion.c,v 1.161 2008/11/11 18:13:32 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/prep/prepunion.c,v 1.162 2008/11/15 19:43:46 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1169,6 +1169,7 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) { Query *parse = root->parse; Oid parentOID; + RowMarkClause *oldrc; Relation oldrelation; LOCKMODE lockmode; List *inhOIDs; @@ -1209,6 +1210,15 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) } /* + * Find out if parent relation is selected FOR UPDATE/SHARE. If so, + * we need to mark its RowMarkClause as isParent = true, and generate + * a new RowMarkClause for each child. + */ + oldrc = get_rowmark(parse, rti); + if (oldrc) + oldrc->isParent = true; + + /* * Must open the parent relation to examine its tupdesc. We need not lock * it since the rewriter already obtained at least AccessShareLock on each * relation used in the query. @@ -1221,14 +1231,15 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) * in the parse/rewrite/plan pipeline. * * If the parent relation is the query's result relation, then we need - * RowExclusiveLock. Otherwise, check to see if the relation is accessed - * FOR UPDATE/SHARE or not. We can't just grab AccessShareLock because - * then the executor would be trying to upgrade the lock, leading to - * possible deadlocks. (This code should match the parser and rewriter.) + * RowExclusiveLock. Otherwise, if it's accessed FOR UPDATE/SHARE, we + * need RowShareLock; otherwise AccessShareLock. We can't just grab + * AccessShareLock because then the executor would be trying to upgrade + * the lock, leading to possible deadlocks. (This code should match the + * parser and rewriter.) */ if (rti == parse->resultRelation) lockmode = RowExclusiveLock; - else if (get_rowmark(parse, rti)) + else if (oldrc) lockmode = RowShareLock; else lockmode = AccessShareLock; @@ -1283,6 +1294,22 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) appinfo->parent_reloid = parentOID; appinfos = lappend(appinfos, appinfo); + /* + * Build a RowMarkClause if parent is marked FOR UPDATE/SHARE. + */ + if (oldrc) + { + RowMarkClause *newrc = makeNode(RowMarkClause); + + newrc->rti = childRTindex; + newrc->prti = rti; + newrc->forUpdate = oldrc->forUpdate; + newrc->noWait = oldrc->noWait; + newrc->isParent = false; + + parse->rowMarks = lappend(parse->rowMarks, newrc); + } + /* Close child relations, but keep locks */ if (childOID != parentOID) heap_close(newrelation, NoLock); diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 2a99d099d8e..21694ea56e5 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -17,7 +17,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.382 2008/10/07 01:47:54 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.383 2008/11/15 19:43:46 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -2049,8 +2049,10 @@ applyLockingClause(Query *qry, Index rtindex, bool forUpdate, bool noWait) /* Make a new RowMarkClause */ rc = makeNode(RowMarkClause); rc->rti = rtindex; + rc->prti = rtindex; rc->forUpdate = forUpdate; rc->noWait = noWait; + rc->isParent = false; qry->rowMarks = lappend(qry->rowMarks, rc); } diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c index 0322c58f3f0..954e21af181 100644 --- a/src/backend/rewrite/rewriteManip.c +++ b/src/backend/rewrite/rewriteManip.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/rewrite/rewriteManip.c,v 1.117 2008/10/22 20:17:52 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/rewrite/rewriteManip.c,v 1.118 2008/11/15 19:43:46 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -352,6 +352,7 @@ OffsetVarNodes(Node *node, int offset, int sublevels_up) RowMarkClause *rc = (RowMarkClause *) lfirst(l); rc->rti += offset; + rc->prti += offset; } } query_tree_walker(qry, OffsetVarNodes_walker, @@ -536,6 +537,8 @@ ChangeVarNodes(Node *node, int rt_index, int new_index, int sublevels_up) if (rc->rti == rt_index) rc->rti = new_index; + if (rc->prti == rt_index) + rc->prti = new_index; } } query_tree_walker(qry, ChangeVarNodes_walker, diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index f156b1c36da..b3a84bd7fde 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -37,7 +37,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/catalog/catversion.h,v 1.507 2008/11/14 02:09:51 tgl Exp $ + * $PostgreSQL: pgsql/src/include/catalog/catversion.h,v 1.508 2008/11/15 19:43:46 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 200811133 +#define CATALOG_VERSION_NO 200811151 #endif diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 3fe16bd0983..8c8742e286e 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.194 2008/10/31 19:37:56 tgl Exp $ + * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.195 2008/11/15 19:43:46 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -368,14 +368,19 @@ typedef struct EState } EState; -/* es_rowMarks is a list of these structs: */ +/* + * es_rowMarks is a list of these structs. See RowMarkClause for details + * about rti and prti. toidAttno is not used in a "plain" rowmark. + */ typedef struct ExecRowMark { Relation relation; /* opened and RowShareLock'd relation */ Index rti; /* its range table index */ + Index prti; /* parent range table index, if child */ bool forUpdate; /* true = FOR UPDATE, false = FOR SHARE */ bool noWait; /* NOWAIT option */ AttrNumber ctidAttNo; /* resno of its ctid junk attribute */ + AttrNumber toidAttNo; /* resno of tableoid junk attribute, if any */ } ExecRowMark; diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 5e112d178ba..1edd094dbf6 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -13,7 +13,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.377 2008/10/31 08:39:22 heikki Exp $ + * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.378 2008/11/15 19:43:46 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -700,14 +700,23 @@ typedef struct SortGroupClause * RowMarkClause - * representation of FOR UPDATE/SHARE clauses * - * We create a separate RowMarkClause node for each target relation + * We create a separate RowMarkClause node for each target relation. In the + * output of the parser and rewriter, all RowMarkClauses have rti == prti and + * isParent == false. When the planner discovers that a target relation + * is the root of an inheritance tree, it sets isParent true, and adds an + * additional RowMarkClause to the list for each child relation (including + * the target rel itself in its role as a child). The child entries have + * rti == child rel's RT index, prti == parent's RT index, and can therefore + * be recognized as children by the fact that prti != rti. */ typedef struct RowMarkClause { NodeTag type; Index rti; /* range table index of target relation */ + Index prti; /* range table index of parent relation */ bool forUpdate; /* true = FOR UPDATE, false = FOR SHARE */ bool noWait; /* NOWAIT option */ + bool isParent; /* set by planner when expanding inheritance */ } RowMarkClause; /* diff --git a/src/test/regress/expected/portals.out b/src/test/regress/expected/portals.out index 63b8a8be566..66563615d88 100644 --- a/src/test/regress/expected/portals.out +++ b/src/test/regress/expected/portals.out @@ -1118,7 +1118,7 @@ SELECT * FROM uctest; (3 rows) BEGIN; -DECLARE c1 CURSOR FOR SELECT * FROM uctest; +DECLARE c1 CURSOR FOR SELECT * FROM uctest FOR UPDATE; FETCH 1 FROM c1; f1 | f2 ----+------- diff --git a/src/test/regress/sql/portals.sql b/src/test/regress/sql/portals.sql index 63a689666a6..b53eaac786a 100644 --- a/src/test/regress/sql/portals.sql +++ b/src/test/regress/sql/portals.sql @@ -393,7 +393,7 @@ INSERT INTO ucchild values(100, 'hundred'); SELECT * FROM uctest; BEGIN; -DECLARE c1 CURSOR FOR SELECT * FROM uctest; +DECLARE c1 CURSOR FOR SELECT * FROM uctest FOR UPDATE; FETCH 1 FROM c1; UPDATE uctest SET f1 = f1 + 10 WHERE CURRENT OF c1; FETCH 1 FROM c1; |