From dab204ea71baee73006774e26e64e9919542776a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 11 Dec 2017 17:32:06 +0100 Subject: [PATCH] Do not aggregate if rows cout is lower than threshold or the layer's sql has geometries distinct of points --- .../adapter/aggregation-mapconfig-adapter.js | 119 +++++++++++++----- lib/cartodb/server.js | 2 +- lib/cartodb/utils/query-utils.js | 20 ++- test/acceptance/aggregation.js | 103 ++++++++++----- 4 files changed, 182 insertions(+), 62 deletions(-) diff --git a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js index eb78ce65..bc401542 100644 --- a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js @@ -1,10 +1,17 @@ const AggregationProxy = require('../../aggregation/aggregation-proxy'); const { MapConfig } = require('windshaft').model; +const queryUtils = require('../../../utils/query-utils'); const MISSING_AGGREGATION_COLUMNS = 'Missing columns in the aggregation. The map-config defines cartocss expressions,'+ ' interactivity fields or attributes that are not present in the aggregation'; +const unsupportedGeometryTypeErrorMessage = ctx => +`Unsupported geometry type (${ctx.geometryType}) for aggregation. Aggregation is available only for points.`; module.exports = class AggregationMapConfigAdapter { + constructor (pgConnection) { + this.pgConnection = pgConnection; + } + getMapConfig (user, requestMapConfig, params, context, callback) { const mapConfig = new MapConfig(requestMapConfig); @@ -20,12 +27,13 @@ module.exports = class AggregationMapConfigAdapter { return callback(error); } - requestMapConfig.layers = this._adaptLayers(mapConfig, requestMapConfig); - context.aggregation = { - layers: this._getAggregationMetadata(mapConfig, requestMapConfig), - }; + this._adaptLayers(user, mapConfig, requestMapConfig, context, (err, requestMapConfig) => { + if (err) { + return callback(err); + } - callback(null, requestMapConfig); + callback(null, requestMapConfig); + }); } _hasMissingColumns (mapConfig) { @@ -101,39 +109,90 @@ module.exports = class AggregationMapConfigAdapter { return aggregation !== undefined && (typeof aggregation === 'object' || typeof aggregation === 'boolean'); } - _adaptLayers (mapConfig, requestMapConfig) { - const isVectorOnlyMapConfig = mapConfig.isVectorOnlyMapConfig(); - return requestMapConfig.layers.map(layer => { - if (isVectorOnlyMapConfig || this._hasLayerAggregation(layer)) { - const aggregation = new AggregationProxy(mapConfig, layer.options.aggregation); - const sqlQueryWrap = layer.options.sql_wrap; - - let aggregationSql = aggregation.sql(layer.options); - - if (sqlQueryWrap) { - layer.options.sql_raw = aggregationSql; - aggregationSql = sqlQueryWrap.replace(/<%=\s*sql\s*%>/g, aggregationSql); - } - - layer.options.sql = aggregationSql; + _adaptLayers (user, mapConfig, requestMapConfig, context, callback) { + this.pgConnection.getConnection(user, (err, connection) => { + if (err) { + return callback(err); } - return layer; + const isVectorOnlyMapConfig = mapConfig.isVectorOnlyMapConfig(); + + const adaptLayerPromises = requestMapConfig.layers.map((layer, index) => { + return new Promise((resolve, reject) => { + if (!isVectorOnlyMapConfig && !this._hasLayerAggregation(layer)) { + return resolve({ layer, index, adapted: false }); + } + + const threshold = layer.options.aggregation && layer.options.aggregation.threshold ? + layer.options.aggregation.threshold : + 1e5; + + const aggregationMetadata = queryUtils.getAggregationMetadata({ query: layer.options.sql }); + + connection.query(aggregationMetadata, (err, res) => { + if (err) { + return resolve({ layer, index, adapted: false }); + } + + const estimatedFeatureCount = res.rows[0].count; + const geometryType = res.rows[0].type; + + if (estimatedFeatureCount < threshold) { + return resolve({ layer, index, adapted: false }); + } + + if (geometryType !== 'ST_Point') { + return reject(new Error(unsupportedGeometryTypeErrorMessage({ geometryType }))); + } + + const aggregation = new AggregationProxy(mapConfig, layer.options.aggregation); + const sqlQueryWrap = layer.options.sql_wrap; + + let aggregationSql = aggregation.sql(layer.options); + + if (sqlQueryWrap) { + layer.options.sql_raw = aggregationSql; + aggregationSql = sqlQueryWrap.replace(/<%=\s*sql\s*%>/g, aggregationSql); + } + + layer.options.sql = aggregationSql; + + return resolve({ layer, index, adapted: true }); + }); + }); + }); + + Promise.all(adaptLayerPromises) + .then(results => { + context.aggregation = { + layers: [] + }; + + results.forEach(({ layer, index, adapted }) => { + if (adapted) { + requestMapConfig.layers[index] = layer; + } + const aggregatedFormats = this._getAggregationMetadata(isVectorOnlyMapConfig, layer, adapted); + context.aggregation.layers.push(aggregatedFormats); + }); + + return requestMapConfig; + }) + .then(requestMapConfig => callback(null, requestMapConfig)) + .catch(err => callback(err)); }); } - _getAggregationMetadata (mapConfig, requestMapConfig) { - if (mapConfig.isVectorOnlyMapConfig()) { - return requestMapConfig.layers.map((/* layer */) => { + _getAggregationMetadata (isVectorOnlyMapConfig, layer, adapted) { + if (adapted) { + if (isVectorOnlyMapConfig) { return { png: false, mvt: true }; - }); + } + + return { png: true, mvt: true }; } - return requestMapConfig.layers.map(layer => { - return this._hasLayerAggregation(layer) ? - { png: true, mvt: true } : - { png: false, mvt: false }; - }); + return { png: false, mvt: false }; } _getAggregationColumns (aggregation) { diff --git a/lib/cartodb/server.js b/lib/cartodb/server.js index b7f0e8a0..46ff7122 100644 --- a/lib/cartodb/server.js +++ b/lib/cartodb/server.js @@ -191,7 +191,7 @@ module.exports = function(serverOptions) { new SqlWrapMapConfigAdapter(), new DataviewsWidgetsAdapter(), new AnalysisMapConfigAdapter(analysisBackend), - new AggregationMapConfigAdapter(), + new AggregationMapConfigAdapter(pgConnection), new MapConfigOverviewsAdapter(overviewsMetadataApi, filterStatsApi), new TurboCartoAdapter() ); diff --git a/lib/cartodb/utils/query-utils.js b/lib/cartodb/utils/query-utils.js index 47d730f4..873151d8 100644 --- a/lib/cartodb/utils/query-utils.js +++ b/lib/cartodb/utils/query-utils.js @@ -21,6 +21,22 @@ module.exports.extractTableNames = function extractTableNames(query) { ].join(''); }; -module.exports.getQueryRowCount = function getQueryRowEstimation(query) { +function getQueryRowEstimation(query) { return 'select CDB_EstimateRowCount(\'' + query + '\') as rows'; -}; +} +module.exports.getQueryRowCount = getQueryRowEstimation; + +module.exports.getAggregationMetadata = ctx => ` + WITH + rowEstimation AS ( + ${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 + rows AS count, + geom_type AS type + FROM rowEstimation, geometryType; +`; diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js index cf6b3444..2576a09c 100644 --- a/test/acceptance/aggregation.js +++ b/test/acceptance/aggregation.js @@ -38,14 +38,18 @@ describe('aggregation', function () { `; - const POINTS_SQL_NO_THE_GEOM = ` + const POLYGONS_SQL_1 = ` select - st_transform(st_setsrid(st_makepoint(x*10, x*10*(-1)), 4326), 3857) as the_geom_webmercator, - x as value, - x*x as sqrt_value + st_buffer(st_setsrid(st_makepoint(x*10, x*10), 4326)::geography, 100000)::geometry as the_geom, + st_transform( + st_buffer(st_setsrid(st_makepoint(x*10, x*10), 4326)::geography, 100000)::geometry, + 3857 + ) as the_geom_webmercator, + x as value from generate_series(-3, 3) x `; + function createVectorMapConfig (layers = [ { type: 'cartodb', @@ -87,7 +91,26 @@ describe('aggregation', function () { }); it('should return a layergroup indicating the mapconfig was aggregated', function (done) { - this.mapConfig = createVectorMapConfig(); + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + threshold: 1 + } + } + }, + { + type: 'cartodb', + options: { + sql: POINTS_SQL_2, + aggregation: { + threshold: 1 + } + } + } + ]); this.testClient = new TestClient(this.mapConfig); this.testClient.getLayergroup((err, body) => { @@ -117,7 +140,8 @@ describe('aggregation', function () { aggregate_function: 'sum', aggregated_column: 'value' } - } + }, + threshold: 1 }, cartocss: '#layer { marker-width: [value]; }', cartocss_version: '2.3.0' @@ -250,18 +274,29 @@ describe('aggregation', function () { }); }); - it('without the_geom defined in the sql should return a layergroup ', function (done) { - this.mapConfig = createVectorMapConfig([ + it('when the layer\'s row count is lower than threshold should skip aggregation', function (done) { + const mapConfig = createVectorMapConfig([ { type: 'cartodb', options: { - sql: POINTS_SQL_NO_THE_GEOM, + sql: POINTS_SQL_1, + aggregation: { + columns: { + total: { + aggregate_function: 'sum', + aggregated_column: 'value' + } + }, + threshold: 1001 + } } } ]); - this.testClient = new TestClient(this.mapConfig); - this.testClient.getLayergroup((err, body) => { + this.testClient = new TestClient(mapConfig); + const options = {}; + + this.testClient.getLayergroup(options, (err, body) => { if (err) { return done(err); } @@ -269,44 +304,54 @@ describe('aggregation', function () { 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)); + body.metadata.layers.forEach(layer =>{ + assert.deepEqual(layer.meta.aggregation, { png: false, mvt: false }); + }); done(); }); }); - it('without the_geom defined in the sql should get a vector tile', function (done) { - this.mapConfig = createVectorMapConfig([ + it('when the layer\'s geometry type is not point should responds with error', function (done) { + const mapConfig = createVectorMapConfig([ { type: 'cartodb', options: { - sql: POINTS_SQL_NO_THE_GEOM, + sql: POLYGONS_SQL_1, + aggregation: { + threshold: 1 + } } } ]); - this.testClient = new TestClient(this.mapConfig); - this.testClient.getTile(0, 0, 0, { format: 'mvt' }, (err, res, tile) => { + this.testClient = new TestClient(mapConfig); + const options = { + response: { + status: 400 + } + }; + + this.testClient.getLayergroup(options, (err, body) => { if (err) { return done(err); } - assert.equal(tile.tileSize, 4096); - assert.equal(tile.z, 0); - assert.equal(tile.x, 0); - assert.equal(tile.y, 0); - - const layer0 = JSON.parse(tile.toGeoJSONSync(0)); - - assert.equal(layer0.name, 'layer0'); - assert.equal(layer0.features[0].type, 'Feature'); - assert.equal(layer0.features[0].geometry.type, 'Point'); + assert.deepEqual(body, { + errors: [ + 'Unsupported geometry type (ST_Polygon) for aggregation.' + + ' Aggregation is available only for points.' + ], + errors_with_context:[{ + type: 'unknown', + message: 'Unsupported geometry type (ST_Polygon) for aggregation.' + + ' Aggregation is available only for points.' + }] + }); done(); }); }); - }); }); });