mvantellingen / psycopg2-ctypes

ctypes port of psycopg2 (see https://github.com/chtd/psycopg2cffi)
Other
56 stars 59 forks source link

Report connection as closed if internal flag is set or libpq knows that connection is closed. #18

Closed smira closed 12 years ago

smira commented 12 years ago

This fixes problem when connection is closed on backend side and conn.closed() still reports False, while conn.fileno() would return -1, indicating that connection is broken.

It is strange though, that even the implementation of closed() is the same in psycopg2 and psycopg2-ctypes, psycopg2 doesn't expose this problem.

mvantellingen commented 12 years ago

I actually think we need to mirror the error handling as done by psycopg2, see https://github.com/dvarrazzo/psycopg/blob/devel/psycopg/pqpath.c#L846

smira commented 12 years ago

The issue here is not with query execution, but with txpostgres + psycopg2-ctypes (psycopg2 doesn't expose this problem for some reason). The problem is that PQstatus is not exposed through psycopg2/psycopg2-ctypes interface, while it gives much better information about connection status compared to current _closed flag (and method closed()). Right now _closed becomes True only when we close connection on our side, while connection could be dropped on backend side, and this thing goes almost completely unnoticed.

Do you think that it is better to get path like this accepted in psycopg2 and then ported to psycopg2-ctypes?

mvantellingen commented 12 years ago

Well the idea is to implement the psycopg2 module via ctypes as perfect as possible. I just committed a patch which should properly set the closed attribute when a fatal error occurs (as done by psycopg2).

I do actually think it is a good idea to make the PQstatus() accessible via a public API, but I think this should then also be done in psycopg2. (so to your last question, yes :-))

smira commented 12 years ago

Your commit 0a6af1cce2ad4d10089a33ba5244be0dc5367f87 fixes the issue! Thanks!