perf: Optimise BOM Update Tool (#19236)

* perf: Optimize BOM update tool
- Remove redundant traverse_tree calls
- Cache bom_children data

Co-authored-by: Rohit Waghchaure <rohitw1991@gmail.com>

* fix: Replace get_list with db.sql

* fix: Enable versioning for updated BOM

* fix: Directly save doc_before_save from bom obj instead of using  load_doc_before_save

* fix: recurssion check performance issue

erpnext/manufacturing/doctype/bom/bom.py

@@ -420,8 +420,12 @@ class BOM(WebsiteGenerator):
 
 	def traverse_tree(self, bom_list=None):
 		def _get_children(bom_no):
-			return frappe.db.sql_list("""select bom_no from `tabBOM Item`
+			children = frappe.cache().hget('bom_children', bom_no)
-				where parent = %s and ifnull(bom_no, '') != '' and parenttype='BOM'""", bom_no)
+			if children is None:
+				children = frappe.db.sql_list("""SELECT `bom_no` FROM `tabBOM Item`
+					WHERE `parent`=%s AND `bom_no`!='' AND `parenttype`='BOM'""", bom_no)
+				frappe.cache().hset('bom_children', bom_no, children)
+			return children
 
 		count = 0
 		if not bom_list:
@@ -534,12 +538,24 @@ class BOM(WebsiteGenerator):
 	def get_child_exploded_items(self, bom_no, stock_qty):
 		""" Add all items from Flat BOM of child BOM"""
 		# Did not use qty_consumed_per_unit in the query, as it leads to rounding loss
-		child_fb_items = frappe.db.sql("""select bom_item.item_code, bom_item.item_name,
+		child_fb_items = frappe.db.sql("""
-			bom_item.description, bom_item.source_warehouse, bom_item.operation,
+			SELECT
-			bom_item.stock_uom, bom_item.stock_qty, bom_item.rate, bom_item.include_item_in_manufacturing,
+				bom_item.item_code,
-			bom_item.stock_qty / ifnull(bom.quantity, 1) as qty_consumed_per_unit
+				bom_item.item_name,
-			from `tabBOM Explosion Item` bom_item, tabBOM bom
+				bom_item.description,
-			where bom_item.parent = bom.name and bom.name = %s and bom.docstatus = 1""", bom_no, as_dict = 1)
+				bom_item.source_warehouse,
+				bom_item.operation,
+				bom_item.stock_uom,
+				bom_item.stock_qty,
+				bom_item.rate,
+				bom_item.include_item_in_manufacturing,
+				bom_item.stock_qty / ifnull(bom.quantity, 1) AS qty_consumed_per_unit
+			FROM `tabBOM Explosion Item` bom_item, tabBOM bom
+			WHERE
+				bom_item.parent = bom.name
+				AND bom.name = %s
+				AND bom.docstatus = 1
+		""", bom_no, as_dict = 1)
 
 		for d in child_fb_items:
 			self.add_to_cur_exploded_items(frappe._dict({

erpnext/manufacturing/doctype/bom_update_tool/bom_update_tool.py

@@ -14,23 +14,23 @@ class BOMUpdateTool(Document):
 	def replace_bom(self):
 		self.validate_bom()
 		self.update_new_bom()
+		frappe.cache().delete_key('bom_children')
 		bom_list = self.get_parent_boms(self.new_bom)
 		updated_bom = []
 
 		for bom in bom_list:
 			try:
-				bom_obj = frappe.get_doc("BOM", bom)
+				bom_obj = frappe.get_cached_doc('BOM', bom)
-				bom_obj.load_doc_before_save()
+				# this is only used for versioning and we do not want
-				updated_bom = bom_obj.update_cost_and_exploded_items(updated_bom)
+				# to make separate db calls by using load_doc_before_save
+				# which proves to be expensive while doing bulk replace
+				bom_obj._doc_before_save = bom_obj.as_dict()
 				bom_obj.calculate_cost()
 				bom_obj.update_parent_cost()
 				bom_obj.db_update()
-				if (getattr(bom_obj.meta, 'track_changes', False) and not bom_obj.flags.ignore_version):
+				if bom_obj.meta.get('track_changes') and not bom_obj.flags.ignore_version:
 					bom_obj.save_version()
-
-				frappe.db.commit()
 			except Exception:
-				frappe.db.rollback()
 				frappe.log_error(frappe.get_traceback())
 
 	def validate_bom(self):
@@ -42,22 +42,22 @@ class BOMUpdateTool(Document):
 				frappe.throw(_("The selected BOMs are not for the same item"))
 
 	def update_new_bom(self):
-		new_bom_unitcost = frappe.db.sql("""select total_cost/quantity
+		new_bom_unitcost = frappe.db.sql("""SELECT `total_cost`/`quantity`
-			from `tabBOM` where name = %s""", self.new_bom)
+			FROM `tabBOM` WHERE name = %s""", self.new_bom)
 		new_bom_unitcost = flt(new_bom_unitcost[0][0]) if new_bom_unitcost else 0
 
 		frappe.db.sql("""update `tabBOM Item` set bom_no=%s,
 			rate=%s, amount=stock_qty*%s where bom_no = %s and docstatus < 2 and parenttype='BOM'""",
 			(self.new_bom, new_bom_unitcost, new_bom_unitcost, self.current_bom))
 
-	def get_parent_boms(self, bom, bom_list=None):
+	def get_parent_boms(self, bom, bom_list=[]):
-		if not bom_list:
+		data = frappe.db.sql("""SELECT DISTINCT parent FROM `tabBOM Item`
-			bom_list = []
+			WHERE bom_no = %s AND docstatus < 2 AND parenttype='BOM'""", bom)
-
-		data = frappe.db.sql(""" select distinct parent from `tabBOM Item`
-			where bom_no = %s and docstatus < 2 and parenttype='BOM'""", bom)
 
 		for d in data:
+			if self.new_bom == d[0]:
+				frappe.throw(_("BOM recursion: {0} cannot be child of {1}").format(bom, self.new_bom))
+
 			bom_list.append(d[0])
 			self.get_parent_boms(d[0], bom_list)