Fixed #24921 -- set_autocommit(False) + ORM queries.

This commits lifts the restriction that the outermost atomic block must
be declared with savepoint=False. This restriction was overly cautious.

The logic that makes it safe not to create savepoints for inner blocks
also applies to the outermost block when autocommit is disabled and a
transaction is already active.

This makes it possible to use the ORM after set_autocommit(False).
Previously it didn't work because ORM write operations are protected
with atomic(savepoint=False).

4 files changed, 25 insertions, 11 deletions

diff --git a/django/db/transaction.py b/django/db/transaction.py
index 6c174bf2d3..e392c13b32 100644
--- a/django/db/transaction.py
+++ b/django/db/transaction.py
@@ -164,13 +164,6 @@ class Atomic(ContextDecorator):
                     raise TransactionManagementError(
                         "Your database backend doesn't behave properly when "
                         "autocommit is off. Turn it on before using 'atomic'.")
-                # When entering an atomic block with autocommit turned off,
-                # Django should only use savepoints and shouldn't commit.
-                # This requires at least a savepoint for the outermost block.
-                if not self.savepoint:
-                    raise TransactionManagementError(
-                        "The outermost 'atomic' block cannot use "
-                        "savepoint = False when autocommit is off.")
                 # Pretend we're already in an atomic block to bypass the code
                 # that disables autocommit to enter a transaction, and make a
                 # note to deal with this case in __exit__.
diff --git a/docs/releases/1.8.5.txt b/docs/releases/1.8.5.txt
index 9cdd802057..d412c4b653 100644
--- a/docs/releases/1.8.5.txt
+++ b/docs/releases/1.8.5.txt
@@ -46,3 +46,7 @@ Bugfixes
 
 * Readded inline foreign keys to form instances when validating model formsets
   (:ticket:`25431`).
+
+* Allowed using ORM write methods after disabling autocommit with
+  :func:`set_autocommit(False) <django.db.transaction.set_autocommit>`
+  (:ticket:`24921`).
diff --git a/docs/topics/db/transactions.txt b/docs/topics/db/transactions.txt
index 5261f301a0..ad4e027ef5 100644
--- a/docs/topics/db/transactions.txt
+++ b/docs/topics/db/transactions.txt
@@ -200,8 +200,12 @@ Django provides a single API to control database transactions.
     the error handling described above.
 
     You may use ``atomic`` when autocommit is turned off. It will only use
-    savepoints, even for the outermost block, and it will raise an exception
-    if the outermost block is declared with ``savepoint=False``.
+    savepoints, even for the outermost block.
+
+    .. versionchanged:: 1.8.5
+
+        Previously the outermost atomic block couldn't be declared with
+        ``savepoint=False`` when autocommit was turned off.
 
 .. admonition:: Performance considerations
 
diff --git a/tests/transactions/tests.py b/tests/transactions/tests.py
index dab406dbac..5d7851032f 100644
--- a/tests/transactions/tests.py
+++ b/tests/transactions/tests.py
@@ -398,16 +398,18 @@ class AtomicMiscTests(TransactionTestCase):
     available_apps = []
 
     def test_wrap_callable_instance(self):
-        # Regression test for #20028
+        """#20028 -- Atomic must support wrapping callable instances."""
+
         class Callable(object):
             def __call__(self):
                 pass
+
         # Must not raise an exception
         transaction.atomic(Callable())
 
     @skipUnlessDBFeature('can_release_savepoints')
     def test_atomic_does_not_leak_savepoints_on_failure(self):
-        # Regression test for #23074
+        """#23074 -- Savepoints must be released after rollback."""
 
         # Expect an error when rolling back a savepoint that doesn't exist.
         # Done outside of the transaction block to ensure proper recovery.
@@ -426,3 +428,14 @@ class AtomicMiscTests(TransactionTestCase):
 
                 # This is expected to fail because the savepoint no longer exists.
                 connection.savepoint_rollback(sid)
+
+    @skipIf(connection.features.autocommits_when_autocommit_is_off,
+            "This test requires a non-autocommit mode that doesn't autocommit.")
+    def test_orm_query_without_autocommit(self):
+        """#24921 -- ORM queries must be possible after set_autocommit(False)."""
+        transaction.set_autocommit(False)
+        try:
+            Reporter.objects.create(first_name="Tintin")
+        finally:
+            transaction.rollback()
+            transaction.set_autocommit(True)