Fix improper matching of resjunk column names for FOR UPDATE in subselect.

Flattening of subquery range tables during setrefs.c could lead to the
rangetable indexes in PlanRowMark nodes not matching up with the column
names previously assigned to the corresponding resjunk ctid (resp. tableoid
or wholerow) columns.  Typical symptom would be either a "cannot extract
system attribute from virtual tuple" error or an Assert failure.  This
wasn't a problem before 9.0 because we didn't support FOR UPDATE below the
top query level, and so the final flattening could never renumber an RTE
that was relevant to FOR UPDATE.  Fix by using a plan-tree-wide unique
number for each PlanRowMark to label the associated resjunk columns, so
that the number need not change during flattening.

Per report from David Johnston (though I'm darned if I can see how this got
past initial testing of the relevant code).  Back-patch to 9.0.

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 6d4a77328a4f0610c996094e6ba8639d9aceeabf..c477314efa343a34f30e2a3810692ba7ee29d5ba 100644 (file)
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -726,6 +726,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
                erm->relation = relation;
                erm->rti = rc->rti;
                erm->prti = rc->prti;
+               erm->rowmarkId = rc->rowmarkId;
                erm->markType = rc->markType;
                erm->noWait = rc->noWait;
                ItemPointerSetInvalid(&(erm->curCtid));
@@ -1373,23 +1374,29 @@ ExecBuildAuxRowMark(ExecRowMark *erm, List *targetlist)
                /* if child rel, need tableoid */
                if (erm->rti != erm->prti)
                {
-                       snprintf(resname, sizeof(resname), "tableoid%u", erm->prti);
+                       snprintf(resname, sizeof(resname), "tableoid%u", erm->rowmarkId);
                        aerm->toidAttNo = ExecFindJunkAttributeInTlist(targetlist,
                                                                                                                   resname);
+                       if (!AttributeNumberIsValid(aerm->toidAttNo))
+                               elog(ERROR, "could not find junk %s column", resname);
                }
 
                /* always need ctid for real relations */
-               snprintf(resname, sizeof(resname), "ctid%u", erm->prti);
+               snprintf(resname, sizeof(resname), "ctid%u", erm->rowmarkId);
                aerm->ctidAttNo = ExecFindJunkAttributeInTlist(targetlist,
                                                                                                           resname);
+               if (!AttributeNumberIsValid(aerm->ctidAttNo))
+                       elog(ERROR, "could not find junk %s column", resname);
        }
        else
        {
                Assert(erm->markType == ROW_MARK_COPY);
 
-               snprintf(resname, sizeof(resname), "wholerow%u", erm->prti);
+               snprintf(resname, sizeof(resname), "wholerow%u", erm->rowmarkId);
                aerm->wholeAttNo = ExecFindJunkAttributeInTlist(targetlist,
                                                                                                                resname);
+               if (!AttributeNumberIsValid(aerm->wholeAttNo))
+                       elog(ERROR, "could not find junk %s column", resname);
        }
 
        return aerm;

diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 0b2aa3dfff8549d1eb9773dd32bf20c5d9aefc98..778c53063ec72da42f5e76c7a10706f6a759c058 100644 (file)
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -856,6 +856,7 @@ _copyPlanRowMark(PlanRowMark *from)
 
        COPY_SCALAR_FIELD(rti);
        COPY_SCALAR_FIELD(prti);
+       COPY_SCALAR_FIELD(rowmarkId);
        COPY_SCALAR_FIELD(markType);
        COPY_SCALAR_FIELD(noWait);
        COPY_SCALAR_FIELD(isParent);

diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 032150e1bb737edf9e367d6729545f43a1bc8bc4..15a9aa2f939328673a5ddee21135ee0b9d31c1db 100644 (file)
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -756,6 +756,7 @@ _outPlanRowMark(StringInfo str, PlanRowMark *node)
 
        WRITE_UINT_FIELD(rti);
        WRITE_UINT_FIELD(prti);
+       WRITE_UINT_FIELD(rowmarkId);
        WRITE_ENUM_FIELD(markType, RowMarkType);
        WRITE_BOOL_FIELD(noWait);
        WRITE_BOOL_FIELD(isParent);
@@ -1524,6 +1525,7 @@ _outPlannerGlobal(StringInfo str, PlannerGlobal *node)
        WRITE_NODE_FIELD(relationOids);
        WRITE_NODE_FIELD(invalItems);
        WRITE_UINT_FIELD(lastPHId);
+       WRITE_UINT_FIELD(lastRowMarkId);
        WRITE_BOOL_FIELD(transientPlan);
 }
 

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 45ba902fbab1bf6743b50d9f694cc9c818c3199f..55206573c7209d8abaa19a5731f25cb049e55290 100644 (file)
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -165,6 +165,7 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
        glob->relationOids = NIL;
        glob->invalItems = NIL;
        glob->lastPHId = 0;
+       glob->lastRowMarkId = 0;
        glob->transientPlan = false;
 
        /* Determine what fraction of the plan is likely to be scanned */
@@ -1852,6 +1853,7 @@ preprocess_rowmarks(PlannerInfo *root)
 
                newrc = makeNode(PlanRowMark);
                newrc->rti = newrc->prti = rc->rti;
+               newrc->rowmarkId = ++(root->glob->lastRowMarkId);
                if (rc->forUpdate)
                        newrc->markType = ROW_MARK_EXCLUSIVE;
                else
@@ -1877,6 +1879,7 @@ preprocess_rowmarks(PlannerInfo *root)
 
                newrc = makeNode(PlanRowMark);
                newrc->rti = newrc->prti = i;
+               newrc->rowmarkId = ++(root->glob->lastRowMarkId);
                /* real tables support REFERENCE, anything else needs COPY */
                if (rte->rtekind == RTE_RELATION)
                        newrc->markType = ROW_MARK_REFERENCE;

diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 70be2e66f2d20d7dcd7e64cc45fc98e9f37fcfa9..2618ef14a830ba232e6f20576f8517dd599d1405 100644 (file)
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -252,7 +252,7 @@ set_plan_references(PlannerGlobal *glob, Plan *plan,
                newrc = (PlanRowMark *) palloc(sizeof(PlanRowMark));
                memcpy(newrc, rc, sizeof(PlanRowMark));
 
-               /* adjust indexes */
+               /* adjust indexes ... but *not* the rowmarkId */
                newrc->rti += rtoffset;
                newrc->prti += rtoffset;
 

diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c
index 7a09d1f830999acf6266375d738a2ea156974667..425d80c62442834bdb10b41f8ffa2989b5e79f80 100644 (file)
--- a/src/backend/optimizer/prep/preptlist.c
+++ b/src/backend/optimizer/prep/preptlist.c
@@ -132,7 +132,7 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
                                                  TIDOID,
                                                  -1,
                                                  0);
-                       snprintf(resname, sizeof(resname), "ctid%u", rc->rti);
+                       snprintf(resname, sizeof(resname), "ctid%u", rc->rowmarkId);
                        tle = makeTargetEntry((Expr *) var,
                                                                  list_length(tlist) + 1,
                                                                  pstrdup(resname),
@@ -147,7 +147,7 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
                                                          OIDOID,
                                                          -1,
                                                          0);
-                               snprintf(resname, sizeof(resname), "tableoid%u", rc->rti);
+                               snprintf(resname, sizeof(resname), "tableoid%u", rc->rowmarkId);
                                tle = makeTargetEntry((Expr *) var,
                                                                          list_length(tlist) + 1,
                                                                          pstrdup(resname),
@@ -161,7 +161,7 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
                        var = makeWholeRowVar(rt_fetch(rc->rti, range_table),
                                                                  rc->rti,
                                                                  0);
-                       snprintf(resname, sizeof(resname), "wholerow%u", rc->rti);
+                       snprintf(resname, sizeof(resname), "wholerow%u", rc->rowmarkId);
                        tle = makeTargetEntry((Expr *) var,
                                                                  list_length(tlist) + 1,
                                                                  pstrdup(resname),

diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c
index 35f9f980e9068875fecdd0ed048becb8afb93508..cabc93d5df9b9d42f7161223830c8629e3d6cb9c 100644 (file)
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -1288,6 +1288,7 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
 
                        newrc->rti = childRTindex;
                        newrc->prti = rti;
+                       newrc->rowmarkId = oldrc->rowmarkId;
                        newrc->markType = oldrc->markType;
                        newrc->noWait = oldrc->noWait;
                        newrc->isParent = false;

diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index fda44255b08dde1cdc4599b910d7c65b48cb8e27..ac0de093f1671229fb439c883547c9f27ad60b56 100644 (file)
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -417,6 +417,7 @@ typedef struct ExecRowMark
        Relation        relation;               /* opened and suitably locked relation */
        Index           rti;                    /* its range table index */
        Index           prti;                   /* parent range table index, if child */
+       Index           rowmarkId;              /* unique identifier for resjunk columns */
        RowMarkType markType;           /* see enum in nodes/plannodes.h */
        bool            noWait;                 /* NOWAIT option */
        ItemPointerData curCtid;        /* ctid of currently locked tuple, if any */

diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index e14257932e692100993b13e148241953ce34e405..729e35557d81d683baefaa754c60be2d63fefbbd 100644 (file)
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -701,7 +701,11 @@ typedef enum RowMarkType
  * The tableoid column is only present for an inheritance hierarchy.
  * When markType == ROW_MARK_COPY, there is instead a single column named
  *             wholerow%u                      whole-row value of relation
- * In all three cases, %u represents the parent rangetable index (prti).
+ * In all three cases, %u represents the rowmark ID number (rowmarkId).
+ * This number is unique within a plan tree, except that child relation
+ * entries copy their parent's rowmarkId.  (Assigning unique numbers
+ * means we needn't renumber rowmarkIds when flattening subqueries, which
+ * would require finding and renaming the resjunk columns as well.)
  * Note this means that all tables in an inheritance hierarchy share the
  * same resjunk column names.  However, in an inherited UPDATE/DELETE the
  * columns could have different physical column numbers in each subplan.
@@ -711,6 +715,7 @@ typedef struct PlanRowMark
        NodeTag         type;
        Index           rti;                    /* range table index of markable relation */
        Index           prti;                   /* range table index of parent relation */
+       Index           rowmarkId;              /* unique identifier for resjunk columns */
        RowMarkType markType;           /* see enum above */
        bool            noWait;                 /* NOWAIT option */
        bool            isParent;               /* true if this is a "dummy" parent entry */

diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index a3678a5b58ddb5dfd02f96e24e4728b3ff03fd27..f109a1de520921caf39c5987f2884901fb416e28 100644 (file)
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -82,6 +82,8 @@ typedef struct PlannerGlobal
 
        Index           lastPHId;               /* highest PlaceHolderVar ID assigned */
 
+       Index           lastRowMarkId;  /* highest PlanRowMark ID assigned */
+
        bool            transientPlan;  /* redo plan when TransactionXmin changes? */
 } PlannerGlobal;