Re: Enable pl/python to return records based on multiple OUT params

attached is a patch which enables plpython to recognize function with
multiple OUT params as returning a record

py=# create or replace function addsub( in i1 int, in i2 int,
py=# out o1 int, out o2 int) as
py=# $$
py$# return i1 + i2, i1-i2
py$# $$ language plpythonu;
CREATE FUNCTION
py=#
py=# select * from addsub(1,2);
o1 | o2
----+----
3 | -1
(1 row)

This version is quite rough, though passes tests here.

I will clean it up more during commitfest.

--
------------------------------------------
Hannu Krosing http://www.2ndQuadrant.com
PostgreSQL Scalability and Availability
Services, Consulting and Training

On Sat, 2008-11-01 at 06:13 +0200, Hannu Krosing wrote:
> attached is a patch which enables plpython to recognize function with
> multiple OUT params as returning a record

Overrides previous patch.

Fixed some bugs, added regression tests.

> This version is quite rough, though passes tests here.
>
> I will clean it up more during commitfest.

probably still more things to do

------------------------------------------
Hannu Krosing http://www.2ndQuadrant.com
PostgreSQL Scalability and Availability
Services, Consulting and Training

Hannu Krosing <hannu(at)2ndQuadrant(dot)com> writes:
> On Sat, 2008-11-01 at 06:13 +0200, Hannu Krosing wrote:
>> This version is quite rough, though passes tests here.
>>
>> I will clean it up more during commitfest.

> probably still more things to do

The status of this patch isn't clear --- are you still working on it?
There certainly appear to be a lot of debug leftovers that need to
be removed, error messages to clean up, etc.

regards, tom lane

On Mon, 2008-11-03 at 19:07 -0500, Tom Lane wrote:
> Hannu Krosing <hannu(at)2ndQuadrant(dot)com> writes:
> > On Sat, 2008-11-01 at 06:13 +0200, Hannu Krosing wrote:
> >> This version is quite rough, though passes tests here.
> >>
> >> I will clean it up more during commitfest.
>
> > probably still more things to do
>
> The status of this patch isn't clear --- are you still working on it?
> There certainly appear to be a lot of debug leftovers that need to
> be removed, error messages to clean up, etc.

It passes all existing regression tests and works fine for "correct"
use, but even the code currently in CVS crashes the backend for this

py=# create or replace function add_any(in i1 anyelement, in i2
anyelement, out t text) language plpythonu as $$
return i1 + i2
$$;
CREATE FUNCTION
py=# select * from add_any(1,2);
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
!>

Though it is a somewhat separate problem from current patch I'd like to
do something about it before having it all committed, as the fix must
touch the very same places than this patch.

I think it takes two-tree days to figure out proper way to fix it.

I'd like it to just accept ANY* and do the right thing but I may end up
just rejecting ANY* on both IN and OUT args.

------------------------------------------
Hannu Krosing http://www.2ndQuadrant.com
PostgreSQL Scalability and Availability
Services, Consulting and Training

Hannu Krosing <hannu(at)2ndQuadrant(dot)com> writes:
> ... even the code currently in CVS crashes the backend for this

> py=# create or replace function add_any(in i1 anyelement, in i2
> anyelement, out t text) language plpythonu as $$
> return i1 + i2
> $$;
> CREATE FUNCTION
> py=# select * from add_any(1,2);
> server closed the connection unexpectedly

Well, that's just a stupid uninitialized-variable bug :-(

regards, tom lane

On Tue, 2008-11-04 at 09:57 -0500, Tom Lane wrote:
> Hannu Krosing <hannu(at)2ndQuadrant(dot)com> writes:
> > ... even the code currently in CVS crashes the backend for this
>
> > py=# create or replace function add_any(in i1 anyelement, in i2
> > anyelement, out t text) language plpythonu as $$
> > return i1 + i2
> > $$;
> > CREATE FUNCTION
> > py=# select * from add_any(1,2);
> > server closed the connection unexpectedly
>
> Well, that's just a stupid uninitialized-variable bug :-(

there are probably more complex ones, if a ANYELEMENT taking function is
used more than one time, with different types of args

> regards, tom lane
--
------------------------------------------
Hannu Krosing http://www.2ndQuadrant.com
PostgreSQL Scalability and Availability
Services, Consulting and Training

On Sat, Nov 1, 2008 at 7:52 AM, Hannu Krosing <hannu(at)2ndquadrant(dot)com> wrote:
> On Sat, 2008-11-01 at 06:13 +0200, Hannu Krosing wrote:
>> attached is a patch which enables plpython to recognize function with
>> multiple OUT params as returning a record
>
> Overrides previous patch.
>
> Fixed some bugs, added regression tests.

Hi Hannu:

I was wondering if it would be possible to get plpython to convert IN
parameters of type ARRAY to Python lists? I see some example functions
here [1], but it would be nice if it was done automatically.

David

1. http://archives.postgresql.org/pgsql-general/2007-01/msg01417.php

On Tue, 2008-11-04 at 14:05 -0500, David Blewett wrote:
> On Sat, Nov 1, 2008 at 7:52 AM, Hannu Krosing <hannu(at)2ndquadrant(dot)com> wrote:
> > On Sat, 2008-11-01 at 06:13 +0200, Hannu Krosing wrote:
> >> attached is a patch which enables plpython to recognize function with
> >> multiple OUT params as returning a record
> >
> > Overrides previous patch.
> >
> > Fixed some bugs, added regression tests.
>
> Hi Hannu:
>
> I was wondering if it would be possible to get plpython to convert IN
> parameters of type ARRAY to Python lists? I see some example functions
> here [1], but it would be nice if it was done automatically.

This is one thing I definitely will do, praobably right after getting
ANY* to work, maybe even before, if getting ANY* to work requires too
many changes.

One open question is how to translate arrays with non-default subscript
values

Quote: "Subscripted assignment allows creation of arrays that do not use
one-based subscripts. For example one might assign to myarray[-2:7] to
create an array with subscript values running from -2 to 7."

Should I just shift it to standard python tuple, or would it be better
to return it as a dictionary with keys from -2 to 7

sample:

hannu=# create table ta(ia int[]);
CREATE TABLE
hannu=# insert into ta values('{27000,27000}');
INSERT 0 1
hannu=# update ta set ia[-2:1] = '{-2,-1,0,1}';
UPDATE 1
hannu=# select * from ta;
ia
--------------------------
[-2:2]={-2,-1,0,1,27000}
(1 row)

and if I do return a dictionary , the after this

hannu=# update ta set ia[7:7] = '{7}';
UPDATE 1
hannu=# select * from ta;
ia
------------------------------------------------
[-2:7]={-2,-1,0,1,27000,NULL,NULL,NULL,NULL,7}
(1 row)

should the returned python dict have keys 3-6 with None, or could they
just be omitted ?

Actually I have quite long todo list (attached) of what I'd like to do
with pl/python, but I'm not sure how much will be accepted in 8.4 under
current commitfest/feature freeze scheme.

Anyway, I should put that up on wiki for comments.

> David
>
> 1. http://archives.postgresql.org/pgsql-general/2007-01/msg01417.php
>
--
------------------------------------------
Hannu Krosing http://www.2ndQuadrant.com
PostgreSQL Scalability and Availability
Services, Consulting and Training

On Tue, Nov 4, 2008 at 4:17 PM, Hannu Krosing <hannu(at)2ndquadrant(dot)com> wrote:
> One open question is how to translate arrays with non-default subscript
> values
>
> Quote: "Subscripted assignment allows creation of arrays that do not use
> one-based subscripts. For example one might assign to myarray[-2:7] to
> create an array with subscript values running from -2 to 7."
>
> Should I just shift it to standard python tuple, or would it be better
> to return it as a dictionary with keys from -2 to 7

I think changing the base type is bound to cause issues. For example,
suppose someone expects to be able to simply iterate over the array.
If they're assuming it's a list, they will expect the values to be
returned. If it's a dictionary, the keys will be. If you're going to
do that, you'd need to do a custom dict class that iterated over the
values I think.

David Blewett

> Though it is a somewhat separate problem from current patch I'd like to
> do something about it before having it all committed, as the fix must
> touch the very same places than this patch.
>
> I think it takes two-tree days to figure out proper way to fix it.
>
> I'd like it to just accept ANY* and do the right thing but I may end up
> just rejecting ANY* on both IN and OUT args.

The text above makes it sound like you're still working on this, but
that was more than three weeks ago. What is the status of this now?

...Robert

On Wed, 2008-11-26 at 23:17 -0500, Robert Haas wrote:
> > Though it is a somewhat separate problem from current patch I'd like to
> > do something about it before having it all committed, as the fix must
> > touch the very same places than this patch.
> >
> > I think it takes two-tree days to figure out proper way to fix it.
> >
> > I'd like it to just accept ANY* and do the right thing but I may end up
> > just rejecting ANY* on both IN and OUT args.
>
> The text above makes it sound like you're still working on this, but
> that was more than three weeks ago. What is the status of this now?

It is ready to be applied without support for ANY*, just with the patch
for uninitialized pointer vector (uses pymalloc0 instead of pymalloc )
tom mentioned.

I am working on getting any supported as well though I was tied up with
other things wor last 1.5 weeks

--
------------------------------------------
Hannu Krosing http://www.2ndQuadrant.com
PostgreSQL Scalability and Availability
Services, Consulting and Training

Hannu Krosing <hannu(at)2ndQuadrant(dot)com> writes:
> On Mon, 2008-11-03 at 19:07 -0500, Tom Lane wrote:
>> The status of this patch isn't clear --- are you still working on it?
>> There certainly appear to be a lot of debug leftovers that need to
>> be removed, error messages to clean up, etc.

> It passes all existing regression tests and works fine for "correct"
> use,

I'm returning this patch for rework. You still have done nothing about
the above complaints (useless noise added to elog messages, elog(NOTICE)
debug messages that should have been removed, etc). I started to clean
this up myself but got too annoyed when I found that the patch had
removed security-critical checks that reject pseudotype result types.
I have other things to do than clean up WIP patches.

regards, tom lane