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).
This commit is contained in:
Aymeric Augustin 2015-09-21 20:53:10 +02:00
parent 361254810e
commit 91e9f1c972
4 changed files with 25 additions and 11 deletions

View File

@ -164,13 +164,6 @@ class Atomic(ContextDecorator):
raise TransactionManagementError( raise TransactionManagementError(
"Your database backend doesn't behave properly when " "Your database backend doesn't behave properly when "
"autocommit is off. Turn it on before using 'atomic'.") "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 # Pretend we're already in an atomic block to bypass the code
# that disables autocommit to enter a transaction, and make a # that disables autocommit to enter a transaction, and make a
# note to deal with this case in __exit__. # note to deal with this case in __exit__.

View File

@ -46,3 +46,7 @@ Bugfixes
* Readded inline foreign keys to form instances when validating model formsets * Readded inline foreign keys to form instances when validating model formsets
(:ticket:`25431`). (:ticket:`25431`).
* Allowed using ORM write methods after disabling autocommit with
:func:`set_autocommit(False) <django.db.transaction.set_autocommit>`
(:ticket:`24921`).

View File

@ -200,8 +200,12 @@ Django provides a single API to control database transactions.
the error handling described above. the error handling described above.
You may use ``atomic`` when autocommit is turned off. It will only use 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 savepoints, even for the outermost block.
if the outermost block is declared with ``savepoint=False``.
.. versionchanged:: 1.8.5
Previously the outermost atomic block couldn't be declared with
``savepoint=False`` when autocommit was turned off.
.. admonition:: Performance considerations .. admonition:: Performance considerations

View File

@ -398,16 +398,18 @@ class AtomicMiscTests(TransactionTestCase):
available_apps = [] available_apps = []
def test_wrap_callable_instance(self): def test_wrap_callable_instance(self):
# Regression test for #20028 """#20028 -- Atomic must support wrapping callable instances."""
class Callable(object): class Callable(object):
def __call__(self): def __call__(self):
pass pass
# Must not raise an exception # Must not raise an exception
transaction.atomic(Callable()) transaction.atomic(Callable())
@skipUnlessDBFeature('can_release_savepoints') @skipUnlessDBFeature('can_release_savepoints')
def test_atomic_does_not_leak_savepoints_on_failure(self): 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. # Expect an error when rolling back a savepoint that doesn't exist.
# Done outside of the transaction block to ensure proper recovery. # 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. # This is expected to fail because the savepoint no longer exists.
connection.savepoint_rollback(sid) 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)