fix(postgres): payment ledger outstanding query groupby

[ili-ad]

Summary

Fix Sales Invoice submit crash on Postgres in Payment Ledger outstanding computation by making GROUP BY deterministic.

Problem

Submitting a Sales Invoice on Postgres can fail with:

psycopg2.errors.GroupingError: column "tabPayment Ledger Entry.account" must appear in the GROUP BY clause or be used in an aggregate function

Root cause: the voucher/outstanding CTE queries select account (and other non-aggregated fields) while using SUM(...) without including those fields in GROUP BY, which Postgres rejects.

Fix

• Group by the true key columns (including account, voucher identifiers, and party identifiers)
• Use MAX(...) for “decorator” fields that are stable per group (posting_date, due_date, currency, cost_center, remarks)

This keeps the logic deterministic and Postgres-compliant without changing behavior on MariaDB/MySQL.

Tests

• pre-commit run --all-files ✅
• Manual: Sales Invoice submit no longer fails on Postgres at Payment Ledger outstanding stage.

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request modifies SQL query construction in erpnext/accounts/utils.py to comply with SQL standard requirements for GROUP BY clauses. Non-aggregated columns in voucher-outstanding queries are wrapped with MAX() functions, and corresponding fields are added to the GROUP BY clause. The account field is added to grouping keys, and dimension filters receive related adjustments. This ensures deterministic aggregation behavior across different database backends.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly addresses the main change: fixing a Postgres-specific issue with GROUP BY in the payment ledger outstanding query.
Description check	✅ Passed	The description comprehensively explains the problem, root cause, solution, and testing approach for the GROUP BY aggregation fix in the payment ledger outstanding query.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 57624e1 and b46ee14.

📒 Files selected for processing (1)

• erpnext/accounts/utils.py

🧰 Additional context used

🧠 Learnings (3)

📓 Common learnings

Learnt from: vorasmit
Repo: frappe/erpnext PR: 51099
File: erpnext/patches/v16_0/migrate_tax_withholding_data.py:904-910
Timestamp: 2025-12-23T11:28:49.128Z
Learning: In the old Tax Withholding system (before the Tax Withholding Entry refactor), only one TCS account per Sales Invoice was supported. When reviewing migration code in erpnext/patches/v16_0/migrate_tax_withholding_data.py, the query selecting TCS accounts from GL Entries doesn't need explicit aggregation because historical data will only have one TCS account per invoice.

📚 Learning: 2025-12-23T11:28:49.128Z

Learnt from: vorasmit
Repo: frappe/erpnext PR: 51099
File: erpnext/patches/v16_0/migrate_tax_withholding_data.py:904-910
Timestamp: 2025-12-23T11:28:49.128Z
Learning: In the old Tax Withholding system (before the Tax Withholding Entry refactor), only one TCS account per Sales Invoice was supported. When reviewing migration code in erpnext/patches/v16_0/migrate_tax_withholding_data.py, the query selecting TCS accounts from GL Entries doesn't need explicit aggregation because historical data will only have one TCS account per invoice.

Applied to files:

• erpnext/accounts/utils.py

📚 Learning: 2025-12-16T05:33:58.723Z

Learnt from: Abdeali099
Repo: frappe/erpnext PR: 51078
File: erpnext/accounts/doctype/financial_report_template/financial_report_engine.py:486-491
Timestamp: 2025-12-16T05:33:58.723Z
Learning: In ERPNext/Frappe codebase, query.run(as_dict=True) returns frappe._dict objects that support both dict-style access (obj["key"]) and attribute-style access (obj.key). Therefore, attribute access on query results is valid and will not raise AttributeError. When reviewing Python code, prefer attribute access (obj.key) for readability where the key is known to exist, but ensure existence checks or fallback handling if there is any doubt about missing keys.

Applied to files:

• erpnext/accounts/utils.py

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Summary

🔇 Additional comments (2)

erpnext/accounts/utils.py (2)

2251-2276: LGTM! Postgres-compliant GROUP BY fix for voucher amount query.

The changes correctly address the SQL standard requirement by:

1. Adding account to the GROUP BY clause alongside other key columns
2. Wrapping decorator fields (posting_date, due_date, account_currency, cost_center, remarks) with MAX() since they're effectively constant per group

The inline comment provides helpful context for future maintainers.

---

2279-2305: LGTM! Consistent Postgres-compliant fix for voucher outstanding query.

The changes correctly mirror the fix applied to the voucher amount query:

1. Non-aggregated columns (account, against_voucher_type, against_voucher_no, party_type, party) are in GROUP BY
2. Decorator fields use MAX() for deterministic aggregation

Note that cost_center and remarks are not selected in this query (unlike query_voucher_amount), which is correct since they're only needed in the voucher amount CTE. The dimensions_filter and voucher_posting_date filters are consistently applied across all three queries in this change.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[ruthra-kumar]

@ili-ad
Can't reproduce the error