On Mon, Jan 11, 2016 at 7:33 PM, Pavel Stehule <pavel.steh...@gmail.com> wrote: > I see it. > > it looks like distinguish between Error and SPIError is wrong way. And I > have any other ugly use case. > > If I raise a Error from one PLPythonu function, then in other PLPython > function I'll trap a SPIError exception, because the information about class > was lost inside Postgres. And it should be pretty messy. > I have not information if any exception was User based or SPI based.
This isn't that bad. The real Python exception is only lost if the first function calls into Postgres which then ends into another PL/Python function which raises. That's just the way it is. If you use Psycopg2 on the client and a PL/Python function in the server you also don't get in the client the real exception that PL/Python raised even though it's Python at both ends. The analogy isn't perfect since one is in process and the other cross process but you get the point. If a PL/Python function calls another one directly and that one raises something, the caller can catch that just like in regular Python. > The differentiation between Error and SPIError is wrong, because there isn't > any difference in reality. They're similar but not really the same thing. raise Error and plpy.error are both ways to call ereport(ERROR, ...) while SPIError is raised when coming back after calling into Postgres to execute SQL that itself raises an error. Now indeed, that executed SQL raised an error itself via another ereport(ERROR, ...) somewhere but that's a different thing. I'm getting more and more convinced that SPIError should be left unchanged and should not even inherit from BaseError as it's a different thing. And as I said this means BaseError isn't the base of all exceptions that can be raised by the plpy module so then it should probably not be called BaseError. Maybe something like ReportableError (the base class of ereport frontends). Or don't have a base class at all and just allow constructing both Error and Fatal instances with message, detail, hint etc. What you loose is you can't catch both Error and Fatal with a single except ReportableError block but Fatal is maybe not meant to be caught and Error is also not really meant to be caught either, it's meant to be raised in order to call ereport(ERROR, ...). I now like this option (no base class but same attributes in both instances) most. As I see it, what this patch tries to solve is that raise Error and plpy.error (and all the others) are not exposing all features of ereport, they can only pass a message to ereport but can't pass all the other things ereport accepts: detail, hint etc. The enhancement is being able to pass all those arguments (as positional or keyword arguments) when constructing and raising an Error or Fatal instance or when using the plpy.error helpers. Since the existing helpers accept multiple arguments already (which they unfortunately and weirdly concatenate in a tuple whose string representation is pushed into ereport as message) we can't repurpose the multiple arguments as ereport detail, hint etc. so Pavel invented paralel plpy.raise_error and friends which do that. If a bold committer says the string representation of the tuple is too weird to be relied on we could even just change meaning of plpy.error and accept (and document) the compatibility break. So the patch is almost there I think, it just needs to stop messing around with SPIError and the spidata attribute. -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers