Hi, I forgot to add documentation changes in the earlier patch. In the attached patch, I have added documentation as well as fixed other issues you raised.
Regards, --Asif On Thu, Nov 22, 2012 at 10:47 PM, Asif Rehman <asifr.reh...@gmail.com>wrote: > Thanks for the review Pavel. I have taken care of the points you raised. > Please see the updated patch. > > Regards, > --Asif > > > On Wed, Nov 21, 2012 at 1:47 AM, Pavel Stehule <pavel.steh...@gmail.com>wrote: > >> related to >> http://archives.postgresql.org/message-id/caauglxwpedfwae6dajmf7sxewfusa0f68p07retbbpf_fsa...@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 >> > >
return_rowtype.upd.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
