From ae58f1430abb4b0c309c40b377f91bf9d080334b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 12 Jun 2015 13:44:06 -0400 Subject: [PATCH] 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. --- src/pl/plpgsql/src/pl_exec.c | 58 ++++++++++++++++++++++----- src/test/regress/expected/plpgsql.out | 32 +++++++++++++++ src/test/regress/sql/plpgsql.sql | 33 +++++++++++++++ 3 files changed, 112 insertions(+), 11 deletions(-) diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index aac7cdaf7c..79dd6a22fc 100644 --- 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 78e5a85810..7ce5415f05 100644 --- 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 e19e415386..aaf3e8479f 100644 --- 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; -- 2.40.0