2 line patch to allow plpythonu functions to return void ...

Started by James Robinsonover 20 years ago8 messagespatches
Jump to latest
#1James Robinson
jlrobins@socialserve.com

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;

Attachments:

allow_plpython_return_void.diffapplication/octet-stream; name=allow_plpython_return_void.diff; x-unix-mode=0664Download+2-0
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: James Robinson (#1)
Re: 2 line patch to allow plpythonu functions to return void ...

James Robinson <jlrobins@socialserve.com> 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

#3Harald Armin Massa
haraldarminmassa@gmail.com
In reply to: Tom Lane (#2)
Re: 2 line patch to allow plpythonu functions to return void ...

Tom,

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)?

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 Massa
persuadere et programmare
Harald Armin Massa
Reinsburgstraße 202b
70197 Stuttgart
0173/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.

#4James Robinson
jlrobins@socialserve.com
In reply to: Tom Lane (#2)
Re: 2 line patch to allow plpythonu functions to return void ...

On Feb 25, 2006, at 12:10 PM, Tom Lane wrote:

James Robinson <jlrobins@socialserve.com> 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

#5Neil Conway
neilc@samurai.com
In reply to: Tom Lane (#2)
Re: 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

Attachments:

plpython_return_void-1.patchtext/x-patch; name=plpython_return_void-1.patch; x-mac-creator=0; x-mac-type=0Download+50-29
#6Neil Conway
neilc@samurai.com
In reply to: Neil Conway (#5)
Re: 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

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Neil Conway (#6)
Re: 2 line patch to allow plpythonu functions to return

Neil Conway <neilc@samurai.com> 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

#8Neil Conway
neilc@samurai.com
In reply to: Tom Lane (#7)
Re: 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

Attachments:

plpython_err_msg_tweak-1.patchtext/x-patch; charset=us-ascii; name=plpython_err_msg_tweak-1.patchDownload+8-8