Repository: zeppelin Updated Branches: refs/heads/master 971631c1a -> f8cd64cb5
[ZEPPELIN-3091] Correct aggregation functionality in charts ### What is this PR for? The aggregation functions interpret NaN columns as 1 which leads to incorrect output being shown in charts. This PR fixes this by correcting the sum, min, max and average aggregation method. ### What type of PR is it? Bug Fix ### Todos ### What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-3091 ### How should this be tested? * Update the data field of a paragraph results with %table to contain null values, e.g. `"data": "age\tvalue\n19\t4\n20\t3\n21\t7\n22\t9\n23\t20\n24\t24\n25\t44\n26\t77\n27\t94\n28\t103\n29\t97\n20\t5\n20\tnull\n"` The "null" values should be ignored for sum, min, max but included for count (and hence average). ### Screenshots (if appropriate)  ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: Naman Mishra <[email protected]> Closes #2696 from namanmishra91/ZEPPELIN-3091 and squashes the following commits: d8a57c238 [Naman Mishra] Add test 38ad39c65 [Naman Mishra] Merge branch 'master' into ZEPPELIN-3091 568ae3f2a [Naman Mishra] Correct aggregation functionality in charts Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/f8cd64cb Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/f8cd64cb Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/f8cd64cb Branch: refs/heads/master Commit: f8cd64cb50028031ab321144697d98822a60c63f Parents: 971631c Author: Naman Mishra <[email protected]> Authored: Tue Dec 12 10:45:34 2017 +0530 Committer: Lee moon soo <[email protected]> Committed: Thu Dec 14 08:39:14 2017 -0800 ---------------------------------------------------------------------- zeppelin-web/src/app/tabledata/pivot.js | 36 +++++++++++----- .../src/app/tabledata/tabledata.test.js | 45 ++++++++++++++++++++ 2 files changed, 71 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/f8cd64cb/zeppelin-web/src/app/tabledata/pivot.js ---------------------------------------------------------------------- diff --git a/zeppelin-web/src/app/tabledata/pivot.js b/zeppelin-web/src/app/tabledata/pivot.js index a0f61b2..da29900 100644 --- a/zeppelin-web/src/app/tabledata/pivot.js +++ b/zeppelin-web/src/app/tabledata/pivot.js @@ -138,8 +138,8 @@ export default class PivotTransformation extends Transformation { pivot (data, keys, groups, values) { let aggrFunc = { sum: function (a, b) { - let varA = (a !== undefined) ? (isNaN(a) ? 1 : parseFloat(a)) : 0 - let varB = (b !== undefined) ? (isNaN(b) ? 1 : parseFloat(b)) : 0 + let varA = (a !== undefined) ? (isNaN(a) ? 0 : parseFloat(a)) : 0 + let varB = (b !== undefined) ? (isNaN(b) ? 0 : parseFloat(b)) : 0 return varA + varB }, count: function (a, b) { @@ -148,22 +148,38 @@ export default class PivotTransformation extends Transformation { return varA + varB }, min: function (a, b) { - let varA = (a !== undefined) ? (isNaN(a) ? 1 : parseFloat(a)) : 0 - let varB = (b !== undefined) ? (isNaN(b) ? 1 : parseFloat(b)) : 0 - return Math.min(varA, varB) + let aIsValid = isValidNumber(a) + let bIsValid = isValidNumber(b) + if (!aIsValid) { + return parseFloat(b) + } else if (!bIsValid) { + return parseFloat(a) + } else { + return Math.min(parseFloat(a), parseFloat(b)) + } }, max: function (a, b) { - let varA = (a !== undefined) ? (isNaN(a) ? 1 : parseFloat(a)) : 0 - let varB = (b !== undefined) ? (isNaN(b) ? 1 : parseFloat(b)) : 0 - return Math.max(varA, varB) + let aIsValid = isValidNumber(a) + let bIsValid = isValidNumber(b) + if (!aIsValid) { + return parseFloat(b) + } else if (!bIsValid) { + return parseFloat(a) + } else { + return Math.max(parseFloat(a), parseFloat(b)) + } }, avg: function (a, b, c) { - let varA = (a !== undefined) ? (isNaN(a) ? 1 : parseFloat(a)) : 0 - let varB = (b !== undefined) ? (isNaN(b) ? 1 : parseFloat(b)) : 0 + let varA = (a !== undefined) ? (isNaN(a) ? 0 : parseFloat(a)) : 0 + let varB = (b !== undefined) ? (isNaN(b) ? 0 : parseFloat(b)) : 0 return varA + varB } } + let isValidNumber = function(num) { + return num !== undefined && !isNaN(num) + } + let aggrFuncDiv = { sum: false, count: false, http://git-wip-us.apache.org/repos/asf/zeppelin/blob/f8cd64cb/zeppelin-web/src/app/tabledata/tabledata.test.js ---------------------------------------------------------------------- diff --git a/zeppelin-web/src/app/tabledata/tabledata.test.js b/zeppelin-web/src/app/tabledata/tabledata.test.js index 3de2fa3..e24b073 100644 --- a/zeppelin-web/src/app/tabledata/tabledata.test.js +++ b/zeppelin-web/src/app/tabledata/tabledata.test.js @@ -83,4 +83,49 @@ describe('PivotTransformation build', function() { expect(config.common.pivot.keys[1].index).toBe(3) expect(config.common.pivot.keys[2].index).toBe(5) }) + + it('should aggregate values correctly', function() { + let td = new TableData() + td.loadParagraphResult({ + type: 'TABLE', + msg: 'key\tvalue\na\t10\na\tnull\na\t0\na\t1\n' + }) + + let config = { + common: { + pivot: { + keys: [ + { + 'name': 'key', + 'index': 0.0, + } + ], + groups: [], + values: [ + { + 'name': 'value', + 'index': 1.0, + 'aggr': 'sum' + } + ] + } + } + } + + pt.setConfig(config) + let transformed = pt.transform(td) + expect(transformed.rows['a']['value(sum)'].value).toBe(11) + + pt.config.common.pivot.values[0].aggr = 'max' + transformed = pt.transform(td) + expect(transformed.rows['a']['value(max)'].value).toBe(10) + + pt.config.common.pivot.values[0].aggr = 'min' + transformed = pt.transform(td) + expect(transformed.rows['a']['value(min)'].value).toBe(0) + + pt.config.common.pivot.values[0].aggr = 'count' + transformed = pt.transform(td) + expect(transformed.rows['a']['value(count)'].value).toBe(4) + }) })
