From f572ee0c65ec5eac9edb0cb3e35c96ec86d89ffb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anssi=20K=C3=A4=C3=A4ri=C3=A4inen?= Date: Sat, 26 May 2012 23:19:13 +0300 Subject: [PATCH] Fixed #16047 -- Restore autocommit state correctly on psycopg2 When the postgresql_psycopg2 backend was used with DB-level autocommit mode enabled, after entering transaction management and then leaving it, the isolation level was never set back to autocommit mode. Thanks brodie for report and working on this issue. --- django/db/backends/__init__.py | 14 +++-- docs/releases/1.5.txt | 9 +++ .../transactions_regress/tests.py | 60 ++++++++++++++++++- 3 files changed, 76 insertions(+), 7 deletions(-) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index dab9b7e213..05ef7bf62a 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -109,16 +109,18 @@ class BaseDatabaseWrapper(object): over to the surrounding block, as a commit will commit all changes, even those from outside. (Commits are on connection level.) """ - self._leave_transaction_management(self.is_managed()) if self.transaction_state: del self.transaction_state[-1] else: - raise TransactionManagementError("This code isn't under transaction " - "management") + raise TransactionManagementError( + "This code isn't under transaction management") + # We will pass the next status (after leaving the previous state + # behind) to subclass hook. + self._leave_transaction_management(self.is_managed()) if self._dirty: self.rollback() - raise TransactionManagementError("Transaction managed block ended with " - "pending COMMIT/ROLLBACK") + raise TransactionManagementError( + "Transaction managed block ended with pending COMMIT/ROLLBACK") self._dirty = False def validate_thread_sharing(self): @@ -176,6 +178,8 @@ class BaseDatabaseWrapper(object): """ if self.transaction_state: return self.transaction_state[-1] + # Note that this setting isn't documented, and is only used here, and + # in enter_transaction_management() return settings.TRANSACTIONS_MANAGED def managed(self, flag=True): diff --git a/docs/releases/1.5.txt b/docs/releases/1.5.txt index 2f20f5f9f9..4544be0eac 100644 --- a/docs/releases/1.5.txt +++ b/docs/releases/1.5.txt @@ -168,6 +168,15 @@ number was inside the existing page range. It does check it now and raises an :exc:`InvalidPage` exception when the number is either too low or too high. +Behavior of autocommit database option on PostgreSQL changed +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +PostgreSQL's autocommit option didn't work as advertised previously. It did +work for single transaction block, but after the first block was left the +autocommit behavior was never restored. This bug is now fixed in 1.5. While +this is only a bug fix, it is worth checking your applications behavior if +you are using PostgreSQL together with the autocommit option. + Features deprecated in 1.5 ========================== diff --git a/tests/regressiontests/transactions_regress/tests.py b/tests/regressiontests/transactions_regress/tests.py index abd7a4ceaa..fb26138eed 100644 --- a/tests/regressiontests/transactions_regress/tests.py +++ b/tests/regressiontests/transactions_regress/tests.py @@ -1,11 +1,11 @@ from __future__ import absolute_import from django.core.exceptions import ImproperlyConfigured -from django.db import connection, transaction +from django.db import connection, connections, transaction, DEFAULT_DB_ALIAS from django.db.transaction import commit_on_success, commit_manually, TransactionManagementError from django.test import TransactionTestCase, skipUnlessDBFeature from django.test.utils import override_settings -from django.utils.unittest import skipIf +from django.utils.unittest import skipIf, skipUnless from .models import Mod, M2mA, M2mB @@ -175,6 +175,62 @@ class TestTransactionClosing(TransactionTestCase): self.test_failing_query_transaction_closed() +class TestPostgresAutocommit(TransactionTestCase): + """ + Tests to make sure psycopg2's autocommit mode is restored after entering + and leaving transaction management. Refs #16047. + """ + def setUp(self): + from psycopg2.extensions import (ISOLATION_LEVEL_AUTOCOMMIT, + ISOLATION_LEVEL_READ_COMMITTED) + self._autocommit = ISOLATION_LEVEL_AUTOCOMMIT + self._read_committed = ISOLATION_LEVEL_READ_COMMITTED + + # We want a clean backend with autocommit = True, so + # first we need to do a bit of work to have that. + self._old_backend = connections[DEFAULT_DB_ALIAS] + settings = self._old_backend.settings_dict.copy() + opts = settings['OPTIONS'].copy() + opts['autocommit'] = True + settings['OPTIONS'] = opts + new_backend = self._old_backend.__class__(settings, DEFAULT_DB_ALIAS) + connections[DEFAULT_DB_ALIAS] = new_backend + + def test_initial_autocommit_state(self): + self.assertTrue(connection.features.uses_autocommit) + self.assertEqual(connection.isolation_level, self._autocommit) + + def test_transaction_management(self): + transaction.enter_transaction_management() + transaction.managed(True) + self.assertEqual(connection.isolation_level, self._read_committed) + + transaction.leave_transaction_management() + self.assertEqual(connection.isolation_level, self._autocommit) + + def test_transaction_stacking(self): + transaction.enter_transaction_management() + transaction.managed(True) + self.assertEqual(connection.isolation_level, self._read_committed) + + transaction.enter_transaction_management() + self.assertEqual(connection.isolation_level, self._read_committed) + + transaction.leave_transaction_management() + self.assertEqual(connection.isolation_level, self._read_committed) + + transaction.leave_transaction_management() + self.assertEqual(connection.isolation_level, self._autocommit) + + def tearDown(self): + connections[DEFAULT_DB_ALIAS] = self._old_backend + +TestPostgresAutocommit = skipUnless(connection.vendor == 'postgresql', + "This test only valid for PostgreSQL")(TestPostgresAutocommit) +TestPostgresAutoCommit = skipUnlessDBFeature('supports_transactions')( + TestPostgresAutocommit) + + class TestManyToManyAddTransaction(TransactionTestCase): def test_manyrelated_add_commit(self): "Test for https://code.djangoproject.com/ticket/16818"