diff options
| -rw-r--r-- | django/db/models/sql/compiler.py | 4 | ||||
| -rw-r--r-- | docs/ref/models/querysets.txt | 10 | ||||
| -rw-r--r-- | docs/releases/1.6.3.txt | 28 | ||||
| -rw-r--r-- | docs/topics/db/transactions.txt | 5 | ||||
| -rw-r--r-- | tests/select_for_update/tests.py | 28 |
5 files changed, 73 insertions, 2 deletions
diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index 0ab53ecc71..ea7f9f4925 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -10,6 +10,7 @@ from django.db.models.sql.constants import (SINGLE, MULTI, ORDER_DIR, from django.db.models.sql.datastructures import EmptyResultSet from django.db.models.sql.expressions import SQLEvaluator from django.db.models.sql.query import get_order_dir, Query +from django.db.transaction import TransactionManagementError from django.db.utils import DatabaseError from django.utils import six from django.utils.six.moves import zip @@ -142,6 +143,9 @@ class SQLCompiler(object): result.append('OFFSET %d' % self.query.low_mark) if self.query.select_for_update and self.connection.features.has_select_for_update: + if self.connection.get_autocommit(): + raise TransactionManagementError("select_for_update cannot be used outside of a transaction.") + # If we've been asked for a NOWAIT query but the backend does not support it, # raise a DatabaseError otherwise we could get an unexpected deadlock. nowait = self.query.select_for_update_nowait diff --git a/docs/ref/models/querysets.txt b/docs/ref/models/querysets.txt index 69e15dc0e9..14e2cfac18 100644 --- a/docs/ref/models/querysets.txt +++ b/docs/ref/models/querysets.txt @@ -1314,9 +1314,19 @@ do not support ``nowait``, such as MySQL, will cause a :exc:`~django.db.DatabaseError` to be raised. This is in order to prevent code unexpectedly blocking. +Executing a queryset with ``select_for_update`` in autocommit mode is +an error because the rows are then not locked. If allowed, this would +facilitate data corruption, and could easily be caused by calling, +outside of any transaction, code that expects to be run in one. + Using ``select_for_update`` on backends which do not support ``SELECT ... FOR UPDATE`` (such as SQLite) will have no effect. +.. versionchanged:: 1.6.3 + + It is now an error to execute a query with ``select_for_update()`` in + autocommit mode. With earlier releases in the 1.6 series it was a no-op. + Methods that do not return QuerySets ------------------------------------ diff --git a/docs/releases/1.6.3.txt b/docs/releases/1.6.3.txt index ca9069d250..c6981fa96b 100644 --- a/docs/releases/1.6.3.txt +++ b/docs/releases/1.6.3.txt @@ -5,7 +5,33 @@ Django 1.6.3 release notes *Under development* This is Django 1.6.3, a bugfix release for Django 1.6. Django 1.6.3 fixes -several bugs in 1.6.2: +several bugs in 1.6.2 and makes one backwards-incompatible change: + +``select_for_update()`` requires a transaction +============================================== + +Historically, queries that use +:meth:`~django.db.models.query.QuerySet.select_for_update()` could be +executed in autocommit mode, outside of a transaction. Before Django +1.6, Django's automatic transactions mode allowed this to be used to +lock records until the next write operation. Django 1.6 introduced +database-level autocommit; since then, execution in such a context +voids the effect of ``select_for_update()``. It is, therefore, assumed +now to be an error, and raises an exception. + +This change may cause test failures if you use ``select_for_update()`` +in a test class which is a subclass of +:class:`~django.test.TransactionTestCase` rather than +:class:`~django.test.TestCase`. + +This change was made because such errors can be caused by including an +app which expects global transactions (e.g. :setting:`ATOMIC_REQUESTS +<DATABASE-ATOMIC_REQUESTS>` set to True), or Django's old autocommit +behavior, in a project which runs without them; and further, such +errors may manifest as data-corruption bugs. + +Other bugfixes and changes +========================== * Content retrieved from the GeoIP library is now properly decoded from its default ``iso-8859-1`` encoding diff --git a/docs/topics/db/transactions.txt b/docs/topics/db/transactions.txt index 427bef32f2..13cfbcf1bb 100644 --- a/docs/topics/db/transactions.txt +++ b/docs/topics/db/transactions.txt @@ -722,7 +722,10 @@ Select for update If you were relying on "automatic transactions" to provide locking between :meth:`~django.db.models.query.QuerySet.select_for_update` and a subsequent write operation — an extremely fragile design, but nonetheless possible — you -must wrap the relevant code in :func:`atomic`. +must wrap the relevant code in :func:`atomic`. Since Django 1.6.3, executing +a query with :meth:`~django.db.models.query.QuerySet.select_for_update` in +autocommit mode will raise a +:exc:`~django.db.transaction.TransactionManagementError`. Using a high isolation level ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/select_for_update/tests.py b/tests/select_for_update/tests.py index edef455154..687d7d5f5d 100644 --- a/tests/select_for_update/tests.py +++ b/tests/select_for_update/tests.py @@ -151,6 +151,34 @@ class SelectForUpdateTests(TransactionTestCase): Person.objects.all().select_for_update(nowait=True) ) + @skipUnlessDBFeature('has_select_for_update') + def test_for_update_requires_transaction(self): + """ + Test that a TransactionManagementError is raised + when a select_for_update query is executed outside of a transaction. + """ + transaction.set_autocommit(True) + try: + with self.assertRaises(transaction.TransactionManagementError): + list(Person.objects.all().select_for_update()) + finally: + transaction.set_autocommit(True) + + @skipUnlessDBFeature('has_select_for_update') + def test_for_update_requires_transaction_only_in_execution(self): + """ + Test that no TransactionManagementError is raised + when select_for_update is invoked outside of a transaction - + only when the query is executed. + """ + transaction.set_autocommit(True) + try: + people = Person.objects.all().select_for_update() + with self.assertRaises(transaction.TransactionManagementError): + list(people) + finally: + transaction.set_autocommit(True) + def run_select_for_update(self, status, nowait=False): """ Utility method that runs a SELECT FOR UPDATE against all |