summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSimon Charette <charette.s@gmail.com>2022-02-09 01:34:45 -0500
committerMariusz Felisiak <felisiak.mariusz@gmail.com>2022-02-09 11:14:50 +0100
commitd35ce682e31ea4a86c2079c60721fae171f03d7c (patch)
treeef28db03ec81c0d5835e0ff34d49099fbd7b80c9
parentd70b4bea18c96e518ce14dca96085e9265e8ebb6 (diff)
Fixed #33506 -- Made QuerySet.bulk_update() perform atomic writes against write database.
The lack of _for_write = True assignment in bulk_update prior to accessing self.db resulted in the db_for_read database being used to wrap batched UPDATEs in a transaction. Also tweaked the batch queryset creation to also ensure they are executed against the same database as the opened transaction under all circumstances. Refs #23646, #33501.
-rw-r--r--django/db/models/query.py4
-rw-r--r--tests/queries/test_bulk_update.py28
2 files changed, 30 insertions, 2 deletions
diff --git a/django/db/models/query.py b/django/db/models/query.py
index aa10176dc0..0cebcc70d6 100644
--- a/django/db/models/query.py
+++ b/django/db/models/query.py
@@ -725,6 +725,7 @@ class QuerySet:
)
# PK is used twice in the resulting update query, once in the filter
# and once in the WHEN. Each field will also have one CAST.
+ self._for_write = True
connection = connections[self.db]
max_batch_size = connection.ops.bulk_batch_size(["pk", "pk"] + fields, objs)
batch_size = min(batch_size, max_batch_size) if batch_size else max_batch_size
@@ -746,9 +747,10 @@ class QuerySet:
update_kwargs[field.attname] = case_statement
updates.append(([obj.pk for obj in batch_objs], update_kwargs))
rows_updated = 0
+ queryset = self.using(self.db)
with transaction.atomic(using=self.db, savepoint=False):
for pks, update_kwargs in updates:
- rows_updated += self.filter(pk__in=pks).update(**update_kwargs)
+ rows_updated += queryset.filter(pk__in=pks).update(**update_kwargs)
return rows_updated
bulk_update.alters_data = True
diff --git a/tests/queries/test_bulk_update.py b/tests/queries/test_bulk_update.py
index 389d6c1c41..bc252c21c6 100644
--- a/tests/queries/test_bulk_update.py
+++ b/tests/queries/test_bulk_update.py
@@ -3,13 +3,15 @@ import datetime
from django.core.exceptions import FieldDoesNotExist
from django.db.models import F
from django.db.models.functions import Lower
-from django.test import TestCase, skipUnlessDBFeature
+from django.db.utils import IntegrityError
+from django.test import TestCase, override_settings, skipUnlessDBFeature
from .models import (
Article,
CustomDbColumn,
CustomPk,
Detail,
+ Food,
Individual,
JSONFieldNullable,
Member,
@@ -25,6 +27,11 @@ from .models import (
)
+class WriteToOtherRouter:
+ def db_for_write(self, model, **hints):
+ return "other"
+
+
class BulkUpdateNoteTests(TestCase):
@classmethod
def setUpTestData(cls):
@@ -107,6 +114,8 @@ class BulkUpdateNoteTests(TestCase):
class BulkUpdateTests(TestCase):
+ databases = {"default", "other"}
+
def test_no_fields(self):
msg = "Field names must be given to bulk_update()."
with self.assertRaisesMessage(ValueError, msg):
@@ -302,3 +311,20 @@ class BulkUpdateTests(TestCase):
parent.refresh_from_db()
self.assertEqual(parent.f, 42)
self.assertIsNone(parent.single)
+
+ @override_settings(DATABASE_ROUTERS=[WriteToOtherRouter()])
+ def test_database_routing(self):
+ note = Note.objects.create(note="create")
+ note.note = "bulk_update"
+ with self.assertNumQueries(1, using="other"):
+ Note.objects.bulk_update([note], fields=["note"])
+
+ @override_settings(DATABASE_ROUTERS=[WriteToOtherRouter()])
+ def test_database_routing_batch_atomicity(self):
+ f1 = Food.objects.create(name="Banana")
+ f2 = Food.objects.create(name="Apple")
+ f1.name = "Kiwi"
+ f2.name = "Kiwi"
+ with self.assertRaises(IntegrityError):
+ Food.objects.bulk_update([f1, f2], fields=["name"], batch_size=1)
+ self.assertIs(Food.objects.filter(name="Kiwi").exists(), False)