Обсуждение: Record returning function accept not matched columns declaration

postgres=# SHOW SERVER_VERSION;
 server_version
----------------
 16.2
(1 row)


postgres=# with a(b) as (values (row(1,2,3)))
postgres-# select * from a, coalesce(b) as c(d int,e int, f int)
postgres-# union all
postgres-# select * from a, nullif(b, null) as c(d int,e int, f int)
postgres-# union all
postgres-# select * from a, unnest(array[b]) as c(d int,e int, f int)
postgres-# union all
postgres-# select * from a, json_populate_record(b, null) as c(d int,e
int, f int); --expect OK
    b    | d | e | f
---------+---+---+---
 (1,2,3) | 1 | 2 | 3
 (1,2,3) | 1 | 2 | 3
 (1,2,3) | 1 | 2 | 3
 (1,2,3) | 1 | 2 | 3
(4 rows)


postgres=#
postgres=# with a(b) as (values (row(1,2,3)))
postgres-# select * from a, coalesce(b) as c(d int,e int)
postgres-# union all
postgres-# select * from a, nullif(b, null) as c(d int,e int); --expect Error
ERROR:  function return row and query-specified return row do not match
DETAIL:  Returned row contains 3 attributes, but query expects 2.
postgres=#
postgres=# with a(b) as (values (row(1,2,3)))
postgres-# select * from a, unnest(array[b]) as c(d int,e int); --expect Error
ERROR:  function return row and query-specified return row do not match
DETAIL:  Returned row contains 3 attributes, but query expects 2.
postgres=#
postgres=# with a(b) as (values (row(1,2,3)))
postgres-# select * from a, json_populate_record(b, null) as c(d int,e
int); --expect Error
ERROR:  function return row and query-specified return row do not match
DETAIL:  Returned row contains 3 attributes, but query expects 2.
postgres=#
postgres=# with a(b) as (values (row(1,2,3)))
postgres-# select * from a, coalesce(b) as c(d int,e int); --expect Error
    b    | d | e
---------+---+---
 (1,2,3) | 1 | 2
(1 row)


postgres=#
postgres=# with a(b) as (values (row(1,2,3)))
postgres-# select * from a, nullif(b, null) as c(d int,e int); --expect Error
    b    | d | e
---------+---+---
 (1,2,3) | 1 | 2
(1 row)


postgres=#

Expect last two commands to fail with the same error.

postgres=# SHOW SERVER_VERSION;
 server_version
----------------
 16.2
(1 row)


postgres=# with a(b) as (values (row(1,2,3)))
postgres-# select * from a, coalesce(b) as c(d int,e int, f int)
postgres-# union all
postgres-# select * from a, nullif(b, null) as c(d int,e int, f int)
postgres-# union all
postgres-# select * from a, unnest(array[b]) as c(d int,e int, f int)
postgres-# union all
postgres-# select * from a, json_populate_record(b, null) as c(d int,e
int, f int); --expect OK
    b    | d | e | f
---------+---+---+---
 (1,2,3) | 1 | 2 | 3
 (1,2,3) | 1 | 2 | 3
 (1,2,3) | 1 | 2 | 3
 (1,2,3) | 1 | 2 | 3
(4 rows)

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Thursday, February 29, 2024, PetSerAl <petseral@gmail.com> wrote:
>> postgres=# with a(b) as (values (row(1,2,3)))
>> postgres-# select * from a, coalesce(b) as c(d int,e int, f int)
>> postgres-# union all
>> postgres-# select * from a, nullif(b, null) as c(d int,e int, f int)
>> postgres-# union all
>> postgres-# select * from a, unnest(array[b]) as c(d int,e int, f int)
>> postgres-# union all
>> postgres-# select * from a, json_populate_record(b, null) as c(d int,e
>> int, f int); --expect OK

> My concern with all of this is accepting the specification of column
> definitions for functions that don’t return the record pseudo-type.

Hm?  These cases all *do* return record, because that's what a.b is
typed as.

It looks to me like we broke these edge cases in refactoring.
v11 gives me the expected errors:

regression=# with a(b) as (values (row(1,2,3)))
select * from a, nullif(b, null) as c(d int,e int);
ERROR:  function return row and query-specified return row do not match
DETAIL:  Returned row contains 3 attributes, but query expects 2.
regression=# with a(b) as (values (row(1,2,3)))
select * from a, coalesce(b) as c(d int,e int);
ERROR:  function return row and query-specified return row do not match
DETAIL:  Returned row contains 3 attributes, but query expects 2.

v12 rejects the nullif example but not coalesce.  v14 and up lets them
both by.  I think the v14 change has to do with the function calls
having got flattened to constants, but I've not fingered the exact
culprit yet.  I didn't look yet for the earlier behavioral change.

Even more amusing, since v14:

regression=# with a(b) as (values (row(1,2,3)))
select * from a, coalesce(b) as c(d int,e int,f int, g int);
server closed the connection unexpectedly

I think that Assert is just wrong and can be removed, though;
it doesn't seem to be guarding anything of interest.

            regards, tom lane

postgres=# with a(b) as (values (row(1,2,3)))
postgres-# select * from a, coalesce(b);
ERROR:  a column definition list is required for functions returning "record"
LINE 2: select * from a, coalesce(b);
                         ^
postgres=#
postgres=# with a(b) as (values (row(1,2,3)))
postgres-# select * from a, nullif(b, null);
ERROR:  a column definition list is required for functions returning "record"
LINE 2: select * from a, nullif(b, null);
                         ^
postgres=#
postgres=# with a(b) as (values (row(1,2,3)))
postgres-# select * from a, unnest(array[b]);
ERROR:  a column definition list is required for functions returning "record"
LINE 2: select * from a, unnest(array[b]);
                         ^
postgres=#
postgres=# with a(b) as (values (row(1,2,3)))
postgres-# select * from a, json_populate_record(b, null);
ERROR:  a column definition list is required for functions returning "record"
LINE 2: select * from a, json_populate_record(b, null);
                         ^
postgres=#

It seems PostgreSQL does not care about function being polymorphic,
but only about return type being "record". It explicitly require
column definition list in all this cases.

пт, 1 мар. 2024 г. в 00:32, David G. Johnston <david.g.johnston@gmail.com>:
>
> On Thu, Feb 29, 2024 at 1:11 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>
>> "David G. Johnston" <david.g.johnston@gmail.com> writes:
>> > On Thursday, February 29, 2024, PetSerAl <petseral@gmail.com> wrote:
>> >> postgres=# with a(b) as (values (row(1,2,3)))
>> >> postgres-# select * from a, coalesce(b) as c(d int,e int, f int)
>> >> postgres-# union all
>> >> postgres-# select * from a, nullif(b, null) as c(d int,e int, f int)
>> >> postgres-# union all
>> >> postgres-# select * from a, unnest(array[b]) as c(d int,e int, f int)
>> >> postgres-# union all
>> >> postgres-# select * from a, json_populate_record(b, null) as c(d int,e
>> >> int, f int); --expect OK
>>
>> > My concern with all of this is accepting the specification of column
>> > definitions for functions that don’t return the record pseudo-type.
>>
>> Hm?  These cases all *do* return record, because that's what a.b is
>> typed as.
>
>
> I strongly dislike the seemingly overloaded terminology in this area.  Namely I was trying to distinguish these two
examplefunction signatures. 
>
> json_populate_record ( base anyelement, from_json json ) → anyelement
> jsonb_to_record ( jsonb ) → record
>
> Polymorphic functions do not require a column definition list.  The non-polymorphic function signature does require
thecolumn definition list.  That we accept a column definition list in the polymorphic case is settled code but seems
likeit led to this bug. 
>
> David J.

I had this issue as well.

There is much inet talk about a bug in PGadmin in creating functions v cli, and how pgAdmin doesn't like some
functions.

I had to change my function to this method to get out of the error.

The error wants you to have the columns in the function, not at SELECT statement.

at least this was my issue, your mileage may vary.

DROP FUNCTION IF EXISTS schema.function_name;

CREATE OR REPLACE FUNCTION schema.function_name;(
    _typeahead character varying,
    _rolename character varying,
    _requiremisconductaccess character varying,
    _test1 character varying,
    _test2 character varying,
    _test3 character varying)
    RETURNS TABLE(col1 character varying, col2 character varying, col3 character varying)
    LANGUAGE 'plpgsql'
    COST 100
    VOLATILE PARALLEL UNSAFE
    ROWS 1000

AS $BODY$
BEGIN



-----Original Message-----
From: PetSerAl <petseral@gmail.com> 
Sent: Thursday, February 29, 2024 2:02 PM
To: David G. Johnston <david.g.johnston@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-bugs@lists.postgresql.org
Subject: [EXTERNAL] Re: Record returning function accept not matched columns declaration

postgres=# with a(b) as (values (row(1,2,3))) postgres-# select * from a, coalesce(b);
ERROR:  a column definition list is required for functions returning "record"
LINE 2: select * from a, coalesce(b);
                         ^
postgres=#
postgres=# with a(b) as (values (row(1,2,3))) postgres-# select * from a, nullif(b, null);
ERROR:  a column definition list is required for functions returning "record"
LINE 2: select * from a, nullif(b, null);
                         ^
postgres=#
postgres=# with a(b) as (values (row(1,2,3))) postgres-# select * from a, unnest(array[b]);
ERROR:  a column definition list is required for functions returning "record"
LINE 2: select * from a, unnest(array[b]);
                         ^
postgres=#
postgres=# with a(b) as (values (row(1,2,3))) postgres-# select * from a, json_populate_record(b, null);
ERROR:  a column definition list is required for functions returning "record"
LINE 2: select * from a, json_populate_record(b, null);
                         ^
postgres=#

It seems PostgreSQL does not care about function being polymorphic, but only about return type being "record". It
explicitlyrequire column definition list in all this cases.
 

пт, 1 мар. 2024 г. в 00:32, David G. Johnston <david.g.johnston@gmail.com>:
>
> On Thu, Feb 29, 2024 at 1:11 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>
>> "David G. Johnston" <david.g.johnston@gmail.com> writes:
>> > On Thursday, February 29, 2024, PetSerAl <petseral@gmail.com> wrote:
>> >> postgres=# with a(b) as (values (row(1,2,3))) postgres-# select * 
>> >> from a, coalesce(b) as c(d int,e int, f int) postgres-# union all 
>> >> postgres-# select * from a, nullif(b, null) as c(d int,e int, f 
>> >> int) postgres-# union all postgres-# select * from a, 
>> >> unnest(array[b]) as c(d int,e int, f int) postgres-# union all 
>> >> postgres-# select * from a, json_populate_record(b, null) as c(d 
>> >> int,e int, f int); --expect OK
>>
>> > My concern with all of this is accepting the specification of 
>> > column definitions for functions that don’t return the record pseudo-type.
>>
>> Hm?  These cases all *do* return record, because that's what a.b is 
>> typed as.
>
>
> I strongly dislike the seemingly overloaded terminology in this area.  Namely I was trying to distinguish these two
examplefunction signatures.
 
>
> json_populate_record ( base anyelement, from_json json ) → anyelement 
> jsonb_to_record ( jsonb ) → record
>
> Polymorphic functions do not require a column definition list.  The non-polymorphic function signature does require
thecolumn definition list.  That we accept a column definition list in the polymorphic case is settled code but seems
likeit led to this bug.
 
>
> David J.

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> I strongly dislike the seemingly overloaded terminology in this area.

Fair.  By "returns RECORD" I meant that it actually is declared as
returning that pseudotype, rather than a named composite type.

> Namely I was trying to distinguish these two example function signatures.

> json_populate_record ( base anyelement, from_json json ) → anyelement
> jsonb_to_record ( jsonb ) → record

> Polymorphic functions do not require a column definition list.  The
> non-polymorphic function signature does require the column definition
> list.  That we accept a column definition list in the polymorphic case is
> settled code but seems like it led to this bug.

No, I don't think that has much to do with it; note that the
json_populate_record case is one of the ones *not* misbehaving here.
Also note that what we've got here is that the actual input type
for json_populate_record is RECORD, and therefore so is its resolved
output type, and that means you need a coldeflist.

I bisected to see where the behavior changed, and arrived at

Author: Tom Lane <tgl@sss.pgh.pa.us>
Branch: master Release: REL_16_BR [d57534740] 2022-10-16 19:18:08 -0400
Branch: REL_15_STABLE Release: REL_15_1 [d4abb0bc5] 2022-10-16 19:18:08 -0400
Branch: REL_14_STABLE Release: REL_14_6 [8122160ff] 2022-10-16 19:18:08 -0400

    Fix EXPLAIN of SEARCH BREADTH FIRST with a constant initial value.

which surprised me more than a little.  What that patch did was merely
to teach get_expr_result_type() how to extract a tuple descriptor out
of a RECORD-type constant (again, using RECORD in the specific sense
above).  That affects this case because the WITH has been flattened
sufficiently that the function-in-FROM expresssion is just a RECORD
constant, as you can see with EXPLAIN:

regression=# explain verbose with a(b) as (values (row(1,2,3)))
select * from a, coalesce(b) as c(d int,e int,f int);
                      QUERY PLAN                       
-------------------------------------------------------
 Function Scan on c  (cost=0.00..0.01 rows=1 width=44)
   Output: '(1,2,3)'::record, c.d, c.e, c.f
   Function Call: '(1,2,3)'::record
(3 rows)

(If you mark the WITH as MATERIALIZED, the bug goes away because
things aren't flattened so much.)

I believe the problem here is that ExecInitFunctionScan has its
priorities wrong.  It consults the coldeflist (rtfunc->funccolnames
etc) only if get_expr_result_type says TYPEFUNC_RECORD, which it
would have done before this commit but not after.  When the answer
is TYPEFUNC_COMPOSITE we believe the tupdesc extracted from the
expression.  Which essentially means that the later check for
"tupdesc returned by expression matches what the query expects"
is comparing that tupdesc to itself, so it always succeeds.

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.

This is a bit scary because it seems plausible that other callers
of get_expr_result_type might've made the same mistake.  I can only
find one other place in core that is doing the same sort of thing:
inline_set_returning_function looks like it has a related bug.
I'm worried about third-party callers though.

Note that reverting d57534740 wouldn't be much of a fix, because
there were already cases where get_expr_result_type could return
TYPEFUNC_COMPOSITE from a nominally-RECORD expression.  Also,
we later back-patched that, meaning that (I think) there are
related hazards in all active branches.  The specific example
given here only fails in branches that will flatten WITH queries,
but I bet you can break it further back with (for example) an
inline-able function that expands to a RowExpr.

I still haven't looked into why the older branches behave differently
for nullif() than coalesce().  Most likely the answer is not very
interesting but just devolves to whether eval_const_expressions
knew how to fold those things to constants.

            regards, tom lane

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

On Sat, Mar 2, 2024 at 1:56 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> 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.

context: in postgres 9.3.25, dbfiddle[1]
this query will fail:
`
with a(b) as (values (row(1,2,3)))
select * from a, coalesce(b) as c(d int, e int);
`

+ * 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.
  */

I think by ExecMakeTableFunctionResult you mean `mainly
ExecMakeTableFunctionResult's function: tupledesc_match`
since ExecMakeTableFunctionResult is quite long.

also looking around the code, `ExecMakeTableFunctionResult handles
some of this,`
actually is  `ExecMakeTableFunctionResult handles most of this`?

So overall, I think `ExecMakeTableFunctionResult's inner function,
tupledesc_match handle most of this`
would be more accurate.

[1] https://dbfiddle.uk/4SrE-1JR

jian he <jian.universality@gmail.com> writes:
> On Sat, Mar 2, 2024 at 1:56 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I'm having mixed feelings about whether to back-patch this.  Somebody
>> might complain that we broke a working query in a minor release.

> context: in postgres 9.3.25, dbfiddle[1]
> this query will fail:

> with a(b) as (values (row(1,2,3)))
> select * from a, coalesce(b) as c(d int, e int);

Sure, but in v12 and up (i.e., the supported versions that we might
back-patch into) the query succeeds, and it might not be obvious
to someone that its behavior isn't as-intended.

Admittedly, this seems like a rather contrived case.  In cases
where the record-returning function call isn't reducible to a
constant, you'll get the expected error.  So maybe I'm worrying
over something that no one is depending on in practice.

> I think by ExecMakeTableFunctionResult you mean `mainly
> ExecMakeTableFunctionResult's function: tupledesc_match`
> since ExecMakeTableFunctionResult is quite long.

Perhaps.  I mentioned ExecMakeTableFunctionResult because that's
what's called directly from nodeFunctionscan.c.  Referencing a
static function of another module is risky because it's pretty
unlikely the comment would get updated if refactoring in that
module makes it false.

            regards, tom lane

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> with a(b) as (values (row(1,2,3)))
> select (a.b).* from a;
> ERROR:  record type has not been registered

I looked into this case.  The failure occurs when the parser tries
to expand the ".*" notation into separate output columns, as required
per SQL spec.  All it has is a Var of type RECORD referencing the
VALUES RTE entry, so it has to fail.

In the specific example given here, you could imagine drilling down
into the VALUES and figuring out what concrete rowtype the RECORD
value will have at runtime, but I'm not excited about going there.
There are too many cases where it wouldn't work.

Note that as long as you don't need parse-time expansion of the
record, it works:

=# with a(b) as (values (row(1,2,3)))
select a.b from a;
    b    
---------
 (1,2,3)
(1 row)

I pushed the patch for the original problem.  After debating with
myself I concluded that back-patching it was probably less risky
than not back-patching, so I did that.

            regards, tom lane