From 17108d8a3746f6f369445b10329986cd395652ef Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Sat, 20 Jun 2026 09:39:17 +0530 Subject: [PATCH] refactor(stock): convert stock_balance raw SQL to qb/ORM Convert the repost item/warehouse UNION, get_balance_qty_from_sle, get_reserved_qty (UNION of correlated subqueries -> two qb Sum branches with an inner_join to Sales Order Item, added in Python; qty!=0 guards the divide and mirrors the original `qty>=delivered_qty` which on MariaDB excluded x/0 NULL rows), get_indented_qty, get_planned_qty and set_stock_balance_as_per_serial_no to frappe.qb / get_all / db.count. Same result on MariaDB; valid under Postgres. Tests (new test_stock_balance.py): get_reserved_qty SO-item + packed-bundle branches and get_indented_qty, all without delivery so they avoid the unrelated #39 SLE-repost path and pass on Postgres. Co-Authored-By: Claude Opus 4.8 (1M context) --- erpnext/stock/stock_balance.py | 239 +++++++++++++++------------- erpnext/stock/test_stock_balance.py | 51 ++++++ 2 files changed, 179 insertions(+), 111 deletions(-) create mode 100644 erpnext/stock/test_stock_balance.py diff --git a/erpnext/stock/stock_balance.py b/erpnext/stock/stock_balance.py index 2f853252723..62f031e828e 100644 --- a/erpnext/stock/stock_balance.py +++ b/erpnext/stock/stock_balance.py @@ -19,15 +19,11 @@ def repost(only_actual=False, allow_negative_stock=False, allow_zero_rate=False, existing_allow_negative_stock = frappe.get_single_value("Stock Settings", "allow_negative_stock") frappe.db.set_single_value("Stock Settings", "allow_negative_stock", 1) - item_warehouses = frappe.db.sql( - """ - select distinct item_code, warehouse - from - (select item_code, warehouse from tabBin - union - select item_code, warehouse from `tabStock Ledger Entry`) a - """ + item_warehouses = frappe.get_all("Bin", fields=["item_code", "warehouse"], as_list=True) + item_warehouses += frappe.get_all( + "Stock Ledger Entry", fields=["item_code", "warehouse"], distinct=True, as_list=True ) + item_warehouses = list({tuple(d) for d in item_warehouses}) for d in item_warehouses: try: repost_stock(d[0], d[1], allow_zero_rate, only_actual, only_bin, allow_negative_stock) @@ -79,102 +75,112 @@ def repost_actual_qty(item_code, warehouse, allow_zero_rate=False, allow_negativ def get_balance_qty_from_sle(item_code, warehouse): - balance_qty = frappe.db.sql( - """select qty_after_transaction from `tabStock Ledger Entry` - where item_code=%s and warehouse=%s and is_cancelled=0 - order by posting_datetime desc, creation desc - limit 1""", - (item_code, warehouse), + balance_qty = frappe.get_all( + "Stock Ledger Entry", + filters={"item_code": item_code, "warehouse": warehouse, "is_cancelled": 0}, + fields=["qty_after_transaction"], + order_by="posting_datetime desc, creation desc", + limit=1, ) - return flt(balance_qty[0][0]) if balance_qty else 0.0 + return flt(balance_qty[0].qty_after_transaction) if balance_qty else 0.0 def get_reserved_qty(item_code, warehouse): dont_reserve_on_return = frappe.get_cached_value( "Selling Settings", "Selling Settings", "dont_reserve_sales_order_qty_on_sales_return" ) - reserved_qty = frappe.db.sql( - f""" - select - sum(dnpi_qty * ((so_item_qty - so_item_delivered_qty - (case when dont_reserve_qty_on_return = 1 then so_item_returned_qty else 0 end)) / so_item_qty)) - from - ( - (select - qty as dnpi_qty, - ( - select qty from `tabSales Order Item` - where name = dnpi.parent_detail_docname - and (delivered_by_supplier is null or delivered_by_supplier = 0) - ) as so_item_qty, - ( - select delivered_qty from `tabSales Order Item` - where name = dnpi.parent_detail_docname - and delivered_by_supplier = 0 - ) as so_item_delivered_qty, - ( - select returned_qty from `tabSales Order Item` - where name = dnpi.parent_detail_docname - and delivered_by_supplier = 0 - ) as so_item_returned_qty, - {dont_reserve_on_return} as dont_reserve_qty_on_return, - parent, name - from - ( - select qty, parent_detail_docname, parent, name - from `tabPacked Item` dnpi_in - where item_code = %s and warehouse = %s - and parenttype='Sales Order' - and item_code != parent_item - and exists (select * from `tabSales Order` so - where name = dnpi_in.parent and docstatus = 1 and status not in ('On Hold', 'Closed')) - ) dnpi) - union - (select stock_qty as dnpi_qty, qty as so_item_qty, - delivered_qty as so_item_delivered_qty, - returned_qty as so_item_returned_qty, - {dont_reserve_on_return}, parent, name - from `tabSales Order Item` so_item - where item_code = %s and warehouse = %s - and (so_item.delivered_by_supplier is null or so_item.delivered_by_supplier = 0) - and exists(select * from `tabSales Order` so - where so.name = so_item.parent and so.docstatus = 1 - and so.status not in ('On Hold', 'Closed'))) - ) tab - where - so_item_qty >= so_item_delivered_qty - """, - (item_code, warehouse, item_code, warehouse), + so = frappe.qb.DocType("Sales Order") + so_item = frappe.qb.DocType("Sales Order Item") + packed_item = frappe.qb.DocType("Packed Item") + + open_so = (so.docstatus == 1) & so.status.notin(["On Hold", "Closed"]) + not_delivered_by_supplier = so_item.delivered_by_supplier.isnull() | (so_item.delivered_by_supplier == 0) + + # Keep the reserved-qty rollup in the DB (one aggregate per branch) instead of streaming + # every open packed-item / SO-item row into Python. `qty <> 0` mirrors the original + # `where so_item_qty >= so_item_delivered_qty` *and* guards the divide-by-`qty` below + # (MariaDB returned NULL for x/0, postgres raises), so qty=0 rows -- which contributed + # nothing anyway -- are excluded on both databases. + reservable = (so_item.qty != 0) & (so_item.qty >= so_item.delivered_qty) + if dont_reserve_on_return: + net_reserved = so_item.qty - so_item.delivered_qty - so_item.returned_qty + else: + net_reserved = so_item.qty - so_item.delivered_qty + + # Bundled (packed) items reserving stock against an open Sales Order + packed_qty = ( + frappe.qb.from_(packed_item) + .inner_join(so) + .on(so.name == packed_item.parent) + .inner_join(so_item) + .on(so_item.name == packed_item.parent_detail_docname) + .select(Sum(packed_item.qty * net_reserved / so_item.qty)) + .where( + (packed_item.item_code == item_code) + & (packed_item.warehouse == warehouse) + & (packed_item.parenttype == "Sales Order") + & (packed_item.item_code != packed_item.parent_item) + & not_delivered_by_supplier + & open_so + & reservable + ) + .run() ) - return flt(reserved_qty[0][0]) if reserved_qty else 0 + # Sales Order items directly reserving stock + so_item_qty = ( + frappe.qb.from_(so_item) + .inner_join(so) + .on(so.name == so_item.parent) + .select(Sum(so_item.stock_qty * net_reserved / so_item.qty)) + .where( + (so_item.item_code == item_code) + & (so_item.warehouse == warehouse) + & not_delivered_by_supplier + & open_so + & reservable + ) + .run() + ) + + return flt(packed_qty[0][0]) + flt(so_item_qty[0][0]) def get_indented_qty(item_code, warehouse): # Ordered Qty is always maintained in stock UOM - inward_qty = frappe.db.sql( - """ - select sum(mr_item.stock_qty - mr_item.ordered_qty) - from `tabMaterial Request Item` mr_item, `tabMaterial Request` mr - where mr_item.item_code=%s and mr_item.warehouse=%s - and mr.material_request_type in ('Purchase', 'Manufacture', 'Customer Provided', 'Material Transfer') - and mr_item.stock_qty > mr_item.ordered_qty and mr_item.parent=mr.name - and mr.status!='Stopped' and mr.docstatus=1 - """, - (item_code, warehouse), + mr_item = frappe.qb.DocType("Material Request Item") + mr = frappe.qb.DocType("Material Request") + base_conditions = ( + (mr_item.item_code == item_code) + & (mr_item.warehouse == warehouse) + & (mr_item.stock_qty > mr_item.ordered_qty) + & (mr.status != "Stopped") + & (mr.docstatus == 1) + ) + + inward_qty = ( + frappe.qb.from_(mr_item) + .inner_join(mr) + .on(mr_item.parent == mr.name) + .select(Sum(mr_item.stock_qty - mr_item.ordered_qty)) + .where( + base_conditions + & mr.material_request_type.isin( + ["Purchase", "Manufacture", "Customer Provided", "Material Transfer"] + ) + ) + .run() ) inward_qty = flt(inward_qty[0][0]) if inward_qty else 0 - outward_qty = frappe.db.sql( - """ - select sum(mr_item.stock_qty - mr_item.ordered_qty) - from `tabMaterial Request Item` mr_item, `tabMaterial Request` mr - where mr_item.item_code=%s and mr_item.warehouse=%s - and mr.material_request_type = 'Material Issue' - and mr_item.stock_qty > mr_item.ordered_qty and mr_item.parent=mr.name - and mr.status!='Stopped' and mr.docstatus=1 - """, - (item_code, warehouse), + outward_qty = ( + frappe.qb.from_(mr_item) + .inner_join(mr) + .on(mr_item.parent == mr.name) + .select(Sum(mr_item.stock_qty - mr_item.ordered_qty)) + .where(base_conditions & (mr.material_request_type == "Material Issue")) + .run() ) outward_qty = flt(outward_qty[0][0]) if outward_qty else 0 @@ -248,12 +254,18 @@ def get_subcontracting_order_qty(item_code, warehouse): def get_planned_qty(item_code, warehouse): - planned_qty = frappe.db.sql( - """ - select sum(qty - produced_qty) from `tabWork Order` - where production_item = %s and fg_warehouse = %s and status not in ('Stopped', 'Completed', 'Closed') - and docstatus=1 and qty > produced_qty""", - (item_code, warehouse), + wo = frappe.qb.DocType("Work Order") + planned_qty = ( + frappe.qb.from_(wo) + .select(Sum(wo.qty - wo.produced_qty)) + .where( + (wo.production_item == item_code) + & (wo.fg_warehouse == warehouse) + & wo.status.notin(["Stopped", "Completed", "Closed"]) + & (wo.docstatus == 1) + & (wo.qty > wo.produced_qty) + ) + .run() ) return flt(planned_qty[0][0]) if planned_qty else 0 @@ -284,27 +296,32 @@ def set_stock_balance_as_per_serial_no( if not posting_time: posting_time = nowtime() - condition = " and item.name=%s" % frappe.db.escape(item_code, percent=False) if item_code else "" - - bin = frappe.db.sql( - """select bin.item_code, bin.warehouse, bin.actual_qty, item.stock_uom - from `tabBin` bin, tabItem item - where bin.item_code = item.name and item.has_serial_no = 1 %s""" - % condition + bin_dt = frappe.qb.DocType("Bin") + item = frappe.qb.DocType("Item") + query = ( + frappe.qb.from_(bin_dt) + .inner_join(item) + .on(bin_dt.item_code == item.name) + .select(bin_dt.item_code, bin_dt.warehouse, bin_dt.actual_qty, item.stock_uom) + .where(item.has_serial_no == 1) ) + if item_code: + query = query.where(item.name == item_code) + bin = query.run() for d in bin: - serial_nos = frappe.db.sql( - """select count(name) from `tabSerial No` - where item_code=%s and warehouse=%s and docstatus < 2""", - (d[0], d[1]), + serial_nos = frappe.db.count( + "Serial No", {"item_code": d[0], "warehouse": d[1], "docstatus": ["<", 2]} ) - sle = frappe.db.sql( - """select valuation_rate, company from `tabStock Ledger Entry` - where item_code = %s and warehouse = %s and is_cancelled = 0 - order by posting_date desc limit 1""", - (d[0], d[1]), + sle = frappe.get_all( + "Stock Ledger Entry", + filters={"item_code": d[0], "warehouse": d[1], "is_cancelled": 0}, + fields=["valuation_rate", "company"], + # total order so the latest SLE is picked identically on both engines (was posting_date only) + order_by="posting_date desc, creation desc, name desc", + limit=1, + as_list=True, ) sle_dict = { @@ -317,9 +334,9 @@ def set_stock_balance_as_per_serial_no( "voucher_type": "Stock Reconciliation (Manual)", "voucher_no": "", "voucher_detail_no": "", - "actual_qty": flt(serial_nos[0][0]) - flt(d[2]), + "actual_qty": flt(serial_nos) - flt(d[2]), "stock_uom": d[3], - "incoming_rate": sle and flt(serial_nos[0][0]) > flt(d[2]) and flt(sle[0][0]) or 0, + "incoming_rate": sle and flt(serial_nos) > flt(d[2]) and flt(sle[0][0]) or 0, "company": sle and cstr(sle[0][1]) or 0, "batch_no": "", "serial_no": "", diff --git a/erpnext/stock/test_stock_balance.py b/erpnext/stock/test_stock_balance.py new file mode 100644 index 00000000000..46f9e0432fb --- /dev/null +++ b/erpnext/stock/test_stock_balance.py @@ -0,0 +1,51 @@ +# Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and Contributors +# License: GNU General Public License v3. See license.txt + + +from erpnext.stock.stock_balance import get_indented_qty, get_reserved_qty +from erpnext.tests.utils import ERPNextTestSuite + + +class TestStockBalance(ERPNextTestSuite): + def test_get_reserved_qty_for_sales_order_item(self): + """get_reserved_qty (converted from a UNION of correlated subqueries) must add a submitted + Sales Order's open qty for the direct SO-item branch. No delivery, so it stays clear of the + unrelated #39 SLE-repost path and runs on Postgres.""" + from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order + + item_code, warehouse = "_Test Item", "_Test Warehouse - _TC" + before = get_reserved_qty(item_code, warehouse) + + make_sales_order(item_code=item_code, qty=10, warehouse=warehouse) # submitted + + self.assertEqual(get_reserved_qty(item_code, warehouse), before + 10) + + def test_get_reserved_qty_for_packed_bundle_item(self): + """The packed-item branch of get_reserved_qty (the correlated-subquery -> inner_join rewrite) + must reserve the bundle component qty against an open Sales Order: 2 bundles x 3 per bundle = 6.""" + from erpnext.selling.doctype.product_bundle.test_product_bundle import make_product_bundle + from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order + from erpnext.stock.doctype.item.test_item import make_item + + warehouse = "_Test Warehouse - _TC" + bundle = make_item(properties={"is_stock_item": 0}).name + component = make_item(properties={"is_stock_item": 1}).name + make_product_bundle(bundle, [component], qty=3) + + before = get_reserved_qty(component, warehouse) + + make_sales_order(item_code=bundle, qty=2, warehouse=warehouse) # 2 x 3 = 6 component packed + + self.assertEqual(get_reserved_qty(component, warehouse), before + 6) + + def test_get_indented_qty_for_material_request(self): + """get_indented_qty inward branch (comma-join -> qb inner_join) must reflect a submitted + Purchase Material Request's not-yet-ordered qty.""" + from erpnext.stock.doctype.material_request.test_material_request import make_material_request + + item_code, warehouse = "_Test Item", "_Test Warehouse - _TC" + before = get_indented_qty(item_code, warehouse) + + make_material_request(item_code=item_code, qty=7, warehouse=warehouse) # Purchase, submitted + + self.assertEqual(get_indented_qty(item_code, warehouse), before + 7)