mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-12 06:01:46 +00:00
fix(controllers): keep Sales/Purchase Trends one row per based-on key (MariaDB parity)
#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.
This commit is contained in:
@@ -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":
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user