diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js index b674bb2d..ca350110 100644 --- a/test/acceptance/aggregation.js +++ b/test/acceptance/aggregation.js @@ -27,6 +27,15 @@ describe('aggregation', function () { from generate_series(-3, 3) x `; + const POINTS_SQL_0 = ` + select + x + 4 as cartodb_id, + st_setsrid(st_makepoint(x*10+1, x*10+1), 4326) as the_geom, + st_transform(st_setsrid(st_makepoint(x*10+1, x*10+1), 4326), 3857) as the_geom_webmercator, + x as value + from generate_series(-3, 3) x + `; + const POINTS_SQL_TIMESTAMP_1 = ` select row_number() over() AS cartodb_id, @@ -103,13 +112,26 @@ describe('aggregation', function () { const POINTS_SQL_PAIRS = ` -- Generate pairs of near points select - x + 4 as cartodb_id, - st_setsrid(st_makepoint(Floor(x/2)*10 + x/1000.0, Floor(x/2)*10 + x/1000.0), 4326) as the_geom, + x + 1 as cartodb_id, + st_setsrid( + st_makepoint( + Floor((x-6)/2)*10 + 9E-3*(x % 2 + 1), + Floor((x-6)/2)*10 + 9E-3*(x % 2 + 1) + ), + 4326 + ) as the_geom, st_transform( - st_setsrid(st_makepoint(Floor(x/2)*10 + x/1000.0, Floor(x/2)*10 + x/1000.0),4326), - 3857) as the_geom_webmercator, + st_setsrid( + st_makepoint( + Floor((x-6)/2)*10 + 9E-3*(x % 2 + 1), + Floor((x-6)/2)*10 + 9E-3*(x % 2 + 1) + ), + 4326 + ), + 3857 + ) as the_geom_webmercator, x as value - from generate_series(-6, 6) x + from generate_series(0, 13) x `; function createVectorMapConfig (layers = [ @@ -2129,9 +2151,8 @@ describe('aggregation', function () { }); }); - ['default', 'centroid', 'point-sample', 'point-grid'].forEach(placement => { - it(`aggregated ids are unique for ${placement} aggregation`, function (done) { + it(`for ${placement} and no points between tiles has unique ids`, function (done) { this.mapConfig = { version: '1.6.0', buffersize: { 'mvt': 0 }, @@ -2140,7 +2161,7 @@ describe('aggregation', function () { type: 'cartodb', options: { - sql: POINTS_SQL_1, + sql: POINTS_SQL_0, resolution: 1, aggregation: { threshold: 1 @@ -2178,6 +2199,7 @@ describe('aggregation', function () { 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(); @@ -2185,6 +2207,79 @@ describe('aggregation', function () { }); }); + it(`for ${placement} has unique ids save between tiles`, 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)); + + // It is not guaranteed that features appear in a single tile: + // features on the border of tiles can appear in multiple tiles + if (repeatedIds.length > 0) { + repeatedIds.forEach(id => { + const tile1Features = tile1.features.filter(f => f.properties.cartodb_id === id); + const tile2Features = tile2.features.filter(f => f.properties.cartodb_id === id); + // repetitions cannot occur inside a tile + assert.equal(tile1Features.length, 1); + assert.equal(tile2Features.length, 1); + const feature1 = tile1Features[0]; + const feature2 = tile2Features[0]; + // features should be identical (geometry and properties) + assert.deepEqual(feature1.properties, feature2.properties); + assert.deepEqual(feature1.geometry, feature2.geometry); + // and geometry should be on the border; + // for the dataset and zoom 1, only point with cartodb_id=4 (0,0) + assert.equal(feature1.properties.cartodb_id, 4); + assert.equal(feature2.properties.cartodb_id, 4); + }); + } + done(); + }); + + }); + }); }); ['default', 'centroid', 'point-sample', 'point-grid'].forEach(placement => { @@ -2302,7 +2397,7 @@ describe('aggregation', function () { assert.equal(typeof body.metadata, 'object'); assert.ok(Array.isArray(body.metadata.layers)); assert.ok(body.metadata.layers[0].meta.aggregation.mvt); - assert.equal(body.metadata.layers[0].meta.stats.aggrFeatureCount, 9); + assert.equal(body.metadata.layers[0].meta.stats.aggrFeatureCount, 7); done(); }); @@ -2343,7 +2438,7 @@ describe('aggregation', function () { assert.equal(typeof body.metadata, 'object'); assert.ok(Array.isArray(body.metadata.layers)); assert.ok(body.metadata.layers[0].meta.aggregation.mvt); - assert.equal(body.metadata.layers[0].meta.stats.featureCount, 13); + assert.equal(body.metadata.layers[0].meta.stats.featureCount, 14); done(); }); diff --git a/test/acceptance/stats/mapnik_stats_layergroup.js b/test/acceptance/stats/mapnik_stats_layergroup.js index 4e8413ae..dd58fd85 100644 --- a/test/acceptance/stats/mapnik_stats_layergroup.js +++ b/test/acceptance/stats/mapnik_stats_layergroup.js @@ -56,6 +56,17 @@ describe('Create mapnik layergroup', function() { } }; + var mapnikLayer100 = { + type: 'mapnik', + options: { + sql: [ + 'SELECT * FROM test_table_100' + ].join(''), + cartocss_version: cartocssVersion, + cartocss: cartocss + } + }; + var httpLayer = { type: 'http', options: { @@ -512,8 +523,8 @@ describe('Create mapnik layergroup', function() { var testClient = new TestClient({ version: '1.4.0', layers: [ - layerWithMetadata(mapnikLayer4, { - sample: { num_rows: 3 } + layerWithMetadata(mapnikLayer100, { + sample: { num_rows: 30 } }) ] }); @@ -521,9 +532,9 @@ describe('Create mapnik layergroup', function() { testClient.getLayergroup(function(err, layergroup) { assert.ifError(err); assert.equal(layergroup.metadata.layers[0].id, mapnikBasicLayerId(0)); - assert.equal(layergroup.metadata.layers[0].meta.stats.estimatedFeatureCount, 5); + assert.equal(layergroup.metadata.layers[0].meta.stats.estimatedFeatureCount, 100); assert(layergroup.metadata.layers[0].meta.stats.sample.length > 0); - const expectedCols = [ 'cartodb_id', 'address', 'the_geom', 'the_geom_webmercator' ].sort(); + const expectedCols = [ 'cartodb_id', 'value', 'the_geom', 'the_geom_webmercator' ].sort(); assert.deepEqual(Object.keys(layergroup.metadata.layers[0].meta.stats.sample[0]).sort(), expectedCols); testClient.drain(done); }); @@ -533,10 +544,10 @@ describe('Create mapnik layergroup', function() { var testClient = new TestClient({ version: '1.4.0', layers: [ - layerWithMetadata(mapnikLayer4, { + layerWithMetadata(mapnikLayer100, { sample: { - num_rows: 3, - include_columns: [ 'cartodb_id', 'address', 'the_geom' ] + num_rows: 30, + include_columns: [ 'cartodb_id', 'the_geom' ] } }) ] @@ -545,9 +556,9 @@ describe('Create mapnik layergroup', function() { testClient.getLayergroup(function(err, layergroup) { assert.ifError(err); assert.equal(layergroup.metadata.layers[0].id, mapnikBasicLayerId(0)); - assert.equal(layergroup.metadata.layers[0].meta.stats.estimatedFeatureCount, 5); + assert.equal(layergroup.metadata.layers[0].meta.stats.estimatedFeatureCount, 100); assert(layergroup.metadata.layers[0].meta.stats.sample.length > 0); - const expectedCols = [ 'cartodb_id', 'address', 'the_geom' ].sort(); + const expectedCols = [ 'cartodb_id', 'the_geom' ].sort(); assert.deepEqual(Object.keys(layergroup.metadata.layers[0].meta.stats.sample[0]).sort(), expectedCols); testClient.drain(done); }); diff --git a/test/support/sql/windshaft.test.sql b/test/support/sql/windshaft.test.sql index 51a6de8a..3c957db6 100644 --- a/test/support/sql/windshaft.test.sql +++ b/test/support/sql/windshaft.test.sql @@ -778,4 +778,46 @@ CREATE OR REPLACE FUNCTION cdb_crankshaft.CDB_KMeans(query text, no_clusters int $$ LANGUAGE plpgsql; GRANT ALL ON FUNCTION cdb_crankshaft.CDB_KMeans(text, integer, integer) TO :TESTUSER; +-- Table with 100 rows +-- first table +CREATE TABLE test_table_100 ( + cartodb_id integer NOT NULL, + value int, + the_geom geometry, + the_geom_webmercator geometry, + CONSTRAINT enforce_dims_the_geom CHECK ((st_ndims(the_geom) = 2)), + CONSTRAINT enforce_dims_the_geom_webmercator CHECK ((st_ndims(the_geom_webmercator) = 2)), + CONSTRAINT enforce_geotype_the_geom CHECK (((geometrytype(the_geom) = 'POINT'::text) OR (the_geom IS NULL))), + CONSTRAINT enforce_geotype_the_geom_webmercator CHECK (((geometrytype(the_geom_webmercator) = 'POINT'::text) OR (the_geom_webmercator IS NULL))), + CONSTRAINT enforce_srid_the_geom CHECK ((st_srid(the_geom) = 4326)), + CONSTRAINT enforce_srid_the_geom_webmercator CHECK ((st_srid(the_geom_webmercator) = 3857)) +); + +CREATE SEQUENCE test_table_100_cartodb_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE test_table_100_cartodb_id_seq OWNED BY test_table_100.cartodb_id; + +SELECT pg_catalog.setval('test_table_100_cartodb_id_seq', 60, true); + +ALTER TABLE test_table_100 ALTER COLUMN cartodb_id SET DEFAULT nextval('test_table_100_cartodb_id_seq'::regclass); + +INSERT INTO test_table_100(the_geom, value) + SELECT + ST_SetSRID(ST_MakePoint(n*10 + 9E-3, n*10 + 9E-3), 4326) AS the_geom,n AS value + FROM generate_series(1, 100) n; + +ALTER TABLE ONLY test_table_100 ADD CONSTRAINT test_table_100_pkey PRIMARY KEY (cartodb_id); + +CREATE INDEX test_table_100_the_geom_idx ON test_table_100 USING gist (the_geom); +CREATE INDEX test_table_100_the_geom_webmercator_idx ON test_table_100 USING gist (the_geom_webmercator); + +GRANT ALL ON TABLE test_table_100 TO :TESTUSER; +GRANT SELECT ON TABLE test_table_100 TO :PUBLICUSER; + + ANALYZE;