diff --git a/NEWS.md b/NEWS.md index aab947e4..b3a26e19 100644 --- a/NEWS.md +++ b/NEWS.md @@ -5,6 +5,7 @@ Released 2019-mm-dd Announcements: - Experimental support for listing features in a grid when the map uses the dynamic agregation. +- Numeric histogram performance improvement (#1080) - Update deps: - windshaft@4.13.3: Upgrade grainstore to version 1.11.0, do not hang when child process is not able to generate a Mapnik XML diff --git a/lib/cartodb/models/dataview/histograms/numeric-histogram.js b/lib/cartodb/models/dataview/histograms/numeric-histogram.js index ddf3577f..492316ba 100644 --- a/lib/cartodb/models/dataview/histograms/numeric-histogram.js +++ b/lib/cartodb/models/dataview/histograms/numeric-histogram.js @@ -4,50 +4,37 @@ const BaseHistogram = require('./base-histogram'); const debug = require('debug')('windshaft:dataview:numeric-histogram'); const utils = require('../../../utils/query-utils'); -/** Query to get min and max values from the query */ +/** Query to get min, max, count and (if necessary) bin number of the query */ const irqQueryTpl = ctx => ` - __cdb_filtered_source AS ( - SELECT * - FROM (${ctx.query}) __cdb_filtered_source_query - WHERE ${utils.handleFloatColumn(ctx)} IS NOT NULL - ), - __cdb_basics AS ( + __cdb_basics AS ( + SELECT + *, + CASE + WHEN __cdb_total_rows = 0 OR __cdb_iqr = 0 THEN 1 + ELSE GREATEST( + LEAST( + ${ctx.minBins}, + __cdb_total_rows::int), + LEAST( + ${ctx.maxBins}, + ((__cdb_max_val - __cdb_min_val) / (2 * __cdb_iqr * power(__cdb_total_rows, 1/3)))::int) + ) + END AS __cdb_bins_number + FROM + ( SELECT max(${ctx.column}) AS __cdb_max_val, min(${ctx.column}) AS __cdb_min_val, - count(1) AS __cdb_total_rows - FROM __cdb_filtered_source - ) -`; - -/* Query to calculate the number of bins (needs irqQueryTpl before it*/ -const binsQueryTpl = ctx => ` - __cdb_iqrange AS ( - SELECT max(quartile_max) - min(quartile_max) AS __cdb_iqr - FROM ( - SELECT quartile, max(_cdb_iqr_column) AS quartile_max from ( - SELECT ${ctx.column} AS _cdb_iqr_column, ntile(4) over (order by ${ctx.column} - ) AS quartile - FROM __cdb_filtered_source) _cdb_quartiles - WHERE quartile = 1 or quartile = 3 - GROUP BY 1 - ) __cdb_iqr - ), - __cdb_bins AS ( - SELECT - CASE WHEN __cdb_total_rows = 0 OR __cdb_iqr = 0 - THEN 1 - ELSE GREATEST( - LEAST(${ctx.minBins}, CAST(__cdb_total_rows AS INT)), - LEAST( - CAST(((__cdb_max_val - __cdb_min_val) / (2 * __cdb_iqr * power(__cdb_total_rows, 1/3))) AS INT), - ${ctx.maxBins} - ) - ) - END AS __cdb_bins_number - FROM __cdb_basics, __cdb_iqrange, __cdb_filtered_source - LIMIT 1 - ) + count(1) AS __cdb_total_rows, + ${ctx.irq ? ctx.irq : `0`} AS __cdb_iqr + FROM + ( + SELECT * + FROM (${ctx.query}) __cdb_filtered_source_query + WHERE ${utils.handleFloatColumn(ctx)} IS NOT NULL + ) __cdb_filtered_source + ) __cdb_basics_2 +) `; const BIN_MIN_NUMBER = 6; @@ -117,10 +104,12 @@ module.exports = class NumericHistogram extends BaseHistogram { } if (ctx.bins <= 0) { - ctx.bins = `__cdb_bins.__cdb_bins_number`; - extra_groupby += `, __cdb_bins.__cdb_bins_number`; - extra_tables += `, __cdb_bins`; - extra_queries = `WITH ${irqQueryTpl(ctx)}, ${binsQueryTpl(ctx)}`; + ctx.bins = `__cdb_basics.__cdb_bins_number`; + ctx.irq = `percentile_disc(0.75) within group (order by ${ctx.column}) + - percentile_disc(0.25) within group (order by ${ctx.column})`; + extra_groupby += `, __cdb_basics.__cdb_bins_number`; + extra_tables = `, __cdb_basics`; + extra_queries = `WITH ${irqQueryTpl(ctx)}`; } return ` diff --git a/test/acceptance/dataviews/histogram.js b/test/acceptance/dataviews/histogram.js index aa89d53e..f517a59a 100644 --- a/test/acceptance/dataviews/histogram.js +++ b/test/acceptance/dataviews/histogram.js @@ -90,6 +90,26 @@ describe('histogram-dataview', function() { }); }); + it('should work with min >= start and max <= end, autodetect bins', function(done) { + var params = { + start: 50, + end: 500 + }; + + this.testClient = new TestClient(mapConfig, 1234); + this.testClient.getDataview('pop_max_histogram', params, function(err, dataview) { + assert.ok(!err, err); + + assert.ok(6 === dataview.bins_count, 'Unexpected bin count: ' + dataview.bins_count); + assert.ok(6 === dataview.bins.length, 'Unexpected number of bins: ' + dataview.bins.length); + dataview.bins.forEach(function(bin) { + assert.ok(bin.min >= params.start, 'bin min < start: ' + JSON.stringify(bin)); + assert.ok(bin.max <= params.end, 'bin max > end: ' + JSON.stringify(bin)); + }); + done(); + }); + }); + it('should get bin_width right when max > min in filter', function(done) { var params = { bins: 10,