From d1fcd797a320874a826b819bb88e3cbcb276190b Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 31 Jan 2018 17:46:13 +0100 Subject: [PATCH 1/5] Limit the minimum size of the the aggregation grid Fixes #860 --- lib/cartodb/models/aggregation/aggregation-query.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js index 7bb133d9..f85a2bc6 100644 --- a/lib/cartodb/models/aggregation/aggregation-query.js +++ b/lib/cartodb/models/aggregation/aggregation-query.js @@ -131,7 +131,12 @@ const dimensionDefs = ctx => { // This is equivalent to `${256/ctx.res}*CDB_XYZ_Resolution(CDB_ZoomFromScale(!scale_denominator!))` // This is defined by the ctx.res parameter, which is the number of grid cells per tile linear dimension // (i.e. each tile is divided into ctx.res*ctx.res cells). -const gridResolution = ctx => `(${256*0.00028/ctx.res}*!scale_denominator!)::double precision`; +const gridResolution = ctx => { + const factor = 256*0.00028/ctx.res; + const minimum_resolution = 2*Math.PI*6378137/Math.pow(2,38); + let minimum_scale_denominator = minimum_resolution/factor; + return `(${factor}*GREATEST(!scale_denominator!,${minimum_scale_denominator}))::double precision`; +}; // Notes: // * We need to filter spatially using !bbox! to make the queries efficient because From 7db0744f677545bed079b517f85762e9b5868bca Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 31 Jan 2018 17:54:40 +0100 Subject: [PATCH 2/5] Simplify expression --- lib/cartodb/models/aggregation/aggregation-query.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js index f85a2bc6..53960c1a 100644 --- a/lib/cartodb/models/aggregation/aggregation-query.js +++ b/lib/cartodb/models/aggregation/aggregation-query.js @@ -132,10 +132,8 @@ const dimensionDefs = ctx => { // This is defined by the ctx.res parameter, which is the number of grid cells per tile linear dimension // (i.e. each tile is divided into ctx.res*ctx.res cells). const gridResolution = ctx => { - const factor = 256*0.00028/ctx.res; const minimum_resolution = 2*Math.PI*6378137/Math.pow(2,38); - let minimum_scale_denominator = minimum_resolution/factor; - return `(${factor}*GREATEST(!scale_denominator!,${minimum_scale_denominator}))::double precision`; + return `GREATEST(${256*0.00028/ctx.res}*!scale_denominator!, ${minimum_resolution})::double precision`; }; // Notes: From d88fbbaa87e120ce3c50061ed6db9e0cadc89b0d Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 31 Jan 2018 18:55:28 +0100 Subject: [PATCH 3/5] Use camelCase --- lib/cartodb/models/aggregation/aggregation-query.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js index 53960c1a..e3ad24e3 100644 --- a/lib/cartodb/models/aggregation/aggregation-query.js +++ b/lib/cartodb/models/aggregation/aggregation-query.js @@ -132,8 +132,8 @@ const dimensionDefs = ctx => { // This is defined by the ctx.res parameter, which is the number of grid cells per tile linear dimension // (i.e. each tile is divided into ctx.res*ctx.res cells). const gridResolution = ctx => { - const minimum_resolution = 2*Math.PI*6378137/Math.pow(2,38); - return `GREATEST(${256*0.00028/ctx.res}*!scale_denominator!, ${minimum_resolution})::double precision`; + const minimumResolution = 2*Math.PI*6378137/Math.pow(2,38); + return `GREATEST(${256*0.00028/ctx.res}*!scale_denominator!, ${minimumResolution})::double precision`; }; // Notes: From 80604b739a287b171e3c43a86c620ad129986e8d Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 31 Jan 2018 18:56:24 +0100 Subject: [PATCH 4/5] Add test for aggregation with attributes This reveals #860 --- test/acceptance/aggregation.js | 37 ++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js index 1c76165d..d8870c8a 100644 --- a/test/acceptance/aggregation.js +++ b/test/acceptance/aggregation.js @@ -1438,6 +1438,43 @@ describe('aggregation', function () { done(); }); }); + + it('aggregation should work with attributes', function (done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + cartocss: '#layer { marker-width: 7; }', + cartocss_version: '2.3.0', + aggregation: { + threshold: 1 + }, + attributes: { + id: 'cartodb_id', + columns: [ + 'value' + ] + } + } + } + ]); + this.testClient = new TestClient(this.mapConfig); + + this.testClient.getLayergroup((err, body) => { + if (err) { + return done(err); + } + + assert.equal(typeof body.metadata, 'object'); + assert.ok(Array.isArray(body.metadata.layers)); + + body.metadata.layers.forEach(layer => assert.ok(layer.meta.aggregation.mvt)); + body.metadata.layers.forEach(layer => assert.ok(layer.meta.aggregation.png)); + + done(); + }); + }); }); }); }); From 2e7784ddf2d8f8829b5bccea0e26ac5d6d1708be Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 1 Feb 2018 10:26:52 +0100 Subject: [PATCH 5/5] Add comment to clafify aggregation resolution limit --- lib/cartodb/models/aggregation/aggregation-query.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js index e3ad24e3..16e897ab 100644 --- a/lib/cartodb/models/aggregation/aggregation-query.js +++ b/lib/cartodb/models/aggregation/aggregation-query.js @@ -131,6 +131,8 @@ const dimensionDefs = ctx => { // This is equivalent to `${256/ctx.res}*CDB_XYZ_Resolution(CDB_ZoomFromScale(!scale_denominator!))` // This is defined by the ctx.res parameter, which is the number of grid cells per tile linear dimension // (i.e. each tile is divided into ctx.res*ctx.res cells). +// We limit the the minimum resolution to avoid division by zero problems. The limit used is +// the pixel size of zoom level 30 (i.e. 1/2*(30+8) of the full earth web-mercator extent), which is about 0.15 mm. const gridResolution = ctx => { const minimumResolution = 2*Math.PI*6378137/Math.pow(2,38); return `GREATEST(${256*0.00028/ctx.res}*!scale_denominator!, ${minimumResolution})::double precision`;