review: plpgsql return a row-expression - Mailing list pgsql-hackers

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,
sosafe to
 
do this: */                               td = DatumGetHeapTupleHeader(retval);                               /*
Extractrowtype 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