Thread: Python setof patch

Python setof patch

From
Gerrit van Dyk
Date:
Hi,

This patch allows the PL/Python module to do (SRF) functions.

The patch was taken from the CVS version.

I have modified the plpython.c file and have added a test sql script for
testing the functionality. It was actually the script that was in the
8.0.3 version but have since been removed.

In order to signal the end of a set, the called python function must
simply return plpy.EndOfSet and the set would be returned.

Please feel free to email me if you experience any problems.

My next project I am working in is to get the PL/Python module to return
records (ie. structured data)

Regards
Gerrit van Dyk
AgileWorks (Pty) Ltd
diff -c -r -P orig/pgsql/src/pl/plpython/plpython.c new/pgsql/src/pl/plpython/plpython.c
*** orig/pgsql/src/pl/plpython/plpython.c    2005-06-15 10:55:02.000000000 +0200
--- new/pgsql/src/pl/plpython/plpython.c    2005-06-14 14:12:01.000000000 +0200
***************
*** 286,291 ****
--- 286,294 ----
  static PyObject *PLy_exc_fatal = NULL;
  static PyObject *PLy_exc_spi_error = NULL;

+ /* End-of-set Indication */
+ static PyObject *PLy_endofset = NULL;
+
  /* some globals for the python module
   */
  static char PLy_plan_doc[] = {
***************
*** 770,775 ****
--- 773,788 ----
              fcinfo->isnull = true;
              rv = (Datum) NULL;
          }
+         /* test for end-of-set condition */
+         else if (fcinfo->flinfo->fn_retset && plrv == PLy_endofset)
+         {
+            ReturnSetInfo *rsi;
+
+            fcinfo->isnull = true;
+            rv = (Datum)NULL;
+            rsi = (ReturnSetInfo *)fcinfo->resultinfo;
+            rsi->isDone = ExprEndResult;
+         }
          else
          {
              fcinfo->isnull = false;
***************
*** 2317,2325 ****
--- 2330,2340 ----
      PLy_exc_error = PyErr_NewException("plpy.Error", NULL, NULL);
      PLy_exc_fatal = PyErr_NewException("plpy.Fatal", NULL, NULL);
      PLy_exc_spi_error = PyErr_NewException("plpy.SPIError", NULL, NULL);
+     PLy_endofset = PyErr_NewException("plpy.EndOfSet",NULL,NULL);
      PyDict_SetItemString(plpy_dict, "Error", PLy_exc_error);
      PyDict_SetItemString(plpy_dict, "Fatal", PLy_exc_fatal);
      PyDict_SetItemString(plpy_dict, "SPIError", PLy_exc_spi_error);
+     PyDict_SetItemString(plpy_dict, "EndOfSet", PLy_endofset);

      /*
       * initialize main module, and add plpy
diff -c -r -P orig/pgsql/src/pl/plpython/sql/plpython_setof.sql new/pgsql/src/pl/plpython/sql/plpython_setof.sql
*** orig/pgsql/src/pl/plpython/sql/plpython_setof.sql    1970-01-01 02:00:00.000000000 +0200
--- new/pgsql/src/pl/plpython/sql/plpython_setof.sql    2005-06-14 14:12:01.000000000 +0200
***************
*** 0 ****
--- 1,12 ----
+
+ CREATE or replace FUNCTION test_setof() returns setof text
+     AS
+ 'if GD.has_key("calls"):
+     GD["calls"] = GD["calls"] + 1
+     if GD["calls"] > 2:
+         del GD["calls"]
+         return plpy.EndOfSet
+ else:
+     GD["calls"] = 1
+ return str(GD["calls"])'
+     LANGUAGE plpythonu;

Re: Python setof patch

From
Michael Fuhr
Date:
On Wed, Jun 15, 2005 at 03:03:46PM +0200, Gerrit van Dyk wrote:
>
> My next project I am working in is to get the PL/Python module to return
> records (ie. structured data)

Hmmm...if you're interested in working on PL/Python, then you might
mosey over to pgsql-hackers and see the "process crash when a
plpython function returns unicode" thread:

http://archives.postgresql.org/pgsql-hackers/2005-06/msg00806.php

I was going to submit a simple patch to fix the immediate problem,
calling ereport(ERROR, ...) if PyObject_Str() returned NULL.  I
haven't decided on an error message yet, not being sure if I should
hard-code something generic or if there's a way to get a more
specific error message from Python.  Tino Wildenhain followed up
with a suggestion to improve PL/Python's Unicode support.  Is that
anything you'd be interested in working on?  If so then please reply
to the thread in pgsql-hackers to keep the discussion in one place
(I already moved it to -hackers from -bugs because the former seemed
more a appropriate place for extended discussion).

--
Michael Fuhr
http://www.fuhr.org/~mfuhr/

Re: Python setof patch

From
Bruce Momjian
Date:
Patch applied.  Thanks.

---------------------------------------------------------------------------


Gerrit van Dyk wrote:
> Hi,
>
> This patch allows the PL/Python module to do (SRF) functions.
>
> The patch was taken from the CVS version.
>
> I have modified the plpython.c file and have added a test sql script for
> testing the functionality. It was actually the script that was in the
> 8.0.3 version but have since been removed.
>
> In order to signal the end of a set, the called python function must
> simply return plpy.EndOfSet and the set would be returned.
>
> Please feel free to email me if you experience any problems.
>
> My next project I am working in is to get the PL/Python module to return
> records (ie. structured data)
>
> Regards
> Gerrit van Dyk
> AgileWorks (Pty) Ltd

> diff -c -r -P orig/pgsql/src/pl/plpython/plpython.c new/pgsql/src/pl/plpython/plpython.c
> *** orig/pgsql/src/pl/plpython/plpython.c    2005-06-15 10:55:02.000000000 +0200
> --- new/pgsql/src/pl/plpython/plpython.c    2005-06-14 14:12:01.000000000 +0200
> ***************
> *** 286,291 ****
> --- 286,294 ----
>   static PyObject *PLy_exc_fatal = NULL;
>   static PyObject *PLy_exc_spi_error = NULL;
>
> + /* End-of-set Indication */
> + static PyObject *PLy_endofset = NULL;
> +
>   /* some globals for the python module
>    */
>   static char PLy_plan_doc[] = {
> ***************
> *** 770,775 ****
> --- 773,788 ----
>               fcinfo->isnull = true;
>               rv = (Datum) NULL;
>           }
> +         /* test for end-of-set condition */
> +         else if (fcinfo->flinfo->fn_retset && plrv == PLy_endofset)
> +         {
> +            ReturnSetInfo *rsi;
> +
> +            fcinfo->isnull = true;
> +            rv = (Datum)NULL;
> +            rsi = (ReturnSetInfo *)fcinfo->resultinfo;
> +            rsi->isDone = ExprEndResult;
> +         }
>           else
>           {
>               fcinfo->isnull = false;
> ***************
> *** 2317,2325 ****
> --- 2330,2340 ----
>       PLy_exc_error = PyErr_NewException("plpy.Error", NULL, NULL);
>       PLy_exc_fatal = PyErr_NewException("plpy.Fatal", NULL, NULL);
>       PLy_exc_spi_error = PyErr_NewException("plpy.SPIError", NULL, NULL);
> +     PLy_endofset = PyErr_NewException("plpy.EndOfSet",NULL,NULL);
>       PyDict_SetItemString(plpy_dict, "Error", PLy_exc_error);
>       PyDict_SetItemString(plpy_dict, "Fatal", PLy_exc_fatal);
>       PyDict_SetItemString(plpy_dict, "SPIError", PLy_exc_spi_error);
> +     PyDict_SetItemString(plpy_dict, "EndOfSet", PLy_endofset);
>
>       /*
>        * initialize main module, and add plpy
> diff -c -r -P orig/pgsql/src/pl/plpython/sql/plpython_setof.sql new/pgsql/src/pl/plpython/sql/plpython_setof.sql
> *** orig/pgsql/src/pl/plpython/sql/plpython_setof.sql    1970-01-01 02:00:00.000000000 +0200
> --- new/pgsql/src/pl/plpython/sql/plpython_setof.sql    2005-06-14 14:12:01.000000000 +0200
> ***************
> *** 0 ****
> --- 1,12 ----
> +
> + CREATE or replace FUNCTION test_setof() returns setof text
> +     AS
> + 'if GD.has_key("calls"):
> +     GD["calls"] = GD["calls"] + 1
> +     if GD["calls"] > 2:
> +         del GD["calls"]
> +         return plpy.EndOfSet
> + else:
> +     GD["calls"] = 1
> + return str(GD["calls"])'
> +     LANGUAGE plpythonu;

>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: the planner will ignore your desire to choose an index scan if your
>       joining column's datatypes do not match

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: Python setof patch

From
Michael Fuhr
Date:
On Mon, Jul 04, 2005 at 03:04:51PM -0400, Bruce Momjian wrote:
>
> Patch applied.  Thanks.
>
> Gerrit van Dyk wrote:
> >
> > This patch allows the PL/Python module to do (SRF) functions.

Does this patch work?  The test_setof() function in sql/plpython_setof.sql
gives me the following:

SELECT * FROM test_setof();
 test_setof
------------
 1
(1 row)

If I call the function again I get this:

SELECT * FROM test_setof();
 test_setof
------------
 2
(1 row)

Calling the function a third time gives this:

SELECT * FROM test_setof();
 test_setof
------------
(0 rows)

Am I misreading the code, or shouldn't the function return two rows
on each invocation?

I don't see the setof functionality in the regression tests,
presumably because there's no expected/plpython_setof.sql file:

============== running regression test queries        ==============
test plpython_schema      ... ok
test plpython_populate    ... ok
test plpython_function    ... ok
test plpython_test        ... ok
test plpython_error       ... ok
test plpython_drop        ... ok

=====================
 All 6 tests passed.
=====================

What about documentation updates?  Still in the works?

--
Michael Fuhr
http://www.fuhr.org/~mfuhr/

Re: Python setof patch

From
Michael Fuhr
Date:
On Tue, Jul 05, 2005 at 04:23:42PM +0200, Gerrit van Dyk wrote:
>
> Ok, I just looked at the code again and the results I am getting, is
> what you expect.
>
> 2 rows every time returning 1 and 2.
>
> What version of postgres are you running, I am running 8.0.3

I'm running HEAD (8.1devel), which is where the patch was committed.
Could somebody else test HEAD and see what they get?

--
Michael Fuhr
http://www.fuhr.org/~mfuhr/

Re: Python setof patch

From
Tom Lane
Date:
Michael Fuhr <mike@fuhr.org> writes:
> I'm running HEAD (8.1devel), which is where the patch was committed.
> Could somebody else test HEAD and see what they get?

Same as you --- ie, this patch is utterly broken.

> I don't see the setof functionality in the regression tests,
> presumably because there's no expected/plpython_setof.sql file:

That's because it wasn't added to the Makefile.  However, a test that
only defines a function and doesn't actually exercise it seems a bit
useless anyway :-(

            regards, tom lane

Re: Python setof patch

From
Tom Lane
Date:
Michael Fuhr <mike@fuhr.org> writes:
>> This patch allows the PL/Python module to do (SRF) functions.

> Does this patch work?

Aside from minor problems like being broken and undocumented, there is a
more serious question here: is this even the functionality we want?

The proposed test case is:

CREATE or replace FUNCTION test_setof() returns setof text
    AS
'if GD.has_key("calls"):
    GD["calls"] = GD["calls"] + 1
    if GD["calls"] > 2:
        del GD["calls"]
        return plpy.EndOfSet
else:
    GD["calls"] = 1
return str(GD["calls"])'
    LANGUAGE plpythonu;

which is essentially exposing the old value-per-call SRF implementation
to the Python programmer.  Do we really want to do that?  plperl and
plpgsql have not taken that tack.  One reason this doesn't seem a
particularly great idea is that the above example would likely fail
if invoked twice in one query, due to it using only one global state
variable for both invocations.

            regards, tom lane

Re: Python setof patch

From
Michael Fuhr
Date:
On Tue, Jul 05, 2005 at 01:14:25PM -0400, Tom Lane wrote:
>
> Aside from minor problems like being broken and undocumented, there is a
> more serious question here: is this even the functionality we want?

I'd rather see something akin to PL/pgSQL's RETURN NEXT or PL/Perl's
return_next.

--
Michael Fuhr
http://www.fuhr.org/~mfuhr/

Re: Python setof patch

From
Andrew Dunstan
Date:

Michael Fuhr wrote:

>On Tue, Jul 05, 2005 at 01:14:25PM -0400, Tom Lane wrote:
>
>
>>Aside from minor problems like being broken and undocumented, there is a
>>more serious question here: is this even the functionality we want?
>>
>>
>
>I'd rather see something akin to PL/pgSQL's RETURN NEXT or PL/Perl's
>return_next.
>
>
>

Agreed. My rudimentary testing shows that plperl's shiny new return_next
functionality not only avoids memory blowup but has a 40% speed
improvement over the old 'return the lot at once' API.

cheers

andrew

Re: Python setof patch

From
Bruce Momjian
Date:
OK, patch backed out, new and regression file removed.

---------------------------------------------------------------------------

Andrew Dunstan wrote:
>
>
> Michael Fuhr wrote:
>
> >On Tue, Jul 05, 2005 at 01:14:25PM -0400, Tom Lane wrote:
> >
> >
> >>Aside from minor problems like being broken and undocumented, there is a
> >>more serious question here: is this even the functionality we want?
> >>
> >>
> >
> >I'd rather see something akin to PL/pgSQL's RETURN NEXT or PL/Perl's
> >return_next.
> >
> >
> >
>
> Agreed. My rudimentary testing shows that plperl's shiny new return_next
> functionality not only avoids memory blowup but has a 40% speed
> improvement over the old 'return the lot at once' API.
>
> cheers
>
> andrew
>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: don't forget to increase your free space map settings
>

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: src/pl/plpython/plpython.c
===================================================================
RCS file: /cvsroot/pgsql/src/pl/plpython/plpython.c,v
retrieving revision 1.62
retrieving revision 1.63
diff -c -r1.62 -r1.63
*** src/pl/plpython/plpython.c    6 May 2005 17:24:55 -0000    1.62
--- src/pl/plpython/plpython.c    4 Jul 2005 18:59:42 -0000    1.63
***************
*** 29,35 ****
   * MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
   *
   * IDENTIFICATION
!  *    $PostgreSQL: pgsql/src/pl/plpython/plpython.c,v 1.62 2005/05/06 17:24:55 tgl Exp $
   *
   *********************************************************************
   */
--- 29,35 ----
   * MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
   *
   * IDENTIFICATION
!  *    $PostgreSQL: pgsql/src/pl/plpython/plpython.c,v 1.63 2005/07/04 18:59:42 momjian Exp $
   *
   *********************************************************************
   */
***************
*** 286,291 ****
--- 286,294 ----
  static PyObject *PLy_exc_fatal = NULL;
  static PyObject *PLy_exc_spi_error = NULL;

+ /* End-of-set Indication */
+ static PyObject *PLy_endofset = NULL;
+
  /* some globals for the python module
   */
  static char PLy_plan_doc[] = {
***************
*** 770,775 ****
--- 773,788 ----
              fcinfo->isnull = true;
              rv = (Datum) NULL;
          }
+         /* test for end-of-set condition */
+         else if (fcinfo->flinfo->fn_retset && plrv == PLy_endofset)
+         {
+            ReturnSetInfo *rsi;
+
+            fcinfo->isnull = true;
+            rv = (Datum)NULL;
+            rsi = (ReturnSetInfo *)fcinfo->resultinfo;
+            rsi->isDone = ExprEndResult;
+         }
          else
          {
              fcinfo->isnull = false;
***************
*** 2317,2325 ****
--- 2330,2340 ----
      PLy_exc_error = PyErr_NewException("plpy.Error", NULL, NULL);
      PLy_exc_fatal = PyErr_NewException("plpy.Fatal", NULL, NULL);
      PLy_exc_spi_error = PyErr_NewException("plpy.SPIError", NULL, NULL);
+     PLy_endofset = PyErr_NewException("plpy.EndOfSet",NULL,NULL);
      PyDict_SetItemString(plpy_dict, "Error", PLy_exc_error);
      PyDict_SetItemString(plpy_dict, "Fatal", PLy_exc_fatal);
      PyDict_SetItemString(plpy_dict, "SPIError", PLy_exc_spi_error);
+     PyDict_SetItemString(plpy_dict, "EndOfSet", PLy_endofset);

      /*
       * initialize main module, and add plpy