perf: Perform db.set_value with single query only (backport #18305) (…

…#18349)

* perf: single query `db.set_value`

(cherry picked from commit cee2b50)

* fix: better cache validation

- Only delete a single doc if we know which doc changed
- Drop all docs other wise (kinda bad, but this isn't used frequently,
  will fix when visiting entire caching system again)

(cherry picked from commit bfa6a5f)

Co-authored-by: Ankush Menat <ankush@frappe.io>

frappe/database/database.py

@@ -27,7 +27,6 @@
 from frappe.exceptions import DoesNotExistError, ImplicitCommitError
 from frappe.model.utils.link_count import flush_local_link_count
 from frappe.query_builder.functions import Count
-from frappe.query_builder.utils import DocType
 from frappe.utils import cast as cast_fieldtype
 from frappe.utils import get_datetime, get_table_name, getdate, now, sbool
 
@@ -857,7 +856,7 @@ def set_value(
 		:param modified_by: Set this user as `modified_by`.
 		:param update_modified: default True. Set as false, if you don't want to update the timestamp.
 		:param debug: Print the query in the developer / js console.
-		:param for_update: Will add a row-level lock to the value that is being set so that it can be released on commit.
+		:param for_update: [DEPRECATED] This function now performs updates in single query, locking is not required.
 		"""
 		is_single_doctype = not (dn and dt != dn)
 		to_update = field if isinstance(field, dict) else {field: val}
@@ -879,19 +878,11 @@ def set_value(
 			frappe.clear_document_cache(dt, dt)
 
 		else:
-			table = DocType(dt)
-
-			if for_update:
-				docnames = tuple(
-					self.get_values(dt, dn, "name", debug=debug, for_update=for_update, pluck=True)
-				) or (NullValue(),)
-				query = frappe.qb.update(table).where(table.name.isin(docnames))
-
-				for docname in docnames:
-					frappe.clear_document_cache(dt, docname)
+			query = frappe.qb.engine.build_conditions(table=dt, filters=dn, update=True)
 
+			if isinstance(dn, str):
+				frappe.clear_document_cache(dt, dn)
 			else:
-				query = frappe.qb.engine.build_conditions(table=dt, filters=dn, update=True)
 				# TODO: Fix this; doesn't work rn - gavin@frappe.io
 				# frappe.cache().hdel_keys(dt, "document_cache")
 				# Workaround: clear all document caches

frappe/tests/test_db.py

@@ -734,7 +734,7 @@ def test_update_modified_options(self):
 			frappe.db.get_value("ToDo", todo.name, ["modified", "modified_by"]),
 		)
 
-	def test_for_update(self):
+	def test_set_value(self):
 		self.todo1.reload()
 
 		with patch.object(Database, "sql") as sql_called:
@@ -745,28 +745,23 @@ def test_for_update(self):
 				f"{self.todo1.description}-edit by `test_for_update`",
 			)
 			first_query = sql_called.call_args_list[0].args[0]
-			second_query = sql_called.call_args_list[1].args[0]
 
-			self.assertTrue(sql_called.call_count == 2)
-			self.assertTrue("FOR UPDATE" in first_query)
 			if frappe.conf.db_type == "postgres":
 				from frappe.database.postgres.database import modify_query
 
-				self.assertTrue(modify_query("UPDATE `tabToDo` SET") in second_query)
+				self.assertTrue(modify_query("UPDATE `tabToDo` SET") in first_query)
 			if frappe.conf.db_type == "mariadb":
-				self.assertTrue("UPDATE `tabToDo` SET" in second_query)
+				self.assertTrue("UPDATE `tabToDo` SET" in first_query)
 
 	def test_cleared_cache(self):
 		self.todo2.reload()
+		frappe.get_cached_doc(self.todo2.doctype, self.todo2.name)  # init cache
 
-		with patch.object(frappe, "clear_document_cache") as clear_cache:
-			frappe.db.set_value(
-				self.todo2.doctype,
-				self.todo2.name,
-				"description",
-				f"{self.todo2.description}-edit by `test_cleared_cache`",
-			)
-			clear_cache.assert_called()
+		description = f"{self.todo2.description}-edit by `test_cleared_cache`"
+
+		frappe.db.set_value(self.todo2.doctype, self.todo2.name, "description", description)
+		cached_doc = frappe.get_cached_doc(self.todo2.doctype, self.todo2.name)
+		self.assertEqual(cached_doc.description, description)
 
 	def test_update_alias(self):
 		args = (self.todo1.doctype, self.todo1.name, "description", "Updated by `test_update_alias`")

frappe/tests/test_perf.py

@@ -50,6 +50,20 @@ def test_meta_caching(self):
 		with self.assertQueryCount(0):
 			frappe.get_meta("User")
 
+	def test_set_value_query_count(self):
+		frappe.db.set_value("User", "Administrator", "interest", "Nothing")
+
+		with self.assertQueryCount(1):
+			frappe.db.set_value("User", "Administrator", "interest", "Nothing")
+
+		with self.assertQueryCount(1):
+			frappe.db.set_value("User", {"user_type": "System User"}, "interest", "Nothing")
+
+		with self.assertQueryCount(1):
+			frappe.db.set_value(
+				"User", {"user_type": "System User"}, {"interest": "Nothing", "bio": "boring person"}
+			)
+
 	def test_controller_caching(self):
 
 		get_controller("User")