From f7fad736c355d89ffa299cc3f596c1718abb140e Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 5 Apr 2018 16:01:29 +0200 Subject: [PATCH 1/3] Add test for uniqueness of aggregated cartodb_id --- test/acceptance/aggregation.js | 59 ++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js index db03ad8e..e17db2a5 100644 --- a/test/acceptance/aggregation.js +++ b/test/acceptance/aggregation.js @@ -1979,6 +1979,65 @@ describe('aggregation', function () { }); }); + + ['default', 'centroid', 'point-sample', 'point-grid'].forEach(placement => { + it(`aggregated ids are unique for ${placement} aggregation`, function (done) { + this.mapConfig = { + version: '1.6.0', + buffersize: { 'mvt': 0 }, + layers: [ + { + type: 'cartodb', + + options: { + sql: POINTS_SQL_1, + resolution: 1, + aggregation: { + threshold: 1 + } + } + } + ] + }; + if (placement !== 'default') { + this.mapConfig.layers[0].options.aggregation.placement = placement; + } + + this.testClient = new TestClient(this.mapConfig); + + this.testClient.getTile(1, 0, 1, { format: 'mvt' }, (err, res, mvt) => { + if (err) { + return done(err); + } + + const tile1 = JSON.parse(mvt.toGeoJSONSync(0)); + + assert.ok(Array.isArray(tile1.features)); + assert.ok(tile1.features.length > 0); + + this.testClient.getTile(1, 1, 0, { format: 'mvt' }, (err, res, mvt) => { + if (err) { + return done(err); + } + + const tile2 = JSON.parse(mvt.toGeoJSONSync(0)); + + assert.ok(Array.isArray(tile2.features)); + assert.ok(tile2.features.length > 0); + + const tile1Ids = tile1.features.map(f => f.properties.cartodb_id); + const tile2Ids = tile2.features.map(f => f.properties.cartodb_id); + const repeatedIds = tile1Ids.filter(id => tile2Ids.includes(id)); + assert.equal(repeatedIds.length, 0); + + done(); + }); + + }); + }); + }); + + }); }); }); From ffa3a96f1a56d6cacd584bbf1bdd0b89581523cb Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 4 Apr 2018 11:05:03 +0200 Subject: [PATCH 2/3] Use unique cartodb_id in aggregated results See #889 FOr centroid and point-grid the cartodb_id wasn't unique across tiles. --- lib/cartodb/models/aggregation/aggregation-query.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js index 02211a7d..cfb9855a 100644 --- a/lib/cartodb/models/aggregation/aggregation-query.js +++ b/lib/cartodb/models/aggregation/aggregation-query.js @@ -290,7 +290,7 @@ const aggregationQueryTemplates = { !bbox! AS bbox ) SELECT - row_number() over() AS cartodb_id, + MIN(_cdb_query.cartodb_id) AS cartodb_id, ST_SetSRID( ST_MakePoint( AVG(ST_X(_cdb_query.the_geom_webmercator)), @@ -317,6 +317,7 @@ const aggregationQueryTemplates = { ), _cdb_clusters AS ( SELECT + MIN(_cdb_query.cartodb_id) AS cartodb_id, Floor(ST_X(_cdb_query.the_geom_webmercator)/_cdb_params.res)::int AS _cdb_gx, Floor(ST_Y(_cdb_query.the_geom_webmercator)/_cdb_params.res)::int AS _cdb_gy ${dimensionDefs(ctx)} @@ -327,7 +328,7 @@ const aggregationQueryTemplates = { ${havingClause(ctx)} ) SELECT - row_number() over() AS cartodb_id, + _cdb_clusters.cartodb_id AS cartodb_id, ST_SetSRID(ST_MakePoint((_cdb_gx+0.5)*res, (_cdb_gy+0.5)*res), 3857) AS the_geom_webmercator ${dimensionNames(ctx)} ${aggregateColumnNames(ctx)} From 26c5ff1f9362e0bf19370452ff6898272f39eafd Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 5 Apr 2018 16:36:07 +0200 Subject: [PATCH 3/3] Update news --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index 75756417..20b8f06a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -9,6 +9,7 @@ New features: Bug Fixes: - Non-default aggregation selected the wrong columns (e.g. for vector tiles) - Aggregation dimensions with alias where broken +- cartodb_id was not unique accross aggregated vector tiles ## 6.0.0 Released 2018-03-19