Weird PL/Python elog output

On 10/30/09, Peter Eisentraut <peter_e@gmx.net> wrote:

Calling PL/Python's elog functions exposes some curious behavior. For
example, calling plpy.error('foo') prints

ERROR: ('foo',)

(instead of the

ERROR: foo

that one might have hoped for.) This is an implementation artifact,
because those functions don't check their arguments, just take them as a
tuple, convert the tuple to a string, and a singleton tuples look like
the above as a string.

The simple way to amend this is to force these functions to take exactly
one argument print that. If people then actually want to pass a tuple,
they should form one explicitly. This approach might break user's
applications, however, if they have felt free to write things like
plpy.error('error code', n). Although passing more than one argument is
not documented, so arguably it can't be expected to work.

Other ways to fix this would be: Check if the number of arguments is
one. If yes, print that; else print the whole tuple. Or perhaps loop
through all the arguments and explicitly print each separated by a
comma.

Comments?

I vote for handling tuple with 1 element better, otherwise keep old
behaviour.

I don't think breaking multi-arg calls is good idea, as they may be used
only in special situations. OTOH, it does not seem worthwhile to
spend effort trying to handle them better.

--
marko

On fre, 2009-10-30 at 17:13 +0200, Marko Kreen wrote:

On 10/30/09, Peter Eisentraut <peter_e@gmx.net> wrote:

Calling PL/Python's elog functions exposes some curious behavior. For
example, calling plpy.error('foo') prints

ERROR: ('foo',)

(instead of the

ERROR: foo

that one might have hoped for.) This is an implementation artifact,
because those functions don't check their arguments, just take them as a
tuple, convert the tuple to a string, and a singleton tuples look like
the above as a string.

I vote for handling tuple with 1 element better, otherwise keep old
behaviour.

I don't think breaking multi-arg calls is good idea, as they may be used
only in special situations. OTOH, it does not seem worthwhile to
spend effort trying to handle them better.

OK, how about this:

diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index 313b760..783f625 100644
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -3080,6 +3080,13 @@ PLy_output(volatile int level, PyObject *self,
PyObject *args)
    char       *volatile sv;
    MemoryContext oldcontext;

+   if (PyTuple_Size(args) == 1)
+   {
+       PyObject *o;
+       PyArg_UnpackTuple(args, "plpy.elog", 1, 1, &o);
+       so = PyObject_Str(o);
+   }
+   else
    so = PyObject_Str(args);
    if (so == NULL || ((sv = PyString_AsString(so)) == NULL))
    {

On 10/31/09, Peter Eisentraut <peter_e@gmx.net> wrote:

On fre, 2009-10-30 at 17:13 +0200, Marko Kreen wrote:

I vote for handling tuple with 1 element better, otherwise keep old
behaviour.

I don't think breaking multi-arg calls is good idea, as they may be used
only in special situations. OTOH, it does not seem worthwhile to
spend effort trying to handle them better.

OK, how about this:

Seems OK by me.

--
marko