From dbd1388b404bafcfc9a0fbef6bda9cdac0759482 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Sun, 21 Jun 2026 22:38:30 +0530 Subject: [PATCH] fix(controllers): keep Sales/Purchase Trends one row per based-on key (MariaDB parity) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #56192 made the trends queries Postgres-strict-GROUP-BY-valid by widening based_on_group_by to include the selected descriptive columns. For Item it added t2.item_name, for Customer t1.territory (and customer_name) — but item_name is an editable per-line field and territory an editable per-document field, not functionally dependent on the item_code/customer key. On MariaDB (ONLY_FULL_GROUP_BY off) this SPLITS the single row per key into one row per distinct (key, item_name)/(key, territory), so a customer transacting across two territories (or an item with an edited item_name) now shows duplicate rows with fractured per-period subtotals. Group by the KEY only and aggregate the non-key descriptive columns with Max(): one row per based-on key (identical to the pre-#56192 MariaDB output) and still Postgres-valid. Supplier columns are master-joined / fetch-locked (functionally dependent) so they stay unchanged. --- erpnext/controllers/trends.py | 23 +++++++++++-------- .../test_sales_order_trends.py | 5 ++-- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/erpnext/controllers/trends.py b/erpnext/controllers/trends.py index 03e2b483d14..82b8bd30088 100644 --- a/erpnext/controllers/trends.py +++ b/erpnext/controllers/trends.py @@ -369,8 +369,11 @@ def based_wise_columns_query(based_on, trans): # based_on_cols, based_on_select, based_on_group_by, addl_tables if based_on == "Item": based_on_details["based_on_cols"] = ["Item:Link/Item:120", "Item Name:Data:120"] - based_on_details["based_on_select"] = "t2.item_code, t2.item_name," - based_on_details["based_on_group_by"] = "t2.item_code, t2.item_name" + # item_name is an editable per-line field, not functionally dependent on item_code, so it + # is aggregated (one row per item_code) rather than added to GROUP BY (which would split + # the row and change the MariaDB row count). See get_data's group-by query. + based_on_details["based_on_select"] = "t2.item_code, Max(t2.item_name) as item_name," + based_on_details["based_on_group_by"] = "t2.item_code" based_on_details["addl_tables"] = "" elif based_on == "Item Group": @@ -386,19 +389,21 @@ def based_wise_columns_query(based_on, trans): "Party Name:Data:120", "Territory:Link/Territory:120", ] - based_on_details["based_on_select"] = "t1.party_name, t1.customer_name, t1.territory," + based_on_details[ + "based_on_select" + ] = "t1.party_name, Max(t1.customer_name) as customer_name, Max(t1.territory) as territory," else: based_on_details["based_on_cols"] = [ "Customer:Link/Customer:120", "Customer Name:Data:120", "Territory:Link/Territory:120", ] - based_on_details["based_on_select"] = "t1.customer, t1.customer_name, t1.territory," - based_on_details["based_on_group_by"] = ( - "t1.party_name, t1.customer_name, t1.territory" - if trans == "Quotation" - else "t1.customer, t1.customer_name, t1.territory" - ) + based_on_details[ + "based_on_select" + ] = "t1.customer, Max(t1.customer_name) as customer_name, Max(t1.territory) as territory," + # territory (and customer_name) are not functionally dependent on the customer key, so they + # are aggregated rather than grouped — one row per customer, matching the prior MariaDB output. + based_on_details["based_on_group_by"] = "t1.party_name" if trans == "Quotation" else "t1.customer" based_on_details["addl_tables"] = "" elif based_on == "Customer Group": diff --git a/erpnext/selling/report/sales_order_trends/test_sales_order_trends.py b/erpnext/selling/report/sales_order_trends/test_sales_order_trends.py index 9fbae8f2e31..61d768d410c 100644 --- a/erpnext/selling/report/sales_order_trends/test_sales_order_trends.py +++ b/erpnext/selling/report/sales_order_trends/test_sales_order_trends.py @@ -7,8 +7,9 @@ from erpnext.tests.utils import ERPNextTestSuite class TestSalesOrderTrends(ERPNextTestSuite): def test_report_executes_with_group_by(self): # trends.get_data builds per-period SUM(CASE ...) aggregates (converted from MySQL SUM(IF)), - # with a GROUP BY widened to every selected non-aggregated column and a based_on_key for the - # group-by detail subqueries. Setting group_by exercises that full path on both engines. + # groups by the based-on KEY only (non-key descriptive columns like item_name/territory are + # MAX()-aggregated so the report stays one row per key on both engines), and uses a based_on_key + # for the group-by detail subqueries. Setting group_by exercises that full path on both engines. from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order from erpnext.selling.report.sales_order_trends.sales_order_trends import execute