Thread: plpython improvements

plpython improvements

From
"Sven Suursoho"
Date:
1) named parameters additionally to args[]
2) return composite-types from plpython as dictionary
3) return result-set from plpython as list, iterator or generator

Test script attached (patch-test.sql) but not integrated to plpython
test-suite.


--
Sven Suursoho

Attachment

Re: plpython improvements

From
Hannu Krosing
Date:
Ühel kenal päeval, L, 2006-04-15 kell 17:59, kirjutas Sven Suursoho:
> 1) named parameters additionally to args[]
> 2) return composite-types from plpython as dictionary
> 3) return result-set from plpython as list, iterator or generator
>
> Test script attached (patch-test.sql) but not integrated to plpython
> test-suite.

If you wonder why you can't apply the patch, it is against postgres
8.0.7.

------------
Hannu


Re: plpython improvements

From
Bruce Momjian
Date:
Hannu Krosing wrote:
> ?hel kenal p?eval, L, 2006-04-15 kell 17:59, kirjutas Sven Suursoho:
> > 1) named parameters additionally to args[]
> > 2) return composite-types from plpython as dictionary
> > 3) return result-set from plpython as list, iterator or generator
> >
> > Test script attached (patch-test.sql) but not integrated to plpython
> > test-suite.
>
> If you wonder why you can't apply the patch, it is against postgres
> 8.0.7.

Can I apply it by merging it in manually, or are you working on a
version against CVS HEAD?

--
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: plpython improvements

From
"Sven Suursoho"
Date:
Hi,


Mon, 17 Apr 2006 19:20:38 +0300, Bruce Momjian <pgman@candle.pha.pa.us>:

>> > Hannu Krosing wrote:
>> >
>> >> > 1) named parameters additionally to args[]
>> >> > 2) return composite-types from plpython as dictionary
>> >> > 3) return result-set from plpython as list, iterator or generator
>> >> >
>> >> > Test script attached (patch-test.sql) but not integrated to
>> >> > plpython test-suite.
>> >>
>> >> If you wonder why you can't apply the patch, it is against postgres
>> >> 8.0.7.
>> >
>> > Can I apply it by merging it in manually, or are you working on a
>> > version against CVS HEAD?

Attached patch for CVS HEAD.
Testscripts remained same (obviously :) but still not integrated.


--
Sven Suursoho

Attachment

Re: plpython improvements

From
Bruce Momjian
Date:
Patch applied.  Thanks.

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


Sven Suursoho wrote:
> Hi,
>
>
> Mon, 17 Apr 2006 19:20:38 +0300, Bruce Momjian <pgman@candle.pha.pa.us>:
>
> >> > Hannu Krosing wrote:
> >> >
> >> >> > 1) named parameters additionally to args[]
> >> >> > 2) return composite-types from plpython as dictionary
> >> >> > 3) return result-set from plpython as list, iterator or generator
> >> >> >
> >> >> > Test script attached (patch-test.sql) but not integrated to
> >> >> > plpython test-suite.
> >> >>
> >> >> If you wonder why you can't apply the patch, it is against postgres
> >> >> 8.0.7.
> >> >
> >> > Can I apply it by merging it in manually, or are you working on a
> >> > version against CVS HEAD?
>
> Attached patch for CVS HEAD.
> Testscripts remained same (obviously :) but still not integrated.
>
>
> --
> Sven Suursoho

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Have you searched our list archives?
>
>                http://archives.postgresql.org

--
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: plpython improvements

From
Bruce Momjian
Date:
Sorry, I have to revert this patch because it is causing crashes in the
plpython regression tests.  Would you please run those tests, fix the
bug, and resubmit.  Thanks.

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

pgman wrote:
>
> Patch applied.  Thanks.
>
> ---------------------------------------------------------------------------
>
>
> Sven Suursoho wrote:
> > Hi,
> >
> >
> > Mon, 17 Apr 2006 19:20:38 +0300, Bruce Momjian <pgman@candle.pha.pa.us>:
> >
> > >> > Hannu Krosing wrote:
> > >> >
> > >> >> > 1) named parameters additionally to args[]
> > >> >> > 2) return composite-types from plpython as dictionary
> > >> >> > 3) return result-set from plpython as list, iterator or generator
> > >> >> >
> > >> >> > Test script attached (patch-test.sql) but not integrated to
> > >> >> > plpython test-suite.
> > >> >>
> > >> >> If you wonder why you can't apply the patch, it is against postgres
> > >> >> 8.0.7.
> > >> >
> > >> > Can I apply it by merging it in manually, or are you working on a
> > >> > version against CVS HEAD?
> >
> > Attached patch for CVS HEAD.
> > Testscripts remained same (obviously :) but still not integrated.
> >
> >
> > --
> > Sven Suursoho
>
> [ Attachment, skipping... ]
>
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 4: Have you searched our list archives?
> >
> >                http://archives.postgresql.org
>
> --
>   Bruce Momjian   http://candle.pha.pa.us
>   EnterpriseDB    http://www.enterprisedb.com
>
>   + If your life is a hard drive, Christ can be your backup. +

--
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: plpython.c
===================================================================
RCS file: /cvsroot/pgsql/src/pl/plpython/plpython.c,v
retrieving revision 1.77
retrieving revision 1.78
diff -c -c -r1.77 -r1.78
*** plpython.c    4 Apr 2006 19:35:37 -0000    1.77
--- plpython.c    27 Apr 2006 01:05:05 -0000    1.78
***************
*** 19,24 ****
--- 19,25 ----
  #include "catalog/pg_type.h"
  #include "commands/trigger.h"
  #include "executor/spi.h"
+ #include "funcapi.h"
  #include "fmgr.h"
  #include "nodes/makefuncs.h"
  #include "parser/parse_type.h"
***************
*** 108,113 ****
--- 109,119 ----
      bool        fn_readonly;
      PLyTypeInfo result;            /* also used to store info for trigger tuple
                                   * type */
+     bool        is_setof;        /* true, if procedure returns result set */
+     PyObject    *setof;        /* contents of result set. */
+     int        setof_count;    /* numbef of items to return in result set */
+     int        setof_current;    /* current item in result set */
+     char        **argnames;        /* Argument names */
      PLyTypeInfo args[FUNC_MAX_ARGS];
      int            nargs;
      PyObject   *code;            /* compiled procedure code */
***************
*** 184,189 ****
--- 190,196 ----
  static HeapTuple PLy_trigger_handler(FunctionCallInfo fcinfo, PLyProcedure *);

  static PyObject *PLy_function_build_args(FunctionCallInfo fcinfo, PLyProcedure *);
+ static void PLy_function_delete_args(PLyProcedure *);
  static PyObject *PLy_trigger_build_args(FunctionCallInfo fcinfo, PLyProcedure *,
                         HeapTuple *);
  static HeapTuple PLy_modify_tuple(PLyProcedure *, PyObject *,
***************
*** 218,223 ****
--- 225,231 ----
  static PyObject *PLyInt_FromString(const char *);
  static PyObject *PLyLong_FromString(const char *);
  static PyObject *PLyString_FromString(const char *);
+ static HeapTuple PLyDict_ToTuple(PLyTypeInfo *, PyObject *);


  /* global data */
***************
*** 726,736 ****

      PG_TRY();
      {
!         plargs = PLy_function_build_args(fcinfo, proc);
!         plrv = PLy_procedure_call(proc, "args", plargs);
!
!         Assert(plrv != NULL);
!         Assert(!PLy_error_in_progress);

          /*
           * Disconnect from SPI manager and then create the return values datum
--- 734,750 ----

      PG_TRY();
      {
!         if (!proc->is_setof || proc->setof_count == -1)
!         {
!             /* python function not called yet, do it */
!             plargs = PLy_function_build_args(fcinfo, proc);
!             plrv = PLy_procedure_call(proc, "args", plargs);
!             if (!proc->is_setof)
!                 /* SETOF function parameters are deleted when called last row is returned */
!                 PLy_function_delete_args(proc);
!             Assert(plrv != NULL);
!             Assert(!PLy_error_in_progress);
!         }

          /*
           * Disconnect from SPI manager and then create the return values datum
***************
*** 741,746 ****
--- 755,830 ----
          if (SPI_finish() != SPI_OK_FINISH)
              elog(ERROR, "SPI_finish failed");

+         if (proc->is_setof)
+         {
+             bool is_done = false;
+             ReturnSetInfo *rsi = (ReturnSetInfo *)fcinfo->resultinfo;
+
+             if (proc->setof_current == -1)
+             {
+                 /* first time -- do checks and setup */
+                 if (!rsi || !IsA(rsi, ReturnSetInfo) ||
+                         (rsi->allowedModes & SFRM_ValuePerCall) == 0)
+                 {
+                     ereport(ERROR,
+                             (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                              errmsg("only value per call is allowed")));
+                 }
+                 rsi->returnMode = SFRM_ValuePerCall;
+
+                 /* fetch information about returned object */
+                 proc->setof = plrv;
+                 plrv = NULL;
+                 if (PyList_Check(proc->setof))
+                     /* SETOF as list */
+                     proc->setof_count = PyList_GET_SIZE(proc->setof);
+                 else if (PyIter_Check(proc->setof))
+                     /* SETOF as iterator, unknown number of items */
+                     proc->setof_current = proc->setof_count = 0;
+                 else
+                 {
+                     ereport(ERROR,
+                             (errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
+                              errmsg("SETOF must be returned as list or iterator")));
+                 }
+             }
+
+             Assert(proc->setof != NULL);
+
+             /* Fetch next of SETOF */
+             if (PyList_Check(proc->setof))
+             {
+                 is_done = ++proc->setof_current == proc->setof_count;
+                 if (!is_done)
+                     plrv = PyList_GET_ITEM(proc->setof, proc->setof_current);
+             }
+             else if (PyIter_Check(proc->setof))
+             {
+                 plrv = PyIter_Next(proc->setof);
+                 is_done = plrv == NULL;
+             }
+
+             if (!is_done)
+             {
+                 rsi->isDone = ExprMultipleResult;
+             }
+             else
+             {
+                 rsi->isDone = ExprEndResult;
+                 proc->setof_count = proc->setof_current = -1;
+                 Py_DECREF(proc->setof);
+                 proc->setof = NULL;
+
+                 Py_XDECREF(plargs);
+                 Py_XDECREF(plrv);
+                 Py_XDECREF(plrv_so);
+
+                 PLy_function_delete_args(proc);
+                 fcinfo->isnull = true;
+                 return (Datum)NULL;
+             }
+         }
+
          /*
           * If the function is declared to return void, the Python
           * return value must be None. For void-returning functions, we
***************
*** 767,772 ****
--- 851,876 ----
                                     proc->result.out.d.typioparam,
                                     -1);
          }
+         else if (proc->result.is_rowtype >= 1)
+         {
+             HeapTuple   tuple;
+
+             /* returning composite type */
+             if (!PyDict_Check(plrv))
+                 elog(ERROR, "tuple must be returned as dictionary");
+
+             tuple = PLyDict_ToTuple(&proc->result, plrv);
+             if (tuple != NULL)
+             {
+                 fcinfo->isnull = false;
+                 rv = HeapTupleGetDatum(tuple);
+             }
+             else
+             {
+                 fcinfo->isnull = true;
+                 rv = (Datum) NULL;
+             }
+         }
          else
          {
              fcinfo->isnull = false;
***************
*** 893,898 ****
--- 997,1003 ----
               * FIXME -- error check this
               */
              PyList_SetItem(args, i, arg);
+             PyDict_SetItemString(proc->globals, proc->argnames[i], arg);
              arg = NULL;
          }
      }
***************
*** 909,914 ****
--- 1014,1029 ----
  }


+ static void
+ PLy_function_delete_args(PLyProcedure *proc)
+ {
+     int    i;
+
+     for (i = 0; i < proc->nargs; i++)
+         PyDict_DelItemString(proc->globals, proc->argnames[i]);
+ }
+
+
  /*
   * PLyProcedure functions
   */
***************
*** 979,984 ****
--- 1094,1102 ----
      bool        isnull;
      int            i,
                  rv;
+     Datum        argnames;
+     Datum            *elems;
+     int        nelems;

      procStruct = (Form_pg_proc) GETSTRUCT(procTup);

***************
*** 1010,1015 ****
--- 1128,1137 ----
      proc->nargs = 0;
      proc->code = proc->statics = NULL;
      proc->globals = proc->me = NULL;
+     proc->is_setof = procStruct->proretset;
+     proc->setof = NULL;
+     proc->setof_count = proc->setof_current = -1;
+     proc->argnames = NULL;

      PG_TRY();
      {
***************
*** 1046,1054 ****
              }

              if (rvTypeStruct->typtype == 'c')
!                 ereport(ERROR,
!                         (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
!                      errmsg("plpython functions cannot return tuples yet")));
              else
                  PLy_output_datum_func(&proc->result, rvTypeTup);

--- 1168,1178 ----
              }

              if (rvTypeStruct->typtype == 'c')
!             {
!                 /* Tuple: set up later, during first call to PLy_function_handler */
!                 proc->result.out.d.typoid = procStruct->prorettype;
!                 proc->result.is_rowtype = 2;
!             }
              else
                  PLy_output_datum_func(&proc->result, rvTypeTup);

***************
*** 1071,1076 ****
--- 1195,1215 ----
           * arguments.
           */
          proc->nargs = fcinfo->nargs;
+         proc->argnames = NULL;
+         if (proc->nargs)
+         {
+             argnames = SysCacheGetAttr(PROCOID, procTup, Anum_pg_proc_proargnames, &isnull);
+             if (!isnull)
+             {
+                 deconstruct_array(DatumGetArrayTypeP(argnames), TEXTOID, -1, false, 'i',
+                         &elems, NULL, &nelems);
+                 if (nelems != proc->nargs)
+                     elog(ERROR,
+                             "proargnames must have the same number of elements "
+                             "as the function has arguments");
+                 proc->argnames = (char **) PLy_malloc(sizeof(char *)*proc->nargs);
+             }
+         }
          for (i = 0; i < fcinfo->nargs; i++)
          {
              HeapTuple    argTypeTup;
***************
*** 1099,1106 ****
                  proc->args[i].is_rowtype = 2;    /* still need to set I/O funcs */

              ReleaseSysCache(argTypeTup);
-         }


          /*
           * get the text of the function.
--- 1238,1248 ----
                  proc->args[i].is_rowtype = 2;    /* still need to set I/O funcs */

              ReleaseSysCache(argTypeTup);

+             /* Fetch argument name */
+             if (proc->argnames)
+                 proc->argnames[i] = PLy_strdup(DatumGetCString(DirectFunctionCall1(textout, elems[i])));
+         }

          /*
           * get the text of the function.
***************
*** 1236,1241 ****
--- 1378,1384 ----
      if (proc->pyname)
          PLy_free(proc->pyname);
      for (i = 0; i < proc->nargs; i++)
+     {
          if (proc->args[i].is_rowtype == 1)
          {
              if (proc->args[i].in.r.atts)
***************
*** 1243,1248 ****
--- 1386,1396 ----
              if (proc->args[i].out.r.atts)
                  PLy_free(proc->args[i].out.r.atts);
          }
+         if (proc->argnames && proc->argnames[i])
+             PLy_free(proc->argnames[i]);
+     }
+     if (proc->argnames)
+         PLy_free(proc->argnames);
  }

  /* conversion functions.  remember output from python is
***************
*** 1501,1506 ****
--- 1649,1726 ----
      return dict;
  }

+
+ static HeapTuple
+ PLyDict_ToTuple(PLyTypeInfo *info, PyObject *dict)
+ {
+     TupleDesc    desc;
+     HeapTuple    tuple;
+     Datum        *values;
+     char        *nulls;
+     int        i;
+
+     desc = CreateTupleDescCopy(lookup_rowtype_tupdesc(info->out.d.typoid, -1));
+
+     /* Set up tuple type, if neccessary */
+     if (info->is_rowtype == 2)
+     {
+         PLy_output_tuple_funcs(info, desc);
+         info->is_rowtype = 1;
+     }
+     Assert(info->is_rowtype == 1);
+
+     /* Build tuple */
+     values = palloc(sizeof(Datum)*desc->natts);
+     nulls = palloc(sizeof(char)*desc->natts);
+     for (i = 0;  i < desc->natts;  ++i)
+     {
+         char        *key;
+         PyObject    *value,
+                 *so;
+
+         key = NameStr(desc->attrs[i]->attname);
+         value = so = NULL;
+         PG_TRY();
+         {
+             value = PyDict_GetItemString(dict, key);
+             if (value != Py_None && value != NULL)
+             {
+                 char *valuestr;
+
+                 so = PyObject_Str(value);
+                 valuestr = PyString_AsString(so);
+                 values[i] = InputFunctionCall(&info->out.r.atts[i].typfunc
+                         , valuestr
+                         , info->out.r.atts[i].typioparam
+                         , -1);
+                 Py_DECREF(so);
+                 value = so = NULL;
+                 nulls[i] = ' ';
+             }
+             else
+             {
+                 value = NULL;
+                 values[i] = (Datum) NULL;
+                 nulls[i] = 'n';
+             }
+         }
+         PG_CATCH();
+         {
+             Py_XDECREF(value);
+             Py_XDECREF(so);
+             PG_RE_THROW();
+         }
+         PG_END_TRY();
+     }
+
+     tuple = heap_formtuple(desc, values, nulls);
+     FreeTupleDesc(desc);
+     pfree(values);
+     pfree(nulls);
+
+     return tuple;
+ }
+
  /* initialization, some python variables function declared here */

  /* interface to postgresql elog */

Re: plpython improvements

From
Hannu Krosing
Date:
Ühel kenal päeval, N, 2006-04-27 kell 10:17, kirjutas Bruce Momjian:
> Sorry, I have to revert this patch because it is causing crashes in the
> plpython regression tests.  Would you please run those tests, fix the
> bug, and resubmit.  Thanks.

Where exactly does it crash ?

Please tell us the version (and buildflags) of python you are using.

There is a superfluous assert in some versions of python that has
troubled us as well.

---------------
Hannu


Re: plpython improvements

From
Bruce Momjian
Date:
Hannu Krosing wrote:
> ?hel kenal p?eval, N, 2006-04-27 kell 10:17, kirjutas Bruce Momjian:
> > Sorry, I have to revert this patch because it is causing crashes in the
> > plpython regression tests.  Would you please run those tests, fix the
> > bug, and resubmit.  Thanks.
>
> Where exactly does it crash ?
>
> Please tell us the version (and buildflags) of python you are using.
>
> There is a superfluous assert in some versions of python that has
> troubled us as well.

Uh, I can't test plpython here because my libraries do not support it,
but I see this in the build farm:

  http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=rook&dt=2006-04-27%2010:39:02

--
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: plpython improvements

From
"Sven Suursoho"
Date:
Hi,


Thu, 27 Apr 2006 17:17:36 +0300, Bruce Momjian <pgman@candle.pha.pa.us>:

> Sorry, I have to revert this patch because it is causing crashes in the
> plpython regression tests.  Would you please run those tests, fix the
> bug, and resubmit.  Thanks.

Found and fixed two problems:
1) named parameters handling if there were no parameters at all
2) didn't increment counter for borrowed reference fetched from list

Also integrated tests to plpython test suite and updated existing tests to
use named parameters.

Unfortunately, there is still one problem when using unpatched python,
caused by too aggressive assert.
See
http://mail.python.org/pipermail/python-checkins/2005-August/046571.html.
I guess there should be warning somewhere as Hannu said but didn't know
where to put it.


--
Sven Suursoho

Attachment

Re: plpython improvements

From
Tom Lane
Date:
"Sven Suursoho" <sven@spam.pri.ee> writes:
> Unfortunately, there is still one problem when using unpatched python,
> caused by too aggressive assert.
> See
> http://mail.python.org/pipermail/python-checkins/2005-August/046571.html.
> I guess there should be warning somewhere as Hannu said but didn't know
> where to put it.

I don't think we are going to be able to accept a patch that causes the
server to crash when using any but a bleeding-edge copy of Python.

            regards, tom lane

Re: plpython improvements

From
"Sven Suursoho"
Date:
Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane <tgl@sss.pgh.pa.us>:

> "Sven Suursoho" <sven@spam.pri.ee> writes:
>> Unfortunately, there is still one problem when using unpatched python,
>> caused by too aggressive assert.
>> See
>> http://mail.python.org/pipermail/python-checkins/2005-August/046571.html.
>> I guess there should be warning somewhere as Hannu said but didn't know
>> where to put it.
>
> I don't think we are going to be able to accept a patch that causes the
> server to crash when using any but a bleeding-edge copy of Python.

Actually normal python installations do not cause problem, only debugging
versions do.

Anyway, if you think that this doesn't count as an argument, there is
nothing that we can do from PG-side except drop returning SETOF as
iterator/generator and only allow return SETOF as list.


--
Sven Suursoho

Re: plpython improvements

From
Bruce Momjian
Date:
Sven Suursoho wrote:
> Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane <tgl@sss.pgh.pa.us>:
>
> > "Sven Suursoho" <sven@spam.pri.ee> writes:
> >> Unfortunately, there is still one problem when using unpatched python,
> >> caused by too aggressive assert.
> >> See
> >> http://mail.python.org/pipermail/python-checkins/2005-August/046571.html.
> >> I guess there should be warning somewhere as Hannu said but didn't know
> >> where to put it.
> >
> > I don't think we are going to be able to accept a patch that causes the
> > server to crash when using any but a bleeding-edge copy of Python.
>
> Actually normal python installations do not cause problem, only debugging
> versions do.
>
> Anyway, if you think that this doesn't count as an argument, there is
> nothing that we can do from PG-side except drop returning SETOF as
> iterator/generator and only allow return SETOF as list.

Can't we detect a debug build and disable the feature?

--
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: plpython improvements

From
"Sven Suursoho"
Date:
Sun, 30 Apr 2006 20:48:48 +0300, Bruce Momjian <pgman@candle.pha.pa.us>:

>> Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane <tgl@sss.pgh.pa.us>:
>>
>> > "Sven Suursoho" <sven@spam.pri.ee> writes:
>> >> Unfortunately, there is still one problem when using unpatched
>> python,
>> >> caused by too aggressive assert.
>> >>
>> http://mail.python.org/pipermail/python-checkins/2005-August/046571.html.
>> >> I guess there should be warning somewhere as Hannu said but didn't
>> know
>> >> where to put it.
>> >
>> > I don't think we are going to be able to accept a patch that causes
>> the
>> > server to crash when using any but a bleeding-edge copy of Python.
>>
>> Actually normal python installations do not cause problem, only
>> debugging versions do.
>>
>> Anyway, if you think that this doesn't count as an argument, there is
>> nothing that we can do from PG-side except drop returning SETOF as
>> iterator/generator and only allow return SETOF as list.
>
> Can't we detect a debug build and disable the feature?

Yes, we can, but newer python versions are already fixed.

So, what about this in configure:
if --with-python && test_iterator_app_crashes
   # errcode(FEATURE_NOT_SUPPORTED), errmsg(patch your python)
   disable_iterator_feature
fi

In this way we disable feature only if it is absolutely neccessary and
will give developer enough information how to fix it.


--
Sven Suursoho

Re: plpython improvements

From
Tom Lane
Date:
"Sven Suursoho" <sven@spam.pri.ee> writes:
> So, what about this in configure:
> if --with-python && test_iterator_app_crashes
>    # errcode(FEATURE_NOT_SUPPORTED), errmsg(patch your python)
>    disable_iterator_feature
> fi

Testing it in configure is wrong, because there's no guarantee the same
python library will be used at runtime.

            regards, tom lane

Re: plpython improvements

From
"Sven Suursoho"
Date:
Sun, 30 Apr 2006 21:43:03 +0300, Tom Lane <tgl@sss.pgh.pa.us>:

> "Sven Suursoho" <sven@spam.pri.ee> writes:
>> So, what about this in configure:
>> if --with-python && test_iterator_app_crashes
>>    # errcode(FEATURE_NOT_SUPPORTED), errmsg(patch your python)
>>    disable_iterator_feature
>> fi
>
> Testing it in configure is wrong, because there's no guarantee the same
> python library will be used at runtime.

Yes, that's right.

Ok, will do Py_DEBUG && Py_GetVersion() check at runtime.


--
Sven Suursoho

Re: plpython improvements

From
Hannu Krosing
Date:
Ühel kenal päeval, P, 2006-04-30 kell 14:43, kirjutas Tom Lane:
> "Sven Suursoho" <sven@spam.pri.ee> writes:
> > So, what about this in configure:
> > if --with-python && test_iterator_app_crashes
> >    # errcode(FEATURE_NOT_SUPPORTED), errmsg(patch your python)
> >    disable_iterator_feature
> > fi
>
> Testing it in configure is wrong, because there's no guarantee the same
> python library will be used at runtime.

As it is a crash bug, I can see two other ways to test:

1) do the test in startup script (maybe have pg_ctl run something)

2) test it in postmaster by running an external testprogram and see if
it crashes.


How do we handle other buggy library routines, like if some system
library crashes on divide-by-zero or similar ?

----------------
Hannu


Re: plpython improvements

From
"Sven Suursoho"
Date:
Hi,


Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane <tgl@sss.pgh.pa.us>:

> "Sven Suursoho" <sven@spam.pri.ee> writes:
>> Unfortunately, there is still one problem when using unpatched python,
>> caused by too aggressive assert.
>> http://mail.python.org/pipermail/python-checkins/2005-August/046571.html.
>
> I don't think we are going to be able to accept a patch that causes the
> server to crash when using any but a bleeding-edge copy of Python.

Did complete rewrite for SETOF functions: now accepts any python object
for which iter(object) returns iterable object. In this way we don't have
to deal with specific containers but can use unified python iterator API.
It means that plpython is future-proof -- whenever python introduces new
container, stored procedures already can use those without recompiling
language handler.

Also integrated with regression tests and updated existing tests to use
named parameters.

When using python interpreter with asserts enabled, generators still
crash. But I don't think that we should drop this feature because of that.
Reasons:
1) this is someone else's bug, we are using documented API correctly
2) it doesn't concern majority of users because probably there is no
asserts in production packages (tested with gentoo, ubuntu, suse). This is
true even for older python versions that are not patched.

And after all, we can document using sets, lists, tuples, iterators etc
and explicitly state that returning generator is undefined.


--
Sven Suursoho

Attachment

Re: plpython improvements

From
Hannu Krosing
Date:
Ühel kenal päeval, N, 2006-05-04 kell 18:21, kirjutas Sven Suursoho:
> Hi,
>
>
> Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane <tgl@sss.pgh.pa.us>:
>
> > "Sven Suursoho" <sven@spam.pri.ee> writes:
> >> Unfortunately, there is still one problem when using unpatched python,
> >> caused by too aggressive assert.
> >> http://mail.python.org/pipermail/python-checkins/2005-August/046571.html.
> >
> > I don't think we are going to be able to accept a patch that causes the
> > server to crash when using any but a bleeding-edge copy of Python.

Actually not bleeding-edge, but just version 2.4.x as distributed in
Fedora Core (and possibly in RHAS), which have assert() enabled in
python.so.

The assert there is buggy (bug
http://sourceforge.net/tracker/index.php?func=detail&aid=1257960&group_id=5470&atid=105470)

> Did complete rewrite for SETOF functions: now accepts any python object
> for which iter(object) returns iterable object. In this way we don't have
> to deal with specific containers but can use unified python iterator API.
> It means that plpython is future-proof -- whenever python introduces new
> container, stored procedures already can use those without recompiling
> language handler.
>
> Also integrated with regression tests and updated existing tests to use
> named parameters.
>
> When using python interpreter with asserts enabled, generators still
> crash. But I don't think that we should drop this feature because of that.
> Reasons:
> 1) this is someone else's bug, we are using documented API correctly
> 2) it doesn't concern majority of users because probably there is no
> asserts in production packages (tested with gentoo, ubuntu, suse). This is
> true even for older python versions that are not patched.

From reading the bug, it seems that older versions of python also don't
have this bug, only 2.4.

> And after all, we can document using sets, lists, tuples, iterators etc
> and explicitly state that returning generator is undefined.

I think that a less confusing way of saying it would be :

 "Generators crash if python version used is 2.4.x and it is compiled
 with asserts.

 Currently only known linux distributions to distibute such python.so
 files are Fedora and possibly other RedHat distributions, while
 Gentoo, Ubuntu and Suse are OK.

 If you need to use generators on such a platform, compile your own
 python from source and make sure that configure uses your version."


I think the patch should be commited so we can collect data about where
else the buggy version of python exists.

And if some buildfarm machines start crashing, python should be fixed
there.

----------------
Hannu




Re: plpython improvements

From
"Joshua D. Drake"
Date:
>
> I think that a less confusing way of saying it would be :
>
>  "Generators crash if python version used is 2.4.x and it is compiled
>  with asserts.
>
>  Currently only known linux distributions to distibute such python.so
>  files are Fedora and possibly other RedHat distributions, while
>  Gentoo, Ubuntu and Suse are OK.

Ubuntu ships 2.4 I don't know about SuSE. 2.4 has been out for sometime
and it would be a mistake to assume that we won't run into this.

People who are "developing" in Python are going to run 2.4 because it is
the latest stable release of the language.

Sincerely,

Joshua D. Drake

>  If you need to use generators on such a platform, compile your own
>  python from source and make sure that configure uses your version."
>
>
> I think the patch should be commited so we can collect data about where
> else the buggy version of python exists.
>
> And if some buildfarm machines start crashing, python should be fixed
> there.
>
> ----------------
> Hannu
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings
>


--

            === The PostgreSQL Company: Command Prompt, Inc. ===
      Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240
      Providing the most comprehensive  PostgreSQL solutions since 1997
                     http://www.commandprompt.com/



Re: plpython improvements

From
"Sven Suursoho"
Date:
Fri, 05 May 2006 19:20:55 +0300, Joshua D. Drake <jd@commandprompt.com>:

>>  I think that a less confusing way of saying it would be :
>>   "Generators crash if python version used is 2.4.x and it is compiled
>>  with asserts.   Currently only known linux distributions to distibute
>> such python.so
>>  files are Fedora and possibly other RedHat distributions, while
>>  Gentoo, Ubuntu and Suse are OK.
>
> Ubuntu ships 2.4 I don't know about SuSE. 2.4 has been out for sometime
> and it would be a mistake to assume that we won't run into this.

Sure, but it is known problem and there is patch for this bug. In the
documentation we can clearly state that python2.4 with asserts enabled
causes problem and describe how it can be tested and fixed (regardless of
distribution used).

As an example of absurdity of this problem: let's assume there is known
distribution with buggy gethostbyname(). Fact, that we know about this,
shouldn't stop us developing TCP/IP applications. Especially, if there is
also patch for this bug :)

It would be real shame to prevent using generator for SETOF functions
because it is most natural match for plpgsql's "return next"


--
Sven Suursoho

Re: plpython improvements

From
"Joshua D. Drake"
Date:
>
> As an example of absurdity of this problem: let's assume there is known
> distribution with buggy gethostbyname(). Fact, that we know about this,
> shouldn't stop us developing TCP/IP applications. Especially, if there
> is also patch for this bug :)
>
> It would be real shame to prevent using generator for SETOF functions
> because it is most natural match for plpgsql's "return next"

All I was saying was that we need to account for the use of 2.4 :). So
as long as we document (as you suggest) we should be good. Sorry if I
wasn't clear.

Sincerely,

Joshua D. Drake


>
>
> --Sven Suursoho
>


--

            === The PostgreSQL Company: Command Prompt, Inc. ===
      Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240
      Providing the most comprehensive  PostgreSQL solutions since 1997
                     http://www.commandprompt.com/



Re: plpython improvements

From
Hannu Krosing
Date:
Ühel kenal päeval, R, 2006-05-05 kell 09:20, kirjutas Joshua D. Drake:
> >
> > I think that a less confusing way of saying it would be :
> >
> >  "Generators crash if python version used is 2.4.x and it is compiled
> >  with asserts.
> >
> >  Currently only known linux distributions to distibute such python.so
> >  files are Fedora and possibly other RedHat distributions, while
> >  Gentoo, Ubuntu and Suse are OK.
>
> Ubuntu ships 2.4 I don't know about SuSE. 2.4 has been out for sometime
> and it would be a mistake to assume that we won't run into this.

Marko Kreen has tested the patch on Ubuntu and it is ok there.

The problem is not using 2.4, it is using 2.4 compiled with a specific set of flags.

---------------
Hannu


Re: plpython improvements

From
Bruce Momjian
Date:
Sven Suursoho wrote:
> Fri, 05 May 2006 19:20:55 +0300, Joshua D. Drake <jd@commandprompt.com>:
>
> >>  I think that a less confusing way of saying it would be :
> >>   "Generators crash if python version used is 2.4.x and it is compiled
> >>  with asserts.   Currently only known linux distributions to distibute
> >> such python.so
> >>  files are Fedora and possibly other RedHat distributions, while
> >>  Gentoo, Ubuntu and Suse are OK.
> >
> > Ubuntu ships 2.4 I don't know about SuSE. 2.4 has been out for sometime
> > and it would be a mistake to assume that we won't run into this.
>
> Sure, but it is known problem and there is patch for this bug. In the
> documentation we can clearly state that python2.4 with asserts enabled
> causes problem and describe how it can be tested and fixed (regardless of
> distribution used).
>
> As an example of absurdity of this problem: let's assume there is known
> distribution with buggy gethostbyname(). Fact, that we know about this,
> shouldn't stop us developing TCP/IP applications. Especially, if there is
> also patch for this bug :)
>
> It would be real shame to prevent using generator for SETOF functions
> because it is most natural match for plpgsql's "return next"

For buggy distributions of gethostbyname(), we supply our own version.
We don't just barrel ahead and tell them to fix it, if we can avoid it.
I don't see us bundling a fixed python, however.  I realize it is only
Red Hat, but that is a large userbase.

I still do not know why we can't do some kind of runtime test in python
and disable this feature for 2.4 builds that have debugging enabled.
Can we do a dynamic function load test from plpython?  There must be
some function that is only visible in debug builds.

--
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: plpython improvements

From
"Sven Suursoho"
Date:
Sat, 06 May 2006 20:38:48 +0300, Bruce Momjian <pgman@candle.pha.pa.us>:

> I still do not know why we can't do some kind of runtime test in python
> and disable this feature for 2.4 builds that have debugging enabled.
> Can we do a dynamic function load test from plpython?  There must be
> some function that is only visible in debug builds.

Yes, I already did research last week after discussions about that. In
unmodified Python distribution, in configure:
if --with-pydebug
   define Py_DEBUG
   undef NDEBUG
else
   undef Py_DEBUG
   define NDEBUG
fi

Unfortunately, this is not case for Fedora Core 4, where assertions are
used unconditionally. And to make things worse, there is no runtime symbol
at all to indicate whether Python is compiled with debugging/assertions
enabled (Py_DEBUG & NDEBUG are preprocessor symbols)

I see 3 options (from best to worst, IMHO)
1) document misbehaviour when using Python2.4 with assertions allowed with
reference to bug fix.
2) new configure flag to optionally allow/disallow using generator
3) drop generators

Btw, we developed returning compose types further. Basically did same as
for SETOF functions -- allow to return any Python object that conforms to
mapping protocol. Currently only dict as previously, though. But still,
ready for new fancy unforeseen Python features without recompiling PG.
Additionally, now it is possible to return compose types as Python tuples.
Will create patch whenever final decision is made about generators.


--
Sven Suursoho

Re: plpython improvements

From
"Sven Suursoho"
Date:
Sun, 07 May 2006 12:36:59 +0300, Sven Suursoho <sven@spam.pri.ee>:

> Btw, we developed returning compose types further. Basically did same as
> for SETOF functions -- allow to return any Python object that conforms
> to mapping protocol. Currently only dict as previously, though. But
> still, ready for new fancy unforeseen Python features without
> recompiling PG.
> Additionally, now it is possible to return compose types as Python
> tuples.

Sorry, not just Python tuples but actually any object conforming to
sequence protocol (tuple, list etc)


--
Sven Suursoho

Re: plpython improvements

From
Bruce Momjian
Date:
Sven Suursoho wrote:
> Sat, 06 May 2006 20:38:48 +0300, Bruce Momjian <pgman@candle.pha.pa.us>:
>
> > I still do not know why we can't do some kind of runtime test in python
> > and disable this feature for 2.4 builds that have debugging enabled.
> > Can we do a dynamic function load test from plpython?  There must be
> > some function that is only visible in debug builds.
>
> Yes, I already did research last week after discussions about that. In
> unmodified Python distribution, in configure:
> if --with-pydebug
>    define Py_DEBUG
>    undef NDEBUG
> else
>    undef Py_DEBUG
>    define NDEBUG
> fi
>
> Unfortunately, this is not case for Fedora Core 4, where assertions are
> used unconditionally. And to make things worse, there is no runtime symbol
> at all to indicate whether Python is compiled with debugging/assertions
> enabled (Py_DEBUG & NDEBUG are preprocessor symbols)

Can you test dynamically loading a function that is only visible in the
symbol table of debug builds, and check the return code?

In the Fedora Core 4 case, how did they make assertions always enabled?
I see the fix:

    http://mail.python.org/pipermail/python-checkins/2005-August/046571.html

How did Fedora Core 4 enable asserts?  I see the patch calling libc's
assert() directly, which should be enabled all the time.

--
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: plpython improvements

From
"Sven Suursoho"
Date:
Sun, 07 May 2006 15:58:49 +0300, Bruce Momjian <pgman@candle.pha.pa.us>:

>> Unfortunately, this is not case for Fedora Core 4, where assertions are
>> used unconditionally. And to make things worse, there is no runtime
>> symbol
>> at all to indicate whether Python is compiled with debugging/assertions
>> enabled (Py_DEBUG & NDEBUG are preprocessor symbols)
>
> Can you test dynamically loading a function that is only visible in the
> symbol table of debug builds, and check the return code?

As it came out, that wouldn't help us because FC4's Python has debugging
disabled but assertions are still active. See below.


> In the Fedora Core 4 case, how did they make assertions always enabled?

Building FC4's Python rpm contains bug:
When creating package rpm from spec-file, even though Python is built
without --with-pydebug, rpmbuild loses -DNDEBUG from CFLAGS.
Can't tell how or when it happens, I'm not so good with rpm packaging
system, just looked output of compilation stage.

So, we are back on square one...


--
Sven Suursoho

Re: plpython improvements

From
Bruce Momjian
Date:
Added to TODO:

                o Allow PL/python to composite types and result sets
                  once buggy assert-enabled versions of python can be detected

                 http://archives.postgresql.org/pgsql-patches/2006-04/msg00087$


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

Hannu Krosing wrote:
> ?hel kenal p?eval, N, 2006-05-04 kell 18:21, kirjutas Sven Suursoho:
> > Hi,
> >
> >
> > Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane <tgl@sss.pgh.pa.us>:
> >
> > > "Sven Suursoho" <sven@spam.pri.ee> writes:
> > >> Unfortunately, there is still one problem when using unpatched python,
> > >> caused by too aggressive assert.
> > >> http://mail.python.org/pipermail/python-checkins/2005-August/046571.html.
> > >
> > > I don't think we are going to be able to accept a patch that causes the
> > > server to crash when using any but a bleeding-edge copy of Python.
>
> Actually not bleeding-edge, but just version 2.4.x as distributed in
> Fedora Core (and possibly in RHAS), which have assert() enabled in
> python.so.
>
> The assert there is buggy (bug
> http://sourceforge.net/tracker/index.php?func=detail&aid=1257960&group_id=5470&atid=105470)
>
> > Did complete rewrite for SETOF functions: now accepts any python object
> > for which iter(object) returns iterable object. In this way we don't have
> > to deal with specific containers but can use unified python iterator API.
> > It means that plpython is future-proof -- whenever python introduces new
> > container, stored procedures already can use those without recompiling
> > language handler.
> >
> > Also integrated with regression tests and updated existing tests to use
> > named parameters.
> >
> > When using python interpreter with asserts enabled, generators still
> > crash. But I don't think that we should drop this feature because of that.
> > Reasons:
> > 1) this is someone else's bug, we are using documented API correctly
> > 2) it doesn't concern majority of users because probably there is no
> > asserts in production packages (tested with gentoo, ubuntu, suse). This is
> > true even for older python versions that are not patched.
>
> >From reading the bug, it seems that older versions of python also don't
> have this bug, only 2.4.
>
> > And after all, we can document using sets, lists, tuples, iterators etc
> > and explicitly state that returning generator is undefined.
>
> I think that a less confusing way of saying it would be :
>
>  "Generators crash if python version used is 2.4.x and it is compiled
>  with asserts.
>
>  Currently only known linux distributions to distibute such python.so
>  files are Fedora and possibly other RedHat distributions, while
>  Gentoo, Ubuntu and Suse are OK.
>
>  If you need to use generators on such a platform, compile your own
>  python from source and make sure that configure uses your version."
>
>
> I think the patch should be commited so we can collect data about where
> else the buggy version of python exists.
>
> And if some buildfarm machines start crashing, python should be fixed
> there.
>
> ----------------
> Hannu
>
>
>

--
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: plpython improvements

From
Sven
Date:
Quoting Bruce Momjian <pgman@candle.pha.pa.us>:

>
> Added to TODO:
>
>            o Allow PL/python to composite types and result sets
>              once buggy assert-enabled versions of python can be
detected

I actually tried to work other way around -- get all known buggy systems
fixed:
1) Debugging Python version: bug acknowledged, v2.5 fixed, v2.4 waiting
(http://sourceforge.net/tracker/index.php?func=detail&aid=1483133&group_id=5470&atid=105470)
2) Fedora: >=v5 fixed, will be backported to version 4 also
(https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=192592)

If this approach is not satisfactionary and assertion predetection is
still requested, only solution I see is to catch signal SIGABRT, set
flag, longjmp back and report error.
But I really don't like it.


--
Sven Suursoho

Re: plpython improvements

From
Tom Lane
Date:
Sven <sven@spam.pri.ee> writes:
> If this approach is not satisfactionary and assertion predetection is
> still requested, only solution I see is to catch signal SIGABRT, set
> flag, longjmp back and report error.
> But I really don't like it.

No, ignoring SIGABRT is surely right out.

It suddenly strikes me though that we are being too picky.  The reason
(which I admit having forgotten) is that plpython only comes in an
untrusted variant.  That means that anyone writing plpython functions
is already a database superuser, and has much more direct means of
crashing the backend if he chooses to.  So the idea that this python bug
constitutes a security threat seems overblown.

If there ever is a future Python release with a new sandbox mechanism,
enabling resurrection of the trusted language, it would presumably
contain the bug fix.

Obviously we should document the problem in the plpython documentation
("don't try to use this feature with python versions before XXX"), but
I'm not any longer convinced that we have to reject this patch on
security grounds.

[ Whether the patch is any good is a separate question ;-).  I have
not reviewed it. ]

            regards, tom lane

Re: plpython improvements

From
"Sven Suursoho"
Date:
On Sat, 17 Jun 2006 18:47:46 +0300, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Obviously we should document the problem in the plpython documentation
> ("don't try to use this feature with python versions before XXX"), but
> I'm not any longer convinced that we have to reject this patch on
> security grounds.

Here it is, updated against CVS HEAD.
Will create documentation after acceptance of patch.


--
Sven Suursoho

Attachment

Re: plpython improvements

From
Sven Suursoho
Date:
Quoting Sven Suursoho <public@spam.pri.ee>:


> On Sat, 17 Jun 2006 18:47:46 +0300, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> > Obviously we should document the problem in the plpython documentation
> > ("don't try to use this feature with python versions before XXX"), but
> > I'm not any longer convinced that we have to reject this patch on
> > security grounds.
>
> Here it is, updated against CVS HEAD.
> Will create documentation after acceptance of patch.

And here it comes again :)
Fedora has fixed bug that caused initial rejection
(https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=192592).


--
Sven Suursoho

Attachment

Re: plpython improvements

From
Bruce Momjian
Date:
Sven Suursoho wrote:
> Quoting Sven Suursoho <public@spam.pri.ee>:
>
>
> > On Sat, 17 Jun 2006 18:47:46 +0300, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> > > Obviously we should document the problem in the plpython documentation
> > > ("don't try to use this feature with python versions before XXX"), but
> > > I'm not any longer convinced that we have to reject this patch on
> > > security grounds.
> >
> > Here it is, updated against CVS HEAD.
> > Will create documentation after acceptance of patch.
>
> And here it comes again :)
> Fedora has fixed bug that caused initial rejection
> (https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=192592).

Great.  Please supply documentation and it will be applied.  Thanks.

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: plpython improvements

From
Sven Suursoho
Date:
Hi,


Quoting Bruce Momjian <bruce@momjian.us>:

> Great.  Please supply documentation and it will be applied.  Thanks.

Here it comes, including src+doc patches.
Updated sources according to Michael Fuhr's comments and fixed one FIXME.

Please check documentation patch thoroughly as I'm not native English
speaker nor didn't manage to generate documentation from SGML sources (
openjade:postgres.sgml:3:55:W: cannot generate system identifier for public
text "-//OASIS//DTD DocBook V4.2//EN")


--
Sven Suursoho

Attachment

Re: plpython improvements

From
Bruce Momjian
Date:
Sven Suursoho wrote:
> Hi,
>
>
> Quoting Bruce Momjian <bruce@momjian.us>:
>
> > Great.  Please supply documentation and it will be applied.  Thanks.
>
> Here it comes, including src+doc patches.
> Updated sources according to Michael Fuhr's comments and fixed one FIXME.
>
> Please check documentation patch thoroughly as I'm not native English
> speaker nor didn't manage to generate documentation from SGML sources (
> openjade:postgres.sgml:3:55:W: cannot generate system identifier for public
> text "-//OASIS//DTD DocBook V4.2//EN")

Patch applied.  Thanks.  I improved your documentation wording, updated
version attached.

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/plpython.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/plpython.sgml,v
retrieving revision 1.30
diff -c -c -r1.30 plpython.sgml
*** doc/src/sgml/plpython.sgml    26 May 2006 19:23:09 -0000    1.30
--- doc/src/sgml/plpython.sgml    2 Sep 2006 12:12:40 -0000
***************
*** 46,67 ****
    <title>PL/Python Functions</title>

    <para>
!    Functions in PL/Python are declared via the usual <xref
     linkend="sql-createfunction" endterm="sql-createfunction-title">
!    syntax. For example:
  <programlisting>
! CREATE FUNCTION myfunc(text) RETURNS text
!     AS 'return args[0]'
!     LANGUAGE plpythonu;
  </programlisting>

     The Python code that is given as the body of the function definition
!    gets transformed into a Python function.
!    For example, the above results in

  <programlisting>
! def __plpython_procedure_myfunc_23456():
!         return args[0]
  </programlisting>

     assuming that 23456 is the OID assigned to the function by
--- 46,95 ----
    <title>PL/Python Functions</title>

    <para>
!    Functions in PL/Python are declared via the standard <xref
     linkend="sql-createfunction" endterm="sql-createfunction-title">
!    syntax:
!
! <programlisting>
! CREATE FUNCTION <replaceable>funcname</replaceable> (<replaceable>argument-list</replaceable>)
!   RETURNS <replaceable>return-type</replaceable>
! AS $$
!   # PL/Python function body
! $$ LANGUAGE plpythonu;
! </programlisting>
!   </para>
!
!   <para>
!    The body of a function is simply a Python script. When the function
!    is called, all unnamed arguments are passed as elements to the array
!    <varname>args[]</varname> and named arguments as ordinary variables to the
!    Python script. The result is returned from the Python code in the usual way,
!    with <literal>return</literal> or <literal>yield</literal> (in case of
!    a resultset statement).
!   </para>
!
!   <para>
!    For example, a function to return the greater of two integers can be
!    defined as:
!
  <programlisting>
! CREATE FUNCTION pymax (a integer, b integer)
!   RETURNS integer
! AS $$
!   if a > b:
!     return a
!   return b
! $$ LANGUAGE plpythonu;
  </programlisting>

     The Python code that is given as the body of the function definition
!    is transformed into a Python function. For example, the above results in

  <programlisting>
! def __plpython_procedure_pymax_23456():
!   if a > b:
!     return a
!   return b
  </programlisting>

     assuming that 23456 is the OID assigned to the function by
***************
*** 69,74 ****
--- 97,257 ----
    </para>

    <para>
+    The <productname>PostgreSQL</> function parameters are available in
+    the global <varname>args</varname> list.  In the
+    <function>pymax</function> example, <varname>args[0]</varname> contains
+    whatever was passed in as the first argument and
+    <varname>args[1]</varname> contains the second argument's value. Alternatively,
+    one can use named parameters as shown in the example above. This greatly simplifies
+    the reading and writing of <application>PL/Python</application> code.
+   </para>
+
+   <para>
+    If an SQL null value<indexterm><primary>null value</primary><secondary
+    sortas="PL/Python">PL/Python</secondary></indexterm> is passed to a
+    function, the argument value will appear as <symbol>None</symbol> in
+    Python. The above function definition will return the wrong answer for null
+    inputs. We could add <literal>STRICT</literal> to the function definition
+    to make <productname>PostgreSQL</productname> do something more reasonable:
+    if a null value is passed, the function will not be called at all,
+    but will just return a null result automatically. Alternatively,
+    we could check for null inputs in the function body:
+
+ <programlisting>
+ CREATE FUNCTION pymax (a integer, b integer)
+   RETURNS integer
+ AS $$
+   if (a is None) or (b is None):
+     return None
+   if a > b:
+     return a
+   return b
+ $$ LANGUAGE plpythonu;
+ </programlisting>
+
+    As shown above, to return an SQL null value from a PL/Python
+    function, return the value <symbol>None</symbol>. This can be done whether the
+    function is strict or not.
+   </para>
+
+   <para>
+    Composite-type arguments are passed to the function as Python mappings. The
+    element names of the mapping are the attribute names of the composite type.
+    If an attribute in the passed row has the null value, it has the value
+    <symbol>None</symbol> in the mapping. Here is an example:
+
+ <programlisting>
+ CREATE TABLE employee (
+   name text,
+   salary integer,
+   age integer
+ );
+
+ CREATE FUNCTION overpaid (e employee)
+   RETURNS boolean
+ AS $$
+   if e["salary"] > 200000:
+     return True
+   if (e["age"] < 30) and (e["salary"] > 100000):
+     return True
+   return False
+ $$ LANGUAGE plpythonu;
+ </programlisting>
+   </para>
+
+   <para>
+    There are multiple ways to return row or composite types from a Python
+    scripts. In following examples we assume to have:
+
+ <programlisting>
+ CREATE TABLE named_value (
+   name   text,
+   value  integer
+ );
+ </programlisting>
+    or
+ <programlisting>
+ CREATE TYPE named_value AS (
+   name   text,
+   value  integer
+ );
+ </programlisting>
+
+    <variablelist>
+     <varlistentry>
+      <term>Sequence types (tuple or list), but not <literal>set</literal> (because
+      it is not indexable)</term>
+      <listitem>
+       <para>
+        Returned sequence objects must have the same number of items as
+        composite types have fields. Item with index 0 is assigned to the first field
+        of the composite type, 1 to second and so on. For example:
+
+ <programlisting>
+ CREATE FUNCTION make_pair (name text, value integer)
+   RETURNS named_value
+ AS $$
+   return [ name, value ]
+   # or alternatively, as tuple: return ( name, value )
+ $$ LANGUAGE plpythonu;
+ </programlisting>
+
+        To return SQL null in any column, insert <symbol>None</symbol> at
+        the corresponding position.
+       </para>
+      </listitem>
+
+     <varlistentry>
+      <term>Mapping (dictionary)</term>
+      <listitem>
+       <para>
+        Value for a composite type's column is retrieved from the mapping with
+        the column name as key. Example:
+
+ <programlisting>
+ CREATE FUNCTION make_pair (name text, value integer)
+   RETURNS named_value
+ AS $$
+   return { "name": name, "value": value }
+ $$ LANGUAGE plpythonu;
+ </programlisting>
+
+        Additional dictionary key/value pairs are ignored. Missing keys are
+        treated as errors, i.e. to return an SQL null value for any column, insert
+        <symbol>None</symbol> with the corresponding column name as the key.
+       </para>
+      </listitem>
+
+     <varlistentry>
+      <term>Object (any object providing method <literal>__getattr__</literal>)</term>
+      <listitem>
+       <para>
+        Example:
+
+ <programlisting>
+ CREATE FUNCTION make_pair (name text, value integer)
+   RETURNS named_value
+ AS $$
+   class named_value:
+     def __init__ (self, n, v):
+       self.name = n
+       self.value = v
+   return named_value(name, value)
+
+   # or simply
+   class nv: pass
+   nv.name = name
+   nv.value = value
+   return nv
+ $$ LANGUAGE plpythonu;
+ </programlisting>
+       </para>
+      </listitem>
+     </varlistentry>
+    </variablelist>
+   </para>
+
+   <para>
     If you do not provide a return value, Python returns the default
     <symbol>None</symbol>. <application>PL/Python</application> translates
     Python's <symbol>None</symbol> into the SQL null
***************
*** 77,89 ****
    </para>

    <para>
!    The <productname>PostgreSQL</> function parameters are available in
!    the global <varname>args</varname> list.  In the
!    <function>myfunc</function> example, <varname>args[0]</> contains
!    whatever was passed in as the text argument.  For
!    <literal>myfunc2(text, integer)</literal>, <varname>args[0]</>
!    would contain the <type>text</type> argument and
!    <varname>args[1]</varname> the <type>integer</type> argument.
    </para>

    <para>
--- 260,359 ----
    </para>

    <para>
!    A <application>PL/Python</application> function can also return sets of
!    scalar or composite types. There are serveral ways to achieve this because
!    the returned object is internally turned into an iterator. For following
!    examples, let's assume to have composite type:
!
! <programlisting>
! CREATE TYPE greeting AS (
!   how text,
!   who text
! );
! </programlisting>
!
!    Currently known iterable types are:
!    <variablelist>
!     <varlistentry>
!      <term>Sequence types (tuple, list, set)</term>
!      <listitem>
!       <para>
! <programlisting>
! CREATE FUNCTION greet (how text)
!   RETURNS SETOF greeting
! AS $$
!   # return tuple containing lists as composite types
!   # all other combinations work also
!   return ( [ how, "World" ], [ how, "PostgreSQL" ], [ how, "PL/Python" ] )
! $$ LANGUAGE plpythonu;
! </programlisting>
!       </para>
!      </listitem>
!     </varlistentry>
!
!     <varlistentry>
!      <term>Iterator (any object providing <symbol>__iter__</symbol> and
!       <symbol>next</symbol> methods)</term>
!      <listitem>
!       <para>
! <programlisting>
! CREATE FUNCTION greet (how text)
!   RETURNS SETOF greeting
! AS $$
!   class producer:
!     def __init__ (self, how, who):
!       self.how = how
!       self.who = who
!       self.ndx = -1
!
!     def __iter__ (self):
!       return self
!
!     def next (self):
!       self.ndx += 1
!       if self.ndx == len(self.who):
!         raise StopIteration
!       return ( self.how, self.who[self.ndx] )
!
!   return producer(how, [ "World", "PostgreSQL", "PL/Python" ])
! $$ LANGUAGE plpythonu;
! </programlisting>
!       </para>
!      </listitem>
!     </varlistentry>
!
!     <varlistentry>
!      <term>Generator (<literal>yield</literal>)</term>
!      <listitem>
!       <para>
! <programlisting>
! CREATE FUNCTION greet (how text)
!   RETURNS SETOF greeting
! AS $$
!   for who in [ "World", "PostgreSQL", "PL/Python" ]:
!     yield ( how, who )
! $$ LANGUAGE plpythonu;
! </programlisting>
!
!        <warning>
!         <para>
!          Currently, due to Python
!          <ulink
url="http://sourceforge.net/tracker/index.php?func=detail&aid=1483133&group_id=5470&atid=105470">bug
#1483133</ulink>,
!          some debug versions of Python 2.4
!          (configured and compiled with option <literal>--with-pydebug</literal>)
!          are known to crash the <productname>PostgreSQL</productname> server.
!          Unpatched versions of Fedora 4 contain this bug.
!          It does not happen in production version of Python or on patched
!          versions of Fedora 4.
!         </para>
!        </warning>
!       </para>
!      </listitem>
!     </varlistentry>
!    </variablelist>
!
!    Whenever new iterable types are added to Python language,
!    <application>PL/Python</application> is ready to use it.
    </para>

    <para>