Merge pull request #56695 from mihir-kandoi/pg/recursive-cte

perf: use recursive CTEs for BOM and Task tree traversal

erpnext/manufacturing/doctype/bom/bom.py

@@ -739,7 +739,7 @@ class BOM(WebsiteGenerator):
 				)
 			)
 
-	def check_recursion(self, bom_list=None):
+	def check_recursion(self):
 		"""Check whether recursion occurs in any bom"""
 		bom_list = self.traverse_tree()
 		child_items = frappe.get_all(
@@ -861,21 +861,30 @@ class BOM(WebsiteGenerator):
 
 		self.append("items", row)
 
-	def traverse_tree(self, bom_list=None):
-		count = 0
-		if not bom_list:
-			bom_list = []
+	def traverse_tree(self):
+		"""Return this BOM and every descendant BOM. The whole sub-tree is fetched in one recursive
+		CTE (frappe.qb) instead of a query-per-node walk; the only caller (check_recursion) uses the
+		result purely as a membership set. Portable across postgres and mariadb 10.2+."""
+		bom_item = frappe.qb.DocType("BOM Item")
+		tree = frappe.qb.Table("bom_tree")
 
-		if self.name not in bom_list:
-			bom_list.append(self.name)
+		seed = (
+			frappe.qb.from_(bom_item)
+			.select(bom_item.bom_no.as_("bom"))
+			.where((bom_item.parent == self.name) & (bom_item.bom_no != "") & (bom_item.parenttype == "BOM"))
+		)
+		recursion = (
+			frappe.qb.from_(bom_item)
+			.join(tree)
+			.on(bom_item.parent == tree.bom)
+			.select(bom_item.bom_no)
+			.where((bom_item.bom_no != "") & (bom_item.parenttype == "BOM"))
+		)
+		descendants = (
+			frappe.qb.with_(seed + recursion, "bom_tree", recursive=True).from_(tree).select(tree.bom)
+		).run(pluck=True)
 
-		while count < len(bom_list):
-			for child_bom in _get_bom_children(bom_list[count]):
-				if child_bom not in bom_list:
-					bom_list.append(child_bom)
-			count += 1
-		bom_list.reverse()
-		return bom_list
+		return [self.name, *descendants]
 
 	def company_currency(self):
 		return erpnext.get_company_currency(self.company)

erpnext/manufacturing/doctype/bom_update_log/bom_updation_utils.py

@@ -67,29 +67,33 @@ def update_cost_in_level(doc: "BOMUpdateLog", bom_list: list[str], batch_name: i
 			frappe.db.commit()  # nosemgrep
 
 
-def get_ancestor_boms(new_bom: str, bom_list: list | None = None) -> list:
-	"Recursively get all ancestors of BOM."
-
-	bom_list = bom_list or []
+def get_ancestor_boms(new_bom: str) -> list:
+	"""Return every ancestor BOM of `new_bom` (BOMs that consume it, transitively) in one recursive
+	CTE built with frappe.qb -- portable across postgres and mariadb 10.2+. `UNION` makes it
+	cycle-safe (it stops once no new BOM is reached); a BOM that is its own ancestor is rejected."""
 	bom_item = frappe.qb.DocType("BOM Item")
+	tree = frappe.qb.Table("ancestor_boms")
 
-	parents = (
+	seed = (
 		frappe.qb.from_(bom_item)
-		.select(bom_item.parent)
+		.select(bom_item.parent.as_("bom"))
 		.where((bom_item.bom_no == new_bom) & (bom_item.docstatus < 2) & (bom_item.parenttype == "BOM"))
-		.run(as_dict=True)
 	)
+	recursion = (
+		frappe.qb.from_(bom_item)
+		.join(tree)
+		.on(bom_item.bom_no == tree.bom)
+		.select(bom_item.parent)
+		.where((bom_item.docstatus < 2) & (bom_item.parenttype == "BOM"))
+	)
+	ancestors = (
+		frappe.qb.with_(seed + recursion, "ancestor_boms", recursive=True).from_(tree).select(tree.bom)
+	).run(pluck=True)
 
-	for d in parents:
-		if new_bom == d.parent:
-			frappe.throw(_("BOM recursion: {0} cannot be child of {1}").format(new_bom, d.parent))
+	if new_bom in ancestors:
+		frappe.throw(_("BOM recursion: {0} cannot be an ancestor of itself").format(new_bom))
 
-		if d.parent not in tuple(bom_list):
-			bom_list.append(d.parent)
-
-		get_ancestor_boms(d.parent, bom_list)
-
-	return bom_list
+	return ancestors
 
 
 def update_new_bom_in_bom_items(unit_cost: float, current_bom: str, new_bom: str) -> None:

erpnext/manufacturing/report/bom_explorer/bom_explorer.py

@@ -2,6 +2,8 @@
 # For license information, please see license.txt
 
 
+from collections import defaultdict
+
 import frappe
 from frappe import _
 
@@ -14,29 +16,47 @@ def execute(filters=None):
 
 
 def get_data(filters, data):
-	get_exploded_items(filters.bom, data)
+	children_map = fetch_exploded_bom_items(filters.bom)
+	build_exploded_rows(filters.bom, children_map, data)
 
 
-def get_exploded_items(bom, data, indent=0, qty=1):
-	exploded_items = frappe.get_all(
-		"BOM Item",
-		filters={"parent": bom},
-		fields=[
-			"qty",
-			"bom_no",
-			"qty",
-			"item_code",
-			"item_name",
-			"description",
-			"uom",
-			"idx",
-			"is_phantom_item",
-		],
-		order_by="idx ASC",
+def fetch_exploded_bom_items(root_bom):
+	"""Every BOM Item in the exploded tree of `root_bom`, grouped by its parent BOM, in one
+	recursive CTE -- replaces a query-per-node walk with a single query. UNION keeps it cycle-safe
+	and fetches each sub-BOM's items only once even when it is reused across the tree."""
+	bom_item = frappe.qb.DocType("BOM Item")
+	tree = frappe.qb.Table("exploded_bom")
+	fields = [
+		bom_item.parent,
+		bom_item.qty,
+		bom_item.bom_no,
+		bom_item.item_code,
+		bom_item.item_name,
+		bom_item.description,
+		bom_item.uom,
+		bom_item.idx,
+		bom_item.is_phantom_item,
+	]
+	seed = frappe.qb.from_(bom_item).select(*fields).where(bom_item.parent == root_bom)
+	recursion = (
+		frappe.qb.from_(bom_item)
+		.join(tree)
+		.on(bom_item.parent == tree.bom_no)
+		.select(*fields)
+		.where(tree.bom_no != "")
 	)
+	rows = (
+		frappe.qb.with_(seed + recursion, "exploded_bom", recursive=True).from_(tree).select(tree.star)
+	).run(as_dict=True)
 
-	for item in exploded_items:
-		item["indent"] = indent
+	children_map = defaultdict(list)
+	for row in rows:
+		children_map[row.parent].append(row)
+	return children_map
+
+
+def build_exploded_rows(bom, children_map, data, indent=0, qty=1):
+	for item in sorted(children_map.get(bom, []), key=lambda row: row.idx):
 		data.append(
 			{
 				"item_code": item.item_code,
@@ -51,7 +71,7 @@ def get_exploded_items(bom, data, indent=0, qty=1):
 			}
 		)
 		if item.bom_no:
-			get_exploded_items(item.bom_no, data, indent=indent + 1, qty=item.qty)
+			build_exploded_rows(item.bom_no, children_map, data, indent + 1, item.qty)
 
 
 def get_columns():

erpnext/projects/doctype/task/task.py

@@ -9,7 +9,7 @@ from frappe import _, throw
 from frappe.desk.form.assign_to import clear, close_all_assignments
 from frappe.model.mapper import get_mapped_doc
 from frappe.query_builder.functions import Max, Min, Sum
-from frappe.utils import add_days, add_to_date, cstr, date_diff, flt, get_link_to_form, getdate, today
+from frappe.utils import add_days, add_to_date, date_diff, flt, get_link_to_form, getdate, today
 from frappe.utils.data import format_date
 from frappe.utils.nestedset import NestedSet
 
@@ -247,25 +247,32 @@ class Task(NestedSet):
 	def check_recursion(self):
 		if self.flags.ignore_recursion_check:
 			return
-		check_list = [["task", "parent"], ["parent", "task"]]
-		for d in check_list:
-			task_list, count = [self.name], 0
-			while len(task_list) > count:
-				tasks = frappe.get_all(
-					"Task Depends On",
-					filters={d[1]: cstr(task_list[count])},
-					fields=[d[0]],
-					as_list=True,
-				)
-				count = count + 1
-				for b in tasks:
-					if b[0] == self.name:
-						frappe.throw(_("Circular Reference Error"), CircularReferenceError)
-					if b[0]:
-						task_list.append(b[0])
+		# "Task Depends On" is a directed edge (parent depends on `task`); a cycle exists if this
+		# task is reachable from itself along either direction. One recursive CTE per direction
+		# fetches the whole reachable set in a single query -- UNION makes it cycle-safe at any
+		# depth, so unlike the old per-node BFS it needs no arbitrary depth cap.
+		for select_field, filter_field in (("task", "parent"), ("parent", "task")):
+			if self._reaches_self(select_field, filter_field):
+				frappe.throw(_("Circular Reference Error"), CircularReferenceError)
 
-				if count == 15:
-					break
+	def _reaches_self(self, select_field: str, filter_field: str) -> bool:
+		depends_on = frappe.qb.DocType("Task Depends On")
+		tree = frappe.qb.Table("dependency_tree")
+		seed = (
+			frappe.qb.from_(depends_on)
+			.select(depends_on[select_field].as_("node"))
+			.where(depends_on[filter_field] == self.name)
+		)
+		recursion = (
+			frappe.qb.from_(depends_on)
+			.join(tree)
+			.on(depends_on[filter_field] == tree.node)
+			.select(depends_on[select_field])
+		)
+		reachable = (
+			frappe.qb.with_(seed + recursion, "dependency_tree", recursive=True).from_(tree).select(tree.node)
+		).run(pluck=True)
+		return self.name in reachable
 
 	def reschedule_dependent_tasks(self):
 		end_date = self.exp_end_date or self.act_end_date