mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-18 08:58:43 +00:00
fix(controllers): make trends queries Postgres-valid (SUM(CASE), GROUP BY)
The *_trends reports (Sales/Purchase Order/Invoice, Delivery Note, etc.) built raw SQL that is invalid on Postgres: - `SUM(IF(...))` -> `SUM(CASE WHEN ... ELSE NULL END)` (IF is MySQL-only). - Loose GROUP BY: each based_on `group by` listed only the key column while the SELECT also returned name/territory/group/currency columns. Widen the GROUP BY to include every selected non-aggregated column so the query is valid on Postgres. - Add a based_on_key (the first group-by column) for the group-by detail subqueries, which equate against a single column (a multi-column group_by spliced into an equality produced malformed SQL on both engines). Behaviour note: widening the GROUP BY can split one based-on group into multiple report rows when the snapshot columns (territory, renamed customer/item) differ across transactions, vs MariaDB's previous one-arbitrary-row-per-group. Grand totals are unchanged (calculate_total_row); per-group subtotals become deterministic partial sums. This is the accepted widen-vs-arbitrary-pick tradeoff. Adds a test (no test file existed) running Sales Order Trends with a group_by, exercising the widened GROUP BY / based_on_key / SUM(CASE) on both engines. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,26 @@
|
||||
# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# License: GNU General Public License v3. See license.txt
|
||||
|
||||
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.
|
||||
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
|
||||
|
||||
make_sales_order(item_code="_Test Item", qty=3, rate=100)
|
||||
|
||||
filters = {
|
||||
"company": "_Test Company",
|
||||
"period": "Monthly",
|
||||
"based_on": "Item",
|
||||
"group_by": "Customer",
|
||||
}
|
||||
columns, data, _chart_none, _chart = execute(filters)
|
||||
|
||||
self.assertTrue(columns)
|
||||
self.assertTrue(any("_Test Item" in [str(cell) for cell in row] for row in data))
|
||||
Reference in New Issue
Block a user