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

Commit 2ed8f9a01 intended to institute a policy that if a
RangeTblFunction has a coldeflist, then the function return type is
certainly RECORD, and we should use the coldeflist as the source of
truth about what the columns of the record type are.  When the
original function has been folded to a constant, inspection of the
constant might give a different answer.  This situation will lead to
a tuple-type-mismatch error at execution, but up until that point we
need to consistently believe the coldeflist, or we'll have problems
from different bits of code reaching different conclusions.

expandRTE didn't get that memo though, and would try to produce a
tupdesc based on the constant in this situation, leading to an
assertion failure.  (Desultory testing suggests that non-assert
builds often manage to give the expected error, although I also
saw a "cache lookup failed for type 0" error, and it seems at
least possible that a crash could happen.)

Some other callers of get_expr_result_type and get_expr_result_tupdesc
were also being incautious about this.  While none of them seem to
have actual bugs, they're working harder than necessary in this case,
besides which it seems safest to have an explicit policy of not using
those functions on an RTE with a coldeflist.  Adjust the code
accordingly, and add commentary to funcapi.c about this policy.

Also fix an obsolete comment that claimed "get_expr_result_type()
doesn't know how to extract type info from a RECORD constant".
That hasn't been true since commit d57534740.

Per bug #18422 from Alexander Lakhin.
As with the previous commit, back-patch to all supported branches.

Discussion: https://postgr.es/m/18422-89ca86c8eac5246d@postgresql.org

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index c8c8e4297c45ee55be220b59df9aad3e58484237..3cca107449034a7b94448cf8c774b2ac53b21534 100644 (file)
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -2385,7 +2385,11 @@ process_function_rte_ref(RangeTblEntry *rte, AttrNumber attnum,
                {
                        TupleDesc       tupdesc;
 
-                       tupdesc = get_expr_result_tupdesc(rtfunc->funcexpr, true);
+                       /* If it has a coldeflist, it certainly returns RECORD */
+                       if (rtfunc->funccolnames != NIL)
+                               tupdesc = NULL; /* no need to work hard */
+                       else
+                               tupdesc = get_expr_result_tupdesc(rtfunc->funcexpr, true);
                        if (tupdesc && tupdesc->tdtypeid != RECORDOID)
                        {
                                /*

diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index ea0576305d0a5f7c9d153bab276fea036080ce39..0efcc3b24bcd2a5e56bfa65a76a00be16fef5c16 100644 (file)
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -1822,6 +1822,10 @@ pull_up_constant_function(PlannerInfo *root, Node *jtnode,
        if (rtf->funccolcount != 1)
                return jtnode;                  /* definitely composite */
 
+       /* If it has a coldeflist, it certainly returns RECORD */
+       if (rtf->funccolnames != NIL)
+               return jtnode;                  /* must be a one-column RECORD type */
+
        functypclass = get_expr_result_type(rtf->funcexpr,
                                                                                &funcrettype,
                                                                                &tupdesc);

diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 9d817747c883325435addd8b99a60cf6f16f1333..9fcfd55d89ba21d4f430707d428e99fc1c8787c0 100644 (file)
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -4323,12 +4323,11 @@ evaluate_function(Oid funcid, Oid result_type, int32 result_typmod,
         * Can't simplify if it returns RECORD.  The immediate problem is that it
         * will be needing an expected tupdesc which we can't supply here.
         *
-        * In the case where it has OUT parameters, it could get by without an
-        * expected tupdesc, but we still have issues: get_expr_result_type()
-        * doesn't know how to extract type info from a RECORD constant, and in
-        * the case of a NULL function result there doesn't seem to be any clean
-        * way to fix that.  In view of the likelihood of there being still other
-        * gotchas, seems best to leave the function call unreduced.
+        * In the case where it has OUT parameters, we could build an expected
+        * tupdesc from those, but there may be other gotchas lurking.  In
+        * particular, if the function were to return NULL, we would produce a
+        * null constant with no remaining indication of which concrete record
+        * type it is.  For now, seems best to leave the function call unreduced.
         */
        if (funcform->prorettype == RECORDOID)
                return NULL;

diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index 863c25f303e3c83f6fb56aaf8f41a45e3fa2fa1b..e00668302990daabfcaf7f0d1d9adc62a5923c9e 100644 (file)
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -2722,12 +2722,17 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
                                {
                                        RangeTblFunction *rtfunc = (RangeTblFunction *) lfirst(lc);
                                        TypeFuncClass functypclass;
-                                       Oid                     funcrettype;
-                                       TupleDesc       tupdesc;
+                                       Oid                     funcrettype = InvalidOid;
+                                       TupleDesc       tupdesc = NULL;
+
+                                       /* If it has a coldeflist, it returns RECORD */
+                                       if (rtfunc->funccolnames != NIL)
+                                               functypclass = TYPEFUNC_RECORD;
+                                       else
+                                               functypclass = get_expr_result_type(rtfunc->funcexpr,
+                                                                                                                       &funcrettype,
+                                                                                                                       &tupdesc);
 
-                                       functypclass = get_expr_result_type(rtfunc->funcexpr,
-                                                                                                               &funcrettype,
-                                                                                                               &tupdesc);
                                        if (functypclass == TYPEFUNC_COMPOSITE ||
                                                functypclass == TYPEFUNC_COMPOSITE_DOMAIN)
                                        {
@@ -3345,6 +3350,10 @@ get_rte_attribute_is_dropped(RangeTblEntry *rte, AttrNumber attnum)
                                        {
                                                TupleDesc       tupdesc;
 
+                                               /* If it has a coldeflist, it returns RECORD */
+                                               if (rtfunc->funccolnames != NIL)
+                                                       return false;   /* can't have any dropped columns */
+
                                                tupdesc = get_expr_result_tupdesc(rtfunc->funcexpr,
                                                                                                                  true);
                                                if (tupdesc)

diff --git a/src/backend/utils/fmgr/funcapi.c b/src/backend/utils/fmgr/funcapi.c
index 31f6662768ea9699c08c6be3593cce159124855e..89c7ed9fb64d448f20693f443fd985e340e0c09f 100644 (file)
--- a/src/backend/utils/fmgr/funcapi.c
+++ b/src/backend/utils/fmgr/funcapi.c
@@ -296,6 +296,13 @@ get_call_result_type(FunctionCallInfo fcinfo,
 /*
  * get_expr_result_type
  *             As above, but work from a calling expression node tree
+ *
+ * Beware of using this on the funcexpr of a RTE that has a coldeflist.
+ * The correct conclusion in such cases is always that the function returns
+ * RECORD with the columns defined by the coldeflist fields (funccolnames etc).
+ * If it does not, it's the executor's responsibility to catch the discrepancy
+ * at runtime; but code processing the query in advance of that point might
+ * come to inconsistent conclusions if it checks the actual expression.
  */
 TypeFuncClass
 get_expr_result_type(Node *expr,
@@ -546,7 +553,8 @@ internal_get_result_type(Oid funcid,
  * if noError is true, else throws error.
  *
  * This is a simpler version of get_expr_result_type() for use when the caller
- * is only interested in determinate rowtype results.
+ * is only interested in determinate rowtype results.  As with that function,
+ * beware of using this on the funcexpr of a RTE that has a coldeflist.
  */
 TupleDesc
 get_expr_result_tupdesc(Node *expr, bool noError)

diff --git a/src/test/regress/expected/rangefuncs.out b/src/test/regress/expected/rangefuncs.out
index d8d1f944f347836e6ef41114b0c256a7393a8071..6b16e754de46acab80f41e9f7d2df608e5fec359 100644 (file)
--- a/src/test/regress/expected/rangefuncs.out
+++ b/src/test/regress/expected/rangefuncs.out
@@ -2498,3 +2498,6 @@ 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.
+select * from int8_tbl, coalesce(row(1)) as (a int, b int);  -- fail
+ERROR:  function return row and query-specified return row do not match
+DETAIL:  Returned row contains 1 attribute, but query expects 2.

diff --git a/src/test/regress/sql/rangefuncs.sql b/src/test/regress/sql/rangefuncs.sql
index 06d598c5e9bb7f5999ccbf60661544db74e7a8f2..3c47c98e1136d23554b9d9a0791114b53963fc1c 100644 (file)
--- a/src/test/regress/sql/rangefuncs.sql
+++ b/src/test/regress/sql/rangefuncs.sql
@@ -824,3 +824,4 @@ 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
+select * from int8_tbl, coalesce(row(1)) as (a int, b int);  -- fail