Fix type-checking of RECORD-returning functions in FROM.

In the corner case where a function returning RECORD has been
simplified to a RECORD constant or an inlined ROW() expression,
ExecInitFunctionScan failed to cross-check the function's result
rowtype against the coldeflist provided by the calling query.
That happened because get_expr_result_type is able to extract a
tupdesc from such expressions, which led ExecInitFunctionScan to
ignore the coldeflist.  (Instead, it used the extracted tupdesc
to check the function's output, which of course always succeeds.)

I have not been able to demonstrate any really serious consequences
from this, because if some column of the result is of the wrong
type and is directly referenced by a Var of the calling query,
CheckVarSlotCompatibility will catch it.  However, we definitely do
fail to report the case where the function returns more columns than
the coldeflist expects, and in the converse case where it returns
fewer columns, we get an assert failure (but, seemingly, no worse
results in non-assert builds).

To fix, always build the expected tupdesc from the coldeflist if there
is one, and consult get_expr_result_type only when there isn't one.

Also remove the failing Assert, even though it is no longer reached
after this fix.  It doesn't seem to be adding anything useful, since
later checking will deal with cases with the wrong number of columns.

The only other place I could find that is doing something similar
is inline_set_returning_function.  There's no live bug there because
we cannot be looking at a Const or RowExpr, but for consistency
change that code to agree with ExecInitFunctionScan.

Per report from PetSerAl.  After some debate I've concluded that
this should be back-patched.  There is a small risk that somebody
has been relying on such a case not throwing an error, but I judge
this outweighed by the risk that I've missed some way in which the
failure to cross-check has worse consequences than sketched above.

Discussion: https://postgr.es/m/CAKygsHSerA1eXsJHR9wft3Gn3wfHQ5RfP8XHBzF70_qcrrRvEg@mail.gmail.com

diff --git a/src/backend/executor/nodeFunctionscan.c b/src/backend/executor/nodeFunctionscan.c
index 434379a5aabd0c1ddc89c7b9cdab504abfdef149..cc257317e58e69a836b2efe968ab57190226e0a0 100644 (file)
--- a/src/backend/executor/nodeFunctionscan.c
+++ b/src/backend/executor/nodeFunctionscan.c
@@ -344,8 +344,6 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, int eflags)
                Node       *funcexpr = rtfunc->funcexpr;
                int                     colcount = rtfunc->funccolcount;
                FunctionScanPerFuncState *fs = &scanstate->funcstates[i];
-               TypeFuncClass functypclass;
-               Oid                     funcrettype;
                TupleDesc       tupdesc;
 
                fs->setexpr =
@@ -362,39 +360,18 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, int eflags)
                fs->rowcount = -1;
 
                /*
-                * Now determine if the function returns a simple or composite type,
-                * and build an appropriate tupdesc.  Note that in the composite case,
-                * the function may now return more columns than it did when the plan
-                * was made; we have to ignore any columns beyond "colcount".
+                * Now build a tupdesc showing the result type we expect from the
+                * function.  If we have a coldeflist then that takes priority (note
+                * the parser enforces that there is one if the function's nominal
+                * output type is RECORD).  Otherwise use get_expr_result_type.
+                *
+                * Note that if the function returns a named composite type, that may
+                * now contain more or different columns than it did when the plan was
+                * made.  For both that and the RECORD case, we need to check tuple
+                * compatibility.  ExecMakeTableFunctionResult handles some of this,
+                * and CheckVarSlotCompatibility provides a backstop.
                 */
-               functypclass = get_expr_result_type(funcexpr,
-                                                                                       &funcrettype,
-                                                                                       &tupdesc);
-
-               if (functypclass == TYPEFUNC_COMPOSITE ||
-                       functypclass == TYPEFUNC_COMPOSITE_DOMAIN)
-               {
-                       /* Composite data type, e.g. a table's row type */
-                       Assert(tupdesc);
-                       Assert(tupdesc->natts >= colcount);
-                       /* Must copy it out of typcache for safety */
-                       tupdesc = CreateTupleDescCopy(tupdesc);
-               }
-               else if (functypclass == TYPEFUNC_SCALAR)
-               {
-                       /* Base data type, i.e. scalar */
-                       tupdesc = CreateTemplateTupleDesc(1);
-                       TupleDescInitEntry(tupdesc,
-                                                          (AttrNumber) 1,
-                                                          NULL,        /* don't care about the name here */
-                                                          funcrettype,
-                                                          -1,
-                                                          0);
-                       TupleDescInitEntryCollation(tupdesc,
-                                                                               (AttrNumber) 1,
-                                                                               exprCollation(funcexpr));
-               }
-               else if (functypclass == TYPEFUNC_RECORD)
+               if (rtfunc->funccolnames != NIL)
                {
                        tupdesc = BuildDescFromLists(rtfunc->funccolnames,
                                                                                 rtfunc->funccoltypes,
@@ -410,8 +387,40 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, int eflags)
                }
                else
                {
-                       /* crummy error message, but parser should have caught this */
-                       elog(ERROR, "function in FROM has unsupported return type");
+                       TypeFuncClass functypclass;
+                       Oid                     funcrettype;
+
+                       functypclass = get_expr_result_type(funcexpr,
+                                                                                               &funcrettype,
+                                                                                               &tupdesc);
+
+                       if (functypclass == TYPEFUNC_COMPOSITE ||
+                               functypclass == TYPEFUNC_COMPOSITE_DOMAIN)
+                       {
+                               /* Composite data type, e.g. a table's row type */
+                               Assert(tupdesc);
+                               /* Must copy it out of typcache for safety */
+                               tupdesc = CreateTupleDescCopy(tupdesc);
+                       }
+                       else if (functypclass == TYPEFUNC_SCALAR)
+                       {
+                               /* Base data type, i.e. scalar */
+                               tupdesc = CreateTemplateTupleDesc(1);
+                               TupleDescInitEntry(tupdesc,
+                                                                  (AttrNumber) 1,
+                                                                  NULL,        /* don't care about the name here */
+                                                                  funcrettype,
+                                                                  -1,
+                                                                  0);
+                               TupleDescInitEntryCollation(tupdesc,
+                                                                                       (AttrNumber) 1,
+                                                                                       exprCollation(funcexpr));
+                       }
+                       else
+                       {
+                               /* crummy error message, but parser should have caught this */
+                               elog(ERROR, "function in FROM has unsupported return type");
+                       }
                }
 
                fs->tupdesc = tupdesc;

diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index e1cedd9448d06bbb534b5c0760eeb568d5bb966d..3ef684a75d38195f41eb1c7e3cb2342cf902a40c 100644 (file)
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -5134,16 +5134,20 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
        }
 
        /*
-        * Also resolve the actual function result tupdesc, if composite.  If the
-        * function is just declared to return RECORD, dig the info out of the AS
-        * clause.
+        * Also resolve the actual function result tupdesc, if composite.  If we
+        * have a coldeflist, believe that; otherwise use get_expr_result_type.
+        * (This logic should match ExecInitFunctionScan.)
         */
-       functypclass = get_expr_result_type((Node *) fexpr, NULL, &rettupdesc);
-       if (functypclass == TYPEFUNC_RECORD)
+       if (rtfunc->funccolnames != NIL)
+       {
+               functypclass = TYPEFUNC_RECORD;
                rettupdesc = BuildDescFromLists(rtfunc->funccolnames,
                                                                                rtfunc->funccoltypes,
                                                                                rtfunc->funccoltypmods,
                                                                                rtfunc->funccolcollations);
+       }
+       else
+               functypclass = get_expr_result_type((Node *) fexpr, NULL, &rettupdesc);
 
        /*
         * The single command must be a plain SELECT.

diff --git a/src/test/regress/expected/rangefuncs.out b/src/test/regress/expected/rangefuncs.out
index e2e62db6a21dbc97db67599f41b4bf2c95fbdc18..d8d1f944f347836e6ef41114b0c256a7393a8071 100644 (file)
--- a/src/test/regress/expected/rangefuncs.out
+++ b/src/test/regress/expected/rangefuncs.out
@@ -2485,3 +2485,16 @@ select * from
  [{"id": "1"}] | 1
 (1 row)
 
+-- check detection of mismatching record types with a const-folded expression
+with a(b) as (values (row(1,2,3)))
+select * from a, coalesce(b) as c(d int, e int);  -- fail
+ERROR:  function return row and query-specified return row do not match
+DETAIL:  Returned row contains 3 attributes, but query expects 2.
+with a(b) as (values (row(1,2,3)))
+select * from a, coalesce(b) as c(d int, e int, f int, g int);  -- fail
+ERROR:  function return row and query-specified return row do not match
+DETAIL:  Returned row contains 3 attributes, but query expects 4.
+with a(b) as (values (row(1,2,3)))
+select * from a, coalesce(b) as c(d int, e int, f float);  -- fail
+ERROR:  function return row and query-specified return row do not match
+DETAIL:  Returned type integer at ordinal position 3, but query expects double precision.

diff --git a/src/test/regress/sql/rangefuncs.sql b/src/test/regress/sql/rangefuncs.sql
index 63351e1412d4bfec68478a8435e8db181dc1c309..06d598c5e9bb7f5999ccbf60661544db74e7a8f2 100644 (file)
--- a/src/test/regress/sql/rangefuncs.sql
+++ b/src/test/regress/sql/rangefuncs.sql
@@ -815,3 +815,12 @@ select * from
    from unnest(array['{"lectures": [{"id": "1"}]}'::jsonb])
         as unnested_modules(module)) as ss,
   jsonb_to_recordset(ss.lecture) as j (id text);
+
+-- check detection of mismatching record types with a const-folded expression
+
+with a(b) as (values (row(1,2,3)))
+select * from a, coalesce(b) as c(d int, e int);  -- fail
+with a(b) as (values (row(1,2,3)))
+select * from a, coalesce(b) as c(d int, e int, f int, g int);  -- fail
+with a(b) as (values (row(1,2,3)))
+select * from a, coalesce(b) as c(d int, e int, f float);  -- fail