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
---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster
