fix(accounts): key the payment ledger CTEs on account, not Max(account) (#57720)

* fix(accounts): key the payment ledger CTEs on account, not Max(account)

QueryPaymentLedger builds two CTEs -- voucher amount and outstanding -- and
joins them on account among other columns. Both sides selected Max(account)
while grouping without it, so the join key was an aggregate over two different
row sets. A voucher posting ledger entries against two party accounts could
have the two sides pick different accounts, the join miss, and the outstanding
come back NULL. Max() over text is a sort, so which account wins is also
collation-dependent, and the engines sort text differently.

Group both CTEs by account instead. That makes the join key a real column and
scopes each Sum() to a single account -- so amount_in_account_currency is no
longer summed across accounts that may not share a currency. Row shape only
changes for a voucher that genuinely spans two party accounts for one party,
where today's single row is already an arbitrary pick over mixed currencies.

cost_center and remarks stay descriptive but genuinely vary per entry, and were
aggregated independently, so they could be stitched together from different
entries into a row that was never posted. They now come off one real entry,
picked by Min(name) -- Payment Ledger Entry declares no autoname rule, so
frappe names it by hash, and those are lower-case, which keeps the pick free of
the collation divergence.

* test(accounts): cover payment ledger metadata coherence

A Journal Entry posting two receivable lines for one customer with different
cost centers and remarks. Whatever row the ledger returns, its cost center and
remarks must be a pair that was actually posted. Guards the fixture itself, so
it cannot pass by posting only one distinct pair.

* test(accounts): cover the account-keyed payment ledger aggregation

The coherence test posts both party lines to one account, so it exercises the
representative-row metadata but not the account-keyed grouping or the CTE join.
Adds a Journal Entry posting to two receivable accounts for one customer and
asserts each account comes back as its own row, with its own amount and a
non-null outstanding.

erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py

@@ -187,6 +187,103 @@ class TestPaymentReconciliation(ERPNextTestSuite):
 		)
 		return je
 
+	def test_voucher_outstanding_metadata_comes_from_one_ledger_entry(self):
+		"""cost_center and remarks must describe the same Payment Ledger Entry.
+
+		A voucher can post several ledger entries for one party with different cost centers and
+		remarks. Aggregating each column on its own can pair one entry's cost center with another's
+		remarks -- a row that was never posted -- and because Max() over text is a sort, MariaDB and
+		PostgreSQL can pick differently on top of that.
+		"""
+		from erpnext.accounts.utils import QueryPaymentLedger
+
+		je = frappe.new_doc("Journal Entry")
+		je.posting_date = nowdate()
+		je.company = self.company
+		je.user_remark = "aaa base remark"
+		for cost_center, remark, amount in (
+			(self.main_cc, "aaa main line", 100),
+			(self.sub_cc, "zzz sub line", 50),
+		):
+			je.append(
+				"accounts",
+				{
+					"account": self.debit_to,
+					"party_type": "Customer",
+					"party": self.customer,
+					"cost_center": cost_center,
+					"user_remark": remark,
+					"debit_in_account_currency": amount,
+				},
+			)
+		je.append(
+			"accounts", {"account": self.cash, "cost_center": self.main_cc, "credit_in_account_currency": 150}
+		)
+		je.save()
+		je.submit()
+
+		posted = {
+			(row.cost_center, row.remarks)
+			for row in frappe.get_all(
+				"Payment Ledger Entry",
+				filters={"voucher_no": je.name, "delinked": 0},
+				fields=["cost_center", "remarks"],
+			)
+		}
+		self.assertGreater(len(posted), 1, "fixture must post more than one ledger entry to be meaningful")
+
+		ledger = QueryPaymentLedger()
+		rows = ledger.get_voucher_outstandings(
+			vouchers=[frappe._dict(voucher_type="Journal Entry", voucher_no=je.name)]
+		)
+		self.assertTrue(rows)
+
+		for row in rows:
+			self.assertIn((row.cost_center, row.remarks), posted)
+
+	def test_voucher_outstanding_splits_by_party_account(self):
+		"""A voucher posting to two party accounts must report each account separately.
+
+		account is the join key between the amount and outstanding CTEs. Selecting Max(account)
+		while grouping without it made that key an aggregate over two different row sets, so the two
+		sides could pick different accounts, the join would miss and the outstanding come back NULL.
+		It also summed amounts across accounts that need not share a currency.
+		"""
+		from erpnext.accounts.utils import QueryPaymentLedger
+
+		second_receivable = "_Test Receivable - _TC"
+		je = frappe.new_doc("Journal Entry")
+		je.posting_date = nowdate()
+		je.company = self.company
+		je.user_remark = "two receivable accounts"
+		for account, amount in ((self.debit_to, 100), (second_receivable, 60)):
+			je.append(
+				"accounts",
+				{
+					"account": account,
+					"party_type": "Customer",
+					"party": self.customer,
+					"cost_center": self.main_cc,
+					"debit_in_account_currency": amount,
+				},
+			)
+		je.append(
+			"accounts", {"account": self.cash, "cost_center": self.main_cc, "credit_in_account_currency": 160}
+		)
+		je.save()
+		je.submit()
+
+		rows = QueryPaymentLedger().get_voucher_outstandings(
+			vouchers=[frappe._dict(voucher_type="Journal Entry", voucher_no=je.name)]
+		)
+		by_account = {row.account: row for row in rows}
+
+		self.assertEqual(set(by_account), {self.debit_to, second_receivable})
+		self.assertEqual(flt(by_account[self.debit_to].invoice_amount), 100)
+		self.assertEqual(flt(by_account[second_receivable].invoice_amount), 60)
+		for row in rows:
+			self.assertIsNotNone(row.outstanding)
+
 	def test_filter_min_max(self):
 		# check filter condition minimum and maximum amount
 		self.create_sales_invoice(qty=1, rate=300)

erpnext/accounts/utils.py

@@ -2379,13 +2379,16 @@ class QueryPaymentLedger:
 				)
 
 		# build query for voucher amount
-		query_voucher_amount = (
+		# account is grouped, not aggregated: it is a join key against the outstanding CTE below, and
+		# it fixes the currency the amounts are summed in. The two CTEs aggregate over different row
+		# sets, so two Max() picks could disagree and the join would silently miss, leaving the
+		# outstanding NULL. posting_date/due_date are dates, so Max() there cannot depend on
+		# collation. cost_center and remarks are free text that genuinely varies per row, so they
+		# come off one real row instead -- see representative below.
+		grouped_voucher_amount = (
 			qb.from_(ple)
 			.select(
-				# 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.account,
 				ple.voucher_type,
 				ple.voucher_no,
 				ple.party_type,
@@ -2393,25 +2396,47 @@ class QueryPaymentLedger:
 				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"),
-				Max(ple.remarks).as_("remarks"),
+				Min(ple.name).as_("representative"),
 			)
 			.where(ple.delinked == 0)
 			.where(Criterion.all(filter_on_voucher_no))
 			.where(Criterion.all(self.common_filter))
 			.where(Criterion.all(self.dimensions_filter))
 			.where(Criterion.all(self.voucher_posting_date))
-			.groupby(ple.voucher_type, ple.voucher_no, ple.party_type, ple.party)
+			.groupby(ple.account, ple.voucher_type, ple.voucher_no, ple.party_type, ple.party)
+		).as_("grouped")
+
+		# Payment Ledger Entry has no autoname rule, so frappe names it by hash -- lower-case, which
+		# keeps Min(name) free of the collation divergence that picking Max() over free text has.
+		representative_ple = qb.DocType("Payment Ledger Entry").as_("representative_ple")
+		query_voucher_amount = (
+			qb.from_(grouped_voucher_amount)
+			.inner_join(representative_ple)
+			.on(representative_ple.name == grouped_voucher_amount.representative)
+			.select(
+				grouped_voucher_amount.account,
+				grouped_voucher_amount.voucher_type,
+				grouped_voucher_amount.voucher_no,
+				grouped_voucher_amount.party_type,
+				grouped_voucher_amount.party,
+				grouped_voucher_amount.posting_date,
+				grouped_voucher_amount.due_date,
+				grouped_voucher_amount.currency,
+				grouped_voucher_amount.amount,
+				grouped_voucher_amount.amount_in_account_currency,
+				representative_ple.cost_center.as_("cost_center"),
+				representative_ple.remarks.as_("remarks"),
+			)
 		)
 
 		# build query for voucher outstanding
 		query_voucher_outstanding = (
 			qb.from_(ple)
 			.select(
-				# Max() on columns constant per group keeps this valid on postgres (see above)
-				Max(ple.account).as_("account"),
+				# grouped, not aggregated: this is the other side of the join key -- see above
+				ple.account,
 				ple.against_voucher_type.as_("voucher_type"),
 				ple.against_voucher_no.as_("voucher_no"),
 				ple.party_type,
@@ -2425,7 +2450,7 @@ class QueryPaymentLedger:
 			.where(ple.delinked == 0)
 			.where(Criterion.all(filter_on_against_voucher_no))
 			.where(Criterion.all(self.common_filter))
-			.groupby(ple.against_voucher_type, ple.against_voucher_no, ple.party_type, ple.party)
+			.groupby(ple.account, ple.against_voucher_type, ple.against_voucher_no, ple.party_type, ple.party)
 		)
 
 		# build CTE for combining voucher amount and outstanding