From 41d7563aff363ecb057a7b7d72e81cc68623074c Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Fri, 5 May 2023 14:24:57 +0530 Subject: [PATCH 1/3] feat: `child_field[]` syntax sugar for qb - fetch child table rows in qb.get_query - runs one query each for each child field --- frappe/database/query.py | 40 ++++++++++++++++++++++++++++++++++- frappe/query_builder/utils.py | 14 +++++++++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 3bf6824ab4..7ab65176da 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -88,9 +88,12 @@ class Engine: if not self.fields: self.fields = [getattr(self.table, "name")] + self.query._child_queries = [] for field in self.fields: if isinstance(field, DynamicTableField): self.query = field.apply_select(self.query) + elif isinstance(field, ChildTableFields): + self.query._child_queries.append(field) else: self.query = self.query.select(field) @@ -302,12 +305,14 @@ class Engine: if isinstance(field, Criterion): _fields.append(field) elif isinstance(field, str): - if "," in field: + if "," in field and "[" not in field: field = field.casefold() if "`" not in field else field field_list = COMMA_PATTERN.split(field) for field in field_list: if _field := field.strip(): _fields.append(parse_field(_field)) + elif "[" in field and "]" in field: + _fields.append(parse_field(field)) else: _fields.append(parse_field(field)) @@ -396,6 +401,17 @@ class DynamicTableField: elif linked_field.fieldtype in frappe.model.table_fields: return ChildTableField(linked_doctype, fieldname, doctype, alias=alias) + if "[" in field and "]" in field: + child_fieldname, child_fields = field.split("[", 1) + child_field = frappe.get_meta(doctype).get_field(child_fieldname) + child_doctype = child_field.options + child_fields = child_fields.rsplit("]", 1)[0] + if not child_fields: + child_fields = ["*"] + else: + child_fields = [f.strip() for f in child_fields.split(",")] + return ChildTableFields(child_doctype, child_fieldname, child_fields, doctype) + def apply_select(self, query: QueryBuilder) -> QueryBuilder: raise NotImplementedError @@ -457,6 +473,28 @@ class LinkTableField(DynamicTableField): return query +class ChildTableFields: + def __init__( + self, + doctype: str, + fieldname: str, + fields: list, + parent_doctype: str, + ) -> None: + self.doctype = doctype + self.fieldname = fieldname + self.fields = fields + ["parent", "parentfield"] + self.parent_doctype = parent_doctype + + def get_query(self, parent_names=None) -> QueryBuilder: + filters = { + "parenttype": self.parent_doctype, + "parentfield": self.fieldname, + "parent": ["in", parent_names], + } + return frappe.qb.get_query(self.doctype, fields=self.fields, filters=filters, order_by="idx asc") + + def literal_eval_(literal): try: return literal_eval(literal) diff --git a/frappe/query_builder/utils.py b/frappe/query_builder/utils.py index bfc2c49b8e..15f62bb44e 100644 --- a/frappe/query_builder/utils.py +++ b/frappe/query_builder/utils.py @@ -81,8 +81,20 @@ def patch_query_execute(): """ def execute_query(query, *args, **kwargs): + child_queries = query._child_queries if isinstance(query._child_queries, list) else [] + query, params = prepare_query(query) - return frappe.db.sql(query, params, *args, **kwargs) # nosemgrep + result = frappe.db.sql(query, params, *args, **kwargs) # nosemgrep + + if result and isinstance(result[0], dict) and result[0].name: + parent_names = [d.name for d in result] + for child_query in child_queries or []: + data = child_query.get_query(parent_names).run(as_dict=1) + for row in result: + row[child_query.fieldname] = [ + d for d in data if str(d.parent) == str(row.name) and d.parentfield == child_query.fieldname + ] + return result def prepare_query(query): import inspect From c4bb732eaae792e45eeb70aa2c1e1ad2483e545a Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Fri, 5 May 2023 16:39:38 +0530 Subject: [PATCH 2/3] fix: use dict syntax instead of string --- frappe/database/query.py | 37 ++++++++++++++++------------------- frappe/query_builder/utils.py | 27 +++++++++++++++---------- 2 files changed, 34 insertions(+), 30 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 7ab65176da..595bd5a3ff 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -92,7 +92,7 @@ class Engine: for field in self.fields: if isinstance(field, DynamicTableField): self.query = field.apply_select(self.query) - elif isinstance(field, ChildTableFields): + elif isinstance(field, ChildQuery): self.query._child_queries.append(field) else: self.query = self.query.select(field) @@ -304,15 +304,16 @@ class Engine: for field in fields: if isinstance(field, Criterion): _fields.append(field) + elif isinstance(field, dict): + for child_field, fields in field.items(): + _fields.append(ChildQuery(child_field, fields, self.doctype)) elif isinstance(field, str): - if "," in field and "[" not in field: + if "," in field: field = field.casefold() if "`" not in field else field field_list = COMMA_PATTERN.split(field) for field in field_list: if _field := field.strip(): _fields.append(parse_field(_field)) - elif "[" in field and "]" in field: - _fields.append(parse_field(field)) else: _fields.append(parse_field(field)) @@ -401,17 +402,6 @@ class DynamicTableField: elif linked_field.fieldtype in frappe.model.table_fields: return ChildTableField(linked_doctype, fieldname, doctype, alias=alias) - if "[" in field and "]" in field: - child_fieldname, child_fields = field.split("[", 1) - child_field = frappe.get_meta(doctype).get_field(child_fieldname) - child_doctype = child_field.options - child_fields = child_fields.rsplit("]", 1)[0] - if not child_fields: - child_fields = ["*"] - else: - child_fields = [f.strip() for f in child_fields.split(",")] - return ChildTableFields(child_doctype, child_fieldname, child_fields, doctype) - def apply_select(self, query: QueryBuilder) -> QueryBuilder: raise NotImplementedError @@ -473,18 +463,20 @@ class LinkTableField(DynamicTableField): return query -class ChildTableFields: +class ChildQuery: def __init__( self, - doctype: str, fieldname: str, fields: list, parent_doctype: str, ) -> None: - self.doctype = doctype + field = frappe.get_meta(parent_doctype).get_field(fieldname) + if field.fieldtype not in frappe.model.table_fields: + return self.fieldname = fieldname - self.fields = fields + ["parent", "parentfield"] + self.fields = fields self.parent_doctype = parent_doctype + self.doctype = field.options def get_query(self, parent_names=None) -> QueryBuilder: filters = { @@ -492,7 +484,12 @@ class ChildTableFields: "parentfield": self.fieldname, "parent": ["in", parent_names], } - return frappe.qb.get_query(self.doctype, fields=self.fields, filters=filters, order_by="idx asc") + return frappe.qb.get_query( + self.doctype, + fields=self.fields + ["parent", "parentfield"], + filters=filters, + order_by="idx asc", + ) def literal_eval_(literal): diff --git a/frappe/query_builder/utils.py b/frappe/query_builder/utils.py index 15f62bb44e..af2c871e83 100644 --- a/frappe/query_builder/utils.py +++ b/frappe/query_builder/utils.py @@ -82,20 +82,27 @@ def patch_query_execute(): def execute_query(query, *args, **kwargs): child_queries = query._child_queries if isinstance(query._child_queries, list) else [] - query, params = prepare_query(query) result = frappe.db.sql(query, params, *args, **kwargs) # nosemgrep - - if result and isinstance(result[0], dict) and result[0].name: - parent_names = [d.name for d in result] - for child_query in child_queries or []: - data = child_query.get_query(parent_names).run(as_dict=1) - for row in result: - row[child_query.fieldname] = [ - d for d in data if str(d.parent) == str(row.name) and d.parentfield == child_query.fieldname - ] + execute_child_queries(child_queries, result) return result + def execute_child_queries(queries, result): + if not result or not isinstance(result[0], dict) or not result[0].name: + return + parent_names = [d.name for d in result] + for child_query in queries: + data = child_query.get_query(parent_names).run(as_dict=1) + for row in result: + row[child_query.fieldname] = [] + for d in data: + if str(d.parent) == str(row.name) and d.parentfield == child_query.fieldname: + if "parent" not in child_query.fields: + del d["parent"] + if "parentfield" not in child_query.fields: + del d["parentfield"] + row[child_query.fieldname].append(d) + def prepare_query(query): import inspect From 7e7b1e024a9fc38169381c73c25a17efa24e68a2 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Fri, 5 May 2023 16:40:16 +0530 Subject: [PATCH 3/3] test: for child query in qb.get_query --- frappe/tests/test_query.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 82218e5952..dfebf5e890 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -419,3 +419,37 @@ class TestQuery(FrappeTestCase): frappe.db.sql("delete from `tabDocType` where `name` = 'Test Tree DocType'") frappe.db.sql_ddl("drop table if exists `tabTest Tree DocType`") + + def test_child_field_syntax(self): + note1 = frappe.get_doc( + doctype="Note", title="Note 1", seen_by=[{"user": "Administrator"}] + ).insert() + note2 = frappe.get_doc( + doctype="Note", title="Note 2", seen_by=[{"user": "Administrator"}, {"user": "Guest"}] + ).insert() + + result = frappe.qb.get_query( + "Note", + filters={"name": ["in", [note1.name, note2.name]]}, + fields=["name", {"seen_by": ["*"]}], + order_by="title asc", + ).run(as_dict=1) + + self.assertTrue(isinstance(result[0].seen_by, list)) + self.assertTrue(isinstance(result[1].seen_by, list)) + self.assertEqual(len(result[0].seen_by), 1) + self.assertEqual(len(result[1].seen_by), 2) + self.assertEqual(result[0].seen_by[0].user, "Administrator") + + result = frappe.qb.get_query( + "Note", + filters={"name": ["in", [note1.name, note2.name]]}, + fields=["name", {"seen_by": ["user"]}], + order_by="title asc", + ).run(as_dict=1) + + self.assertEqual(len(result[0].seen_by[0].keys()), 1) + self.assertEqual(result[1].seen_by[1].user, "Guest") + + note1.delete() + note2.delete()