From c9df86f9efc52fb56ba81ad01d123fa7a4b63180 Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Thu, 7 Apr 2022 20:25:12 +0530 Subject: [PATCH 1/7] feat: multi table queries --- frappe/database/database.py | 4 ++-- frappe/database/query.py | 30 +++++++++++++++++++++++------- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index fb631951fa..df476c20cc 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -1019,13 +1019,13 @@ class Database(object): return self.get_value(dt, dn, ignore=True, cache=cache) - def count(self, dt, filters=None, debug=False, cache=False): + def count(self, dt, filters=None, debug=False, cache=False, distinct:bool = True): """Returns `COUNT(*)` for given DocType and filters.""" if cache and not filters: cache_count = frappe.cache().get_value("doctype:count:{}".format(dt)) if cache_count is not None: return cache_count - query = self.query.get_sql(table=dt, filters=filters, fields=Count("*")) + query = self.query.get_sql(table=dt, filters=filters, fields=Count("*"), distinct=distinct) if filters: count = self.sql(query, debug=debug)[0][0] return count diff --git a/frappe/database/query.py b/frappe/database/query.py index 136f5c86b6..7bb3597574 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -4,7 +4,7 @@ from typing import Any, Dict, List, Tuple, Union import frappe from frappe import _ -from frappe.query_builder import Criterion, Field, Order +from frappe.query_builder import Criterion, Field, Order, Table def like(key: str, value: str) -> frappe.qb: @@ -139,7 +139,9 @@ OPERATOR_MAP = { class Query: - def get_condition(self, table: str, **kwargs) -> frappe.qb: + tables:dict = {} + + def get_condition(self, table: Union[str, Table], **kwargs) -> frappe.qb: """Get initial table object Args: @@ -148,11 +150,20 @@ class Query: Returns: frappe.qb: DocType with initial condition """ + table_object = self.get_table(table) if kwargs.get("update"): - return frappe.qb.update(table) + return frappe.qb.update(table_object) if kwargs.get("into"): - return frappe.qb.into(table) - return frappe.qb.from_(table) + return frappe.qb.into(table_object) + return frappe.qb.from_(table_object) + + def get_table(self, table_name: Union[str, Table])->Table: + if isinstance(table_name, Table): + return table_name + table_name = table_name.strip('"').strip("'") + if table_name not in self.tables: + self.tables[table_name] = frappe.qb.DocType(table_name) + return self.tables[table_name] def criterion_query(self, table: str, criterion: Criterion, **kwargs) -> frappe.qb: """Generate filters from Criterion objects @@ -217,8 +228,13 @@ class Query: conditions = conditions.where(_operator(Field(filters[0]), filters[2])) break else: - _operator = OPERATOR_MAP[f[1]] - conditions = conditions.where(_operator(Field(f[0]), f[2])) + _operator = OPERATOR_MAP[f[-2]] + if len(f) == 4: + table_object = self.get_table(f[0]) + _field = table_object[f[1]] + else: + _field = Field(f[0]) + conditions = conditions.where(_operator(_field, f[-1])) return self.add_conditions(conditions, **kwargs) From d39e6a78901f09dfefd8957a81fcd5a2dc974754 Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Wed, 13 Apr 2022 01:28:59 +0530 Subject: [PATCH 2/7] refactor: new query engine in `get_count()` --- frappe/database/database.py | 14 +++++--------- frappe/database/query.py | 26 +++++++++++++------------- frappe/desk/reportview.py | 7 ++----- 3 files changed, 20 insertions(+), 27 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index df476c20cc..eca1656424 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -1019,21 +1019,17 @@ class Database(object): return self.get_value(dt, dn, ignore=True, cache=cache) - def count(self, dt, filters=None, debug=False, cache=False, distinct:bool = True): + def count(self, dt, filters=None, debug=False, cache=False, distinct : bool = True): """Returns `COUNT(*)` for given DocType and filters.""" if cache and not filters: cache_count = frappe.cache().get_value("doctype:count:{}".format(dt)) if cache_count is not None: return cache_count query = self.query.get_sql(table=dt, filters=filters, fields=Count("*"), distinct=distinct) - if filters: - count = self.sql(query, debug=debug)[0][0] - return count - else: - count = self.sql(query, debug=debug)[0][0] - if cache: - frappe.cache().set_value("doctype:count:{}".format(dt), count, expires_in_sec=86400) - return count + count = query.run(debug=debug)[0][0] + if not filters and cache: + frappe.cache().set_value("doctype:count:{}".format(dt), count, expires_in_sec=86400) + return count @staticmethod def format_date(date): diff --git a/frappe/database/query.py b/frappe/database/query.py index 7bb3597574..44c341b6a2 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -7,7 +7,7 @@ from frappe import _ from frappe.query_builder import Criterion, Field, Order, Table -def like(key: str, value: str) -> frappe.qb: +def like(key: Field, value: str) -> frappe.qb: """Wrapper method for `LIKE` Args: @@ -17,10 +17,10 @@ def like(key: str, value: str) -> frappe.qb: Returns: frappe.qb: `frappe.qb object with `LIKE` """ - return Field(key).like(value) + return key.like(value) -def func_in(key: str, value: Union[List, Tuple]) -> frappe.qb: +def func_in(key: Field, value: Union[List, Tuple]) -> frappe.qb: """Wrapper method for `IN` Args: @@ -30,10 +30,10 @@ def func_in(key: str, value: Union[List, Tuple]) -> frappe.qb: Returns: frappe.qb: `frappe.qb object with `IN` """ - return Field(key).isin(value) + return key.isin(value) -def not_like(key: str, value: str) -> frappe.qb: +def not_like(key: Field, value: str) -> frappe.qb: """Wrapper method for `NOT LIKE` Args: @@ -43,10 +43,10 @@ def not_like(key: str, value: str) -> frappe.qb: Returns: frappe.qb: `frappe.qb object with `NOT LIKE` """ - return Field(key).not_like(value) + return key.not_like(value) -def func_not_in(key: str, value: Union[List, Tuple]): +def func_not_in(key: Field, value: Union[List, Tuple]): """Wrapper method for `NOT IN` Args: @@ -56,10 +56,10 @@ def func_not_in(key: str, value: Union[List, Tuple]): Returns: frappe.qb: `frappe.qb object with `NOT IN` """ - return Field(key).notin(value) + return key.notin(value) -def func_regex(key: str, value: str) -> frappe.qb: +def func_regex(key: Field, value: str) -> frappe.qb: """Wrapper method for `REGEX` Args: @@ -69,10 +69,10 @@ def func_regex(key: str, value: str) -> frappe.qb: Returns: frappe.qb: `frappe.qb object with `REGEX` """ - return Field(key).regex(value) + return key.regex(value) -def func_between(key: str, value: Union[List, Tuple]) -> frappe.qb: +def func_between(key: Field, value: Union[List, Tuple]) -> frappe.qb: """Wrapper method for `BETWEEN` Args: @@ -82,7 +82,7 @@ def func_between(key: str, value: Union[List, Tuple]) -> frappe.qb: Returns: frappe.qb: `frappe.qb object with `BETWEEN` """ - return Field(key)[slice(*value)] + return key[slice(*value)] def make_function(key: Any, value: Union[int, str]): @@ -265,7 +265,7 @@ class Query: if isinstance(value, (list, tuple)): if isinstance(value[1], (list, tuple)) or value[0] in list(OPERATOR_MAP.keys())[-4:]: _operator = OPERATOR_MAP[value[0]] - conditions = conditions.where(_operator(key, value[1])) + conditions = conditions.where(_operator(Field(key), value[1])) else: _operator = OPERATOR_MAP[value[0]] conditions = conditions.where(_operator(Field(key), value[1])) diff --git a/frappe/desk/reportview.py b/frappe/desk/reportview.py index b45f80f6ff..2813061347 100644 --- a/frappe/desk/reportview.py +++ b/frappe/desk/reportview.py @@ -48,15 +48,12 @@ def get_list(): @frappe.read_only() def get_count(): args = get_form_params() - if is_virtual_doctype(args.doctype): controller = get_controller(args.doctype) data = controller(args.doctype).get_count(args) else: - distinct = "distinct " if args.distinct == "true" else "" - args.fields = [f"count({distinct}`tab{args.doctype}`.name) as total_count"] - data = execute(**args)[0].get("total_count") - + distinct = args["distinct"] == "true" + data = frappe.db.count(args["doctype"], args["filters"], distinct=distinct) return data From 3498445733db3fae7216e1743a89892b3548144c Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Wed, 13 Apr 2022 16:06:43 +0530 Subject: [PATCH 3/7] test: multiple_tables_in_filters --- frappe/database/query.py | 6 +++--- frappe/tests/test_query.py | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) create mode 100644 frappe/tests/test_query.py diff --git a/frappe/database/query.py b/frappe/database/query.py index 44c341b6a2..65ac375148 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -139,7 +139,7 @@ OPERATOR_MAP = { class Query: - tables:dict = {} + tables: dict = {} def get_condition(self, table: Union[str, Table], **kwargs) -> frappe.qb: """Get initial table object @@ -157,7 +157,7 @@ class Query: return frappe.qb.into(table_object) return frappe.qb.from_(table_object) - def get_table(self, table_name: Union[str, Table])->Table: + def get_table(self, table_name: Union[str, Table]) -> Table: if isinstance(table_name, Table): return table_name table_name = table_name.strip('"').strip("'") @@ -309,7 +309,7 @@ class Query: self, table: str, fields: Union[List, Tuple], - filters: Union[Dict[str, Union[str, int]], str, int] = None, + filters: Union[Dict[str, Union[str, int]], str, int, List[Union[List, str, int]]] = None, **kwargs, ): criterion = self.build_conditions(table, filters, **kwargs) diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py new file mode 100644 index 0000000000..fc03a66166 --- /dev/null +++ b/frappe/tests/test_query.py @@ -0,0 +1,19 @@ +import unittest +import frappe + +from frappe.tests.test_query_builder import db_type_is, run_only_if + +@run_only_if(db_type_is.MARIADB) +class TestQuery(unittest.TestCase): + def test_multiple_tables_in_filters(self): + self.assertEqual( + frappe.db.query.get_sql( + "DocType", + ["*"], + [ + ["BOM Update Log", "name", "like", "f%"], + ["DocType", "parent", "=", "something"], + ], + ).get_sql(), + "SELECT * FROM `tabDocType` WHERE `tabBOM Update Log`.`name` LIKE 'f%' AND `tabDocType`.`parent`='something'", + ) \ No newline at end of file From 0450a25822af2e18a9a17702c456e518b0a6d72c Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Tue, 19 Apr 2022 12:15:37 +0530 Subject: [PATCH 4/7] test: get_count --- frappe/tests/test_db.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 6cba55c425..4d105ef28e 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -482,6 +482,33 @@ class TestDB(unittest.TestCase): frappe.db.delete("ToDo", {"description": test_body}) + def test_count(self): + frappe.db.delete("Note") + + frappe.get_doc(doctype="Note", title="note1", content="something").insert() + frappe.get_doc(doctype="Note", title="note2", content="someting else").insert() + + # Count with no filtes + self.assertEquals((frappe.db.count("Note")), 2) + + # simple filters + self.assertEquals((frappe.db.count("Note", ["title", "=", "note1"])), 1) + + frappe.get_doc(doctype="Note", title="note3", content="something other").insert() + + # List of list filters with tables + self.assertEquals( + ( + frappe.db.count( + "Note", + [["Note", "title", "like", "note%"], ["Note", "content", "like", "some%"]], + ) + ), + 3, + ) + + frappe.db.rollback() + @run_only_if(db_type_is.MARIADB) class TestDDLCommandsMaria(unittest.TestCase): From b1a0a3816b9a6dd30bfdb36820e09b1c5225cf92 Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Tue, 19 Apr 2022 13:02:21 +0530 Subject: [PATCH 5/7] style: Applied Frappe linting --- frappe/database/database.py | 2 +- frappe/tests/test_query.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index eca1656424..9e249fc2ad 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -1019,7 +1019,7 @@ class Database(object): return self.get_value(dt, dn, ignore=True, cache=cache) - def count(self, dt, filters=None, debug=False, cache=False, distinct : bool = True): + def count(self, dt, filters=None, debug=False, cache=False, distinct: bool = True): """Returns `COUNT(*)` for given DocType and filters.""" if cache and not filters: cache_count = frappe.cache().get_value("doctype:count:{}".format(dt)) diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index fc03a66166..85d1355d71 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -1,8 +1,9 @@ import unittest -import frappe +import frappe from frappe.tests.test_query_builder import db_type_is, run_only_if + @run_only_if(db_type_is.MARIADB) class TestQuery(unittest.TestCase): def test_multiple_tables_in_filters(self): @@ -16,4 +17,4 @@ class TestQuery(unittest.TestCase): ], ).get_sql(), "SELECT * FROM `tabDocType` WHERE `tabBOM Update Log`.`name` LIKE 'f%' AND `tabDocType`.`parent`='something'", - ) \ No newline at end of file + ) From 36de21c059b5bd1c133817ddf9a3473c07db4ae4 Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Tue, 26 Apr 2022 15:37:54 +0530 Subject: [PATCH 6/7] feat: left join tables by default --- frappe/database/query.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/frappe/database/query.py b/frappe/database/query.py index 65ac375148..f7393bfa54 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -312,7 +312,16 @@ class Query: filters: Union[Dict[str, Union[str, int]], str, int, List[Union[List, str, int]]] = None, **kwargs, ): + # Clean up state before each query + self.tables = {} criterion = self.build_conditions(table, filters, **kwargs) + + if len(self.tables) > 1: + primary_table = self.tables[table] + del self.tables[table] + for table_object in self.tables.values(): + criterion = criterion.left_join(table_object).on(table_object.parent == primary_table.name) + if isinstance(fields, (list, tuple)): query = criterion.select(*kwargs.get("field_objects", fields)) From ba51289c3727919feb49d7b1c2286734dc473c09 Mon Sep 17 00:00:00 2001 From: saxenabhishek Date: Tue, 26 Apr 2022 15:59:50 +0530 Subject: [PATCH 7/7] test: update test_multiple_tables_in_filters --- frappe/tests/test_query.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 85d1355d71..949c3e9433 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -16,5 +16,5 @@ class TestQuery(unittest.TestCase): ["DocType", "parent", "=", "something"], ], ).get_sql(), - "SELECT * FROM `tabDocType` WHERE `tabBOM Update Log`.`name` LIKE 'f%' AND `tabDocType`.`parent`='something'", + "SELECT * FROM `tabDocType` LEFT JOIN `tabBOM Update Log` ON `tabBOM Update Log`.`parent`=`tabDocType`.`name` WHERE `tabBOM Update Log`.`name` LIKE 'f%' AND `tabDocType`.`parent`='something'", )