Ver código fonte

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

* perf: single query `db.set_value`

(cherry picked from commit cee2b50461)

* 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 bfa6a5fbdf)

Co-authored-by: Ankush Menat <ankush@frappe.io>
version-14
mergify[bot] 2 anos atrás
committed by GitHub
pai
commit
23caa1f93b
Nenhuma chave conhecida encontrada para esta assinatura no banco de dados ID da chave GPG: 4AEE18F83AFDEB23
3 arquivos alterados com 27 adições e 27 exclusões
  1. +4
    -13
      frappe/database/database.py
  2. +9
    -14
      frappe/tests/test_db.py
  3. +14
    -0
      frappe/tests/test_perf.py

+ 4
- 13
frappe/database/database.py Ver arquivo

@@ -27,7 +27,6 @@ from frappe.database.utils import (
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 @@ class Database:
: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 @@ class Database:
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


+ 9
- 14
frappe/tests/test_db.py Ver arquivo

@@ -734,7 +734,7 @@ class TestDBSetValue(FrappeTestCase):
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 @@ class TestDBSetValue(FrappeTestCase):
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`")


+ 14
- 0
frappe/tests/test_perf.py Ver arquivo

@@ -50,6 +50,20 @@ class TestPerformance(FrappeTestCase):
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")


Carregando…
Cancelar
Salvar