On Tue, Dec 1, 2015 at 2:17 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> here is complete patch - regress tests for all supported Python branches
I had a look at what changed in v10 since my last reviewed version and
indeed most of it is straightforward: renames from SPIError to Error.
The patch also changes plpy.Fatal and plpy.SPIError to inherit from
the existing plpy.Error which is a backward incompatibility: catching
a plpy.Error will now also catch SPIError and Fatal.
But by doing this I noticed plpy.Error already existed without the
patch and raising plpy.Error(msg) is documented as equivalent to
calling plpy.error(msg), similar for plpy.Fatal and plpy.fatal(). This
patch makes it possible to raise plpy.Error with more arguments
including keyword ones but doesn't change plpy.error(msg). And Fatal
is not touched so it becomes inconsistent with Error.
So I now think the approach this patch took is wrong. We should
instead enhance the existing error and fatal functions and Error and
Fatal exceptions to accept other arguments that ereport accepts (hint,
sqlstate) and be able to pass all those as keyword parameters.
SPIError should be left unchanged as that's for errors raised by query
execution not by the PL/Python user. This is also close to Pavel's
original ereport proposal but by extending existing functionality
instead of adding a new function and it covers Peter's observation
that in Python the ereport function should be "raise an exception" as
that's already an alternative way of doing it.