Thread: [PATCH] Fix documentation about PL/Python exception handling
Hi lists, It seems that PL/Python exception functions (plpy.error, plpy.fatal) and exception types (plpy.Error, plpy.Fatal) have never worked as documented, and the behavior is quite surprising. This is an attempt at properly documenting the current semantics. I tested these behaviors on PostgreSQL 8.1.22 as well as 9.1alpha2. Patch 1 updates the documentation in doc/src/sgml/plpython.sgml Patch 2 adds tests to src/pl/plpython/sql/plpython_error.sql Should I submit separate documentation patches for older maintenance releases? Versions 8.1 - 8.4 contain the same incorrect text, but in the "Database Access" section. Regards, Marti
Attachment
Re: [PATCH] Fix documentation about PL/Python exception handling
From
Euler Taveira de Oliveira
Date:
Marti Raudsepp escreveu: > Should I submit separate documentation patches for older maintenance > releases? Versions 8.1 - 8.4 contain the same incorrect text, but in > the "Database Access" section. > Sure. But bear in mind that we don't usually backpatch documentation unless there is incorrect information; improvement belongs to HEAD. -- Euler Taveira de Oliveira http://www.timbira.com/
On Wed, Nov 10, 2010 at 16:07, Euler Taveira de Oliveira <euler@timbira.com> wrote: > Sure. But bear in mind that we don't usually backpatch documentation unless > there is incorrect information; improvement belongs to HEAD. Yeah, it is quite incorrect (as you can see by comparing test results from 1st post with the documentation). Attached is a documentation patch that will apply to PostgreSQL 8.1 - 8.4 Regards, Marti
Attachment
Marti Raudsepp <marti@juffo.org> writes: > On Wed, Nov 10, 2010 at 16:07, Euler Taveira de Oliveira > <euler@timbira.com> wrote: >> Sure. But bear in mind that we don't usually backpatch documentation unless >> there is incorrect information; improvement belongs to HEAD. > Yeah, it is quite incorrect (as you can see by comparing test results > from 1st post with the documentation). > Attached is a documentation patch that will apply to PostgreSQL 8.1 - 8.4 Hmmm ... I'm less than excited about documenting agreed-to-be-broken behavior in exquisite detail. Doing so will just encourage people to rely on it. It's particularly unfriendly to document it in detail without pointing out that it's likely to change. My inclination is to forget about these doc changes and spend our time on fixing the behavior instead. regards, tom lane
On Mon, Nov 15, 2010 at 22:25, Tom Lane <tgl@sss.pgh.pa.us> wrote: > My inclination is to forget about these doc changes and spend our time > on fixing the behavior instead. I thought the first step in fixing a problem is admitting the problem ;) But you raise a fair point. Regards, Marti
Tom Lane wrote: > Hmmm ... I'm less than excited about documenting agreed-to-be-broken > behavior in exquisite detail. Doing so will just encourage people to > rely on it. It's particularly unfriendly to document it in detail > without pointing out that it's likely to change. > > My inclination is to forget about these doc changes and spend our time > on fixing the behavior instead. > The open question for this one then is the right way for exceptions to act. Marti, since you've looked at this a bit already, any thoughts on what that should look like? You initially commented that what happens was surprising, but I'm not clear on what you expected to happen instead. -- Greg Smith 2ndQuadrant US greg@2ndQuadrant.com Baltimore, MD PostgreSQL Training, Services and Support www.2ndQuadrant.us
On Fri, Nov 19, 2010 at 05:29, Greg Smith <greg@2ndquadrant.com> wrote: > The open question for this one then is the right way for exceptions to act. > Marti, since you've looked at this a bit already, any thoughts on what that > should look like? You initially commented that what happens was surprising, > but I'm not clear on what you expected to happen instead. The documented behavior would make a lot more sense. For instance, currently, when you call plpy.error(), the exception that's thrown can be caught, but even if you handle it, the function still ends up failing. Similarly you CANNOT handle SQL errors -- at all! All errors are fatal. CREATE FUNCTION foo() RETURNS text AS 'try: plpy.execute("SELECT syntax error") except: plpy.notice("exception caught but cannot ignore") return "retval" ' LANGUAGE plpythonu; WARNING: PL/Python: plpy.SPIError: unrecognized error in PLy_spi_execute_query CONTEXT: PL/Python function "foo" NOTICE: exception caught but cannot ignore CONTEXT: PL/Python function "foo" ERROR: column "syntax" does not exist LINE 1: SELECT syntax error ^ QUERY: SELECT syntax error CONTEXT: PL/Python function "foo" Notice how the return value in the above is ignored, although the NOTICE is still output. Any new exceptions raised in the exception handler -- such as coding errors -- are also silently ignored. Another surprise is that plpy.Error and plpy.Fatal are just empty exception subclasses -- they behave nothing like plpy.error()/plpy.fatal(). Since they're normal Python exceptions, they can be caught and silenced. But an unhandled plpy.Fatal still results in an ERROR level message, go figure. I'm surprised that PL/Python got merged into Postgres at all without proper error handling. Regards, Marti
Added to TODO: /* PL/Python */ Add: |Improve behaviour of exception functions and types --------------------------------------------------------------------------- Marti Raudsepp wrote: > Hi lists, > > It seems that PL/Python exception functions (plpy.error, plpy.fatal) > and exception types (plpy.Error, plpy.Fatal) have never worked as > documented, and the behavior is quite surprising. > > This is an attempt at properly documenting the current semantics. I > tested these behaviors on PostgreSQL 8.1.22 as well as 9.1alpha2. > > Patch 1 updates the documentation in doc/src/sgml/plpython.sgml > Patch 2 adds tests to src/pl/plpython/sql/plpython_error.sql > > Should I submit separate documentation patches for older maintenance > releases? Versions 8.1 - 8.4 contain the same incorrect text, but in > the "Database Access" section. > > Regards, > Marti [ Attachment, skipping... ] [ Attachment, skipping... ] > > -- > Sent via pgsql-docs mailing list (pgsql-docs@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-docs -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce Momjian wrote: > Added to TODO: > > /* PL/Python */ Add: |Improve behaviour of exception functions > and types > I updated that to also link to Marti's message with more details about the problem and potential solutions here, to speed up future archive trawling. At this point I think the only sensible thing is to mark the submitted documentation patch "Returned with feedback". Then we wait to see if someone gets motivated to provide the more invasive behavior fix that seems the preferred improvement here instead. Maybe there's an advance slimmed down docs patch that makes sense too, for briefly documenting the limitations without so much detail that the description turns into a backward compatibility requirement. -- Greg Smith 2ndQuadrant US greg@2ndQuadrant.com Baltimore, MD
On tis, 2010-11-09 at 18:40 +0200, Marti Raudsepp wrote: > It seems that PL/Python exception functions (plpy.error, plpy.fatal) > and exception types (plpy.Error, plpy.Fatal) have never worked as > documented, and the behavior is quite surprising. This is now changed in 9.1devel, so I consider this matter closed.