Fixed session.transaction.commit() on a autocommit=False session not starting a new transaction.

Moved starting a new transaction in case of previous closing into SessionTransaction.

3 files changed, 14 insertions, 7 deletions

diff --git a/CHANGES b/CHANGES
index 3f981b927..6ce7d07c7 100644
--- a/CHANGES
+++ b/CHANGES
@@ -40,6 +40,9 @@ CHANGES
     - Removed the "raiseerror" keyword argument from object_mapper()
       and class_mapper().  These functions raise in all cases
       if the given class/instance is not mapped.
+
+    - Fixed session.transaction.commit() on a autocommit=False
+      session not starting a new transaction.
 - sql
     - column.in_(someselect) can now be used as a columns-clause
       expression without the subquery bleeding into the FROM clause
diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index 0ca59141e..cbff0ef0d 100644
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -428,6 +428,8 @@ class SessionTransaction(object):
                     connection.close()
                 else:
                     transaction.close()
+            if not self.session.autocommit:
+                self.session.begin()
         self._deactivate()
         self.session = None
         self._connections = None
@@ -641,8 +643,6 @@ class Session(object):
             pass
         else:
             self.transaction.rollback()
-        if self.transaction is None and not self.autocommit:
-            self.begin()
 
     def commit(self):
         """Flush pending changes and commit the current transaction.
@@ -667,8 +667,6 @@ class Session(object):
                 raise sa_exc.InvalidRequestError("No transaction is begun.")
 
         self.transaction.commit()
-        if self.transaction is None and not self.autocommit:
-            self.begin()
 
     def prepare(self):
         """Prepare the current transaction in progress for two phase commit.
@@ -772,9 +770,6 @@ class Session(object):
         if self.transaction is not None:
             for transaction in self.transaction._iterate_parents():
                 transaction.close()
-        if not self.autocommit:
-            # note this doesnt use any connection resources
-            self.begin()
 
     def close_all(cls):
         """Close *all* sessions in memory."""
diff --git a/test/orm/session.py b/test/orm/session.py
index f6cd2cc91..459713793 100644
--- a/test/orm/session.py
+++ b/test/orm/session.py
@@ -572,6 +572,15 @@ class SessionTest(_fixtures.FixtureTest):
         self.assertRaisesMessage(sa.exc.InvalidRequestError, "inactive due to a rollback in a subtransaction", sess.begin, subtransactions=True)
         sess.close()
 
+    @testing.resolve_artifact_names
+    def test_no_autocommit_with_explicit_commit(self):
+        mapper(User, users)
+        session = create_session(autocommit=False)
+
+        session.add(User(name='ed'))
+        session.transaction.commit()
+        assert session.transaction is not None, "autocommit=False should start a new transaction"
+
     @engines.close_open_connections
     @testing.resolve_artifact_names
     def test_bound_connection(self):