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

From: Alex Hunsaker <badalex(at)gmail(dot)com>
To: Alexey Klyukin <alexk(at)commandprompt(dot)com>
Cc: "David E(dot) Wheeler" <david(at)kineticode(dot)com>, pgsql-hackers(at)postgresql(dot)org, Andrew Dunstan <andrew(at)dunslane(dot)net>
Subject: Re: arrays as pl/perl input arguments [PATCH]
Date: 2011-02-09 01:44:00
Message-ID: AANLkTin7t0+tMqagterE0PUxeG=wtLHotzS6hN4WdrA0@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Feb 8, 2011 at 10:33, Alex Hunsaker <badalex(at)gmail(dot)com> wrote:
> On Tue, Feb 8, 2011 at 08:18, Alexey Klyukin <alexk(at)commandprompt(dot)com> wrote:
>>
>> On Feb 6, 2011, at 9:43 AM, Alex Hunsaker wrote:
>
>>> So here is a v6
>>> ....
>>> 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 * ?
>

> Err, even simpler would be to just access the 'array' member directly
out of the hash object.

Done as the above, an added bonus is we no longer have to SvREF_dec()
so its even simpler.

>> 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).
>
> Good catch, will fix. All of those checks should be outside of the while loop.

I clearly needed more caffeine in my system when i wrote that. Partly
due to the shadowed "av" variable. I've fixed it by initiating all of
the dims to 0. I also renamed the shadowed av var to "nav" for nested
av.

> While Im at it Ill also rebase against HEAD (im sure there will be
> some conflicts with that other plperl patch that just went in ;)).

So the merge while not exactly trivial was fairly simple. However it
would be great if you could give it another look over.

Find attached v7 changes include:
- rebased against HEAD
- fix potential use of uninitialized dims[cur_depth]
- took out accidental (broken) hack to try and support composite types
in ::encode_array_literal (added in v4 or something)
- make_array_ref() now uses plperl_hash_from_datum() for composite
types instead of its own hand rolled version
- get_perl_array_ref() now grabs the 'array' directly instead of
through the magic interface for simplicity
- moved added static declarations to the "bottom" instead of being
half on top and half on bottom

Attachment Content-Type Size
pg_to_perl_arrays_v7.patch.gz application/x-gzip 16.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Glenn Maynard 2011-02-09 01:50:06 Re: Issues with generate_series using integer boundaries
Previous Message Andrew Dunstan 2011-02-09 01:37:57 Re: Revised patches to add table function support to PL/Tcl (TODO item)