review: plpgsql return a row-expression

related to http://archives.postgresql.org/message-id/CAAuGLxWpEDfwAE6DAJMF7SxEwFUsA0f68P07RetBbpf_FSaShA@mail.gmail.com

* patched and compiled withou warnings

* All 133 tests passed.

but

I don't like

* call invalid function from anonymous block - it is messy (regress
tests) - there is no reason why do it

+create or replace function foo() returns footype as $$
+declare
+ v record;
+ v2 record;
+begin
+ v := (1, 'hello');
+ v2 := (1, 'hello');
+ return (v || v2);
+end;
+$$ language plpgsql;
+DO $$
+declare
+ v footype;
+begin
+ v := foo();
+ raise info 'x = %', v.x;
+ raise info 'y = %', v.y;
+end; $$;
+ERROR: operator does not exist: record || record
+LINE 1: SELECT (v || v2)
+ ^

* there is some performance issue

create or replace function fx2(a int)
returns footype as $$
declare x footype;
begin
x = (10,20);
return x;
end;
$$ language plpgsql;

postgres=# select sum(fx2.x) from generate_series(1,100000) g(i),
lateral fx2(i);
sum
---------
1000000
(1 row)

Time: 497.129 ms

returns footype as $$
begin
return (10,20);
end;
$$ language plpgsql;

postgres=# select sum(fx2.x) from generate_series(1,100000) g(i),
lateral fx2(i);
sum
---------
1000000
(1 row)

Time: 941.192 ms

following code has same functionality and it is faster

if (stmt->expr != NULL)
{
if (estate->retistuple)
{
TupleDesc tupdesc;
Datum retval;
Oid rettype;
bool isnull;
int32 tupTypmod;
Oid tupType;
HeapTupleHeader td;
HeapTupleData tmptup;

retval = exec_eval_expr(estate,
stmt->expr,
&isnull,
&rettype);

/* Source must be of RECORD or composite type */
if (!type_is_rowtype(rettype))
ereport(ERROR,

(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("cannot return
non-composite value from composite type returning function")));

if (!isnull)
{
/* Source is a tuple Datum, so safe to
do this: */
td = DatumGetHeapTupleHeader(retval);
/* Extract rowtype info and find a tupdesc */
tupType = HeapTupleHeaderGetTypeId(td);
tupTypmod = HeapTupleHeaderGetTypMod(td);
tupdesc =
lookup_rowtype_tupdesc(tupType, tupTypmod);

/* Build a temporary HeapTuple control
structure */
tmptup.t_len =
HeapTupleHeaderGetDatumLength(td);
ItemPointerSetInvalid(&(tmptup.t_self));
tmptup.t_tableOid = InvalidOid;
tmptup.t_data = td;

estate->retval =
PointerGetDatum(heap_copytuple(&tmptup));
estate->rettupdesc =
CreateTupleDescCopy(tupdesc);
ReleaseTupleDesc(tupdesc);
}

estate->retisnull = isnull;
}

* it is to restrictive (maybe) - almost all plpgsql' statements does
automatic conversions (IO conversions when is necessary)

create type footype2 as (a numeric, b varchar)

postgres=# create or replace function fx3(a int)
returns footype2 as
$$
begin
return (10000000.22234213412342143,'ewqerwqreeeee');
end;
$$ language plpgsql;
CREATE FUNCTION
postgres=# select fx3(10);
ERROR: returned record type does not match expected record type
DETAIL: Returned type unknown does not match expected type character
varying in column 2.
CONTEXT: PL/pgSQL function fx3(integer) while casting return value to
function's return type
postgres=#

* it doesn't support RETURN NEXT

postgres=# create or replace function fx4()
postgres-# returns setof footype as $$
postgres$# begin
postgres$# for i in 1..10
postgres$# loop
postgres$# return next (10,20);
postgres$# end loop;
postgres$# return;
postgres$# end;
postgres$# $$ language plpgsql;
ERROR: RETURN NEXT must specify a record or row variable in function
returning row
LINE 6: return next (10,20);

* missing any documentation

* repeated code - following code is used on more places in pl_exec.c
and maybe it is candidate for subroutine

+ /* Source is a tuple Datum, so safe to do this: */
+ td = DatumGetHeapTupleHeader(value);
+ /* Extract rowtype info and find a tupdesc */
+ tupType = HeapTupleHeaderGetTypeId(td);
+ tupTypmod = HeapTupleHeaderGetTypMod(td);
+ tupdesc = lookup_rowtype_tupdesc_copy(tupType, tupTypmod);
+ /* Build a HeapTuple control structure */
+ htup.t_len = HeapTupleHeaderGetDatumLength(td);
+ ItemPointerSetInvalid(&(htup.t_self));
+ htup.t_tableOid = InvalidOid;
+ htup.t_data = td;
+ tuple = heap_copytuple(&htup);

Regards

Pavel Stehule