Don't execute a ROLLBACK on close()/GC.

The command wasn't sent since 2.2.0 due to a bug, but after a ML
discussion this behaviour proved more correct so the bug has become a
feature.

ChangeLog

@@ -1,3 +1,10 @@
+2010-08-05  Daniele Varrazzo  <daniele.varrazzo@gmail.com>
+
+	* psycopg/connection_int.c: don't execute a ROLLBACK on close()/GC.
+	The command wasn't sent since 2.2.0 due to a bug, but after a ML
+	discussion this behaviour proved more correct so the bug has become a
+	feature.
+
 2010-07-18  Federico Di Gregorio  <fog@initd.org>
 
 	* Release 2.2.2.

doc/src/connection.rst

@@ -67,10 +67,28 @@ The ``connection`` class
         `~psycopg2.InterfaceError` will be raised if any operation is
         attempted with the connection.  The same applies to all cursor objects
         trying to use the connection.  Note that closing a connection without
-        committing the changes first will cause an implicit rollback to be
+        committing the changes first will cause any pending change to be
-        performed (unless a different isolation level has been selected: see
+        discarded as if a :sql:`ROLLBACK` was performed (unless a different
+        isolation level has been selected: see
         `~connection.set_isolation_level()`).
 
+        .. index::
+            single: PgBouncer; unclean server
+
+        .. versionchanged:: 2.2
+            previously an explicit :sql:`ROLLBACK` was issued by Psycopg on
+            `!close()`. The command could have been sent to the backend at an
+            inappropriate time, so Psycopg currently relies on the backend to
+            implicitly discard uncommitted changes. Some middleware are known
+            to behave incorrectly though when the connection is closed during
+            a transaction (when `~connection.status` is
+            `~psycopg2.extensions.STATUS_IN_TRANSACTION`), e.g. PgBouncer_
+            reports an ``unclean server`` and discards the connection. To
+            avoid this problem you can ensure to terminate the transaction
+            with a `~connection.commit()`/`~connection.rollback()` before
+            closing.
+
+            .. _PgBouncer: http://pgbouncer.projects.postgresql.org/
 
     .. index::
         single: Exceptions; In the connection class

psycopg/connection_int.c

@@ -743,18 +743,15 @@ conn_close(connectionObject *self)
     Py_BEGIN_ALLOW_THREADS;
     pthread_mutex_lock(&self->lock);
 
-    /* execute a forced rollback on the connection (but don't check the
+    /* We used to call pq_abort_locked here, but the idea of issuing a
-       result, we're going to close the pq connection anyway */
+     * rollback on close/GC has been considered inappropriate.
-    if (self->pgconn && self->closed == 1) {
+     *
-        PGresult *pgres = NULL;
+     * Dropping the connection on the server has the same effect as the
-        char *error = NULL;
+     * transaction is automatically rolled back. Some middleware, such as
-
+     * PgBouncer, have problem with connections closed in the middle of the
-        if (pq_abort_locked(self, &pgres, &error, &_save) < 0) {
+     * transaction though: to avoid these problems the transaction should be
-            IFCLEARPGRES(pgres);
+     * closed only in status CONN_STATUS_READY.
-            if (error)
+     */
-                free (error);
-        }
-    }
 
     if (self->closed == 0)
         self->closed = 1;
@@ -763,7 +760,7 @@ conn_close(connectionObject *self)
         PQfinish(self->pgconn);
         Dprintf("conn_close: PQfinish called");
         self->pgconn = NULL;
-   }
+    }
 
     pthread_mutex_unlock(&self->lock);
     Py_END_ALLOW_THREADS;