diff --git a/lib/cartodb/models/dataview/aggregation.js b/lib/cartodb/models/dataview/aggregation.js index 3b0642b9..a80efbc7 100644 --- a/lib/cartodb/models/dataview/aggregation.js +++ b/lib/cartodb/models/dataview/aggregation.js @@ -19,25 +19,37 @@ var rankedCategoriesQueryTpl = dot.template([ ' SELECT {{=it._column}} AS category, {{=it._aggregation}} AS value,', ' row_number() OVER (ORDER BY {{=it._aggregation}} desc) as rank', ' FROM ({{=it._query}}) _cdb_aggregation_all', + ' {{?it._aggregationColumn!==null}}WHERE {{=it._aggregationColumn}} IS NOT NULL{{?}}', ' GROUP BY {{=it._column}}', ' ORDER BY 2 DESC', ')' ].join('\n')); -var categoriesSummaryQueryTpl = dot.template([ - 'categories_summary AS(', - ' SELECT count(1) categories_count, max(value) max_val, min(value) min_val', +var categoriesSummaryMinMaxQueryTpl = dot.template([ + 'categories_summary_min_max AS(', + ' SELECT max(value) max_val, min(value) min_val', ' FROM categories', ')' ].join('\n')); +var categoriesSummaryCountQueryTpl = dot.template([ + 'categories_summary_count AS(', + ' SELECT count(1) AS categories_count', + ' FROM (', + ' SELECT {{=it._column}} AS category', + ' FROM ({{=it._query}}) _cdb_categories', + ' GROUP BY {{=it._column}}', + ' ) _cdb_categories_count', + ')' +].join('\n')); + var rankedAggregationQueryTpl = dot.template([ 'SELECT CAST(category AS text), value, false as agg, nulls_count, min_val, max_val, count, categories_count', - ' FROM categories, summary, categories_summary', + ' 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', - ' FROM categories, summary, categories_summary', + ' 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' ].join('\n')); @@ -45,7 +57,7 @@ var rankedAggregationQueryTpl = dot.template([ var aggregationQueryTpl = dot.template([ 'SELECT CAST({{=it._column}} AS text) AS category, {{=it._aggregation}} AS value, false as agg,', ' nulls_count, min_val, max_val, count, categories_count', - 'FROM ({{=it._query}}) _cdb_aggregation_all, summary, categories_summary', + 'FROM ({{=it._query}}) _cdb_aggregation_all, summary, categories_summary_min_max, categories_summary_count', 'GROUP BY category, nulls_count, min_val, max_val, count, categories_count', 'ORDER BY value DESC' ].join('\n')); @@ -114,6 +126,7 @@ Aggregation.prototype.sql = function(psql, override, callback) { var _query = this.query; var aggregationSql; + if (!!override.ownFilter) { aggregationSql = [ "WITH", @@ -125,9 +138,14 @@ Aggregation.prototype.sql = function(psql, override, callback) { rankedCategoriesQueryTpl({ _query: _query, _column: this.column, - _aggregation: this.getAggregationSql() + _aggregation: this.getAggregationSql(), + _aggregationColumn: this.aggregation !== 'count' ? this.aggregationColumn : null }), - categoriesSummaryQueryTpl({ + categoriesSummaryMinMaxQueryTpl({ + _query: _query, + _column: this.column + }), + categoriesSummaryCountQueryTpl({ _query: _query, _column: this.column }) @@ -150,9 +168,14 @@ Aggregation.prototype.sql = function(psql, override, callback) { rankedCategoriesQueryTpl({ _query: _query, _column: this.column, - _aggregation: this.getAggregationSql() + _aggregation: this.getAggregationSql(), + _aggregationColumn: this.aggregation !== 'count' ? this.aggregationColumn : null }), - categoriesSummaryQueryTpl({ + categoriesSummaryMinMaxQueryTpl({ + _query: _query, + _column: this.column + }), + categoriesSummaryCountQueryTpl({ _query: _query, _column: this.column }) diff --git a/lib/cartodb/models/dataview/overviews/aggregation.js b/lib/cartodb/models/dataview/overviews/aggregation.js index a3d01b06..da63b27f 100644 --- a/lib/cartodb/models/dataview/overviews/aggregation.js +++ b/lib/cartodb/models/dataview/overviews/aggregation.js @@ -18,25 +18,37 @@ var rankedCategoriesQueryTpl = dot.template([ ' SELECT {{=it._column}} AS category, {{=it._aggregation}} AS value,', ' row_number() OVER (ORDER BY {{=it._aggregation}} desc) as rank', ' FROM ({{=it._query}}) _cdb_aggregation_all', + ' {{?it._aggregationColumn!==null}}WHERE {{=it._aggregationColumn}} IS NOT NULL{{?}}', ' GROUP BY {{=it._column}}', ' ORDER BY 2 DESC', ')' ].join('\n')); -var categoriesSummaryQueryTpl = dot.template([ - 'categories_summary AS(', - ' SELECT count(1) categories_count, max(value) max_val, min(value) min_val', +var categoriesSummaryMinMaxQueryTpl = dot.template([ + 'categories_summary_min_max AS(', + ' SELECT max(value) max_val, min(value) min_val', ' FROM categories', ')' ].join('\n')); +var categoriesSummaryCountQueryTpl = dot.template([ + 'categories_summary_count AS(', + ' SELECT count(1) AS categories_count', + ' FROM (', + ' SELECT {{=it._column}} AS category', + ' FROM ({{=it._query}}) _cdb_categories', + ' GROUP BY {{=it._column}}', + ' ) _cdb_categories_count', + ')' +].join('\n')); + var rankedAggregationQueryTpl = dot.template([ 'SELECT CAST(category AS text), value, false as agg, nulls_count, min_val, max_val, count, categories_count', - ' FROM categories, summary, categories_summary', + ' 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', - ' FROM categories, summary, categories_summary', + ' 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' ].join('\n')); @@ -44,7 +56,7 @@ var rankedAggregationQueryTpl = dot.template([ var aggregationQueryTpl = dot.template([ 'SELECT CAST({{=it._column}} AS text) AS category, {{=it._aggregation}} AS value, false as agg,', ' nulls_count, min_val, max_val, count, categories_count', - 'FROM ({{=it._query}}) _cdb_aggregation_all, summary, categories_summary', + 'FROM ({{=it._query}}) _cdb_aggregation_all, summary, categories_summary_min_max, categories_summary_count', 'GROUP BY category, nulls_count, min_val, max_val, count, categories_count', 'ORDER BY value DESC' ].join('\n')); @@ -85,9 +97,14 @@ Aggregation.prototype.sql = function(psql, override, callback) { rankedCategoriesQueryTpl({ _query: _query, _column: this.column, - _aggregation: this.getAggregationSql() + _aggregation: this.getAggregationSql(), + _aggregationColumn: this.aggregation !== 'count' ? this.aggregationColumn : null }), - categoriesSummaryQueryTpl({ + categoriesSummaryMinMaxQueryTpl({ + _query: _query, + _column: this.column + }), + categoriesSummaryCountQueryTpl({ _query: _query, _column: this.column }) @@ -110,9 +127,14 @@ Aggregation.prototype.sql = function(psql, override, callback) { rankedCategoriesQueryTpl({ _query: _query, _column: this.column, - _aggregation: this.getAggregationSql() + _aggregation: this.getAggregationSql(), + _aggregationColumn: this.aggregation !== 'count' ? this.aggregationColumn : null }), - categoriesSummaryQueryTpl({ + categoriesSummaryMinMaxQueryTpl({ + _query: _query, + _column: this.column + }), + categoriesSummaryCountQueryTpl({ _query: _query, _column: this.column }) diff --git a/test/acceptance/dataviews/aggregation.js b/test/acceptance/dataviews/aggregation.js index 2986a174..9fc9a219 100644 --- a/test/acceptance/dataviews/aggregation.js +++ b/test/acceptance/dataviews/aggregation.js @@ -3,7 +3,7 @@ require('../../support/test_helper'); var assert = require('../../support/assert'); var TestClient = require('../../support/test-client'); -describe('aggregations', function() { +describe('aggregations happy cases', function() { afterEach(function(done) { if (this.testClient) { @@ -13,30 +13,36 @@ describe('aggregations', function() { } }); - function aggregationOperationMapConfig(operation) { - return { + function aggregationOperationMapConfig(operation, query, column, aggregationColumn) { + column = column || 'adm0name'; + aggregationColumn = aggregationColumn || 'pop_max'; + query = query || 'select * from populated_places_simple_reduced'; + + var mapConfig = { version: '1.5.0', layers: [ { type: 'mapnik', options: { - sql: 'select * from populated_places_simple_reduced', + sql: query, cartocss: '#layer0 { marker-fill: red; marker-width: 10; }', cartocss_version: '2.0.1', - widgets: { - adm0name: { - type: 'aggregation', - options: { - column: 'adm0name', - aggregation: operation, - aggregationColumn: 'pop_max' - } - } - } + widgets: {} } } ] }; + + mapConfig.layers[0].options.widgets[column] = { + type: 'aggregation', + options: { + column: column, + aggregation: operation, + aggregationColumn: aggregationColumn + } + }; + + return mapConfig; } var operations = ['count', 'sum', 'avg', 'max', 'min']; @@ -56,4 +62,49 @@ describe('aggregations', function() { }); }); }); + + var query = [ + 'select 1 as val, \'a\' as cat, ST_Transform(ST_SetSRID(ST_MakePoint(0,0),4326),3857) as the_geom_webmercator', + 'select null, \'b\', ST_Transform(ST_SetSRID(ST_MakePoint(0,1),4326),3857)', + 'select null, \'b\', ST_Transform(ST_SetSRID(ST_MakePoint(1,0),4326),3857)', + 'select null, null, ST_Transform(ST_SetSRID(ST_MakePoint(1,1),4326),3857)' + ].join(' UNION ALL '); + + operations.forEach(function (operation) { + var not = operation === 'count' ? ' not ' : ' '; + var description = 'should' + + not + + 'handle NULL values in category and aggregation columns using "' + + operation + + '" as aggregation operation'; + + it(description, function (done) { + this.testClient = new TestClient(aggregationOperationMapConfig(operation, query, '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, 3); + assert.equal(aggregation.count, 4); + assert.equal(aggregation.nulls, 1); + + var hasNullCategory = false; + aggregation.categories.forEach(function (category) { + if (category.category === null) { + hasNullCategory = true; + } + }); + + if (operation === 'count') { + assert.ok(hasNullCategory, 'aggregation has not a category NULL'); + } else { + assert.ok(!hasNullCategory, 'aggregation has category NULL'); + } + + done(); + }); + }); + }); });