Fix failure with SQL-procedure polymorphic output arguments in v12.

Before the v13-era commit 913bbd88d, check_sql_fn_retval fails to
resolve polymorphic output types and then just throws up its hands and
assumes the check will be made at runtime.  I think that's true for
ordinary functions returning RECORD, but it doesn't happen in CALL,
potentially resulting in crashes if the actual output of the SQL
procedure's SELECT doesn't match the type inferred from polymorphism.
With a little bit of rearrangement, we can use get_call_result_type
instead of get_func_result_type and thereby infer the correct types.

I'm still unwilling to back-patch all of 913bbd88d, so if the types
don't match you'll get an error rather than perhaps silently inserting
a cast as v13 and later can.  That's consistent with prior behavior
though, so it seems fine.

Prior to 70ffb27b2, you'd typically get other errors due to other
shortcomings of CALL's management of polymorphism.  Nonetheless,
this is an independent bug.

Although there is no bug in v13 and up, it seems prudent to add
the test case for this to the newer branches too.  It's clearly
an under-tested area.

Per report from Andrew Bille.

Discussion: https://postgr.es/m/CAJnzarw9EeWHAQRm76dXd=7j+rgw6ERqC=nCay8jeFqTwKwhqQ@mail.gmail.com

diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 2e4d5d9f4541096ac82bd8b12a5607191671629f..4ec1be83e32f61e43723219cd03d21702059422e 100644 (file)
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -154,7 +154,7 @@ static Node *sql_fn_resolve_param_name(SQLFunctionParseInfoPtr pinfo,
 static List *init_execution_state(List *queryTree_list,
                                                                  SQLFunctionCachePtr fcache,
                                                                  bool lazyEvalOK);
-static void init_sql_fcache(FmgrInfo *finfo, Oid collation, bool lazyEvalOK);
+static void init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK);
 static void postquel_start(execution_state *es, SQLFunctionCachePtr fcache);
 static bool postquel_getnext(execution_state *es, SQLFunctionCachePtr fcache);
 static void postquel_end(execution_state *es);
@@ -166,6 +166,12 @@ static Datum postquel_get_single_result(TupleTableSlot *slot,
                                                                                MemoryContext resultcontext);
 static void sql_exec_error_callback(void *arg);
 static void ShutdownSQLFunction(Datum arg);
+static bool check_sql_fn_retval_ext2(Oid func_id,
+                                                                        FunctionCallInfo fcinfo,
+                                                                        Oid rettype, char prokind,
+                                                                        List *queryTreeList,
+                                                                        bool *modifyTargetList,
+                                                                        JunkFilter **junkFilter);
 static void sqlfunction_startup(DestReceiver *self, int operation, TupleDesc typeinfo);
 static bool sqlfunction_receive(TupleTableSlot *slot, DestReceiver *self);
 static void sqlfunction_shutdown(DestReceiver *self);
@@ -591,8 +597,9 @@ init_execution_state(List *queryTree_list,
  * Initialize the SQLFunctionCache for a SQL function
  */
 static void
-init_sql_fcache(FmgrInfo *finfo, Oid collation, bool lazyEvalOK)
+init_sql_fcache(FunctionCallInfo fcinfo, Oid collation, bool lazyEvalOK)
 {
+       FmgrInfo   *finfo = fcinfo->flinfo;
        Oid                     foid = finfo->fn_oid;
        MemoryContext fcontext;
        MemoryContext oldcontext;
@@ -744,12 +751,13 @@ init_sql_fcache(FmgrInfo *finfo, Oid collation, bool lazyEvalOK)
         * coerce the returned rowtype to the desired form (unless the result type
         * is VOID, in which case there's nothing to coerce to).
         */
-       fcache->returnsTuple = check_sql_fn_retval_ext(foid,
-                                                                                                  rettype,
-                                                                                                  procedureStruct->prokind,
-                                                                                                  flat_query_list,
-                                                                                                  NULL,
-                                                                                                  &fcache->junkFilter);
+       fcache->returnsTuple = check_sql_fn_retval_ext2(foid,
+                                                                                                       fcinfo,
+                                                                                                       rettype,
+                                                                                                       procedureStruct->prokind,
+                                                                                                       flat_query_list,
+                                                                                                       NULL,
+                                                                                                       &fcache->junkFilter);
 
        if (fcache->returnsTuple)
        {
@@ -1066,7 +1074,7 @@ fmgr_sql(PG_FUNCTION_ARGS)
 
        if (fcache == NULL)
        {
-               init_sql_fcache(fcinfo->flinfo, PG_GET_COLLATION(), lazyEvalOK);
+               init_sql_fcache(fcinfo, PG_GET_COLLATION(), lazyEvalOK);
                fcache = (SQLFunctionCachePtr) fcinfo->flinfo->fn_extra;
        }
 
@@ -1593,11 +1601,11 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList,
                                        JunkFilter **junkFilter)
 {
        /* Wrapper function to preserve ABI compatibility in released branches */
-       return check_sql_fn_retval_ext(func_id, rettype,
-                                                                  PROKIND_FUNCTION,
-                                                                  queryTreeList,
-                                                                  modifyTargetList,
-                                                                  junkFilter);
+       return check_sql_fn_retval_ext2(func_id, NULL, rettype,
+                                                                       PROKIND_FUNCTION,
+                                                                       queryTreeList,
+                                                                       modifyTargetList,
+                                                                       junkFilter);
 }
 
 bool
@@ -1605,6 +1613,22 @@ check_sql_fn_retval_ext(Oid func_id, Oid rettype, char prokind,
                                                List *queryTreeList,
                                                bool *modifyTargetList,
                                                JunkFilter **junkFilter)
+{
+       /* Wrapper function to preserve ABI compatibility in released branches */
+       return check_sql_fn_retval_ext2(func_id, NULL, rettype,
+                                                                       prokind,
+                                                                       queryTreeList,
+                                                                       modifyTargetList,
+                                                                       junkFilter);
+}
+
+static bool
+check_sql_fn_retval_ext2(Oid func_id,
+                                                FunctionCallInfo fcinfo,
+                                                Oid rettype, char prokind,
+                                                List *queryTreeList,
+                                                bool *modifyTargetList,
+                                                JunkFilter **junkFilter)
 {
        Query      *parse;
        List      **tlist_ptr;
@@ -1750,6 +1774,7 @@ check_sql_fn_retval_ext(Oid func_id, Oid rettype, char prokind,
                 * result type, so there is no way to produce a domain-over-composite
                 * result except by computing it as an explicit single-column result.
                 */
+               TypeFuncClass tfclass;
                TupleDesc       tupdesc;
                int                     tupnatts;       /* physical number of columns in tuple */
                int                     tuplogcols; /* # of nondeleted columns in tuple */
@@ -1806,10 +1831,19 @@ check_sql_fn_retval_ext(Oid func_id, Oid rettype, char prokind,
                }
 
                /*
-                * Is the rowtype fixed, or determined only at runtime?  (Note we
+                * Identify the output rowtype, resolving polymorphism if possible
+                * (that is, if we were passed an fcinfo).
+                */
+               if (fcinfo)
+                       tfclass = get_call_result_type(fcinfo, NULL, &tupdesc);
+               else
+                       tfclass = get_func_result_type(func_id, NULL, &tupdesc);
+
+               /*
+                * Is the rowtype known, or determined only at runtime?  (Note we
                 * cannot see TYPEFUNC_COMPOSITE_DOMAIN here.)
                 */
-               if (get_func_result_type(func_id, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+               if (tfclass != TYPEFUNC_COMPOSITE)
                {
                        /*
                         * Assume we are returning the whole tuple. Crosschecking against

diff --git a/src/test/regress/expected/create_procedure.out b/src/test/regress/expected/create_procedure.out
index 4758744071fec8b065508c08d804fef690936e99..2f646a02b552b0a0d1ab83116d00c9e6540cf2c2 100644 (file)
--- a/src/test/regress/expected/create_procedure.out
+++ b/src/test/regress/expected/create_procedure.out
@@ -185,6 +185,21 @@ CALL ptest6b(1.1, null, null);
  1.1 | {1.1}
 (1 row)
 
+CREATE PROCEDURE ptest6c(inout a anyelement, inout b anyelement)
+LANGUAGE SQL
+AS $$
+SELECT $1, 1;
+$$;
+CALL ptest6c(1, null);
+ a | b 
+---+---
+ 1 | 1
+(1 row)
+
+CALL ptest6c(1.1, null);  -- fails before v13
+ERROR:  return type mismatch in function declared to return record
+DETAIL:  Final statement returns integer instead of numeric at column 2.
+CONTEXT:  SQL function "ptest6c" during startup
 -- collation assignment
 CREATE PROCEDURE ptest7(a text, b text)
 LANGUAGE SQL

diff --git a/src/test/regress/sql/create_procedure.sql b/src/test/regress/sql/create_procedure.sql
index 2dfa0ac2fd36fd80cb565c256578b1b1499560ae..0ba98fe240f4d846af9f80bdaca18e57d6380326 100644 (file)
--- a/src/test/regress/sql/create_procedure.sql
+++ b/src/test/regress/sql/create_procedure.sql
@@ -127,6 +127,15 @@ $$;
 CALL ptest6b(1, null, null);
 CALL ptest6b(1.1, null, null);
 
+CREATE PROCEDURE ptest6c(inout a anyelement, inout b anyelement)
+LANGUAGE SQL
+AS $$
+SELECT $1, 1;
+$$;
+
+CALL ptest6c(1, null);
+CALL ptest6c(1.1, null);  -- fails before v13
+
 
 -- collation assignment