From debb174af457edc08cfa9b4cff665dcd20a8c305 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Mon, 29 Jan 2018 15:44:24 +0100 Subject: [PATCH 1/2] Add test for aggregation without the_geom Only the_geom_webmercator is required for aggregation See #841 --- test/acceptance/aggregation.js | 37 ++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js index 4978b920..1c76165d 100644 --- a/test/acceptance/aggregation.js +++ b/test/acceptance/aggregation.js @@ -92,6 +92,14 @@ describe('aggregation', function () { } `; + const POINTS_SQL_ONLY_WEBMERCATOR = ` + select + x + 4 as cartodb_id, + st_transform(st_setsrid(st_makepoint(x*10, x*10), 4326), 3857) as the_geom_webmercator, + x as value + from generate_series(-3, 3) x + `; + function createVectorMapConfig (layers = [ { type: 'cartodb', @@ -1401,6 +1409,35 @@ describe('aggregation', function () { }); }); }); + + it('should only require the_geom_webmercator for aggregation', function (done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_ONLY_WEBMERCATOR, + aggregation: { + threshold: 1 + } + } + } + ]); + 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 7641542e67f58ec745410ac998d8b55c77ca9d0c Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Mon, 29 Jan 2018 15:48:35 +0100 Subject: [PATCH 2/2] Check the type of the_geom_webmercator for aggregation Fixes #841 --- lib/cartodb/models/aggregation/aggregation-mapconfig.js | 4 ++++ lib/cartodb/models/aggregation/aggregation-query.js | 1 + .../models/mapconfig/adapter/aggregation-mapconfig-adapter.js | 3 ++- lib/cartodb/utils/query-utils.js | 4 ++-- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/models/aggregation/aggregation-mapconfig.js b/lib/cartodb/models/aggregation/aggregation-mapconfig.js index 99ed32c1..50e4dcfe 100644 --- a/lib/cartodb/models/aggregation/aggregation-mapconfig.js +++ b/lib/cartodb/models/aggregation/aggregation-mapconfig.js @@ -47,6 +47,10 @@ module.exports = class AggregationMapConfig extends MapConfig { return AggregationMapConfig.SUPPORTED_GEOMETRY_TYPES.includes(geometryType); } + static getAggregationGeometryColumn() { + return aggregationQuery.GEOMETRY_COLUMN; + } + constructor (user, config, connection, datasource) { super(config, datasource); diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js index d50bb458..7bb133d9 100644 --- a/lib/cartodb/models/aggregation/aggregation-query.js +++ b/lib/cartodb/models/aggregation/aggregation-query.js @@ -250,3 +250,4 @@ const aggregationQueryTemplates = { }; module.exports.SUPPORTED_PLACEMENTS = Object.keys(aggregationQueryTemplates); +module.exports.GEOMETRY_COLUMN = 'the_geom_webmercator'; diff --git a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js index 6aa0eaf2..67c5eac4 100644 --- a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js @@ -122,7 +122,8 @@ module.exports = class AggregationMapConfigAdapter { } const aggregationMetadata = queryUtils.getAggregationMetadata({ - query: layer.options.sql_raw ? layer.options.sql_raw : layer.options.sql + query: layer.options.sql_raw ? layer.options.sql_raw : layer.options.sql, + geometryColumn: AggregationMapConfig.getAggregationGeometryColumn() }); connection.query(aggregationMetadata, (err, res) => { diff --git a/lib/cartodb/utils/query-utils.js b/lib/cartodb/utils/query-utils.js index c0d0aec2..c1b56276 100644 --- a/lib/cartodb/utils/query-utils.js +++ b/lib/cartodb/utils/query-utils.js @@ -32,8 +32,8 @@ module.exports.getAggregationMetadata = ctx => ` ${getQueryRowEstimation(ctx.query)} ), geometryType AS ( - SELECT ST_GeometryType(the_geom) as geom_type - FROM (${ctx.query}) AS __cdb_query WHERE the_geom IS NOT NULL LIMIT 1 + SELECT ST_GeometryType(${ctx.geometryColumn}) as geom_type + FROM (${ctx.query}) AS __cdb_query WHERE ${ctx.geometryColumn} IS NOT NULL LIMIT 1 ) SELECT rows AS count,