Other category now uses the selected aggregated function (#633)

* Other category in category widget uses selected aggregation function
Fixes https://github.com/CartoDB/Windshaft-cartodb/issues/628

NEWS.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## 2.88.3
+Released 2017-03-02
+
+Bug fixes:
+- Category dataviews now uses the proper aggregation function for the 'Other' category. See https://github.com/CartoDB/Windshaft-cartodb/issues/628
+
 ## 2.88.2
 Released 2017-02-23
 

lib/cartodb/models/dataview/aggregation.js

@@ -48,7 +48,8 @@ var rankedAggregationQueryTpl = dot.template([
     '  FROM categories, summary, categories_summary_min_max, categories_summary_count',
     '  WHERE rank < {{=it._limit}}',
     'UNION ALL',
-    'SELECT \'Other\' category, sum(value), true as agg, nulls_count, min_val, max_val, count, categories_count',
+    'SELECT \'Other\' category, {{=it._aggregationFn}}(value) as value, true as agg, nulls_count, min_val, max_val,',
+    '       count, categories_count',
     '  FROM categories, summary, categories_summary_min_max, categories_summary_count',
     '  WHERE rank >= {{=it._limit}}',
     'GROUP BY nulls_count, min_val, max_val, count, categories_count'
@@ -129,27 +130,7 @@ Aggregation.prototype.sql = function(psql, override, callback) {
 
     if (!!override.ownFilter) {
         aggregationSql = [
-            "WITH",
-            [
-                summaryQueryTpl({
-                    _query: _query,
-                    _column: this.column
-                }),
-                rankedCategoriesQueryTpl({
-                    _query: _query,
-                    _column: this.column,
-                    _aggregation: this.getAggregationSql(),
-                    _aggregationColumn: this.aggregation !== 'count' ? this.aggregationColumn : null
-                }),
-                categoriesSummaryMinMaxQueryTpl({
-                    _query: _query,
-                    _column: this.column
-                }),
-                categoriesSummaryCountQueryTpl({
-                    _query: _query,
-                    _column: this.column
-                })
-            ].join(',\n'),
+            this.getCategoriesCTESql(_query, this.column, this.aggregation, this.aggregationColumn),
             aggregationQueryTpl({
                 _query: _query,
                 _column: this.column,
@@ -159,30 +140,11 @@ Aggregation.prototype.sql = function(psql, override, callback) {
         ].join('\n');
     } else {
         aggregationSql = [
-            "WITH",
-            [
-                summaryQueryTpl({
-                    _query: _query,
-                    _column: this.column
-                }),
-                rankedCategoriesQueryTpl({
-                    _query: _query,
-                    _column: this.column,
-                    _aggregation: this.getAggregationSql(),
-                    _aggregationColumn: this.aggregation !== 'count' ? this.aggregationColumn : null
-                }),
-                categoriesSummaryMinMaxQueryTpl({
-                    _query: _query,
-                    _column: this.column
-                }),
-                categoriesSummaryCountQueryTpl({
-                    _query: _query,
-                    _column: this.column
-                })
-            ].join(',\n'),
+            this.getCategoriesCTESql(_query, this.column, this.aggregation, this.aggregationColumn),
             rankedAggregationQueryTpl({
                 _query: _query,
                 _column: this.column,
+                _aggregationFn: this.aggregation !== 'count' ? this.aggregation : 'sum',
                 _limit: CATEGORIES_LIMIT
             })
         ].join('\n');
@@ -193,6 +155,32 @@ Aggregation.prototype.sql = function(psql, override, callback) {
     return callback(null, aggregationSql);
 };
 
+Aggregation.prototype.getCategoriesCTESql = function(query, column, aggregation, aggregationColumn) {
+    return [
+                "WITH",
+                [
+                    summaryQueryTpl({
+                        _query: query,
+                        _column: column
+                    }),
+                    rankedCategoriesQueryTpl({
+                        _query: query,
+                        _column: column,
+                        _aggregation: this.getAggregationSql(),
+                        _aggregationColumn: aggregation !== 'count' ? aggregationColumn : null
+                    }),
+                    categoriesSummaryMinMaxQueryTpl({
+                        _query: query,
+                        _column: column
+                    }),
+                    categoriesSummaryCountQueryTpl({
+                        _query: query,
+                        _column: column
+                    })
+                ].join(',\n')
+            ].join('\n');
+};
+
 var aggregationFnQueryTpl = dot.template('{{=it._aggregationFn}}({{=it._aggregationColumn}})');
 Aggregation.prototype.getAggregationSql = function() {
     return aggregationFnQueryTpl({

npm-shrinkwrap.json

@@ -1,6 +1,6 @@
 {
   "name": "windshaft-cartodb",
-  "version": "2.88.2",
+  "version": "2.88.3",
   "dependencies": {
     "body-parser": {
       "version": "1.14.2",
@@ -90,7 +90,7 @@
             },
             "mime-types": {
               "version": "2.1.14",
-              "from": "mime-types@>=2.1.7 <2.2.0",
+              "from": "mime-types@>=2.1.13 <2.2.0",
               "resolved": "https://registry.npmjs.org/mime-types/-/mime-types-2.1.14.tgz",
               "dependencies": {
                 "mime-db": {
@@ -228,9 +228,9 @@
               }
             },
             "safe-json-stringify": {
-              "version": "1.0.3",
+              "version": "1.0.4",
               "from": "safe-json-stringify@>=1.0.0 <2.0.0",
-              "resolved": "https://registry.npmjs.org/safe-json-stringify/-/safe-json-stringify-1.0.3.tgz"
+              "resolved": "https://registry.npmjs.org/safe-json-stringify/-/safe-json-stringify-1.0.4.tgz"
             },
             "moment": {
               "version": "2.17.1",
@@ -609,7 +609,7 @@
         },
         "type-is": {
           "version": "1.6.14",
-          "from": "type-is@>=1.6.10 <1.7.0",
+          "from": "type-is@>=1.6.6 <1.7.0",
           "resolved": "https://registry.npmjs.org/type-is/-/type-is-1.6.14.tgz",
           "dependencies": {
             "media-typer": {
@@ -869,9 +869,9 @@
               }
             },
             "is-my-json-valid": {
-              "version": "2.15.0",
+              "version": "2.16.0",
               "from": "is-my-json-valid@>=2.12.4 <3.0.0",
-              "resolved": "https://registry.npmjs.org/is-my-json-valid/-/is-my-json-valid-2.15.0.tgz",
+              "resolved": "https://registry.npmjs.org/is-my-json-valid/-/is-my-json-valid-2.16.0.tgz",
               "dependencies": {
                 "generate-function": {
                   "version": "2.0.0",
@@ -976,9 +976,9 @@
               }
             },
             "sshpk": {
-              "version": "1.10.2",
+              "version": "1.11.0",
               "from": "sshpk@>=1.7.0 <2.0.0",
-              "resolved": "https://registry.npmjs.org/sshpk/-/sshpk-1.10.2.tgz",
+              "resolved": "https://registry.npmjs.org/sshpk/-/sshpk-1.11.0.tgz",
               "dependencies": {
                 "asn1": {
                   "version": "0.2.3",
@@ -3919,7 +3919,7 @@
                     },
                     "minimatch": {
                       "version": "3.0.3",
-                      "from": "minimatch@>=2.0.0 <3.0.0||>=3.0.0 <4.0.0",
+                      "from": "minimatch@>=3.0.2 <4.0.0",
                       "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.0.3.tgz",
                       "dependencies": {
                         "brace-expansion": {
@@ -4030,7 +4030,7 @@
                     },
                     "minimatch": {
                       "version": "3.0.3",
-                      "from": "minimatch@>=2.0.0 <3.0.0||>=3.0.0 <4.0.0",
+                      "from": "minimatch@>=3.0.0 <4.0.0",
                       "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.0.3.tgz",
                       "dependencies": {
                         "brace-expansion": {

package.json

@@ -1,7 +1,7 @@
 {
   "private": true,
   "name": "windshaft-cartodb",
-  "version": "2.88.2",
+  "version": "2.88.3",
   "description": "A map tile server for CartoDB",
   "keywords": [
     "cartodb"

test/acceptance/dataviews/aggregation.js

@@ -1,5 +1,4 @@
 require('../../support/test_helper');
-
 var assert = require('../../support/assert');
 var TestClient = require('../../support/test-client');
 
@@ -107,4 +106,43 @@ describe('aggregations happy cases', function() {
             });
         });
     });
+
+    var operations_and_values = {'count': 9, 'sum': 45, 'avg': 5, 'max': 9, 'min': 1};
+
+    var query_other = [
+        'select generate_series(1,3) as val, \'other_a\' as cat, NULL as the_geom_webmercator',
+        'select generate_series(4,6) as val, \'other_b\' as cat, NULL as the_geom_webmercator',
+        'select generate_series(7,9) as val, \'other_c\' as cat, NULL as the_geom_webmercator',
+        'select generate_series(10,12) as val, \'category_1\' as cat, NULL as the_geom_webmercator',
+        'select generate_series(10,12) as val, \'category_2\' as cat, NULL as the_geom_webmercator',
+        'select generate_series(10,12) as val, \'category_3\' as cat, NULL as the_geom_webmercator',
+        'select generate_series(10,12) as val, \'category_4\' as cat, NULL as the_geom_webmercator',
+        'select generate_series(10,12) as val, \'category_5\' as cat, NULL as the_geom_webmercator'
+    ].join(' UNION ALL ');
+
+    Object.keys(operations_and_values).forEach(function (operation) {
+        var description = 'should aggregate OTHER category using "' + operation + '"';
+
+        it(description, function (done) {
+            this.testClient = new TestClient(aggregationOperationMapConfig(operation, query_other, 'cat', 'val'));
+            this.testClient.getDataview('cat', { own_filter: 0 }, function (err, aggregation) {
+                assert.ifError(err);
+
+                assert.ok(aggregation);
+                assert.equal(aggregation.type, 'aggregation');
+                assert.ok(aggregation.categories);
+                assert.equal(aggregation.categoriesCount, 8);
+                assert.equal(aggregation.count, 24);
+                assert.equal(aggregation.nulls, 0);
+
+                var aggregated_categories = aggregation.categories.filter( function(category) {
+                    return category.agg === true;
+                });
+                assert.equal(aggregated_categories.length, 1);
+                assert.equal(aggregated_categories[0].value, operations_and_values[operation]);
+
+                done();
+            });
+        });
+    });
 });