From 33b4ff0a84b6c67211a0db5ab2a562c83eaa93f2 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Fri, 17 Dec 2021 13:27:17 +0530 Subject: [PATCH 1/3] fix(db): fixed distinct in get_values refactor: remove redundant imports --- frappe/database/database.py | 47 ++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 0f325a746e..320b45e708 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -372,7 +372,7 @@ class Database(object): def get_values(self, doctype, filters=None, fieldname="name", ignore=None, as_dict=False, debug=False, order_by="KEEP_DEFAULT_ORDERING", update=None, cache=False, for_update=False, - run=True, pluck=False): + run=True, pluck=False, distinct=False): """Returns multiple document properties. :param doctype: DocType name. @@ -381,7 +381,8 @@ class Database(object): :param ignore: Don't raise exception if table, column is missing. :param as_dict: Return values as dict. :param debug: Print query in error log. - :param order_by: Column to order by + :param order_by: Column to order by, + :param distinct: Get Distinct results. Example: @@ -396,8 +397,20 @@ class Database(object): (doctype, filters, fieldname) in self.value_cache: return self.value_cache[(doctype, filters, fieldname)] + if distinct: + order_by = None + if isinstance(filters, list): - out = self._get_value_for_many_names(doctype, filters, fieldname, order_by, debug=debug, run=run, pluck=pluck) + out = self._get_value_for_many_names( + doctype, + filters, + fieldname, + order_by, + debug=debug, + run=run, + pluck=pluck, + distinct=distinct, + ) else: fields = fieldname @@ -422,6 +435,7 @@ class Database(object): for_update=for_update, run=run, pluck=pluck, + distinct=distinct ) except Exception as e: if ignore and (frappe.db.is_missing_column(e) or frappe.db.is_table_missing(e)): @@ -429,12 +443,12 @@ class Database(object): 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, run=run) + out = self.get_values_from_single(fields, filters, doctype, as_dict, debug, update, run=run, distinct=distinct) else: raise else: - out = self.get_values_from_single(fields, filters, doctype, as_dict, debug, update, run=run, pluck=pluck) + out = self.get_values_from_single(fields, filters, doctype, as_dict, debug, update, run=run, pluck=pluck, distinct=distinct) if cache and isinstance(filters, str): self.value_cache[(doctype, filters, fieldname)] = out @@ -451,6 +465,7 @@ class Database(object): update=None, run=True, pluck=False, + distinct=False, ): """Get values from `tabSingles` (Single DocTypes) (internal). @@ -477,16 +492,13 @@ class Database(object): return [map(values.get, fields)] else: - 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, - run=run, - pluck=pluck, - ) + r = self.query.get_sql( + "Singles", + filters={"field": ("in", tuple(fields)), "doctype": doctype}, + fields=["field", "value"], + distinct=distinct, + ).run(pluck=pluck, debug=debug, as_dict=False) + if not run: return r if as_dict: @@ -577,6 +589,7 @@ class Database(object): for_update=False, run=True, pluck=False, + distinct=False, ): field_objects = [] @@ -594,6 +607,7 @@ class Database(object): for_update=for_update, field_objects=field_objects, fields=fields, + distinct=distinct, ) if ( fields == "*" @@ -607,7 +621,7 @@ class Database(object): ) return r - def _get_value_for_many_names(self, doctype, names, field, order_by, debug=False, run=True, pluck=False): + def _get_value_for_many_names(self, doctype, names, field, order_by, debug=False, run=True, pluck=False, distinct=False): names = list(filter(None, names)) if names: return self.get_all( @@ -619,6 +633,7 @@ class Database(object): debug=debug, as_list=1, run=run, + distinct=distinct, ) else: return {} From e1360a3770a067553174aca76a669ddea4047c88 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Fri, 17 Dec 2021 13:35:29 +0530 Subject: [PATCH 2/3] test: Added tests for distinct in get values --- frappe/database/database.py | 16 ++++++++++++++-- frappe/tests/test_db.py | 17 +++++++++++++---- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 320b45e708..402a0bc7de 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -335,8 +335,20 @@ class Database(object): """Returns `get_value` with fieldname='*'""" 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="KEEP_DEFAULT_ORDERING", cache=False, for_update=False, run=True, pluck=False): + def get_value( + self, + doctype, + filters=None, + fieldname="name", + ignore=None, + as_dict=False, + debug=False, + order_by="KEEP_DEFAULT_ORDERING", + cache=False, + for_update=False, + run=True, + pluck=False, + ): """Returns a document property or list of properties. :param doctype: DocType name. diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 6501d753ff..8d10635ace 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -38,13 +38,13 @@ class TestDB(unittest.TestCase): "User", Field("name") == "Administrator", for_update=True, run=False ).lower(), ) - doctype = frappe.qb.DocType("User") + user_doctype = frappe.qb.DocType("User") self.assertEqual( - frappe.qb.from_(doctype).select(doctype.name, doctype.email).run(), + frappe.qb.from_(user_doctype).select(user_doctype.name, user_doctype.email).run(), frappe.db.get_values( - doctype, + user_doctype, filters={}, - fieldname=[doctype.name, doctype.email], + fieldname=[user_doctype.name, user_doctype.email], order_by=None, ), ) @@ -53,6 +53,15 @@ class TestDB(unittest.TestCase): self.assertEqual(frappe.db.sql("""SELECT name FROM `tabUser` WHERE name >= 't' ORDER BY MODIFIED DESC""")[0][0], frappe.db.get_value("User", {"name": [">=", "t"]})) + self.assertEqual( + frappe.db.get_values( + "User", filters={"name": "Administrator"}, distinct=True, fieldname="*" + ), + frappe.qb.from_(user_doctype) + .where(user_doctype.name == "Administrator") + .select("*") + .distinct(), + ) self.assertIn( "concat_ws", From 7faa843e976a704054a2d138a45b349aa60fa6a5 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Fri, 17 Dec 2021 14:31:28 +0530 Subject: [PATCH 3/3] feat: Added distinct in get_value --- frappe/database/database.py | 3 ++- frappe/tests/test_db.py | 10 +++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 402a0bc7de..e8c81c4bc1 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -348,6 +348,7 @@ class Database(object): for_update=False, run=True, pluck=False, + distinct=False, ): """Returns a document property or list of properties. @@ -375,7 +376,7 @@ class Database(object): """ ret = self.get_values(doctype, filters, fieldname, ignore, as_dict, debug, - order_by, cache=cache, for_update=for_update, run=run, pluck=pluck) + order_by, cache=cache, for_update=for_update, run=run, pluck=pluck, distinct=distinct) if not run: return ret diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 8d10635ace..5db56e5046 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -55,12 +55,16 @@ class TestDB(unittest.TestCase): frappe.db.get_value("User", {"name": [">=", "t"]})) self.assertEqual( frappe.db.get_values( - "User", filters={"name": "Administrator"}, distinct=True, fieldname="*" + "User", + filters={"name": "Administrator"}, + distinct=True, + fieldname="email", ), frappe.qb.from_(user_doctype) .where(user_doctype.name == "Administrator") - .select("*") - .distinct(), + .select("email") + .distinct() + .run(), ) self.assertIn(