Re: arrays as pl/perl input arguments [PATCH]

On Feb 6, 2011, at 9:43 AM, Alex Hunsaker wrote:

> On Thu, Feb 3, 2011 at 18:29, Alex Hunsaker <badalex(at)gmail(dot)com> wrote:
>> On Mon, Jan 31, 2011 at 01:34, Alexey Klyukin <alexk(at)commandprompt(dot)com> wrote:
>>> I've looked at the patch and added a test for arrays exceeding or equal maximum dimensions to check, whether the recursive function won't bring surprises there. I've also added check_stack_depth calls to both split_array and plperl_hash_from_tuple. Note that the regression fails currently due to the incorrect error reporting in
>>> PostgreSQL, per http://archives.postgresql.org/pgsql-hackers/2011-01/msg02888.php.
>>>
>>> The v5 is attached.
>
>> One thing I find odd is we allow for nested arrays, but not nested
>> composites. For example:
> ...
>> On the other end, the same is true when returning. If you try to
>> return a nested composite or array, the child better be a string as it
>> wont let you pass a hash:
>
> So here is a v6 that does the above. It does so by providing a generic
> plperl_sv_to_datum() function and converting the various places to use
> it. One cool thing is you can now use the composite types or arrays
> passed in (or returned from another spi!) as arguments for
> spi_exec_prepared(). Before you would have to convert the hashes to
> strings. It also provides a real plperl_convert_to_pg_array (now
> plperl_array_to_datum) that can handle composite and other random
> datatypes. This also means we don't have to convert arrays to a string
> representation first. (which removes part of the argument for #5 @
> http://archives.postgresql.org/pgsql-hackers/2011-02/msg00197.php)
>
> I have attached a stand alone version of the above so its easier to
> look over. The diffstat is fairly small (ignoring added regression
> tests)
> 1 files changed, 259 insertions(+), 165 deletions(-)
>
> Comments?

Thanks, looks great to me. It passes all the tests on my OS X system. I wonder
what's the purpose of the amagic_call in get_perl_array_ref, instead of
calling newRV_noinc on the target SV * ?

Also, in array_to_datum (line 1050), if get_perl_array_ref returns NULL for
the first element of the corresponding dimension, but non-NULL for the second
one - it would use uninitialized dims[cur_depth] value in comparison (which,
although, would probably lead to the desired comparison result).

/A
--
Alexey Klyukin
The PostgreSQL Company - Command Prompt, Inc.