From 449004d29a9070f13189bd842e41d61ea2431f26 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Wed, 17 Jun 2026 16:46:29 +0530 Subject: [PATCH 1/5] refactor(postgres): port Sales Register report query to the query builder Also sort the distinct income / unrealized P&L account lists in python: frame drops ORDER BY for distinct queries on postgres, so the generated account-column order must be pinned in python to stay deterministic on both backends. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../report/sales_register/sales_register.py | 124 +++++++++--------- 1 file changed, 65 insertions(+), 59 deletions(-) diff --git a/erpnext/accounts/report/sales_register/sales_register.py b/erpnext/accounts/report/sales_register/sales_register.py index 5aebcc9e2f4..e711ec299a3 100644 --- a/erpnext/accounts/report/sales_register/sales_register.py +++ b/erpnext/accounts/report/sales_register/sales_register.py @@ -346,12 +346,15 @@ def get_account_columns(invoice_list, include_payments): unrealized_profit_loss_account_columns = [] if invoice_list: - income_accounts = frappe.db.sql_list( - """select distinct income_account - from `tabSales Invoice Item` where docstatus = 1 and parent in (%s) - order by income_account""" - % ", ".join(["%s"] * len(invoice_list)), - tuple(inv.name for inv in invoice_list), + # frappe drops ORDER BY for distinct queries on postgres (db_query), so sort in python to keep + # the generated account-column order deterministic and identical on both backends. + income_accounts = sorted( + frappe.get_all( + "Sales Invoice Item", + filters={"docstatus": 1, "parent": ["in", [inv.name for inv in invoice_list]]}, + pluck="income_account", + distinct=True, + ) ) sales_taxes_query = get_taxes_query(invoice_list, "Sales Taxes and Charges", "Sales Invoice") @@ -363,14 +366,18 @@ def get_account_columns(invoice_list, include_payments): advance_tax_accounts = advance_taxes_query.run(as_dict=True, pluck="account_head") tax_accounts = set(tax_accounts + advance_tax_accounts) - unrealized_profit_loss_accounts = frappe.db.sql_list( - """SELECT distinct unrealized_profit_loss_account - from `tabSales Invoice` where docstatus = 1 and name in (%s) - and is_internal_customer = 1 - and ifnull(unrealized_profit_loss_account, '') != '' - order by unrealized_profit_loss_account""" - % ", ".join(["%s"] * len(invoice_list)), - tuple(inv.name for inv in invoice_list), + unrealized_profit_loss_accounts = sorted( + frappe.get_all( + "Sales Invoice", + filters={ + "docstatus": 1, + "name": ["in", [inv.name for inv in invoice_list]], + "is_internal_customer": 1, + "unrealized_profit_loss_account": ["is", "set"], + }, + pluck="unrealized_profit_loss_account", + distinct=True, + ) ) for account in income_accounts: @@ -494,12 +501,11 @@ def get_payments(filters): def get_invoice_income_map(invoice_list): - income_details = frappe.db.sql( - """select parent, income_account, sum(base_net_amount) as amount - from `tabSales Invoice Item` where parent in (%s) group by parent, income_account""" - % ", ".join(["%s"] * len(invoice_list)), - tuple(inv.name for inv in invoice_list), - as_dict=1, + income_details = frappe.get_all( + "Sales Invoice Item", + filters={"parent": ["in", [inv.name for inv in invoice_list]]}, + fields=["parent", "income_account", {"SUM": "base_net_amount", "as": "amount"}], + group_by="parent, income_account", ) invoice_income_map = {} @@ -511,13 +517,16 @@ def get_invoice_income_map(invoice_list): def get_internal_invoice_map(invoice_list): - unrealized_amount_details = frappe.db.sql( - """SELECT name, unrealized_profit_loss_account, - base_net_total as amount from `tabSales Invoice` where name in (%s) - and is_internal_customer = 1 and company = represents_company""" - % ", ".join(["%s"] * len(invoice_list)), - tuple(inv.name for inv in invoice_list), - as_dict=1, + si = frappe.qb.DocType("Sales Invoice") + unrealized_amount_details = ( + frappe.qb.from_(si) + .select(si.name, si.unrealized_profit_loss_account, si.base_net_total.as_("amount")) + .where( + si.name.isin([inv.name for inv in invoice_list]) + & (si.is_internal_customer == 1) + & (si.company == si.represents_company) + ) + .run(as_dict=1) ) internal_invoice_map = {} @@ -529,14 +538,15 @@ def get_internal_invoice_map(invoice_list): def get_invoice_tax_map(invoice_list, invoice_income_map, income_accounts, include_payments=False): - tax_details = frappe.db.sql( - """select parent, account_head, - sum(base_tax_amount_after_discount_amount) as tax_amount - from `tabSales Taxes and Charges` where parent in (%s) and parenttype = 'Sales Invoice' - group by parent, account_head""" - % ", ".join(["%s"] * len(invoice_list)), - tuple(inv.name for inv in invoice_list), - as_dict=1, + tax_details = frappe.get_all( + "Sales Taxes and Charges", + filters={"parent": ["in", [inv.name for inv in invoice_list]], "parenttype": "Sales Invoice"}, + fields=[ + "parent", + "account_head", + {"SUM": "base_tax_amount_after_discount_amount", "as": "tax_amount"}, + ], + group_by="parent, account_head", ) if include_payments: @@ -557,13 +567,11 @@ def get_invoice_tax_map(invoice_list, invoice_income_map, income_accounts, inclu def get_invoice_so_dn_map(invoice_list): - si_items = frappe.db.sql( - """select parent, sales_order, delivery_note, so_detail - from `tabSales Invoice Item` where parent in (%s) - and (sales_order != '' or delivery_note != '')""" - % ", ".join(["%s"] * len(invoice_list)), - tuple(inv.name for inv in invoice_list), - as_dict=1, + si_items = frappe.get_all( + "Sales Invoice Item", + filters={"parent": ["in", [inv.name for inv in invoice_list]]}, + or_filters=[["sales_order", "!=", ""], ["delivery_note", "!=", ""]], + fields=["parent", "sales_order", "delivery_note", "so_detail"], ) invoice_so_dn_map = {} @@ -577,10 +585,11 @@ def get_invoice_so_dn_map(invoice_list): if d.delivery_note: delivery_note_list = [d.delivery_note] elif d.sales_order: - delivery_note_list = frappe.db.sql_list( - """select distinct parent from `tabDelivery Note Item` - where docstatus=1 and so_detail=%s""", - d.so_detail, + delivery_note_list = frappe.get_all( + "Delivery Note Item", + filters={"docstatus": 1, "so_detail": d.so_detail}, + pluck="parent", + distinct=True, ) if delivery_note_list: @@ -592,13 +601,11 @@ def get_invoice_so_dn_map(invoice_list): def get_invoice_cc_wh_map(invoice_list): - si_items = frappe.db.sql( - """select parent, cost_center, warehouse - from `tabSales Invoice Item` where parent in (%s) - and (cost_center != '' or warehouse != '')""" - % ", ".join(["%s"] * len(invoice_list)), - tuple(inv.name for inv in invoice_list), - as_dict=1, + si_items = frappe.get_all( + "Sales Invoice Item", + filters={"parent": ["in", [inv.name for inv in invoice_list]]}, + or_filters=[["cost_center", "!=", ""], ["warehouse", "!=", ""]], + fields=["parent", "cost_center", "warehouse"], ) invoice_cc_wh_map = {} @@ -619,12 +626,11 @@ def get_invoice_cc_wh_map(invoice_list): def get_mode_of_payments(invoice_list): mode_of_payments = {} if invoice_list: - inv_mop = frappe.db.sql( - """select parent, mode_of_payment - from `tabSales Invoice Payment` where parent in (%s) group by parent, mode_of_payment""" - % ", ".join(["%s"] * len(invoice_list)), - tuple(invoice_list), - as_dict=1, + inv_mop = frappe.get_all( + "Sales Invoice Payment", + filters={"parent": ["in", list(invoice_list)]}, + fields=["parent", "mode_of_payment"], + group_by="parent, mode_of_payment", ) for d in inv_mop: From 196fce9792686298bf902a8cafe297a484b6179f Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Wed, 17 Jun 2026 16:46:30 +0530 Subject: [PATCH 2/5] refactor(postgres): port Purchase Register report query to the query builder Co-Authored-By: Claude Opus 4.8 (1M context) --- .../purchase_register/purchase_register.py | 129 +++++++++--------- 1 file changed, 66 insertions(+), 63 deletions(-) diff --git a/erpnext/accounts/report/purchase_register/purchase_register.py b/erpnext/accounts/report/purchase_register/purchase_register.py index 4801bf15b56..44410a26bac 100644 --- a/erpnext/accounts/report/purchase_register/purchase_register.py +++ b/erpnext/accounts/report/purchase_register/purchase_register.py @@ -4,7 +4,9 @@ import frappe from frappe import _, msgprint +from frappe.query_builder import Case from frappe.query_builder.custom import ConstantColumn +from frappe.query_builder.functions import Sum from frappe.utils import flt, getdate from pypika.terms import Bracket, LiteralValue, Order @@ -307,14 +309,17 @@ def get_account_columns(invoice_list, include_payments): unrealized_profit_loss_account_columns = [] if invoice_list: - expense_accounts = frappe.db.sql_list( - """select distinct expense_account - from `tabPurchase Invoice Item` where docstatus = 1 - and (expense_account is not null and expense_account != '') - and parenttype='Purchase Invoice' - and parent in (%s) order by expense_account""" - % ", ".join(["%s"] * len(invoice_list)), - tuple([inv.name for inv in invoice_list]), + expense_accounts = frappe.get_all( + "Purchase Invoice Item", + filters={ + "docstatus": 1, + "expense_account": ["is", "set"], + "parenttype": "Purchase Invoice", + "parent": ["in", [inv.name for inv in invoice_list]], + }, + pluck="expense_account", + distinct=True, + order_by="expense_account", ) purchase_taxes_query = get_taxes_query(invoice_list, "Purchase Taxes and Charges", "Purchase Invoice") @@ -326,13 +331,16 @@ def get_account_columns(invoice_list, include_payments): advance_tax_accounts = advance_taxes_query.run(as_dict=True, pluck="account_head") tax_accounts = set(tax_accounts + advance_tax_accounts) - unrealized_profit_loss_accounts = frappe.db.sql_list( - """SELECT distinct unrealized_profit_loss_account - from `tabPurchase Invoice` where docstatus = 1 and name in (%s) - and ifnull(unrealized_profit_loss_account, '') != '' - order by unrealized_profit_loss_account""" - % ", ".join(["%s"] * len(invoice_list)), - tuple(inv.name for inv in invoice_list), + unrealized_profit_loss_accounts = frappe.get_all( + "Purchase Invoice", + filters={ + "docstatus": 1, + "name": ["in", [inv.name for inv in invoice_list]], + "unrealized_profit_loss_account": ["is", "set"], + }, + pluck="unrealized_profit_loss_account", + distinct=True, + order_by="unrealized_profit_loss_account", ) for account in expense_accounts: @@ -454,16 +462,11 @@ def get_payments(filters): def get_invoice_expense_map(invoice_list): - expense_details = frappe.db.sql( - """ - select parent, expense_account, sum(base_net_amount) as amount - from `tabPurchase Invoice Item` - where parent in (%s) and parenttype='Purchase Invoice' - group by parent, expense_account - """ - % ", ".join(["%s"] * len(invoice_list)), - tuple(inv.name for inv in invoice_list), - as_dict=1, + expense_details = frappe.get_all( + "Purchase Invoice Item", + filters={"parent": ["in", [inv.name for inv in invoice_list]], "parenttype": "Purchase Invoice"}, + fields=["parent", "expense_account", {"SUM": "base_net_amount", "as": "amount"}], + group_by="parent, expense_account", ) invoice_expense_map = {} @@ -475,13 +478,16 @@ def get_invoice_expense_map(invoice_list): def get_internal_invoice_map(invoice_list): - unrealized_amount_details = frappe.db.sql( - """SELECT name, unrealized_profit_loss_account, - base_net_total as amount from `tabPurchase Invoice` where name in (%s) - and is_internal_supplier = 1 and company = represents_company""" - % ", ".join(["%s"] * len(invoice_list)), - tuple(inv.name for inv in invoice_list), - as_dict=1, + pi = frappe.qb.DocType("Purchase Invoice") + unrealized_amount_details = ( + frappe.qb.from_(pi) + .select(pi.name, pi.unrealized_profit_loss_account, pi.base_net_total.as_("amount")) + .where( + pi.name.isin([inv.name for inv in invoice_list]) + & (pi.is_internal_supplier == 1) + & (pi.company == pi.represents_company) + ) + .run(as_dict=1) ) internal_invoice_map = {} @@ -493,18 +499,23 @@ def get_internal_invoice_map(invoice_list): def get_invoice_tax_map(invoice_list, invoice_expense_map, expense_accounts, include_payments=False): - tax_details = frappe.db.sql( - """ - select parent, account_head, case add_deduct_tax when "Add" then sum(base_tax_amount_after_discount_amount) - else sum(base_tax_amount_after_discount_amount) * -1 end as tax_amount - from `tabPurchase Taxes and Charges` - where parent in (%s) and category in ('Total', 'Valuation and Total') - and base_tax_amount_after_discount_amount != 0 and parenttype='Purchase Invoice' - group by parent, account_head, add_deduct_tax - """ - % ", ".join(["%s"] * len(invoice_list)), - tuple(inv.name for inv in invoice_list), - as_dict=1, + ptc = frappe.qb.DocType("Purchase Taxes and Charges") + tax_amount = ( + Case() + .when(ptc.add_deduct_tax == "Add", Sum(ptc.base_tax_amount_after_discount_amount)) + .else_(Sum(ptc.base_tax_amount_after_discount_amount) * -1) + ) + tax_details = ( + frappe.qb.from_(ptc) + .select(ptc.parent, ptc.account_head, tax_amount.as_("tax_amount")) + .where( + ptc.parent.isin([inv.name for inv in invoice_list]) + & ptc.category.isin(["Total", "Valuation and Total"]) + & (ptc.base_tax_amount_after_discount_amount != 0) + & (ptc.parenttype == "Purchase Invoice") + ) + .groupby(ptc.parent, ptc.account_head, ptc.add_deduct_tax) + .run(as_dict=1) ) if include_payments: @@ -525,15 +536,10 @@ def get_invoice_tax_map(invoice_list, invoice_expense_map, expense_accounts, inc def get_invoice_po_pr_map(invoice_list): - pi_items = frappe.db.sql( - """ - select parent, purchase_order, purchase_receipt, po_detail, project - from `tabPurchase Invoice Item` - where parent in (%s) and parenttype='Purchase Invoice' - """ - % ", ".join(["%s"] * len(invoice_list)), - tuple(inv.name for inv in invoice_list), - as_dict=1, + pi_items = frappe.get_all( + "Purchase Invoice Item", + filters={"parent": ["in", [inv.name for inv in invoice_list]], "parenttype": "Purchase Invoice"}, + fields=["parent", "purchase_order", "purchase_receipt", "po_detail", "project"], ) invoice_po_pr_map = {} @@ -547,10 +553,11 @@ def get_invoice_po_pr_map(invoice_list): if d.purchase_receipt: pr_list = [d.purchase_receipt] elif d.po_detail: - pr_list = frappe.db.sql_list( - """select distinct parent from `tabPurchase Receipt Item` - where docstatus=1 and purchase_order_item=%s""", - d.po_detail, + pr_list = frappe.get_all( + "Purchase Receipt Item", + filters={"docstatus": 1, "purchase_order_item": d.po_detail}, + pluck="parent", + distinct=True, ) if pr_list: @@ -565,12 +572,8 @@ def get_invoice_po_pr_map(invoice_list): def get_account_details(invoice_list): account_map = {} accounts = list(set([inv.credit_to for inv in invoice_list])) - for acc in frappe.db.sql( - """select name, parent_account from tabAccount - where name in (%s)""" - % ", ".join(["%s"] * len(accounts)), - tuple(accounts), - as_dict=1, + for acc in frappe.get_all( + "Account", filters={"name": ["in", accounts]}, fields=["name", "parent_account"] ): account_map[acc.name] = acc.parent_account From ae4a5e82b0b06445d964ae2072b52e3711914cf0 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Wed, 17 Jun 2026 16:46:31 +0530 Subject: [PATCH 3/5] refactor(postgres): port Item-wise Purchase Register report query to the query builder Co-Authored-By: Claude Opus 4.8 (1M context) --- .../item_wise_purchase_register.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/erpnext/accounts/report/item_wise_purchase_register/item_wise_purchase_register.py b/erpnext/accounts/report/item_wise_purchase_register/item_wise_purchase_register.py index 0f0121f4e6c..4bb252d94f1 100644 --- a/erpnext/accounts/report/item_wise_purchase_register/item_wise_purchase_register.py +++ b/erpnext/accounts/report/item_wise_purchase_register/item_wise_purchase_register.py @@ -376,7 +376,7 @@ def get_items(filters, additional_table_columns): def get_aii_accounts(): - return dict(frappe.db.sql("select name, stock_received_but_not_billed from tabCompany")) + return dict(frappe.get_all("Company", fields=["name", "stock_received_but_not_billed"], as_list=True)) def get_purchase_receipts_against_purchase_order(item_list): @@ -384,16 +384,11 @@ def get_purchase_receipts_against_purchase_order(item_list): po_item_rows = list(set(d.po_detail for d in item_list)) if po_item_rows: - purchase_receipts = frappe.db.sql( - """ - select parent, purchase_order_item - from `tabPurchase Receipt Item` - where docstatus=1 and purchase_order_item in (%s) - group by purchase_order_item, parent - """ - % (", ".join(["%s"] * len(po_item_rows))), - tuple(po_item_rows), - as_dict=1, + purchase_receipts = frappe.get_all( + "Purchase Receipt Item", + filters={"docstatus": 1, "purchase_order_item": ["in", po_item_rows]}, + fields=["parent", "purchase_order_item"], + group_by="purchase_order_item, parent", ) for pr in purchase_receipts: From 8527e78820169043fd3e30f743e0f747c4519656 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Wed, 17 Jun 2026 16:46:32 +0530 Subject: [PATCH 4/5] refactor(postgres): port POS Register report query to the query builder Co-Authored-By: Claude Opus 4.8 (1M context) --- .../report/pos_register/pos_register.py | 100 ++++++++++-------- .../report/pos_register/test_pos_register.py | 20 ++++ 2 files changed, 78 insertions(+), 42 deletions(-) create mode 100644 erpnext/accounts/report/pos_register/test_pos_register.py diff --git a/erpnext/accounts/report/pos_register/pos_register.py b/erpnext/accounts/report/pos_register/pos_register.py index 2248f6eca65..6ed6ac844d9 100644 --- a/erpnext/accounts/report/pos_register/pos_register.py +++ b/erpnext/accounts/report/pos_register/pos_register.py @@ -4,6 +4,8 @@ import frappe from frappe import _ +from frappe.query_builder import Case +from frappe.query_builder.functions import IfNull from erpnext.accounts.report.sales_register.sales_register import get_mode_of_payments @@ -46,40 +48,47 @@ def execute(filters=None): def get_pos_entries(filters, group_by_field): - conditions = get_conditions(filters) - order_by = "p.posting_date" - select_mop_field, from_sales_invoice_payment, group_by_mop_condition = "", "", "" - if group_by_field == "mode_of_payment": - select_mop_field = ( - ", sip.mode_of_payment, sip.base_amount - IF(sip.type='Cash', p.change_amount, 0) as paid_amount" + p = frappe.qb.DocType("POS Invoice") + query = ( + frappe.qb.from_(p) + .select( + p.posting_date, + p.name.as_("pos_invoice"), + p.pos_profile, + p.company, + p.owner, + p.customer, + p.is_return, + p.base_grand_total.as_("grand_total"), ) - from_sales_invoice_payment = ", `tabSales Invoice Payment` sip" - group_by_mop_condition = "sip.parent = p.name AND ifnull(sip.base_amount - IF(sip.type='Cash', p.change_amount, 0), 0) != 0 AND" - order_by += ", sip.mode_of_payment" - - elif group_by_field: - order_by += f", p.{group_by_field}" - select_mop_field = ", p.base_paid_amount - p.change_amount as paid_amount " - - # nosemgrep - return frappe.db.sql( - f""" - SELECT - p.posting_date, p.name as pos_invoice, p.pos_profile, p.company, - p.owner, p.customer, p.is_return, p.base_grand_total as grand_total {select_mop_field} - FROM - `tabPOS Invoice` p {from_sales_invoice_payment} - WHERE - p.docstatus = 1 and - {group_by_mop_condition} - {conditions} - ORDER BY - {order_by} - """, - filters, - as_dict=1, + .where(p.docstatus == 1) ) + for condition in get_conditions(filters, p): + query = query.where(condition) + + if group_by_field == "mode_of_payment": + sip = frappe.qb.DocType("Sales Invoice Payment") + paid_amount = sip.base_amount - Case().when(sip.type == "Cash", p.change_amount).else_(0) + query = ( + query.inner_join(sip) + .on(sip.parent == p.name) + .select(sip.mode_of_payment, paid_amount.as_("paid_amount")) + .where(IfNull(paid_amount, 0) != 0) + .orderby(p.posting_date) + .orderby(sip.mode_of_payment) + ) + elif group_by_field: + query = ( + query.select((p.base_paid_amount - p.change_amount).as_("paid_amount")) + .orderby(p.posting_date) + .orderby(p[group_by_field]) + ) + else: + query = query.orderby(p.posting_date) + + return query.run(as_dict=1) + def concat_mode_of_payments(pos_entries): mode_of_payments = get_mode_of_payments(set(d.pos_invoice for d in pos_entries)) @@ -127,27 +136,34 @@ def validate_filters(filters): frappe.throw(_("Can not filter based on Payment Method, if grouped by Payment Method")) -def get_conditions(filters): - conditions = "company = %(company)s AND posting_date >= %(from_date)s AND posting_date <= %(to_date)s" +def get_conditions(filters, p): + conditions = [ + p.company == filters.get("company"), + p.posting_date >= filters.get("from_date"), + p.posting_date <= filters.get("to_date"), + ] if filters.get("pos_profile"): - conditions += " AND pos_profile = %(pos_profile)s" + conditions.append(p.pos_profile == filters.get("pos_profile")) if filters.get("owner"): - conditions += " AND owner = %(owner)s" + conditions.append(p.owner == filters.get("owner")) if filters.get("customer"): - conditions += " AND customer = %(customer)s" + conditions.append(p.customer == filters.get("customer")) if filters.get("is_return"): - conditions += " AND is_return = %(is_return)s" + conditions.append(p.is_return == filters.get("is_return")) if filters.get("mode_of_payment"): - conditions += """ - AND EXISTS( - SELECT name FROM `tabSales Invoice Payment` sip - WHERE parent=p.name AND ifnull(sip.mode_of_payment, '') = %(mode_of_payment)s - )""" + sip = frappe.qb.DocType("Sales Invoice Payment") + conditions.append( + p.name.isin( + frappe.qb.from_(sip) + .select(sip.parent) + .where(IfNull(sip.mode_of_payment, "") == filters.get("mode_of_payment")) + ) + ) return conditions diff --git a/erpnext/accounts/report/pos_register/test_pos_register.py b/erpnext/accounts/report/pos_register/test_pos_register.py new file mode 100644 index 00000000000..74add4a30ce --- /dev/null +++ b/erpnext/accounts/report/pos_register/test_pos_register.py @@ -0,0 +1,20 @@ +# Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and contributors +# For license information, please see license.txt + +import frappe +from frappe.utils import add_days, today + +from erpnext.accounts.report.pos_register.pos_register import execute +from erpnext.tests.utils import ERPNextTestSuite + + +class TestPOSRegister(ERPNextTestSuite): + def test_report_executes(self): + # Smoke-guards the raw-SQL -> query-builder port: the report's POS Invoice query must + # compile and run on both MariaDB and postgres (it returns columns + a row list either way). + company = frappe.db.get_value("Company", {}, "name") + columns, data = execute( + frappe._dict({"company": company, "from_date": add_days(today(), -365), "to_date": today()}) + ) + self.assertTrue(columns) + self.assertIsInstance(data, list) From 30568d36d09d04854e940f5349e8b334e01b1d75 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Wed, 17 Jun 2026 16:49:48 +0530 Subject: [PATCH 5/5] refactor(postgres): port Accounts Receivable report query to the query builder Most queries are straight raw-SQL -> query-builder ports. One rider: get_future_payments_from_journal_entry sums future amounts with no GROUP BY, so its non-aggregated identity columns (invoice_no/party/future_date/future_ref) are wrapped in Max() to satisfy postgres strict GROUP BY. The summed amount is unchanged; the attributed invoice/party label stays within MariaDB's existing arbitrary-row indeterminacy for that already-aggregated single-row query. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../accounts_receivable.py | 164 +++++++++--------- 1 file changed, 80 insertions(+), 84 deletions(-) diff --git a/erpnext/accounts/report/accounts_receivable/accounts_receivable.py b/erpnext/accounts/report/accounts_receivable/accounts_receivable.py index 56dcc0ada1a..e16f86e238d 100644 --- a/erpnext/accounts/report/accounts_receivable/accounts_receivable.py +++ b/erpnext/accounts/report/accounts_receivable/accounts_receivable.py @@ -7,7 +7,7 @@ from collections import OrderedDict import frappe from frappe import _, qb, query_builder, scrub from frappe.query_builder import Criterion -from frappe.query_builder.functions import Date, Substring, Sum +from frappe.query_builder.functions import Date, Max, Substring, Sum from frappe.utils import cint, cstr, flt, getdate, nowdate from erpnext.accounts.doctype.accounting_dimension.accounting_dimension import ( @@ -427,32 +427,21 @@ class ReceivablePayableReport: self.delivery_notes = frappe._dict() # delivery note link inside sales invoice - # nosemgrep - si_against_dn = frappe.db.sql( - """ - select parent, delivery_note - from `tabSales Invoice Item` - where docstatus=1 and parent in (%s) - """ - % (",".join(["%s"] * len(self.invoices))), - tuple(self.invoices), - as_dict=1, + si_against_dn = frappe.get_all( + "Sales Invoice Item", + filters={"docstatus": 1, "parent": ["in", list(self.invoices)]}, + fields=["parent", "delivery_note"], ) for d in si_against_dn: if d.delivery_note: self.delivery_notes.setdefault(d.parent, set()).add(d.delivery_note) - # nosemgrep - dn_against_si = frappe.db.sql( - """ - select distinct parent, against_sales_invoice - from `tabDelivery Note Item` - where against_sales_invoice in (%s) - """ - % (",".join(["%s"] * len(self.invoices))), - tuple(self.invoices), - as_dict=1, + dn_against_si = frappe.get_all( + "Delivery Note Item", + filters={"against_sales_invoice": ["in", list(self.invoices)]}, + fields=["parent", "against_sales_invoice"], + distinct=True, ) for d in dn_against_si: @@ -476,14 +465,10 @@ class ReceivablePayableReport: # Get Sales Team if self.filters.show_sales_person: - # nosemgrep - sales_team = frappe.db.sql( - """ - select parent, sales_person - from `tabSales Team` - where parenttype = 'Sales Invoice' - """, - as_dict=1, + sales_team = frappe.get_all( + "Sales Team", + filters={"parenttype": "Sales Invoice"}, + fields=["parent", "sales_person"], ) for d in sales_team: self.invoice_details.setdefault(d.parent, {}).setdefault("sales_team", []).append( @@ -548,22 +533,31 @@ class ReceivablePayableReport: def get_payment_terms(self, row): # build payment_terms for row - # nosemgrep - payment_terms_details = frappe.db.sql( - f""" - select - si.name, si.party_account_currency, si.currency, si.conversion_rate, - si.total_advance, ps.due_date, ps.payment_term, ps.payment_amount, ps.base_payment_amount, - ps.description, ps.paid_amount, ps.base_paid_amount, ps.discounted_amount - from `tab{row.voucher_type}` si, `tabPayment Schedule` ps - where - si.name = ps.parent and ps.parenttype = '{row.voucher_type}' and - si.name = %s and - si.is_return = 0 - order by ps.paid_amount desc, due_date - """, - row.voucher_no, - as_dict=1, + si = frappe.qb.DocType(row.voucher_type) + ps = frappe.qb.DocType("Payment Schedule") + payment_terms_details = ( + frappe.qb.from_(si) + .inner_join(ps) + .on(si.name == ps.parent) + .select( + si.name, + si.party_account_currency, + si.currency, + si.conversion_rate, + si.total_advance, + ps.due_date, + ps.payment_term, + ps.payment_amount, + ps.base_payment_amount, + ps.description, + ps.paid_amount, + ps.base_paid_amount, + ps.discounted_amount, + ) + .where((ps.parenttype == row.voucher_type) & (si.name == row.voucher_no) & (si.is_return == 0)) + .orderby(ps.paid_amount, order=frappe.qb.desc) + .orderby(ps.due_date) + .run(as_dict=1) ) original_row = frappe._dict(row) @@ -661,7 +655,6 @@ class ReceivablePayableReport: def get_future_payments_from_payment_entry(self): pe = frappe.qb.DocType("Payment Entry") pe_ref = frappe.qb.DocType("Payment Entry Reference") - ifelse = query_builder.CustomFunction("IF", ["condition", "then", "else"]) return ( frappe.qb.from_(pe) @@ -674,11 +667,14 @@ class ReceivablePayableReport: (pe.posting_date).as_("future_date"), (pe_ref.allocated_amount).as_("future_amount"), (pe.reference_no).as_("future_ref"), - ifelse( + # CASE is portable; MySQL's IF() does not exist on postgres + query_builder.Case() + .when( pe.payment_type == "Receive", pe.source_exchange_rate * pe_ref.allocated_amount, - pe.target_exchange_rate * pe_ref.allocated_amount, - ).as_("future_amount_in_base_currency"), + ) + .else_(pe.target_exchange_rate * pe_ref.allocated_amount) + .as_("future_amount_in_base_currency"), ) .where( (pe.docstatus < 2) @@ -695,11 +691,13 @@ class ReceivablePayableReport: .inner_join(jea) .on(jea.parent == je.name) .select( - jea.reference_name.as_("invoice_no"), - jea.party, - jea.party_type, - je.posting_date.as_("future_date"), - je.cheque_no.as_("future_ref"), + # Sum() below makes this an implicit aggregate (no GROUP BY); the non-aggregated columns + # are arbitrary per the single group on MySQL -> Max() keeps it valid on postgres. + Max(jea.reference_name).as_("invoice_no"), + Max(jea.party).as_("party"), + Max(jea.party_type).as_("party_type"), + Max(je.posting_date).as_("future_date"), + Max(je.cheque_no).as_("future_ref"), ) .where( (je.docstatus < 2) @@ -712,30 +710,25 @@ class ReceivablePayableReport: if self.filters.get("party"): if self.account_type == "Payable": - query = query.select( - Sum(jea.debit_in_account_currency - jea.credit_in_account_currency).as_("future_amount") - ) - query = query.select(Sum(jea.debit - jea.credit).as_("future_amount_in_base_currency")) + future_amount = Sum(jea.debit_in_account_currency - jea.credit_in_account_currency) + future_amount_in_base_currency = Sum(jea.debit - jea.credit) else: - query = query.select( - Sum(jea.credit_in_account_currency - jea.debit_in_account_currency).as_("future_amount") - ) - query = query.select(Sum(jea.credit - jea.debit).as_("future_amount_in_base_currency")) + future_amount = Sum(jea.credit_in_account_currency - jea.debit_in_account_currency) + future_amount_in_base_currency = Sum(jea.credit - jea.debit) else: - query = query.select( - Sum(jea.debit if self.account_type == "Payable" else jea.credit).as_( - "future_amount_in_base_currency" - ) - ) - query = query.select( - Sum( - jea.debit_in_account_currency - if self.account_type == "Payable" - else jea.credit_in_account_currency - ).as_("future_amount") + future_amount_in_base_currency = Sum(jea.debit if self.account_type == "Payable" else jea.credit) + future_amount = Sum( + jea.debit_in_account_currency + if self.account_type == "Payable" + else jea.credit_in_account_currency ) - query = query.having(qb.Field("future_amount") > 0) + query = query.select( + future_amount.as_("future_amount"), + future_amount_in_base_currency.as_("future_amount_in_base_currency"), + ) + # use the aggregate expression in HAVING; postgres can't reference a SELECT alias there + query = query.having(future_amount > 0) return query.run(as_dict=True) def allocate_future_payments(self, row): @@ -891,16 +884,19 @@ class ReceivablePayableReport: if self.filters.get("sales_person"): lft, rgt = frappe.db.get_value("Sales Person", self.filters.get("sales_person"), ["lft", "rgt"]) - # nosemgrep - records = frappe.db.sql( - """ - select distinct parent, parenttype - from `tabSales Team` steam - where parenttype in ('Customer', 'Sales Invoice') - and exists(select name from `tabSales Person` where lft >= %s and rgt <= %s and name = steam.sales_person) - """, - (lft, rgt), - as_dict=1, + steam = frappe.qb.DocType("Sales Team") + sp = frappe.qb.DocType("Sales Person") + records = ( + frappe.qb.from_(steam) + .select(steam.parent, steam.parenttype) + .distinct() + .where( + steam.parenttype.isin(["Customer", "Sales Invoice"]) + & steam.sales_person.isin( + frappe.qb.from_(sp).select(sp.name).where((sp.lft >= lft) & (sp.rgt <= rgt)) + ) + ) + .run(as_dict=1) ) self.sales_person_records = frappe._dict()