Re: Improved transaction error handling

On 02/01/2008, James Henstridge <[email protected]> wrote:
> On 29/12/2007, Federico Di Gregorio <fog-NGVKUo/i/[email protected]> wrote:
> >
> > Il giorno ven, 28/12/2007 alle 05.50 +0900, James Henstridge ha scritto:
> > > Attached is a patch to improve the transaction error handling.
> > >
> > > It gets rid of the pq_set_critical() usage in Connection.commit() and
> > > Connection.rollback(), and preserves the PGresult from commit/rollback
> > > errors so that it can be used to raise an appropriate exception.
> > >
> > > This means that we can set an appropriate "pgcode" attribute on the
> > > exceptions, allowing for better error handling.
> >
> > This would be very useful but I don't understand if you need all the
> > other changes (I mean the _locked versions of the various functions) or
> > that is a different feature. Can you explain, please?
>
> Sorry for the delay in replying -- I was on holiday without any
> internet access (or mobile signal).
>
> I see the patch I sent as an intermediate step towards cleaning up
> this part of Psycopg.  While it isn't as clean as it could be, it is
> an improvement over what's there and provides a basis for further
> improvements.
>
> As it is now, some of the pqpath.[ch] functions are expected to be
> called with the GIL held, while others expect to be called without the
> GIL held and the connection's lock acquired.
>
> In my patch, the functions named as *_locked() expect a locked
> connection (without the GIL held), and the other ones expect to be
> called with the GIL held.  In order to properly convert the libpq
> errors to Python exceptions, the pq_commit() and pq_abort() functions
> were converted to run with the GIL held.
>
> There were some places where pq_abort() was being called as part of
> another method (e.g. setting charset, changing isolation level).  For
> these, I added the pq_abort_locked() call.  To simplify matters, I
> gave it the same calling conventions as the old pq_abort() call.  As a
> further improvement, it would be good for it to pass on the libpq
> error so that callers can raise appropriate exceptions.  I haven't
> made that change yet to keep the rest of the patch clear.

Do you have any changes you'd like me to make to this patch, or is it
okay to commit?

James.