Re: 2 line patch to allow plpythonu functions to return - Mailing list pgsql-patches

Tom Lane wrote:
> This sort of thing normally requires more thought than just removing
> the safety check.  What happens when the python code does/doesn't return
> a value, in both cases (declared return type void or not)?

Attached is a more complete patch:

- if the function is declared to return void, we only accept "None" as
the return value from Python. Returning anything else produces an error.

- if the function is declared to return void and Python returns "None",
we return this to PG as a void datum (*not* NULL)

- if the function is not declared to return void and Python returns
"None", we return this to PG as a NULL datum

One minor inconsistency is that PL/PgSQL (for example) actually
disallows "RETURN expr;" in void-returning functions: only "RETURN;" can
be used. In PL/Python we don't place any restrictions on the syntax of
the function: "return expr" is allowed in void-returning functions so
long as `expr' evaluates to None. I don't think this is a major problem,
though.

The error message for the first case isn't quite right, and I need to
update the "expected" regression tests and possibly the documentation.
But otherwise I'll apply this patch to HEAD tomorrow, barring any
objections.

-Neil
Index: src/pl/plpython/plpython.c
===================================================================
RCS file: /Users/neilc/postgres/cvs_root/pgsql/src/pl/plpython/plpython.c,v
retrieving revision 1.71
diff -c -r1.71 plpython.c
*** src/pl/plpython/plpython.c    20 Feb 2006 20:10:37 -0000    1.71
--- src/pl/plpython/plpython.c    26 Feb 2006 23:22:27 -0000
***************
*** 91,97 ****
   */
  typedef struct PLyObToDatum
  {
!     FmgrInfo    typfunc;
      Oid            typioparam;
      bool        typbyval;
  }    PLyObToDatum;
--- 91,98 ----
   */
  typedef struct PLyObToDatum
  {
!     FmgrInfo    typfunc;        /* The type's input function */
!     Oid            typoid;            /* The OID of the type */
      Oid            typioparam;
      bool        typbyval;
  }    PLyObToDatum;
***************
*** 138,144 ****
      int            nargs;
      PyObject   *code;            /* compiled procedure code */
      PyObject   *statics;        /* data saved across calls, local scope */
!     PyObject   *globals;        /* data saved across calls, global score */
      PyObject   *me;                /* PyCObject containing pointer to this
                                   * PLyProcedure */
  }    PLyProcedure;
--- 139,145 ----
      int            nargs;
      PyObject   *code;            /* compiled procedure code */
      PyObject   *statics;        /* data saved across calls, local scope */
!     PyObject   *globals;        /* data saved across calls, global scope */
      PyObject   *me;                /* PyCObject containing pointer to this
                                   * PLyProcedure */
  }    PLyProcedure;
***************
*** 757,765 ****
              elog(ERROR, "SPI_finish failed");

          /*
!          * convert the python PyObject to a postgresql Datum
           */
!         if (plrv == Py_None)
          {
              fcinfo->isnull = true;
              rv = PointerGetDatum(NULL);
--- 758,781 ----
              elog(ERROR, "SPI_finish failed");

          /*
!          * If the function is declared to return void, the Python
!          * return value must be None. For void-returning functions, we
!          * also treat a None return value as a special "void datum"
!          * rather than NULL (as is the case for non-void-returning
!          * functions).
           */
!         if (proc->result.out.d.typoid == VOIDOID)
!         {
!             if (plrv != Py_None)
!                 ereport(ERROR,
!                         (errcode(ERRCODE_DATA_EXCEPTION),
!                          errmsg("unexpected return value from plpython procedure"),
!                          errdetail("function returns void")));
!
!             fcinfo->isnull = false;
!             rv = (Datum) 0;
!         }
!         else if (plrv == Py_None)
          {
              fcinfo->isnull = true;
              rv = PointerGetDatum(NULL);
***************
*** 1031,1038 ****
                       procStruct->prorettype);
              rvTypeStruct = (Form_pg_type) GETSTRUCT(rvTypeTup);

!             /* Disallow pseudotype result */
!             if (rvTypeStruct->typtype == 'p')
              {
                  if (procStruct->prorettype == TRIGGEROID)
                      ereport(ERROR,
--- 1047,1055 ----
                       procStruct->prorettype);
              rvTypeStruct = (Form_pg_type) GETSTRUCT(rvTypeTup);

!             /* Disallow pseudotype result, except for void */
!             if (rvTypeStruct->typtype == 'p' &&
!                 procStruct->prorettype != VOIDOID)
              {
                  if (procStruct->prorettype == TRIGGEROID)
                      ereport(ERROR,
***************
*** 1329,1334 ****
--- 1346,1352 ----
      Form_pg_type typeStruct = (Form_pg_type) GETSTRUCT(typeTup);

      perm_fmgr_info(typeStruct->typinput, &arg->typfunc);
+     arg->typoid = HeapTupleGetOid(typeTup);
      arg->typioparam = getTypeIOParam(typeTup);
      arg->typbyval = typeStruct->typbyval;
  }
Index: src/pl/plpython/sql/plpython_function.sql
===================================================================
RCS file: /Users/neilc/postgres/cvs_root/pgsql/src/pl/plpython/sql/plpython_function.sql,v
retrieving revision 1.3
diff -c -r1.3 plpython_function.sql
*** src/pl/plpython/sql/plpython_function.sql    10 Jul 2005 04:56:55 -0000    1.3
--- src/pl/plpython/sql/plpython_function.sql    26 Feb 2006 23:27:49 -0000
***************
*** 341,343 ****
--- 341,358 ----
  rv = plpy.execute(plan, u"\\x80", 1)
  return rv[0]["testvalue1"]
  ' LANGUAGE plpythonu;
+
+ -- Tests for functions that return void
+
+ CREATE FUNCTION test_void_func1() RETURNS void AS $$
+ x = 10
+ $$ LANGUAGE plpythonu;
+
+ -- illegal: can't return non-None value in void-returning func
+ CREATE FUNCTION test_void_func2() RETURNS void AS $$
+ return 10
+ $$ LANGUAGE plpythonu;
+
+ CREATE FUNCTION test_return_none() RETURNS int AS $$
+ None
+ $$ LANGUAGE plpythonu;
\ No newline at end of file
Index: src/pl/plpython/sql/plpython_test.sql
===================================================================
RCS file: /Users/neilc/postgres/cvs_root/pgsql/src/pl/plpython/sql/plpython_test.sql,v
retrieving revision 1.1
diff -c -r1.1 plpython_test.sql
*** src/pl/plpython/sql/plpython_test.sql    14 May 2005 17:55:21 -0000    1.1
--- src/pl/plpython/sql/plpython_test.sql    26 Feb 2006 23:29:38 -0000
***************
*** 68,70 ****
--- 68,75 ----
  SELECT newline_lf();
  SELECT newline_cr();
  SELECT newline_crlf();
+
+ -- Tests for functions returning void
+ SELECT test_void_func1(), test_void_func1() IS NULL AS "is null";
+ SELECT test_void_func2(); -- should fail
+ SELECT test_return_none(), test_return_none() IS NULL AS "is null";
\ No newline at end of file