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