From 23c3079433c35b23a520b2648f0f02dc0f75afed Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Fri, 22 Oct 2021 23:43:47 +0530 Subject: [PATCH 1/6] fix: set_value now takes for_update for pypika objects --- frappe/database/database.py | 2 +- frappe/database/query.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index a7efeb4b20..3ea52556b7 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -599,7 +599,7 @@ class Database(object): for key in to_update: set_values.append('`{0}`=%({0})s'.format(key)) - for name in self.get_values(dt, dn, 'name', for_update=for_update): + for name in self.get_values(dt, dn, 'name', for_update=for_update, debug=debug): values = dict(name=name[0]) values.update(to_update) diff --git a/frappe/database/query.py b/frappe/database/query.py index 7d7de85646..3545efb412 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -156,7 +156,7 @@ class Query: Returns: frappe.qb: condition object """ - condition = self.get_condition(table, **kwargs) + condition = self.add_conditions(self.get_condition(table, **kwargs), **kwargs) return condition.where(criterion) def add_conditions(self, conditions: frappe.qb, **kwargs): From 94f2a6e275efeae70886ca6a61411663fe04570c Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Sat, 23 Oct 2021 15:08:02 +0530 Subject: [PATCH 2/6] feat: Added run kwarg to get_all, get_values --- frappe/database/database.py | 50 +++++++++++++++++++++++++------------ frappe/model/db_query.py | 13 +++------- 2 files changed, 38 insertions(+), 25 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 3ea52556b7..82f8543d48 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -328,7 +328,7 @@ class Database(object): return self.get_value(doctype, filters, "*", as_dict=as_dict, cache=cache) def get_value(self, doctype, filters=None, fieldname="name", ignore=None, as_dict=False, - debug=False, order_by=None, cache=False, for_update=False): + debug=False, order_by=None, cache=False, for_update=False, run=True): """Returns a document property or list of properties. :param doctype: DocType name. @@ -355,12 +355,15 @@ class Database(object): """ ret = self.get_values(doctype, filters, fieldname, ignore, as_dict, debug, - order_by, cache=cache, for_update=for_update) + order_by, cache=cache, for_update=for_update, run=run) + + if not run: + return ret return ((len(ret[0]) > 1 or as_dict) and ret[0] or ret[0][0]) if ret else None def get_values(self, doctype, filters=None, fieldname="name", ignore=None, as_dict=False, - debug=False, order_by=None, update=None, cache=False, for_update=False): + debug=False, order_by=None, update=None, cache=False, for_update=False, run=True): """Returns multiple document properties. :param doctype: DocType name. @@ -386,7 +389,7 @@ class Database(object): if isinstance(filters, list): order_by = order_by or "modified_desc" - out = self._get_value_for_many_names(doctype, filters, fieldname, debug=debug) + out = self._get_value_for_many_names(doctype, filters, fieldname, debug=debug, run=run) else: fields = fieldname @@ -399,26 +402,28 @@ class Database(object): if (filters is not None) and (filters!=doctype or doctype=="DocType"): try: order_by = order_by or "modified" - out = self._get_values_from_table(fields, filters, doctype, as_dict, debug, order_by, update, for_update=for_update) + out = self._get_values_from_table( + fields, filters, doctype, as_dict, debug, order_by, update, for_update=for_update, run=run + ) except Exception as e: if ignore and (frappe.db.is_missing_column(e) or frappe.db.is_table_missing(e)): # table or column not found, return None out = None elif (not ignore) and frappe.db.is_table_missing(e): # table not found, look in singles - out = self.get_values_from_single(fields, filters, doctype, as_dict, debug, update) + out = self.get_values_from_single(fields, filters, doctype, as_dict, debug, update, run=run) else: raise else: - out = self.get_values_from_single(fields, filters, doctype, as_dict, debug, update) + out = self.get_values_from_single(fields, filters, doctype, as_dict, debug, update, run=run) if cache and isinstance(filters, str): self.value_cache[(doctype, filters, fieldname)] = out return out - def get_values_from_single(self, fields, filters, doctype, as_dict=False, debug=False, update=None): + def get_values_from_single(self, fields, filters, doctype, as_dict=False, debug=False, update=None, run=True): """Get values from `tabSingles` (Single DocTypes) (internal). :param fields: List of fields, @@ -447,8 +452,9 @@ class Database(object): r = self.sql("""select field, value from `tabSingles` where field in (%s) and doctype=%s""" % (', '.join(['%s'] * len(fields)), '%s'), - tuple(fields) + (doctype,), as_dict=False, debug=debug) - + tuple(fields) + (doctype,), as_dict=False, debug=debug, run=run) + if not run: + return r if as_dict: if r: r = frappe._dict(r) @@ -526,7 +532,18 @@ class Database(object): """Alias for get_single_value""" return self.get_single_value(*args, **kwargs) - def _get_values_from_table(self, fields, filters, doctype, as_dict, debug, order_by=None, update=None, for_update=False): + def _get_values_from_table( + self, + fields, + filters, + doctype, + as_dict, + debug, + order_by=None, + update=None, + for_update=False, + run=True, + ): field_objects = [] for field in fields: @@ -535,7 +552,9 @@ class Database(object): else: field_objects.append(field) - criterion = self.query.build_conditions(table=doctype, filters=filters, orderby=order_by, for_update=for_update) + criterion = self.query.build_conditions( + table=doctype, filters=filters, orderby=order_by, for_update=for_update + ) if isinstance(fields, (list, tuple)): query = criterion.select(*field_objects) @@ -543,18 +562,17 @@ class Database(object): if fields=="*": query = criterion.select(fields) as_dict = True - r = self.sql(query, as_dict=as_dict, debug=debug, update=update) - + r = self.sql(query, as_dict=as_dict, debug=debug, update=update, run=run) return r - def _get_value_for_many_names(self, doctype, names, field, debug=False): + def _get_value_for_many_names(self, doctype, names, field, debug=False, run=True): names = list(filter(None, names)) if names: return self.get_all(doctype, fields=['name', field], filters=[['name', 'in', names]], - debug=debug, as_list=1) + debug=debug, as_list=1, run=run) else: return {} diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 8f0e0aaefc..44f1398cc7 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -35,7 +35,7 @@ class DatabaseQuery(object): join='left join', distinct=False, start=None, page_length=None, limit=None, ignore_ifnull=False, save_user_settings=False, save_user_settings_fields=False, update=None, add_total_row=None, user_settings=None, reference_doctype=None, - return_query=False, strict=True, pluck=None, ignore_ddl=False) -> List: + run=True, strict=True, pluck=None, ignore_ddl=False) -> List: if not ignore_permissions and \ not frappe.has_permission(self.doctype, "select", user=user) and \ not frappe.has_permission(self.doctype, "read", user=user): @@ -87,7 +87,7 @@ class DatabaseQuery(object): self.user = user or frappe.session.user self.update = update self.user_settings_fields = copy.deepcopy(self.fields) - self.return_query = return_query + self.run = run self.strict = strict self.ignore_ddl = ignore_ddl @@ -104,8 +104,6 @@ class DatabaseQuery(object): if not self.columns: return [] result = self.build_and_run() - if return_query: - return result if with_comment_count and not as_list and self.doctype: self.add_comment_count(result) @@ -137,11 +135,8 @@ class DatabaseQuery(object): %(order_by)s %(limit)s""" % args - if self.return_query: - return query - else: - return frappe.db.sql(query, as_dict=not self.as_list, debug=self.debug, - update=self.update, ignore_ddl=self.ignore_ddl) + return frappe.db.sql(query, as_dict=not self.as_list, debug=self.debug, + update=self.update, ignore_ddl=self.ignore_ddl, run=self.run) def prepare_args(self): self.parse_args() From d6b978116391843aa7bba4ccd79aa4debea22629 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Sat, 23 Oct 2021 15:08:28 +0530 Subject: [PATCH 3/6] feat: Added test to assert lock for pypika objects --- frappe/tests/test_db.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 0d32a72756..978e3f4f7f 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -11,6 +11,7 @@ import frappe from frappe.custom.doctype.custom_field.custom_field import create_custom_field from frappe.utils import random_string from frappe.utils.testutils import clear_custom_fields +from frappe.query_builder import Field from .test_query_builder import run_only_if, db_type_is @@ -24,6 +25,7 @@ class TestDB(unittest.TestCase): self.assertEqual(frappe.db.get_value("User", {"name": ["<=", "Administrator"]}), "Administrator") self.assertEqual(frappe.db.get_value("User", {}, ["Max(name)"]), frappe.db.sql("SELECT Max(name) FROM tabUser")[0][0]) self.assertEqual(frappe.db.get_value("User", {}, "Min(name)"), frappe.db.sql("SELECT Min(name) FROM tabUser")[0][0]) + self.assertIn("for update", frappe.db.get_value("User", Field("name") == "Administrator", for_update=True, run=False).lower()) self.assertEqual(frappe.db.sql("""SELECT name FROM `tabUser` WHERE name > 's' ORDER BY MODIFIED DESC""")[0][0], frappe.db.get_value("User", {"name": [">", "s"]})) From 33775b53734793babc752415d398845fcc0891c3 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Sat, 23 Oct 2021 15:17:40 +0530 Subject: [PATCH 4/6] style: reverting erroneous formatting --- frappe/database/database.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 82f8543d48..b7048af7d3 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -532,18 +532,8 @@ class Database(object): """Alias for get_single_value""" return self.get_single_value(*args, **kwargs) - def _get_values_from_table( - self, - fields, - filters, - doctype, - as_dict, - debug, - order_by=None, - update=None, - for_update=False, - run=True, - ): + def _get_values_from_table(self, fields, filters, doctype, as_dict, debug, order_by=None, + update=None, for_update=False, run=True): field_objects = [] for field in fields: From 672ae5cd87457a9dfb4485177943b50bcf3dbe65 Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Sat, 23 Oct 2021 15:36:32 +0530 Subject: [PATCH 5/6] fix: replaced return_query with run --- frappe/core/doctype/server_script/test_server_script.py | 2 +- frappe/desk/listview.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/server_script/test_server_script.py b/frappe/core/doctype/server_script/test_server_script.py index de858327a9..100e3c2790 100644 --- a/frappe/core/doctype/server_script/test_server_script.py +++ b/frappe/core/doctype/server_script/test_server_script.py @@ -122,7 +122,7 @@ class TestServerScript(unittest.TestCase): self.assertEqual(frappe.get_doc('Server Script', 'test_return_value').execute_method(), 'hello') def test_permission_query(self): - self.assertTrue('where (1 = 1)' in frappe.db.get_list('ToDo', return_query=1)) + self.assertTrue('where (1 = 1)' in frappe.db.get_list('ToDo', run=False)) self.assertTrue(isinstance(frappe.db.get_list('ToDo'), list)) def test_attribute_error(self): diff --git a/frappe/desk/listview.py b/frappe/desk/listview.py index e733adf868..43ad104f0d 100644 --- a/frappe/desk/listview.py +++ b/frappe/desk/listview.py @@ -26,7 +26,7 @@ def get_group_by_count(doctype, current_filters, field): current_filters = frappe.parse_json(current_filters) subquery_condition = '' - subquery = frappe.get_all(doctype, filters=current_filters, return_query = True) + subquery = frappe.get_all(doctype, filters=current_filters, run=False) if field == 'assigned_to': subquery_condition = ' and `tabToDo`.reference_name in ({subquery})'.format(subquery = subquery) return frappe.db.sql("""select `tabToDo`.owner as name, count(*) as count From ca1d9b4514400b556e367a0f7e75f80053b6c69c Mon Sep 17 00:00:00 2001 From: Aradhya-Tripathi Date: Sat, 23 Oct 2021 16:03:29 +0530 Subject: [PATCH 6/6] fix: fixed sider issues --- frappe/database/database.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index b7048af7d3..a29b6893ad 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -533,7 +533,7 @@ class Database(object): return self.get_single_value(*args, **kwargs) def _get_values_from_table(self, fields, filters, doctype, as_dict, debug, order_by=None, - update=None, for_update=False, run=True): + update=None, for_update=False, run=True): field_objects = [] for field in fields: