Used st_collect only when it's needed

lib/node/nodes/centroid.js

@@ -1,17 +1,32 @@
 'use strict';
 
 var Node = require('../node');
+var debug = require('../../util/debug')('analysis:centroid');
 
 var TYPE = 'centroid';
 
 var PARAMS = {
     source : Node.PARAM.NODE(Node.GEOMETRY.ANY),
     category_column : Node.PARAM.NULLABLE(Node.PARAM.STRING()),
-    aggregation: Node.PARAM.NULLABLE(Node.PARAM.STRING(), 'count'),
-    aggregation_column: Node.PARAM.NULLABLE(Node.PARAM.STRING())
+    aggregation: Node.PARAM.NULLABLE(Node.PARAM.ENUM('avg', 'count', 'max', 'min', 'sum'), null),
+    aggregation_column: Node.PARAM.NULLABLE(Node.PARAM.STRING()),
 };
 
-var Centroid = Node.create(TYPE, PARAMS);
+var Centroid = Node.create(TYPE, PARAMS, { version: 1,
+    beforeCreate: function(node) {
+        debug(node);
+
+        if (node.aggregation && node.aggregation !== 'count' && node.aggregation_column === null) {
+            throw new Error('Param `aggregation` != "count" requires an existent `aggregation_column` column');
+        }
+        if (node.aggregation === null && node.aggregation_column !== null) {
+            throw new Error('Param `aggregation_column` requires an `aggregation` operation');
+        }
+        if (node.aggregation === 'count' && node.aggregation_column === null) {
+            node.aggregation_column = '*';
+        }
+    }
+});
 
 module.exports = Centroid;
 module.exports.TYPE = TYPE;
@@ -20,22 +35,52 @@ module.exports.PARAMS = PARAMS;
 var centroidTemplate = Node.template([
     'SELECT',
     '  row_number() over() as cartodb_id,',
-    '  ST_Centroid(ST_Collect(the_geom)) as the_geom,',
-    '  {{? it.categoryColumn }}{{=it.categoryColumn}} as category,{{?}}',
-    '  {{=it.aggregation}} as value',
+    '  {{=it.columns}}',
     'FROM ({{=it.query}}) q',
-    '{{? it.categoryColumn }}GROUP BY {{=it.categoryColumn}}{{?}}'
+    '{{? it.categoryColumn }} GROUP BY {{=it.categoryColumn}}{{?}}'
 ].join('\n'));
 
-var aggregationFnQueryTpl = Node.template('{{=it._aggregationFn}}({{=it._aggregationColumn}})');
+var centroidColumnTemplate = Node.template([
+    'ST_Centroid({{? it.aggregation || it.categoryColumn }}ST_Collect(the_geom){{??}}the_geom{{?}}) as the_geom'
+]);
 
-Centroid.prototype.sql = function(){
-    return centroidTemplate({
-        query: this.source.getQuery(),
-        categoryColumn: this.category_column,
-        aggregation: aggregationFnQueryTpl({
-            _aggregationFn: this.aggregation,
-            _aggregationColumn: this.aggregation_column || 1
+var categoryColumnTemplate = Node.template([
+    '{{=it.categoryColumn}} as category'
+]);
+
+var aggregationColumnTemplate = Node.template([
+    '{{=it.aggregation}}({{=it.aggregationColumn}}) as value',
+]);
+
+Centroid.prototype.sql = function() {
+    var columns = [
+        centroidColumnTemplate({
+            aggregation: this.aggregation,
+            aggregationColumn: this.aggregation_column,
+            categoryColumn: this.category_column
         })
+    ];
+
+    if (this.category_column) {
+        columns.push(categoryColumnTemplate({
+            categoryColumn: this.category_column
+        }));
+    }
+
+    if (this.aggregation && this.aggregation_column) {
+        columns.push(aggregationColumnTemplate({
+            aggregation: this.aggregation,
+            aggregationColumn: this.aggregation_column
+        }));
+    }
+
+    var sql = centroidTemplate({
+        query: this.source.getQuery(),
+        columns: columns.join(', '),
+        categoryColumn: this.category_column
     });
+
+    debug(sql);
+
+    return sql;
 };

test/acceptance/centroid.js

@@ -0,0 +1,217 @@
+'use strict';
+
+var assert = require('assert');
+
+var Analysis = require('../../lib/analysis');
+
+var testConfig = require('../test-config');
+var QueryRunner = require('../../lib/postgresql/query-runner');
+
+describe('centroid analysis', function() {
+
+    var queryRunner;
+
+    before(function() {
+        queryRunner = new QueryRunner(testConfig.db);
+    });
+
+    var QUERY = 'select * from atm_machines';
+    var CATEGORY_COLUMN = 'kind';
+    var AGGREGATION = 'avg';
+    var AGGREGATION_COLUMN = 'cartodb_id';
+
+    var sourceAtmMachines = {
+        type: 'source',
+        params: {
+            query: QUERY
+        }
+    };
+
+    var config = testConfig.create({
+        batch: {
+            inlineExecution: true
+        }
+    });
+
+    function performAnalysis(definition, callback) {
+        Analysis.create(config, definition, function (err, analysis) {
+            if (err) {
+                return callback(err);
+            }
+
+            queryRunner.run(analysis.getQuery(), function(err, result) {
+                if (err) {
+                    return callback(err);
+                }
+
+                assert.ok(Array.isArray(result.rows));
+                var values = result.rows.map(function (value) {
+                    return value;
+                });
+
+                callback(null, values);
+            });
+        });
+    }
+
+    describe('non optional params', function () {
+        var centroidDefinition = {
+            type: 'centroid',
+            params: {
+                source: sourceAtmMachines,
+            }
+        };
+
+        it('should create an analysis', function (done) {
+            performAnalysis(centroidDefinition, function (err, values) {
+                if(err) {
+                    return done(err);
+                }
+                assert.ok(values);
+                done();
+            });
+        });
+    });
+
+    describe('with category column', function () {
+        var centroidDefinition = {
+            type: 'centroid',
+            params: {
+                source: sourceAtmMachines,
+                category_column: CATEGORY_COLUMN
+            }
+        };
+
+        it('should create an analysis', function (done) {
+            performAnalysis(centroidDefinition, function (err, values) {
+                if(err) {
+                    return done(err);
+                }
+                assert.ok(values);
+                done();
+            });
+        });
+    });
+
+    describe('with category column and aggregation (miss aggregation_column)', function () {
+        var centroidDefinition = {
+            type: 'centroid',
+            params: {
+                source: sourceAtmMachines,
+                category_column: CATEGORY_COLUMN,
+                aggregation: AGGREGATION
+            }
+        };
+
+        it('should return error', function (done) {
+            performAnalysis(centroidDefinition, function (err, values) {
+                assert.ok(err);
+                assert.ok(!values);
+                done();
+            });
+        });
+    });
+
+    describe('with category column, aggregation and aggregation column', function () {
+        var centroidDefinition = {
+            type: 'centroid',
+            params: {
+                source: sourceAtmMachines,
+                category_column: CATEGORY_COLUMN,
+                aggregation: AGGREGATION,
+                aggregation_column: AGGREGATION_COLUMN
+            }
+        };
+
+        it('should create an analysis', function (done) {
+            performAnalysis(centroidDefinition, function (err, values) {
+                if(err) {
+                    return done(err);
+                }
+                assert.ok(values);
+                done();
+            });
+        });
+    });
+
+
+    describe('with aggregation (miss aggregation_colum for avg method)', function () {
+        var centroidDefinition = {
+            type: 'centroid',
+            params: {
+                source: sourceAtmMachines,
+                aggregation: AGGREGATION
+            }
+        };
+
+        it('should return error', function (done) {
+            performAnalysis(centroidDefinition, function (err, values) {
+                assert.ok(err);
+                assert.ok(!values);
+                done();
+            });
+        });
+    });
+
+
+    describe('with aggregation (miss aggregation_colum for avg method)', function () {
+        var centroidDefinition = {
+            type: 'centroid',
+            params: {
+                source: sourceAtmMachines,
+                aggregation: 'count'
+            }
+        };
+
+        it('should create an analysis', function (done) {
+            performAnalysis(centroidDefinition, function (err, values) {
+                if(err) {
+                    return done(err);
+                }
+                assert.ok(values);
+                done();
+            });
+        });
+    });
+
+    describe('with aggregation and aggregation column', function () {
+        var centroidDefinition = {
+            type: 'centroid',
+            params: {
+                source: sourceAtmMachines,
+                aggregation: AGGREGATION,
+                aggregation_column: AGGREGATION_COLUMN
+            }
+        };
+
+        it('should create an analysis', function (done) {
+            performAnalysis(centroidDefinition, function (err, values) {
+                if(err) {
+                    return done(err);
+                }
+                assert.ok(values);
+                done();
+            });
+        });
+    });
+
+
+    describe('with category_column and aggregation column', function () {
+        var centroidDefinition = {
+            type: 'centroid',
+            params: {
+                source: sourceAtmMachines,
+                category_column: CATEGORY_COLUMN,
+                aggregation_column: AGGREGATION_COLUMN
+            }
+        };
+
+        it('should return error', function (done) {
+            performAnalysis(centroidDefinition, function (err, values) {
+                assert.ok(err);
+                assert.ok(!values);
+                done();
+            });
+        });
+    });
+});