Re: proposal: PL/Pythonu - function ereport

Hi

2016-02-16 17:18 GMT+01:00 Catalin Iacob <iacobcatalin(at)gmail(dot)com>:

> On Sat, Feb 13, 2016 at 4:26 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> > I am sending new version. Current version does:
>
> Hello,
>
> I had a look and I like the big picture.
>
> Tests fail when built against Python 3.5 with stuff like this in
> regression.diffs:
> -ERROR: plpy.Error: stop on error
> -DETAIL: some detail
> -HINT: some hint
> -CONTEXT: Traceback (most recent call last):
> - PL/Python function "elog_test", line 18, in <module>
> - plpy.error('stop on error', 'some detail','some hint')
> -PL/Python function "elog_test"
> +ERROR: could not convert Python Unicode object to bytes
> +DETAIL: TypeError: bad argument type for built-in operation
> +CONTEXT: PL/Python function "elog_test"
>

fixed - the object serialization fails on Py_None

>
> This is related to the use of PyString_AsString and the changed
> semantics of that in Python 3 (due to the fact that strings are now
> Unicode objects and so on). Didn't have time to dig more deeply into
> the exact cause.
>
> Similarly, there are alternative expected test outputs that you didn't
> update, for example src/pl/plpython/expected/plpython_types_3.out so
> tests fail on some Python versions due to those as well.
>
> > 1. the plpy utility functions can use all ErrorData fields,
> > 2. there are no new functions,
> > 3. via GUC plpythonu.legacy_custom_exception we can return previous
> behave,
> > 4. only exception Error is raised with natural structure - no composite
> > value spidata.
> > 5. fields: message, detail and hint are implicitly translated to string
> - it
> > decrease a necessity of legacy mode
>
> I disagree that 5. is a good idea. I think we should just treat
> message, detail and hint like the other ones (schema, table etc.). Use
> s in PyArg_ParseTupleAndKeywords and let the user explicitly cast to a
> string. Explicit is better than implicit. The way you did it you keep
> part of the weird old interface which used to cast to string for you.
> We shouldn't keep warts of the old behaviour, especially with the GUC
> to ask for the old behaviour.
>

removed @5

>
> By the way, getting rid of object_to_string and its usage in
> PLy_output_kw removed some "ERROR: could not convert Python Unicode
> object to bytes" failures in the tests so I'm quite sure that the
> usage of PyString_AsString is responsible for those.
>
> I don't like that you set legacy_custom_exception=true in some
> existing tests, probably to avoid changing them to the new behaviour.
> We should trust that the new behaviour is what we want and the tests
> should reflect that. If it's too much work, remember we're asking
> users to do the same work to convert their code. We have
> elog_test_legacy to test elog_test_legacy=true, the rest of the tests
> should use legacy_custom_exception=false.
>

all regress tests now works in new mode

Regards

Pavel