Merge pull request #56037 from mihir-kandoi/pg-groupby-doctypes

fix(postgres): satisfy strict GROUP BY in doctype & core queries

erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py

@@ -8,7 +8,7 @@ import frappe
 from frappe import _
 from frappe.model.document import Document
 from frappe.query_builder.custom import ConstantColumn
-from frappe.query_builder.functions import Sum
+from frappe.query_builder.functions import Max, Sum
 from frappe.utils import cint, create_batch, flt
 
 from erpnext import get_default_cost_center
@@ -1410,12 +1410,14 @@ def get_je_matching_query(
 			Sum(getattr(jea, amount_field)).as_("paid_amount"),
 			ConstantColumn("Journal Entry").as_("doctype"),
 			je.name,
-			je.cheque_no.as_("reference_no"),
-			je.cheque_date.as_("reference_date"),
-			je.pay_to_recd_from.as_("party"),
-			jea.party_type,
-			je.posting_date,
-			jea.account_currency.as_("currency"),
+			# non-grouped columns are constant per grouped JE name (party_type/currency come from the
+			# single bank-account line) -> Max() keeps the GROUP BY valid on postgres with the same value
+			Max(je.cheque_no).as_("reference_no"),
+			Max(je.cheque_date).as_("reference_date"),
+			Max(je.pay_to_recd_from).as_("party"),
+			Max(jea.party_type).as_("party_type"),
+			Max(je.posting_date).as_("posting_date"),
+			Max(jea.account_currency).as_("currency"),
 		)
 		.where(je.docstatus == 1)
 		.where(je.voucher_type != "Opening Entry")
@@ -1423,7 +1425,7 @@ def get_je_matching_query(
 		.where(jea.account == common_filters.bank_account)
 		.where(filter_by_date)
 		.groupby(je.name)
-		.orderby(je.cheque_date if cint(filter_by_reference_date) else je.posting_date)
+		.orderby(Max(je.cheque_date) if cint(filter_by_reference_date) else Max(je.posting_date))
 	)
 
 	if frappe.flags.auto_reconcile_vouchers is True:

erpnext/accounts/doctype/exchange_rate_revaluation/exchange_rate_revaluation.py

@@ -8,7 +8,7 @@ from frappe import _, qb
 from frappe.model.document import Document
 from frappe.model.meta import get_field_precision
 from frappe.query_builder import Criterion, Order
-from frappe.query_builder.functions import NullIf, Sum
+from frappe.query_builder.functions import Max, NullIf, Sum
 from frappe.utils import flt, get_link_to_form
 
 import erpnext
@@ -188,12 +188,18 @@ class ExchangeRateRevaluation(Document):
 				accounts = [x[0] for x in res]
 
 			if accounts:
-				having_clause = (qb.Field("balance") != qb.Field("balance_in_account_currency")) & (
-					(qb.Field("balance_in_account_currency") != 0) | (qb.Field("balance") != 0)
-				)
-
 				gle = qb.DocType("GL Entry")
 
+				# balance expressions reused in both SELECT and HAVING; postgres can't reference a
+				# SELECT alias inside HAVING, so the aggregate expression must be repeated there.
+				balance = Sum(gle.debit) - Sum(gle.credit)
+				balance_in_account_currency = Sum(gle.debit_in_account_currency) - Sum(
+					gle.credit_in_account_currency
+				)
+				having_clause = (balance != balance_in_account_currency) & (
+					(balance_in_account_currency != 0) | (balance != 0)
+				)
+
 				# conditions
 				conditions = []
 				conditions.append(gle.account.isin(accounts))
@@ -209,17 +215,15 @@ class ExchangeRateRevaluation(Document):
 					qb.from_(gle)
 					.select(
 						gle.account,
-						gle.party_type,
-						gle.party,
-						gle.account_currency,
-						(Sum(gle.debit_in_account_currency) - Sum(gle.credit_in_account_currency)).as_(
-							"balance_in_account_currency"
-						),
-						(Sum(gle.debit) - Sum(gle.credit)).as_("balance"),
-						(Sum(gle.debit) - Sum(gle.credit) == 0)
-						^ (Sum(gle.debit_in_account_currency) - Sum(gle.credit_in_account_currency) == 0).as_(
-							"zero_balance"
-						),
+						# grouped by NullIf(party_type/party, ""); the bare columns + account_currency are
+						# constant per group -> Max() keeps the GROUP BY valid on postgres with the same value.
+						Max(gle.party_type).as_("party_type"),
+						Max(gle.party).as_("party"),
+						Max(gle.account_currency).as_("account_currency"),
+						balance_in_account_currency.as_("balance_in_account_currency"),
+						balance.as_("balance"),
+						# zero_balance is recomputed in Python below (after rounding), so the SQL value is
+						# unused -- dropped (it used MySQL's XOR operator, which postgres lacks).
 					)
 					.where(Criterion.all(conditions))
 					.groupby(gle.account, NullIf(gle.party_type, ""), NullIf(gle.party, ""))

erpnext/accounts/doctype/process_period_closing_voucher/process_period_closing_voucher.py

@@ -553,7 +553,8 @@ def process_individual_date(docname: str, date, report_type, parentfield):
 		Sum(gle.credit).as_("credit"),
 		Sum(gle.debit_in_account_currency).as_("debit_in_account_currency"),
 		Sum(gle.credit_in_account_currency).as_("credit_in_account_currency"),
-		gle.account_currency,
+		# account_currency is constant per grouped account -> Max() keeps the GROUP BY postgres-valid
+		Max(gle.account_currency).as_("account_currency"),
 	).where(
 		(gle.company.eq(company))
 		& (gle.is_cancelled.eq(0))

erpnext/accounts/doctype/unreconcile_payment/unreconcile_payment.py

@@ -7,7 +7,7 @@ import frappe
 from frappe import _, qb
 from frappe.model.document import Document
 from frappe.query_builder import Criterion
-from frappe.query_builder.functions import Abs, Sum
+from frappe.query_builder.functions import Abs, Max, Sum
 from frappe.utils.data import comma_and
 
 from erpnext.accounts.utils import (
@@ -72,7 +72,7 @@ class UnreconcilePayment(Document):
 				alloc.party,
 			)
 
-			frappe.db.set_value("Unreconcile Payment Entries", alloc.name, "unlinked", True)
+			frappe.db.set_value("Unreconcile Payment Entries", alloc.name, "unlinked", 1)
 
 
 @frappe.whitelist()
@@ -120,18 +120,20 @@ def get_linked_payments_for_doc(
 			res = (
 				qb.from_(ple)
 				.select(
-					ple.account,
-					ple.party_type,
-					ple.party,
-					ple.company,
-					ple.voucher_type.as_("reference_doctype"),
+					Max(ple.account).as_("account"),
+					Max(ple.party_type).as_("party_type"),
+					Max(ple.party).as_("party"),
+					Max(ple.company).as_("company"),
+					Max(ple.voucher_type).as_("reference_doctype"),
 					ple.voucher_no.as_("reference_name"),
 					Abs(Sum(ple.amount_in_account_currency)).as_("allocated_amount"),
-					ple.account_currency,
+					Max(ple.account_currency).as_("account_currency"),
 				)
 				.where(Criterion.all(criteria))
 				.groupby(ple.voucher_no, ple.against_voucher_no)
-				.having(qb.Field("allocated_amount") > 0)
+				.having(Abs(Sum(ple.amount_in_account_currency)) > 0)
+				# deterministic order across backends (postgres GROUP BY does not imply ordering)
+				.orderby(ple.voucher_no)
 				.run(as_dict=True)
 			)
 			return res
@@ -146,17 +148,19 @@ def get_linked_payments_for_doc(
 			query = (
 				qb.from_(ple)
 				.select(
-					ple.company,
-					ple.account,
-					ple.party_type,
-					ple.party,
-					ple.against_voucher_type.as_("reference_doctype"),
+					Max(ple.company).as_("company"),
+					Max(ple.account).as_("account"),
+					Max(ple.party_type).as_("party_type"),
+					Max(ple.party).as_("party"),
+					Max(ple.against_voucher_type).as_("reference_doctype"),
 					ple.against_voucher_no.as_("reference_name"),
 					Abs(Sum(ple.amount_in_account_currency)).as_("allocated_amount"),
-					ple.account_currency,
+					Max(ple.account_currency).as_("account_currency"),
 				)
 				.where(Criterion.all(criteria))
 				.groupby(ple.against_voucher_no)
+				# deterministic order across backends (postgres GROUP BY does not imply ordering)
+				.orderby(ple.against_voucher_no)
 			)
 
 			res = query.run(as_dict=True)
@@ -180,15 +184,18 @@ def get_linked_advances(company, docname):
 	return (
 		qb.from_(adv)
 		.select(
-			adv.company,
-			adv.against_voucher_type.as_("reference_doctype"),
+			# non-grouped columns are constant per against_voucher_no -> Max() is unchanged and postgres-valid
+			Max(adv.company).as_("company"),
+			Max(adv.against_voucher_type).as_("reference_doctype"),
 			adv.against_voucher_no.as_("reference_name"),
 			Abs(Sum(adv.amount)).as_("allocated_amount"),
-			adv.currency,
+			Max(adv.currency).as_("currency"),
 		)
 		.where(Criterion.all(criteria))
-		.having(qb.Field("allocated_amount") > 0)
+		.having(Abs(Sum(adv.amount)) > 0)
 		.groupby(adv.against_voucher_no)
+		# deterministic order across backends (postgres GROUP BY does not imply ordering)
+		.orderby(adv.against_voucher_no)
 		.run(as_dict=True)
 	)
 

erpnext/accounts/services/advances.py

@@ -12,7 +12,7 @@ import frappe
 from frappe import _
 from frappe.query_builder import Criterion
 from frappe.query_builder.custom import ConstantColumn
-from frappe.query_builder.functions import Abs, Sum
+from frappe.query_builder.functions import Abs, Max, Sum
 from frappe.utils import flt
 
 import erpnext
@@ -150,7 +150,7 @@ def calculate_total_advance_from_ledger(doc) -> list:
 	adv = frappe.qb.DocType("Advance Payment Ledger Entry")
 	return (
 		frappe.qb.from_(adv)
-		.select(Abs(Sum(adv.amount)).as_("amount"), adv.currency.as_("account_currency"))
+		.select(Abs(Sum(adv.amount)).as_("amount"), Max(adv.currency).as_("account_currency"))
 		.where(adv.company == doc.company)
 		.where(adv.delinked == 0)
 		.where(adv.against_voucher_type == doc.doctype)

erpnext/manufacturing/doctype/production_plan/services/bom_explosion.py

@@ -4,7 +4,7 @@
 """BOM explosion helpers for Production Plan material planning."""
 
 import frappe
-from frappe.query_builder.functions import IfNull, Sum
+from frappe.query_builder.functions import IfNull, Max, Min, Sum
 
 from erpnext.manufacturing.doctype.production_plan.services.planning_queries import get_uom_conversion_factor
 
@@ -38,22 +38,25 @@ def _exploded_items_query(company, bom_no, include_non_stock_items, planned_qty)
 
 
 def _exploded_item_columns(bei, bom, item, item_default, item_uom, planned_qty):
+	# only item_code/stock_uom are grouped; the rest are functionally dependent on the grouped item
+	# or arbitrary per BOM Item on MySQL -> Max() keeps the GROUP BY valid on postgres with the same
+	# value MySQL picked.
 	return [
 		(IfNull(Sum(bei.stock_qty / IfNull(bom.quantity, 1)), 0) * planned_qty).as_("qty"),
-		item.item_name,
-		item.name.as_("item_code"),
-		bei.description,
+		Max(item.item_name).as_("item_name"),
+		Max(item.name).as_("item_code"),
+		Max(bei.description).as_("description"),
 		bei.stock_uom,
-		item.min_order_qty,
-		bei.source_warehouse,
-		item.default_material_request_type,
-		item.min_order_qty,
-		item_default.default_warehouse,
-		item.purchase_uom,
-		item_uom.conversion_factor,
-		item.safety_stock,
-		bom.item.as_("main_bom_item"),
-		bom.name.as_("main_bom"),
+		Max(item.min_order_qty).as_("min_order_qty"),
+		Max(bei.source_warehouse).as_("source_warehouse"),
+		Max(item.default_material_request_type).as_("default_material_request_type"),
+		Max(item.min_order_qty).as_("min_order_qty"),
+		Max(item_default.default_warehouse).as_("default_warehouse"),
+		Max(item.purchase_uom).as_("purchase_uom"),
+		Max(item_uom.conversion_factor).as_("conversion_factor"),
+		Max(item.safety_stock).as_("safety_stock"),
+		Max(bom.item).as_("main_bom_item"),
+		Max(bom.name).as_("main_bom"),
 	]
 
 
@@ -106,30 +109,34 @@ def _subitems_query(company, bom_no, include_non_stock_items, parent_qty, planne
 		.select(*_subitem_columns(bom_item, bom, item, item_default, item_uom, parent_qty, planned_qty))
 		.where(_subitem_filter(bom_item, bom, item, bom_no, include_non_stock_items))
 		.groupby(bom_item.item_code)
-		.orderby(bom_item.idx)
+		# idx is not grouped; Min() preserves the original ordering and is valid on postgres
+		.orderby(Min(bom_item.idx))
 	).run(as_dict=True)
 
 
 def _subitem_columns(bom_item, bom, item, item_default, item_uom, parent_qty, planned_qty):
 	qty = IfNull(parent_qty * Sum(bom_item.stock_qty / IfNull(bom.quantity, 1)) * planned_qty, 0).as_("qty")
+	# only item_code is grouped; the rest are functionally dependent on the grouped item (item
+	# attributes) or arbitrary per BOM Item on MySQL -> Max() keeps the GROUP BY valid on postgres
+	# while returning the same value MySQL picked.
 	return [
 		bom_item.item_code,
-		item.default_material_request_type,
-		item.item_name,
+		Max(item.default_material_request_type).as_("default_material_request_type"),
+		Max(item.item_name).as_("item_name"),
 		qty,
-		item.is_sub_contracted_item.as_("is_sub_contracted"),
-		bom_item.source_warehouse,
-		item.default_bom.as_("default_bom"),
-		bom_item.description.as_("description"),
-		bom_item.stock_uom.as_("stock_uom"),
-		item.min_order_qty.as_("min_order_qty"),
-		item.safety_stock.as_("safety_stock"),
-		item_default.default_warehouse,
-		item.purchase_uom,
-		item_uom.conversion_factor,
-		bom.item.as_("main_bom_item"),
-		bom.name.as_("main_bom"),
-		bom_item.is_phantom_item,
+		Max(item.is_sub_contracted_item).as_("is_sub_contracted"),
+		Max(bom_item.source_warehouse).as_("source_warehouse"),
+		Max(item.default_bom).as_("default_bom"),
+		Max(bom_item.description).as_("description"),
+		Max(bom_item.stock_uom).as_("stock_uom"),
+		Max(item.min_order_qty).as_("min_order_qty"),
+		Max(item.safety_stock).as_("safety_stock"),
+		Max(item_default.default_warehouse).as_("default_warehouse"),
+		Max(item.purchase_uom).as_("purchase_uom"),
+		Max(item_uom.conversion_factor).as_("conversion_factor"),
+		Max(bom.item).as_("main_bom_item"),
+		Max(bom.name).as_("main_bom"),
+		Max(bom_item.is_phantom_item).as_("is_phantom_item"),
 	]
 
 

erpnext/manufacturing/doctype/production_plan/services/sub_assembly_queries.py

@@ -4,7 +4,7 @@
 """Sub-assembly resolution helpers for Production Plan."""
 
 import frappe
-from frappe.query_builder.functions import IfNull, Sum
+from frappe.query_builder.functions import IfNull, Max, Sum
 from frappe.utils import flt
 
 from erpnext.manufacturing.doctype.bom.bom import get_children as get_bom_children
@@ -184,24 +184,27 @@ def _sub_assembly_rm_query(company, bom_no, include_non_stock_items, planned_qty
 
 
 def _sub_assembly_rm_columns(bei, bom, item, item_default, item_uom, planned_qty):
+	# only item_code/stock_uom are grouped; every other column is functionally dependent on the
+	# grouped item (item attributes) or arbitrary per BOM Item on MySQL -> Max() keeps the GROUP BY
+	# valid on postgres while returning the same value MySQL picked.
 	return [
 		(IfNull(Sum(bei.stock_qty / IfNull(bom.quantity, 1)), 0) * planned_qty).as_("qty"),
-		item.item_name,
-		item.name.as_("item_code"),
-		bei.description,
+		Max(item.item_name).as_("item_name"),
+		Max(item.name).as_("item_code"),
+		Max(bei.description).as_("description"),
 		bei.stock_uom,
-		bei.is_phantom_item,
-		bei.bom_no,
-		item.min_order_qty,
-		bei.source_warehouse,
-		item.default_material_request_type,
-		item.min_order_qty,
-		item_default.default_warehouse,
-		item.purchase_uom,
-		item_uom.conversion_factor,
-		item.safety_stock,
-		bom.item.as_("main_bom_item"),
-		bom.name.as_("main_bom"),
+		Max(bei.is_phantom_item).as_("is_phantom_item"),
+		Max(bei.bom_no).as_("bom_no"),
+		Max(item.min_order_qty).as_("min_order_qty"),
+		Max(bei.source_warehouse).as_("source_warehouse"),
+		Max(item.default_material_request_type).as_("default_material_request_type"),
+		Max(item.min_order_qty).as_("min_order_qty"),
+		Max(item_default.default_warehouse).as_("default_warehouse"),
+		Max(item.purchase_uom).as_("purchase_uom"),
+		Max(item_uom.conversion_factor).as_("conversion_factor"),
+		Max(item.safety_stock).as_("safety_stock"),
+		Max(bom.item).as_("main_bom_item"),
+		Max(bom.name).as_("main_bom"),
 	]
 
 

erpnext/manufacturing/doctype/work_order/services/required_items.py

@@ -191,7 +191,13 @@ class RequiredItemsService:
 			frappe.qb.from_(ste)
 			.inner_join(ste_child)
 			.on(ste_child.parent == ste.name)
-			.select(ste_child.item_code, ste_child.original_item, fn.Sum(ste_child.transfer_qty).as_("qty"))
+			# original_item is arbitrary per grouped item_code on MySQL -> Max() keeps the GROUP BY valid
+			# on postgres while returning the same value (it is only used as a dict key fallback below)
+			.select(
+				ste_child.item_code,
+				fn.Max(ste_child.original_item).as_("original_item"),
+				fn.Sum(ste_child.transfer_qty).as_("qty"),
+			)
 			.where(self._material_transfer_filter(ste, is_return))
 			.groupby(ste_child.item_code)
 		)

erpnext/stock/doctype/serial_and_batch_bundle/serial_and_batch_bundle.py

@@ -11,7 +11,7 @@ import frappe.query_builder
 from frappe import _, _dict, bold
 from frappe.model.document import Document
 from frappe.model.naming import make_autoname
-from frappe.query_builder.functions import Concat_ws, Sum
+from frappe.query_builder.functions import Concat_ws, Max, Sum
 from frappe.utils import (
 	cint,
 	cstr,
@@ -3067,7 +3067,7 @@ def get_available_batches(kwargs):
 			batch_ledger.batch_no,
 			batch_ledger.warehouse,
 			Sum(batch_ledger.qty).as_("qty"),
-			batch_table.expiry_date,
+			Max(batch_table.expiry_date).as_("expiry_date"),
 		)
 		.where(batch_table.disabled == 0)
 		.where(stock_ledger_entry.is_cancelled == 0)
@@ -3107,12 +3107,13 @@ def get_available_batches(kwargs):
 		else:
 			query = query.where(batch_ledger.batch_no == kwargs.batch_no)
 
+	# order by aggregates (one row per batch_no+warehouse); raw columns aren't valid under GROUP BY on postgres
 	if kwargs.based_on == "LIFO":
-		query = query.orderby(batch_table.creation, order=frappe.qb.desc)
+		query = query.orderby(Max(batch_table.creation), order=frappe.qb.desc)
 	elif kwargs.based_on == "Expiry":
-		query = query.orderby(batch_table.expiry_date)
+		query = query.orderby(Max(batch_table.expiry_date))
 	else:
-		query = query.orderby(batch_table.creation)
+		query = query.orderby(Max(batch_table.creation))
 
 	if kwargs.get("ignore_voucher_nos"):
 		query = query.where(stock_ledger_entry.voucher_no.notin(kwargs.get("ignore_voucher_nos")))
@@ -3329,6 +3330,10 @@ def get_stock_ledgers_for_serial_nos(kwargs):
 			stock_ledger_entry.actual_qty,
 			stock_ledger_entry.serial_no,
 			stock_ledger_entry.serial_and_batch_bundle,
+			# creation is the ORDER BY tiebreaker; postgres requires ORDER BY columns to be in the
+			# select list when the query is DISTINCT (added below for serial-no filters). It is unique
+			# per SLE so it doesn't change the distinct row set (serial_and_batch_bundle already is).
+			stock_ledger_entry.creation,
 		)
 		.where(stock_ledger_entry.is_cancelled == 0)
 		.orderby(stock_ledger_entry.posting_datetime)
@@ -3395,10 +3400,10 @@ def get_stock_ledgers_batches(kwargs):
 		.on(stock_ledger_entry.batch_no == batch_table.name)
 		.select(
 			stock_ledger_entry.warehouse,
-			stock_ledger_entry.item_code,
+			Max(stock_ledger_entry.item_code).as_("item_code"),
 			Sum(stock_ledger_entry.actual_qty).as_("qty"),
 			stock_ledger_entry.batch_no,
-			batch_table.expiry_date,
+			Max(batch_table.expiry_date).as_("expiry_date"),
 		)
 		.where((stock_ledger_entry.is_cancelled == 0) & (stock_ledger_entry.batch_no.isnotnull()))
 		.groupby(stock_ledger_entry.batch_no, stock_ledger_entry.warehouse)
@@ -3434,12 +3439,13 @@ def get_stock_ledgers_batches(kwargs):
 	if kwargs.get("ignore_voucher_nos"):
 		query = query.where(stock_ledger_entry.voucher_no.notin(kwargs.get("ignore_voucher_nos")))
 
+	# order by aggregates (one row per batch_no+warehouse); raw columns aren't valid under GROUP BY on postgres
 	if kwargs.based_on == "LIFO":
-		query = query.orderby(batch_table.creation, order=frappe.qb.desc)
+		query = query.orderby(Max(batch_table.creation), order=frappe.qb.desc)
 	elif kwargs.based_on == "Expiry":
-		query = query.orderby(batch_table.expiry_date)
+		query = query.orderby(Max(batch_table.expiry_date))
 	else:
-		query = query.orderby(batch_table.creation)
+		query = query.orderby(Max(batch_table.creation))
 
 	data = query.run(as_dict=True)
 	batches = {}