Re: why can't plpgsql return a row-expression? - Mailing list pgsql-hackers

Hello

I fully agree with Asif's arguments

previous tupconvert implementation was really strict - so using
enhanced tupconver has very minimal impact for old user - and I
checked same speed for plpgsql function - patched and unpatched pg.

tested

CREATE OR REPLACE FUNCTION public.foo(i integer)
 RETURNS SETOF record
 LANGUAGE plpgsql
AS $function$
declare r record;
begin
  r := (10.1,20.1); for i in 1..10 loop return next r; end loop; return;
end;
$function$

select sum(a) from generate_series(1,3000) g(i), lateral foo(i) as (a
numeric, b numeric);

More - everywhere where we use tupconvert internally, then we use
cached conversion map - so I am sure, so speed of ANALYZE cannot be
impacted by this patch

There are other two issue:

it allow to write new differnt slow code - IO cast is about 5-10-20%
slower, and with this path anybody has new possibilities for new "bad"
code. But it is not a problem of this patch. It is related to plpgsql
design and probably we should to move some conversions to outside
plpgsql to be possible reuse conversion map and enhance plpgsql. I
have a simple half solutions - plpgsql_check_function can detect this
situation in almost typical use cases and can raises a performance
warning. So this issue is not stopper for me - because it is not new
issue in plpgsql.

Second issue is more significant:

there are bug:

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
foo(i) as (a numeric, b numeric);
   sum
----------
 303000.0
(1 row)

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
foo(i) as (a float, b numeric);
          sum
-----------------------
 7.33675699577682e-232
(1 row)

it produces wrong result

And with minimal change it kill session

create or replace function foo(i integer)
returns setof record as $$
declare r record;
begin
  r := (10,20); for i in 1..10 loop return next r; end loop; return;
end;
$$ language plpgsql;

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
foo(i) as (a numeric, b numeric);
The connection to the server was lost. Attempting reset: Failed.

create or replace function fo(i integer)
returns record as $$
declare r record;
begin
  r := (10,20); return r;
end;
$$ language plpgsql;

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
fo(i) as (a int, b numeric);
  sum
-------
 30000
(1 row)

postgres=# select sum(a) from generate_series(1,3000) g(i), lateral
fo(i) as (a numeric, b numeric);
The connection to the server was lost. Attempting reset: Failed.

Regards

Pavel Stehule


2012/12/3 Asif Rehman <asifr.rehman@gmail.com>:

> Hi,
>
> Thanks for the review. Please see the updated patch.
>
>> Hmm.  We're running an I/O cast during do_tup_convert() now, and look
>> up the required functions for each tuple.  I think if we're going to
>> support this operation at that level, we need to look up the necessary
>> functions at convert_tuples_by_foo level, and then apply unconditionally
>> if they've been set up.
>
> Done. TupleConversionMap struct should keep the array of functions oid's
> that
> needs to be applied. Though only for those cases where both attribute type
> id's
> do not match. This way no unnecessary casting will happen.
>
>>
>> Also, what are the implicancies for existing users of tupconvert?  Do we
>> want to apply casting during ANALYZE for example?  AFAICS the patch
>> shouldn't break any case that works today, but I don't see that there
>> has been any analysis of this.
>
> I believe this part of the code should not impact existing users of
> tupconvert.
> As this part of code is relaxing a restriction upon which an error would
> have been
> generated. Though it could have a little impact on performance but should be
> only for
> cases where attribute type id's are different and are implicitly cast able.
>
> Besides existing users involve tupconvert in case of inheritance. And in
> that case
> an exact match is expected.
>
> Regards,
> --Asif