diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index b16960c994e..e9edef76944 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/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) diff --git a/erpnext/manufacturing/doctype/bom_update_log/bom_updation_utils.py b/erpnext/manufacturing/doctype/bom_update_log/bom_updation_utils.py index 853de3ea945..ebc064396bf 100644 --- a/erpnext/manufacturing/doctype/bom_update_log/bom_updation_utils.py +++ b/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: diff --git a/erpnext/manufacturing/report/bom_explorer/bom_explorer.py b/erpnext/manufacturing/report/bom_explorer/bom_explorer.py index 680cb83b312..1f82ec847b3 100644 --- a/erpnext/manufacturing/report/bom_explorer/bom_explorer.py +++ b/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(): diff --git a/erpnext/projects/doctype/task/task.py b/erpnext/projects/doctype/task/task.py index 9eda760a4e7..c431af5cf11 100755 --- a/erpnext/projects/doctype/task/task.py +++ b/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