- [feature] Added pool_reset_on_return argument

to create_engine, allows control over
"connection return" behavior.  Also added
new arguments 'rollback', 'commit', None
to pool.reset_on_return to allow more control
over connection return activity. [ticket:2378]

5 files changed, 88 insertions, 11 deletions

diff --git a/CHANGES b/CHANGES
index a9c823d28..5034b2475 100644
--- a/CHANGES
+++ b/CHANGES
@@ -19,6 +19,14 @@ CHANGES
     happen if there's really an UPDATE to occur.
     [ticket:2390]
 
+- engine
+  - [feature] Added pool_reset_on_return argument
+    to create_engine, allows control over 
+    "connection return" behavior.  Also added
+    new arguments 'rollback', 'commit', None
+    to pool.reset_on_return to allow more control
+    over connection return activity. [ticket:2378]
+
 0.7.5 (January 28, 2012)
 =====
 - orm
diff --git a/lib/sqlalchemy/engine/__init__.py b/lib/sqlalchemy/engine/__init__.py
index 4fac3e532..23b4b0b3b 100644
--- a/lib/sqlalchemy/engine/__init__.py
+++ b/lib/sqlalchemy/engine/__init__.py
@@ -306,6 +306,12 @@ def create_engine(*args, **kwargs):
         this is configurable with the MySQLDB connection itself and the
         server configuration as well).
 
+    :param pool_reset_on_return='rollback': set the "reset on return"
+        behavior of the pool, which is whether ``rollback()``, 
+        ``commit()``, or nothing is called upon connections
+        being returned to the pool.  See the docstring for
+        ``reset_on_return`` at :class:`.Pool`. (new as of 0.7.6)
+
     :param pool_timeout=30: number of seconds to wait before giving
         up on getting a connection from the pool. This is only used
         with :class:`~sqlalchemy.pool.QueuePool`.
diff --git a/lib/sqlalchemy/engine/strategies.py b/lib/sqlalchemy/engine/strategies.py
index 7b2da68c4..e3a6c5026 100644
--- a/lib/sqlalchemy/engine/strategies.py
+++ b/lib/sqlalchemy/engine/strategies.py
@@ -108,7 +108,8 @@ class DefaultEngineStrategy(EngineStrategy):
                          'timeout': 'pool_timeout',
                          'recycle': 'pool_recycle',
                          'events':'pool_events',
-                         'use_threadlocal':'pool_threadlocal'}
+                         'use_threadlocal':'pool_threadlocal',
+                         'reset_on_return':'pool_reset_on_return'}
             for k in util.get_cls_kwargs(poolclass):
                 tk = translate.get(k, k)
                 if tk in kwargs:
diff --git a/lib/sqlalchemy/pool.py b/lib/sqlalchemy/pool.py
index a615e8c60..cd935d4ae 100644
--- a/lib/sqlalchemy/pool.py
+++ b/lib/sqlalchemy/pool.py
@@ -57,6 +57,10 @@ def clear_managers():
         manager.close()
     proxies.clear()
 
+reset_rollback = util.symbol('reset_rollback')
+reset_commit = util.symbol('reset_commit')
+reset_none = util.symbol('reset_none')
+
 
 class Pool(log.Identified):
     """Abstract base class for connection pools."""
@@ -130,7 +134,16 @@ class Pool(log.Identified):
         self._creator = creator
         self._recycle = recycle
         self._use_threadlocal = use_threadlocal
-        self._reset_on_return = reset_on_return
+        if reset_on_return in ('rollback', True):
+            self._reset_on_return = reset_rollback
+        elif reset_on_return in (None, False):
+            self._reset_on_return = reset_none
+        elif reset_on_return == 'commit':
+            self._reset_on_return = reset_commit
+        else:
+            raise exc.ArgumentError("Invalid value for 'reset_on_return': %r" 
+                                    % reset_on_return)
+
         self.echo = echo
         if _dispatch:
             self.dispatch._update(_dispatch, only_propagate=False)
@@ -330,8 +343,10 @@ def _finalize_fairy(connection, connection_record, pool, ref, echo):
 
     if connection is not None:
         try:
-            if pool._reset_on_return:
+            if pool._reset_on_return is reset_rollback:
                 connection.rollback()
+            elif pool._reset_on_return is reset_commit:
+                connection.commit()
             # Immediately close detached instances
             if connection_record is None:
                 connection.close()
@@ -624,11 +639,37 @@ class QueuePool(Pool):
           :meth:`unique_connection` method is provided to bypass the
           threadlocal behavior installed into :meth:`connect`.
 
-        :param reset_on_return: If true, reset the database state of
-          connections returned to the pool.  This is typically a
-          ROLLBACK to release locks and transaction resources.
-          Disable at your own peril.  Defaults to True.
-
+        :param reset_on_return: Determine steps to take on 
+          connections as they are returned to the pool.   
+          As of SQLAlchemy 0.7.6, reset_on_return can have any 
+          of these values:
+          
+          * 'rollback' - call rollback() on the connection,
+            to release locks and transaction resources.
+            This is the default value.  The vast majority
+            of use cases should leave this value set.
+          * True - same as 'rollback', this is here for 
+            backwards compatibility.
+          * 'commit' - call commit() on the connection,
+            to release locks and transaction resources. 
+            A commit here may be desirable for databases that
+            cache query plans if a commit is emitted,
+            such as Microsoft SQL Server.  However, this
+            value is more dangerous than 'rollback' because
+            any data changes present on the transaction
+            are committed unconditionally.
+           * None - don't do anything on the connection.
+             This setting should only be made on a database
+             that has no transaction support at all,
+             namely MySQL MyISAM.   By not doing anything,
+             performance can be improved.   This
+             setting should **never be selected** for a 
+             database that supports transactions,
+             as it will lead to deadlocks and stale
+             state.
+            * False - same as None, this is here for
+              backwards compatibility.
+          
         :param listeners: A list of
           :class:`~sqlalchemy.interfaces.PoolListener`-like objects or
           dictionaries of callables that receive events when DB-API
diff --git a/test/engine/test_parseconnect.py b/test/engine/test_parseconnect.py
index ef15cde9f..032639557 100644
--- a/test/engine/test_parseconnect.py
+++ b/test/engine/test_parseconnect.py
@@ -2,7 +2,7 @@ from test.lib.testing import assert_raises, assert_raises_message, eq_
 import ConfigParser
 import StringIO
 import sqlalchemy.engine.url as url
-from sqlalchemy import create_engine, engine_from_config, exc
+from sqlalchemy import create_engine, engine_from_config, exc, pool
 from sqlalchemy.engine import _coerce_config
 import sqlalchemy as tsa
 from test.lib import fixtures, testing
@@ -175,6 +175,27 @@ pool_timeout=10
                           module=dbapi, _initialize=False)
         assert e.pool._recycle == 472
 
+    def test_reset_on_return(self):
+        dbapi = MockDBAPI(foober=12, lala=18, hoho={'this': 'dict'},
+                          fooz='somevalue')
+        for (value, expected) in [
+            ('rollback', pool.reset_rollback),
+            ('commit', pool.reset_commit),
+            (None, pool.reset_none),
+            (True, pool.reset_rollback),
+            (False, pool.reset_none),
+        ]:
+            e = create_engine('postgresql://', pool_reset_on_return=value,
+                          module=dbapi, _initialize=False)
+            assert e.pool._reset_on_return is expected
+
+        assert_raises(
+            exc.ArgumentError, 
+            create_engine, "postgresql://",
+            pool_reset_on_return='hi', module=dbapi,
+            _initialize=False
+        )
+
     def test_bad_args(self):
         assert_raises(exc.ArgumentError, create_engine, 'foobar://',
                       module=mock_dbapi)
@@ -223,10 +244,10 @@ pool_timeout=10
     @testing.requires.sqlite
     def test_invalidate_on_connect(self):
         """test that is_disconnect() is called during connect.
-        
+
         interpretation of connection failures are not supported by
         every backend.
-        
+
         """
         # pretend pysqlite throws the 
         # "Cannot operate on a closed database." error