From 3f360dde3aef188cbefa0e366e0082d95e926583 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Sun, 21 Jun 2026 06:01:13 +0530 Subject: [PATCH] fix(stock): convert stock_ledger raw SQL to qb + case-insensitive serial match (Postgres) Convert four raw frappe.db.sql statements to frappe.qb: - set_as_cancel (UPDATE -> frappe.qb.update) - the invalid-serial-no incoming_rate lookup - get_valuation_rate's last-valuation lookup - get_future_sle_with_negative_qty The serial-no comparisons (invalid-serial lookup and the get_stock_ledger_entries condition builder, which stays raw) are wrapped in lower()/Lower() so serial matching is case-insensitive on Postgres too -- MariaDB's collation already is, so this is a no-op there. Deterministic creation/name tiebreakers are added to the "ORDER BY posting_date DESC LIMIT 1" lookups so Postgres picks the same row MariaDB did. Surgical re-apply (not a whole-file port): develop's reposting valuation-recalc clause (`recalculate_valuation_rate`) in update_entries_after and the already-shipped Min()-wrapped get_items_to_be_repost GROUP BY are preserved. The dynamic-condition / row-locking raw queries (get_previous_sle, get_stock_ledger_entries builder, get_future_sle_with_negative_batch_qty, the qty_shift UPDATE) are intentionally left raw. Verified: full test_stock_ledger_entry suite 22/22 on MariaDB; added focused tests for set_as_cancel / get_valuation_rate / get_future_sle_with_negative_qty that pass on MariaDB and Postgres. Co-Authored-By: Claude Opus 4.8 (1M context) --- erpnext/stock/stock_ledger.py | 131 +++++++++++++---------- erpnext/stock/tests/test_stock_ledger.py | 66 ++++++++++++ 2 files changed, 138 insertions(+), 59 deletions(-) create mode 100644 erpnext/stock/tests/test_stock_ledger.py diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index 99dd4ae675f..f34b176b6cc 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -10,7 +10,7 @@ import frappe from frappe import _, bold, scrub from frappe.model.meta import get_field_precision from frappe.query_builder import Order -from frappe.query_builder.functions import Sum +from frappe.query_builder.functions import Lower, Sum from frappe.utils import ( cint, flt, @@ -185,12 +185,14 @@ def validate_cancellation(kargs): def set_as_cancel(voucher_type, voucher_no): - frappe.db.sql( - """update `tabStock Ledger Entry` set is_cancelled=1, - modified=%s, modified_by=%s - where voucher_type=%s and voucher_no=%s and is_cancelled = 0""", - (now(), frappe.session.user, voucher_type, voucher_no), - ) + sle = frappe.qb.DocType("Stock Ledger Entry") + ( + frappe.qb.update(sle) + .set(sle.is_cancelled, 1) + .set(sle.modified, now()) + .set(sle.modified_by, frappe.session.user) + .where((sle.voucher_type == voucher_type) & (sle.voucher_no == voucher_no) & (sle.is_cancelled == 0)) + ).run() def make_entry(args, allow_negative_stock=False, via_landed_cost_voucher=False): @@ -1481,25 +1483,27 @@ class update_entries_after: # Get rate for serial nos which has been transferred to other company invalid_serial_nos = [d.name for d in all_serial_nos if d.company != sle.company] + sle_entry = frappe.qb.DocType("Stock Ledger Entry") for serial_no in invalid_serial_nos: - incoming_rate = frappe.db.sql( - """ - select incoming_rate - from `tabStock Ledger Entry` - where - company = %s - and actual_qty > 0 - and is_cancelled = 0 - and (serial_no = %s - or serial_no like %s - or serial_no like %s - or serial_no like %s + incoming_rate = ( + frappe.qb.from_(sle_entry) + .select(sle_entry.incoming_rate) + .where( + (sle_entry.company == sle.company) + & (sle_entry.actual_qty > 0) + & (sle_entry.is_cancelled == 0) + & ( + (Lower(sle_entry.serial_no) == serial_no.lower()) + | Lower(sle_entry.serial_no).like((serial_no + "\n%").lower()) + | Lower(sle_entry.serial_no).like(("%\n" + serial_no).lower()) + | Lower(sle_entry.serial_no).like(("%\n" + serial_no + "\n%").lower()) ) - order by posting_date desc - limit 1 - """, - (sle.company, serial_no, serial_no + "\n%", "%\n" + serial_no, "%\n" + serial_no + "\n%"), - ) + ) + .orderby(sle_entry.posting_date, order=frappe.qb.desc) + .orderby(sle_entry.creation, order=frappe.qb.desc) + .orderby(sle_entry.name, order=frappe.qb.desc) + .limit(1) + ).run() incoming_values += flt(incoming_rate[0][0]) if incoming_rate else 0 @@ -1869,13 +1873,16 @@ def get_stock_ledger_entries( if check_serial_no and previous_sle.get("serial_no"): # conditions += " and serial_no like {}".format(frappe.db.escape('%{0}%'.format(previous_sle.get("serial_no")))) serial_no = previous_sle.get("serial_no") + # lower() both sides so the match is case-insensitive on postgres too (MariaDB's collation + # already is); a no-op on MariaDB. The set is already narrowed by item_code/warehouse, so the + # functional comparison does not cost an index here. conditions += ( """ and ( - serial_no = {} - or serial_no like {} - or serial_no like {} - or serial_no like {} + lower(serial_no) = lower({}) + or lower(serial_no) like lower({}) + or lower(serial_no) like lower({}) + or lower(serial_no) like lower({}) ) """ ).format( @@ -1990,18 +1997,21 @@ def get_valuation_rate( return batch_obj.get_incoming_rate() # Get valuation rate from last sle for the same item and warehouse - if last_valuation_rate := frappe.db.sql( # nosemgrep - """select valuation_rate - from `tabStock Ledger Entry` - where - item_code = %s - AND warehouse = %s - AND valuation_rate >= 0 - AND is_cancelled = 0 - AND NOT (voucher_no = %s AND voucher_type = %s) - order by posting_datetime desc, creation desc limit 1""", - (item_code, warehouse, voucher_no, voucher_type), - ): + sle_entry = frappe.qb.DocType("Stock Ledger Entry") + if last_valuation_rate := ( + frappe.qb.from_(sle_entry) + .select(sle_entry.valuation_rate) + .where( + (sle_entry.item_code == item_code) + & (sle_entry.warehouse == warehouse) + & (sle_entry.valuation_rate >= 0) + & (sle_entry.is_cancelled == 0) + & ~((sle_entry.voucher_no == voucher_no) & (sle_entry.voucher_type == voucher_type)) + ) + .orderby(sle_entry.posting_datetime, order=frappe.qb.desc) + .orderby(sle_entry.creation, order=frappe.qb.desc) + .limit(1) + ).run(): return flt(last_valuation_rate[0][0]) if fallbacks: @@ -2233,25 +2243,28 @@ def is_negative_with_precision(neg_sle, is_batch=False): def get_future_sle_with_negative_qty(sle_args): - return frappe.db.sql( # nosemgrep - """ - select - qty_after_transaction, posting_date, posting_time, - voucher_type, voucher_no - from `tabStock Ledger Entry` - where - item_code = %(item_code)s - and warehouse = %(warehouse)s - and voucher_no != %(voucher_no)s - and posting_datetime >= %(posting_datetime)s - and is_cancelled = 0 - and qty_after_transaction < 0 - order by posting_datetime asc, creation asc - limit 1 - """, - sle_args, - as_dict=1, - ) + sle = frappe.qb.DocType("Stock Ledger Entry") + return ( + frappe.qb.from_(sle) + .select( + sle.qty_after_transaction, + sle.posting_date, + sle.posting_time, + sle.voucher_type, + sle.voucher_no, + ) + .where( + (sle.item_code == sle_args["item_code"]) + & (sle.warehouse == sle_args["warehouse"]) + & (sle.voucher_no != sle_args["voucher_no"]) + & (sle.posting_datetime >= sle_args["posting_datetime"]) + & (sle.is_cancelled == 0) + & (sle.qty_after_transaction < 0) + ) + .orderby(sle.posting_datetime) + .orderby(sle.creation) + .limit(1) + ).run(as_dict=1) def get_future_sle_with_negative_batch_qty(sle_args): diff --git a/erpnext/stock/tests/test_stock_ledger.py b/erpnext/stock/tests/test_stock_ledger.py new file mode 100644 index 00000000000..a47e348c803 --- /dev/null +++ b/erpnext/stock/tests/test_stock_ledger.py @@ -0,0 +1,66 @@ +# Copyright (c) 2025, Frappe Technologies Pvt. Ltd. and Contributors +# See license.txt + +import frappe + +from erpnext.tests.utils import ERPNextTestSuite + + +class TestStockLedgerConversions(ERPNextTestSuite): + """Exercises the stock_ledger.py raw-SQL -> query-builder conversions on both engines.""" + + def test_set_as_cancel_marks_entries_cancelled(self): + # set_as_cancel runs an UPDATE (raw SQL -> frappe.qb.update) marking the voucher's SLEs + # is_cancelled=1. Cancelling a receipt exercises it. + from erpnext.stock.doctype.item.test_item import make_item + from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry + + item = make_item("_Test SL Cancel Item", {"is_stock_item": 1}).name + se = make_stock_entry(item_code=item, target="_Test Warehouse - _TC", qty=5, basic_rate=100) + # register cleanup before the assertions so the entry is removed even if one fails + self.addCleanup(self._cancel_and_delete, "Stock Entry", se.name) + + self.assertTrue(frappe.db.exists("Stock Ledger Entry", {"voucher_no": se.name, "is_cancelled": 0})) + + se.cancel() + + self.assertFalse(frappe.db.exists("Stock Ledger Entry", {"voucher_no": se.name, "is_cancelled": 0})) + self.assertTrue(frappe.db.exists("Stock Ledger Entry", {"voucher_no": se.name, "is_cancelled": 1})) + + def test_get_valuation_rate_returns_last_sle_rate(self): + # get_valuation_rate's last-valuation lookup (raw SQL -> frappe.qb) returns the most recent + # valuation_rate for the item+warehouse. + from erpnext.stock.doctype.item.test_item import make_item + from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry + from erpnext.stock.stock_ledger import get_valuation_rate + + item = make_item("_Test SL Valuation Item", {"is_stock_item": 1}).name + se = make_stock_entry(item_code=item, target="_Test Warehouse - _TC", qty=10, basic_rate=250) + self.addCleanup(self._cancel_and_delete, "Stock Entry", se.name) + + rate = get_valuation_rate(item, "_Test Warehouse - _TC", "Stock Entry", "_TEST-NO-SUCH-VOUCHER") + self.assertEqual(rate, 250) + + def test_get_future_sle_with_negative_qty_runs(self): + # get_future_sle_with_negative_qty (raw SQL -> frappe.qb) must execute on both engines. With no + # negative future entry it returns an empty result; this guards the converted query's validity. + from frappe.utils import now_datetime + + from erpnext.stock.stock_ledger import get_future_sle_with_negative_qty + + args = { + "item_code": "_Test Item", + "warehouse": "_Test Warehouse - _TC", + "voucher_no": "_TEST-NO-SUCH-VOUCHER", + "posting_datetime": now_datetime(), + } + self.assertIsInstance(get_future_sle_with_negative_qty(args), list | tuple) + + @staticmethod + def _cancel_and_delete(doctype, name): + if not frappe.db.exists(doctype, name): + return + doc = frappe.get_doc(doctype, name) + if doc.docstatus == 1: + doc.cancel() + frappe.delete_doc(doctype, name, force=1)