Ensure that whole-row junk Vars are always of composite type.

The EvalPlanQual machinery assumes that whole-row Vars generated for the
outputs of non-table RTEs will be of composite types.  However, for the
case where the RTE is a function call returning a scalar type, we were
doing the wrong thing, as a result of sharing code with a parser case
where the function's scalar output is wanted.  (Or at least, that's what
that case has done historically; it does seem a bit inconsistent.)

To fix, extend makeWholeRowVar's API so that it can support both use-cases.
This fixes Belinda Cussen's report of crashes during concurrent execution
of UPDATEs involving joins to the result of UNNEST() --- in READ COMMITTED
mode, we'd run the EvalPlanQual machinery after a conflicting row update
commits, and it was expecting to get a HeapTuple not a scalar datum from
the "wholerowN" variable referencing the function RTE.

Back-patch to 9.0 where the current EvalPlanQual implementation appeared.

In 9.1 and up, this patch also fixes failure to attach the correct
collation to the Var generated for a scalar-result case.  An example:
regression=# select upper(x.*) from textcat('ab', 'cd') x;
ERROR:  could not determine which collation to use for upper() function

diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index 9070cd2d739abecc60db22efa1cfb842ce20b417..683e751148d0907232c9c01dba643f33fd65a25a 100644 (file)
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -121,11 +121,17 @@ makeVarFromTargetEntry(Index varno,
  * with error cases, but it's not worth changing now.)  The vartype indicates
  * a rowtype; either a named composite type, or RECORD.  This function
  * encapsulates the logic for determining the correct rowtype OID to use.
+ *
+ * If allowScalar is true, then for the case where the RTE is a function
+ * returning a non-composite result type, we produce a normal Var referencing
+ * the function's result directly, instead of the single-column composite
+ * value that the whole-row notation might otherwise suggest.
  */
 Var *
 makeWholeRowVar(RangeTblEntry *rte,
                Index varno,
-               Index varlevelsup)
+               Index varlevelsup,
+               bool allowScalar)
 {
    Var        *result;
    Oid         toid;
@@ -157,39 +163,34 @@ makeWholeRowVar(RangeTblEntry *rte,
                                 InvalidOid,
                                 varlevelsup);
            }
-           else
+           else if (allowScalar)
            {
-               /*
-                * func returns scalar; instead of making a whole-row Var,
-                * just reference the function's scalar output.  (XXX this
-                * seems a tad inconsistent, especially if "f.*" was
-                * explicitly written ...)
-                */
+               /* func returns scalar; just return its output as-is */
                result = makeVar(varno,
                                 1,
                                 toid,
                                 -1,
+                                exprCollation(rte->funcexpr),
+                                varlevelsup);
+           }
+           else
+           {
+               /* func returns scalar, but we want a composite result */
+               result = makeVar(varno,
+                                InvalidAttrNumber,
+                                RECORDOID,
+                                -1,
                                 InvalidOid,
                                 varlevelsup);
            }
            break;
-       case RTE_VALUES:
-           toid = RECORDOID;
-           /* returns composite; same as relation case */
-           result = makeVar(varno,
-                            InvalidAttrNumber,
-                            toid,
-                            -1,
-                            InvalidOid,
-                            varlevelsup);
-           break;
        default:
 
            /*
-            * RTE is a join or subselect.  We represent this as a whole-row
-            * Var of RECORD type.  (Note that in most cases the Var will be
-            * expanded to a RowExpr during planning, but that is not our
-            * concern here.)
+            * RTE is a join, subselect, or VALUES.  We represent this as a
+            * whole-row Var of RECORD type. (Note that in most cases the Var
+            * will be expanded to a RowExpr during planning, but that is not
+            * our concern here.)
             */
            result = makeVar(varno,
                             InvalidAttrNumber,

diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c
index 077ac49631f00bc4f14fbfc77d6de61e8ead7b2a..dccc557f031eebc00037edefbc929653b73a2f68 100644 (file)
--- a/src/backend/optimizer/prep/preptlist.c
+++ b/src/backend/optimizer/prep/preptlist.c
@@ -129,7 +129,8 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
            /* Not a table, so we need the whole row as a junk var */
            var = makeWholeRowVar(rt_fetch(rc->rti, range_table),
                                  rc->rti,
-                                 0);
+                                 0,
+                                 false);
            snprintf(resname, sizeof(resname), "wholerow%u", rc->rowmarkId);
            tle = makeTargetEntry((Expr *) var,
                                  list_length(tlist) + 1,

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 79328c997974ad5c50a016b7ade3c3eb4d64156e..75236c76a1f5aabdaff0d0710a5c5d40d08a64d8 100644 (file)
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -2059,8 +2059,15 @@ transformWholeRowRef(ParseState *pstate, RangeTblEntry *rte, int location)
    /* Find the RTE's rangetable location */
    vnum = RTERangeTablePosn(pstate, rte, &sublevels_up);
 
-   /* Build the appropriate referencing node */
-   result = makeWholeRowVar(rte, vnum, sublevels_up);
+   /*
+    * Build the appropriate referencing node.  Note that if the RTE is a
+    * function returning scalar, we create just a plain reference to the
+    * function value, not a composite containing a single column.  This is
+    * pretty inconsistent at first sight, but it's what we've done
+    * historically.  One argument for it is that "rel" and "rel.*" mean the
+    * same thing for composite relations, so why not for scalar functions...
+    */
+   result = makeWholeRowVar(rte, vnum, sublevels_up, true);
 
    /* location is not filled in by makeWholeRowVar */
    result->location = location;

diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 3b311087756fecb2bc3778bf9678f4d61b1c1e63..a6f4141dd7b677785e8cd45af0aeee7a70a657f7 100644 (file)
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -1188,7 +1188,8 @@ rewriteTargetListUD(Query *parsetree, RangeTblEntry *target_rte,
         */
        var = makeWholeRowVar(target_rte,
                              parsetree->resultRelation,
-                             0);
+                             0,
+                             false);
 
        attrname = "wholerow";
    }

diff --git a/src/include/nodes/makefuncs.h b/src/include/nodes/makefuncs.h
index 532681f8c015e5606d9de395e97d51745a10b5f7..c5ed6cb4133c8d4376f4d8a587405cdc9f4a5996 100644 (file)
--- a/src/include/nodes/makefuncs.h
+++ b/src/include/nodes/makefuncs.h
@@ -35,7 +35,8 @@ extern Var *makeVarFromTargetEntry(Index varno,
 
 extern Var *makeWholeRowVar(RangeTblEntry *rte,
                Index varno,
-               Index varlevelsup);
+               Index varlevelsup,
+               bool allowScalar);
 
 extern TargetEntry *makeTargetEntry(Expr *expr,
                AttrNumber resno,