Commit 91e9f1c9 authored by Aymeric Augustin's avatar Aymeric Augustin
Browse files

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).
parent 36125481

django/db/transaction.py

+0 −7
Original line number Diff line number Diff line
@@ -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__.

docs/releases/1.8.5.txt

+4 −0
Original line number Diff line number Diff line
@@ -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`).

docs/topics/db/transactions.txt

+6 −2
Original line number Diff line number Diff line
@@ -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

tests/transactions/tests.py

+15 −2
Original line number Diff line number Diff line
@@ -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)