cartodb4/Windshaft-cartodb
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #1082 from CartoDB/1073-aggregation-false

Browse Source 
Fix boolean aggregation layer option
This commit is contained in:
Raúl Marín
2019-03-14 17:14:26 +01:00
committed by GitHub
parent 514fc908b9 ab9cfee7b1
commit 9a98422076
4 changed files with 135 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
NEWS.md
+1
View File
@@ -6,6 +6,7 @@ Released 2019-mm-dd
Announcements:
- Experimental support for listing features in a grid when the map uses the dynamic agregation.
- Numeric histogram performance improvement (#1080)
- Fix boolean aggregation layer option not working when numbers of rows are above the threshold (#1082)
- Update deps:
- windshaft@4.13.3: Upgrade grainstore to version 1.11.0, do not hang when child process is not able to generate a Mapnik XML
lib/cartodb/models/aggregation/aggregation-mapconfig.js
+28 -19
View File
@@ -49,6 +49,10 @@ module.exports = class AggregationMapConfig extends MapConfig {
];
}
static get HAS_AGGREGATION_DISABLED () {
return null;
}
static supportsGeometryType(geometryType) {
return AggregationMapConfig.SUPPORTED_GEOMETRY_TYPES.includes(geometryType);
}
@@ -103,19 +107,19 @@ module.exports = class AggregationMapConfig extends MapConfig {
});
}
isAggregationMapConfig () {
return this.isVectorOnlyMapConfig() || this.hasAnyLayerAggregation();
}
isAggregationLayer (index) {
return this.isVectorOnlyMapConfig() || this.hasLayerAggregation(index);
let hasAggregation = this.hasLayerAggregation(index);
// for vector-only MapConfig are aggregated unless explicitly disabled
return hasAggregation || (
this.isVectorOnlyMapConfig() && hasAggregation !== AggregationMapConfig.HAS_AGGREGATION_DISABLED
);
}
hasAnyLayerAggregation () {
isAggregationMapConfig () {
const layers = this.getLayers();
for (let index = 0; index < layers.length; index++) {
if (this.hasLayerAggregation(index)) {
if (this.isAggregationLayer(index)) {
return true;
}
}
@@ -123,22 +127,30 @@ module.exports = class AggregationMapConfig extends MapConfig {
return false;
}
/* Three possible return values:
* * true: explicit aggregation ({}, true)
* * false: no aggregation (undefined)
* * AggregationMapConfig.HAS_AGGREGATION_DISABLED: explicitly disabled aggregation (false)
*/
hasLayerAggregation (index) {
const layer = this.getLayer(index);
const { aggregation } = layer.options;
return aggregation !== undefined && (typeof aggregation === 'object' || typeof aggregation === 'boolean');
if (aggregation !== undefined && (typeof aggregation === 'object' || typeof aggregation === 'boolean')) {
if (aggregation === false) {
return AggregationMapConfig.HAS_AGGREGATION_DISABLED;
}
return true;
}
return false;
}
getAggregation (index) {
if (this.isVectorOnlyMapConfig() && !this.hasLayerAggregation(index)) {
return {};
}
const { aggregation } = this.getLayer(index).options;
if (typeof aggregation === 'boolean') {
return {};
if (this.isAggregationLayer(index)) {
if (typeof aggregation === 'boolean' || !aggregation) {
return {};
}
}
return aggregation;
@@ -221,10 +233,7 @@ module.exports = class AggregationMapConfig extends MapConfig {
_isDefaultLayerAggregation (index) {
const aggregation = this.getAggregation(index);
return (this.isVectorOnlyMapConfig() && !this.hasLayerAggregation(index)) ||
aggregation === true ||
this._isDefaultAggregation(aggregation);
return aggregation && this._isDefaultAggregation(aggregation);
}
_isDefaultAggregation (aggregation) {
test/acceptance/aggregation.js
+65 -1
View File
@@ -91,6 +91,8 @@ describe('aggregation', function () {
from generate_series(-3, 3) x
`;
const POINTS_OVER_THRESHOLD = 'SELECT * FROM test_table_200k';
const POLYGONS_SQL_1 = `
select
x + 4 as cartodb_id,
@@ -665,7 +667,9 @@ describe('aggregation', function () {
{
type: 'cartodb',
options: {
sql: POINTS_SQL_2,
// Note that we need the table to have more than AggregationMapConfig.THRESHOLD rows
// otherwise it won't get aggregated in any case
sql: POINTS_OVER_THRESHOLD,
aggregation: false
}
}
@@ -689,6 +693,66 @@ describe('aggregation', function () {
});
});
it('use default aggregation by setting `aggregation: true`', function (done) {
const mapConfig = createVectorMapConfig([
{
type: 'cartodb',
options: {
sql: POINTS_OVER_THRESHOLD,
cartocss: '#layer { marker-width: 7; }',
cartocss_version: '2.3.0',
aggregation: true
}
}
]);
this.testClient = new TestClient(mapConfig);
this.testClient.getLayergroup((err, body) => {
if (err) {
return done(err);
}
assert.equal(typeof body.metadata, 'object');
assert.ok(Array.isArray(body.metadata.layers));
assert.equal(body.metadata.layers[0].meta.aggregation.mvt, true);
assert.equal(body.metadata.layers[0].meta.aggregation.png, true);
done();
});
});
it('but do not aggregate below threshold by setting `aggregation: true`', function (done) {
const mapConfig = createVectorMapConfig([
{
type: 'cartodb',
options: {
sql: POINTS_SQL_2,
cartocss: '#layer { marker-width: 7; }',
cartocss_version: '2.3.0',
aggregation: true
}
}
]);
this.testClient = new TestClient(mapConfig);
this.testClient.getLayergroup((err, body) => {
if (err) {
return done(err);
}
assert.equal(typeof body.metadata, 'object');
assert.ok(Array.isArray(body.metadata.layers));
assert.equal(body.metadata.layers[0].meta.aggregation.mvt, false);
assert.equal(body.metadata.layers[0].meta.aggregation.png, false);
done();
});
});
it('when the aggregation param is not valid should respond with error', function (done) {
const mapConfig = createVectorMapConfig([
{
test/support/sql/windshaft.test.sql
+41
View File
@@ -819,5 +819,46 @@ CREATE INDEX test_table_100_the_geom_webmercator_idx ON test_table_100 USING gis
GRANT ALL ON TABLE test_table_100 TO :TESTUSER;
GRANT SELECT ON TABLE test_table_100 TO :PUBLICUSER;
-- Table with 200K rows
CREATE TABLE test_table_200k (
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_200k_cartodb_id_seq
START WITH 1
INCREMENT BY 1
NO MINVALUE
NO MAXVALUE
CACHE 1;
ALTER SEQUENCE test_table_200k_cartodb_id_seq OWNED BY test_table_200k.cartodb_id;
SELECT pg_catalog.setval('test_table_200k_cartodb_id_seq', 60, true);
ALTER TABLE test_table_200k ALTER COLUMN cartodb_id SET DEFAULT nextval('test_table_200k_cartodb_id_seq'::regclass);
INSERT INTO test_table_200k(the_geom, the_geom_webmercator, value)
SELECT
ST_SetSRID(ST_MakePoint(n*1E-4 + 9E-3, n*1E-4 + 9E-3), 4326) AS the_geom,
ST_Transform(ST_SetSRID(ST_MakePoint(n*1E-4 + 9E-3, n*1E-4 + 9E-3), 4326), 3857) AS the_geom_webmercator,
n AS value
FROM generate_series(1, 200000) n;
ALTER TABLE ONLY test_table_200k ADD CONSTRAINT test_table_200k_pkey PRIMARY KEY (cartodb_id);
CREATE INDEX test_table_200k_the_geom_idx ON test_table_200k USING gist (the_geom);
CREATE INDEX test_table_200k_the_geom_webmercator_idx ON test_table_200k USING gist (the_geom_webmercator);
GRANT ALL ON TABLE test_table_200k TO :TESTUSER;
GRANT SELECT ON TABLE test_table_200k TO :PUBLICUSER;
ANALYZE;