From aedd2fb2b691a8706a72244b80b86cca3d05c0cb Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Fri, 1 Apr 2022 20:39:27 +0530 Subject: [PATCH] Revert "perf: 80% faster `doc.get` for fields with `None` as value" (#16490) --- frappe/model/base_document.py | 53 +++++++++-------------------------- frappe/model/document.py | 32 ++++++++++----------- frappe/model/meta.py | 12 ++++++-- 3 files changed, 39 insertions(+), 58 deletions(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 05aa0ef2cd..3e9d1317e8 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -3,7 +3,7 @@ import datetime import frappe -from frappe import _, _dict +from frappe import _ from frappe.model import child_table_fields, default_fields, display_fieldtypes, table_fields from frappe.model.naming import set_new_name from frappe.model.utils.link_count import notify_link_count @@ -20,14 +20,6 @@ max_positive_value = { DOCTYPES_FOR_DOCTYPE = ('DocType', 'DocField', 'DocPerm', 'DocType Action', 'DocType Link') -DOCTYPE_TABLE_FIELDS = [ - _dict({"fieldname": "fields", "options": "DocField"}), - _dict({"fieldname": "permissions", "options": "DocPerm"}), - _dict({"fieldname": "actions", "options": "DocType Action"}), - _dict({"fieldname": "links", "options": "DocType Link"}), - _dict({"fieldname": "states", "options": "DocType State"}), -] - def get_controller(doctype): """Returns the **class** object of the given DocType. @@ -154,6 +146,12 @@ class BaseDocument(object): else: value = self.__dict__.get(key, default) + if value is None and key in ( + d.fieldname for d in self.meta.get_table_fields() + ): + value = [] + self.set(key, value) + if limit and isinstance(value, (list, tuple)) and len(value) > limit: value = value[:limit] @@ -191,7 +189,7 @@ class BaseDocument(object): if value is None: value={} if isinstance(value, (dict, BaseDocument)): - if self.__dict__.get(key) is None: + if not self.__dict__.get(key): self.__dict__[key] = [] value = self._init_child(value, key) @@ -254,23 +252,8 @@ class BaseDocument(object): return value - def _get_table_fields(self): - """ - To get table fields during Document init - Meta.get_table_fields goes into recursion for special doctypes - """ - - # child tables don't have child tables - if getattr(self, "parentfield", None): - return () - - if self.doctype == "DocType": - return DOCTYPE_TABLE_FIELDS - - return self.meta.get_table_fields() - def get_valid_dict(self, sanitize=True, convert_dates_to_str=False, ignore_nulls=False, ignore_virtual=False): - d = _dict() + d = frappe._dict() for fieldname in self.meta.get_valid_columns(): d[fieldname] = self.get(fieldname) @@ -331,16 +314,6 @@ class BaseDocument(object): return d - def init_child_tables(self): - """ - This is needed so that one can loop over child table properties - without worrying about whether or not they have values - """ - - for df in self._get_table_fields(): - if self.__dict__.get(df.fieldname) is None: - self.__dict__[df.fieldname] = [] - def init_valid_columns(self): for key in default_fields: if key not in self.__dict__: @@ -625,7 +598,7 @@ class BaseDocument(object): if self.meta.istable: for fieldname in ("parent", "parenttype"): if not self.get(fieldname): - missing.append((fieldname, get_msg(_dict(label=fieldname)))) + missing.append((fieldname, get_msg(frappe._dict(label=fieldname)))) return missing @@ -670,7 +643,7 @@ class BaseDocument(object): if not frappe.get_meta(doctype).get('is_virtual'): if not fields_to_fetch: # cache a single value type - values = _dict(name=frappe.db.get_value(doctype, docname, + values = frappe._dict(name=frappe.db.get_value(doctype, docname, 'name', cache=True)) else: values_to_fetch = ['name'] + [_df.fetch_from.split('.')[-1] @@ -972,10 +945,10 @@ class BaseDocument(object): cache_key = parentfield or "main" if not hasattr(self, "_precision"): - self._precision = _dict() + self._precision = frappe._dict() if cache_key not in self._precision: - self._precision[cache_key] = _dict() + self._precision[cache_key] = frappe._dict() if fieldname not in self._precision[cache_key]: self._precision[cache_key][fieldname] = None diff --git a/frappe/model/document.py b/frappe/model/document.py index 0d1b0a6af3..15e9c28d83 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -113,7 +113,6 @@ class Document(BaseDocument): if kwargs: # init base document super(Document, self).__init__(kwargs) - self.init_child_tables() self.init_valid_columns() else: @@ -149,20 +148,20 @@ class Document(BaseDocument): super(Document, self).__init__(d) - for df in self._get_table_fields(): - children = frappe.db.get_values( - df.options, - { - "parent": self.name, - "parenttype": self.doctype, - "parentfield": df.fieldname - }, - "*", - as_dict=True, - order_by="idx asc" - ) or [] - - self.set(df.fieldname, children) + if self.name=="DocType" and self.doctype=="DocType": + from frappe.model.meta import DOCTYPE_TABLE_FIELDS + table_fields = DOCTYPE_TABLE_FIELDS + else: + table_fields = self.meta.get_table_fields() + + for df in table_fields: + children = frappe.db.get_values(df.options, + {"parent": self.name, "parenttype": self.doctype, "parentfield": df.fieldname}, + "*", as_dict=True, order_by="idx asc") + if children: + self.set(df.fieldname, children) + else: + self.set(df.fieldname, []) # sometimes __setup__ can depend on child values, hence calling again at the end if hasattr(self, "__setup__"): @@ -850,7 +849,8 @@ class Document(BaseDocument): if parenttype and df.options != parenttype: continue - if value := self.get(df.fieldname): + value = self.get(df.fieldname) + if isinstance(value, list): children.extend(value) return children diff --git a/frappe/model/meta.py b/frappe/model/meta.py index 6cfba382c6..a3d167fb9b 100644 --- a/frappe/model/meta.py +++ b/frappe/model/meta.py @@ -30,7 +30,7 @@ from frappe.model import ( optional_fields, table_fields, ) -from frappe.model.base_document import BaseDocument, DOCTYPE_TABLE_FIELDS +from frappe.model.base_document import BaseDocument from frappe.model.document import Document from frappe.model.workflow import get_workflow_name from frappe.modules import load_doctype_module @@ -180,7 +180,7 @@ class Meta(Document): def get_table_fields(self): if not hasattr(self, "_table_fields"): - if self.name != "DocType": + if self.name!="DocType": self._table_fields = self.get('fields', {"fieldtype": ['in', table_fields]}) else: self._table_fields = DOCTYPE_TABLE_FIELDS @@ -609,6 +609,14 @@ class Meta(Document): def is_nested_set(self): return self.has_field('lft') and self.has_field('rgt') +DOCTYPE_TABLE_FIELDS = [ + frappe._dict({"fieldname": "fields", "options": "DocField"}), + frappe._dict({"fieldname": "permissions", "options": "DocPerm"}), + frappe._dict({"fieldname": "actions", "options": "DocType Action"}), + frappe._dict({"fieldname": "links", "options": "DocType Link"}), + frappe._dict({"fieldname": "states", "options": "DocType State"}), +] + ####### def is_single(doctype):