Re: proposal: PL/Pythonu - function ereport

2015-12-03 16:57 GMT+01:00 Catalin Iacob <iacobcatalin(at)gmail(dot)com>:

> On Thu, Dec 3, 2015 at 7:58 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
>

understand

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

Regards

Pavel