From a4a1fb930aae8b45711cae543409cbd36c260c81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 17 Jan 2017 17:09:17 +0100 Subject: [PATCH] Be able to not compute NULL categories and null values wheter aggregation operation is not 'count' --- lib/cartodb/models/dataview/aggregation.js | 43 +++++++++++++++++----- test/acceptance/dataviews/aggregation.js | 20 ++++++++-- 2 files changed, 50 insertions(+), 13 deletions(-) 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/test/acceptance/dataviews/aggregation.js b/test/acceptance/dataviews/aggregation.js index 1f45c0c5..9fc9a219 100644 --- a/test/acceptance/dataviews/aggregation.js +++ b/test/acceptance/dataviews/aggregation.js @@ -71,7 +71,14 @@ describe('aggregations happy cases', function() { ].join(' UNION ALL '); operations.forEach(function (operation) { - it('should handle NULL values in category and aggregation columns using "' + operation + '" as aggregation operation', function (done) { + 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); @@ -79,15 +86,22 @@ describe('aggregations happy cases', function() { 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) { - assert.ok(category.value > 0); hasNullCategory = true; } }); - assert.ok(hasNullCategory, 'there is no category NULL'); + + if (operation === 'count') { + assert.ok(hasNullCategory, 'aggregation has not a category NULL'); + } else { + assert.ok(!hasNullCategory, 'aggregation has category NULL'); + } done(); });