Re: Record returning function accept not matched columns declaration - Mailing list pgsql-bugs

From Tom Lane
Subject Re: Record returning function accept not matched columns declaration
Date
Msg-id 2969212.1709315759@sss.pgh.pa.us
Whole thread Raw
In response to Re: Record returning function accept not matched columns declaration  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Record returning function accept not matched columns declaration  (jian he <jian.universality@gmail.com>)
List pgsql-bugs
I wrote:
> I think we just need to flip things around so that we build the
> expected tupdesc from coldeflist if it's present, and only if not do
> we examine the expression.  The cases this might fail to catch should
> all have been handled at parse time in addRangeTableEntryForFunction,
> so we don't have to check again.

Here's a draft patch that fixes it that way.

I'm having mixed feelings about whether to back-patch this.  Somebody
might complain that we broke a working query in a minor release.
Assuming that the visible consequences are all pretty benign, as they
seem to be (noting that end-users won't see the assertion failure),
maybe the conservative course is to leave it unfixed in stable
branches.  However, I'm not totally convinced that the consequences
are all benign, so there would be a risk on that side.  Thoughts?

            regards, tom lane

diff --git a/src/backend/executor/nodeFunctionscan.c b/src/backend/executor/nodeFunctionscan.c
index 4ee8f51f73..6ba0a49668 100644
--- 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 edc25d712e..afdd58626b 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -5219,16 +5219,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 fbb840e848..2bda957f43 100644
--- 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 63351e1412..06d598c5e9 100644
--- 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

pgsql-bugs by date:

Previous
From: Alexey Ermakov
Date:
Subject: Re: BUG #18349: ERROR: invalid DSA memory alloc request size 1811939328, CONTEXT: parallel worker
Next
From: PG Bug reporting form
Date:
Subject: BUG #18374: Printing memory contexts on OOM condition might lead to segmentation fault