Fix failure to cover scalar-vs-rowtype cases in exec_stmt_return().

In commit 9e3ad1aac52454569393a947c06be0d301749362 I modified plpgsql
to use exec_stmt_return's simple-variables fast path in more cases.
However, I overlooked that there are really two different return
conventions in use here, depending on whether estate->retistuple is true,
and the existing fast-path code had only bothered to handle one of them.
So trying to return a scalar in a function returning composite, or vice
versa, could lead to unexpected error messages (typically "cache lookup
failed for type 0") or to a null-pointer-dereference crash.

In the DTYPE_VAR case, we can just throw error if retistuple is true,
corresponding to what happens in the general-expression code path that was
being used previously.  (Perhaps someday both of these code paths should
attempt a coercion, but today is not that day.)

In the REC and ROW cases, just hand the problem to exec_eval_datum()
when not retistuple.  Also clean up the ROW coding slightly so it looks
more like exec_eval_datum().

The previous commit also caused exec_stmt_return_next() to be used in
more cases, but that code seems to be OK as-is.

Per off-list report from Serge Rielau.  This bug is new in 9.5 so no need
to back-patch.

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index aac7cdaf7cbb1bcb4c21521b51c740a1f95da749..79dd6a22fcee485d9e6a9d2c6f2c73893d83840a 100644 (file)
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2508,6 +2508,7 @@ exec_stmt_return(PLpgSQL_execstate *estate, PLpgSQL_stmt_return *stmt)
        estate->retval = (Datum) 0;
        estate->rettupdesc = NULL;
        estate->retisnull = true;
+       estate->rettype = InvalidOid;
 
        /*
         * Special case path when the RETURN expression is a simple variable
@@ -2534,18 +2535,40 @@ exec_stmt_return(PLpgSQL_execstate *estate, PLpgSQL_stmt_return *stmt)
                                        estate->retval = var->value;
                                        estate->retisnull = var->isnull;
                                        estate->rettype = var->datatype->typoid;
+
+                                       /*
+                                        * Cope with retistuple case.  A PLpgSQL_var could not be
+                                        * of composite type, so we needn't make any effort to
+                                        * convert.  However, for consistency with the expression
+                                        * code path, don't throw error if the result is NULL.
+                                        */
+                                       if (estate->retistuple && !estate->retisnull)
+                                               ereport(ERROR,
+                                                               (errcode(ERRCODE_DATATYPE_MISMATCH),
+                                                                errmsg("cannot return non-composite value from function returning composite type")));
                                }
                                break;
 
                        case PLPGSQL_DTYPE_REC:
                                {
                                        PLpgSQL_rec *rec = (PLpgSQL_rec *) retvar;
+                                       int32           rettypmod;
 
                                        if (HeapTupleIsValid(rec->tup))
                                        {
-                                               estate->retval = PointerGetDatum(rec->tup);
-                                               estate->rettupdesc = rec->tupdesc;
-                                               estate->retisnull = false;
+                                               if (estate->retistuple)
+                                               {
+                                                       estate->retval = PointerGetDatum(rec->tup);
+                                                       estate->rettupdesc = rec->tupdesc;
+                                                       estate->retisnull = false;
+                                               }
+                                               else
+                                                       exec_eval_datum(estate,
+                                                                                       retvar,
+                                                                                       &estate->rettype,
+                                                                                       &rettypmod,
+                                                                                       &estate->retval,
+                                                                                       &estate->retisnull);
                                        }
                                }
                                break;
@@ -2553,15 +2576,28 @@ exec_stmt_return(PLpgSQL_execstate *estate, PLpgSQL_stmt_return *stmt)
                        case PLPGSQL_DTYPE_ROW:
                                {
                                        PLpgSQL_row *row = (PLpgSQL_row *) retvar;
+                                       int32           rettypmod;
 
-                                       Assert(row->rowtupdesc);
-                                       estate->retval =
-                                               PointerGetDatum(make_tuple_from_row(estate, row,
-                                                                                                                       row->rowtupdesc));
-                                       if (DatumGetPointer(estate->retval) == NULL)            /* should not happen */
-                                               elog(ERROR, "row not compatible with its own tupdesc");
-                                       estate->rettupdesc = row->rowtupdesc;
-                                       estate->retisnull = false;
+                                       if (estate->retistuple)
+                                       {
+                                               HeapTuple       tup;
+
+                                               if (!row->rowtupdesc)   /* should not happen */
+                                                       elog(ERROR, "row variable has no tupdesc");
+                                               tup = make_tuple_from_row(estate, row, row->rowtupdesc);
+                                               if (tup == NULL)                /* should not happen */
+                                                       elog(ERROR, "row not compatible with its own tupdesc");
+                                               estate->retval = PointerGetDatum(tup);
+                                               estate->rettupdesc = row->rowtupdesc;
+                                               estate->retisnull = false;
+                                       }
+                                       else
+                                               exec_eval_datum(estate,
+                                                                               retvar,
+                                                                               &estate->rettype,
+                                                                               &rettypmod,
+                                                                               &estate->retval,
+                                                                               &estate->retisnull);
                                }
                                break;
 

diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 78e5a85810e26ec23ee98a219393e53e4834813d..7ce5415f053a0c5c268e651daaa1199eb492dd13 100644 (file)
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -4001,6 +4001,38 @@ $$ language plpgsql;
 select compos();
 ERROR:  cannot return non-composite value from function returning composite type
 CONTEXT:  PL/pgSQL function compos() line 3 at RETURN
+-- RETURN variable is a different code path ...
+create or replace function compos() returns compostype as $$
+declare x int := 42;
+begin
+  return x;
+end;
+$$ language plpgsql;
+select * from compos();
+ERROR:  cannot return non-composite value from function returning composite type
+CONTEXT:  PL/pgSQL function compos() line 4 at RETURN
+drop function compos();
+-- test: invalid use of composite variable in scalar-returning function
+create or replace function compos() returns int as $$
+declare
+  v compostype;
+begin
+  v := (1, 'hello');
+  return v;
+end;
+$$ language plpgsql;
+select compos();
+ERROR:  invalid input syntax for integer: "(1,hello)"
+CONTEXT:  PL/pgSQL function compos() while casting return value to function's return type
+-- test: invalid use of composite expression in scalar-returning function
+create or replace function compos() returns int as $$
+begin
+  return (1, 'hello')::compostype;
+end;
+$$ language plpgsql;
+select compos();
+ERROR:  invalid input syntax for integer: "(1,hello)"
+CONTEXT:  PL/pgSQL function compos() while casting return value to function's return type
 drop function compos();
 drop type compostype;
 --

diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index e19e415386775a4c3221ba180f795ce446f016ba..aaf3e8479fef5e53ed71883debe53b89a214922c 100644 (file)
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -3248,6 +3248,39 @@ $$ language plpgsql;
 
 select compos();
 
+-- RETURN variable is a different code path ...
+create or replace function compos() returns compostype as $$
+declare x int := 42;
+begin
+  return x;
+end;
+$$ language plpgsql;
+
+select * from compos();
+
+drop function compos();
+
+-- test: invalid use of composite variable in scalar-returning function
+create or replace function compos() returns int as $$
+declare
+  v compostype;
+begin
+  v := (1, 'hello');
+  return v;
+end;
+$$ language plpgsql;
+
+select compos();
+
+-- test: invalid use of composite expression in scalar-returning function
+create or replace function compos() returns int as $$
+begin
+  return (1, 'hello')::compostype;
+end;
+$$ language plpgsql;
+
+select compos();
+
 drop function compos();
 drop type compostype;