From 6241b23d4f5ef9c99aa774b6ccb60a513eafb6d7 Mon Sep 17 00:00:00 2001 From: Raul Marin Date: Mon, 4 Mar 2019 14:09:30 +0100 Subject: [PATCH 1/4] Histogram: Speed up IRQ calculation --- .../dataview/histograms/numeric-histogram.js | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/lib/cartodb/models/dataview/histograms/numeric-histogram.js b/lib/cartodb/models/dataview/histograms/numeric-histogram.js index ddf3577f..51980b84 100644 --- a/lib/cartodb/models/dataview/histograms/numeric-histogram.js +++ b/lib/cartodb/models/dataview/histograms/numeric-histogram.js @@ -15,24 +15,15 @@ const irqQueryTpl = ctx => ` SELECT max(${ctx.column}) AS __cdb_max_val, min(${ctx.column}) AS __cdb_min_val, - count(1) AS __cdb_total_rows + count(1) AS __cdb_total_rows, + ${ctx.irq ? ctx.irq : `0`} AS __cdb_iqr FROM __cdb_filtered_source ) `; -/* Query to calculate the number of bins (needs irqQueryTpl before it*/ +/* Query to calculate the number of bins (needs irqQueryTpl before it. + * It uses the Freedman–Diaconis rule to calculate the witdh of the bins */ 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 @@ -45,7 +36,7 @@ const binsQueryTpl = ctx => ` ) ) END AS __cdb_bins_number - FROM __cdb_basics, __cdb_iqrange, __cdb_filtered_source + FROM __cdb_basics, __cdb_filtered_source LIMIT 1 ) `; @@ -118,6 +109,7 @@ module.exports = class NumericHistogram extends BaseHistogram { if (ctx.bins <= 0) { ctx.bins = `__cdb_bins.__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_bins.__cdb_bins_number`; extra_tables += `, __cdb_bins`; extra_queries = `WITH ${irqQueryTpl(ctx)}, ${binsQueryTpl(ctx)}`; From 730076469e91f090127ce4d18869e15ef0f56a13 Mon Sep 17 00:00:00 2001 From: Raul Marin Date: Mon, 4 Mar 2019 15:52:27 +0100 Subject: [PATCH 2/4] Numeric histogram: Simplify bin calculation --- .../dataview/histograms/numeric-histogram.js | 64 +++++++++---------- 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/lib/cartodb/models/dataview/histograms/numeric-histogram.js b/lib/cartodb/models/dataview/histograms/numeric-histogram.js index 51980b84..26623b01 100644 --- a/lib/cartodb/models/dataview/histograms/numeric-histogram.js +++ b/lib/cartodb/models/dataview/histograms/numeric-histogram.js @@ -4,41 +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, ${ctx.irq ? ctx.irq : `0`} AS __cdb_iqr - FROM __cdb_filtered_source - ) -`; - -/* Query to calculate the number of bins (needs irqQueryTpl before it. - * It uses the Freedman–Diaconis rule to calculate the witdh of the bins */ -const binsQueryTpl = ctx => ` - __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_filtered_source - LIMIT 1 - ) + 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; @@ -108,11 +104,11 @@ module.exports = class NumericHistogram extends BaseHistogram { } if (ctx.bins <= 0) { - ctx.bins = `__cdb_bins.__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_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_queries = `WITH ${irqQueryTpl(ctx)}`; } return ` From 8db090ae9c35f9b3064c55846ca2b2440bbe0f89 Mon Sep 17 00:00:00 2001 From: Raul Marin Date: Mon, 4 Mar 2019 16:53:55 +0100 Subject: [PATCH 3/4] Numeric histogram: Test when start and end are provided but not bins --- .../dataview/histograms/numeric-histogram.js | 1 + test/acceptance/dataviews/histogram.js | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/lib/cartodb/models/dataview/histograms/numeric-histogram.js b/lib/cartodb/models/dataview/histograms/numeric-histogram.js index 26623b01..492316ba 100644 --- a/lib/cartodb/models/dataview/histograms/numeric-histogram.js +++ b/lib/cartodb/models/dataview/histograms/numeric-histogram.js @@ -108,6 +108,7 @@ module.exports = class NumericHistogram extends BaseHistogram { 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)}`; } 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, From 59107496b4faa2f54da96f6f9b423246c2379f01 Mon Sep 17 00:00:00 2001 From: Raul Marin Date: Thu, 14 Mar 2019 15:17:42 +0100 Subject: [PATCH 4/4] Update NEWS --- NEWS.md | 1 + 1 file changed, 1 insertion(+) 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