From 414e6560af2d6010afcda709417cb026bc254732 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Mon, 3 Aug 2026 00:56:55 +0530 Subject: [PATCH 1/3] fix(postgres): read BOM/SO/MR line columns off one line, not Max() Max() over a text column is a sort, and the engines sort text differently: MariaDB's utf8mb4 collations fold case, the CI PostgreSQL orders by byte value. MAX('abc','ABD') is 'ABD' on MariaDB and 'abc' on PostgreSQL -- confirmed on CI in the probe attached to #56241. The parity effort wrapped many descriptive columns in Max() on the reasoning that it returns the value MySQL picked arbitrarily. Where the column is functionally dependent on the group key that holds and the wrap is a genuine no-op. Where it genuinely varies -- description, item_name, uom and their warehouses all describe a LINE, not the item -- it does not: MySQL picked a row, not a maximum, and the sort now diverges between engines. Aggregating each column separately can also pair one line's description with another's warehouse, or a uom with the wrong conversion factor. Take those columns from a single real line instead, the first by idx. Only groups built from more than one line need it. Each query now also selects Count(.name).distinct(), and the representative pass returns immediately when no group has more than one line -- in that case Max() of a single value is already exact and collation cannot apply. A BOM with no repeated item therefore issues no extra query at all, which matters because the explosion and sub-assembly resolution recurse per sub-BOM. Genuine repeats are memoised per request. Sites covered: BOM explosion and sub-item queries, sub-assembly raw materials, get_bom_items_as_dict, BOM Stock Analysis (both queries), Requested Items to Order and Receive, Pending SO Items for Purchase Request, and Job Card secondary items. --- .../requested_items_to_order_and_receive.py | 33 +++++- erpnext/manufacturing/doctype/bom/bom.py | 78 ++++++++++++- .../production_plan/services/bom_explosion.py | 103 ++++++++++++++++-- .../services/sub_assembly_queries.py | 58 +++++++++- .../bom_stock_analysis/bom_stock_analysis.py | 91 ++++++++++------ .../pending_so_items_for_purchase_request.py | 31 +++++- .../stock_entry/services/manufacturing.py | 44 +++++++- 7 files changed, 375 insertions(+), 63 deletions(-) diff --git a/erpnext/buying/report/requested_items_to_order_and_receive/requested_items_to_order_and_receive.py b/erpnext/buying/report/requested_items_to_order_and_receive/requested_items_to_order_and_receive.py index d1d9bd8266c..945fb82b57e 100644 --- a/erpnext/buying/report/requested_items_to_order_and_receive/requested_items_to_order_and_receive.py +++ b/erpnext/buying/report/requested_items_to_order_and_receive/requested_items_to_order_and_receive.py @@ -51,7 +51,6 @@ def get_data(filters): mr_item.item_code.as_("item_code"), Sum(Coalesce(mr_item.qty, 0)).as_("qty"), Sum(Coalesce(mr_item.stock_qty, 0)).as_("stock_qty"), - Max(Coalesce(mr_item.uom, "")).as_("uom"), Max(Coalesce(mr_item.stock_uom, "")).as_("stock_uom"), Sum(Coalesce(mr_item.ordered_qty, 0)).as_("ordered_qty"), Sum(Coalesce(mr_item.received_qty, 0)).as_("received_qty"), @@ -60,8 +59,6 @@ def get_data(filters): ), Sum(Coalesce(mr_item.received_qty, 0)).as_("received_qty"), (Sum(Coalesce(mr_item.stock_qty, 0)) - Sum(Coalesce(mr_item.ordered_qty, 0))).as_("qty_to_order"), - Max(mr_item.item_name).as_("item_name"), - Max(mr_item.description).as_("description"), Max(mr.company).as_("company"), ) .where( @@ -75,8 +72,34 @@ def get_data(filters): query = get_conditions(filters, query, mr, mr_item) # add conditional conditions query = query.groupby(mr.name, mr_item.item_code).orderby(Max(mr.transaction_date), Max(mr.schedule_date)) - data = query.run(as_dict=True) - return data + rows = query.run(as_dict=True) + apply_representative_lines(rows) + return rows + + +def apply_representative_lines(rows): + """Fill item_name/description/uom from one real Material Request Item line per group. + + All three are editable per line, so a request listing the same item twice holds several values + per group. Aggregating them sorts text, and MariaDB folds case while PostgreSQL orders by byte + value, so the engines pick differently. Take the first line by idx. + """ + material_requests = list({row.material_request for row in rows}) + representative = {} + if material_requests: + for line in frappe.get_all( + "Material Request Item", + filters={"parent": ("in", material_requests), "docstatus": 1}, + fields=["parent", "item_code", "item_name", "description", "uom"], + order_by="idx", + ): + representative.setdefault((line.parent, line.item_code), line) + + for row in rows: + line = representative.get((row.material_request, row.item_code)) + row.item_name = line.item_name if line else None + row.description = line.description if line else None + row.uom = line.uom if line else "" def get_conditions(filters, query, mr, mr_item): diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index a1e154d1333..5e3e493b3d8 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -1208,7 +1208,73 @@ def _query_bom_items(bom, company, opts): query, group_by = _add_bom_item_columns(query, t, bom, opts, track_semi_finished_goods) # qualify + aggregate idx: bare "idx" is ambiguous across the joined tables and isn't grouped # (idx is unique per BOM item, so Min() preserves the original ordering) — needed for postgres - return query.groupby(*group_by).orderby(Min(t.bom_item.idx)).run(as_dict=True) + rows = query.groupby(*group_by).orderby(Min(t.bom_item.idx)).run(as_dict=True) + + if not opts.fetch_secondary_items: + doctype = "BOM Explosion Item" if cint(opts.fetch_exploded) else "BOM Item" + # key only on group-by columns that belong to the line table. stock_uom is grouped from Item + # and can differ from the line's stored copy once an item's stock UOM is changed after the + # BOM was submitted; keying on it would miss and blank the row. It is functionally dependent + # on item_code anyway, so dropping it from the key loses nothing. + keys = [field.name for field in group_by if field.table is t.bom_item] + _apply_representative_lines(rows, doctype, bom, keys) + + return rows + + +def _line_columns_for(doctype): + columns = ["description", "source_warehouse"] + if doctype == "BOM Item": + # uom only means something beside its own conversion_factor, so they travel together + columns += ["uom", "conversion_factor"] + return columns + + +def _apply_representative_lines(rows, doctype, bom, keys): + """Fill the line-level columns from a single real BOM line per group. + + They describe a line, not an item, so a BOM listing the same item more than once holds several + values per group. Aggregating each independently can pair one line's description with another's + warehouse -- or a uom with the wrong conversion_factor -- and Max() over text is a sort, which + MariaDB (case-folding) and PostgreSQL (byte order) resolve differently. Take the first by idx. + """ + repeated = [row for row in rows if (row.pop("line_count", 1) or 1) > 1] + if not repeated: + return + + columns = _line_columns_for(doctype) + representative = _representative_lines(doctype, bom, tuple(keys), tuple(columns)) + + for row in repeated: + line = representative.get(tuple(row.get(key) for key in keys)) + if not line: + continue + for column in columns: + row[column] = line.get(column) + + +def _representative_lines(doctype, bom, keys, columns): + """Cached per request: get_bom_items_as_dict recurses through phantom BOMs, and the same + sub-BOM is commonly reached more than once.""" + cache = getattr(frappe.local, "_bom_representative_lines", None) + if cache is None: + cache = frappe.local._bom_representative_lines = {} + + cache_key = (doctype, bom, keys, columns) + if cache_key in cache: + return cache[cache_key] + + representative = {} + for line in frappe.get_all( + doctype, + filters={"parent": bom, "parenttype": "BOM", "docstatus": ("<", 2)}, + fields=[*keys, *columns], + order_by="idx", + ): + representative.setdefault(tuple(line.get(key) for key in keys), line) + + cache[cache_key] = representative + return representative def _get_bom_item_tables(opts): @@ -1290,10 +1356,11 @@ def _add_exploded_item_columns(query, t, bom, amount_col, stock_item_condition): # keeping the GROUP BY postgres-valid; the correlated idx subquery references only item_code # (a grouped column) so it stays valid and still overrides the explosion idx for display. query = query.select( + Max(t.bom_item.description).as_("description"), Max(t.bom_item.source_warehouse).as_("source_warehouse"), + Count(t.bom_item.name).distinct().as_("line_count"), Max(t.bom_item.operation).as_("operation"), Max(t.bom_item.include_item_in_manufacturing).as_("include_item_in_manufacturing"), - Max(t.bom_item.description).as_("description"), Max(t.bom_item.rate).as_("rate"), Max(t.bom_item.sourced_by_supplier).as_("sourced_by_supplier"), amount_col, @@ -1329,14 +1396,15 @@ def _add_normal_item_columns(query, t, amount_col, stock_item_condition, track_s # under the same alias and silently shadowed (last value wins in the dict), so it is dropped here # -- output is unchanged. query = query.select( - Max(t.bom_item.uom).as_("uom"), - Max(t.bom_item.conversion_factor).as_("conversion_factor"), + Max(t.bom_item.description).as_("description"), Max(t.bom_item.source_warehouse).as_("source_warehouse"), + Count(t.bom_item.name).distinct().as_("line_count"), Max(t.bom_item.operation).as_("operation"), Max(t.bom_item.include_item_in_manufacturing).as_("include_item_in_manufacturing"), Max(t.bom_item.sourced_by_supplier).as_("sourced_by_supplier"), + Max(t.bom_item.uom).as_("uom"), + Max(t.bom_item.conversion_factor).as_("conversion_factor"), amount_col, - Max(t.bom_item.description).as_("description"), Max(t.bom_item.base_rate).as_("rate"), Max(t.bom_item.operation_row_id).as_("operation_row_id"), t.bom_item.is_phantom_item, diff --git a/erpnext/manufacturing/doctype/production_plan/services/bom_explosion.py b/erpnext/manufacturing/doctype/production_plan/services/bom_explosion.py index 07503465451..7faea744898 100644 --- a/erpnext/manufacturing/doctype/production_plan/services/bom_explosion.py +++ b/erpnext/manufacturing/doctype/production_plan/services/bom_explosion.py @@ -4,7 +4,7 @@ """BOM explosion helpers for Production Plan material planning.""" import frappe -from frappe.query_builder.functions import IfNull, Max, Min, Sum +from frappe.query_builder.functions import Count, IfNull, Max, Min, Sum from erpnext.manufacturing.doctype.production_plan.services.planning_queries import get_uom_conversion_factor @@ -21,7 +21,7 @@ def _exploded_items_query(company, bom_no, include_non_stock_items, planned_qty) item = frappe.qb.DocType("Item") item_default = frappe.qb.DocType("Item Default") item_uom = frappe.qb.DocType("UOM Conversion Detail") - return ( + rows = ( frappe.qb.from_(bei) .join(bom) .on(bom.name == bei.parent) @@ -36,19 +36,100 @@ def _exploded_items_query(company, bom_no, include_non_stock_items, planned_qty) .groupby(bei.item_code, bei.stock_uom) ).run(as_dict=True) + _apply_representative_lines( + rows, "BOM Explosion Item", bom_no, ("item_code", "stock_uom"), include_non_stock_items + ) + return rows + + +def _apply_representative_lines(rows, doctype, bom_no, keys, include_non_stock_items=True): + """Fill description/source_warehouse from a single real BOM line per group. + + Both describe a line, not an item, so a BOM listing the same item more than once holds + several values per group. Aggregating each independently can pair one line's description + with another's warehouse, and Max() over text is a sort -- MariaDB folds case, PostgreSQL + orders by byte value, so the two engines pick differently. Take the first line by idx. + + Only groups built from more than one line need this. Where a group has a single line, Max() of + one value is that value, so the selected columns are already exact and no query is issued -- + which matters because this runs once per BOM in a recursive explosion. + """ + repeated = [row for row in rows if (row.pop("line_count", 1) or 1) > 1] + if not repeated: + return + + representative = _representative_lines(doctype, bom_no, tuple(keys), include_non_stock_items) + + for row in repeated: + line = representative.get(tuple(row.get(key) for key in keys)) + if line: + row.description = line.description + row.source_warehouse = line.source_warehouse + + +def _representative_lines(doctype, bom_no, keys, include_non_stock_items): + """Cached per request: the explosion recurses and commonly revisits the same sub-BOM.""" + cache = getattr(frappe.local, "_bom_explosion_representative_lines", None) + if cache is None: + cache = frappe.local._bom_explosion_representative_lines = {} + + cache_key = (doctype, bom_no, keys, include_non_stock_items) + if cache_key in cache: + return cache[cache_key] + + # only BOM Item carries is_phantom_item, and only its query ORs the phantom flag into the stock + # filter; the explosion table has neither + filters_phantom = doctype == "BOM Item" + fields = ["item_code", "stock_uom", "description", "source_warehouse"] + if filters_phantom: + fields.append("is_phantom_item") + + lines = frappe.get_all( + doctype, + filters={ + "parent": bom_no, + "parenttype": "BOM", + "is_sub_assembly_item": 0, + "docstatus": ("<", 2), + }, + fields=fields, + order_by="idx", + ) + + # mirror the caller's stock filter: a non-stock line the main query excluded must not become + # the representative for a group that only exists because of a phantom line + if not include_non_stock_items and filters_phantom and lines: + stock_items = set( + frappe.get_all( + "Item", + filters={"name": ("in", list({line.item_code for line in lines})), "is_stock_item": 1}, + pluck="name", + ) + ) + lines = [line for line in lines if line.item_code in stock_items or line.is_phantom_item] + + representative = {} + for line in lines: + representative.setdefault(tuple(line.get(key) for key in keys), line) + + cache[cache_key] = representative + return representative + def _exploded_item_columns(bei, bom, item, item_default, item_uom, planned_qty): - # only item_code/stock_uom are grouped; the rest are functionally dependent on the grouped item - # or arbitrary per BOM Item on MySQL -> Max() keeps the GROUP BY valid on postgres with the same - # value MySQL picked. + # every column here is functionally dependent on the grouped item_code -- Item, Item Default and + # UOM Conversion Detail are joined on it and the BOM is pinned by the filter -- so Max() returns + # their single value. The BOM-line columns come from a representative line instead; see + # _apply_representative_lines. return [ (IfNull(Sum(bei.stock_qty / IfNull(bom.quantity, 1)), 0) * planned_qty).as_("qty"), Max(item.item_name).as_("item_name"), Max(item.name).as_("item_code"), Max(bei.description).as_("description"), + Max(bei.source_warehouse).as_("source_warehouse"), + Count(bei.name).distinct().as_("line_count"), bei.stock_uom, Max(item.min_order_qty).as_("min_order_qty"), - Max(bei.source_warehouse).as_("source_warehouse"), Max(item.default_material_request_type).as_("default_material_request_type"), Max(item.min_order_qty).as_("min_order_qty"), Max(item_default.default_warehouse).as_("default_warehouse"), @@ -96,7 +177,7 @@ def _subitems_query(company, bom_no, include_non_stock_items, parent_qty, planne item = frappe.qb.DocType("Item") item_default = frappe.qb.DocType("Item Default") item_uom = frappe.qb.DocType("UOM Conversion Detail") - return ( + rows = ( frappe.qb.from_(bom_item) .join(bom) .on(bom.name == bom_item.parent) @@ -113,6 +194,9 @@ def _subitems_query(company, bom_no, include_non_stock_items, parent_qty, planne .orderby(Min(bom_item.idx)) ).run(as_dict=True) + _apply_representative_lines(rows, "BOM Item", bom_no, ("item_code",), include_non_stock_items) + return rows + def _subitem_columns(bom_item, bom, item, item_default, item_uom, parent_qty, planned_qty): qty = IfNull(parent_qty * Sum(bom_item.stock_qty / IfNull(bom.quantity, 1)) * planned_qty, 0).as_("qty") @@ -128,9 +212,10 @@ def _subitem_columns(bom_item, bom, item, item_default, item_uom, parent_qty, pl Max(item.item_name).as_("item_name"), qty, Max(item.is_sub_contracted_item).as_("is_sub_contracted"), - Max(bom_item.source_warehouse).as_("source_warehouse"), - Max(item.default_bom).as_("default_bom"), Max(bom_item.description).as_("description"), + Max(bom_item.source_warehouse).as_("source_warehouse"), + Count(bom_item.name).distinct().as_("line_count"), + Max(item.default_bom).as_("default_bom"), Max(bom_item.stock_uom).as_("stock_uom"), Max(item.min_order_qty).as_("min_order_qty"), Max(item.safety_stock).as_("safety_stock"), diff --git a/erpnext/manufacturing/doctype/production_plan/services/sub_assembly_queries.py b/erpnext/manufacturing/doctype/production_plan/services/sub_assembly_queries.py index 134dee34a2e..46384786a5e 100644 --- a/erpnext/manufacturing/doctype/production_plan/services/sub_assembly_queries.py +++ b/erpnext/manufacturing/doctype/production_plan/services/sub_assembly_queries.py @@ -4,7 +4,7 @@ """Sub-assembly resolution helpers for Production Plan.""" import frappe -from frappe.query_builder.functions import IfNull, Max, Sum +from frappe.query_builder.functions import Count, IfNull, Max, Sum from frappe.utils import flt from erpnext.manufacturing.doctype.bom.bom import get_children as get_bom_children @@ -167,7 +167,7 @@ def _sub_assembly_rm_query(company, bom_no, include_non_stock_items, planned_qty item = frappe.qb.DocType("Item") item_default = frappe.qb.DocType("Item Default") item_uom = frappe.qb.DocType("UOM Conversion Detail") - return ( + rows = ( frappe.qb.from_(bei) .join(bom) .on(bom.name == bei.parent) @@ -182,6 +182,57 @@ def _sub_assembly_rm_query(company, bom_no, include_non_stock_items, planned_qty .groupby(bei.item_code, bei.stock_uom, bei.bom_no, bei.is_phantom_item) ).run(as_dict=True) + _apply_representative_lines(rows, bom_no) + return rows + + +def _apply_representative_lines(rows, bom_no): + """Fill description/source_warehouse from a single real BOM Item line per group. + + Both describe a line, not an item. Aggregating each independently can pair one line's + description with another's warehouse, and Max() over text is a sort -- MariaDB folds case, + PostgreSQL orders by byte value, so the engines pick differently. Take the first line by idx. + """ + repeated = [row for row in rows if (row.pop("line_count", 1) or 1) > 1] + if not repeated: + return + + keys = ("item_code", "stock_uom", "bom_no", "is_phantom_item") + representative = _representative_lines(bom_no, keys) + + for row in repeated: + line = representative.get(tuple(row.get(key) for key in keys)) + if line: + row.description = line.description + row.source_warehouse = line.source_warehouse + + +def _representative_lines(bom_no, keys): + """Cached per request: sub-assembly resolution recurses and revisits the same BOM.""" + cache = getattr(frappe.local, "_sub_assembly_representative_lines", None) + if cache is None: + cache = frappe.local._sub_assembly_representative_lines = {} + + if bom_no in cache: + return cache[bom_no] + + representative = {} + for line in frappe.get_all( + "BOM Item", + filters={ + "parent": bom_no, + "parenttype": "BOM", + "is_sub_assembly_item": 0, + "docstatus": 1, + }, + fields=["item_code", "stock_uom", "bom_no", "is_phantom_item", "description", "source_warehouse"], + order_by="idx", + ): + representative.setdefault(tuple(line.get(key) for key in keys), line) + + cache[bom_no] = representative + return representative + def _sub_assembly_rm_columns(bei, bom, item, item_default, item_uom, planned_qty): # Grouped by item_code/stock_uom plus bom_no/is_phantom_item: those two MUST come from the same @@ -195,11 +246,12 @@ def _sub_assembly_rm_columns(bei, bom, item, item_default, item_uom, planned_qty Max(item.item_name).as_("item_name"), Max(item.name).as_("item_code"), Max(bei.description).as_("description"), + Max(bei.source_warehouse).as_("source_warehouse"), + Count(bei.name).distinct().as_("line_count"), bei.stock_uom, bei.is_phantom_item, bei.bom_no, Max(item.min_order_qty).as_("min_order_qty"), - Max(bei.source_warehouse).as_("source_warehouse"), Max(item.default_material_request_type).as_("default_material_request_type"), Max(item.min_order_qty).as_("min_order_qty"), Max(item_default.default_warehouse).as_("default_warehouse"), diff --git a/erpnext/manufacturing/report/bom_stock_analysis/bom_stock_analysis.py b/erpnext/manufacturing/report/bom_stock_analysis/bom_stock_analysis.py index 037f9176685..03e93ba1072 100644 --- a/erpnext/manufacturing/report/bom_stock_analysis/bom_stock_analysis.py +++ b/erpnext/manufacturing/report/bom_stock_analysis/bom_stock_analysis.py @@ -134,15 +134,15 @@ def get_data_without_qty_to_make(filters): for row in raw_rows: data.append( { - "item": row[0], - "description": row[1], - "from_bom_no": row[2], - "qty_per_unit": fmt_qty(row[3]), - "available_qty": fmt_qty(row[4]), + "item": row.item_code, + "description": row.description, + "from_bom_no": row.from_bom_no, + "qty_per_unit": fmt_qty(row.qty_per_unit), + "available_qty": fmt_qty(row.available_qty), } ) - min_producible = min((row[5] or 0) for row in raw_rows) if raw_rows else 0 + min_producible = min((row.producible_qty or 0) for row in raw_rows) if raw_rows else 0 # blank spacer row data.append({}) @@ -237,7 +237,6 @@ def get_bom_data(filters): .select( bom_item.item_code, # non-grouped columns are constant per grouped item_code -> Max() keeps the GROUP BY valid - Max(bom_item.description).as_("description"), Max(bom_item.parent).as_("from_bom_no"), Sum(bom_item.qty_consumed_per_unit).as_("qty_per_unit"), IfNull(Max(stock_qty.actual_qty), 0).as_("actual_qty"), @@ -249,28 +248,36 @@ def get_bom_data(filters): data = query.run(as_dict=True) + # description belongs to a BOM line, not to the item, so a component listed more than once holds + # several values per group. Max() over text is a sort and the engines sort text differently + # (MariaDB folds case, PostgreSQL orders by byte value), so read it off one real line instead. + # For BOM Item that same line also supplies bom_no + is_phantom_item, which drive whether and + # which sub-BOM explode_phantom_boms recurses into and so must stay coherent with each other: + # the first line, upgraded to the first phantom line if any exists, so a phantom sub-BOM is never + # dropped just because a non-phantom line happens to be listed first. + fields = ["item_code", "description"] + if bom_item_table == "BOM Item": + fields += ["bom_no", "is_phantom_item"] + + representative = {} + for line in frappe.get_all( + bom_item_table, + filters={"parent": filters.get("bom"), "parenttype": "BOM"}, + fields=fields, + order_by="idx", + ): + existing = representative.get(line.item_code) + if existing is None or (line.get("is_phantom_item") and not existing.get("is_phantom_item")): + representative[line.item_code] = line + + for row in data: + line = representative.get(row.item_code) + row.description = line.description if line else None + if bom_item_table == "BOM Item": + row.bom_no = line.bom_no if line else None + row.is_phantom_item = line.is_phantom_item if line else None + if bom_item_table == "BOM Item": - # bom_no + is_phantom_item drive whether/which sub-BOM explode_phantom_boms recurses into, so - # they must come from the SAME BOM Item line. Aggregating each independently (Max) could pair a - # bom_no from one line with is_phantom_item from another when an item_code repeats in the BOM. - # Rows are grouped by item_code (one qty_per_unit total per component), so pick one coherent - # representative line: the first line, but upgrade to the first phantom line if any exists, so a - # phantom sub-BOM is never dropped just because a non-phantom line happens to be listed first. - representative = {} - for line in frappe.get_all( - "BOM Item", - filters={"parent": filters.get("bom"), "parenttype": "BOM"}, - fields=["item_code", "bom_no", "is_phantom_item"], - order_by="idx", - ): - existing = representative.get(line.item_code) - if existing is None or (line.is_phantom_item and not existing.is_phantom_item): - representative[line.item_code] = line - for row in data: - line = representative.get(row.item_code) - if line: - row.bom_no = line.bom_no - row.is_phantom_item = line.is_phantom_item return explode_phantom_boms(data, filters) return data @@ -351,15 +358,37 @@ def get_producible_fg_items(filters): BOM_ITEM.item_code, # Sum() below makes this an aggregate query; the other columns are constant per grouped # item_code -> Max() keeps them valid on postgres with the same value MySQL picked. - Max(BOM_ITEM.description).as_("description"), + # description is not: it belongs to the line, so it comes from a representative one below. Max(BOM_ITEM.parent).as_("from_bom_no"), Max(BOM_ITEM.stock_qty / BOM.quantity).as_("qty_per_unit"), Max(IfNull(bin_subquery.actual_qty, 0)).as_("available_qty"), - Floor(Max(bin_subquery.actual_qty) / ((Sum(BOM_ITEM.stock_qty)) / Max(BOM.quantity))), + Floor(Max(bin_subquery.actual_qty) / ((Sum(BOM_ITEM.stock_qty)) / Max(BOM.quantity))).as_( + "producible_qty" + ), ) .where((BOM_ITEM.parent == filters.get("bom")) & (BOM_ITEM.parenttype == "BOM")) .groupby(BOM_ITEM.item_code) .orderby(Min(BOM_ITEM.idx)) ) - return query.run(as_list=True) + rows = query.run(as_dict=True) + descriptions = get_representative_descriptions("BOM Item", filters.get("bom")) + for row in rows: + row.description = descriptions.get(row.item_code) + + return rows + + +def get_representative_descriptions(doctype, bom): + """First line by idx per item_code. description belongs to a line, not an item, so aggregating it + sorts text -- and MariaDB folds case while PostgreSQL orders by byte value.""" + descriptions = {} + for line in frappe.get_all( + doctype, + filters={"parent": bom, "parenttype": "BOM"}, + fields=["item_code", "description"], + order_by="idx", + ): + descriptions.setdefault(line.item_code, line.description) + + return descriptions diff --git a/erpnext/selling/report/pending_so_items_for_purchase_request/pending_so_items_for_purchase_request.py b/erpnext/selling/report/pending_so_items_for_purchase_request/pending_so_items_for_purchase_request.py index 7d7ea42209f..59374f054ad 100644 --- a/erpnext/selling/report/pending_so_items_for_purchase_request/pending_so_items_for_purchase_request.py +++ b/erpnext/selling/report/pending_so_items_for_purchase_request/pending_so_items_for_purchase_request.py @@ -49,6 +49,29 @@ def get_columns(): return columns +def apply_representative_lines(rows, sales_orders): + """Fill item_name/description from one real Sales Order Item line per group. + + Both are editable per line, so an order listing the same item twice holds several values per + group. Aggregating them sorts text, and MariaDB folds case while PostgreSQL orders by byte + value, so the engines pick differently. Take the first line by idx. + """ + representative = {} + if sales_orders: + for line in frappe.get_all( + "Sales Order Item", + filters={"parent": ("in", sales_orders), "docstatus": 1}, + fields=["parent", "item_code", "item_name", "description"], + order_by="idx", + ): + representative.setdefault((line.parent, line.item_code), line) + + for row in rows: + line = representative.get((row.name, row.item_code)) + row.item_name = line.item_name if line else None + row.description = line.description if line else None + + def get_data(): so = frappe.qb.DocType("Sales Order") so_item = frappe.qb.DocType("Sales Order Item") @@ -58,10 +81,9 @@ def get_data(): .on(so.name == so_item.parent) .select( so_item.item_code, - # non-grouped columns are constant per grouped so.name / item_code -> Max() keeps the - # GROUP BY valid on postgres while returning the same value MySQL picked. - Max(so_item.item_name).as_("item_name"), - Max(so_item.description).as_("description"), + # the Sales Order columns are functionally dependent on the grouped so.name, so Max() + # returns their single value. item_name/description belong to the line and are editable + # per line, so they come from a representative line below. so.name, Max(so.transaction_date).as_("transaction_date"), Max(so.customer).as_("customer"), @@ -75,6 +97,7 @@ def get_data(): ) sales_orders = [row.name for row in sales_order_entry] + apply_representative_lines(sales_order_entry, sales_orders) mr_records = frappe.get_all( "Material Request Item", {"sales_order": ("in", sales_orders), "docstatus": 1}, diff --git a/erpnext/stock/doctype/stock_entry/services/manufacturing.py b/erpnext/stock/doctype/stock_entry/services/manufacturing.py index 26655f41355..672a6ebd12a 100644 --- a/erpnext/stock/doctype/stock_entry/services/manufacturing.py +++ b/erpnext/stock/doctype/stock_entry/services/manufacturing.py @@ -1007,11 +1007,9 @@ def get_secondary_items_from_job_card(work_order, jc_name=None): .select( Sum(job_card_secondary_item.stock_qty).as_("stock_qty"), job_card_secondary_item.item_code, - # non-grouped columns are item attributes / the secondary-item BOM link, constant per - # grouped (item_code, secondary_item_type) -> Max() keeps the GROUP BY valid on postgres - # while returning the value MySQL picked arbitrarily. - Max(job_card_secondary_item.item_name).as_("item_name"), - Max(job_card_secondary_item.description).as_("description"), + # stock_uom and the secondary-item BOM link are constant per grouped + # (item_code, secondary_item_type) -> Max() returns their single value. item_name and + # description are editable per line, so they come from a representative line below. Max(job_card_secondary_item.stock_uom).as_("stock_uom"), job_card_secondary_item.secondary_item_type, Max(job_card_secondary_item.bom_secondary_item).as_("bom_secondary_item"), @@ -1030,7 +1028,41 @@ def get_secondary_items_from_job_card(work_order, jc_name=None): if jc_name: secondary_items = secondary_items.where(job_card.name == jc_name) - return secondary_items.run(as_dict=1) + rows = secondary_items.run(as_dict=1) + apply_representative_secondary_lines(rows, work_order, jc_name) + return rows + + +def apply_representative_secondary_lines(rows, work_order, jc_name=None): + """Fill item_name/description from one real Job Card Secondary Item line per group. + + Both are editable per line, so the same secondary item across a work order's job cards can + carry several values per group. Aggregating them sorts text, and MariaDB folds case while + PostgreSQL orders by byte value, so the engines pick differently. + """ + job_cards = frappe.get_all( + "Job Card", + filters={"work_order": work_order, "docstatus": 1, **({"name": jc_name} if jc_name else {})}, + pluck="name", + ) + + representative = {} + if job_cards: + for line in frappe.get_all( + "Job Card Secondary Item", + filters={"parent": ("in", job_cards)}, + # idx first, so the rule really is "first by idx"; creation breaks ties across job cards. + # Never order by parent -- the Job Card name is text, and sorting text is the divergence + # this is here to avoid. + fields=["item_code", "secondary_item_type", "item_name", "description"], + order_by="idx, creation", + ): + representative.setdefault((line.item_code, line.secondary_item_type), line) + + for row in rows: + line = representative.get((row.item_code, row.secondary_item_type)) + row.item_name = line.item_name if line else None + row.description = line.description if line else None def get_previous_operation_output_sn_batch(work_order, item_code, warehouse): From 100d0ee784a3716d216ed25e182151248be21a77 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Mon, 3 Aug 2026 00:56:56 +0530 Subject: [PATCH 2/3] test(manufacturing): cover the BOM representative-line pick A BOM listing one item on two lines, with descriptions and source warehouses that differ. The second line's description sorts above the first on either engine, so an aggregated value would win; the row must instead carry the first line's description together with that same line's warehouse. --- erpnext/manufacturing/doctype/bom/test_bom.py | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/erpnext/manufacturing/doctype/bom/test_bom.py b/erpnext/manufacturing/doctype/bom/test_bom.py index 9ff57dae852..1ac43b992f6 100644 --- a/erpnext/manufacturing/doctype/bom/test_bom.py +++ b/erpnext/manufacturing/doctype/bom/test_bom.py @@ -128,6 +128,43 @@ class TestBOM(ERPNextTestSuite): self.assertEqual(len([row for row in items_dict if row == rm.name]), 1) self.assertAlmostEqual(flt(items_dict[rm.name].amount), expected, places=2) + @timeout + def test_get_items_takes_line_columns_from_one_line(self): + from erpnext.manufacturing.doctype.bom.bom import get_bom_items_as_dict + from erpnext.manufacturing.doctype.production_plan.test_production_plan import make_bom + from erpnext.stock.doctype.warehouse.test_warehouse import create_warehouse + + rm = make_item(properties={"is_stock_item": 1, "valuation_rate": 10}) + fg_item = make_item(properties={"is_stock_item": 1, "valuation_rate": 10}).name + + first_warehouse = create_warehouse("_Test BOM Line A") + second_warehouse = create_warehouse("_Test BOM Line B") + + bom = make_bom(item=fg_item, raw_materials=[rm.name], rm_qty=2, do_not_save=True) + bom.items[0].description = "bbb first line" + bom.items[0].source_warehouse = first_warehouse + bom.append( + "items", + { + "item_code": rm.name, + "qty": 3, + "uom": rm.stock_uom, + "stock_uom": rm.stock_uom, + "description": "ccc second line", + "source_warehouse": second_warehouse, + }, + ) + bom.save() + bom.submit() + + items_dict = get_bom_items_as_dict(bom.name, "_Test Company", qty=1, fetch_exploded=0) + row = items_dict[rm.name] + + # "ccc" sorts above "bbb" on either engine, so an aggregated description would win here; + # the value must instead come from the first line, together with that line's warehouse + self.assertEqual(row.description, "bbb first line") + self.assertEqual(row.source_warehouse, first_warehouse) + @timeout def test_default_bom(self): def _get_default_bom_in_item(): From c8adf9937bd70ccfed14a9b46e202ba4757c5a8f Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Mon, 3 Aug 2026 01:03:11 +0530 Subject: [PATCH 3/3] refactor(postgres): memoise representative lines with frappe's request_cache Three helpers each managed their own dictionary on frappe.local, duplicating cache lifecycle and key handling. @request_cache does the same thing centrally and is cleared with the request, so the copies cannot drift apart. Behaviour is unchanged: the decorator keys on the call arguments, which are the same tuple each hand-rolled key was built from. --- erpnext/manufacturing/doctype/bom/bom.py | 11 ++--------- .../doctype/production_plan/services/bom_explosion.py | 11 ++--------- .../production_plan/services/sub_assembly_queries.py | 10 ++-------- 3 files changed, 6 insertions(+), 26 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index 5e3e493b3d8..2717da14826 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -11,6 +11,7 @@ from frappe.model.document import Document from frappe.query_builder import Field from frappe.query_builder.functions import Count, IfNull, Max, Min, NullIf, Sum from frappe.utils import cint, cstr, flt, get_link_to_form, parse_json +from frappe.utils.caching import request_cache from frappe.website.website_generator import WebsiteGenerator import erpnext @@ -1253,17 +1254,10 @@ def _apply_representative_lines(rows, doctype, bom, keys): row[column] = line.get(column) +@request_cache def _representative_lines(doctype, bom, keys, columns): """Cached per request: get_bom_items_as_dict recurses through phantom BOMs, and the same sub-BOM is commonly reached more than once.""" - cache = getattr(frappe.local, "_bom_representative_lines", None) - if cache is None: - cache = frappe.local._bom_representative_lines = {} - - cache_key = (doctype, bom, keys, columns) - if cache_key in cache: - return cache[cache_key] - representative = {} for line in frappe.get_all( doctype, @@ -1273,7 +1267,6 @@ def _representative_lines(doctype, bom, keys, columns): ): representative.setdefault(tuple(line.get(key) for key in keys), line) - cache[cache_key] = representative return representative diff --git a/erpnext/manufacturing/doctype/production_plan/services/bom_explosion.py b/erpnext/manufacturing/doctype/production_plan/services/bom_explosion.py index 7faea744898..217feb4c814 100644 --- a/erpnext/manufacturing/doctype/production_plan/services/bom_explosion.py +++ b/erpnext/manufacturing/doctype/production_plan/services/bom_explosion.py @@ -5,6 +5,7 @@ import frappe from frappe.query_builder.functions import Count, IfNull, Max, Min, Sum +from frappe.utils.caching import request_cache from erpnext.manufacturing.doctype.production_plan.services.planning_queries import get_uom_conversion_factor @@ -67,16 +68,9 @@ def _apply_representative_lines(rows, doctype, bom_no, keys, include_non_stock_i row.source_warehouse = line.source_warehouse +@request_cache def _representative_lines(doctype, bom_no, keys, include_non_stock_items): """Cached per request: the explosion recurses and commonly revisits the same sub-BOM.""" - cache = getattr(frappe.local, "_bom_explosion_representative_lines", None) - if cache is None: - cache = frappe.local._bom_explosion_representative_lines = {} - - cache_key = (doctype, bom_no, keys, include_non_stock_items) - if cache_key in cache: - return cache[cache_key] - # only BOM Item carries is_phantom_item, and only its query ORs the phantom flag into the stock # filter; the explosion table has neither filters_phantom = doctype == "BOM Item" @@ -112,7 +106,6 @@ def _representative_lines(doctype, bom_no, keys, include_non_stock_items): for line in lines: representative.setdefault(tuple(line.get(key) for key in keys), line) - cache[cache_key] = representative return representative diff --git a/erpnext/manufacturing/doctype/production_plan/services/sub_assembly_queries.py b/erpnext/manufacturing/doctype/production_plan/services/sub_assembly_queries.py index 46384786a5e..ec1760976a4 100644 --- a/erpnext/manufacturing/doctype/production_plan/services/sub_assembly_queries.py +++ b/erpnext/manufacturing/doctype/production_plan/services/sub_assembly_queries.py @@ -6,6 +6,7 @@ import frappe from frappe.query_builder.functions import Count, IfNull, Max, Sum from frappe.utils import flt +from frappe.utils.caching import request_cache from erpnext.manufacturing.doctype.bom.bom import get_children as get_bom_children from erpnext.manufacturing.doctype.production_plan.services.planning_queries import ( @@ -207,15 +208,9 @@ def _apply_representative_lines(rows, bom_no): row.source_warehouse = line.source_warehouse +@request_cache def _representative_lines(bom_no, keys): """Cached per request: sub-assembly resolution recurses and revisits the same BOM.""" - cache = getattr(frappe.local, "_sub_assembly_representative_lines", None) - if cache is None: - cache = frappe.local._sub_assembly_representative_lines = {} - - if bom_no in cache: - return cache[bom_no] - representative = {} for line in frappe.get_all( "BOM Item", @@ -230,7 +225,6 @@ def _representative_lines(bom_no, keys): ): representative.setdefault(tuple(line.get(key) for key in keys), line) - cache[bom_no] = representative return representative