Re: pl/perl example in the doc no longer works in 9.1

On Wed, Oct 12, 2011 at 15:00, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "David E. Wheeler" <david(at)kineticode(dot)com> writes:
>> On Oct 12, 2011, at 9:15 AM, Tom Lane wrote:
>>> Well, the real question is why a function declared to return VOID cares
>>> at all about what the last command in its body is.  If this has changed
>>> since previous versions then I think it's a bug and we should fix it,
>>> not just change the example.
>
>> It has indeed changed, either in 9.0 or 9.1 (the latter, I think). I had to add a bunch of bare return statements to existing functions.

[ For those that missed it, 9.0 is OK, it is indeed a bug in 9.1. ]

> This appears to have gotten broken in commit
> 87bb2ade2ce646083f39d5ab3e3307490211ad04, which changed the function
> return code to go through plperl_sv_to_datum, which is making
> unwarranted assumptions ... but since it's utterly bereft of comments,
> it's hard for a non Perl hacker to identify exactly what it should do
> instead.

Yeah, its a mess.

> The core of the problem seems to be that if SvROK(sv) then
> the code assumes that it must be intended to convert that to an array or
> composite, no matter whether the declared result type of the function is
> compatible with such a thing.

Hrm, well 9.0 and below did not get this "right" either:
create or replace function test_hash() returns text as $$ return
{'a'=>1}; $$ language plperl;
select test_array();
test_array
-----------------------
ARRAY(0x7fd92384dcb8)
(1 row)

create or replace function test_hash() returns text as $$ return
{'a'=>1}; $$ language plperl;
select test_hash();
test_hash
----------------------
HASH(0x7fd92387f848)
(1 row)

9.1 does this:
select test_array();
test_array
------------
\x01
(1 row)

select test_hash();
ERROR: type text is not composite
CONTEXT: PL/Perl function "test_hash"

>  So I think this probably broke not only
> VOID-result cases, but also cases where a Perl array or hash is supposed
> to be returned as, say, text.

Given the output above (both pre 9.1 and post) it seems unless the
type is a set or composite we should throw an error. Maybe "PL/Perl
function returning type %s must not return a reference" ?

>  It would be more appropriate to drive the
> cases off the nature of the function result type, perhaps.

Ill see if I can cook up something that's not too invasive.

[ I have a patch to fix the VOID issues. it gets rid of that horrid
has_retval variable/logic and makes it look much closer to 9.0's code.
Unfortunately its on my laptop at home as I was hacking on it before I
went to work... ]