From 6c301403e37e4f1901c5d43d54e5928e2de20d22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 8 Jun 2017 15:59:33 +0200 Subject: [PATCH 01/34] Histogram going red: fails while quering Infinity and NanN values --- test/acceptance/dataviews/histogram.js | 86 +++++++++++++++++++++++--- 1 file changed, 76 insertions(+), 10 deletions(-) diff --git a/test/acceptance/dataviews/histogram.js b/test/acceptance/dataviews/histogram.js index abbaef61..c75e9baf 100644 --- a/test/acceptance/dataviews/histogram.js +++ b/test/acceptance/dataviews/histogram.js @@ -3,6 +3,15 @@ require('../../support/test_helper'); var assert = require('../../support/assert'); var TestClient = require('../../support/test-client'); +function createMapConfig(layers, dataviews, analysis) { + return { + version: '1.5.0', + layers: layers, + dataviews: dataviews || {}, + analyses: analysis || [] + }; +} + describe('histogram-dataview', function() { afterEach(function(done) { @@ -13,15 +22,6 @@ describe('histogram-dataview', function() { } }); - function createMapConfig(layers, dataviews, analysis) { - return { - version: '1.5.0', - layers: layers, - dataviews: dataviews || {}, - analyses: analysis || [] - }; - } - var mapConfig = createMapConfig( [ { @@ -89,7 +89,6 @@ describe('histogram-dataview', function() { this.testClient = new TestClient(mapConfig, 1234); this.testClient.getDataview('pop_max_histogram', params, function(err, res) { assert.ok(!err, err); - assert.ok(res.errors); assert.equal(res.errors.length, 1); assert.ok(res.errors[0].match(/Invalid number format for parameter 'bins'/)); @@ -98,3 +97,70 @@ describe('histogram-dataview', function() { }); }); }); + + +describe('histogram-dataview: special float valuer', function() { + + afterEach(function(done) { + if (this.testClient) { + this.testClient.drain(done); + } else { + done(); + } + }); + + var mapConfig = createMapConfig( + [ + { + "type": "cartodb", + "options": { + "source": { + "id": "a0" + }, + "cartocss": "#points { marker-width: 10; marker-fill: red; }", + "cartocss_version": "2.3.0" + } + } + ], + { + val_histogram: { + source: { + id: 'a0' + }, + type: 'histogram', + options: { + column: 'val' + } + } + }, + [ + { + "id": "a0", + "type": "source", + "params": { + "query": [ + 'SELECT', + ' null::geometry the_geom_webmercator,', + ' CASE', + ' WHEN x % 4 = 0 THEN \'infinity\'::float', + ' WHEN x % 4 = 1 THEN \'-infinity\'::float', + ' WHEN x % 4 = 2 THEN \'NaN\'::float', + ' ELSE x', + ' END AS val', + 'FROM generate_series(1, 1000) x' + ].join('\n') + } + } + ] + ); + + it('should filter infinities out and count them in the summary', function(done) { + this.testClient = new TestClient(mapConfig, 1234); + this.testClient.getDataview('val_histogram', {}, function(err, dataview) { + assert.ok(!err, err); + assert.ok(dataview.infinities === (250 + 250)); + assert.ok(dataview.nans === 250); + done(); + }); + }); +}); From cd53eda0a5062551fb7f64efaa11c82acd70bec3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 8 Jun 2017 16:01:41 +0200 Subject: [PATCH 02/34] Handle Infinity and NaN values for histograms --- lib/cartodb/models/dataview/histogram.js | 75 +++++++++++++++++++++--- 1 file changed, 66 insertions(+), 9 deletions(-) diff --git a/lib/cartodb/models/dataview/histogram.js b/lib/cartodb/models/dataview/histogram.js index 5d102bf5..f6b2635d 100644 --- a/lib/cartodb/models/dataview/histogram.js +++ b/lib/cartodb/models/dataview/histogram.js @@ -13,12 +13,27 @@ var columnCastTpl = dot.template("date_part('epoch', {{=it.column}})"); var BIN_MIN_NUMBER = 6; var BIN_MAX_NUMBER = 48; +var filteredQueryTpl = dot.template([ + 'filtered_source AS (', + ' SELECT *', + ' FROM ({{=it._query}}) _cdb_filtered_source', + ' WHERE', + ' {{=it._column}} IS NOT NULL', + ' AND', + ' {{=it._column}} != \'infinity\'::float', + ' AND', + ' {{=it._column}} != \'-infinity\'::float', + ' AND', + ' {{=it._column}} != \'NaN\'::float', + ')' +].join(' \n')); + var basicsQueryTpl = dot.template([ 'basics AS (', ' SELECT', ' max({{=it._column}}) AS max_val, min({{=it._column}}) AS min_val,', ' avg({{=it._column}}) AS avg_val, count(1) AS total_rows', - ' FROM ({{=it._query}}) _cdb_basics', + ' FROM filtered_source', ')' ].join(' \n')); @@ -27,7 +42,7 @@ var overrideBasicsQueryTpl = dot.template([ ' SELECT', ' max({{=it._end}}) AS max_val, min({{=it._start}}) AS min_val,', ' avg({{=it._column}}) AS avg_val, count(1) AS total_rows', - ' FROM ({{=it._query}}) _cdb_basics', + ' FROM filtered_source', ')' ].join('\n')); @@ -38,7 +53,7 @@ var iqrQueryTpl = dot.template([ ' SELECT quartile, max(_cdb_iqr_column) AS quartile_max from (', ' SELECT {{=it._column}} AS _cdb_iqr_column, ntile(4) over (order by {{=it._column}}', ' ) AS quartile', - ' FROM ({{=it._query}}) _cdb_rank) _cdb_quartiles', + ' FROM filtered_source) _cdb_quartiles', ' WHERE quartile = 1 or quartile = 3', ' GROUP BY quartile', ' ) _cdb_iqr', @@ -57,7 +72,7 @@ var binsQueryTpl = dot.template([ ' )', ' )', ' END AS bins_number', - ' FROM basics, iqrange, ({{=it._query}}) _cdb_bins', + ' FROM basics, iqrange, filtered_source', ' LIMIT 1', ')' ].join('\n')); @@ -77,11 +92,34 @@ var nullsQueryTpl = dot.template([ ')' ].join('\n')); +var infinitiesQueryTpl = dot.template([ + 'infinities AS (', + ' SELECT', + ' count(*) AS infinities_count', + ' FROM ({{=it._query}}) _cdb_histogram_infinities', + ' WHERE', + ' {{=it._column}} = \'infinity\'::float', + ' OR', + ' {{=it._column}} = \'-infinity\'::float', + ')' +].join('\n')); + +var nansQueryTpl = dot.template([ + 'nans AS (', + ' SELECT', + ' count(*) AS nans_count', + ' FROM ({{=it._query}}) _cdb_histogram_infinities', + ' WHERE {{=it._column}} = \'NaN\'::float', + ')' +].join('\n')); + var histogramQueryTpl = dot.template([ 'SELECT', ' (max_val - min_val) / cast(bins_number as float) AS bin_width,', ' bins_number,', ' nulls_count,', + ' infinities_count,', + ' nans_count,', ' avg_val,', ' CASE WHEN min_val = max_val', ' THEN 0', @@ -91,9 +129,8 @@ var histogramQueryTpl = dot.template([ ' max({{=it._column}})::numeric AS max,', ' avg({{=it._column}})::numeric AS avg,', ' count(*) AS freq', - 'FROM ({{=it._query}}) _cdb_histogram, basics, nulls, bins', - 'WHERE {{=it._column}} IS NOT NULL', - 'GROUP BY bin, bins_number, bin_width, nulls_count, avg_val', + 'FROM filtered_source, basics, nulls, infinities, nans, bins', + 'GROUP BY bin, bins_number, bin_width, nulls_count, infinities_count, nans_count, avg_val', 'ORDER BY bin' ].join('\n')); @@ -168,7 +205,12 @@ Histogram.prototype.sql = function(psql, override, callback) { var _query = this.query; - var basicsQuery, binsQuery; + var filteredQuery, basicsQuery, binsQuery; + + filteredQuery = filteredQueryTpl({ + _query: _query, + _column: _column + }); if (override && _.has(override, 'start') && _.has(override, 'end') && _.has(override, 'bins')) { debug('overriding with %j', override); @@ -215,11 +257,20 @@ Histogram.prototype.sql = function(psql, override, callback) { var histogramSql = [ "WITH", [ + filteredQuery, basicsQuery, binsQuery, nullsQueryTpl({ _query: _query, _column: _column + }), + infinitiesQueryTpl({ + _query: _query, + _column: _column + }), + nansQueryTpl({ + _query: _query, + _column: _column }) ].join(',\n'), histogramQueryTpl({ @@ -241,6 +292,8 @@ Histogram.prototype.format = function(result, override) { var width = getWidth(override); var binsStart = getBinStart(override); var nulls = 0; + var infinities = 0; + var nans = 0; var avg; if (result.rows.length) { @@ -249,10 +302,12 @@ Histogram.prototype.format = function(result, override) { width = firstRow.bin_width || width; avg = firstRow.avg_val; nulls = firstRow.nulls_count; + infinities = firstRow.infinities_count; + nans = firstRow.nans_count; binsStart = override.hasOwnProperty('start') ? getBinStart(override) : firstRow.min; buckets = result.rows.map(function(row) { - return _.omit(row, 'bins_number', 'bin_width', 'nulls_count', 'avg_val'); + return _.omit(row, 'bins_number', 'bin_width', 'nulls_count', 'infinities_count', 'nans_count', 'avg_val'); }); } @@ -261,6 +316,8 @@ Histogram.prototype.format = function(result, override) { bins_count: binsCount, bins_start: binsStart, nulls: nulls, + infinities: infinities, + nans: nans, avg: avg, bins: buckets }; From 7a3498e8ec2553629f89b0d2298386905e4b2bad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 9 Jun 2017 12:17:16 +0200 Subject: [PATCH 03/34] Going red: formula does not work with Infinity or NaN values --- test/acceptance/dataviews/formula.js | 80 ++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 test/acceptance/dataviews/formula.js diff --git a/test/acceptance/dataviews/formula.js b/test/acceptance/dataviews/formula.js new file mode 100644 index 00000000..93db6b38 --- /dev/null +++ b/test/acceptance/dataviews/formula.js @@ -0,0 +1,80 @@ +require('../../support/test_helper'); +var assert = require('../../support/assert'); +var TestClient = require('../../support/test-client'); + +function createMapConfig(layers, dataviews, analysis) { + return { + version: '1.5.0', + layers: layers, + dataviews: dataviews || {}, + analyses: analysis || [] + }; +} + +describe('formula-dataview: special float valuer', function() { + + afterEach(function(done) { + if (this.testClient) { + this.testClient.drain(done); + } else { + done(); + } + }); + + var mapConfig = createMapConfig( + [ + { + "type": "cartodb", + "options": { + "source": { + "id": "a0" + }, + "cartocss": "#points { marker-width: 10; marker-fill: red; }", + "cartocss_version": "2.3.0" + } + } + ], + { + val_formula: { + source: { + id: 'a0' + }, + type: 'formula', + options: { + column: 'val', + operation: 'avg' + } + } + }, + [ + { + "id": "a0", + "type": "source", + "params": { + "query": [ + 'SELECT', + ' null::geometry the_geom_webmercator,', + ' CASE', + ' WHEN x % 4 = 0 THEN \'infinity\'::float', + ' WHEN x % 4 = 1 THEN \'-infinity\'::float', + ' WHEN x % 4 = 2 THEN \'NaN\'::float', + ' ELSE x', + ' END AS val', + 'FROM generate_series(1, 1000) x' + ].join('\n') + } + } + ] + ); + + it('should filter infinities out and count them in the summary', function(done) { + this.testClient = new TestClient(mapConfig, 1234); + this.testClient.getDataview('val_formula', {}, function(err, dataview) { + assert.ok(!err, err); + assert.equal(dataview.result, 501) + assert.ok(dataview.infinities === (250 + 250)); + assert.ok(dataview.nans === 250); + done(); + }); + }); +}); From 9ba65bd5a482e08a9141148f270ef7636cd8580d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 9 Jun 2017 15:18:37 +0200 Subject: [PATCH 04/34] Going green: Fix test for formula overviews flavour --- lib/cartodb/models/dataview/formula.js | 24 ++++++++++++++--- test/acceptance/dataviews/overviews.js | 36 +++++++++++++++++++++++--- 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index 156985c5..1421032d 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -7,9 +7,21 @@ dot.templateSettings.strip = false; var formulaQueryTpl = dot.template([ 'SELECT', - '{{=it._operation}}({{=it._column}}) AS result,', - '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', - 'FROM ({{=it._query}}) _cdb_formula' + ' {{=it._operation}}({{=it._column}}) AS result,', + ' (SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', + ' {{?it._operation!==\'count\'}}', + ' ,(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count', + ' ,(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} = \'NaN\'::float) AS nans_count', + ' {{?}}', + 'FROM ({{=it._query}}) _cdb_formula', + '{{?it._operation!==\'count\'}}', + 'WHERE', + ' {{=it._column}} != \'infinity\'::float', + 'AND', + ' {{=it._column}} != \'-infinity\'::float', + 'AND', + ' {{=it._column}} != \'NaN\'::float', + '{{?}}' ].join('\n')); var VALID_OPERATIONS = { @@ -78,13 +90,17 @@ Formula.prototype.format = function(result) { var formattedResult = { operation: this.operation, result: 0, - nulls: 0 + nulls: 0, + nans: 0, + infinities: 0 }; if (result.rows.length) { formattedResult.operation = this.operation; formattedResult.result = result.rows[0].result; formattedResult.nulls = result.rows[0].nulls_count; + formattedResult.nans = result.rows[0].nans_count; + formattedResult.infinities = result.rows[0].infinities_count; } return formattedResult; diff --git a/test/acceptance/dataviews/overviews.js b/test/acceptance/dataviews/overviews.js index d23519ae..18fc1240 100644 --- a/test/acceptance/dataviews/overviews.js +++ b/test/acceptance/dataviews/overviews.js @@ -248,7 +248,14 @@ describe('dataviews using tables with overviews', function() { if (err) { return done(err); } - assert.deepEqual(formula_result, {"operation":"max","result":5,"nulls":0,"type":"formula"}); + assert.deepEqual(formula_result, { + "operation": "max", + "result": 5, + "nulls": 0, + "infinities": 0, + "nans": 0, + "type": "formula" + }); testClient.drain(done); }); @@ -260,7 +267,14 @@ describe('dataviews using tables with overviews', function() { if (err) { return done(err); } - assert.deepEqual(formula_result, {"operation":"min","result":1,"nulls":0,"type":"formula"}); + assert.deepEqual(formula_result, { + "operation": "min", + "result": 1, + "nulls": 0, + "infinities": 0, + "nans": 0, + "type": "formula" + }); testClient.drain(done); }); @@ -407,7 +421,14 @@ describe('dataviews using tables with overviews', function() { if (err) { return done(err); } - assert.deepEqual(formula_result, {"operation":"max","result":1,"nulls":0,"type":"formula"}); + assert.deepEqual(formula_result, { + "operation": "max", + "result": 1, + "nulls": 0, + "infinities": 0, + "nans": 0, + "type": "formula" + }); testClient.drain(done); }); @@ -419,7 +440,14 @@ describe('dataviews using tables with overviews', function() { if (err) { return done(err); } - assert.deepEqual(formula_result, {"operation":"min","result":1,"nulls":0,"type":"formula"}); + assert.deepEqual(formula_result, { + "operation": "min", + "result": 1, + "nulls": 0, + "infinities": 0, + "nans": 0, + "type": "formula" + }); testClient.drain(done); }); From 0aae29fb4b64e8dddb7baa3c136fda8db9ea830a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 9 Jun 2017 15:28:18 +0200 Subject: [PATCH 05/34] Fix jshint typo --- lib/cartodb/models/dataview/formula.js | 6 ++++-- test/acceptance/dataviews/formula.js | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index 1421032d..04b89835 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -10,8 +10,10 @@ var formulaQueryTpl = dot.template([ ' {{=it._operation}}({{=it._column}}) AS result,', ' (SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', ' {{?it._operation!==\'count\'}}', - ' ,(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count', - ' ,(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} = \'NaN\'::float) AS nans_count', + ' ,(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls', + ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count', + ' ,(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls', + ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count', ' {{?}}', 'FROM ({{=it._query}}) _cdb_formula', '{{?it._operation!==\'count\'}}', diff --git a/test/acceptance/dataviews/formula.js b/test/acceptance/dataviews/formula.js index 93db6b38..585a3b82 100644 --- a/test/acceptance/dataviews/formula.js +++ b/test/acceptance/dataviews/formula.js @@ -71,7 +71,7 @@ describe('formula-dataview: special float valuer', function() { this.testClient = new TestClient(mapConfig, 1234); this.testClient.getDataview('val_formula', {}, function(err, dataview) { assert.ok(!err, err); - assert.equal(dataview.result, 501) + assert.equal(dataview.result, 501); assert.ok(dataview.infinities === (250 + 250)); assert.ok(dataview.nans === 250); done(); From e6aededf089fff0bb326ad291453d4585f400163 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 12 Jun 2017 17:19:05 +0200 Subject: [PATCH 06/34] Fix typo --- test/acceptance/dataviews/formula.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/dataviews/formula.js b/test/acceptance/dataviews/formula.js index 585a3b82..941dc07e 100644 --- a/test/acceptance/dataviews/formula.js +++ b/test/acceptance/dataviews/formula.js @@ -11,7 +11,7 @@ function createMapConfig(layers, dataviews, analysis) { }; } -describe('formula-dataview: special float valuer', function() { +describe('formula-dataview: special float values', function() { afterEach(function(done) { if (this.testClient) { From ba6dc62a38245f1e18c47600ba165fb312d55e98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 12 Jun 2017 18:15:39 +0200 Subject: [PATCH 07/34] Going red: implementet test to check special float values support --- test/acceptance/dataviews/aggregation.js | 82 ++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/test/acceptance/dataviews/aggregation.js b/test/acceptance/dataviews/aggregation.js index 259cd2af..988f71ff 100644 --- a/test/acceptance/dataviews/aggregation.js +++ b/test/acceptance/dataviews/aggregation.js @@ -146,3 +146,85 @@ describe('aggregations happy cases', function() { }); }); }); + +describe.only('aggregation-dataview: special float values', function() { + + afterEach(function(done) { + if (this.testClient) { + this.testClient.drain(done); + } else { + done(); + } + }); + + function createMapConfig(layers, dataviews, analysis) { + return { + version: '1.5.0', + layers: layers, + dataviews: dataviews || {}, + analyses: analysis || [] + }; + } + + var mapConfig = createMapConfig( + [ + { + "type": "cartodb", + "options": { + "source": { + "id": "a0" + }, + "cartocss": "#points { marker-width: 10; marker-fill: red; }", + "cartocss_version": "2.3.0" + } + } + ], + { + val_aggregation: { + source: { + id: 'a0' + }, + type: 'aggregation', + options: { + column: 'cat', + aggregation: 'sum', + aggregationColumn: 'val' + } + } + }, + [ + { + "id": "a0", + "type": "source", + "params": { + "query": [ + 'SELECT', + ' null::geometry the_geom_webmercator,', + ' CASE', + ' WHEN x % 4 = 0 THEN \'infinity\'::float', + ' WHEN x % 4 = 1 THEN \'-infinity\'::float', + ' WHEN x % 4 = 2 THEN \'NaN\'::float', + ' ELSE x', + ' END AS val,', + ' CASE', + ' WHEN x % 2 = 0 THEN \'category_1\'', + ' ELSE \'category_2\'', + ' END AS cat', + 'FROM generate_series(1, 1000) x' + ].join('\n') + } + } + ] + ); + + it('should filter infinities out and count them in the summary', function(done) { + this.testClient = new TestClient(mapConfig, 1234); + this.testClient.getDataview('val_aggregation', {}, function(err, dataview) { + assert.ifError(err); + assert.equal(dataview.result, 501); + assert.ok(dataview.infinities === (250 + 250)); + assert.ok(dataview.nans === 250); + done(); + }); + }); +}); From e60bb770db7d0e6fe836a927d16db23df58a04d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 12 Jun 2017 18:55:33 +0200 Subject: [PATCH 08/34] Summarizes infinity and NaN values --- lib/cartodb/models/dataview/aggregation.js | 83 +++++++++++++++------- 1 file changed, 57 insertions(+), 26 deletions(-) diff --git a/lib/cartodb/models/dataview/aggregation.js b/lib/cartodb/models/dataview/aggregation.js index c15f0506..8c6fff54 100644 --- a/lib/cartodb/models/dataview/aggregation.js +++ b/lib/cartodb/models/dataview/aggregation.js @@ -5,11 +5,28 @@ var debug = require('debug')('windshaft:widget:aggregation'); var dot = require('dot'); dot.templateSettings.strip = false; +var filteredQueryTpl = dot.template([ + 'filtered_source AS (', + ' SELECT *', + ' FROM ({{=it._query}}) _cdb_filtered_source', + ' WHERE', + ' {{=it._column}} IS NOT NULL', + ' AND', + ' {{=it._aggregationColumn}} != \'infinity\'::float', + ' AND', + ' {{=it._aggregationColumn}} != \'-infinity\'::float', + ' AND', + ' {{=it._aggregationColumn}} != \'NaN\'::float', + ')' +].join(' \n')); + var summaryQueryTpl = dot.template([ 'summary AS (', ' SELECT', ' count(1) AS count,', - ' sum(CASE WHEN {{=it._column}} IS NULL THEN 1 ELSE 0 END) AS nulls_count', + ' sum(CASE WHEN {{=it._column}} IS NULL THEN 1 ELSE 0 END) AS nulls_count,', + ' sum(CASE WHEN {{=it._aggregationColumn}} = \'infinity\'::float OR {{=it._aggregationColumn}} = \'-infinity\'::float THEN 1 ELSE 0 END) AS infinities_count,', + ' sum(CASE WHEN {{=it._aggregationColumn}} = \'NaN\'::float THEN 1 ELSE 0 END) AS nans_count', ' FROM ({{=it._query}}) _cdb_aggregation_nulls', ')' ].join('\n')); @@ -57,9 +74,9 @@ 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', + ' nulls_count, min_val, max_val, count, categories_count, nans_count, infinities_count', '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', + 'GROUP BY category, nulls_count, min_val, max_val, count, categories_count, nans_count, infinities_count', 'ORDER BY value DESC' ].join('\n')); @@ -128,6 +145,8 @@ Aggregation.prototype.sql = function(psql, override, callback) { var aggregationSql; + + if (!!override.ownFilter) { aggregationSql = [ this.getCategoriesCTESql(_query, this.column, this.aggregation, this.aggregationColumn), @@ -157,28 +176,34 @@ Aggregation.prototype.sql = function(psql, override, callback) { 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'); + "WITH", + [ + filteredQueryTpl({ + _query: this.query, + _column: this.column, + _aggregationColumn: aggregation !== 'count' ? aggregationColumn : null + }), + summaryQueryTpl({ + _query: query, + _column: column, + _aggregationColumn: aggregation !== 'count' ? aggregationColumn : null + }), + 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}})'); @@ -193,6 +218,8 @@ Aggregation.prototype.format = function(result) { var categories = []; var count = 0; var nulls = 0; + var nans = 0; + var infinities = 0; var minValue = 0; var maxValue = 0; var categoriesCount = 0; @@ -202,12 +229,14 @@ Aggregation.prototype.format = function(result) { var firstRow = result.rows[0]; count = firstRow.count; nulls = firstRow.nulls_count; + nans = firstRow.nans_count; + infinities = firstRow.infinities_count; minValue = firstRow.min_val; maxValue = firstRow.max_val; categoriesCount = firstRow.categories_count; result.rows.forEach(function(row) { - categories.push(_.omit(row, 'count', 'nulls_count', 'min_val', 'max_val', 'categories_count')); + categories.push(_.omit(row, 'count', 'nulls_count', 'min_val', 'max_val', 'categories_count', 'nans_count', 'infinities_count')); }); } @@ -215,6 +244,8 @@ Aggregation.prototype.format = function(result) { aggregation: this.aggregation, count: count, nulls: nulls, + nans: nans, + infinities: infinities, min: minValue, max: maxValue, categoriesCount: categoriesCount, From 7b5111614c89e1b61f8dbb14cd701edb9ec98c6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 12 Jun 2017 19:21:41 +0200 Subject: [PATCH 09/34] Summarize special float values for ranked aggregation --- lib/cartodb/models/dataview/aggregation.js | 8 ++++---- test/acceptance/dataviews/aggregation.js | 20 +++++++++++--------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/lib/cartodb/models/dataview/aggregation.js b/lib/cartodb/models/dataview/aggregation.js index 8c6fff54..145b6e9d 100644 --- a/lib/cartodb/models/dataview/aggregation.js +++ b/lib/cartodb/models/dataview/aggregation.js @@ -61,15 +61,15 @@ var categoriesSummaryCountQueryTpl = dot.template([ ].join('\n')); var rankedAggregationQueryTpl = dot.template([ - 'SELECT CAST(category AS text), value, false as agg, nulls_count, min_val, max_val, count, categories_count', + 'SELECT CAST(category AS text), value, false as agg, nulls_count, min_val, max_val, count, categories_count, nans_count, infinities_count', ' FROM categories, summary, categories_summary_min_max, categories_summary_count', ' WHERE rank < {{=it._limit}}', 'UNION ALL', 'SELECT \'Other\' category, {{=it._aggregationFn}}(value) as value, true as agg, nulls_count, min_val, max_val,', - ' count, categories_count', + ' count, categories_count, nans_count, infinities_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' + 'GROUP BY nulls_count, min_val, max_val, count, categories_count, nans_count, infinities_count' ].join('\n')); var aggregationQueryTpl = dot.template([ @@ -145,7 +145,7 @@ Aggregation.prototype.sql = function(psql, override, callback) { var aggregationSql; - + console.log('override.ownFilter', override.ownFilter); if (!!override.ownFilter) { aggregationSql = [ diff --git a/test/acceptance/dataviews/aggregation.js b/test/acceptance/dataviews/aggregation.js index 988f71ff..d4f3d65c 100644 --- a/test/acceptance/dataviews/aggregation.js +++ b/test/acceptance/dataviews/aggregation.js @@ -217,14 +217,16 @@ describe.only('aggregation-dataview: special float values', function() { ] ); - it('should filter infinities out and count them in the summary', function(done) { - this.testClient = new TestClient(mapConfig, 1234); - this.testClient.getDataview('val_aggregation', {}, function(err, dataview) { - assert.ifError(err); - assert.equal(dataview.result, 501); - assert.ok(dataview.infinities === (250 + 250)); - assert.ok(dataview.nans === 250); - done(); + [{ own_filter: 0 }, {}].forEach(function (filter) { + it('should handle special float values using filter: ' + JSON.stringify(filter), function(done) { + this.testClient = new TestClient(mapConfig, 1234); + this.testClient.getDataview('val_aggregation', { own_filter: 0 }, function(err, dataview) { + assert.ifError(err); + assert.equal(dataview.result, 501); + assert.ok(dataview.infinities === (250 + 250)); + assert.ok(dataview.nans === 250); + done(); + }); }); - }); + }) }); From 75d07745e6496d4bef923381196f22c0d1758572 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 12 Jun 2017 19:22:37 +0200 Subject: [PATCH 10/34] Improve readability --- test/acceptance/dataviews/aggregation.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/acceptance/dataviews/aggregation.js b/test/acceptance/dataviews/aggregation.js index d4f3d65c..02b19782 100644 --- a/test/acceptance/dataviews/aggregation.js +++ b/test/acceptance/dataviews/aggregation.js @@ -217,7 +217,8 @@ describe.only('aggregation-dataview: special float values', function() { ] ); - [{ own_filter: 0 }, {}].forEach(function (filter) { + var filters = [{ own_filter: 0 }, {}]; + filters.forEach(function (filter) { it('should handle special float values using filter: ' + JSON.stringify(filter), function(done) { this.testClient = new TestClient(mapConfig, 1234); this.testClient.getDataview('val_aggregation', { own_filter: 0 }, function(err, dataview) { From 962fa055747f25548b23479b4c746cc2c25ae63b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 12 Jun 2017 19:28:49 +0200 Subject: [PATCH 11/34] Remove console --- lib/cartodb/models/dataview/aggregation.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/cartodb/models/dataview/aggregation.js b/lib/cartodb/models/dataview/aggregation.js index 145b6e9d..f06f6091 100644 --- a/lib/cartodb/models/dataview/aggregation.js +++ b/lib/cartodb/models/dataview/aggregation.js @@ -145,8 +145,6 @@ Aggregation.prototype.sql = function(psql, override, callback) { var aggregationSql; - console.log('override.ownFilter', override.ownFilter); - if (!!override.ownFilter) { aggregationSql = [ this.getCategoriesCTESql(_query, this.column, this.aggregation, this.aggregationColumn), From 8b2fa27ba75f5f488cc7f6ffc7501f66a747d761 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 12 Jun 2017 19:45:06 +0200 Subject: [PATCH 12/34] Calculate aggregation filtering out special float values --- lib/cartodb/models/dataview/aggregation.js | 2 +- test/acceptance/dataviews/aggregation.js | 23 ++++++++++++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/models/dataview/aggregation.js b/lib/cartodb/models/dataview/aggregation.js index f06f6091..9ce889da 100644 --- a/lib/cartodb/models/dataview/aggregation.js +++ b/lib/cartodb/models/dataview/aggregation.js @@ -35,7 +35,7 @@ var rankedCategoriesQueryTpl = dot.template([ 'categories AS(', ' 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', + ' FROM filtered_source', ' {{?it._aggregationColumn!==null}}WHERE {{=it._aggregationColumn}} IS NOT NULL{{?}}', ' GROUP BY {{=it._column}}', ' ORDER BY 2 DESC', diff --git a/test/acceptance/dataviews/aggregation.js b/test/acceptance/dataviews/aggregation.js index 02b19782..71aec370 100644 --- a/test/acceptance/dataviews/aggregation.js +++ b/test/acceptance/dataviews/aggregation.js @@ -187,7 +187,7 @@ describe.only('aggregation-dataview: special float values', function() { type: 'aggregation', options: { column: 'cat', - aggregation: 'sum', + aggregation: 'avg', aggregationColumn: 'val' } } @@ -217,15 +217,34 @@ describe.only('aggregation-dataview: special float values', function() { ] ); + // the_geom_webmercator | val | cat + // ----------------------+-----------+------------ + // | -Infinity | category_2 + // | NaN | category_1 + // | 3 | category_2 + // | Infinity | category_1 + // | -Infinity | category_2 + // | NaN | category_1 + // | 7 | category_2 + // | Infinity | category_1 + // | -Infinity | category_2 + // | NaN | category_1 + // | 11 | category_2 + // | " | " + var filters = [{ own_filter: 0 }, {}]; filters.forEach(function (filter) { it('should handle special float values using filter: ' + JSON.stringify(filter), function(done) { this.testClient = new TestClient(mapConfig, 1234); this.testClient.getDataview('val_aggregation', { own_filter: 0 }, function(err, dataview) { assert.ifError(err); - assert.equal(dataview.result, 501); assert.ok(dataview.infinities === (250 + 250)); assert.ok(dataview.nans === 250); + assert.ok(dataview.categories.length === 1); + dataview.categories.forEach(function (category) { + assert.ok(category.category === 'category_2'); + assert.ok(category.value === 501); + }) done(); }); }); From cb7ec5d556e4020b697fd82b71e270cc095a8988 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 12 Jun 2017 19:49:58 +0200 Subject: [PATCH 13/34] Fix jshint typos --- lib/cartodb/models/dataview/aggregation.js | 18 +++++++++++++----- test/acceptance/dataviews/aggregation.js | 4 ++-- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/cartodb/models/dataview/aggregation.js b/lib/cartodb/models/dataview/aggregation.js index 9ce889da..6e46ea31 100644 --- a/lib/cartodb/models/dataview/aggregation.js +++ b/lib/cartodb/models/dataview/aggregation.js @@ -25,7 +25,13 @@ var summaryQueryTpl = dot.template([ ' SELECT', ' count(1) AS count,', ' sum(CASE WHEN {{=it._column}} IS NULL THEN 1 ELSE 0 END) AS nulls_count,', - ' sum(CASE WHEN {{=it._aggregationColumn}} = \'infinity\'::float OR {{=it._aggregationColumn}} = \'-infinity\'::float THEN 1 ELSE 0 END) AS infinities_count,', + ' sum(', + ' CASE', + ' WHEN {{=it._aggregationColumn}} = \'infinity\'::float OR {{=it._aggregationColumn}} = \'-infinity\'::float', + ' THEN 1', + ' ELSE 0', + ' END', + ' ) AS infinities_count,', ' sum(CASE WHEN {{=it._aggregationColumn}} = \'NaN\'::float THEN 1 ELSE 0 END) AS nans_count', ' FROM ({{=it._query}}) _cdb_aggregation_nulls', ')' @@ -61,12 +67,13 @@ var categoriesSummaryCountQueryTpl = dot.template([ ].join('\n')); var rankedAggregationQueryTpl = dot.template([ - 'SELECT CAST(category AS text), value, false as agg, nulls_count, min_val, max_val, count, categories_count, nans_count, infinities_count', + 'SELECT CAST(category AS text), value, false as agg, nulls_count, min_val, max_val,', + ' count, categories_count, nans_count, infinities_count', ' FROM categories, summary, categories_summary_min_max, categories_summary_count', ' WHERE rank < {{=it._limit}}', 'UNION ALL', - 'SELECT \'Other\' category, {{=it._aggregationFn}}(value) as value, true as agg, nulls_count, min_val, max_val,', - ' count, categories_count, nans_count, infinities_count', + 'SELECT \'Other\' category, {{=it._aggregationFn}}(value) as value, true as agg, nulls_count,', + ' min_val, max_val, count, categories_count, nans_count, infinities_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, nans_count, infinities_count' @@ -234,7 +241,8 @@ Aggregation.prototype.format = function(result) { categoriesCount = firstRow.categories_count; result.rows.forEach(function(row) { - categories.push(_.omit(row, 'count', 'nulls_count', 'min_val', 'max_val', 'categories_count', 'nans_count', 'infinities_count')); + categories.push(_.omit(row, 'count', 'nulls_count', 'min_val', + 'max_val', 'categories_count', 'nans_count', 'infinities_count')); }); } diff --git a/test/acceptance/dataviews/aggregation.js b/test/acceptance/dataviews/aggregation.js index 71aec370..4f7c0062 100644 --- a/test/acceptance/dataviews/aggregation.js +++ b/test/acceptance/dataviews/aggregation.js @@ -244,9 +244,9 @@ describe.only('aggregation-dataview: special float values', function() { dataview.categories.forEach(function (category) { assert.ok(category.category === 'category_2'); assert.ok(category.value === 501); - }) + }); done(); }); }); - }) + }); }); From 227937bf4c933488728587ad8e034f4ce248a20c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 12 Jun 2017 19:50:24 +0200 Subject: [PATCH 14/34] Remove test filter --- test/acceptance/dataviews/aggregation.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/dataviews/aggregation.js b/test/acceptance/dataviews/aggregation.js index 4f7c0062..5d9dc499 100644 --- a/test/acceptance/dataviews/aggregation.js +++ b/test/acceptance/dataviews/aggregation.js @@ -147,7 +147,7 @@ describe('aggregations happy cases', function() { }); }); -describe.only('aggregation-dataview: special float values', function() { +describe('aggregation-dataview: special float values', function() { afterEach(function(done) { if (this.testClient) { From 3ae66e41437160dfd3cf9bb2cac52cb0df195d23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 13 Jun 2017 09:30:43 +0200 Subject: [PATCH 15/34] Do not filter special values out if aggregation column is not defined --- lib/cartodb/models/dataview/aggregation.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/cartodb/models/dataview/aggregation.js b/lib/cartodb/models/dataview/aggregation.js index 6e46ea31..71574ac5 100644 --- a/lib/cartodb/models/dataview/aggregation.js +++ b/lib/cartodb/models/dataview/aggregation.js @@ -11,12 +11,14 @@ var filteredQueryTpl = dot.template([ ' FROM ({{=it._query}}) _cdb_filtered_source', ' WHERE', ' {{=it._column}} IS NOT NULL', + ' {{?it._aggregationColumn}}', ' AND', ' {{=it._aggregationColumn}} != \'infinity\'::float', ' AND', ' {{=it._aggregationColumn}} != \'-infinity\'::float', ' AND', ' {{=it._aggregationColumn}} != \'NaN\'::float', + ' {{?}}', ')' ].join(' \n')); From 551b6d409a1820fd0a2acc534ff02c246c51e777 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 13 Jun 2017 09:44:53 +0200 Subject: [PATCH 16/34] Remove bad condition --- lib/cartodb/models/dataview/aggregation.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/cartodb/models/dataview/aggregation.js b/lib/cartodb/models/dataview/aggregation.js index 71574ac5..e61b1454 100644 --- a/lib/cartodb/models/dataview/aggregation.js +++ b/lib/cartodb/models/dataview/aggregation.js @@ -9,10 +9,8 @@ var filteredQueryTpl = dot.template([ 'filtered_source AS (', ' SELECT *', ' FROM ({{=it._query}}) _cdb_filtered_source', - ' WHERE', - ' {{=it._column}} IS NOT NULL', ' {{?it._aggregationColumn}}', - ' AND', + ' WHERE', ' {{=it._aggregationColumn}} != \'infinity\'::float', ' AND', ' {{=it._aggregationColumn}} != \'-infinity\'::float', From c08db78a0bc496ea290180eed210091af1f4eba7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 13 Jun 2017 19:01:28 +0200 Subject: [PATCH 17/34] Going red: implement test to check aggregation with overviews support special float values --- test/acceptance/dataviews/overviews.js | 58 +++++++++++++++++++++ test/support/sql/windshaft.test.sql | 72 ++++++++++++++++++++++++++ 2 files changed, 130 insertions(+) diff --git a/test/acceptance/dataviews/overviews.js b/test/acceptance/dataviews/overviews.js index 18fc1240..c822caee 100644 --- a/test/acceptance/dataviews/overviews.js +++ b/test/acceptance/dataviews/overviews.js @@ -124,6 +124,13 @@ describe('dataviews using tables with overviews', function() { params: { query: 'select * from test_table_overviews' } + }, + { + id: 'data-source-special-float-values', + type: 'source', + params: { + query: 'select * from test_special_float_values_table_overviews' + } } ], dataviews: { @@ -144,6 +151,17 @@ describe('dataviews using tables with overviews', function() { aggregationColumn: 'name', } }, + test_categories_special_values: { + type: 'aggregation', + source: { + id: 'data-source-special-float-values' + }, + options: { + column: 'name', + aggregation: 'sum', + aggregationColumn: 'value', + } + }, test_histogram: { type: 'histogram', source: {id: 'data-source'}, @@ -202,6 +220,17 @@ describe('dataviews using tables with overviews', function() { cartocss_version: '2.3.0', source: { id: 'data-source' } } + }, + { + type: 'mapnik', + options: { + sql: 'select * from test_special_float_values_table_overviews', + cartocss: '#layer { marker-fill: red; marker-width: 32; marker-allow-overlap: true; }', + cartocss_version: '2.3.0', + source: { + id: 'data-source-special-float-values' + } + } } ] }; @@ -473,5 +502,34 @@ describe('dataviews using tables with overviews', function() { }); + describe('aggregation special float values', function () { + var params = {}; + + it("should expose an aggregation dataview", function (done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_categories_special_values', params, function (err, dataview) { + if (err) { + return done(err); + } + assert.deepEqual(dataview, { + aggregation: 'sum', + count: 5, + nulls: 0, + nans: 1, + infinities: 1, + min: 6, + max: 6, + categoriesCount: 3, + categories:[ + { category: 'El Rey del Tallarín', value: null, agg: false }, + { category: 'El Lacón', value: null, agg: false }, + { category: 'Hawai', value: 6, agg: false } + ], + type: 'aggregation' + }); + testClient.drain(done); + }); + }); + }); }); }); diff --git a/test/support/sql/windshaft.test.sql b/test/support/sql/windshaft.test.sql index 91484950..996c6b3e 100644 --- a/test/support/sql/windshaft.test.sql +++ b/test/support/sql/windshaft.test.sql @@ -339,6 +339,78 @@ INSERT INTO _vovw_2_test_table_overviews VALUES INSERT INTO _vovw_1_test_table_overviews VALUES ('2011-09-21 14:02:21.358706', '2011-09-21 14:02:21.314252', 1, 'Hawai', 'Calle de Pérez Galdós 9, Madrid, Spain', 3.0, '0101000020E610000000000000000020C00000000000004440', '0101000020110F000076491621312319C122D4663F1DCC5241', 5); +-- table with overviews whit special float values + +CREATE TABLE test_special_float_values_table_overviews ( + cartodb_id integer NOT NULL, + name character varying, + address character varying, + value float8, + the_geom geometry, + the_geom_webmercator geometry, + _feature_count integer, + CONSTRAINT enforce_dims_the_geom CHECK ((st_ndims(the_geom) = 2)), + CONSTRAINT enforce_dims_the_geom_webmercator CHECK ((st_ndims(the_geom_webmercator) = 2)), + CONSTRAINT enforce_geotype_the_geom CHECK (((geometrytype(the_geom) = 'POINT'::text) OR (the_geom IS NULL))), + CONSTRAINT enforce_geotype_the_geom_webmercator CHECK (((geometrytype(the_geom_webmercator) = 'POINT'::text) OR (the_geom_webmercator IS NULL))), + CONSTRAINT enforce_srid_the_geom CHECK ((st_srid(the_geom) = 4326)), + CONSTRAINT enforce_srid_the_geom_webmercator CHECK ((st_srid(the_geom_webmercator) = 3857)) +); + +GRANT ALL ON TABLE test_special_float_values_table_overviews TO :TESTUSER; +GRANT SELECT ON TABLE test_special_float_values_table_overviews TO :PUBLICUSER; + +CREATE SEQUENCE test_special_float_values_table_overviews_cartodb_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE test_special_float_values_table_overviews_cartodb_id_seq OWNED BY test_special_float_values_table_overviews.cartodb_id; + +SELECT pg_catalog.setval('test_special_float_values_table_overviews_cartodb_id_seq', 60, true); + +ALTER TABLE test_special_float_values_table_overviews ALTER COLUMN cartodb_id SET DEFAULT nextval('test_special_float_values_table_overviews_cartodb_id_seq'::regclass); + +INSERT INTO test_special_float_values_table_overviews VALUES +(1, 'Hawai', 'Calle de Pérez Galdós 9, Madrid, Spain', 1.0, '0101000020E6100000A6B73F170D990DC064E8D84125364440', '0101000020110F000076491621312319C122D4663F1DCC5241', 1), +(2, 'El Estocolmo', 'Calle de la Palma 72, Madrid, Spain', 2.0, '0101000020E6100000C90567F0F7AB0DC0AB07CC43A6364440', '0101000020110F0000C4356B29423319C15DD1092DADCC5241', 1), +(3, 'El Rey del Tallarín', 'Plaza Conde de Toreno 2, Madrid, Spain', 'NaN'::float, '0101000020E610000021C8410933AD0DC0CB0EF10F5B364440', '0101000020110F000053E71AC64D3419C10F664E4659CC5241', 1), +(4, 'El Lacón', 'Manuel Fernández y González 8, Madrid, Spain', 4.0, '0101000020E6100000BC5983F755990DC07D923B6C22354440', '0101000020110F00005DACDB056F2319C1EC41A980FCCA5241', 1), +(5, 'El Pico', 'Calle Divino Pastor 12, Madrid, Spain', 'infinity'::float, '0101000020E61000003B6D8D08C6A10DC0371B2B31CF364440', '0101000020110F00005F716E91992A19C17DAAA4D6DACC5241', 1); + +ALTER TABLE ONLY test_special_float_values_table_overviews ADD CONSTRAINT test_special_float_values_table_overviews_pkey PRIMARY KEY (cartodb_id); + +CREATE INDEX test_special_float_values_table_overviews_the_geom_idx ON test_special_float_values_table_overviews USING gist (the_geom); +CREATE INDEX test_special_float_values_table_overviews_the_geom_webmercator_idx ON test_special_float_values_table_overviews USING gist (the_geom_webmercator); + +GRANT ALL ON TABLE test_special_float_values_table_overviews TO :TESTUSER; +GRANT SELECT ON TABLE test_special_float_values_table_overviews TO :PUBLICUSER; + +CREATE TABLE _vovw_1_test_special_float_values_table_overviews ( + cartodb_id integer NOT NULL, + name character varying, + address character varying, + value float8, + the_geom geometry, + the_geom_webmercator geometry, + _feature_count integer, + CONSTRAINT enforce_dims_the_geom CHECK ((st_ndims(the_geom) = 2)), + CONSTRAINT enforce_dims_the_geom_webmercator CHECK ((st_ndims(the_geom_webmercator) = 2)), + CONSTRAINT enforce_geotype_the_geom CHECK (((geometrytype(the_geom) = 'POINT'::text) OR (the_geom IS NULL))), + CONSTRAINT enforce_geotype_the_geom_webmercator CHECK (((geometrytype(the_geom_webmercator) = 'POINT'::text) OR (the_geom_webmercator IS NULL))), + CONSTRAINT enforce_srid_the_geom CHECK ((st_srid(the_geom) = 4326)), + CONSTRAINT enforce_srid_the_geom_webmercator CHECK ((st_srid(the_geom_webmercator) = 3857)) +); + +GRANT ALL ON TABLE _vovw_1_test_special_float_values_table_overviews TO :TESTUSER; +GRANT SELECT ON TABLE _vovw_1_test_special_float_values_table_overviews TO :PUBLICUSER; + +INSERT INTO _vovw_1_test_special_float_values_table_overviews VALUES +(1, 'Hawai', 'Calle de Pérez Galdós 9, Madrid, Spain', 3, '0101000020E610000000000000000020C00000000000004440', '0101000020110F000076491621312319C122D4663F1DCC5241', 2), +(3, 'El Rey del Tallarín', 'Plaza Conde de Toreno 2, Madrid, Spain', 'NaN'::float, '0101000020E610000021C8410933AD0DC0CB0EF10F5B364440', '0101000020110F000053E71AC64D3419C10F664E4659CC5241', 1), +(4, 'El Lacón', 'Manuel Fernández y González 8, Madrid, Spain', 'infinity'::float, '0101000020E6100000BC5983F755990DC07D923B6C22354440', '0101000020110F00005DACDB056F2319C1EC41A980FCCA5241', 2); -- analysis tables ----------------------------------------------- From 668b22628ce4ce5cfbf589ef50ab4868e549c11d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 13 Jun 2017 19:01:59 +0200 Subject: [PATCH 18/34] Going green: support special float values in aggergation dataview with overviews --- .../models/dataview/overviews/aggregation.js | 63 +++++++++++++++---- 1 file changed, 52 insertions(+), 11 deletions(-) diff --git a/lib/cartodb/models/dataview/overviews/aggregation.js b/lib/cartodb/models/dataview/overviews/aggregation.js index da63b27f..1b0e2ec6 100644 --- a/lib/cartodb/models/dataview/overviews/aggregation.js +++ b/lib/cartodb/models/dataview/overviews/aggregation.js @@ -1,14 +1,38 @@ var BaseOverviewsDataview = require('./base'); var BaseDataview = require('../aggregation'); +var debug = require('debug')('windshaft:widget:aggregation:overview'); var dot = require('dot'); dot.templateSettings.strip = false; +var filteredQueryTpl = dot.template([ + 'filtered_source AS (', + ' SELECT *', + ' FROM ({{=it._query}}) _cdb_filtered_source', + ' {{?it._aggregationColumn}}', + ' WHERE', + ' {{=it._aggregationColumn}} != \'infinity\'::float', + ' AND', + ' {{=it._aggregationColumn}} != \'-infinity\'::float', + ' AND', + ' {{=it._aggregationColumn}} != \'NaN\'::float', + ' {{?}}', + ')' +].join(' \n')); + var summaryQueryTpl = dot.template([ 'summary AS (', ' SELECT', ' sum(_feature_count) AS count,', - ' sum(CASE WHEN {{=it._column}} IS NULL THEN 1 ELSE 0 END) AS nulls_count', + ' sum(CASE WHEN {{=it._column}} IS NULL THEN 1 ELSE 0 END) AS nulls_count,', + ' sum(', + ' CASE', + ' WHEN {{=it._aggregationColumn}} = \'infinity\'::float OR {{=it._aggregationColumn}} = \'-infinity\'::float', + ' THEN 1', + ' ELSE 0', + ' END', + ' ) AS infinities_count,', + ' sum(CASE WHEN {{=it._aggregationColumn}} = \'NaN\'::float THEN 1 ELSE 0 END) AS nans_count', ' FROM ({{=it._query}}) _cdb_aggregation_nulls', ')' ].join('\n')); @@ -17,7 +41,7 @@ var rankedCategoriesQueryTpl = dot.template([ 'categories AS(', ' 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', + ' FROM filtered_source', ' {{?it._aggregationColumn!==null}}WHERE {{=it._aggregationColumn}} IS NOT NULL{{?}}', ' GROUP BY {{=it._column}}', ' ORDER BY 2 DESC', @@ -43,21 +67,23 @@ var categoriesSummaryCountQueryTpl = dot.template([ ].join('\n')); var rankedAggregationQueryTpl = dot.template([ - 'SELECT CAST(category AS text), value, false as agg, nulls_count, min_val, max_val, count, categories_count', + 'SELECT CAST(category AS text), value, false as agg, nulls_count, min_val, max_val,', + ' count, categories_count, nans_count, infinities_count', ' 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, sum(value), true as agg, nulls_count, min_val, max_val,', + ' count, categories_count, nans_count, infinities_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' + 'GROUP BY nulls_count, min_val, max_val, count, categories_count, nans_count, infinities_count' ].join('\n')); 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', + ' nulls_count, min_val, max_val, count, categories_count, nans_count, infinities_count', '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', + 'GROUP BY category, nulls_count, min_val, max_val, count, categories_count, nans_count, infinities_count', 'ORDER BY value DESC' ].join('\n')); @@ -84,21 +110,28 @@ Aggregation.prototype.sql = function(psql, override, callback) { } var _query = this.rewrittenQuery(this.query); + var _aggregationColumn = this.aggregation !== 'count' ? this.aggregationColumn : null; var aggregationSql; if (!!override.ownFilter) { aggregationSql = [ "WITH", [ + filteredQueryTpl({ + _query: _query, + _column: this.column, + _aggregationColumn: _aggregationColumn + }), summaryQueryTpl({ _query: _query, - _column: this.column + _column: this.column, + _aggregationColumn: _aggregationColumn }), rankedCategoriesQueryTpl({ _query: _query, _column: this.column, _aggregation: this.getAggregationSql(), - _aggregationColumn: this.aggregation !== 'count' ? this.aggregationColumn : null + _aggregationColumn: _aggregationColumn }), categoriesSummaryMinMaxQueryTpl({ _query: _query, @@ -120,15 +153,21 @@ Aggregation.prototype.sql = function(psql, override, callback) { aggregationSql = [ "WITH", [ + filteredQueryTpl({ + _query: _query, + _column: this.column, + _aggregationColumn: _aggregationColumn + }), summaryQueryTpl({ _query: _query, - _column: this.column + _column: this.column, + _aggregationColumn: _aggregationColumn }), rankedCategoriesQueryTpl({ _query: _query, _column: this.column, _aggregation: this.getAggregationSql(), - _aggregationColumn: this.aggregation !== 'count' ? this.aggregationColumn : null + _aggregationColumn: _aggregationColumn }), categoriesSummaryMinMaxQueryTpl({ _query: _query, @@ -147,6 +186,8 @@ Aggregation.prototype.sql = function(psql, override, callback) { ].join('\n'); } + debug(aggregationSql); + return callback(null, aggregationSql); }; From 3f17c8b15ab9074b1004d08aee23f59656fba42b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 14 Jun 2017 15:05:46 +0200 Subject: [PATCH 19/34] Filter out special float values before categorizing them --- lib/cartodb/models/dataview/overviews/aggregation.js | 4 ++-- test/acceptance/dataviews/overviews.js | 10 +++------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/lib/cartodb/models/dataview/overviews/aggregation.js b/lib/cartodb/models/dataview/overviews/aggregation.js index 1b0e2ec6..cc95e73c 100644 --- a/lib/cartodb/models/dataview/overviews/aggregation.js +++ b/lib/cartodb/models/dataview/overviews/aggregation.js @@ -60,7 +60,7 @@ var categoriesSummaryCountQueryTpl = dot.template([ ' SELECT count(1) AS categories_count', ' FROM (', ' SELECT {{=it._column}} AS category', - ' FROM ({{=it._query}}) _cdb_categories', + ' FROM filtered_source', ' GROUP BY {{=it._column}}', ' ) _cdb_categories_count', ')' @@ -82,7 +82,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, nans_count, infinities_count', - 'FROM ({{=it._query}}) _cdb_aggregation_all, summary, categories_summary_min_max, categories_summary_count', + 'FROM filtered_source, summary, categories_summary_min_max, categories_summary_count', 'GROUP BY category, nulls_count, min_val, max_val, count, categories_count, nans_count, infinities_count', 'ORDER BY value DESC' ].join('\n')); diff --git a/test/acceptance/dataviews/overviews.js b/test/acceptance/dataviews/overviews.js index c822caee..08ee6b7c 100644 --- a/test/acceptance/dataviews/overviews.js +++ b/test/acceptance/dataviews/overviews.js @@ -505,7 +505,7 @@ describe('dataviews using tables with overviews', function() { describe('aggregation special float values', function () { var params = {}; - it("should expose an aggregation dataview", function (done) { + it("should expose an aggregation dataview filtering special float values out", function (done) { var testClient = new TestClient(overviewsMapConfig); testClient.getDataview('test_categories_special_values', params, function (err, dataview) { if (err) { @@ -519,12 +519,8 @@ describe('dataviews using tables with overviews', function() { infinities: 1, min: 6, max: 6, - categoriesCount: 3, - categories:[ - { category: 'El Rey del Tallarín', value: null, agg: false }, - { category: 'El Lacón', value: null, agg: false }, - { category: 'Hawai', value: 6, agg: false } - ], + categoriesCount: 1, + categories: [ { category: 'Hawai', value: 6, agg: false } ], type: 'aggregation' }); testClient.drain(done); From 06d40e8b1e0288e3a6d477c90f40c7fa03869274 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 14 Jun 2017 18:57:26 +0200 Subject: [PATCH 20/34] Going red: add test to check that special float values are not being filtered out when the layer uses overviews --- test/acceptance/dataviews/overviews.js | 31 ++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/acceptance/dataviews/overviews.js b/test/acceptance/dataviews/overviews.js index 08ee6b7c..20a7ca9d 100644 --- a/test/acceptance/dataviews/overviews.js +++ b/test/acceptance/dataviews/overviews.js @@ -178,6 +178,16 @@ describe('dataviews using tables with overviews', function() { bins: 2 } }, + test_histogram_special_values: { + type: 'histogram', + source: { + id: 'data-source-special-float-values' + }, + options: { + column: 'value', + bins: 2 + } + }, test_avg: { type: 'formula', source: {id: 'data-source'}, @@ -526,6 +536,27 @@ describe('dataviews using tables with overviews', function() { testClient.drain(done); }); }); + + it('should expose a histogram dataview filtering special float values out', function (done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_histogram_special_values', params, function (err, dataview) { + if (err) { + return done(err); + } + assert.deepEqual(dataview, { + bin_width: 0, + bins_count: 1, + bins_start: 3, + nulls: 0, + infinities: 1, + nans: 1, + avg: 3, + bins: [ { bin: 0, min: 3, max: 3, avg: 3, freq: 2 } ], + type: 'histogram' + }); + testClient.drain(done); + }); + }); }); }); }); From dee00e6abd10e78583c3d0871e26f73bda989e2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 14 Jun 2017 19:00:37 +0200 Subject: [PATCH 21/34] Going green: handle special float values for histogram when overviews are involved --- .../models/dataview/overviews/histogram.js | 67 ++++++++++++++++--- 1 file changed, 59 insertions(+), 8 deletions(-) diff --git a/lib/cartodb/models/dataview/overviews/histogram.js b/lib/cartodb/models/dataview/overviews/histogram.js index 67da4514..ed88a621 100644 --- a/lib/cartodb/models/dataview/overviews/histogram.js +++ b/lib/cartodb/models/dataview/overviews/histogram.js @@ -12,12 +12,27 @@ var columnTypeQueryTpl = dot.template( var BIN_MIN_NUMBER = 6; var BIN_MAX_NUMBER = 48; +var filteredQueryTpl = dot.template([ + 'filtered_source AS (', + ' SELECT *', + ' FROM ({{=it._query}}) _cdb_filtered_source', + ' WHERE', + ' {{=it._column}} IS NOT NULL', + ' AND', + ' {{=it._column}} != \'infinity\'::float', + ' AND', + ' {{=it._column}} != \'-infinity\'::float', + ' AND', + ' {{=it._column}} != \'NaN\'::float', + ')' +].join(' \n')); + var basicsQueryTpl = dot.template([ 'basics AS (', ' SELECT', ' max({{=it._column}}) AS max_val, min({{=it._column}}) AS min_val,', ' sum({{=it._column}}*_feature_count)/sum(_feature_count) AS avg_val, sum(_feature_count) AS total_rows', - ' FROM ({{=it._query}}) _cdb_basics', + ' FROM filtered_source', ')' ].join(' \n')); @@ -26,7 +41,7 @@ var overrideBasicsQueryTpl = dot.template([ ' SELECT', ' max({{=it._end}}) AS max_val, min({{=it._start}}) AS min_val,', ' sum({{=it._column}}*_feature_count)/sum(_feature_count) AS avg_val, sum(_feature_count) AS total_rows', - ' FROM ({{=it._query}}) _cdb_basics', + ' FROM filtered_source', ')' ].join('\n')); @@ -37,7 +52,7 @@ var iqrQueryTpl = dot.template([ ' SELECT quartile, max(_cdb_iqr_column) AS quartile_max from (', ' SELECT {{=it._column}} AS _cdb_iqr_column, ntile(4) over (order by {{=it._column}}', ' ) AS quartile', - ' FROM ({{=it._query}}) _cdb_rank) _cdb_quartiles', + ' FROM filtered_source) _cdb_quartiles', ' WHERE quartile = 1 or quartile = 3', ' GROUP BY quartile', ' ) _cdb_iqr', @@ -56,7 +71,7 @@ var binsQueryTpl = dot.template([ ' )', ' )', ' END AS bins_number', - ' FROM basics, iqrange, ({{=it._query}}) _cdb_bins', + ' FROM basics, iqrange, filtered_source', ' LIMIT 1', ')' ].join('\n')); @@ -76,11 +91,34 @@ var nullsQueryTpl = dot.template([ ')' ].join('\n')); +var infinitiesQueryTpl = dot.template([ + 'infinities AS (', + ' SELECT', + ' count(*) AS infinities_count', + ' FROM ({{=it._query}}) _cdb_histogram_infinities', + ' WHERE', + ' {{=it._column}} = \'infinity\'::float', + ' OR', + ' {{=it._column}} = \'-infinity\'::float', + ')' +].join('\n')); + +var nansQueryTpl = dot.template([ + 'nans AS (', + ' SELECT', + ' count(*) AS nans_count', + ' FROM ({{=it._query}}) _cdb_histogram_infinities', + ' WHERE {{=it._column}} = \'NaN\'::float', + ')' +].join('\n')); + var histogramQueryTpl = dot.template([ 'SELECT', ' (max_val - min_val) / cast(bins_number as float) AS bin_width,', ' bins_number,', ' nulls_count,', + ' infinities_count,', + ' nans_count,', ' avg_val,', ' CASE WHEN min_val = max_val', ' THEN 0', @@ -90,9 +128,8 @@ var histogramQueryTpl = dot.template([ ' max({{=it._column}})::numeric AS max,', ' sum({{=it._column}}*_feature_count)/sum(_feature_count)::numeric AS avg,', ' sum(_feature_count) AS freq', - 'FROM ({{=it._query}}) _cdb_histogram, basics, nulls, bins', - 'WHERE {{=it._column}} IS NOT NULL', - 'GROUP BY bin, bins_number, bin_width, nulls_count, avg_val', + 'FROM filtered_source, basics, nulls, infinities, nans, bins', + 'GROUP BY bin, bins_number, bin_width, nulls_count, infinities_count, nans_count, avg_val', 'ORDER BY bin' ].join('\n')); @@ -156,7 +193,12 @@ Histogram.prototype.sql = function(psql, override, callback) { var _query = this.rewrittenQuery(this.query); - var basicsQuery, binsQuery; + var filteredQuery, basicsQuery, binsQuery; + + filteredQuery = filteredQueryTpl({ + _query: _query, + _column: _column + }); if (override && _.has(override, 'start') && _.has(override, 'end') && _.has(override, 'bins')) { basicsQuery = overrideBasicsQueryTpl({ @@ -202,11 +244,20 @@ Histogram.prototype.sql = function(psql, override, callback) { var histogramSql = [ "WITH", [ + filteredQuery, basicsQuery, binsQuery, nullsQueryTpl({ _query: _query, _column: _column + }), + infinitiesQueryTpl({ + _query: _query, + _column: _column + }), + nansQueryTpl({ + _query: _query, + _column: _column }) ].join(',\n'), histogramQueryTpl({ From ef849aec342a588ee359f9af70e83480a2a90d6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 14 Jun 2017 19:19:08 +0200 Subject: [PATCH 22/34] Going red: add test to check that special float values are not being filtered out in formula dataview when the layer uses overviews --- test/acceptance/dataviews/overviews.js | 28 ++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/acceptance/dataviews/overviews.js b/test/acceptance/dataviews/overviews.js index 20a7ca9d..a024febd 100644 --- a/test/acceptance/dataviews/overviews.js +++ b/test/acceptance/dataviews/overviews.js @@ -196,6 +196,16 @@ describe('dataviews using tables with overviews', function() { operation: 'avg' } }, + test_sum_special_values: { + type: 'formula', + source: { + id: 'data-source-special-float-values' + }, + options: { + column: 'value', + operation: 'sum' + } + }, test_count: { type: 'formula', source: {id: 'data-source'}, @@ -557,6 +567,24 @@ describe('dataviews using tables with overviews', function() { testClient.drain(done); }); }); + + it('should expose a formula (sum) dataview filtering special float values out', function (done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_sum_special_values', params, function (err, dataview) { + if (err) { + return done(err); + } + assert.deepEqual(dataview, { + operation: 'sum', + result: 6, + nulls: 0, + nans: 1, + infinities: 1, + type: 'formula' + }); + testClient.drain(done); + }); + }); }); }); }); From 81f60959e59901e8f412d9f13a83f17aee62a230 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 14 Jun 2017 19:20:39 +0200 Subject: [PATCH 23/34] Going green: handle special float values for formula when overviews are involved --- .../models/dataview/overviews/formula.js | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/lib/cartodb/models/dataview/overviews/formula.js b/lib/cartodb/models/dataview/overviews/formula.js index 9e331f0b..ba98b2c8 100644 --- a/lib/cartodb/models/dataview/overviews/formula.js +++ b/lib/cartodb/models/dataview/overviews/formula.js @@ -14,14 +14,34 @@ var formulaQueryTpls = { 'sum': dot.template([ 'SELECT', 'sum({{=it._column}}*_feature_count) AS result,', - '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', - 'FROM ({{=it._query}}) _cdb_formula' + '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count,', + '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_infinities', + ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count,', + '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nans', + ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count', + 'FROM ({{=it._query}}) _cdb_formula', + 'WHERE', + ' {{=it._column}} != \'infinity\'::float', + 'AND', + ' {{=it._column}} != \'-infinity\'::float', + 'AND', + ' {{=it._column}} != \'NaN\'::float' ].join('\n')), 'avg': dot.template([ 'SELECT', 'sum({{=it._column}}*_feature_count)/sum(_feature_count) AS result,', - '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', - 'FROM ({{=it._query}}) _cdb_formula' + '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count,', + '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_infinities', + ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count,', + '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nans', + ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count', + 'FROM ({{=it._query}}) _cdb_formula', + 'WHERE', + ' {{=it._column}} != \'infinity\'::float', + 'AND', + ' {{=it._column}} != \'-infinity\'::float', + 'AND', + ' {{=it._column}} != \'NaN\'::float' ].join('\n')), }; From 7d0af4e2596982fb154d2ad4bf495f11d9d254fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 14 Jun 2017 19:20:39 +0200 Subject: [PATCH 24/34] Going green: handle special float values for formula when overviews are involved --- .../models/dataview/overviews/formula.js | 28 ++++++-- test/acceptance/dataviews/overviews.js | 65 ++++++++++++++++--- 2 files changed, 80 insertions(+), 13 deletions(-) diff --git a/lib/cartodb/models/dataview/overviews/formula.js b/lib/cartodb/models/dataview/overviews/formula.js index 9e331f0b..ba98b2c8 100644 --- a/lib/cartodb/models/dataview/overviews/formula.js +++ b/lib/cartodb/models/dataview/overviews/formula.js @@ -14,14 +14,34 @@ var formulaQueryTpls = { 'sum': dot.template([ 'SELECT', 'sum({{=it._column}}*_feature_count) AS result,', - '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', - 'FROM ({{=it._query}}) _cdb_formula' + '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count,', + '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_infinities', + ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count,', + '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nans', + ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count', + 'FROM ({{=it._query}}) _cdb_formula', + 'WHERE', + ' {{=it._column}} != \'infinity\'::float', + 'AND', + ' {{=it._column}} != \'-infinity\'::float', + 'AND', + ' {{=it._column}} != \'NaN\'::float' ].join('\n')), 'avg': dot.template([ 'SELECT', 'sum({{=it._column}}*_feature_count)/sum(_feature_count) AS result,', - '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', - 'FROM ({{=it._query}}) _cdb_formula' + '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count,', + '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_infinities', + ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count,', + '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nans', + ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count', + 'FROM ({{=it._query}}) _cdb_formula', + 'WHERE', + ' {{=it._column}} != \'infinity\'::float', + 'AND', + ' {{=it._column}} != \'-infinity\'::float', + 'AND', + ' {{=it._column}} != \'NaN\'::float' ].join('\n')), }; diff --git a/test/acceptance/dataviews/overviews.js b/test/acceptance/dataviews/overviews.js index a024febd..4362d7a3 100644 --- a/test/acceptance/dataviews/overviews.js +++ b/test/acceptance/dataviews/overviews.js @@ -196,7 +196,7 @@ describe('dataviews using tables with overviews', function() { operation: 'avg' } }, - test_sum_special_values: { + test_formula_sum_special_values: { type: 'formula', source: { id: 'data-source-special-float-values' @@ -261,7 +261,14 @@ describe('dataviews using tables with overviews', function() { if (err) { return done(err); } - assert.deepEqual(formula_result, {"operation":"sum","result":15,"nulls":0,"type":"formula"}); + assert.deepEqual(formula_result, { + "operation":"sum", + "result":15, + "infinities": 0, + "nans": 0, + "nulls":0, + "type":"formula" + }); testClient.drain(done); }); @@ -273,7 +280,14 @@ describe('dataviews using tables with overviews', function() { if (err) { return done(err); } - assert.deepEqual(formula_result, {"operation":"avg","result":3,"nulls":0,"type":"formula"}); + assert.deepEqual(formula_result, { + "operation":"avg", + "result":3, + "nulls":0, + "type":"formula", + "infinities": 0, + "nans": 0 + }); testClient.drain(done); }); @@ -285,7 +299,12 @@ describe('dataviews using tables with overviews', function() { if (err) { return done(err); } - assert.deepEqual(formula_result, {"operation":"count","result":5,"nulls":0,"type":"formula"}); + assert.deepEqual(formula_result, { + "operation":"count", + "result":5, + "nulls":0, + "type":"formula" + }); testClient.drain(done); }); @@ -338,7 +357,14 @@ describe('dataviews using tables with overviews', function() { if (err) { return done(err); } - assert.deepEqual(formula_result, {"operation":"sum","result":15,"nulls":0,"type":"formula"}); + assert.deepEqual(formula_result, { + "operation":"sum", + "result":15, + "nulls":0, + "infinities": 0, + "nans": 0, + "type":"formula" + }); testClient.drain(done); }); @@ -435,7 +461,14 @@ describe('dataviews using tables with overviews', function() { if (err) { return done(err); } - assert.deepEqual(formula_result, {"operation":"sum","result":1,"nulls":0,"type":"formula"}); + assert.deepEqual(formula_result, { + "operation":"sum", + "result":1, + "nulls":0, + "infinities": 0, + "nans": 0, + "type":"formula" + }); testClient.drain(done); }); }); @@ -446,7 +479,14 @@ describe('dataviews using tables with overviews', function() { if (err) { return done(err); } - assert.deepEqual(formula_result, {"operation":"avg","result":1,"nulls":0,"type":"formula"}); + assert.deepEqual(formula_result, { + "operation":"avg", + "result":1, + "nulls":0, + "infinities": 0, + "nans": 0, + "type":"formula" + }); testClient.drain(done); }); @@ -514,7 +554,14 @@ describe('dataviews using tables with overviews', function() { if (err) { return done(err); } - assert.deepEqual(formula_result, {"operation":"sum","result":1,"nulls":0,"type":"formula"}); + assert.deepEqual(formula_result, { + "operation":"sum", + "result":1, + "nulls":0, + "infinities": 0, + "nans": 0, + "type":"formula" + }); testClient.drain(done); }); }); @@ -570,7 +617,7 @@ describe('dataviews using tables with overviews', function() { it('should expose a formula (sum) dataview filtering special float values out', function (done) { var testClient = new TestClient(overviewsMapConfig); - testClient.getDataview('test_sum_special_values', params, function (err, dataview) { + testClient.getDataview('test_formula_sum_special_values', params, function (err, dataview) { if (err) { return done(err); } From 443c1100d7487b2f2d29a8a023eb3bf0d7675f64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 15 Jun 2017 16:31:24 +0200 Subject: [PATCH 25/34] Formula dataview: support special values only if column is a float column --- lib/cartodb/models/dataview/formula.js | 45 +++++++++--- .../models/dataview/overviews/formula.js | 68 +++++++++++++++---- test/acceptance/dataviews/overviews.js | 13 +++- 3 files changed, 102 insertions(+), 24 deletions(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index 04b89835..1d3204e7 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -5,25 +5,25 @@ var debug = require('debug')('windshaft:widget:formula'); var dot = require('dot'); dot.templateSettings.strip = false; +var columnTypeQueryTpl = dot.template( + 'SELECT pg_typeof({{=it.column}})::oid FROM ({{=it.query}}) _cdb_histogram_column_type limit 1' +); + var formulaQueryTpl = dot.template([ 'SELECT', ' {{=it._operation}}({{=it._column}}) AS result,', ' (SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', - ' {{?it._operation!==\'count\'}}', - ' ,(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls', + ' {{?it._isFloatColumn}},(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls', ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count', ' ,(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls', - ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count', - ' {{?}}', + ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count{{?}}', 'FROM ({{=it._query}}) _cdb_formula', - '{{?it._operation!==\'count\'}}', - 'WHERE', + '{{?it._isFloatColumn}}WHERE', ' {{=it._column}} != \'infinity\'::float', 'AND', ' {{=it._column}} != \'-infinity\'::float', 'AND', - ' {{=it._column}} != \'NaN\'::float', - '{{?}}' + ' {{=it._column}} != \'NaN\'::float{{?}}' ].join('\n')); var VALID_OPERATIONS = { @@ -34,6 +34,11 @@ var VALID_OPERATIONS = { max: true }; +var FLOAT_OIDS = { + 700: true, + 701: true +}; + var TYPE = 'formula'; /** @@ -63,6 +68,7 @@ function Formula(query, options) { this.query = query; this.column = options.column || '1'; this.operation = options.operation; + this._isFloatColumn = null; } Formula.prototype = new BaseWidget(); @@ -71,13 +77,36 @@ Formula.prototype.constructor = Formula; module.exports = Formula; Formula.prototype.sql = function(psql, override, callback) { + var self = this; + if (!callback) { callback = override; override = {}; } var _query = this.query; + + var columnTypeQuery = columnTypeQueryTpl({ + column: this.column, query: _query + }); + + if (this._isFloatColumn === null) { + var readOnlyTransaction = true; + psql.query(columnTypeQuery, function(err, result) { + self._isFloatColumn = false; + if (!err && !!result.rows[0]) { + var pgType = result.rows[0].pg_typeof; + if (FLOAT_OIDS.hasOwnProperty(pgType)) { + self._isFloatColumn = true; + } + } + self.sql(psql, override, callback); + }, readOnlyTransaction); + return null; + } + var formulaSql = formulaQueryTpl({ + _isFloatColumn: this._isFloatColumn, _query: _query, _operation: this.operation, _column: this.column diff --git a/lib/cartodb/models/dataview/overviews/formula.js b/lib/cartodb/models/dataview/overviews/formula.js index ba98b2c8..a1437c73 100644 --- a/lib/cartodb/models/dataview/overviews/formula.js +++ b/lib/cartodb/models/dataview/overviews/formula.js @@ -1,47 +1,61 @@ var BaseOverviewsDataview = require('./base'); var BaseDataview = require('../formula'); +var debug = require('debug')('windshaft:widget:formula:overview'); var dot = require('dot'); dot.templateSettings.strip = false; +var FLOAT_OIDS = { + 700: true, + 701: true +}; + +var columnTypeQueryTpl = dot.template( + 'SELECT pg_typeof({{=it.column}})::oid FROM ({{=it.query}}) _cdb_histogram_column_type limit 1' +); + var formulaQueryTpls = { 'count': dot.template([ 'SELECT', 'sum(_feature_count) AS result,', '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', + '{{?it._isFloatColumn}},(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_infinities', + ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count,', + '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nans', + ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count{{?}}', 'FROM ({{=it._query}}) _cdb_formula' ].join('\n')), 'sum': dot.template([ 'SELECT', 'sum({{=it._column}}*_feature_count) AS result,', - '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count,', - '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_infinities', - ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count,', - '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nans', - ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count', + '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', + '{{?it._isFloatColumn}},(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_infinities', + ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count', + ',(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nans', + ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count{{?}}', 'FROM ({{=it._query}}) _cdb_formula', - 'WHERE', + '{{?it._isFloatColumn}}WHERE', ' {{=it._column}} != \'infinity\'::float', 'AND', ' {{=it._column}} != \'-infinity\'::float', 'AND', - ' {{=it._column}} != \'NaN\'::float' + ' {{=it._column}} != \'NaN\'::float{{?}}' ].join('\n')), 'avg': dot.template([ 'SELECT', 'sum({{=it._column}}*_feature_count)/sum(_feature_count) AS result,', - '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count,', - '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_infinities', - ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count,', - '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nans', - ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count', + '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', + '{{?it._isFloatColumn}},(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_infinities', + ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count', + ',(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nans', + ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count{{?}}', 'FROM ({{=it._query}}) _cdb_formula', - 'WHERE', + '{{?it._isFloatColumn}}WHERE', ' {{=it._column}} != \'infinity\'::float', 'AND', ' {{=it._column}} != \'-infinity\'::float', 'AND', - ' {{=it._column}} != \'NaN\'::float' + ' {{=it._column}} != \'NaN\'::float{{?}}' ].join('\n')), }; @@ -49,6 +63,7 @@ function Formula(query, options, queryRewriter, queryRewriteData, params) { BaseOverviewsDataview.call(this, query, options, BaseDataview, queryRewriter, queryRewriteData, params); this.column = options.column || '1'; this.operation = options.operation; + this._isFloatColumn = null; } Formula.prototype = Object.create(BaseOverviewsDataview.prototype); @@ -57,20 +72,45 @@ Formula.prototype.constructor = Formula; module.exports = Formula; Formula.prototype.sql = function(psql, override, callback) { + var self = this; var formulaQueryTpl = formulaQueryTpls[this.operation]; if ( formulaQueryTpl ) { // supported formula for use with overviews + + var columnTypeQuery = columnTypeQueryTpl({ + column: this.column, query: this.query + }); + + if (this._isFloatColumn === null) { + var readOnlyTransaction = true; + psql.query(columnTypeQuery, function(err, result) { + self._isFloatColumn = false; + if (!err && !!result.rows[0]) { + var pgType = result.rows[0].pg_typeof; + if (FLOAT_OIDS.hasOwnProperty(pgType)) { + self._isFloatColumn = true; + } + } + self.sql(psql, override, callback); + }, readOnlyTransaction); + return null; + } + var formulaSql = formulaQueryTpl({ + _isFloatColumn: this._isFloatColumn, _query: this.rewrittenQuery(this.query), _operation: this.operation, _column: this.column }); callback = callback || override; + debug(formulaSql); + return callback(null, formulaSql); } + // default behaviour return this.defaultSql(psql, override, callback); }; diff --git a/test/acceptance/dataviews/overviews.js b/test/acceptance/dataviews/overviews.js index 4362d7a3..dcff687f 100644 --- a/test/acceptance/dataviews/overviews.js +++ b/test/acceptance/dataviews/overviews.js @@ -303,7 +303,9 @@ describe('dataviews using tables with overviews', function() { "operation":"count", "result":5, "nulls":0, - "type":"formula" + "type":"formula", + "infinities": 0, + "nans": 0 }); testClient.drain(done); @@ -498,7 +500,14 @@ describe('dataviews using tables with overviews', function() { if (err) { return done(err); } - assert.deepEqual(formula_result, {"operation":"count","result":1,"nulls":0,"type":"formula"}); + assert.deepEqual(formula_result, { + "operation":"count", + "result":1, + "infinities": 0, + "nans": 0, + "nulls":0, + "type":"formula" + }); testClient.drain(done); }); From ad570ab6f23d8b5b5e017dcecf75b09e4af7ee0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 15 Jun 2017 18:04:35 +0200 Subject: [PATCH 26/34] Use dataview base to get column type in formula dataviews --- lib/cartodb/models/dataview/base.js | 34 +++++ lib/cartodb/models/dataview/formula.js | 31 +---- .../models/dataview/overviews/formula.js | 121 ++++++++---------- 3 files changed, 92 insertions(+), 94 deletions(-) diff --git a/lib/cartodb/models/dataview/base.js b/lib/cartodb/models/dataview/base.js index b2e2f188..d7479b6d 100644 --- a/lib/cartodb/models/dataview/base.js +++ b/lib/cartodb/models/dataview/base.js @@ -1,3 +1,6 @@ +var dot = require('dot'); +dot.templateSettings.strip = false; + function BaseDataview() {} module.exports = BaseDataview; @@ -24,3 +27,34 @@ BaseDataview.prototype.getResult = function(psql, override, callback) { BaseDataview.prototype.search = function(psql, userQuery, callback) { return callback(null, this.format({ rows: [] })); }; + +var FLOAT_OIDS = { + 700: true, + 701: true +}; + +var columnTypeQueryTpl = dot.template( + 'SELECT pg_typeof({{=it.column}})::oid FROM ({{=it.query}}) _cdb_column_type limit 1' +); + +BaseDataview.prototype.getColumnType = function (psql, column, query, callback) { + var readOnlyTransaction = true; + + var columnTypeQuery = columnTypeQueryTpl({ + column: column, query: query + }); + + psql.query(columnTypeQuery, function(err, result) { + if (err) { + return callback(err); + } + var pgType = result.rows[0].pg_typeof; + callback(null, getPGTypeName(pgType)); + }, readOnlyTransaction); +}; + +function getPGTypeName (pgType) { + return { + float: FLOAT_OIDS.hasOwnProperty(pgType) + }; +} diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index 1d3204e7..2272089c 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -5,10 +5,6 @@ var debug = require('debug')('windshaft:widget:formula'); var dot = require('dot'); dot.templateSettings.strip = false; -var columnTypeQueryTpl = dot.template( - 'SELECT pg_typeof({{=it.column}})::oid FROM ({{=it.query}}) _cdb_histogram_column_type limit 1' -); - var formulaQueryTpl = dot.template([ 'SELECT', ' {{=it._operation}}({{=it._column}}) AS result,', @@ -34,11 +30,6 @@ var VALID_OPERATIONS = { max: true }; -var FLOAT_OIDS = { - 700: true, - 701: true -}; - var TYPE = 'formula'; /** @@ -84,30 +75,20 @@ Formula.prototype.sql = function(psql, override, callback) { override = {}; } - var _query = this.query; - - var columnTypeQuery = columnTypeQueryTpl({ - column: this.column, query: _query - }); - if (this._isFloatColumn === null) { - var readOnlyTransaction = true; - psql.query(columnTypeQuery, function(err, result) { - self._isFloatColumn = false; - if (!err && !!result.rows[0]) { - var pgType = result.rows[0].pg_typeof; - if (FLOAT_OIDS.hasOwnProperty(pgType)) { - self._isFloatColumn = true; - } + this._isFloatColumn = false; + this.getColumnType(psql, this.column, this.query, function (err, type) { + if (!err && !!type) { + self._isFloatColumn = type.float; } self.sql(psql, override, callback); - }, readOnlyTransaction); + }); return null; } var formulaSql = formulaQueryTpl({ _isFloatColumn: this._isFloatColumn, - _query: _query, + _query: this.query, _operation: this.operation, _column: this.column }); diff --git a/lib/cartodb/models/dataview/overviews/formula.js b/lib/cartodb/models/dataview/overviews/formula.js index a1437c73..533dd921 100644 --- a/lib/cartodb/models/dataview/overviews/formula.js +++ b/lib/cartodb/models/dataview/overviews/formula.js @@ -5,58 +5,49 @@ var debug = require('debug')('windshaft:widget:formula:overview'); var dot = require('dot'); dot.templateSettings.strip = false; -var FLOAT_OIDS = { - 700: true, - 701: true -}; - -var columnTypeQueryTpl = dot.template( - 'SELECT pg_typeof({{=it.column}})::oid FROM ({{=it.query}}) _cdb_histogram_column_type limit 1' -); - var formulaQueryTpls = { - 'count': dot.template([ - 'SELECT', - 'sum(_feature_count) AS result,', - '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', - '{{?it._isFloatColumn}},(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_infinities', - ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count,', - '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nans', - ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count{{?}}', - 'FROM ({{=it._query}}) _cdb_formula' - ].join('\n')), - 'sum': dot.template([ - 'SELECT', - 'sum({{=it._column}}*_feature_count) AS result,', - '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', - '{{?it._isFloatColumn}},(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_infinities', - ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count', - ',(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nans', - ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count{{?}}', - 'FROM ({{=it._query}}) _cdb_formula', - '{{?it._isFloatColumn}}WHERE', - ' {{=it._column}} != \'infinity\'::float', - 'AND', - ' {{=it._column}} != \'-infinity\'::float', - 'AND', - ' {{=it._column}} != \'NaN\'::float{{?}}' - ].join('\n')), - 'avg': dot.template([ - 'SELECT', - 'sum({{=it._column}}*_feature_count)/sum(_feature_count) AS result,', - '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', - '{{?it._isFloatColumn}},(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_infinities', - ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count', - ',(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nans', - ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count{{?}}', - 'FROM ({{=it._query}}) _cdb_formula', - '{{?it._isFloatColumn}}WHERE', - ' {{=it._column}} != \'infinity\'::float', - 'AND', - ' {{=it._column}} != \'-infinity\'::float', - 'AND', - ' {{=it._column}} != \'NaN\'::float{{?}}' - ].join('\n')), + 'count': dot.template([ + 'SELECT', + 'sum(_feature_count) AS result,', + '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', + '{{?it._isFloatColumn}},(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_infinities', + ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count,', + '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nans', + ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count{{?}}', + 'FROM ({{=it._query}}) _cdb_formula' + ].join('\n')), + 'sum': dot.template([ + 'SELECT', + 'sum({{=it._column}}*_feature_count) AS result,', + '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', + '{{?it._isFloatColumn}},(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_infinities', + ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count', + ',(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nans', + ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count{{?}}', + 'FROM ({{=it._query}}) _cdb_formula', + '{{?it._isFloatColumn}}WHERE', + ' {{=it._column}} != \'infinity\'::float', + 'AND', + ' {{=it._column}} != \'-infinity\'::float', + 'AND', + ' {{=it._column}} != \'NaN\'::float{{?}}' + ].join('\n')), + 'avg': dot.template([ + 'SELECT', + 'sum({{=it._column}}*_feature_count)/sum(_feature_count) AS result,', + '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', + '{{?it._isFloatColumn}},(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_infinities', + ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count', + ',(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nans', + ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count{{?}}', + 'FROM ({{=it._query}}) _cdb_formula', + '{{?it._isFloatColumn}}WHERE', + ' {{=it._column}} != \'infinity\'::float', + 'AND', + ' {{=it._column}} != \'-infinity\'::float', + 'AND', + ' {{=it._column}} != \'NaN\'::float{{?}}' + ].join('\n')), }; function Formula(query, options, queryRewriter, queryRewriteData, params) { @@ -71,38 +62,30 @@ Formula.prototype.constructor = Formula; module.exports = Formula; -Formula.prototype.sql = function(psql, override, callback) { +Formula.prototype.sql = function (psql, override, callback) { var self = this; var formulaQueryTpl = formulaQueryTpls[this.operation]; - if ( formulaQueryTpl ) { + if (formulaQueryTpl) { // supported formula for use with overviews - - var columnTypeQuery = columnTypeQueryTpl({ - column: this.column, query: this.query - }); - if (this._isFloatColumn === null) { - var readOnlyTransaction = true; - psql.query(columnTypeQuery, function(err, result) { - self._isFloatColumn = false; - if (!err && !!result.rows[0]) { - var pgType = result.rows[0].pg_typeof; - if (FLOAT_OIDS.hasOwnProperty(pgType)) { - self._isFloatColumn = true; - } + this._isFloatColumn = false; + this.getColumnType(psql, this.column, this.query, function (err, type) { + if (!err && !!type) { + self._isFloatColumn = type.float; } self.sql(psql, override, callback); - }, readOnlyTransaction); + }); return null; } var formulaSql = formulaQueryTpl({ _isFloatColumn: this._isFloatColumn, - _query: this.rewrittenQuery(this.query), + _query: this.rewrittenQuery(this.query), _operation: this.operation, - _column: this.column + _column: this.column }); + callback = callback || override; debug(formulaSql); From 115d8fe6854f6971690a2a8ad7d55b02ed802b0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 15 Jun 2017 19:07:31 +0200 Subject: [PATCH 27/34] Handle special float values only when aggregation columns is float --- lib/cartodb/models/dataview/aggregation.js | 63 ++++++++++++++++------ 1 file changed, 47 insertions(+), 16 deletions(-) diff --git a/lib/cartodb/models/dataview/aggregation.js b/lib/cartodb/models/dataview/aggregation.js index e61b1454..0d576614 100644 --- a/lib/cartodb/models/dataview/aggregation.js +++ b/lib/cartodb/models/dataview/aggregation.js @@ -9,14 +9,12 @@ var filteredQueryTpl = dot.template([ 'filtered_source AS (', ' SELECT *', ' FROM ({{=it._query}}) _cdb_filtered_source', - ' {{?it._aggregationColumn}}', - ' WHERE', + ' {{?it._aggregationColumn && it._isFloatColumn}}WHERE', ' {{=it._aggregationColumn}} != \'infinity\'::float', ' AND', ' {{=it._aggregationColumn}} != \'-infinity\'::float', ' AND', - ' {{=it._aggregationColumn}} != \'NaN\'::float', - ' {{?}}', + ' {{=it._aggregationColumn}} != \'NaN\'::float{{?}}', ')' ].join(' \n')); @@ -24,15 +22,15 @@ var summaryQueryTpl = dot.template([ 'summary AS (', ' SELECT', ' count(1) AS count,', - ' sum(CASE WHEN {{=it._column}} IS NULL THEN 1 ELSE 0 END) AS nulls_count,', - ' sum(', + ' sum(CASE WHEN {{=it._column}} IS NULL THEN 1 ELSE 0 END) AS nulls_count', + ' {{?it._isFloatColumn}},sum(', ' CASE', ' WHEN {{=it._aggregationColumn}} = \'infinity\'::float OR {{=it._aggregationColumn}} = \'-infinity\'::float', ' THEN 1', ' ELSE 0', ' END', ' ) AS infinities_count,', - ' sum(CASE WHEN {{=it._aggregationColumn}} = \'NaN\'::float THEN 1 ELSE 0 END) AS nans_count', + ' sum(CASE WHEN {{=it._aggregationColumn}} = \'NaN\'::float THEN 1 ELSE 0 END) AS nans_count{{?}}', ' FROM ({{=it._query}}) _cdb_aggregation_nulls', ')' ].join('\n')); @@ -68,22 +66,24 @@ var categoriesSummaryCountQueryTpl = dot.template([ var rankedAggregationQueryTpl = dot.template([ 'SELECT CAST(category AS text), value, false as agg, nulls_count, min_val, max_val,', - ' count, categories_count, nans_count, infinities_count', + ' count, categories_count{{?it._isFloatColumn}}, nans_count, infinities_count{{?}}', ' FROM categories, summary, categories_summary_min_max, categories_summary_count', ' WHERE rank < {{=it._limit}}', 'UNION ALL', 'SELECT \'Other\' category, {{=it._aggregationFn}}(value) as value, true as agg, nulls_count,', - ' min_val, max_val, count, categories_count, nans_count, infinities_count', + ' min_val, max_val, count, categories_count{{?it._isFloatColumn}}, nans_count, infinities_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, nans_count, infinities_count' + 'GROUP BY nulls_count, min_val, max_val, count,', + ' categories_count{{?it._isFloatColumn}}, nans_count, infinities_count{{?}}' ].join('\n')); 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, nans_count, infinities_count', + ' nulls_count, min_val, max_val, count, categories_count{{?it._isFloatColumn}}, nans_count, infinities_count{{?}}', '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, nans_count, infinities_count', + 'GROUP BY category, nulls_count, min_val, max_val, count,', + ' categories_count{{?it._isFloatColumn}}, nans_count, infinities_count{{?}}', 'ORDER BY value DESC' ].join('\n')); @@ -108,7 +108,7 @@ var TYPE = 'aggregation'; } } */ -function Aggregation(query, options) { +function Aggregation(query, options, queries) { if (!_.isString(options.column)) { throw new Error('Aggregation expects `column` in widget options'); } @@ -132,9 +132,11 @@ function Aggregation(query, options) { BaseWidget.apply(this); this.query = query; + this.queries = queries; this.column = options.column; this.aggregation = options.aggregation; this.aggregationColumn = options.aggregationColumn; + this._isFloatColumn = null; } Aggregation.prototype = new BaseWidget(); @@ -143,19 +145,39 @@ Aggregation.prototype.constructor = Aggregation; module.exports = Aggregation; Aggregation.prototype.sql = function(psql, override, callback) { + var self = this; + if (!callback) { callback = override; override = {}; } + if (this.aggregationColumn && this._isFloatColumn === null) { + this._isFloatColumn = false; + this.getColumnType(psql, this.aggregationColumn, this.queries.no_filters, function (err, type) { + if (!err && !!type) { + self._isFloatColumn = type.float; + } + self.sql(psql, override, callback); + }); + return null; + } + var _query = this.query; var aggregationSql; if (!!override.ownFilter) { aggregationSql = [ - this.getCategoriesCTESql(_query, this.column, this.aggregation, this.aggregationColumn), + this.getCategoriesCTESql( + _query, + this.column, + this.aggregation, + this.aggregationColumn, + this._isFloatColumn + ), aggregationQueryTpl({ + _isFloatColumn: this._isFloatColumn, _query: _query, _column: this.column, _aggregation: this.getAggregationSql(), @@ -164,8 +186,15 @@ Aggregation.prototype.sql = function(psql, override, callback) { ].join('\n'); } else { aggregationSql = [ - this.getCategoriesCTESql(_query, this.column, this.aggregation, this.aggregationColumn), + this.getCategoriesCTESql( + _query, + this.column, + this.aggregation, + this.aggregationColumn, + this._isFloatColumn + ), rankedAggregationQueryTpl({ + _isFloatColumn: this._isFloatColumn, _query: _query, _column: this.column, _aggregationFn: this.aggregation !== 'count' ? this.aggregation : 'sum', @@ -179,16 +208,18 @@ Aggregation.prototype.sql = function(psql, override, callback) { return callback(null, aggregationSql); }; -Aggregation.prototype.getCategoriesCTESql = function(query, column, aggregation, aggregationColumn) { +Aggregation.prototype.getCategoriesCTESql = function(query, column, aggregation, aggregationColumn, isFloatColumn) { return [ "WITH", [ filteredQueryTpl({ + _isFloatColumn: isFloatColumn, _query: this.query, _column: this.column, _aggregationColumn: aggregation !== 'count' ? aggregationColumn : null }), summaryQueryTpl({ + _isFloatColumn: isFloatColumn, _query: query, _column: column, _aggregationColumn: aggregation !== 'count' ? aggregationColumn : null From 20d7f1a7c56749d8e18acb930c58b558c7b1bc12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 15 Jun 2017 19:22:26 +0200 Subject: [PATCH 28/34] Handle special float values only when aggregation columns is float (overviews) --- .../models/dataview/overviews/aggregation.js | 47 ++++++++++++++----- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/lib/cartodb/models/dataview/overviews/aggregation.js b/lib/cartodb/models/dataview/overviews/aggregation.js index cc95e73c..730ba822 100644 --- a/lib/cartodb/models/dataview/overviews/aggregation.js +++ b/lib/cartodb/models/dataview/overviews/aggregation.js @@ -9,14 +9,12 @@ var filteredQueryTpl = dot.template([ 'filtered_source AS (', ' SELECT *', ' FROM ({{=it._query}}) _cdb_filtered_source', - ' {{?it._aggregationColumn}}', - ' WHERE', + ' {{?it._aggregationColumn && it._isFloatColumn}}WHERE', ' {{=it._aggregationColumn}} != \'infinity\'::float', ' AND', ' {{=it._aggregationColumn}} != \'-infinity\'::float', ' AND', - ' {{=it._aggregationColumn}} != \'NaN\'::float', - ' {{?}}', + ' {{=it._aggregationColumn}} != \'NaN\'::float{{?}}', ')' ].join(' \n')); @@ -24,15 +22,15 @@ var summaryQueryTpl = dot.template([ 'summary AS (', ' SELECT', ' sum(_feature_count) AS count,', - ' sum(CASE WHEN {{=it._column}} IS NULL THEN 1 ELSE 0 END) AS nulls_count,', - ' sum(', + ' sum(CASE WHEN {{=it._column}} IS NULL THEN 1 ELSE 0 END) AS nulls_count', + ' {{?it._isFloatColumn}},sum(', ' CASE', ' WHEN {{=it._aggregationColumn}} = \'infinity\'::float OR {{=it._aggregationColumn}} = \'-infinity\'::float', ' THEN 1', ' ELSE 0', ' END', ' ) AS infinities_count,', - ' sum(CASE WHEN {{=it._aggregationColumn}} = \'NaN\'::float THEN 1 ELSE 0 END) AS nans_count', + ' sum(CASE WHEN {{=it._aggregationColumn}} = \'NaN\'::float THEN 1 ELSE 0 END) AS nans_count{{?}}', ' FROM ({{=it._query}}) _cdb_aggregation_nulls', ')' ].join('\n')); @@ -68,34 +66,38 @@ var categoriesSummaryCountQueryTpl = dot.template([ var rankedAggregationQueryTpl = dot.template([ 'SELECT CAST(category AS text), value, false as agg, nulls_count, min_val, max_val,', - ' count, categories_count, nans_count, infinities_count', + ' count, categories_count{{?it._isFloatColumn}}, nans_count, infinities_count{{?}}', ' 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, nans_count, infinities_count', + ' count, categories_count{{?it._isFloatColumn}}, nans_count, infinities_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, nans_count, infinities_count' + 'GROUP BY nulls_count, min_val, max_val, count,', + ' categories_count{{?it._isFloatColumn}}, nans_count, infinities_count{{?}}' ].join('\n')); 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, nans_count, infinities_count', + ' nulls_count, min_val, max_val, count, categories_count{{?it._isFloatColumn}}, nans_count, infinities_count{{?}}', 'FROM filtered_source, summary, categories_summary_min_max, categories_summary_count', - 'GROUP BY category, nulls_count, min_val, max_val, count, categories_count, nans_count, infinities_count', + 'GROUP BY category, nulls_count, min_val, max_val, count,', + ' categories_count{{?it._isFloatColumn}}, nans_count, infinities_count{{?}}', 'ORDER BY value DESC' ].join('\n')); var CATEGORIES_LIMIT = 6; - function Aggregation(query, options, queryRewriter, queryRewriteData, params) { + function Aggregation(query, options, queryRewriter, queryRewriteData, params, queries) { BaseOverviewsDataview.call(this, query, options, BaseDataview, queryRewriter, queryRewriteData, params); this.query = query; + this.queries = queries; this.column = options.column; this.aggregation = options.aggregation; this.aggregationColumn = options.aggregationColumn; + this._isFloatColumn = null; } Aggregation.prototype = Object.create(BaseOverviewsDataview.prototype); @@ -104,6 +106,8 @@ Aggregation.prototype.constructor = Aggregation; module.exports = Aggregation; Aggregation.prototype.sql = function(psql, override, callback) { + var self = this; + if (!callback) { callback = override; override = {}; @@ -112,17 +116,30 @@ Aggregation.prototype.sql = function(psql, override, callback) { var _query = this.rewrittenQuery(this.query); var _aggregationColumn = this.aggregation !== 'count' ? this.aggregationColumn : null; + if (this.aggregationColumn && this._isFloatColumn === null) { + this._isFloatColumn = false; + this.getColumnType(psql, this.aggregationColumn, this.queries.no_filters, function (err, type) { + if (!err && !!type) { + self._isFloatColumn = type.float; + } + self.sql(psql, override, callback); + }); + return null; + } + var aggregationSql; if (!!override.ownFilter) { aggregationSql = [ "WITH", [ filteredQueryTpl({ + _isFloatColumn: this._isFloatColumn, _query: _query, _column: this.column, _aggregationColumn: _aggregationColumn }), summaryQueryTpl({ + _isFloatColumn: this._isFloatColumn, _query: _query, _column: this.column, _aggregationColumn: _aggregationColumn @@ -143,6 +160,7 @@ Aggregation.prototype.sql = function(psql, override, callback) { }) ].join(',\n'), aggregationQueryTpl({ + _isFloatColumn: this._isFloatColumn, _query: _query, _column: this.column, _aggregation: this.getAggregationSql(), @@ -154,11 +172,13 @@ Aggregation.prototype.sql = function(psql, override, callback) { "WITH", [ filteredQueryTpl({ + _isFloatColumn: this._isFloatColumn, _query: _query, _column: this.column, _aggregationColumn: _aggregationColumn }), summaryQueryTpl({ + _isFloatColumn: this._isFloatColumn, _query: _query, _column: this.column, _aggregationColumn: _aggregationColumn @@ -179,6 +199,7 @@ Aggregation.prototype.sql = function(psql, override, callback) { }) ].join(',\n'), rankedAggregationQueryTpl({ + _isFloatColumn: this._isFloatColumn, _query: _query, _column: this.column, _limit: CATEGORIES_LIMIT From b1ac5b8ca9475dddb859a2e539e0538d062c0f9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 16 Jun 2017 12:57:46 +0200 Subject: [PATCH 29/34] Handle special float values only if column is float --- lib/cartodb/models/dataview/base.js | 9 +- lib/cartodb/models/dataview/histogram.js | 101 +++++++++-------- .../models/dataview/overviews/histogram.js | 104 ++++++++++-------- 3 files changed, 120 insertions(+), 94 deletions(-) diff --git a/lib/cartodb/models/dataview/base.js b/lib/cartodb/models/dataview/base.js index d7479b6d..2cd24261 100644 --- a/lib/cartodb/models/dataview/base.js +++ b/lib/cartodb/models/dataview/base.js @@ -33,6 +33,12 @@ var FLOAT_OIDS = { 701: true }; +var DATE_OIDS = { + 1082: true, + 1114: true, + 1184: true +}; + var columnTypeQueryTpl = dot.template( 'SELECT pg_typeof({{=it.column}})::oid FROM ({{=it.query}}) _cdb_column_type limit 1' ); @@ -55,6 +61,7 @@ BaseDataview.prototype.getColumnType = function (psql, column, query, callback) function getPGTypeName (pgType) { return { - float: FLOAT_OIDS.hasOwnProperty(pgType) + float: FLOAT_OIDS.hasOwnProperty(pgType), + date: DATE_OIDS.hasOwnProperty(pgType) }; } diff --git a/lib/cartodb/models/dataview/histogram.js b/lib/cartodb/models/dataview/histogram.js index f6b2635d..73cc8724 100644 --- a/lib/cartodb/models/dataview/histogram.js +++ b/lib/cartodb/models/dataview/histogram.js @@ -5,9 +5,6 @@ var debug = require('debug')('windshaft:dataview:histogram'); var dot = require('dot'); dot.templateSettings.strip = false; -var columnTypeQueryTpl = dot.template( - 'SELECT pg_typeof({{=it.column}})::oid FROM ({{=it.query}}) _cdb_histogram_column_type limit 1' -); var columnCastTpl = dot.template("date_part('epoch', {{=it.column}})"); var BIN_MIN_NUMBER = 6; @@ -19,12 +16,12 @@ var filteredQueryTpl = dot.template([ ' FROM ({{=it._query}}) _cdb_filtered_source', ' WHERE', ' {{=it._column}} IS NOT NULL', - ' AND', + ' {{?it._isFloatColumn}}AND', ' {{=it._column}} != \'infinity\'::float', ' AND', ' {{=it._column}} != \'-infinity\'::float', ' AND', - ' {{=it._column}} != \'NaN\'::float', + ' {{=it._column}} != \'NaN\'::float{{?}}', ')' ].join(' \n')); @@ -118,8 +115,8 @@ var histogramQueryTpl = dot.template([ ' (max_val - min_val) / cast(bins_number as float) AS bin_width,', ' bins_number,', ' nulls_count,', - ' infinities_count,', - ' nans_count,', + ' {{?it._isFloatColumn}}infinities_count,', + ' nans_count,{{?}}', ' avg_val,', ' CASE WHEN min_val = max_val', ' THEN 0', @@ -129,8 +126,9 @@ var histogramQueryTpl = dot.template([ ' max({{=it._column}})::numeric AS max,', ' avg({{=it._column}})::numeric AS avg,', ' count(*) AS freq', - 'FROM filtered_source, basics, nulls, infinities, nans, bins', - 'GROUP BY bin, bins_number, bin_width, nulls_count, infinities_count, nans_count, avg_val', + 'FROM filtered_source, basics, nulls, bins{{?it._isFloatColumn}}, infinities, nans{{?}}', + 'GROUP BY bin, bins_number, bin_width, nulls_count,', + ' avg_val{{?it._isFloatColumn}}, infinities_count, nans_count{{?}}', 'ORDER BY bin' ].join('\n')); @@ -164,55 +162,49 @@ Histogram.prototype.constructor = Histogram; module.exports = Histogram; -var DATE_OIDS = { - 1082: true, - 1114: true, - 1184: true -}; - Histogram.prototype.sql = function(psql, override, callback) { + var self = this; + if (!callback) { callback = override; override = {}; } - var self = this; - - var _column = this.column; - - var columnTypeQuery = columnTypeQueryTpl({ - column: _column, query: this.queries.no_filters - }); - if (this._columnType === null) { - psql.query(columnTypeQuery, function(err, result) { + this.getColumnType(psql, this.column, this.queries.no_filters, function (err, type) { // assume numeric, will fail later self._columnType = 'numeric'; - if (!err && !!result.rows[0]) { - var pgType = result.rows[0].pg_typeof; - if (DATE_OIDS.hasOwnProperty(pgType)) { - self._columnType = 'date'; - } + if (!err && !!type) { + self._columnType = Object.keys(type).find(function (key) { + return type[key]; + }); } self.sql(psql, override, callback); }, true); // use read-only transaction return null; } + var histogramSql = this._buildQuery(override); + + return callback(null, histogramSql); +}; + +Histogram.prototype._buildQuery = function (override) { + var filteredQuery, basicsQuery, binsQuery; + var _column = this.column; + var _query = this.query; + if (this._columnType === 'date') { _column = columnCastTpl({column: _column}); } - var _query = this.query; - - var filteredQuery, basicsQuery, binsQuery; - filteredQuery = filteredQueryTpl({ + _isFloatColumn: this._columnType === 'float', _query: _query, _column: _column }); - if (override && _.has(override, 'start') && _.has(override, 'end') && _.has(override, 'bins')) { + if (this._shouldOverride(override)) { debug('overriding with %j', override); basicsQuery = overrideBasicsQueryTpl({ _query: _query, @@ -232,7 +224,7 @@ Histogram.prototype.sql = function(psql, override, callback) { _column: _column }); - if (override && _.has(override, 'bins')) { + if (this._shouldOverrideBins(override)) { binsQuery = [ overrideBinsQueryTpl({ _bins: override.bins @@ -253,17 +245,18 @@ Histogram.prototype.sql = function(psql, override, callback) { } } + var cteSql = [ + filteredQuery, + basicsQuery, + binsQuery, + nullsQueryTpl({ + _query: _query, + _column: _column + }) + ]; - var histogramSql = [ - "WITH", - [ - filteredQuery, - basicsQuery, - binsQuery, - nullsQueryTpl({ - _query: _query, - _column: _column - }), + if (this._columnType === 'float') { + cteSql.push( infinitiesQueryTpl({ _query: _query, _column: _column @@ -272,8 +265,14 @@ Histogram.prototype.sql = function(psql, override, callback) { _query: _query, _column: _column }) - ].join(',\n'), + ); + } + + var histogramSql = [ + "WITH", + cteSql.join(',\n'), histogramQueryTpl({ + _isFloatColumn: this._columnType === 'float', _query: _query, _column: _column }) @@ -281,7 +280,15 @@ Histogram.prototype.sql = function(psql, override, callback) { debug(histogramSql); - return callback(null, histogramSql); + return histogramSql; +}; + +Histogram.prototype._shouldOverride = function (override) { + return override && _.has(override, 'start') && _.has(override, 'end') && _.has(override, 'bins'); +}; + +Histogram.prototype._shouldOverrideBins = function (override) { + return override && _.has(override, 'bins'); }; Histogram.prototype.format = function(result, override) { diff --git a/lib/cartodb/models/dataview/overviews/histogram.js b/lib/cartodb/models/dataview/overviews/histogram.js index ed88a621..361cc8f2 100644 --- a/lib/cartodb/models/dataview/overviews/histogram.js +++ b/lib/cartodb/models/dataview/overviews/histogram.js @@ -1,14 +1,11 @@ var _ = require('underscore'); var BaseOverviewsDataview = require('./base'); var BaseDataview = require('../histogram'); +var debug = require('debug')('windshaft:dataview:histogram:overview'); var dot = require('dot'); dot.templateSettings.strip = false; -var columnTypeQueryTpl = dot.template( - 'SELECT pg_typeof({{=it.column}})::oid FROM ({{=it.query}}) _cdb_histogram_column_type limit 1' -); - var BIN_MIN_NUMBER = 6; var BIN_MAX_NUMBER = 48; @@ -18,12 +15,12 @@ var filteredQueryTpl = dot.template([ ' FROM ({{=it._query}}) _cdb_filtered_source', ' WHERE', ' {{=it._column}} IS NOT NULL', - ' AND', + ' {{?it._isFloatColumn}}AND', ' {{=it._column}} != \'infinity\'::float', ' AND', ' {{=it._column}} != \'-infinity\'::float', ' AND', - ' {{=it._column}} != \'NaN\'::float', + ' {{=it._column}} != \'NaN\'::float{{?}}', ')' ].join(' \n')); @@ -117,8 +114,8 @@ var histogramQueryTpl = dot.template([ ' (max_val - min_val) / cast(bins_number as float) AS bin_width,', ' bins_number,', ' nulls_count,', - ' infinities_count,', - ' nans_count,', + ' {{?it._isFloatColumn}}infinities_count,', + ' nans_count,{{?}}', ' avg_val,', ' CASE WHEN min_val = max_val', ' THEN 0', @@ -128,8 +125,9 @@ var histogramQueryTpl = dot.template([ ' max({{=it._column}})::numeric AS max,', ' sum({{=it._column}}*_feature_count)/sum(_feature_count)::numeric AS avg,', ' sum(_feature_count) AS freq', - 'FROM filtered_source, basics, nulls, infinities, nans, bins', - 'GROUP BY bin, bins_number, bin_width, nulls_count, infinities_count, nans_count, avg_val', + 'FROM filtered_source, basics, nulls, bins{{?it._isFloatColumn}},infinities, nans{{?}}', + 'GROUP BY bin, bins_number, bin_width, nulls_count, avg_val', + ' {{?it._isFloatColumn}}, infinities_count, nans_count{{?}}', 'ORDER BY bin' ].join('\n')); @@ -149,36 +147,23 @@ Histogram.prototype.constructor = Histogram; module.exports = Histogram; - -var DATE_OIDS = { - 1082: true, - 1114: true, - 1184: true -}; - Histogram.prototype.sql = function(psql, override, callback) { + var self = this; + if (!callback) { callback = override; override = {}; } - var self = this; - - var _column = this.column; - - var columnTypeQuery = columnTypeQueryTpl({ - column: _column, query: this.rewrittenQuery(this.queries.no_filters) - }); if (this._columnType === null) { - psql.query(columnTypeQuery, function(err, result) { + this.getColumnType(psql, this.column, this.queries.no_filters, function (err, type) { // assume numeric, will fail later self._columnType = 'numeric'; - if (!err && !!result.rows[0]) { - var pgType = result.rows[0].pg_typeof; - if (DATE_OIDS.hasOwnProperty(pgType)) { - self._columnType = 'date'; - } + if (!err && !!type) { + self._columnType = Object.keys(type).find(function (key) { + return type[key]; + }); } self.sql(psql, override, callback); }, true); // use read-only transaction @@ -191,16 +176,24 @@ Histogram.prototype.sql = function(psql, override, callback) { return this.defaultSql(psql, override, callback); } + var histogramSql = this._buildQuery(override); + + return callback(null, histogramSql); +}; + +Histogram.prototype._buildQuery = function (override) { + var filteredQuery, basicsQuery, binsQuery; + var _column = this.column; var _query = this.rewrittenQuery(this.query); - var filteredQuery, basicsQuery, binsQuery; - filteredQuery = filteredQueryTpl({ + _isFloatColumn: this._columnType === 'float', _query: _query, _column: _column }); - if (override && _.has(override, 'start') && _.has(override, 'end') && _.has(override, 'bins')) { + if (this._shouldOverride(override)) { + debug('overriding with %j', override); basicsQuery = overrideBasicsQueryTpl({ _query: _query, _column: _column, @@ -219,7 +212,7 @@ Histogram.prototype.sql = function(psql, override, callback) { _column: _column }); - if (override && _.has(override, 'bins')) { + if (this._shouldOverrideBins(override)) { binsQuery = [ overrideBinsQueryTpl({ _bins: override.bins @@ -240,17 +233,18 @@ Histogram.prototype.sql = function(psql, override, callback) { } } + var cteSql = [ + filteredQuery, + basicsQuery, + binsQuery, + nullsQueryTpl({ + _query: _query, + _column: _column + }) + ]; - var histogramSql = [ - "WITH", - [ - filteredQuery, - basicsQuery, - binsQuery, - nullsQueryTpl({ - _query: _query, - _column: _column - }), + if (this._columnType === 'float') { + cteSql.push( infinitiesQueryTpl({ _query: _query, _column: _column @@ -259,12 +253,30 @@ Histogram.prototype.sql = function(psql, override, callback) { _query: _query, _column: _column }) - ].join(',\n'), + ); + } + + var histogramSql = [ + "WITH", + cteSql.join(',\n'), histogramQueryTpl({ + _isFloatColumn: this._columnType === 'float', _query: _query, _column: _column }) ].join('\n'); - return callback(null, histogramSql); + debug(histogramSql); + + return histogramSql; }; + +Histogram.prototype._shouldOverride = function (override) { + return override && _.has(override, 'start') && _.has(override, 'end') && _.has(override, 'bins'); +}; + +Histogram.prototype._shouldOverrideBins = function (override) { + return override && _.has(override, 'bins'); +}; + + From e44d418db39dec200ffcdfa8ca6d03f0d717a40a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 21 Jun 2017 18:44:21 +0200 Subject: [PATCH 30/34] Going red: Add test to check that aggregation dataview supports numeric special value --- test/acceptance/dataviews/aggregation.js | 46 ++++++++++++++++++++++++ test/support/test-client.js | 2 +- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/test/acceptance/dataviews/aggregation.js b/test/acceptance/dataviews/aggregation.js index 5d9dc499..86eb0613 100644 --- a/test/acceptance/dataviews/aggregation.js +++ b/test/acceptance/dataviews/aggregation.js @@ -190,6 +190,17 @@ describe('aggregation-dataview: special float values', function() { aggregation: 'avg', aggregationColumn: 'val' } + }, + sum_aggregation_numeric: { + source: { + id: 'a1' + }, + type: 'aggregation', + options: { + column: 'cat', + aggregation: 'sum', + aggregationColumn: 'val' + } } }, [ @@ -213,10 +224,31 @@ describe('aggregation-dataview: special float values', function() { 'FROM generate_series(1, 1000) x' ].join('\n') } + }, { + "id": "a1", + "type": "source", + "params": { + "query": [ + 'SELECT', + ' null::geometry the_geom_webmercator,', + ' CASE', + ' WHEN x % 3 = 0 THEN \'NaN\'::numeric', + ' WHEN x % 3 = 1 THEN x', + ' ELSE x', + ' END AS val,', + ' CASE', + ' WHEN x % 2 = 0 THEN \'category_1\'', + ' ELSE \'category_2\'', + ' END AS cat', + 'FROM generate_series(1, 1000) x' + ].join('\n') + } } ] ); + // Source a0 + // ----------------------------------------------- // the_geom_webmercator | val | cat // ----------------------+-----------+------------ // | -Infinity | category_2 @@ -248,5 +280,19 @@ describe('aggregation-dataview: special float values', function() { done(); }); }); + + it('should handle special numeric values using filter: ' + JSON.stringify(filter), function(done) { + this.testClient = new TestClient(mapConfig, 1234); + this.testClient.getDataview('sum_aggregation_numeric', { own_filter: 0 }, function(err, dataview) { + assert.ifError(err); + assert.ok(dataview.nans === 333); + assert.ok(dataview.categories.length === 2); + dataview.categories.forEach(function (category) { + assert.ok(category.value !== null); + }); + done(); + }); + }); + }); }); diff --git a/test/support/test-client.js b/test/support/test-client.js index 04980a5a..659d6686 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -435,7 +435,7 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { } params.placeholders = params.placeholders || {}; - + assert.response(server, { url: urlNamed + '?' + qs.stringify({ api_key: self.apiKey }), From 5555b8ad8ec76025cb1770336271e1802b526477 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 21 Jun 2017 18:59:36 +0200 Subject: [PATCH 31/34] Going green: support numeric NaN values for dataviews --- lib/cartodb/models/dataview/base.js | 3 ++- test/acceptance/dataviews/aggregation.js | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/models/dataview/base.js b/lib/cartodb/models/dataview/base.js index 2cd24261..f8eb1027 100644 --- a/lib/cartodb/models/dataview/base.js +++ b/lib/cartodb/models/dataview/base.js @@ -30,7 +30,8 @@ BaseDataview.prototype.search = function(psql, userQuery, callback) { var FLOAT_OIDS = { 700: true, - 701: true + 701: true, + 1700: true }; var DATE_OIDS = { diff --git a/test/acceptance/dataviews/aggregation.js b/test/acceptance/dataviews/aggregation.js index 86eb0613..697004e8 100644 --- a/test/acceptance/dataviews/aggregation.js +++ b/test/acceptance/dataviews/aggregation.js @@ -293,6 +293,5 @@ describe('aggregation-dataview: special float values', function() { done(); }); }); - }); }); From 32274e66fdb7aa3668af59ec2de2d7eb23d3bbf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 23 Jun 2017 12:24:22 +0200 Subject: [PATCH 32/34] Dataview formula: count infinities and nans as we do with nulls --- lib/cartodb/models/dataview/formula.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index 2272089c..50869762 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -14,7 +14,7 @@ var formulaQueryTpl = dot.template([ ' ,(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls', ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count{{?}}', 'FROM ({{=it._query}}) _cdb_formula', - '{{?it._isFloatColumn}}WHERE', + '{{?it._isFloatColumn && it._operation !== \'count\'}}WHERE', ' {{=it._column}} != \'infinity\'::float', 'AND', ' {{=it._column}} != \'-infinity\'::float', From 166e29e8ce9d95aac099df5282ff750d6314e077 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Fri, 23 Jun 2017 16:53:16 +0200 Subject: [PATCH 33/34] Forward queries parameter from overview dataviews to base dataviews --- lib/cartodb/models/dataview/overviews/aggregation.js | 2 +- lib/cartodb/models/dataview/overviews/base.js | 5 +++-- lib/cartodb/models/dataview/overviews/formula.js | 4 ++-- lib/cartodb/models/dataview/overviews/histogram.js | 2 +- lib/cartodb/models/dataview/overviews/list.js | 4 ++-- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/cartodb/models/dataview/overviews/aggregation.js b/lib/cartodb/models/dataview/overviews/aggregation.js index 730ba822..5df092f4 100644 --- a/lib/cartodb/models/dataview/overviews/aggregation.js +++ b/lib/cartodb/models/dataview/overviews/aggregation.js @@ -90,7 +90,7 @@ var aggregationQueryTpl = dot.template([ var CATEGORIES_LIMIT = 6; function Aggregation(query, options, queryRewriter, queryRewriteData, params, queries) { - BaseOverviewsDataview.call(this, query, options, BaseDataview, queryRewriter, queryRewriteData, params); + BaseOverviewsDataview.call(this, query, options, BaseDataview, queryRewriter, queryRewriteData, params, queries); this.query = query; this.queries = queries; diff --git a/lib/cartodb/models/dataview/overviews/base.js b/lib/cartodb/models/dataview/overviews/base.js index 1425e2d1..38b2c119 100644 --- a/lib/cartodb/models/dataview/overviews/base.js +++ b/lib/cartodb/models/dataview/overviews/base.js @@ -1,14 +1,15 @@ var _ = require('underscore'); var BaseDataview = require('../base'); -function BaseOverviewsDataview(query, queryOptions, BaseDataview, queryRewriter, queryRewriteData, options) { +function BaseOverviewsDataview(query, queryOptions, BaseDataview, queryRewriter, queryRewriteData, options, queries) { this.BaseDataview = BaseDataview; this.query = query; this.queryOptions = queryOptions; this.queryRewriter = queryRewriter; this.queryRewriteData = queryRewriteData; this.options = options; - this.baseDataview = new this.BaseDataview(this.query, this.queryOptions); + this.queries = queries; + this.baseDataview = new this.BaseDataview(this.query, this.queryOptions, this.queries); } module.exports = BaseOverviewsDataview; diff --git a/lib/cartodb/models/dataview/overviews/formula.js b/lib/cartodb/models/dataview/overviews/formula.js index 533dd921..a30bc783 100644 --- a/lib/cartodb/models/dataview/overviews/formula.js +++ b/lib/cartodb/models/dataview/overviews/formula.js @@ -50,8 +50,8 @@ var formulaQueryTpls = { ].join('\n')), }; -function Formula(query, options, queryRewriter, queryRewriteData, params) { - BaseOverviewsDataview.call(this, query, options, BaseDataview, queryRewriter, queryRewriteData, params); +function Formula(query, options, queryRewriter, queryRewriteData, params, queries) { + BaseOverviewsDataview.call(this, query, options, BaseDataview, queryRewriter, queryRewriteData, params, queries); this.column = options.column || '1'; this.operation = options.operation; this._isFloatColumn = null; diff --git a/lib/cartodb/models/dataview/overviews/histogram.js b/lib/cartodb/models/dataview/overviews/histogram.js index 361cc8f2..6674f6a0 100644 --- a/lib/cartodb/models/dataview/overviews/histogram.js +++ b/lib/cartodb/models/dataview/overviews/histogram.js @@ -132,7 +132,7 @@ var histogramQueryTpl = dot.template([ ].join('\n')); function Histogram(query, options, queryRewriter, queryRewriteData, params, queries) { - BaseOverviewsDataview.call(this, query, options, BaseDataview, queryRewriter, queryRewriteData, params); + BaseOverviewsDataview.call(this, query, options, BaseDataview, queryRewriter, queryRewriteData, params, queries); this.query = query; this.queries = queries; diff --git a/lib/cartodb/models/dataview/overviews/list.js b/lib/cartodb/models/dataview/overviews/list.js index 7e3b3161..6ec731f4 100644 --- a/lib/cartodb/models/dataview/overviews/list.js +++ b/lib/cartodb/models/dataview/overviews/list.js @@ -1,8 +1,8 @@ var BaseOverviewsDataview = require('./base'); var BaseDataview = require('../list'); -function List(query, options, queryRewriter, queryRewriteData, params) { - BaseOverviewsDataview.call(this, query, options, BaseDataview, queryRewriter, queryRewriteData, params); +function List(query, options, queryRewriter, queryRewriteData, params, queries) { + BaseOverviewsDataview.call(this, query, options, BaseDataview, queryRewriter, queryRewriteData, params, queries); } List.prototype = Object.create(BaseOverviewsDataview.prototype); From b2f3735e95d6c7820d7e1a18b494c1a997711221 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Fri, 23 Jun 2017 18:59:51 +0200 Subject: [PATCH 34/34] The formula widget wasn't using the no_filters query for checking column types --- lib/cartodb/models/dataview/formula.js | 5 +++-- lib/cartodb/models/dataview/overviews/formula.js | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index 50869762..7ec356b7 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -41,7 +41,7 @@ var TYPE = 'formula'; } } */ -function Formula(query, options) { +function Formula(query, options, queries) { if (!_.isString(options.operation)) { throw new Error('Formula expects `operation` in widget options'); } @@ -57,6 +57,7 @@ function Formula(query, options) { BaseWidget.apply(this); this.query = query; + this.queries = queries; this.column = options.column || '1'; this.operation = options.operation; this._isFloatColumn = null; @@ -77,7 +78,7 @@ Formula.prototype.sql = function(psql, override, callback) { if (this._isFloatColumn === null) { this._isFloatColumn = false; - this.getColumnType(psql, this.column, this.query, function (err, type) { + this.getColumnType(psql, this.column, this.queries.no_filters, function (err, type) { if (!err && !!type) { self._isFloatColumn = type.float; } diff --git a/lib/cartodb/models/dataview/overviews/formula.js b/lib/cartodb/models/dataview/overviews/formula.js index a30bc783..64d612c9 100644 --- a/lib/cartodb/models/dataview/overviews/formula.js +++ b/lib/cartodb/models/dataview/overviews/formula.js @@ -55,6 +55,7 @@ function Formula(query, options, queryRewriter, queryRewriteData, params, querie this.column = options.column || '1'; this.operation = options.operation; this._isFloatColumn = null; + this.queries = queries; } Formula.prototype = Object.create(BaseOverviewsDataview.prototype); @@ -70,7 +71,7 @@ Formula.prototype.sql = function (psql, override, callback) { // supported formula for use with overviews if (this._isFloatColumn === null) { this._isFloatColumn = false; - this.getColumnType(psql, this.column, this.query, function (err, type) { + this.getColumnType(psql, this.column, this.queries.no_filters, function (err, type) { if (!err && !!type) { self._isFloatColumn = type.float; }