SBIN2010 commented on code in PR #35322:
URL: https://github.com/apache/superset/pull/35322#discussion_r2495927449
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/utilities.js:
##########
@@ -129,6 +129,101 @@ const naturalSort = (as, bs) => {
return a.length - b.length;
};
+/**
+ * Targets: https://github.com/apache/superset/issues/20564 issue
+ * Topic: order by value (asc/desc)
+ * Fixed problems:
+ * 1. valid ordering with multiple rows/columns (groupping due to subtotal
values)
+ * 2. guarantee for normal `subtotal` row/col position both
top(left)/bottom(right) in any asc/desc case
+ * 3. fixed a-z order in equal group/row aggregate values
+ * KNOWN ISSUES: now is applyable only for monotonic sumable aggregates (sum,
count, avg), not for min/max
+ * @param keys - [[key,...],...] - array of row or col keys
+ * @param dataFunc - function to retrieve data by key (for natural sum order)
+ * @param top - bool - top/left (true) or bottom/right (false) position of
subtotal rows
+ * @param asc - ascending(true) or descending(false) sort
+ * NOTE: we REQUIRE to use `asc` parameter and not just
`(+|-)groupingValueSort(keys, dataFunc, top)`
+ * because top/bottom position should be evaluated correctly without
dependency of asc/desc
+ */
+const groupingValueSort = (keys, dataFunc, top, asc) => {
+ // precalculate subtotal for every group and subgroup
+ const groups = {};
+ for (let i = 0; i < keys.length; i += 1) {
+ const rk = keys[i];
+ let current = groups;
+ // we should not visit whole key ignore last key part (only groups)
+ for (let ki = 0; ki < rk.length - 1; ki += 1) {
+ const k = rk[ki];
+ if (!current[k]) {
+ //
+ current[k] = { auto_agg_sum: 0 };
+ }
+ current[k].auto_agg_sum += dataFunc(rk, []);
+ current = current[k];
+ }
+ }
Review Comment:
It looks good. But I'd break the groupingValueSort function into smaller
functions for better code readability.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
