On Fri, Nov 20, 2009 at 12:00:24AM +0200, Peter Eisentraut wrote:
> On fre, 2009-11-13 at 18:46 +0300, Teodor Sigaev wrote:
> > CREATE OR REPLACE FUNCTION incr(stuff int[]) RETURNS int[] AS $$
> > for x in stuff:
> > yield x+1
> > $$
> > LANGUAGE 'plpythonu';
> >
> > # select incr(ARRAY[1,2,3]);
> > ERROR: invalid memory alloc request size 18446744073709551608
> > CONTEXT: while creating return value
> > PL/Python function "incr"
>
> Fixed with additional error check and regression test. (The problem
> could be more simply demonstrated by returning any non-sequence from the
> function.) Thanks for catching it.
I've finally gotten around to getting a review done, and basically it looks
good to me. There appears to be a problem with the tests in that the expected
output doesn't include the test_type_conversion_array_error() function
mentioned in sql/plpython_types.sql. Diff generated by the regression test is
attached. Other than that problem, though, the code looks fine to me (should I
presume to judge Peter's code? :). Aside from the problem mentioned above, the
tests work fine, and seem fairly comprehensive. Other testing I've done also
passes.
This patch doesn't include any documentation; my reading of the PL/Python docs
suggests that's probably acceptable, as the existing docs don't talk about its
array handling. That said, it might be useful to include an example, to show
for instance that identical PL/Python code could create either an array of a
type or a set of rows of that type:
5432 josh@josh*# create function return_set() returns setof int as $$ return
(1, 2, 3, 4, 5) $$ language plpythonu;
CREATE FUNCTION
5432 josh@josh*# create function return_arr() returns int[] as $$ return (1,
2, 3, 4, 5) $$ language plpythonu;
CREATE FUNCTION
5432 josh@josh*# select return_arr();return_arr
-------------{1,2,3,4,5}
(1 row)
5432 josh@josh*# select * from return_set();return_set
------------ 1 2 3 4 5
(5 rows)
Perhaps that's overkill, though.
--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com