From: Tom Lane <tgl@sss.pgh.pa.us>
To: David G. Johnston <david.g.johnston@gmail.com>
Cc: PetSerAl <petseral@gmail.com>
Cc: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Subject: Re: Record returning function accept not matched columns declaration
Date: Fri, 01 Mar 2024 12:55:59 -0500
Message-ID: <2969212.1709315759@sss.pgh.pa.us> (raw)
In-Reply-To: <2756762.1709245862@sss.pgh.pa.us>
References: <CAKygsHSerA1eXsJHR9wft3Gn3wfHQ5RfP8XHBzF70_qcrrRvEg@mail.gmail.com>
<CAKFQuwZ1-Ha1oHbHgveVgjNu6RWGQfVPNTO87y_=LxOmbyFBvA@mail.gmail.com>
<2741768.1709237477@sss.pgh.pa.us>
<CAKFQuwYs7f4OGJJXWCVEfjqab5U9vyZ-j-4oFJN-NJKzMKuKiw@mail.gmail.com>
<2756762.1709245862@sss.pgh.pa.us>
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
Attachments:v1-fix-record-constant-type-checking.patch (text/x-diff, 6.8K) downloaddiff --git a/src/backend/executor/nodeFunctionscan.c b/src/backend/executor/nodeFunctionscan.cindex 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.cindex 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.outindex 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.sqlindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, david.g.johnston@gmail.com, petseral@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: Record returning function accept not matched columns declaration
In-Reply-To: <2969212.1709315759@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox