From d8a2f53a29e3859c3f555442b817ad5704ede2fc Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Thu, 18 Jun 2026 21:01:11 +0530 Subject: [PATCH 1/2] refactor(postgres): port accounts doctypes & match-condition helper to the query builder Pure MariaDB-identical conversion for Postgres compatibility. Split out of #56082. bank_clearance, pos_closing_entry, process_statement_of_accounts, party, utils, and sales_invoice/services/pos converted to frappe.qb; bundles erpnext/utilities/query.py (the get_match_conditions_qb helper, frappe#40075 follow-up) which process_statement_of_accounts depends on. No behaviour change on MariaDB. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../doctype/bank_clearance/bank_clearance.py | 31 ++-- .../pos_closing_entry/pos_closing_entry.py | 8 +- .../process_statement_of_accounts.py | 69 +++---- .../doctype/sales_invoice/services/pos.py | 3 +- erpnext/accounts/party.py | 15 +- erpnext/accounts/utils.py | 170 ++++++++++-------- erpnext/utilities/query.py | 96 ++++++++++ erpnext/utilities/test_query.py | 27 +++ 8 files changed, 288 insertions(+), 131 deletions(-) create mode 100644 erpnext/utilities/query.py create mode 100644 erpnext/utilities/test_query.py diff --git a/erpnext/accounts/doctype/bank_clearance/bank_clearance.py b/erpnext/accounts/doctype/bank_clearance/bank_clearance.py index 5b628cd87bf..059abdae1b5 100644 --- a/erpnext/accounts/doctype/bank_clearance/bank_clearance.py +++ b/erpnext/accounts/doctype/bank_clearance/bank_clearance.py @@ -7,7 +7,7 @@ from frappe import _, msgprint from frappe.model.document import Document from frappe.query_builder import Case from frappe.query_builder.custom import ConstantColumn -from frappe.query_builder.functions import Coalesce, Sum +from frappe.query_builder.functions import Coalesce, Max, Sum from frappe.utils import cint, flt, fmt_money, getdate from pypika import Order @@ -195,14 +195,17 @@ def get_payment_entries_for_bank_clearance( .select( ConstantColumn("Journal Entry").as_("payment_document"), journal_entry.name.as_("payment_entry"), - journal_entry.cheque_no.as_("cheque_number"), - journal_entry.cheque_date, + # non-grouped columns are constant per grouped JE name / account (against_account is + # arbitrary per group on MySQL) -> Max() keeps the GROUP BY valid on postgres with the + # same value MySQL picked. + Max(journal_entry.cheque_no).as_("cheque_number"), + Max(journal_entry.cheque_date).as_("cheque_date"), Sum(journal_entry_account.debit_in_account_currency).as_("debit"), Sum(journal_entry_account.credit_in_account_currency).as_("credit"), - journal_entry.posting_date, - journal_entry_account.against_account, - journal_entry.clearance_date, - journal_entry_account.account_currency, + Max(journal_entry.posting_date).as_("posting_date"), + Max(journal_entry_account.against_account).as_("against_account"), + Max(journal_entry.clearance_date).as_("clearance_date"), + Max(journal_entry_account.account_currency).as_("account_currency"), ) .where( (journal_entry_account.account == account) @@ -215,12 +218,13 @@ def get_payment_entries_for_bank_clearance( if not include_reconciled_entries: journal_entry_query = journal_entry_query.where( - (journal_entry.clearance_date.isnull()) | (journal_entry.clearance_date == "0000-00-00") + (journal_entry.clearance_date.isnull()) + | (journal_entry.clearance_date == ("0000-00-00" if frappe.db.db_type != "postgres" else None)) ) journal_entries = ( journal_entry_query.groupby(journal_entry_account.account, journal_entry.name) - .orderby(journal_entry.posting_date) + .orderby(Max(journal_entry.posting_date)) .orderby(journal_entry.name, order=Order.desc) ).run(as_dict=True) @@ -290,7 +294,8 @@ def get_payment_entries_for_bank_clearance( if not include_reconciled_entries: payment_entry_query = payment_entry_query.where( - (pe.clearance_date.isnull()) | (pe.clearance_date == "0000-00-00") + (pe.clearance_date.isnull()) + | (pe.clearance_date == ("0000-00-00" if frappe.db.db_type != "postgres" else None)) ) payment_entries = (payment_entry_query.orderby(pe.posting_date).orderby(pe.name, order=Order.desc)).run( @@ -327,7 +332,8 @@ def get_payment_entries_for_bank_clearance( if not include_reconciled_entries: paid_purchase_invoices_query = paid_purchase_invoices_query.where( - (pi.clearance_date.isnull()) | (pi.clearance_date == "0000-00-00") + (pi.clearance_date.isnull()) + | (pi.clearance_date == ("0000-00-00" if frappe.db.db_type != "postgres" else None)) ) paid_purchase_invoices = ( @@ -367,7 +373,8 @@ def get_payment_entries_for_bank_clearance( if not include_reconciled_entries: pos_sales_invoices_query = pos_sales_invoices_query.where( - (si_payment.clearance_date.isnull()) | (si_payment.clearance_date == "0000-00-00") + (si_payment.clearance_date.isnull()) + | (si_payment.clearance_date == ("0000-00-00" if frappe.db.db_type != "postgres" else None)) ) pos_sales_invoices = ( diff --git a/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.py b/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.py index 566688fd1fe..fff99369ef0 100644 --- a/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.py +++ b/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.py @@ -295,7 +295,7 @@ def get_payments(invoices): .groupby(SalesInvoicePayment.mode_of_payment) .select( SalesInvoicePayment.mode_of_payment, - SalesInvoicePayment.account, + SalesInvoicePayment.account.as_("account"), fn.Sum(SalesInvoicePayment.amount).as_("amount"), ) ) @@ -419,7 +419,7 @@ def build_invoice_query(invoice_doctype, user, pos_profile, start, end): InvoiceDocType.account_for_change_amount, InvoiceDocType.is_return, InvoiceDocType.return_against, - fn.Timestamp(InvoiceDocType.posting_date, InvoiceDocType.posting_time).as_("timestamp"), + fn.CombineDatetime(InvoiceDocType.posting_date, InvoiceDocType.posting_time).as_("timestamp"), ConstantColumn(invoice_doctype).as_("doctype"), ) .where( @@ -428,8 +428,8 @@ def build_invoice_query(invoice_doctype, user, pos_profile, start, end): & (InvoiceDocType.is_pos == 1) & (InvoiceDocType.pos_profile == pos_profile) & ( - (fn.Timestamp(InvoiceDocType.posting_date, InvoiceDocType.posting_time) >= start) - & (fn.Timestamp(InvoiceDocType.posting_date, InvoiceDocType.posting_time) <= end) + (fn.CombineDatetime(InvoiceDocType.posting_date, InvoiceDocType.posting_time) >= start) + & (fn.CombineDatetime(InvoiceDocType.posting_date, InvoiceDocType.posting_time) <= end) ) ) ) diff --git a/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.py b/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.py index 69c1b43f25e..a2dc1d62836 100644 --- a/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.py +++ b/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.py @@ -6,7 +6,6 @@ import copy import frappe from frappe import _ -from frappe.desk.reportview import get_match_cond from frappe.model.document import Document from frappe.utils import add_days, add_months, format_date, getdate, today from frappe.utils.jinja import validate_template @@ -20,6 +19,7 @@ from erpnext.accounts.report.accounts_receivable_summary.accounts_receivable_sum execute as get_ageing, ) from erpnext.accounts.report.general_ledger.general_ledger import execute as get_soa +from erpnext.utilities.query import get_match_conditions_qb class ProcessStatementOfAccounts(Document): @@ -366,15 +366,19 @@ def get_customers_based_on_territory_or_customer_group(customer_collection, coll def get_customers_based_on_sales_person(sales_person): lft, rgt = frappe.db.get_value("Sales Person", sales_person, ["lft", "rgt"]) - records = frappe.db.sql( - """ - select distinct parent, parenttype - from `tabSales Team` steam - where parenttype = 'Customer' - 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 == "Customer") + & steam.sales_person.isin( + frappe.qb.from_(sp).select(sp.name).where((sp.lft >= lft) & (sp.rgt <= rgt)) + ) + ) + .run(as_dict=1) ) sales_person_records = frappe._dict() for d in records: @@ -469,31 +473,30 @@ def get_customer_emails(customer_name: str, primary_mandatory: str | int, billin frappe.has_permission("Customer", "read", customer_name, throw=True) - billing_email = frappe.db.sql( - """ - SELECT - email.email_id - FROM - `tabContact Email` AS email - JOIN - `tabDynamic Link` AS link - ON - email.parent=link.parent - JOIN - `tabContact` AS contact - ON - contact.name=link.parent - WHERE - link.link_doctype='Customer' - and link.link_name=%s - and contact.is_billing_contact=1 - {mcond} - ORDER BY - contact.creation desc - """.format(mcond=get_match_cond("Contact")), - customer_name, + email = frappe.qb.DocType("Contact Email") + link = frappe.qb.DocType("Dynamic Link") + contact = frappe.qb.DocType("Contact") + + query = ( + frappe.qb.from_(email) + .join(link) + .on(email.parent == link.parent) + .join(contact) + .on(contact.name == link.parent) + .select(email.email_id) + .where( + (link.link_doctype == "Customer") + & (link.link_name == customer_name) + & (contact.is_billing_contact == 1) + ) + .orderby(contact.creation, order=frappe.qb.desc) ) + for condition in get_match_conditions_qb("Contact", table=contact): + query = query.where(condition) + + billing_email = query.run() + if len(billing_email) == 0 or (billing_email[0][0] is None): if billing_and_primary: frappe.throw(_("No billing email found for customer: {0}").format(customer_name)) diff --git a/erpnext/accounts/doctype/sales_invoice/services/pos.py b/erpnext/accounts/doctype/sales_invoice/services/pos.py index 4fdcd46aa1d..1b19b7eef32 100644 --- a/erpnext/accounts/doctype/sales_invoice/services/pos.py +++ b/erpnext/accounts/doctype/sales_invoice/services/pos.py @@ -402,7 +402,8 @@ def get_mode_of_payments_info(mode_of_payments: list, company: str) -> dict: .where(ModeOfPaymentAccount.company == company) .where(ModeOfPayment.enabled == 1) .where(ModeOfPayment.name.isin(mode_of_payments)) - .groupby(ModeOfPayment.name) + # group by all selected columns so postgres accepts it (one row per mode of payment) + .groupby(ModeOfPaymentAccount.default_account, ModeOfPaymentAccount.parent, ModeOfPayment.type) ) data = query.run(as_dict=1) diff --git a/erpnext/accounts/party.py b/erpnext/accounts/party.py index 17734462355..7627d8749b9 100644 --- a/erpnext/accounts/party.py +++ b/erpnext/accounts/party.py @@ -900,16 +900,13 @@ def get_dashboard_info(party_type, party, loyalty_program=None): d.company, {"grand_total": d.grand_total, "base_grand_total": d.base_grand_total} ) + gle = frappe.qb.DocType("GL Entry") company_wise_total_unpaid = frappe._dict( - frappe.db.sql( - """ - select company, sum(debit_in_account_currency) - sum(credit_in_account_currency) - from `tabGL Entry` - where party_type = %s and party=%s - and is_cancelled = 0 - group by company""", - (party_type, party), - ) + frappe.qb.from_(gle) + .select(gle.company, Sum(gle.debit_in_account_currency) - Sum(gle.credit_in_account_currency)) + .where((gle.party_type == party_type) & (gle.party == party) & (gle.is_cancelled == 0)) + .groupby(gle.company) + .run() ) for d in companies: diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index e2f78f00116..d9974cf8c71 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -13,7 +13,7 @@ from frappe.desk.reportview import build_match_conditions from frappe.model.meta import get_field_precision from frappe.model.naming import determine_consecutive_week_number from frappe.query_builder import AliasedQuery, Case, Criterion, Field, Table -from frappe.query_builder.functions import Count, IfNull, Max, Round, Sum +from frappe.query_builder.functions import Count, IfNull, Max, Min, Round, Sum from frappe.query_builder.utils import DocType from frappe.utils import ( add_days, @@ -411,10 +411,9 @@ def get_count_on(account, fieldname, date): else: dr_or_cr = "debit" if fieldname == "invoiced_amount" else "credit" cr_or_dr = "credit" if fieldname == "invoiced_amount" else "debit" - select_fields = ( - "ifnull(sum(credit-debit),0)" - if fieldname == "invoiced_amount" - else "ifnull(sum(debit-credit),0)" + gl = frappe.qb.DocType("GL Entry") + amount_expr = ( + Sum(gl.credit - gl.debit) if fieldname == "invoiced_amount" else Sum(gl.debit - gl.credit) ) if ( @@ -422,14 +421,21 @@ def get_count_on(account, fieldname, date): or (gle.against_voucher_type in ["Sales Order", "Purchase Order"]) or (gle.against_voucher == gle.voucher_no and gle.get(dr_or_cr) > 0) ): - payment_amount = frappe.db.sql( - f""" - SELECT {select_fields} - FROM `tabGL Entry` gle - WHERE docstatus < 2 and posting_date <= %(date)s and against_voucher = %(voucher_no)s - and party = %(party)s and name != %(name)s""", - {"date": date, "voucher_no": gle.voucher_no, "party": gle.party, "name": gle.name}, - )[0][0] + payment_amount = ( + ( + frappe.qb.from_(gl) + .select(amount_expr) + .where( + (gl.docstatus < 2) + & (gl.posting_date <= date) + & (gl.against_voucher == gle.voucher_no) + & (gl.party == gle.party) + & (gl.name != gle.name) + ) + .run()[0][0] + ) + or 0 + ) outstanding_amount = flt(gle.get(dr_or_cr)) - flt(gle.get(cr_or_dr)) - payment_amount currency_precision = get_currency_precision() or 2 @@ -1169,26 +1175,27 @@ def get_company_default(company: str, fieldname: str, ignore_validation: bool = def fix_total_debit_credit(): - vouchers = frappe.db.sql( - """select voucher_type, voucher_no, - sum(debit) - sum(credit) as diff - from `tabGL Entry` - group by voucher_type, voucher_no - having sum(debit) != sum(credit)""", - as_dict=1, + gle = frappe.qb.DocType("GL Entry") + vouchers = ( + frappe.qb.from_(gle) + .select(gle.voucher_type, gle.voucher_no, (Sum(gle.debit) - Sum(gle.credit)).as_("diff")) + .groupby(gle.voucher_type, gle.voucher_no) + .having(Sum(gle.debit) != Sum(gle.credit)) + .run(as_dict=1) ) for d in vouchers: if abs(d.diff) > 0: dr_or_cr = d.voucher_type == "Sales Invoice" and "credit" or "debit" - frappe.db.sql( - """update `tabGL Entry` set {} = {} + {} - where voucher_type = {} and voucher_no = {} and {} > 0 limit 1""".format( - dr_or_cr, dr_or_cr, "%s", "%s", "%s", dr_or_cr - ), - (d.diff, d.voucher_type, d.voucher_no), + gle = frappe.qb.DocType("GL Entry") + name = frappe.db.get_value( + "GL Entry", + {"voucher_type": d.voucher_type, "voucher_no": d.voucher_no, dr_or_cr: [">", 0]}, + "name", ) + if name: + frappe.qb.update(gle).set(gle[dr_or_cr], gle[dr_or_cr] + d.diff).where(gle.name == name).run() def get_currency_precision(): @@ -1230,11 +1237,12 @@ def get_held_invoices(party_type, party): held_invoices = None if party_type == "Supplier": - held_invoices = frappe.db.sql( - "select name from `tabPurchase Invoice` where on_hold = 1 and release_date IS NOT NULL and release_date > CURDATE()", - as_dict=1, + held_invoices = frappe.get_all( + "Purchase Invoice", + filters={"on_hold": 1, "release_date": [">", nowdate()]}, + pluck="name", ) - held_invoices = set(d["name"] for d in held_invoices) + held_invoices = set(held_invoices) return held_invoices @@ -1742,13 +1750,15 @@ def sort_stock_vouchers_by_posting_date( sle = frappe.qb.DocType("Stock Ledger Entry") voucher_nos = [v[1] for v in stock_vouchers] + # only voucher_type/voucher_no are used downstream; order by Min() of the (per-voucher constant) + # posting_datetime so postgres accepts the GROUP BY without selecting non-aggregated columns sles = ( frappe.qb.from_(sle) - .select(sle.voucher_type, sle.voucher_no, sle.posting_date, sle.posting_time, sle.creation) + .select(sle.voucher_type, sle.voucher_no) .where((sle.is_cancelled == 0) & (sle.voucher_no.isin(voucher_nos))) .groupby(sle.voucher_type, sle.voucher_no) - .orderby(sle.posting_datetime) - .orderby(sle.creation) + .orderby(Min(sle.posting_datetime)) + .orderby(Min(sle.creation)) ) if company: @@ -1769,25 +1779,37 @@ def get_future_stock_vouchers(posting_date, posting_time, for_warehouses=None, f SLE = DocType("Stock Ledger Entry") + conditions = (SLE.posting_datetime >= posting_datetime) & (SLE.is_cancelled == 0) + if for_items: + conditions &= SLE.item_code.isin(for_items) + if for_warehouses: + conditions &= SLE.warehouse.isin(for_warehouses) + if company: + conditions &= SLE.company == company + + # These SLE rows must stay locked for the duration of the repost so a concurrent stock + # transaction can't modify them mid-flight (the original DISTINCT ... FOR UPDATE did this). + # MariaDB carries the lock on the grouped query below; postgres rejects FOR UPDATE alongside + # GROUP BY, so lock the matching rows in a separate pass first -- the row locks are held until + # the surrounding transaction ends, giving the same protection. + if frappe.db.db_type == "postgres": + frappe.qb.from_(SLE).select(SLE.name).where(conditions).for_update().run() + + # distinct vouchers in chronological order; expressed as GROUP BY + Min() so it's valid on + # postgres (SELECT DISTINCT can't ORDER BY non-selected cols, and FOR UPDATE is invalid with both). + # posting_datetime is constant per voucher, so the ordering is unchanged vs the DISTINCT form. query = ( frappe.qb.from_(SLE) .select(SLE.voucher_type, SLE.voucher_no) - .distinct() - .where(SLE.posting_datetime >= posting_datetime) - .where(SLE.is_cancelled == 0) - .orderby(SLE.posting_datetime) - .orderby(SLE.creation) - .for_update() + .where(conditions) + .groupby(SLE.voucher_type, SLE.voucher_no) + .orderby(Min(SLE.posting_datetime)) + .orderby(Min(SLE.creation)) ) - if for_items: - query = query.where(SLE.item_code.isin(for_items)) - - if for_warehouses: - query = query.where(SLE.warehouse.isin(for_warehouses)) - - if company: - query = query.where(SLE.company == company) + # lock scanned rows on MariaDB; on postgres they were already locked above + if frappe.db.db_type != "postgres": + query = query.for_update() future_stock_vouchers = query.run(as_dict=True) @@ -1809,14 +1831,10 @@ def get_voucherwise_gl_entries(future_stock_vouchers, posting_date): voucher_nos = [d[1] for d in future_stock_vouchers] - gles = frappe.db.sql( - """ - select name, account, credit, debit, cost_center, project, voucher_type, voucher_no - from `tabGL Entry` - where - posting_date >= {} and voucher_no in ({})""".format("%s", ", ".join(["%s"] * len(voucher_nos))), - tuple([posting_date, *voucher_nos]), - as_dict=1, + gles = frappe.get_all( + "GL Entry", + filters={"posting_date": [">=", posting_date], "voucher_no": ["in", voucher_nos]}, + fields=["name", "account", "credit", "debit", "cost_center", "project", "voucher_type", "voucher_no"], ) for d in gles: @@ -2235,7 +2253,7 @@ def delink_original_entry(pl_entry, partial_cancel=False): qb.update(ple) .set(ple.modified, now()) .set(ple.modified_by, frappe.session.user) - .set(ple.delinked, True) + .set(ple.delinked, 1) # smallint column; postgres rejects boolean true .where( (ple.company == pl_entry.company) & (ple.account_type == pl_entry.account_type) @@ -2350,8 +2368,10 @@ class QueryPaymentLedger: .where(Criterion.all(self.dimensions_filter)) .where(Criterion.all(self.voucher_posting_date)) .groupby(ple.against_voucher_type, ple.against_voucher_no, ple.party_type, ple.party) - .orderby(ple.invoice_date, ple.voucher_no) - .having(qb.Field("amount_in_account_currency") > 0) + # order by the select aliases (postgres can't ORDER BY a non-existent ple column) + .orderby(qb.Field("invoice_date"), qb.Field("voucher_no")) + # postgres HAVING can't reference a select alias; use the aggregate expression + .having(Sum(ple.amount_in_account_currency) > 0) .limit(self.limit) .run() ) @@ -2365,18 +2385,21 @@ class QueryPaymentLedger: query_voucher_amount = ( qb.from_(ple) .select( - ple.account, + # columns that are constant per (voucher_type, voucher_no, party_type, party) are + # wrapped in Max() so the query is valid on postgres (which, unlike MariaDB, requires + # every non-aggregated column to be grouped or aggregated) + Max(ple.account).as_("account"), ple.voucher_type, ple.voucher_no, ple.party_type, ple.party, - ple.posting_date, - ple.due_date, - ple.account_currency.as_("currency"), - ple.cost_center.as_("cost_center"), + Max(ple.posting_date).as_("posting_date"), + Max(ple.due_date).as_("due_date"), + Max(ple.account_currency).as_("currency"), + Max(ple.cost_center).as_("cost_center"), Sum(ple.amount).as_("amount"), Sum(ple.amount_in_account_currency).as_("amount_in_account_currency"), - ple.remarks, + Max(ple.remarks).as_("remarks"), ) .where(ple.delinked == 0) .where(Criterion.all(filter_on_voucher_no)) @@ -2390,14 +2413,15 @@ class QueryPaymentLedger: query_voucher_outstanding = ( qb.from_(ple) .select( - ple.account, + # Max() on columns constant per group keeps this valid on postgres (see above) + Max(ple.account).as_("account"), ple.against_voucher_type.as_("voucher_type"), ple.against_voucher_no.as_("voucher_no"), ple.party_type, ple.party, - ple.posting_date, - ple.due_date, - ple.account_currency.as_("currency"), + Max(ple.posting_date).as_("posting_date"), + Max(ple.due_date).as_("due_date"), + Max(ple.account_currency).as_("currency"), Sum(ple.amount).as_("amount"), Sum(ple.amount_in_account_currency).as_("amount_in_account_currency"), ) @@ -2446,17 +2470,19 @@ class QueryPaymentLedger: # build CTE filter # only fetch invoices + # The combined CTE query has no GROUP BY, so these are row filters. MariaDB tolerates HAVING + # on a select alias here, but postgres does not; express them as WHERE on the source column. if self.get_invoices: self.cte_query_voucher_amount_and_outstanding = ( - self.cte_query_voucher_amount_and_outstanding.having( - qb.Field("outstanding_in_account_currency") > 0 + self.cte_query_voucher_amount_and_outstanding.where( + Table("outstanding").amount_in_account_currency > 0 ) ) # only fetch payments elif self.get_payments: self.cte_query_voucher_amount_and_outstanding = ( - self.cte_query_voucher_amount_and_outstanding.having( - qb.Field("outstanding_in_account_currency") < 0 + self.cte_query_voucher_amount_and_outstanding.where( + Table("outstanding").amount_in_account_currency < 0 ) ) diff --git a/erpnext/utilities/query.py b/erpnext/utilities/query.py new file mode 100644 index 00000000000..ef0ef3e716d --- /dev/null +++ b/erpnext/utilities/query.py @@ -0,0 +1,96 @@ +# Copyright (c) 2025, Frappe Technologies Pvt. Ltd. and contributors +# For license information, please see license.txt + +"""Query-builder helpers for permission & filter conditions. + +These are ERPNext-local because they are only consumed by ERPNext. They return +``pypika`` criteria (rather than the raw SQL strings produced by +``frappe.desk.reportview.get_match_cond`` / ``get_filters_cond``) so the conditions +can be applied to any ``frappe.qb`` query via ``.where(...)`` — including joins and +aliased queries where the permission-checked doctype is not the single base of +``frappe.qb.get_query``. + +They are thin wrappers over ``frappe.database.query.Engine`` (``get_permission_conditions`` +/ ``apply_filters``), which pre-date this code. Where the permission-checked doctype *is* +the base of the query, prefer ``frappe.qb.get_query(doctype, ignore_permissions=False)`` +directly instead of these helpers. +""" + +import json + +import frappe +from frappe import _ + + +def get_match_conditions_qb(doctype, table=None, user=None): + """Return user-permission match conditions for ``doctype`` as query-builder criteria. + + Query-builder equivalent of ``frappe.desk.reportview.get_match_cond`` / + ``build_match_conditions`` (which return raw SQL strings). Returns a list of pypika + criteria (0 or 1 elements) covering role permissions, user permissions, sharing and the + if-owner constraint as well as ``permission_query_conditions`` hooks/server scripts. + + Args: + doctype: doctype to build permission conditions for. + table: pypika table the conditions should reference. Defaults to + ``frappe.qb.DocType(doctype)``. + user: user to evaluate permissions for. Defaults to the session user. + """ + from frappe.database.query import Engine + + engine = Engine() + engine.get_query(doctype, user=user, ignore_permissions=False, db_query_compat=True) + condition = engine.get_permission_conditions(doctype, table or engine.table) + return [condition] if condition is not None else [] + + +def get_filter_conditions_qb(doctype, filters, ignore_permissions=None): + """Return ``filters`` for ``doctype`` as a list of query-builder criteria. + + Query-builder equivalent of ``frappe.desk.reportview.get_filters_cond`` (which returns a + raw SQL string). Accepts the standard frappe filter forms (dict, or list of + ``[doctype, field, op, value]`` rows) and returns pypika criteria that can be applied to + any ``frappe.qb`` query via ``.where(...)``. + """ + if not filters: + return [] + + from pypika.terms import Criterion + + # A pypika Criterion is already a usable condition; apply_filters would route it straight to + # the query and never populate `collect`, silently returning []. Hand it back as-is instead. + if isinstance(filters, Criterion): + return [filters] + + if isinstance(filters, str): + filters = json.loads(filters) + + if isinstance(filters, dict): + # Mirror get_filters_cond's dict normalization: a string value prefixed with "!" means + # "not equal" (e.g. {"enabled": "!1"} -> enabled != "1"). apply_filters' dict path would + # otherwise treat "!1" as a literal value and emit `enabled = "!1"`. + filters = { + field: ("!=", value[1:]) if isinstance(value, str) and value.startswith("!") else value + for field, value in filters.items() + } + + from frappe.database.query import Engine + + engine = Engine() + engine.get_query(doctype, ignore_permissions=ignore_permissions, db_query_compat=True) + criteria = [] + engine.apply_filters(filters, collect=criteria) + return criteria + + +def get_event_conditions_qb(doctype, filters=None): + """Return user-permission match conditions + ``filters`` for event/calendar queries. + + Query-builder equivalent of ``frappe.desk.calendar.get_event_conditions(..., as_qb=True)``: + a list of pypika criteria suitable for applying to a ``frappe.qb`` query via ``.where(...)`` + (e.g. calendar feeds that join across multiple doctypes). + """ + if not frappe.has_permission(doctype): + frappe.throw(_("Not Permitted"), frappe.PermissionError) + + return get_match_conditions_qb(doctype) + get_filter_conditions_qb(doctype, filters) diff --git a/erpnext/utilities/test_query.py b/erpnext/utilities/test_query.py new file mode 100644 index 00000000000..3dd928cb81e --- /dev/null +++ b/erpnext/utilities/test_query.py @@ -0,0 +1,27 @@ +# Copyright (c) 2025, Frappe Technologies Pvt. Ltd. and contributors +# For license information, please see license.txt + +import frappe +from pypika.terms import Criterion + +from erpnext.tests.utils import ERPNextTestSuite +from erpnext.utilities.query import get_filter_conditions_qb + + +class TestQueryHelpers(ERPNextTestSuite): + def test_get_filter_conditions_qb_negation_dict(self): + # get_filter_conditions_qb is the query-builder equivalent of get_filters_cond, so it must + # honour the same dict shorthand where a string value prefixed with "!" means "not equal" + # ({"istable": "!1"} -> istable != "1"), not a literal istable = "!1". + def _where(filters): + dt = frappe.qb.DocType("DocType") + criteria = get_filter_conditions_qb("DocType", filters, ignore_permissions=True) + return frappe.qb.from_(dt).select(dt.name).where(Criterion.all(criteria)).get_sql() + + # "!1" -> not-equal, mirroring the legacy get_filters_cond rewrite + self.assertIn("<>", _where({"istable": "!1"})) + self.assertNotIn("'!1'", _where({"istable": "!1"})) + # plain value stays equality; explicit [op, value] still honoured + self.assertIn("=", _where({"istable": "1"})) + self.assertNotIn("<>", _where({"istable": "1"})) + self.assertIn("<>", _where({"istable": ["!=", "1"]})) From e7b135b51e259ab329cf2f66137e8f309a4959e8 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Thu, 18 Jun 2026 21:29:00 +0530 Subject: [PATCH 2/2] fix(postgres): aggregate bare account in pos_closing payments; explicit limit on gl-entry fetch Address review (#56111): - pos_closing get_payments grouped by mode_of_payment but selected a bare account -> Postgres GroupingError. Wrap in Max() (deterministic, both engines agree; account is consumed downstream for the change-amount adjustment). test_pos_closing_entry 9/9 both engines. - get_voucherwise_gl_entries: add limit=0 to make the unbounded fetch explicit. Co-Authored-By: Claude Opus 4.8 (1M context) --- erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.py | 2 +- erpnext/accounts/utils.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.py b/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.py index fff99369ef0..f697b0ab0af 100644 --- a/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.py +++ b/erpnext/accounts/doctype/pos_closing_entry/pos_closing_entry.py @@ -295,7 +295,7 @@ def get_payments(invoices): .groupby(SalesInvoicePayment.mode_of_payment) .select( SalesInvoicePayment.mode_of_payment, - SalesInvoicePayment.account.as_("account"), + fn.Max(SalesInvoicePayment.account).as_("account"), fn.Sum(SalesInvoicePayment.amount).as_("amount"), ) ) diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index d9974cf8c71..beabadda2bf 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -1835,6 +1835,7 @@ def get_voucherwise_gl_entries(future_stock_vouchers, posting_date): "GL Entry", filters={"posting_date": [">=", posting_date], "voucher_no": ["in", voucher_nos]}, fields=["name", "account", "credit", "debit", "cost_center", "project", "voucher_type", "voucher_no"], + limit=0, ) for d in gles: