Re: [PATCHES] 2 line patch to allow plpythonu functions to return
On Sun, 2006-02-26 at 18:40 -0500, Neil Conway wrote: 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: [...] Applied to HEAD. I'm still not quite satisfied with the error message: ereport(ERROR, (errcode(ERRCODE_DATATYPE_MISMATCH), errmsg(unexpected return value from plpython procedure), errdetail(void-returning functions must return \None\))); Suggestions for something better? -Neil ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] 2 line patch to allow plpythonu functions to return
Neil Conway [EMAIL PROTECTED] writes: Applied to HEAD. I'm still not quite satisfied with the error message: ereport(ERROR, (errcode(ERRCODE_DATATYPE_MISMATCH), errmsg(unexpected return value from plpython procedure), errdetail(void-returning functions must return \None\))); Suggestions for something better? Yeah, unexpected doesn't seem the mot juste here. Perhaps invalid or incorrect (our message style guidelines seem to suggest invalid as a good word in such cases). Also, say function not procedure because that's what it is in SQL terms. Also, the errdetail doesn't follow the guideline that says detail messages should be full sentences with proper capitalization and punctuation. Perhaps Functions returning type void must return None. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] 2 line patch to allow plpythonu functions to return
On Tue, 2006-02-28 at 15:26 -0500, Tom Lane wrote: Yeah, unexpected doesn't seem the mot juste here. [...] All good points -- thanks for the suggestions. I've applied the attached patch to HEAD. -Neil *** src/pl/plpython/expected/plpython_test.out 0f691c1fda20c66e8795d2f953fb79fc92fdd02c --- src/pl/plpython/expected/plpython_test.out 40eb5c736cf022165076b07299e5fad0fd8f841b *** *** 190,197 (1 row) SELECT test_void_func2(); -- should fail ! ERROR: unexpected return value from plpython procedure ! DETAIL: void-returning functions must return None SELECT test_return_none(), test_return_none() IS NULL AS is null; test_return_none | is null --+- --- 190,197 (1 row) SELECT test_void_func2(); -- should fail ! ERROR: invalid return value from plpython function ! DETAIL: Functions returning type void must return None. SELECT test_return_none(), test_return_none() IS NULL AS is null; test_return_none | is null --+- *** src/pl/plpython/plpython.c bb0fdc6a4bfc35b18b98e3c92f421c41c2898052 --- src/pl/plpython/plpython.c 4cce33c125fbf2a9102cc34e94d0cc1d3d47ea01 *** *** 769,776 if (plrv != Py_None) ereport(ERROR, (errcode(ERRCODE_DATATYPE_MISMATCH), ! errmsg(unexpected return value from plpython procedure), ! errdetail(void-returning functions must return \None\))); fcinfo-isnull = false; rv = (Datum) 0; --- 769,776 if (plrv != Py_None) ereport(ERROR, (errcode(ERRCODE_DATATYPE_MISMATCH), ! errmsg(invalid return value from plpython function), ! errdetail(Functions returning type \void\ must return \None\.))); fcinfo-isnull = false; rv = (Datum) 0; ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] 2 line patch to allow plpythonu functions to return
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 - 1.71
--- src/pl/plpython/plpython.c 26 Feb 2006 23:22:27 -
***
*** 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 - 1.3
--- src/pl/plpython/sql/plpython_function.sql 26 Feb 2006 23:27:49 -
***
*** 341,343
--- 341,358
rv =
Re: [PATCHES] 2 line patch to allow plpythonu functions to return void ...
James Robinson [EMAIL PROTECTED] writes: Shamelessly cloned from the parallel code in pltcl, an exception for void in denying pseudotypes being returned. Pl/tcl didn't reference VOIDOID anywhere else, so ... . 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)? regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] 2 line patch to allow plpythonu functions to return void ...
Tom,This sort of thing normally requires more thought than just removingthe safety check.What happens when the python code does/doesn't return a value, in both cases (declared return type void or not)?python functions are specified to return None, if no return is given. I recommend to also see a plpython function as a Python function, and return None if no return is specified. -- GHUM Harald Massapersuadere et programmareHarald Armin MassaReinsburgstraße 202b70197 Stuttgart0173/9409607-When I visit a mosque, I show my respect by taking off my shoes. I follow the customs, just as I do in a church, synagogue or other holy place. But if a believer demands that I, as a nonbeliever, observe his taboos in the public domain, he is not asking for my respect, but for my submission. And that is incompatible with a secular democracy.
Re: [PATCHES] 2 line patch to allow plpythonu functions to return void ...
On Feb 25, 2006, at 12:10 PM, Tom Lane wrote: James Robinson [EMAIL PROTECTED] writes: Shamelessly cloned from the parallel code in pltcl, an exception for void in denying pseudotypes being returned. Pl/tcl didn't reference VOIDOID anywhere else, so ... . 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)? regards, tom lane Yes of course. Here's some permutations of declared void functions explictly returning a value or not, with the closest thing to void in Python being None [ which is currently mapped to SQL NULL ] ... create or replace function void_ret_notvoid() returns void as $$ return 12 $$ language plpythonu; -- return value '' comes decorated with oid 2278, which seems to be VOIDOID. The 12 integer gets discarded. Ugly, yes. create or replace function void_ret_none() returns void as $$ return None $$ language plpythonu; -- Once again, returned oid to client is 2278, and likewise for the subsequent one select void_ret_none() is null; create or replace function void_ret_falloff_none() returns void as $$ x = 12 + 4 $$ language plpythonu; -- This one returns oid 23, with value NULL. create or replace function notvoid_ret_none() returns int as $$ return None $$ language plpythonu; Now, the python language semantics are that if a function does not explictly return a value, None is implictly returned: jlrobins:~ jlrobins$ python Python 2.4.1 (#2, Mar 31 2005, 00:05:10) [GCC 3.3 20030304 (Apple Computer, Inc. build 1666)] on darwin Type help, copyright, credits or license for more information. def falloff(): ... x = 12 ... val = falloff() val is None True So there's a bit of a mismatch between Python and SQL semantics since Python's None is already being used to represent NULL [ of whatever datatype the pl function was described to return at the SQL level ] in plpython. The ugliest case above is certainly the first one -- function declared to be void explicitly returning a not-None value. That should probably cause an SQL-level error. Can't really test it a function compile time, since Python variables are not typed, only what they reference are. The intent was to have to keep from having to declare a bogus return type for a a procedure that returns no meaningful results at all -- such as one which does nothing but inserts or updates or what have you. I suspect that all of the above functions returned VOIDOID decorating the return result due to machinery higher-up than plpython -- the postgres typing system itself, so those cases are probably silly examples, other than showing that it doesn't immediately crash. Which you probably would rather be shown a much higher confidence proof that the system is still correct aside from not immediately going down in flames. I'll go back to lurking and reading -- is the plpgsql source the best model for reading up on procedure language implementation? Thanks. James Robinson Socialserve.com ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
[PATCHES] 2 line patch to allow plpythonu functions to return void ...
Shamelessly cloned from the parallel code in pltcl, an exception for void in denying pseudotypes being returned. Pl/tcl didn't reference VOIDOID anywhere else, so ... . Allowed following trivial test function to succeed: create or replace function set_gd(int) returns void as $$ GD['global_count'] = args[0] $$ language plpythonu volatile; allow_plpython_return_void.diff Description: Binary data James Robinson Socialserve.com ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
