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,