2015-12-03 16:57 GMT+01:00 Catalin Iacob <iacobcata...@gmail.com>:

> On Thu, Dec 3, 2015 at 7:58 AM, Pavel Stehule <pavel.steh...@gmail.com>
> wrote:
> > I am sorry, I don't understand. Now due inheritance plpy.Fatal and
> > plpy.SPIError has availability to use keyword parameters.
> Indeed, I didn't realize this, but I don't think it changes the main
> argument.
> What I think should happen:
> 1. Error should take keyword arguments
> 2. same for Fatal


> 3. Fatal should not be changed to inherit from Error - it should stay
> like it is now, just another exception class
>     You can argue a Fatal error is an Error but these classes already
> exist and are independent, changing their relationship is backward
> incompatible.

Don't understand - if Fatal has same behave as Error, then why it cannot be
inherited from Error?

What can be broken?

> 4. SPIError should not be changed at all
>      It's for errors raised by query execution not user PL/Python code
> so doing raise SPIError in PL/Python doesn't make sense.
>      And again, changing the inheritance relationship of these
> existing classes changes meaning of existing code that catches the
> exceptions.

can be

> 5. all the reporting functions: plpy.debug...plpy.fatal functions
> should also be changed to allow more arguments than the message and
> allow them as keyword arguments

this is maybe bigger source of broken compatibility

lot of people uses plpy.debug(var1, var2, var3)

rich constructor use $1 for message, $2 for detail, $3 for hint. So it was
a reason, why didn't touch these functions

>      They are Python wrappers for ereport and this would make them
> similar in capabilities to the PL/pgSQL RAISE
>      This will make plpy.error(...) stay equivalent in capabilities
> with raise plpy.Error(...), same for fatal and Fatal
> 6. the docs should move to the "Utility Functions" section
>      There, it's already described how to raise errors either via the
> exceptions or the utility functions.
> I think the above doesn't break anything, doesn't confuse user
> exceptions with backend SPIError exceptions, enhances error reporting
> features for the PL/Python user to bring them up to par with ereport
> and PL/pgSQL RAISE and solve your initial use case at the top of the
> thread (but with slightly different spelling to match what already
> exists in PL/Python):
> "We cannot to raise PostgreSQL exception with setting all possible
> fields. I propose new function
> plpy.ereport(level, [ message [, detail [, hint [, sqlstate, ... ]]]])"

I am not against to plpy.ereport function - it can live together with rich
exceptions class, and users can use what they prefer.

It is few lines more

> Is what I mean more clear now? Do you (and others) agree?

not too much  :)



Reply via email to