From 418c8691d1b80e568a1c257ae459bdfc7ffc1d8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 27 Dec 2017 20:08:43 +0100 Subject: [PATCH] Support default full-sample aggregation for postgis vector renderer --- .../aggregation/aggregation-mapconfig.js | 42 ++++++++++++++----- .../adapter/aggregation-mapconfig-adapter.js | 42 ++++++++++--------- test/acceptance/aggregation.js | 6 --- yarn.lock | 18 ++++---- 4 files changed, 63 insertions(+), 45 deletions(-) diff --git a/lib/cartodb/models/aggregation/aggregation-mapconfig.js b/lib/cartodb/models/aggregation/aggregation-mapconfig.js index 8852a821..a463d136 100644 --- a/lib/cartodb/models/aggregation/aggregation-mapconfig.js +++ b/lib/cartodb/models/aggregation/aggregation-mapconfig.js @@ -7,6 +7,17 @@ const { createAggregationColumnsValidator } = aggregationValidator; +const SubstitutionTokens = require('../../utils/substitution-tokens'); + +function prepareSql(sql) { + return sql && SubstitutionTokens.replace(sql, { + bbox: 'ST_MakeEnvelope(0,0,0,0)', + scale_denominator: '0', + pixel_width: '1', + pixel_height: '1' + }); +} + module.exports = class AggregationMapConfig extends MapConfig { static get AGGREGATIONS () { return aggregationQuery.SUPPORTED_AGGREGATE_FUNCTIONS; @@ -132,7 +143,9 @@ module.exports = class AggregationMapConfig extends MapConfig { return callback(err); } - const sql = limitedQuery({ query: layer.options.sql }); + const sql = limitedQuery({ + query: prepareSql(layer.options.sql) + }); connection.query(sql, (err, result) => { if (err) { @@ -141,7 +154,7 @@ module.exports = class AggregationMapConfig extends MapConfig { let columns = result.fields || []; - columns = columns.map(({ name, type }) => ({ name, type })); + columns = columns.map(({ name }) => name); if (skipGeoms) { columns = columns.filter((column) => !geomColumns.includes(column)); @@ -152,16 +165,25 @@ module.exports = class AggregationMapConfig extends MapConfig { }); } - isDefaultAggregation (index) { + isDefaultLayerAggregation (index) { const aggregation = this.getAggregation(index); - return !aggregation || ( - !aggregation.placement && - this._isEmptyParameter(aggregation.columns) && - this._isEmptyParameter(aggregation.dimensions) - ); + + return (this.isVectorOnlyMapConfig() && !this.hasLayerAggregation(index)) || + aggregation === true || + this._isEmptyAggregation(aggregation); } - _isEmptyParameter (parameter) { - return !parameter || Object.keys(parameter).length === 0; + _isEmptyAggregation (aggregation) { + return aggregation.placement === undefined && + aggregation.columns === undefined && + this._isEmptyParameter(aggregation.dimensions); + } + + _isEmptyParameter(parameter) { + return parameter === undefined || parameter === null || this._isEmptyObject(parameter); + } + + _isEmptyObject (parameter) { + return typeof parameter === 'object' && Object.keys(parameter).length === 0; } }; diff --git a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js index 1540faf7..79bd6527 100644 --- a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js @@ -89,32 +89,34 @@ module.exports = class AggregationMapConfigAdapter { return reject(err); } - if (shouldAdapt) { - const sqlQueryWrap = layer.options.sql_wrap; + if (!shouldAdapt) { + return resolve({ layer, index, adapted: shouldAdapt }); + } - let aggregationSql = mapConfig.getAggregatedQuery(index); + const sqlQueryWrap = layer.options.sql_wrap; - if (sqlQueryWrap) { - aggregationSql = sqlQueryWrap.replace(/<%=\s*sql\s*%>/g, aggregationSql); + let aggregationSql = mapConfig.getAggregatedQuery(index); + + if (sqlQueryWrap) { + aggregationSql = sqlQueryWrap.replace(/<%=\s*sql\s*%>/g, aggregationSql); + } + + if (!mapConfig.isDefaultLayerAggregation(index)) { + layer.options.sql = aggregationSql; + return resolve({ layer, index, adapted: shouldAdapt }); + } + + const skipGeoms = true; + mapConfig.getLayerColumns(index, skipGeoms, (err, columns) => { + if (err) { + return reject(err); } layer.options.sql = aggregationSql; + layer.options.columns = columns; - if (mapConfig.isDefaultAggregation(index)) { - const skipGeoms = true; - mapConfig.getLayerColumns(index, skipGeoms, (err, columns) => { - if (err) { - return reject(err); - } - - layer.options.columns = columns; - - return resolve({ layer, index, adapted: shouldAdapt }); - }); - } - } - - return resolve({ layer, index, adapted: shouldAdapt }); + return resolve({ layer, index, adapted: shouldAdapt }); + }); }); }); } diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js index afc27282..7879817a 100644 --- a/test/acceptance/aggregation.js +++ b/test/acceptance/aggregation.js @@ -956,12 +956,6 @@ describe('aggregation', function () { }); it('aggregates with full-sample placement by default', function (done) { - - // FIXME: skip until pg-mvt renderer is able to return all columns - if (process.env.POSTGIS_VERSION === '2.4') { - return done(); - } - this.mapConfig = createVectorMapConfig([ { type: 'cartodb', diff --git a/yarn.lock b/yarn.lock index 4ad1dabc..300bf086 100644 --- a/yarn.lock +++ b/yarn.lock @@ -244,14 +244,6 @@ carto@0.16.3: semver "^5.1.0" yargs "^4.2.0" -carto@CartoDB/carto#0.15.1-cdb1: - version "0.15.1-cdb1" - resolved "https://codeload.github.com/CartoDB/carto/tar.gz/8050ec843f1f32a6469e5d1cf49602773015d398" - dependencies: - mapnik-reference "~6.0.2" - optimist "~0.6.0" - underscore "~1.6.0" - carto@cartodb/carto#0.15.1-cdb3: version "0.15.1-cdb3" resolved "https://codeload.github.com/cartodb/carto/tar.gz/945f5efb74fd1af1f5e1f69f409f9567f94fb5a7" @@ -260,6 +252,14 @@ carto@cartodb/carto#0.15.1-cdb3: optimist "~0.6.0" underscore "1.8.3" +"carto@github:cartodb/carto#0.15.1-cdb1": + version "0.15.1-cdb1" + resolved "https://codeload.github.com/cartodb/carto/tar.gz/8050ec843f1f32a6469e5d1cf49602773015d398" + dependencies: + mapnik-reference "~6.0.2" + optimist "~0.6.0" + underscore "~1.6.0" + cartocolor@4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/cartocolor/-/cartocolor-4.0.0.tgz#841a3222d8b5b22718d9d545b1e5b972cb26eb36" @@ -2394,7 +2394,7 @@ window-size@^0.2.0: windshaft@cartodb/windshaft#pg-mvt-do-not-filter-columns: version "4.1.1" - resolved "https://codeload.github.com/cartodb/windshaft/tar.gz/2de4e21b41787acd1630193fdd42c308815bfee5" + resolved "https://codeload.github.com/cartodb/windshaft/tar.gz/04df77330ea983193345ef65601ee131bc1ce796" dependencies: abaculus cartodb/abaculus#2.0.3-cdb1 canvas cartodb/node-canvas#1.6.2-cdb2