* perf: single query `db.set_value` (cherry picked from commitversion-14cee2b50461
) * 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 commitbfa6a5fbdf
) Co-authored-by: Ankush Menat <ankush@frappe.io>
@@ -27,7 +27,6 @@ from frappe.database.utils import ( | |||||
from frappe.exceptions import DoesNotExistError, ImplicitCommitError | from frappe.exceptions import DoesNotExistError, ImplicitCommitError | ||||
from frappe.model.utils.link_count import flush_local_link_count | from frappe.model.utils.link_count import flush_local_link_count | ||||
from frappe.query_builder.functions import 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 cast as cast_fieldtype | ||||
from frappe.utils import get_datetime, get_table_name, getdate, now, sbool | from frappe.utils import get_datetime, get_table_name, getdate, now, sbool | ||||
@@ -857,7 +856,7 @@ class Database: | |||||
:param modified_by: Set this user as `modified_by`. | :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 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 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) | is_single_doctype = not (dn and dt != dn) | ||||
to_update = field if isinstance(field, dict) else {field: val} | to_update = field if isinstance(field, dict) else {field: val} | ||||
@@ -879,19 +878,11 @@ class Database: | |||||
frappe.clear_document_cache(dt, dt) | frappe.clear_document_cache(dt, dt) | ||||
else: | 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: | else: | ||||
query = frappe.qb.engine.build_conditions(table=dt, filters=dn, update=True) | |||||
# TODO: Fix this; doesn't work rn - gavin@frappe.io | # TODO: Fix this; doesn't work rn - gavin@frappe.io | ||||
# frappe.cache().hdel_keys(dt, "document_cache") | # frappe.cache().hdel_keys(dt, "document_cache") | ||||
# Workaround: clear all document caches | # Workaround: clear all document caches | ||||
@@ -734,7 +734,7 @@ class TestDBSetValue(FrappeTestCase): | |||||
frappe.db.get_value("ToDo", todo.name, ["modified", "modified_by"]), | frappe.db.get_value("ToDo", todo.name, ["modified", "modified_by"]), | ||||
) | ) | ||||
def test_for_update(self): | |||||
def test_set_value(self): | |||||
self.todo1.reload() | self.todo1.reload() | ||||
with patch.object(Database, "sql") as sql_called: | with patch.object(Database, "sql") as sql_called: | ||||
@@ -745,28 +745,23 @@ class TestDBSetValue(FrappeTestCase): | |||||
f"{self.todo1.description}-edit by `test_for_update`", | f"{self.todo1.description}-edit by `test_for_update`", | ||||
) | ) | ||||
first_query = sql_called.call_args_list[0].args[0] | 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": | if frappe.conf.db_type == "postgres": | ||||
from frappe.database.postgres.database import modify_query | 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": | 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): | def test_cleared_cache(self): | ||||
self.todo2.reload() | 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): | def test_update_alias(self): | ||||
args = (self.todo1.doctype, self.todo1.name, "description", "Updated by `test_update_alias`") | args = (self.todo1.doctype, self.todo1.name, "description", "Updated by `test_update_alias`") | ||||
@@ -50,6 +50,20 @@ class TestPerformance(FrappeTestCase): | |||||
with self.assertQueryCount(0): | with self.assertQueryCount(0): | ||||
frappe.get_meta("User") | 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): | def test_controller_caching(self): | ||||
get_controller("User") | get_controller("User") | ||||