From 4e9a74b81df1c7aaea2f90a3a4911920e134b275 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Wed, 19 Sep 2012 11:15:12 -0600 Subject: [PATCH] Revert "Fixed #16865 -- Made get_or_create use read database for initial get query." Thanks to Jeremy Dunck for pointing out the problem with this change. If in a single transaction, the master deletes a record and then get_or_creates a similar record, under the new behavior the get_or_create would find the record in the slave db and fail to re-create it, leaving the record nonexistent, which violates the contract of get_or_create that the record should always exist afterwards. We need to do everything against the master here in order to ensure correctness. This reverts commit 901af865505310a70dd02ea5b3becbf45819b652. --- django/db/models/query.py | 2 +- docs/releases/1.5.txt | 9 --------- tests/modeltests/get_or_create/tests.py | 24 ------------------------ 3 files changed, 1 insertion(+), 34 deletions(-) diff --git a/django/db/models/query.py b/django/db/models/query.py index 441426a107..8bf08b7a93 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -455,9 +455,9 @@ class QuerySet(object): if f.attname in lookup: lookup[f.name] = lookup.pop(f.attname) try: + self._for_write = True return self.get(**lookup), False except self.model.DoesNotExist: - self._for_write = True try: params = dict([(k, v) for k, v in kwargs.items() if '__' not in k]) params.update(defaults) diff --git a/docs/releases/1.5.txt b/docs/releases/1.5.txt index 84c37af9ee..26b6ad1bfa 100644 --- a/docs/releases/1.5.txt +++ b/docs/releases/1.5.txt @@ -338,15 +338,6 @@ Miscellaneous needs. The new default value is `0666` (octal) and the current umask value is first masked out. -* In a multi-database situation, ``get_or_create()`` will now use a read - database for the initial ``get`` attempt (previously, it used only the write - database for all queries). This change reduces load on the write (master) - database, in exchange for slightly more frequent false-negatives on the - initial ``get`` due to replication lag. In those cases the subsequent insert - will still go to the master and fail, after which the existing object will be - fetched from the master. - - Features deprecated in 1.5 ========================== diff --git a/tests/modeltests/get_or_create/tests.py b/tests/modeltests/get_or_create/tests.py index cc7d2c29ab..1e300fbb4d 100644 --- a/tests/modeltests/get_or_create/tests.py +++ b/tests/modeltests/get_or_create/tests.py @@ -64,27 +64,3 @@ class GetOrCreateTests(TestCase): formatted_traceback = traceback.format_exc() self.assertIn('obj.save', formatted_traceback) - - def test_initial_get_on_read_db(self): - """ - get_or_create should only set _for_write when it's actually doing a - create action. This makes sure that the initial .get() will be able to - use a slave database. Specially when some form of database pinning is - in place this will help to not put all the SELECT queries on the - master. Refs #16865. - - """ - qs = Person.objects.get_query_set() - p, created = qs.get_or_create( - first_name="Stuart", last_name="Sutcliffe", defaults={ - "birthday": date(1940, 6, 23), - } - ) - self.assertTrue(created) - self.assertTrue(qs._for_write) - - qs = Person.objects.get_query_set() - p, created = qs.get_or_create( - first_name="Stuart", last_name="Sutcliffe") - self.assertFalse(created) - self.assertFalse(qs._for_write)