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

On Thu, Feb 03, 2011 at 02:23:32PM +0200, Alexey Klyukin wrote:
> Hi,
>
> On Feb 2, 2011, at 7:16 PM, Tim Bunce wrote:
>
> > I'm sorry I'm late to this party. I haven't been keeping up with pgsql-hackers.
>
> Better late than never :)
>
> > I'd kind'a hoped that this functionality could be tied into extending
> > PL/Perl to handle named arguments. That way the perl variables
> > corresponding to the named arguments could be given references without
> > breaking any code.
>
> Franky I don't see a direct connection between conversion of arrays
> into array references and supporting named arguments.

There is no direct connection. I wasn't clear, "tied" was too strong a
word for it.

> Could you, please, show some examples of how you hoped the
> functionality could be extended?

I wasn't suggesting extending the functionality, just a way of adding
the functionality that wouldn't risk impacting existing code.

Imagine that PL/Perl could handle named arguments:

CREATE FUNCTION join_list( separator text, list array ) AS $$
return join( $separator, @$list );
$$ LANGUAGE plperl;

The $list variable, magically created by PL/Perl, could be the array
reference created by your code, without altering the contents of @_.

Existing code that does the traditional explicit unpacking of @_
wouldn't be affected. So there would be no need for the string overload
hack which, although I suggested it, I'm a little uncomfortable with.
(The problems with recursion and the need for is_array_ref with
hardwired class names are a little troubling. Not to mention the
extra overheads in accessing the array.)

On the ther hand, a string version of the array would still need to be
created for @_.

> > Some observations on the current code (based on a quick skim):
> >
> > - I'd like to see the conversion function exposed as a builtin
> > $ref = decode_array_literal("{...}");
>
> In principal, I think that's not hard to built with the functionality provided
> by this patch. I see this as an XS function which takes the array string,
> calls the array_in to convert it to the array datum, and, finally, calls
> plperl_ref_from_pg_array (provided by the patch) to produce the resulting
> array reference.

I think that would be useful.

> > - Every existing plperl function that takes arrays is going to get
> > slower due to the overhead of parsing the string and allocating the
> > array and all its elements.
>
> Well, per my understanding of Alex changes, the string parsing is not invoked
> unless requested by referencing the array in a string context. Normally, onle
> plperl_ref_from_pg_array will be invoked every time the function is called
> with an array argument, which would take little time to convert the PostgreSQL
> internal array representation (not a string) to the perl references, but that's
> no different from what is already done with composite type arguments, which
> are converted to perl hash references on every corresponding function call.

I'd missed that it was using the internal array representation (obvious
in hindsight) but there's still a significant cost in allocating the SVs
that won't be used by existing code. Though I agree it's of the same
order as for composite types.

> > - Some of those functions may not use the array at all and some may
> > simply pass it on as an argument to another function.
>
> I don't see how it would be good to optimize for the functions that are
> declared to get the array but in fact do nothing with it. And considering the
> case of passing an array through to another function, it's currently not
> possible to call another pl/perl function from an existing one directly, and
> when passing muti-dimensional arrays to a perl function one would need to
> convert it to the array references anyway.

I was thinking of calls to other pl/perl functions via sql.

> > - Making the conversion lazy would be a big help.
>
> Converting it to string is already lazy, and, per the argumens above, I don't
> see a real benefit of lazy conversion to the perl reference (as comparing to
> the hurdles of implementing that).

I (now) agree. Thanks.

Tim.