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.

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 7ef01c2effd938a326da2c368322d47f83bc4290..7f9addd41354546baced02363ae60caebd6b8e7a 100644 (file)
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -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 c4ef07ab0082668086cf764583bf3a9f39d12a15..fde5203ef4f58be92fbdde4a3cdf7aec3a890784 100644 (file)
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -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 7555249eb388dfa624a96bd53fb1fcee8973cd47..0da3fb40af94b68aeb6bf4e87f06d5c907739846 100644 (file)
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -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 4dfd61cc96ed0ae41e12cffc199a2780c7893e55..ddd79d42ad746e24907b6d9229d7768df95114e7 100644 (file)
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -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 a66087e6108629734a80d9d94a3b9721d7ba9bc4..48841f631724fe2a59b51fb996ed6081cb6193e5 100644 (file)
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -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 1196d87362455e4db68aa6f769c5951be3ce8ce9..f08744d3b404eacfe5098cb6f0a035280405e6fa 100644 (file)
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -283,17 +283,6 @@ set_append_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
        int                     nattrs;
        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.
         *

diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c
index 3136410458a178ba63daff8af3d3036f1c90dd17..1d9003b55cd931f9bb48875dc3116ee88e0a9844 100644 (file)
--- a/src/backend/optimizer/prep/preptlist.c
+++ b/src/backend/optimizer/prep/preptlist.c
@@ -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 a65467d98318437294588132bc2b3e16ae5ba07e..2f301afaa57b4e7d4c1bb4a97b891e6a5e7dba28 100644 (file)
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -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;
@@ -1208,6 +1209,15 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
                return;
        }
 
+       /*
+        * 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
@@ -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 ac2c7ba84cf3a5de0bc101db05ed864184ba03dd..322c4bc9637e648de86fa36af1a0d977e442e19b 100644 (file)
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -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 1f72330631b9300d075bbae10af265656a32078f..376c3f6667313a0e53c4f2621499ba10e6c9b1b1 100644 (file)
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -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 a9c39b7aef4e6c4d6810e7fe4624091fdf508911..76b174749a115fb8378935c3b0126a252ac70b7e 100644 (file)
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -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 fe744590509ed42e13ee905ab078df585674db1f..349ba772de8a5ba003fe4567b1b3dfff274dff03 100644 (file)
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -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 b442121050063827792dbe09b7ef328d05775bd0..fff7379c7fac09b961f0d2c706322ba4154be167 100644 (file)
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -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 63b8a8be566fcbea554690641fe080d03a07c2a5..66563615d88cefcf7a5087de79dfc19525042276 100644 (file)
--- 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 63a689666a616ea04b2c04e40363b1dd50df27c1..b53eaac786a1cb138f6fb2a9ef26857a7c8a5df6 100644 (file)
--- 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;