Skip to content

Commit ae58f14

Browse files
tglsfdctglsfdc
committed
Fix failure to cover scalar-vs-rowtype cases in exec_stmt_return().
In commit 9e3ad1a 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.
1 parent b009823 commit ae58f14

File tree

3 files changed

+112
-11
lines changed

3 files changed

+112
-11
lines changed

‎src/pl/plpgsql/src/pl_exec.c

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2508,6 +2508,7 @@ exec_stmt_return(PLpgSQL_execstate *estate, PLpgSQL_stmt_return *stmt)
25082508
estate->retval = (Datum) 0;
25092509
estate->rettupdesc = NULL;
25102510
estate->retisnull = true;
2511+
estate->rettype = InvalidOid;
25112512

25122513
/*
25132514
* Special case path when the RETURN expression is a simple variable
@@ -2534,34 +2535,69 @@ exec_stmt_return(PLpgSQL_execstate *estate, PLpgSQL_stmt_return *stmt)
25342535
estate->retval = var->value;
25352536
estate->retisnull = var->isnull;
25362537
estate->rettype = var->datatype->typoid;
2538+
2539+
/*
2540+
* Cope with retistuple case. A PLpgSQL_var could not be
2541+
* of composite type, so we needn't make any effort to
2542+
* convert. However, for consistency with the expression
2543+
* code path, don't throw error if the result is NULL.
2544+
*/
2545+
if (estate->retistuple && !estate->retisnull)
2546+
ereport(ERROR,
2547+
(errcode(ERRCODE_DATATYPE_MISMATCH),
2548+
errmsg("cannot return non-composite value from function returning composite type")));
25372549
}
25382550
break;
25392551

25402552
case PLPGSQL_DTYPE_REC:
25412553
{
25422554
PLpgSQL_rec *rec = (PLpgSQL_rec *) retvar;
2555+
int32 rettypmod;
25432556

25442557
if (HeapTupleIsValid(rec->tup))
25452558
{
2546-
estate->retval = PointerGetDatum(rec->tup);
2547-
estate->rettupdesc = rec->tupdesc;
2548-
estate->retisnull = false;
2559+
if (estate->retistuple)
2560+
{
2561+
estate->retval = PointerGetDatum(rec->tup);
2562+
estate->rettupdesc = rec->tupdesc;
2563+
estate->retisnull = false;
2564+
}
2565+
else
2566+
exec_eval_datum(estate,
2567+
retvar,
2568+
&estate->rettype,
2569+
&rettypmod,
2570+
&estate->retval,
2571+
&estate->retisnull);
25492572
}
25502573
}
25512574
break;
25522575

25532576
case PLPGSQL_DTYPE_ROW:
25542577
{
25552578
PLpgSQL_row *row = (PLpgSQL_row *) retvar;
2579+
int32 rettypmod;
25562580

2557-
Assert(row->rowtupdesc);
2558-
estate->retval =
2559-
PointerGetDatum(make_tuple_from_row(estate, row,
2560-
row->rowtupdesc));
2561-
if (DatumGetPointer(estate->retval) == NULL) /* should not happen */
2562-
elog(ERROR, "row not compatible with its own tupdesc");
2563-
estate->rettupdesc = row->rowtupdesc;
2564-
estate->retisnull = false;
2581+
if (estate->retistuple)
2582+
{
2583+
HeapTuple tup;
2584+
2585+
if (!row->rowtupdesc) /* should not happen */
2586+
elog(ERROR, "row variable has no tupdesc");
2587+
tup = make_tuple_from_row(estate, row, row->rowtupdesc);
2588+
if (tup == NULL) /* should not happen */
2589+
elog(ERROR, "row not compatible with its own tupdesc");
2590+
estate->retval = PointerGetDatum(tup);
2591+
estate->rettupdesc = row->rowtupdesc;
2592+
estate->retisnull = false;
2593+
}
2594+
else
2595+
exec_eval_datum(estate,
2596+
retvar,
2597+
&estate->rettype,
2598+
&rettypmod,
2599+
&estate->retval,
2600+
&estate->retisnull);
25652601
}
25662602
break;
25672603

‎src/test/regress/expected/plpgsql.out

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4001,6 +4001,38 @@ $$ language plpgsql;
40014001
select compos();
40024002
ERROR: cannot return non-composite value from function returning composite type
40034003
CONTEXT: PL/pgSQL function compos() line 3 at RETURN
4004+
-- RETURN variable is a different code path ...
4005+
create or replace function compos() returns compostype as $$
4006+
declare x int := 42;
4007+
begin
4008+
return x;
4009+
end;
4010+
$$ language plpgsql;
4011+
select * from compos();
4012+
ERROR: cannot return non-composite value from function returning composite type
4013+
CONTEXT: PL/pgSQL function compos() line 4 at RETURN
4014+
drop function compos();
4015+
-- test: invalid use of composite variable in scalar-returning function
4016+
create or replace function compos() returns int as $$
4017+
declare
4018+
v compostype;
4019+
begin
4020+
v := (1, 'hello');
4021+
return v;
4022+
end;
4023+
$$ language plpgsql;
4024+
select compos();
4025+
ERROR: invalid input syntax for integer: "(1,hello)"
4026+
CONTEXT: PL/pgSQL function compos() while casting return value to function's return type
4027+
-- test: invalid use of composite expression in scalar-returning function
4028+
create or replace function compos() returns int as $$
4029+
begin
4030+
return (1, 'hello')::compostype;
4031+
end;
4032+
$$ language plpgsql;
4033+
select compos();
4034+
ERROR: invalid input syntax for integer: "(1,hello)"
4035+
CONTEXT: PL/pgSQL function compos() while casting return value to function's return type
40044036
drop function compos();
40054037
drop type compostype;
40064038
--

‎src/test/regress/sql/plpgsql.sql

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3248,6 +3248,39 @@ $$ language plpgsql;
32483248

32493249
select compos();
32503250

3251+
-- RETURN variable is a different code path ...
3252+
create or replace function compos() returns compostype as $$
3253+
declare x int := 42;
3254+
begin
3255+
return x;
3256+
end;
3257+
$$ language plpgsql;
3258+
3259+
select * from compos();
3260+
3261+
drop function compos();
3262+
3263+
-- test: invalid use of composite variable in scalar-returning function
3264+
create or replace function compos() returns int as $$
3265+
declare
3266+
v compostype;
3267+
begin
3268+
v := (1, 'hello');
3269+
return v;
3270+
end;
3271+
$$ language plpgsql;
3272+
3273+
select compos();
3274+
3275+
-- test: invalid use of composite expression in scalar-returning function
3276+
create or replace function compos() returns int as $$
3277+
begin
3278+
return (1, 'hello')::compostype;
3279+
end;
3280+
$$ language plpgsql;
3281+
3282+
select compos();
3283+
32513284
drop function compos();
32523285
drop type compostype;
32533286

0 commit comments

Comments
 (0)