From de376eef86188d758dd74eb75ee54e93b14d200a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 27 Nov 2017 14:52:30 +0100 Subject: [PATCH 01/30] vr-aggregation: link to windshaft#vr-aggregation --- package.json | 2 +- yarn.lock | 109 ++++++++++++++++++++++++++++++++++++--------------- 2 files changed, 78 insertions(+), 33 deletions(-) diff --git a/package.json b/package.json index 8e4c1d84..f68f2acf 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,7 @@ "step-profiler": "~0.3.0", "turbo-carto": "0.20.1", "underscore": "~1.6.0", - "windshaft": "4.0.1", + "windshaft": "cartodb/windshaft#vr-aggretation", "yargs": "~5.0.0" }, "devDependencies": { diff --git a/yarn.lock b/yarn.lock index 7b5394aa..3eafbd0d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2,7 +2,7 @@ # yarn lockfile v1 -"abaculus@github:cartodb/abaculus#2.0.3-cdb1": +abaculus@cartodb/abaculus#2.0.3-cdb1: version "2.0.3-cdb1" resolved "https://codeload.github.com/cartodb/abaculus/tar.gz/f5f34e1c80cdd8d49edd1d6fe3b2220ab2e23aaf" dependencies: @@ -222,7 +222,7 @@ camshaft@0.59.4: dot "^1.0.3" request "^2.69.0" -"canvas@github:cartodb/node-canvas#1.6.2-cdb2": +canvas@cartodb/node-canvas#1.6.2-cdb2: version "1.6.2-cdb2" resolved "https://codeload.github.com/cartodb/node-canvas/tar.gz/8acf04557005c633f9e68524488a2657c04f3766" dependencies: @@ -240,15 +240,7 @@ 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@github:cartodb/carto#0.15.1-cdb3": +carto@cartodb/carto#0.15.1-cdb3: version "0.15.1-cdb3" resolved "https://codeload.github.com/cartodb/carto/tar.gz/945f5efb74fd1af1f5e1f69f409f9567f94fb5a7" dependencies: @@ -256,6 +248,14 @@ carto@CartoDB/carto#0.15.1-cdb1: 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" @@ -484,6 +484,10 @@ destroy@~1.0.4: version "1.0.4" resolved "https://registry.yarnpkg.com/destroy/-/destroy-1.0.4.tgz#978857442c44749e4206613e37946205826abd80" +detect-libc@^1.0.2: + version "1.0.3" + resolved "https://registry.yarnpkg.com/detect-libc/-/detect-libc-1.0.3.tgz#fa137c4bd698edf55cd5cd02ac559f91a4c4ba9b" + diff@3.2.0: version "3.2.0" resolved "https://registry.yarnpkg.com/diff/-/diff-3.2.0.tgz#c9ce393a4b7cbd0b058a725c93df299027868ff9" @@ -746,7 +750,7 @@ gdal@~0.9.2: nan "~2.6.2" node-pre-gyp "~0.6.36" -generic-pool@2.4.3, generic-pool@~2.4.0, generic-pool@~2.4.1: +generic-pool@2.4.3: version "2.4.3" resolved "https://registry.yarnpkg.com/generic-pool/-/generic-pool-2.4.3.tgz#780c36f69dfad05a5a045dd37be7adca11a4f6ff" @@ -758,6 +762,10 @@ generic-pool@~2.2.0, generic-pool@~2.2.1: version "2.2.2" resolved "https://registry.yarnpkg.com/generic-pool/-/generic-pool-2.2.2.tgz#7a89f491d575b42f9f069a0e8e2c6dbaa3c241be" +generic-pool@~2.4.0, generic-pool@~2.4.1: + version "2.4.6" + resolved "https://registry.yarnpkg.com/generic-pool/-/generic-pool-2.4.6.tgz#f1b55e572167dba2fe75d5aa91ebb1e9f72642d7" + get-caller-file@^1.0.1: version "1.0.2" resolved "https://registry.yarnpkg.com/get-caller-file/-/get-caller-file-1.0.2.tgz#f702e63127e7e231c160a80c1554acb70d5047e5" @@ -768,7 +776,7 @@ getpass@^0.1.1: dependencies: assert-plus "^1.0.0" -glob@7.1.1, glob@^7.0.5, glob@^7.1.1: +glob@7.1.1, glob@^7.1.1: version "7.1.1" resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.1.tgz#805211df04faaf1c63a3600306cdf5ade50b2ec8" dependencies: @@ -799,6 +807,17 @@ glob@^6.0.1: once "^1.3.0" path-is-absolute "^1.0.0" +glob@^7.0.5: + version "7.1.2" + resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.2.tgz#c19c9df9a028702d678612384a6552404c636d15" + dependencies: + fs.realpath "^1.0.0" + inflight "^1.0.4" + inherits "2" + minimatch "^3.0.4" + once "^1.3.0" + path-is-absolute "^1.0.0" + graceful-fs@^4.1.2: version "4.1.11" resolved "https://registry.yarnpkg.com/graceful-fs/-/graceful-fs-4.1.11.tgz#0e8bdfe4d1ddb8854d64e04ea7c00e2a026e5658" @@ -963,8 +982,8 @@ inherits@2, inherits@2.0.3, inherits@~2.0.0, inherits@~2.0.1, inherits@~2.0.3: resolved "https://registry.yarnpkg.com/inherits/-/inherits-2.0.3.tgz#633c2c83e3da42a502f52466022480f4208261de" ini@~1.3.0: - version "1.3.4" - resolved "https://registry.yarnpkg.com/ini/-/ini-1.3.4.tgz#0537cb79daf59b59a1a517dff706c86ec039162e" + version "1.3.5" + resolved "https://registry.yarnpkg.com/ini/-/ini-1.3.5.tgz#eee25f56db1c9ec6085e0c22778083f596abf927" invert-kv@^1.0.0: version "1.0.0" @@ -1300,7 +1319,7 @@ mime@~1.3.4: version "1.3.6" resolved "https://registry.yarnpkg.com/mime/-/mime-1.3.6.tgz#591d84d3653a6b0b4a3b9df8de5aa8108e72e5e0" -"minimatch@2 || 3", minimatch@^3.0.0, minimatch@^3.0.2, minimatch@~3.0.2: +"minimatch@2 || 3", minimatch@^3.0.0, minimatch@^3.0.2, minimatch@^3.0.4, minimatch@~3.0.2: version "3.0.4" resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-3.0.4.tgz#5166e286457f03306064be5497e8dbb0c3d32083" dependencies: @@ -1360,10 +1379,14 @@ mv@~2: ncp "~2.0.0" rimraf "~2.4.0" -nan@^2.0.8, nan@^2.3.4, nan@^2.4.0, nan@~2.7.0: +nan@^2.0.8, nan@^2.3.4, nan@~2.7.0: version "2.7.0" resolved "https://registry.yarnpkg.com/nan/-/nan-2.7.0.tgz#d95bf721ec877e08db276ed3fc6eb78f9083ad46" +nan@^2.4.0: + version "2.8.0" + resolved "https://registry.yarnpkg.com/nan/-/nan-2.8.0.tgz#ed715f3fe9de02b57a5e6252d90a96675e1f085a" + nan@~2.4.0: version "2.4.0" resolved "https://registry.yarnpkg.com/nan/-/nan-2.4.0.tgz#fb3c59d45fe4effe215f0b890f8adf6eb32d2232" @@ -1392,9 +1415,10 @@ nock@~2.11.0: propagate "0.3.x" node-pre-gyp@~0.6.30, node-pre-gyp@~0.6.36, node-pre-gyp@~0.6.38: - version "0.6.38" - resolved "https://registry.yarnpkg.com/node-pre-gyp/-/node-pre-gyp-0.6.38.tgz#e92a20f83416415bb4086f6d1fb78b3da73d113d" + version "0.6.39" + resolved "https://registry.yarnpkg.com/node-pre-gyp/-/node-pre-gyp-0.6.39.tgz#c00e96860b23c0e1420ac7befc5044e1d78d8649" dependencies: + detect-libc "^1.0.2" hawk "3.1.3" mkdirp "^0.5.1" nopt "^4.0.1" @@ -1449,10 +1473,14 @@ oauth-sign@~0.8.1, oauth-sign@~0.8.2: version "0.8.2" resolved "https://registry.yarnpkg.com/oauth-sign/-/oauth-sign-0.8.2.tgz#46a6ab7f0aead8deae9ec0565780b7d4efeb9d43" -object-assign@4.1.0, object-assign@^4.1.0: +object-assign@4.1.0: version "4.1.0" resolved "https://registry.yarnpkg.com/object-assign/-/object-assign-4.1.0.tgz#7a3b3d0e98063d43f4c03f2e8ae6cd51a86883a0" +object-assign@^4.1.0: + version "4.1.1" + resolved "https://registry.yarnpkg.com/object-assign/-/object-assign-4.1.1.tgz#2109adc7965887cfc05cbbd442cac8bfbb360863" + object-keys@~0.4.0: version "0.4.0" resolved "https://registry.yarnpkg.com/object-keys/-/object-keys-0.4.0.tgz#28a6aae7428dd2c3a92f3d95f21335dd204e0336" @@ -1721,8 +1749,8 @@ raw-body@2.3.2: unpipe "1.0.0" rc@^1.1.7: - version "1.2.1" - resolved "https://registry.yarnpkg.com/rc/-/rc-1.2.1.tgz#2e03e8e42ee450b8cb3dce65be1bf8974e1dfd95" + version "1.2.2" + resolved "https://registry.yarnpkg.com/rc/-/rc-1.2.2.tgz#d8ce9cb57e8d64d9c7badd9876c7c34cbe3c7077" dependencies: deep-extend "~0.4.0" ini "~1.3.0" @@ -1744,7 +1772,7 @@ read-pkg@^1.0.0: normalize-package-data "^2.3.2" path-type "^1.0.0" -readable-stream@1.1, readable-stream@~1.1.9: +readable-stream@1.1: version "1.1.13" resolved "https://registry.yarnpkg.com/readable-stream/-/readable-stream-1.1.13.tgz#f6eef764f514c89e2b9e23146a75ba106756d23e" dependencies: @@ -1774,6 +1802,15 @@ readable-stream@~1.0.2: isarray "0.0.1" string_decoder "~0.10.x" +readable-stream@~1.1.9: + version "1.1.14" + resolved "https://registry.yarnpkg.com/readable-stream/-/readable-stream-1.1.14.tgz#7cf4c54ef648e3813084c636dd2079e166c081d9" + dependencies: + core-util-is "~1.0.0" + inherits "~2.0.1" + isarray "0.0.1" + string_decoder "~0.10.x" + redis-mpool@0.4.1, redis-mpool@~0.4.1: version "0.4.1" resolved "https://registry.yarnpkg.com/redis-mpool/-/redis-mpool-0.4.1.tgz#d917c0a4ed57a1291a9c6eb35434e6c0b7046f80" @@ -1883,7 +1920,7 @@ safe-json-stringify@~1: version "1.0.4" resolved "https://registry.yarnpkg.com/safe-json-stringify/-/safe-json-stringify-1.0.4.tgz#81a098f447e4bbc3ff3312a243521bc060ef5911" -"semver@2 || 3 || 4 || 5", semver@^5.1.0, semver@^5.3.0, semver@~5.3.0: +"semver@2 || 3 || 4 || 5", semver@~5.3.0: version "5.3.0" resolved "https://registry.yarnpkg.com/semver/-/semver-5.3.0.tgz#9b2ce5d3de02d17c6012ad326aa6b4d0cf54f94f" @@ -1891,6 +1928,10 @@ semver@4.3.2: version "4.3.2" resolved "https://registry.yarnpkg.com/semver/-/semver-4.3.2.tgz#c7a07158a80bedd052355b770d82d6640f803be7" +semver@^5.1.0, semver@^5.3.0: + version "5.4.1" + resolved "https://registry.yarnpkg.com/semver/-/semver-5.4.1.tgz#e059c09d8571f0540823733433505d3a2f00b18e" + semver@~4.3.3: version "4.3.6" resolved "https://registry.yarnpkg.com/semver/-/semver-4.3.6.tgz#300bc6e0e86374f7ba61068b5b1ecd57fc6532da" @@ -1996,10 +2037,14 @@ speedometer@~0.1.2: version "0.1.4" resolved "https://registry.yarnpkg.com/speedometer/-/speedometer-0.1.4.tgz#9876dbd2a169d3115402d48e6ea6329c8816a50d" -sphericalmercator@1.0.4, sphericalmercator@1.0.x, sphericalmercator@~1.0.1, sphericalmercator@~1.0.4: +sphericalmercator@1.0.4: version "1.0.4" resolved "https://registry.yarnpkg.com/sphericalmercator/-/sphericalmercator-1.0.4.tgz#baad4e34187f06e87f2e92fc1280199fa1b01d4e" +sphericalmercator@1.0.x, sphericalmercator@~1.0.1, sphericalmercator@~1.0.4: + version "1.0.5" + resolved "https://registry.yarnpkg.com/sphericalmercator/-/sphericalmercator-1.0.5.tgz#ddc5a049e360e000d0fad9fc22c4071882584980" + split@^1.0.0: version "1.0.1" resolved "https://registry.yarnpkg.com/split/-/split-1.0.1.tgz#605bd9be303aa59fb35f9229fbea0ddec9ea07d9" @@ -2114,8 +2159,8 @@ supports-color@^3.1.0, supports-color@^3.1.2, supports-color@^3.2.3: has-flag "^1.0.0" tar-pack@^3.4.0: - version "3.4.0" - resolved "https://registry.yarnpkg.com/tar-pack/-/tar-pack-3.4.0.tgz#23be2d7f671a8339376cbdb0b8fe3fdebf317984" + version "3.4.1" + resolved "https://registry.yarnpkg.com/tar-pack/-/tar-pack-3.4.1.tgz#e1dbc03a9b9d3ba07e896ad027317eb679a10a1f" dependencies: debug "^2.2.0" fstream "^1.0.10" @@ -2145,7 +2190,7 @@ through@2: version "2.3.8" resolved "https://registry.yarnpkg.com/through/-/through-2.3.8.tgz#0dd4c9ffaabc357960b1b724115d7e0e86a2e1f5" -"tilelive-bridge@github:cartodb/tilelive-bridge#2.3.1-cdb4": +tilelive-bridge@cartodb/tilelive-bridge#2.3.1-cdb4: version "2.3.1-cdb4" resolved "https://codeload.github.com/cartodb/tilelive-bridge/tar.gz/faa2b638da2d119b78281575d40255cb523f6ca6" dependencies: @@ -2153,7 +2198,7 @@ through@2: mapnik-pool "~0.1.3" sphericalmercator "1.0.x" -"tilelive-mapnik@github:cartodb/tilelive-mapnik#0.6.18-cdb3": +tilelive-mapnik@cartodb/tilelive-mapnik#0.6.18-cdb3: version "0.6.18-cdb3" resolved "https://codeload.github.com/cartodb/tilelive-mapnik/tar.gz/23bd1c31dd57d0b76c86b9f1eaf62462b3c17d01" dependencies: @@ -2314,9 +2359,9 @@ window-size@^0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/window-size/-/window-size-0.2.0.tgz#b4315bb4214a3d7058ebeee892e13fa24d98b075" -windshaft@4.0.1: - version "4.0.1" - resolved "https://registry.yarnpkg.com/windshaft/-/windshaft-4.0.1.tgz#16230a0b5b28f08a443491339c37637e3caefa52" +windshaft@cartodb/windshaft#vr-aggretation: + version "4.0.2" + resolved "https://codeload.github.com/cartodb/windshaft/tar.gz/e0cbbafdf7a65537337c3ca185b6fd2f9bac4e59" dependencies: abaculus cartodb/abaculus#2.0.3-cdb1 canvas cartodb/node-canvas#1.6.2-cdb2 From 51c19c0b2eca9d86d4151031c3a32e47b0382765 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Tue, 28 Nov 2017 16:49:55 +0000 Subject: [PATCH 02/30] Skip test --- test/acceptance/multilayer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/multilayer.js b/test/acceptance/multilayer.js index 9d287a58..51545416 100644 --- a/test/acceptance/multilayer.js +++ b/test/acceptance/multilayer.js @@ -1164,7 +1164,7 @@ describe(suiteName, function() { }); // See https://github.com/CartoDB/Windshaft-cartodb/issues/133 - it("MapConfig with mapnik layer and no cartocss", function(done) { + it.skip("MapConfig with mapnik layer and no cartocss", function(done) { var layergroup = { version: '1.0.0', From 8270699b8ea1d32e32304210fbb00f9ec5e7cb2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 29 Nov 2017 13:12:09 +0100 Subject: [PATCH 03/30] Tests to fect mvt tiles without styles --- test/acceptance/multilayer.js | 4 ++ test/acceptance/mvt.js | 120 +++++++++++++++++++++++++++++++++- 2 files changed, 123 insertions(+), 1 deletion(-) diff --git a/test/acceptance/multilayer.js b/test/acceptance/multilayer.js index 51545416..a1a93e40 100644 --- a/test/acceptance/multilayer.js +++ b/test/acceptance/multilayer.js @@ -1163,6 +1163,10 @@ describe(suiteName, function() { ); }); + // WARN: MapConfig with mapnik layer and no cartocss it's valid since + // vector & raster aggregation project, now we can request MVT format w/o defining styles + // for the layer. + // See https://github.com/CartoDB/Windshaft-cartodb/issues/133 it.skip("MapConfig with mapnik layer and no cartocss", function(done) { diff --git a/test/acceptance/mvt.js b/test/acceptance/mvt.js index ded1772e..c1958ca5 100644 --- a/test/acceptance/mvt.js +++ b/test/acceptance/mvt.js @@ -34,6 +34,124 @@ return function () { serverOptions.renderer.mvt.usePostGIS = originalUsePostGIS; }); + describe('vector-layergroup', function () { + const POLYGONS_SQL = ` + select + st_buffer(st_setsrid(st_makepoint(x*10, x*10), 4326)::geography, 1000000)::geometry as the_geom, + st_transform( + st_buffer(st_setsrid(st_makepoint(x*10, x*10), 4326)::geography, 1000000)::geometry, + 3857 + ) as the_geom_webmercator, + x as value + from generate_series(-3, 3) x + `; + + const POINTS_SQL = ` + select + st_setsrid(st_makepoint(x*10, x*10), 4326) as the_geom, + st_transform(st_setsrid(st_makepoint(x*10, x*10), 4326), 3857) as the_geom_webmercator, + x as value + from generate_series(-3, 3) x + `; + + function createVectorLayergroup () { + return { + version: '1.6.0', + layers: [ + { + type: 'cartodb', + options: { + sql: POINTS_SQL + } + }, + { + type: 'cartodb', + options: { + sql: POLYGONS_SQL + } + } + ] + }; + } + + beforeEach(function () { + this.mapConfig = createVectorLayergroup(); + this.testClient = new TestClient(this.mapConfig); + }); + + afterEach(function (done) { + this.testClient.drain(done); + }); + + it('should get vector tiles from layergroup with layers w/o cartocss', function (done) { + this.testClient.getTile(0, 0, 0, { format: 'mvt' }, (err, res, tile) => { + 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'); + + const layer1 = JSON.parse(tile.toGeoJSONSync(1)); + + assert.equal(layer1.name, 'layer1'); + assert.equal(layer1.features[0].type, 'Feature'); + assert.equal(layer1.features[0].geometry.type, 'Polygon'); + done(); + }); + }); + + it('should get vector tiles from specific layer (layer0)', function (done) { + this.testClient.getTile(0, 0, 0, { format: 'mvt', layers: 0 }, (err, res, tile) => { + 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 layer = JSON.parse(tile.toGeoJSONSync(0)); + + assert.equal(layer.name, 'layer0'); + assert.equal(layer.features[0].type, 'Feature'); + assert.equal(layer.features[0].geometry.type, 'Point'); + + done(); + }); + }); + + it('should get vector tiles from specific layer (layer1)', function (done) { + this.testClient.getTile(0, 0, 0, { format: 'mvt', layers: 1 }, (err, res, tile) => { + 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 layer = JSON.parse(tile.toGeoJSONSync(0)); + + assert.equal(layer.name, 'layer1'); + assert.equal(layer.features[0].type, 'Feature'); + assert.equal(layer.features[0].geometry.type, 'Polygon'); + + done(); + }); + }); + }); + describe('analysis-layers-dataviews-mvt', function () { function createMapConfig(layers, dataviews, analysis) { @@ -504,4 +622,4 @@ return function () { } }; -} \ No newline at end of file +} From 330f8f3cb514f5494159694c9a4dcd2caf2c4a17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 29 Nov 2017 14:10:56 +0100 Subject: [PATCH 04/30] Test invalid format for vector layergroup --- test/acceptance/mvt.js | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/acceptance/mvt.js b/test/acceptance/mvt.js index c1958ca5..556f0638 100644 --- a/test/acceptance/mvt.js +++ b/test/acceptance/mvt.js @@ -150,6 +150,37 @@ return function () { done(); }); }); + + it.only('should fail when the format requested is not mvt', function (done) { + const options = { + format: 'png', + response: { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + + this.testClient.getTile(0, 0, 0, options, (err, res, body) => { + if (err) { + return done(err); + } + + assert.deepEqual(body, { + "errors": [ + "Invalid format: only mvt format is available for layers without CartoCSS defined" + ], + "errors_with_context":[ + { + "type":"tile", + "message":"Invalid format: only mvt format is available for layers without CartoCSS defined" + } + ] + }); + done(); + }); + }); }); describe('analysis-layers-dataviews-mvt', function () { From f9428682f936f6bc59ff5e0db8e110127440a277 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 29 Nov 2017 14:38:06 +0100 Subject: [PATCH 05/30] Remove mocha filter --- test/acceptance/mvt.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/mvt.js b/test/acceptance/mvt.js index 556f0638..0c566006 100644 --- a/test/acceptance/mvt.js +++ b/test/acceptance/mvt.js @@ -151,7 +151,7 @@ return function () { }); }); - it.only('should fail when the format requested is not mvt', function (done) { + it('should fail when the format requested is not mvt', function (done) { const options = { format: 'png', response: { From c9e0f330c0e3771594355235ba5143c9005d38a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 29 Nov 2017 16:15:52 +0100 Subject: [PATCH 06/30] Add test to check incompatible layers --- test/acceptance/mvt.js | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/test/acceptance/mvt.js b/test/acceptance/mvt.js index 0c566006..25e32308 100644 --- a/test/acceptance/mvt.js +++ b/test/acceptance/mvt.js @@ -181,6 +181,38 @@ return function () { done(); }); }); + + it('should fail when the map-config mix layers with and without cartocss', function (done) { + const response = { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; + + this.testClient.mapConfig.layers[0].options.cartocss = '#layer0 { marker-fill: red; marker-width: 10; }'; + this.testClient.mapConfig.layers[0].options.cartocss_version = '2.3.0'; + this.testClient.getLayergroup(response, (err, body) => { + if (err) { + return done(err); + } + + assert.deepEqual(body, { + "errors": [ + "The layergroup contains incompatible layers: don't mix styled layers with non styled layers (without cartocss)" + ], + "errors_with_context":[ + { + "type":"layergroup", + "message": "The layergroup contains incompatible layers: don't mix styled layers with non styled layers (without cartocss)" + } + ] + }); + + done(); + }); + }); + }); describe('analysis-layers-dataviews-mvt', function () { From f41af41bd497ddf5e1735a3b531b5767bd80493b Mon Sep 17 00:00:00 2001 From: Mario de Frutos Date: Wed, 29 Nov 2017 16:32:22 +0100 Subject: [PATCH 07/30] Update to turbo-carto 0.20.2 (#805) --- NEWS.md | 6 ++++++ package.json | 2 +- yarn.lock | 16 ++++++++-------- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/NEWS.md b/NEWS.md index 409b5ebc..7748be37 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,11 @@ # Changelog +## 4.1.1 +Released 2017-mm-dd + +Announcements: + - Upgrades turbo-carto to [0.20.2](https://github.com/CartoDB/turbo-carto/releases/tag/0.20.2). + ## 4.1.0 Released 2017-mm-dd diff --git a/package.json b/package.json index 8e4c1d84..43efe416 100644 --- a/package.json +++ b/package.json @@ -42,7 +42,7 @@ "semver": "~5.3.0", "step": "~0.0.6", "step-profiler": "~0.3.0", - "turbo-carto": "0.20.1", + "turbo-carto": "0.20.2", "underscore": "~1.6.0", "windshaft": "4.0.1", "yargs": "~5.0.0" diff --git a/yarn.lock b/yarn.lock index 7b5394aa..7312213c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2,7 +2,7 @@ # yarn lockfile v1 -"abaculus@github:cartodb/abaculus#2.0.3-cdb1": +abaculus@cartodb/abaculus#2.0.3-cdb1: version "2.0.3-cdb1" resolved "https://codeload.github.com/cartodb/abaculus/tar.gz/f5f34e1c80cdd8d49edd1d6fe3b2220ab2e23aaf" dependencies: @@ -222,7 +222,7 @@ camshaft@0.59.4: dot "^1.0.3" request "^2.69.0" -"canvas@github:cartodb/node-canvas#1.6.2-cdb2": +canvas@cartodb/node-canvas#1.6.2-cdb2: version "1.6.2-cdb2" resolved "https://codeload.github.com/cartodb/node-canvas/tar.gz/8acf04557005c633f9e68524488a2657c04f3766" dependencies: @@ -248,7 +248,7 @@ carto@CartoDB/carto#0.15.1-cdb1: optimist "~0.6.0" underscore "~1.6.0" -"carto@github:cartodb/carto#0.15.1-cdb3": +carto@cartodb/carto#0.15.1-cdb3: version "0.15.1-cdb3" resolved "https://codeload.github.com/cartodb/carto/tar.gz/945f5efb74fd1af1f5e1f69f409f9567f94fb5a7" dependencies: @@ -2145,7 +2145,7 @@ through@2: version "2.3.8" resolved "https://registry.yarnpkg.com/through/-/through-2.3.8.tgz#0dd4c9ffaabc357960b1b724115d7e0e86a2e1f5" -"tilelive-bridge@github:cartodb/tilelive-bridge#2.3.1-cdb4": +tilelive-bridge@cartodb/tilelive-bridge#2.3.1-cdb4: version "2.3.1-cdb4" resolved "https://codeload.github.com/cartodb/tilelive-bridge/tar.gz/faa2b638da2d119b78281575d40255cb523f6ca6" dependencies: @@ -2153,7 +2153,7 @@ through@2: mapnik-pool "~0.1.3" sphericalmercator "1.0.x" -"tilelive-mapnik@github:cartodb/tilelive-mapnik#0.6.18-cdb3": +tilelive-mapnik@cartodb/tilelive-mapnik#0.6.18-cdb3: version "0.6.18-cdb3" resolved "https://codeload.github.com/cartodb/tilelive-mapnik/tar.gz/23bd1c31dd57d0b76c86b9f1eaf62462b3c17d01" dependencies: @@ -2190,9 +2190,9 @@ tunnel-agent@^0.6.0: dependencies: safe-buffer "^5.0.1" -turbo-carto@0.20.1: - version "0.20.1" - resolved "https://registry.yarnpkg.com/turbo-carto/-/turbo-carto-0.20.1.tgz#e9f5fa1408d9d4325a1e79333e6d242170f89e6d" +turbo-carto@0.20.2: + version "0.20.2" + resolved "https://registry.yarnpkg.com/turbo-carto/-/turbo-carto-0.20.2.tgz#2b737597a65c2918432f70ea414f12fbec2b6a6f" dependencies: cartocolor "4.0.0" colorbrewer "1.0.0" From a20e789302cf997282f773c71662ae692c02bf15 Mon Sep 17 00:00:00 2001 From: Mario de Frutos Date: Wed, 29 Nov 2017 16:34:48 +0100 Subject: [PATCH 08/30] Release 4.1.1 --- NEWS.md | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index 7748be37..e38e1db6 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,7 +1,7 @@ # Changelog ## 4.1.1 -Released 2017-mm-dd +Released 2017-11-29 Announcements: - Upgrades turbo-carto to [0.20.2](https://github.com/CartoDB/turbo-carto/releases/tag/0.20.2). diff --git a/package.json b/package.json index 43efe416..75de7f0b 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "private": true, "name": "windshaft-cartodb", - "version": "4.0.2", + "version": "4.1.1", "description": "A map tile server for CartoDB", "keywords": [ "cartodb" From 1b6de9961a15dac8eb399df39afecc826c2541a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 29 Nov 2017 16:47:56 +0100 Subject: [PATCH 09/30] Do not use polygons --- test/acceptance/mvt.js | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/test/acceptance/mvt.js b/test/acceptance/mvt.js index 25e32308..3953d3b4 100644 --- a/test/acceptance/mvt.js +++ b/test/acceptance/mvt.js @@ -35,21 +35,18 @@ return function () { }); describe('vector-layergroup', function () { - const POLYGONS_SQL = ` + const POINTS_SQL_1 = ` select - st_buffer(st_setsrid(st_makepoint(x*10, x*10), 4326)::geography, 1000000)::geometry as the_geom, - st_transform( - st_buffer(st_setsrid(st_makepoint(x*10, x*10), 4326)::geography, 1000000)::geometry, - 3857 - ) as the_geom_webmercator, + st_setsrid(st_makepoint(x*10, x*10), 4326) as the_geom, + st_transform(st_setsrid(st_makepoint(x*10, x*10), 4326), 3857) as the_geom_webmercator, x as value from generate_series(-3, 3) x `; - const POINTS_SQL = ` + const POINTS_SQL_2 = ` select - st_setsrid(st_makepoint(x*10, x*10), 4326) as the_geom, - st_transform(st_setsrid(st_makepoint(x*10, x*10), 4326), 3857) as the_geom_webmercator, + st_setsrid(st_makepoint(x*10, x*10*(-1)), 4326) as the_geom, + st_transform(st_setsrid(st_makepoint(x*10, x*10*(-1)), 4326), 3857) as the_geom_webmercator, x as value from generate_series(-3, 3) x `; @@ -61,13 +58,13 @@ return function () { { type: 'cartodb', options: { - sql: POINTS_SQL + sql: POINTS_SQL_1 } }, { type: 'cartodb', options: { - sql: POLYGONS_SQL + sql: POINTS_SQL_2 } } ] @@ -104,7 +101,7 @@ return function () { assert.equal(layer1.name, 'layer1'); assert.equal(layer1.features[0].type, 'Feature'); - assert.equal(layer1.features[0].geometry.type, 'Polygon'); + assert.equal(layer1.features[0].geometry.type, 'Point'); done(); }); }); @@ -145,7 +142,7 @@ return function () { assert.equal(layer.name, 'layer1'); assert.equal(layer.features[0].type, 'Feature'); - assert.equal(layer.features[0].geometry.type, 'Polygon'); + assert.equal(layer.features[0].geometry.type, 'Point'); done(); }); From cded5afdcb23b9f5f4bb034f861ba01c1628c5cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 29 Nov 2017 16:56:28 +0100 Subject: [PATCH 10/30] Ahh! jshint.. my old friend --- test/acceptance/mvt.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/acceptance/mvt.js b/test/acceptance/mvt.js index 3953d3b4..424ac95e 100644 --- a/test/acceptance/mvt.js +++ b/test/acceptance/mvt.js @@ -196,12 +196,14 @@ return function () { assert.deepEqual(body, { "errors": [ - "The layergroup contains incompatible layers: don't mix styled layers with non styled layers (without cartocss)" + "The layergroup contains incompatible layers: " + + "don't mix styled layers with non styled layers (without cartocss)" ], "errors_with_context":[ { "type":"layergroup", - "message": "The layergroup contains incompatible layers: don't mix styled layers with non styled layers (without cartocss)" + "message": "The layergroup contains incompatible layers: " + + "don't mix styled layers with non styled layers (without cartocss)" } ] }); From 0ba49753600bd73a6532f52fddba7b287d4503fb Mon Sep 17 00:00:00 2001 From: Mario de Frutos Date: Wed, 29 Nov 2017 18:02:46 +0100 Subject: [PATCH 11/30] Bump next version --- NEWS.md | 4 ++++ package.json | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index e38e1db6..0aa1535d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,9 @@ # Changelog +## 4.1.2 +Released 2017-mm-dd + + ## 4.1.1 Released 2017-11-29 diff --git a/package.json b/package.json index 75de7f0b..0ea64507 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "private": true, "name": "windshaft-cartodb", - "version": "4.1.1", + "version": "4.1.2", "description": "A map tile server for CartoDB", "keywords": [ "cartodb" From 496778c276b99af4c547378ccfeb5506221aae8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 29 Nov 2017 18:17:51 +0100 Subject: [PATCH 12/30] Separate suites --- test/acceptance/vector-layergroup.js | 218 +++++++++++++++++++++++++++ 1 file changed, 218 insertions(+) create mode 100644 test/acceptance/vector-layergroup.js diff --git a/test/acceptance/vector-layergroup.js b/test/acceptance/vector-layergroup.js new file mode 100644 index 00000000..8e811197 --- /dev/null +++ b/test/acceptance/vector-layergroup.js @@ -0,0 +1,218 @@ +require('../support/test_helper'); + +const assert = require('../support/assert'); +const TestClient = require('../support/test-client'); +const serverOptions = require('../../lib/cartodb/server_options'); + + +const POINTS_SQL_1 = ` +select + st_setsrid(st_makepoint(x*10, x*10), 4326) as the_geom, + st_transform(st_setsrid(st_makepoint(x*10, x*10), 4326), 3857) as the_geom_webmercator, + x as value +from generate_series(-3, 3) x +`; + +const POINTS_SQL_2 = ` +select + st_setsrid(st_makepoint(x*10, x*10*(-1)), 4326) as the_geom, + st_transform(st_setsrid(st_makepoint(x*10, x*10*(-1)), 4326), 3857) as the_geom_webmercator, + x as value +from generate_series(-3, 3) x +`; + +function createVectorLayergroup () { + return { + version: '1.6.0', + layers: [ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1 + } + }, + { + type: 'cartodb', + options: { + sql: POINTS_SQL_2 + } + } + ] + }; +} + +const INCOMPATIBLE_LAYERS_ERROR = { + "errors": [ + "The layergroup contains incompatible layers: " + + "don't mix styled layers with non styled layers (without cartocss)" + ], + "errors_with_context":[ + { + "type":"layergroup", + "message": "The layergroup contains incompatible layers: " + + "don't mix styled layers with non styled layers (without cartocss)" + } + ] +}; + +const INVALID_FORMAT_ERROR = { + "errors": [ + "Invalid format: only mvt format is available for layers without CartoCSS defined" + ], + "errors_with_context":[ + { + "type": "tile", + "message": "Invalid format:" + + " only mvt format is available for layers without CartoCSS defined" + } + ] +}; + +const suites = [{ + desc: 'mvt (mapnik)', + usePostGIS: false +}]; + +if (process.env.POSTGIS_VERSION === '2.4') { + suites.push({ + desc: 'mvt (postgis)', + usePostGIS: true + }); +} + +suites.forEach((suite) => { + const { desc, usePostGIS } = suite; + + describe(desc, function () { + const originalUsePostGIS = serverOptions.renderer.mvt.usePostGIS; + + before(function () { + serverOptions.renderer.mvt.usePostGIS = usePostGIS; + }); + + after(function (){ + serverOptions.renderer.mvt.usePostGIS = originalUsePostGIS; + }); + + describe('vector-layergroup', function () { + beforeEach(function () { + this.mapConfig = createVectorLayergroup(); + this.testClient = new TestClient(this.mapConfig); + }); + + afterEach(function (done) { + this.testClient.drain(done); + }); + + it('should get vector tiles from layergroup with layers w/o cartocss', function (done) { + this.testClient.getTile(0, 0, 0, { format: 'mvt' }, (err, res, tile) => { + 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'); + + const layer1 = JSON.parse(tile.toGeoJSONSync(1)); + + assert.equal(layer1.name, 'layer1'); + assert.equal(layer1.features[0].type, 'Feature'); + assert.equal(layer1.features[0].geometry.type, 'Point'); + done(); + }); + }); + + it('should get vector tiles from specific layer (layer0)', function (done) { + this.testClient.getTile(0, 0, 0, { format: 'mvt', layers: 0 }, (err, res, tile) => { + 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 layer = JSON.parse(tile.toGeoJSONSync(0)); + + assert.equal(layer.name, 'layer0'); + assert.equal(layer.features[0].type, 'Feature'); + assert.equal(layer.features[0].geometry.type, 'Point'); + + done(); + }); + }); + + it('should get vector tiles from specific layer (layer1)', function (done) { + this.testClient.getTile(0, 0, 0, { format: 'mvt', layers: 1 }, (err, res, tile) => { + 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 layer = JSON.parse(tile.toGeoJSONSync(0)); + + assert.equal(layer.name, 'layer1'); + assert.equal(layer.features[0].type, 'Feature'); + assert.equal(layer.features[0].geometry.type, 'Point'); + + done(); + }); + }); + + it('should fail when the format requested is not mvt', function (done) { + const options = { + format: 'png', + response: { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + + this.testClient.getTile(0, 0, 0, options, (err, res, body) => { + if (err) { + return done(err); + } + + assert.deepEqual(body, INVALID_FORMAT_ERROR); + done(); + }); + }); + + it('should fail when the map-config mix layers with and without cartocss', function (done) { + const response = { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; + + this.testClient.mapConfig.layers[0].options.cartocss = '#layer0 { marker-fill: red; marker-width: 10; }'; + this.testClient.mapConfig.layers[0].options.cartocss_version = '2.3.0'; + this.testClient.getLayergroup(response, (err, body) => { + if (err) { + return done(err); + } + + assert.deepEqual(body, INCOMPATIBLE_LAYERS_ERROR); + + done(); + }); + }); + }); + }); +}); From e18e86f565ee27175ec7217442834e9a7e99abb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 29 Nov 2017 18:28:42 +0100 Subject: [PATCH 13/30] jshint, jshint, jshint --- test/acceptance/vector-layergroup.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/acceptance/vector-layergroup.js b/test/acceptance/vector-layergroup.js index 8e811197..794f8fbc 100644 --- a/test/acceptance/vector-layergroup.js +++ b/test/acceptance/vector-layergroup.js @@ -201,8 +201,12 @@ suites.forEach((suite) => { } }; - this.testClient.mapConfig.layers[0].options.cartocss = '#layer0 { marker-fill: red; marker-width: 10; }'; - this.testClient.mapConfig.layers[0].options.cartocss_version = '2.3.0'; + const cartocss = `#layer0 { marker-fill: red; marker-width: 10; }`; + const cartocssVersion = '2.3.0'; + + this.testClient.mapConfig.layers[0].options.cartocss = cartocss; + this.testClient.mapConfig.layers[0].options.cartocss_version = cartocssVersion; + this.testClient.getLayergroup(response, (err, body) => { if (err) { return done(err); From 23c0cb757d84d8e354ae3f70b1caf7c6e571a69f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 1 Dec 2017 13:52:28 +0100 Subject: [PATCH 14/30] Fix tests according to the last changes in windshaft --- test/acceptance/mvt.js | 14 +++++++------- test/acceptance/vector-layergroup.js | 15 +++++++-------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/test/acceptance/mvt.js b/test/acceptance/mvt.js index 424ac95e..25544d5e 100644 --- a/test/acceptance/mvt.js +++ b/test/acceptance/mvt.js @@ -166,12 +166,12 @@ return function () { assert.deepEqual(body, { "errors": [ - "Invalid format: only mvt format is available for layers without CartoCSS defined" + "Unsupported format: 'cartocss' option is missing for png" ], "errors_with_context":[ { "type":"tile", - "message":"Invalid format: only mvt format is available for layers without CartoCSS defined" + "message":"Unsupported format: 'cartocss' option is missing for png" } ] }); @@ -196,14 +196,14 @@ return function () { assert.deepEqual(body, { "errors": [ - "The layergroup contains incompatible layers: " + - "don't mix styled layers with non styled layers (without cartocss)" + "The `mapnik` or `cartodb` layers must be consistent:" + + " `cartocss` option is either present or voided in all layers. Mixing is not allowed." ], "errors_with_context":[ { - "type":"layergroup", - "message": "The layergroup contains incompatible layers: " + - "don't mix styled layers with non styled layers (without cartocss)" + "type":"mapconfig", + "message": "The `mapnik` or `cartodb` layers must be consistent:" + + " `cartocss` option is either present or voided in all layers. Mixing is not allowed." } ] }); diff --git a/test/acceptance/vector-layergroup.js b/test/acceptance/vector-layergroup.js index 794f8fbc..54ec29ef 100644 --- a/test/acceptance/vector-layergroup.js +++ b/test/acceptance/vector-layergroup.js @@ -43,27 +43,26 @@ function createVectorLayergroup () { const INCOMPATIBLE_LAYERS_ERROR = { "errors": [ - "The layergroup contains incompatible layers: " + - "don't mix styled layers with non styled layers (without cartocss)" + "The `mapnik` or `cartodb` layers must be consistent:" + + " `cartocss` option is either present or voided in all layers. Mixing is not allowed." ], "errors_with_context":[ { - "type":"layergroup", - "message": "The layergroup contains incompatible layers: " + - "don't mix styled layers with non styled layers (without cartocss)" + "type":"mapconfig", + "message": "The `mapnik` or `cartodb` layers must be consistent:" + + " `cartocss` option is either present or voided in all layers. Mixing is not allowed." } ] }; const INVALID_FORMAT_ERROR = { "errors": [ - "Invalid format: only mvt format is available for layers without CartoCSS defined" + "Unsupported format: 'cartocss' option is missing for png" ], "errors_with_context":[ { "type": "tile", - "message": "Invalid format:" + - " only mvt format is available for layers without CartoCSS defined" + "message": "Unsupported format: 'cartocss' option is missing for png" } ] }; From 169b95809a830b69f6472fab0c402d48ae886325 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 4 Dec 2017 13:06:45 +0100 Subject: [PATCH 15/30] Upgrade windshaft to version 4.1.0 --- package.json | 2 +- yarn.lock | 38 +++++++++++++++++++------------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/package.json b/package.json index 3cec519f..4f3a2546 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,7 @@ "step-profiler": "~0.3.0", "turbo-carto": "0.20.2", "underscore": "~1.6.0", - "windshaft": "cartodb/windshaft#vr-aggretation", + "windshaft": "4.1.0", "yargs": "~5.0.0" }, "devDependencies": { diff --git a/yarn.lock b/yarn.lock index d58f2157..92315ec1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2,7 +2,7 @@ # yarn lockfile v1 -abaculus@cartodb/abaculus#2.0.3-cdb1: +"abaculus@github:cartodb/abaculus#2.0.3-cdb1": version "2.0.3-cdb1" resolved "https://codeload.github.com/cartodb/abaculus/tar.gz/f5f34e1c80cdd8d49edd1d6fe3b2220ab2e23aaf" dependencies: @@ -222,7 +222,7 @@ camshaft@0.59.4: dot "^1.0.3" request "^2.69.0" -canvas@cartodb/node-canvas#1.6.2-cdb2: +"canvas@github:cartodb/node-canvas#1.6.2-cdb2": version "1.6.2-cdb2" resolved "https://codeload.github.com/cartodb/node-canvas/tar.gz/8acf04557005c633f9e68524488a2657c04f3766" dependencies: @@ -240,14 +240,6 @@ carto@0.16.3: semver "^5.1.0" yargs "^4.2.0" -carto@cartodb/carto#0.15.1-cdb3: - version "0.15.1-cdb3" - resolved "https://codeload.github.com/cartodb/carto/tar.gz/945f5efb74fd1af1f5e1f69f409f9567f94fb5a7" - dependencies: - mapnik-reference "~6.0.2" - 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" @@ -256,6 +248,14 @@ carto@cartodb/carto#0.15.1-cdb3: optimist "~0.6.0" underscore "~1.6.0" +"carto@github:cartodb/carto#0.15.1-cdb3": + version "0.15.1-cdb3" + resolved "https://codeload.github.com/cartodb/carto/tar.gz/945f5efb74fd1af1f5e1f69f409f9567f94fb5a7" + dependencies: + mapnik-reference "~6.0.2" + optimist "~0.6.0" + underscore "1.8.3" + cartocolor@4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/cartocolor/-/cartocolor-4.0.0.tgz#841a3222d8b5b22718d9d545b1e5b972cb26eb36" @@ -826,9 +826,9 @@ graceful-fs@^4.1.2: version "1.0.1" resolved "https://registry.yarnpkg.com/graceful-readlink/-/graceful-readlink-1.0.1.tgz#4cafad76bc62f02fa039b2f94e9a3dd3a391a725" -grainstore@~1.6.0: - version "1.6.4" - resolved "https://registry.yarnpkg.com/grainstore/-/grainstore-1.6.4.tgz#617b93c5e2de8f544375202da89b9208a8b3d762" +grainstore@1.7.0: + version "1.7.0" + resolved "https://registry.yarnpkg.com/grainstore/-/grainstore-1.7.0.tgz#28d78895c82e6201f7d0ff63af1056f3c0fda0d3" dependencies: carto "0.16.3" debug "~3.1.0" @@ -2190,7 +2190,7 @@ through@2: version "2.3.8" resolved "https://registry.yarnpkg.com/through/-/through-2.3.8.tgz#0dd4c9ffaabc357960b1b724115d7e0e86a2e1f5" -tilelive-bridge@cartodb/tilelive-bridge#2.3.1-cdb4: +"tilelive-bridge@github:cartodb/tilelive-bridge#2.3.1-cdb4": version "2.3.1-cdb4" resolved "https://codeload.github.com/cartodb/tilelive-bridge/tar.gz/faa2b638da2d119b78281575d40255cb523f6ca6" dependencies: @@ -2198,7 +2198,7 @@ tilelive-bridge@cartodb/tilelive-bridge#2.3.1-cdb4: mapnik-pool "~0.1.3" sphericalmercator "1.0.x" -tilelive-mapnik@cartodb/tilelive-mapnik#0.6.18-cdb3: +"tilelive-mapnik@github:cartodb/tilelive-mapnik#0.6.18-cdb3": version "0.6.18-cdb3" resolved "https://codeload.github.com/cartodb/tilelive-mapnik/tar.gz/23bd1c31dd57d0b76c86b9f1eaf62462b3c17d01" dependencies: @@ -2359,9 +2359,9 @@ window-size@^0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/window-size/-/window-size-0.2.0.tgz#b4315bb4214a3d7058ebeee892e13fa24d98b075" -windshaft@cartodb/windshaft#vr-aggretation: - version "4.0.2" - resolved "https://codeload.github.com/cartodb/windshaft/tar.gz/e0cbbafdf7a65537337c3ca185b6fd2f9bac4e59" +windshaft@^4.1.0: + version "4.1.0" + resolved "https://registry.yarnpkg.com/windshaft/-/windshaft-4.1.0.tgz#dc17c8369570c305171d1ab5ca130369bba04d58" dependencies: abaculus cartodb/abaculus#2.0.3-cdb1 canvas cartodb/node-canvas#1.6.2-cdb2 @@ -2369,7 +2369,7 @@ windshaft@cartodb/windshaft#vr-aggretation: cartodb-psql "^0.10.1" debug "^3.1.0" dot "~1.0.2" - grainstore "~1.6.0" + grainstore "1.7.0" mapnik "3.5.14" queue-async "~1.0.7" redis-mpool "0.4.1" From 0c8a31fad90d25f3c27789e2d818d076bee417d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 4 Dec 2017 13:17:37 +0100 Subject: [PATCH 16/30] Remove duplicated code --- test/acceptance/mvt.js | 180 ----------------------------------------- 1 file changed, 180 deletions(-) diff --git a/test/acceptance/mvt.js b/test/acceptance/mvt.js index 25544d5e..fa9189f2 100644 --- a/test/acceptance/mvt.js +++ b/test/acceptance/mvt.js @@ -34,186 +34,6 @@ return function () { serverOptions.renderer.mvt.usePostGIS = originalUsePostGIS; }); - describe('vector-layergroup', function () { - const POINTS_SQL_1 = ` - select - st_setsrid(st_makepoint(x*10, x*10), 4326) as the_geom, - st_transform(st_setsrid(st_makepoint(x*10, x*10), 4326), 3857) as the_geom_webmercator, - x as value - from generate_series(-3, 3) x - `; - - const POINTS_SQL_2 = ` - select - st_setsrid(st_makepoint(x*10, x*10*(-1)), 4326) as the_geom, - st_transform(st_setsrid(st_makepoint(x*10, x*10*(-1)), 4326), 3857) as the_geom_webmercator, - x as value - from generate_series(-3, 3) x - `; - - function createVectorLayergroup () { - return { - version: '1.6.0', - layers: [ - { - type: 'cartodb', - options: { - sql: POINTS_SQL_1 - } - }, - { - type: 'cartodb', - options: { - sql: POINTS_SQL_2 - } - } - ] - }; - } - - beforeEach(function () { - this.mapConfig = createVectorLayergroup(); - this.testClient = new TestClient(this.mapConfig); - }); - - afterEach(function (done) { - this.testClient.drain(done); - }); - - it('should get vector tiles from layergroup with layers w/o cartocss', function (done) { - this.testClient.getTile(0, 0, 0, { format: 'mvt' }, (err, res, tile) => { - 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'); - - const layer1 = JSON.parse(tile.toGeoJSONSync(1)); - - assert.equal(layer1.name, 'layer1'); - assert.equal(layer1.features[0].type, 'Feature'); - assert.equal(layer1.features[0].geometry.type, 'Point'); - done(); - }); - }); - - it('should get vector tiles from specific layer (layer0)', function (done) { - this.testClient.getTile(0, 0, 0, { format: 'mvt', layers: 0 }, (err, res, tile) => { - 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 layer = JSON.parse(tile.toGeoJSONSync(0)); - - assert.equal(layer.name, 'layer0'); - assert.equal(layer.features[0].type, 'Feature'); - assert.equal(layer.features[0].geometry.type, 'Point'); - - done(); - }); - }); - - it('should get vector tiles from specific layer (layer1)', function (done) { - this.testClient.getTile(0, 0, 0, { format: 'mvt', layers: 1 }, (err, res, tile) => { - 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 layer = JSON.parse(tile.toGeoJSONSync(0)); - - assert.equal(layer.name, 'layer1'); - assert.equal(layer.features[0].type, 'Feature'); - assert.equal(layer.features[0].geometry.type, 'Point'); - - done(); - }); - }); - - it('should fail when the format requested is not mvt', function (done) { - const options = { - format: 'png', - response: { - status: 400, - headers: { - 'Content-Type': 'application/json; charset=utf-8' - } - } - }; - - this.testClient.getTile(0, 0, 0, options, (err, res, body) => { - if (err) { - return done(err); - } - - assert.deepEqual(body, { - "errors": [ - "Unsupported format: 'cartocss' option is missing for png" - ], - "errors_with_context":[ - { - "type":"tile", - "message":"Unsupported format: 'cartocss' option is missing for png" - } - ] - }); - done(); - }); - }); - - it('should fail when the map-config mix layers with and without cartocss', function (done) { - const response = { - status: 400, - headers: { - 'Content-Type': 'application/json; charset=utf-8' - } - }; - - this.testClient.mapConfig.layers[0].options.cartocss = '#layer0 { marker-fill: red; marker-width: 10; }'; - this.testClient.mapConfig.layers[0].options.cartocss_version = '2.3.0'; - this.testClient.getLayergroup(response, (err, body) => { - if (err) { - return done(err); - } - - assert.deepEqual(body, { - "errors": [ - "The `mapnik` or `cartodb` layers must be consistent:" + - " `cartocss` option is either present or voided in all layers. Mixing is not allowed." - ], - "errors_with_context":[ - { - "type":"mapconfig", - "message": "The `mapnik` or `cartodb` layers must be consistent:" + - " `cartocss` option is either present or voided in all layers. Mixing is not allowed." - } - ] - }); - - done(); - }); - }); - - }); - describe('analysis-layers-dataviews-mvt', function () { function createMapConfig(layers, dataviews, analysis) { From 565cfb7fbe8e09bab95fe91c6e44f3c438bd32e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 4 Dec 2017 13:23:09 +0100 Subject: [PATCH 17/30] Update NEWS --- NEWS.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/NEWS.md b/NEWS.md index e38e1db6..e8086f24 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,11 +1,21 @@ # Changelog +## 4.2.0 +Released 2017-mm-dd + +Announcements: + + - Allow to request MVT tiles without CartoCSS + - Upgrades windshaft to [4.1.0](https://github.com/CartoDB/windshaft/releases/tag/4.1.0). + + ## 4.1.1 Released 2017-11-29 Announcements: - Upgrades turbo-carto to [0.20.2](https://github.com/CartoDB/turbo-carto/releases/tag/0.20.2). + ## 4.1.0 Released 2017-mm-dd From 893b886a1e66d2e85cd8a5055b02f466ad6be006 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 4 Dec 2017 13:34:02 +0100 Subject: [PATCH 18/30] Release 4.2.0 --- NEWS.md | 3 +-- package.json | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/NEWS.md b/NEWS.md index e8086f24..b4899cbe 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,10 +1,9 @@ # Changelog ## 4.2.0 -Released 2017-mm-dd +Released 2017-12-04 Announcements: - - Allow to request MVT tiles without CartoCSS - Upgrades windshaft to [4.1.0](https://github.com/CartoDB/windshaft/releases/tag/4.1.0). diff --git a/package.json b/package.json index 4f823d55..b4b917d6 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "private": true, "name": "windshaft-cartodb", - "version": "4.1.2", + "version": "4.2.0", "description": "A map tile server for CartoDB", "keywords": [ "cartodb" From 16a36a9d7a8d5df18a72040df79dfbf8ee2005cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 4 Dec 2017 13:35:23 +0100 Subject: [PATCH 19/30] Stubs next version --- NEWS.md | 4 ++++ package.json | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index b4899cbe..84cfb4bf 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,9 @@ # Changelog +## 4.2.1 +Released 2017-mm-dd + + ## 4.2.0 Released 2017-12-04 diff --git a/package.json b/package.json index b4b917d6..0a6a9014 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "private": true, "name": "windshaft-cartodb", - "version": "4.2.0", + "version": "4.2.1", "description": "A map tile server for CartoDB", "keywords": [ "cartodb" From 90c9ad18e056c73d800cc2c7f51827fa0aaf115d Mon Sep 17 00:00:00 2001 From: Raul Marin Date: Tue, 28 Nov 2017 17:51:28 +0100 Subject: [PATCH 20/30] Optimize histogram queries --- .../dataview/histograms/numeric-histogram.js | 225 ++++++++---------- 1 file changed, 101 insertions(+), 124 deletions(-) diff --git a/lib/cartodb/models/dataview/histograms/numeric-histogram.js b/lib/cartodb/models/dataview/histograms/numeric-histogram.js index ea191bea..0396e2f8 100644 --- a/lib/cartodb/models/dataview/histograms/numeric-histogram.js +++ b/lib/cartodb/models/dataview/histograms/numeric-histogram.js @@ -1,44 +1,42 @@ const BaseHistogram = require('./base-histogram'); const debug = require('debug')('windshaft:dataview:numeric-histogram'); +/** Cast the column column to epoch */ const columnCastTpl = ctx => `date_part('epoch', ${ctx.column})`; -const filterOutSpecialNumericValues = ctx => ` - ${ctx.column} != 'infinity'::float - AND - ${ctx.column} != '-infinity'::float - AND - ${ctx.column} != 'NaN'::float -`; +/** If the column type is float, ignore any non numeric result (infinity / NaN) */ +const handleFloatColum = ctx => `${!ctx.isFloatColumn ? `${ctx.column}` : + `nullif(nullif(nullif(${ctx.column}, 'infinity'::float), '-infinity'::float), 'NaN'::float)` +}`; -const filteredQueryTpl = ctx => ` +/** Count only infinity (positive and negative) appearances */ +const countInfinites = ctx => `${!ctx.isFloatColumn ? `0` : + `sum(CASE WHEN (${ctx.column} = 'infinity'::float OR ${ctx.column} = '-infinity'::float) THEN 1 ELSE 0 END)` +}`; + +/** Count only NaNs appearances*/ +const countNaNs = ctx => `${!ctx.isFloatColumn ? `0` : + `sum(CASE WHEN (${ctx.column} = 'NaN'::float) THEN 1 ELSE 0 END)` +}`; + +/** Query to get min and max values from the query */ +const irqQueryTpl = ctx => ` __cdb_filtered_source AS ( SELECT * FROM (${ctx.query}) __cdb_filtered_source_query - WHERE ${ctx.column} IS NOT NULL - ${ctx.isFloatColumn ? `AND ${filterOutSpecialNumericValues(ctx)}` : ''} - ) -`; - -const basicsQueryTpl = ctx => ` + WHERE ${handleFloatColum(ctx)} IS NOT NULL + ), __cdb_basics AS ( SELECT - max(${ctx.column}) AS __cdb_max_val, min(${ctx.column}) AS __cdb_min_val, - avg(${ctx.column}) AS __cdb_avg_val, count(1) AS __cdb_total_rows + max(${ctx.column}) AS __cdb_max_val, + min(${ctx.column}) AS __cdb_min_val, + count(1) AS __cdb_total_rows FROM __cdb_filtered_source ) `; -const overrideBasicsQueryTpl = ctx => ` - __cdb_basics AS ( - SELECT - max(${ctx.end}) AS __cdb_max_val, min(${ctx.start}) AS __cdb_min_val, - avg(${ctx.column}) AS __cdb_avg_val, count(1) AS __cdb_total_rows - FROM __cdb_filtered_source - ) -`; - -const iqrQueryTpl = ctx => ` +/* Query to calculate the number of bins (needs irqQueryTpl before it*/ +const binsQueryTpl = ctx => ` __cdb_iqrange AS ( SELECT max(quartile_max) - min(quartile_max) AS __cdb_iqr FROM ( @@ -49,10 +47,7 @@ const iqrQueryTpl = ctx => ` WHERE quartile = 1 or quartile = 3 GROUP BY quartile ) __cdb_iqr - ) -`; - -const binsQueryTpl = ctx => ` + ), __cdb_bins AS ( SELECT CASE WHEN __cdb_total_rows = 0 OR __cdb_iqr = 0 @@ -70,83 +65,6 @@ const binsQueryTpl = ctx => ` ) `; -const overrideBinsQueryTpl = ctx => ` - __cdb_bins AS ( - SELECT ${ctx.override.bins} AS __cdb_bins_number - ) -`; - -const nullsQueryTpl = ctx => ` - __cdb_nulls AS ( - SELECT - count(*) AS __cdb_nulls_count - FROM (${ctx.query}) __cdb_histogram_nulls - WHERE ${ctx.column} IS NULL - ) -`; - -const infinitiesQueryTpl = ctx => ` - __cdb_infinities AS ( - SELECT - count(*) AS __cdb_infinities_count - FROM (${ctx.query}) __cdb_infinities_query - WHERE - ${ctx.column} = 'infinity'::float - OR - ${ctx.column} = '-infinity'::float - ) -`; - -const nansQueryTpl = ctx => ` - __cdb_nans AS ( - SELECT - count(*) AS __cdb_nans_count - FROM (${ctx.query}) __cdb_nans_query - WHERE ${ctx.column} = 'NaN'::float - ) -`; - -const specialNumericValuesColumnDefinitionTpl = () => ` - __cdb_infinities_count AS infinities_count, - __cdb_nans_count AS nans_count -`; - -const specialNumericValuesCTETpl = () => ` - __cdb_infinities, __cdb_nans -`; - -const specialNumericValuesColumnTpl = () => ` - infinities_count, nans_count -`; - -const histogramQueryTpl = ctx => ` - SELECT - (__cdb_max_val - __cdb_min_val) / cast(__cdb_bins_number as float) AS bin_width, - __cdb_bins_number AS bins_number, - __cdb_nulls_count AS nulls_count, - ${ctx.isFloatColumn ? `${specialNumericValuesColumnDefinitionTpl()},` : ''} - __cdb_avg_val AS avg_val, - CASE WHEN __cdb_min_val = __cdb_max_val - THEN 0 - ELSE GREATEST( - 1, - LEAST( - WIDTH_BUCKET(${ctx.column}, __cdb_min_val, __cdb_max_val, __cdb_bins_number), - __cdb_bins_number - ) - ) - 1 - END AS bin, - min(${ctx.column})::numeric AS min, - max(${ctx.column})::numeric AS max, - avg(${ctx.column})::numeric AS avg, - count(*) AS freq - FROM __cdb_filtered_source, __cdb_basics, __cdb_nulls, __cdb_bins - ${ctx.isFloatColumn ? `, ${specialNumericValuesCTETpl()}` : ''} - GROUP BY bin, bins_number, bin_width, nulls_count, avg_val - ${ctx.isFloatColumn ? `, ${specialNumericValuesColumnTpl()}` : ''} - ORDER BY bin -`; - const BIN_MIN_NUMBER = 6; const BIN_MAX_NUMBER = 48; @@ -167,14 +85,14 @@ module.exports = class NumericHistogram extends BaseHistogram { _buildQuery (psql, override, callback) { const histogramSql = this._buildQueryTpl({ - override: override, column: this._columnType === 'date' ? columnCastTpl({ column: this.column }) : this.column, isFloatColumn: this._columnType === 'float', query: this.query, start: this._getBinStart(override), end: this._getBinEnd(override), + bins: this._getBinsCount(override), minBins: BIN_MIN_NUMBER, - maxBins: BIN_MAX_NUMBER, + maxBins: BIN_MAX_NUMBER }); debug(histogramSql); @@ -182,19 +100,61 @@ module.exports = class NumericHistogram extends BaseHistogram { return callback(null, histogramSql); } + +/** + * ctx: Object with the following values + * ctx.column -- Column for the histogram + * ctx.isFloatColumn - Whether the column is float or not + * ctx.query -- Subquery to extract data + * ctx.start -- Start value for the bins. [>= end to force calculation] + * ctx.end -- End value for the bins. + * ctx.bins -- Numbers of bins to generate [<0 to force calculation] + * ctx.minBins - If !full min bins to calculate [Optional] + * ctx.maxBins - If !full max bins to calculate [Optional] + */ _buildQueryTpl (ctx) { + var extra_tables = ``; + var extra_queries = ``; + var extra_groupby = ``; + + if (ctx.start >= ctx.end) { + ctx.end = `__cdb_basics.__cdb_max_val`; + ctx.start = `__cdb_basics.__cdb_min_val`; + extra_groupby = `, __cdb_basics.__cdb_max_val, __cdb_basics.__cdb_min_val`; + extra_tables = `, __cdb_basics`; + extra_queries = `WITH ${irqQueryTpl(ctx)}`; + } + + if (ctx.bins <= 0) { + ctx.bins = `__cdb_bins.__cdb_bins_number`; + extra_groupby += `, __cdb_bins.__cdb_bins_number`; + extra_tables += `, __cdb_bins`; + extra_queries = `WITH ${irqQueryTpl(ctx)}, ${binsQueryTpl(ctx)}`; + } + return ` - WITH - ${filteredQueryTpl(ctx)}, - ${this._hasOverridenRange(ctx.override) ? overrideBasicsQueryTpl(ctx) : basicsQueryTpl(ctx)}, - ${this._hasOverridenBins(ctx.override) ? - overrideBinsQueryTpl(ctx) : - `${iqrQueryTpl(ctx)}, ${binsQueryTpl(ctx)}` - }, - ${nullsQueryTpl(ctx)} - ${ctx.isFloatColumn ? `,${infinitiesQueryTpl(ctx)}, ${nansQueryTpl(ctx)}` : ''} - ${histogramQueryTpl(ctx)} - `; +${extra_queries} +SELECT + (${ctx.end} - ${ctx.start}) / ${ctx.bins}::float AS bin_width, + ${ctx.bins} as bins_number, + sum(CASE WHEN (${ctx.column} IS NULL) THEN 1 ELSE 0 END) AS nulls_count, + ${countInfinites(ctx)} AS infinities_count, + ${countNaNs(ctx)} AS nans_count, + min(${handleFloatColum(ctx)}) AS min, + max(${handleFloatColum(ctx)}) AS max, + avg(${handleFloatColum(ctx)}) AS avg, + sum(CASE WHEN (${handleFloatColum(ctx)} is not NULL) THEN 1 ELSE 0 END) as freq, + CASE WHEN ${ctx.start} = ${ctx.end} + THEN 0 + ELSE GREATEST(1, LEAST(${ctx.bins}, + WIDTH_BUCKET(${handleFloatColum(ctx)}, ${ctx.start}, ${ctx.end}, ${ctx.bins}))) - 1 + END AS bin +FROM +( + ${ctx.query} +) __cdb_filtered_source_query${extra_tables} +GROUP BY bin${extra_groupby} +ORDER BY bin;`; } _hasOverridenBins (override) { @@ -204,14 +164,31 @@ module.exports = class NumericHistogram extends BaseHistogram { _getSummary (result, override) { const firstRow = result.rows[0] || {}; + var total_nulls = 0; + var total_infinities = 0; + var total_nans = 0; + var total_avg = 0; + var total_count = 0; + + result.rows.forEach(function(row) { + total_nulls += row.nulls_count; + total_infinities += row.infinities_count; + total_nans += row.nans_count; + total_avg += row.avg * row.freq; + total_count += row.freq; + }); + if (total_count !== 0) { + total_avg /= total_count; + } + return { bin_width: firstRow.bin_width, bins_count: firstRow.bins_number, bins_start: this._populateBinStart(firstRow, override), - nulls: firstRow.nulls_count, - infinities: firstRow.infinities_count, - nans: firstRow.nans_count, - avg: firstRow.avg_val, + nulls: total_nulls, + infinities: total_infinities, + nans: total_nans, + avg: total_avg }; } From fcbf5ffcc5d542d4318129a1192b0e23bdcfa545 Mon Sep 17 00:00:00 2001 From: Raul Marin Date: Wed, 29 Nov 2017 13:07:59 +0100 Subject: [PATCH 21/30] Move sql helper functions to query-utils.js --- .../dataview/histograms/numeric-histogram.js | 42 ++++++------------- lib/cartodb/utils/query-utils.js | 31 ++++++++++++++ 2 files changed, 44 insertions(+), 29 deletions(-) diff --git a/lib/cartodb/models/dataview/histograms/numeric-histogram.js b/lib/cartodb/models/dataview/histograms/numeric-histogram.js index 0396e2f8..a2d5662d 100644 --- a/lib/cartodb/models/dataview/histograms/numeric-histogram.js +++ b/lib/cartodb/models/dataview/histograms/numeric-histogram.js @@ -1,30 +1,13 @@ const BaseHistogram = require('./base-histogram'); const debug = require('debug')('windshaft:dataview:numeric-histogram'); - -/** Cast the column column to epoch */ -const columnCastTpl = ctx => `date_part('epoch', ${ctx.column})`; - -/** If the column type is float, ignore any non numeric result (infinity / NaN) */ -const handleFloatColum = ctx => `${!ctx.isFloatColumn ? `${ctx.column}` : - `nullif(nullif(nullif(${ctx.column}, 'infinity'::float), '-infinity'::float), 'NaN'::float)` -}`; - -/** Count only infinity (positive and negative) appearances */ -const countInfinites = ctx => `${!ctx.isFloatColumn ? `0` : - `sum(CASE WHEN (${ctx.column} = 'infinity'::float OR ${ctx.column} = '-infinity'::float) THEN 1 ELSE 0 END)` -}`; - -/** Count only NaNs appearances*/ -const countNaNs = ctx => `${!ctx.isFloatColumn ? `0` : - `sum(CASE WHEN (${ctx.column} = 'NaN'::float) THEN 1 ELSE 0 END)` -}`; +const utils = require('../../../utils/query-utils'); /** Query to get min and max values from the query */ const irqQueryTpl = ctx => ` __cdb_filtered_source AS ( SELECT * FROM (${ctx.query}) __cdb_filtered_source_query - WHERE ${handleFloatColum(ctx)} IS NOT NULL + WHERE ${utils.handleFloatColumn(ctx)} IS NOT NULL ), __cdb_basics AS ( SELECT @@ -85,7 +68,7 @@ module.exports = class NumericHistogram extends BaseHistogram { _buildQuery (psql, override, callback) { const histogramSql = this._buildQueryTpl({ - column: this._columnType === 'date' ? columnCastTpl({ column: this.column }) : this.column, + column: this._columnType === 'date' ? utils.columnCastTpl({ column: this.column }) : this.column, isFloatColumn: this._columnType === 'float', query: this.query, start: this._getBinStart(override), @@ -137,17 +120,18 @@ ${extra_queries} SELECT (${ctx.end} - ${ctx.start}) / ${ctx.bins}::float AS bin_width, ${ctx.bins} as bins_number, - sum(CASE WHEN (${ctx.column} IS NULL) THEN 1 ELSE 0 END) AS nulls_count, - ${countInfinites(ctx)} AS infinities_count, - ${countNaNs(ctx)} AS nans_count, - min(${handleFloatColum(ctx)}) AS min, - max(${handleFloatColum(ctx)}) AS max, - avg(${handleFloatColum(ctx)}) AS avg, - sum(CASE WHEN (${handleFloatColum(ctx)} is not NULL) THEN 1 ELSE 0 END) as freq, + ${utils.countNULLs(ctx)} AS nulls_count, + ${utils.countInfinites(ctx)} AS infinities_count, + ${utils.countNaNs(ctx)} AS nans_count, + min(${utils.handleFloatColumn(ctx)}) AS min, + max(${utils.handleFloatColumn(ctx)}) AS max, + avg(${utils.handleFloatColumn(ctx)}) AS avg, + sum(CASE WHEN (${utils.handleFloatColumn(ctx)} is not NULL) THEN 1 ELSE 0 END) as freq, CASE WHEN ${ctx.start} = ${ctx.end} THEN 0 - ELSE GREATEST(1, LEAST(${ctx.bins}, - WIDTH_BUCKET(${handleFloatColum(ctx)}, ${ctx.start}, ${ctx.end}, ${ctx.bins}))) - 1 + ELSE GREATEST(1, LEAST( + ${ctx.bins}, + WIDTH_BUCKET(${utils.handleFloatColumn(ctx)}, ${ctx.start}, ${ctx.end}, ${ctx.bins}))) - 1 END AS bin FROM ( diff --git a/lib/cartodb/utils/query-utils.js b/lib/cartodb/utils/query-utils.js index 47d730f4..8b0c81ba 100644 --- a/lib/cartodb/utils/query-utils.js +++ b/lib/cartodb/utils/query-utils.js @@ -24,3 +24,34 @@ module.exports.extractTableNames = function extractTableNames(query) { module.exports.getQueryRowCount = function getQueryRowEstimation(query) { return 'select CDB_EstimateRowCount(\'' + query + '\') as rows'; }; + +/** Cast the column to epoch */ +module.exports.columnCastTpl = function columnCastTpl(ctx) { + return `date_part('epoch', ${ctx.column})`; +}; + +/** If the column type is float, ignore any non numeric result (infinity / NaN) */ +module.exports.handleFloatColumn = function handleFloatColumn(ctx) { + return `${!ctx.isFloatColumn ? `${ctx.column}` : + `nullif(nullif(nullif(${ctx.column}, 'infinity'::float), '-infinity'::float), 'NaN'::float)` + }`; +}; + +/** Count NULL appearances */ +module.exports.countNULLs= function countNULLs(ctx) { + return `sum(CASE WHEN (${ctx.column} IS NULL) THEN 1 ELSE 0 END)`; +}; + +/** Count only infinity (positive and negative) appearances */ +module.exports.countInfinites = function countInfinites(ctx) { + return `${!ctx.isFloatColumn ? `0` : + `sum(CASE WHEN (${ctx.column} = 'infinity'::float OR ${ctx.column} = '-infinity'::float) THEN 1 ELSE 0 END)` + }`; +}; + +/** Count only NaNs appearances*/ +module.exports.countNaNs = function countNaNs(ctx) { + return `${!ctx.isFloatColumn ? `0` : + `sum(CASE WHEN (${ctx.column} = 'NaN'::float) THEN 1 ELSE 0 END)` + }`; +}; \ No newline at end of file From dfe01c836cd8d6c5b33829b2cf19e7050bcdd1e9 Mon Sep 17 00:00:00 2001 From: Raul Marin Date: Wed, 29 Nov 2017 15:25:35 +0100 Subject: [PATCH 22/30] Escape getQueryRowCount with $$ --- lib/cartodb/utils/query-utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/utils/query-utils.js b/lib/cartodb/utils/query-utils.js index 8b0c81ba..f8b2a0f5 100644 --- a/lib/cartodb/utils/query-utils.js +++ b/lib/cartodb/utils/query-utils.js @@ -22,7 +22,7 @@ module.exports.extractTableNames = function extractTableNames(query) { }; module.exports.getQueryRowCount = function getQueryRowEstimation(query) { - return 'select CDB_EstimateRowCount(\'' + query + '\') as rows'; + return 'select CDB_EstimateRowCount($$' + query + '$$) as rows'; }; /** Cast the column to epoch */ From 243e982bd63b9d679da4874befb986e935d9efa4 Mon Sep 17 00:00:00 2001 From: Raul Marin Date: Wed, 29 Nov 2017 12:33:41 +0100 Subject: [PATCH 23/30] Optimize formulae queries --- lib/cartodb/models/dataview/formula.js | 36 ++---- .../models/dataview/overviews/formula.js | 122 +++++++----------- 2 files changed, 58 insertions(+), 100 deletions(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index daaeaf0c..df009e4b 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -1,34 +1,14 @@ const BaseDataview = require('./base'); const debug = require('debug')('windshaft:dataview:formula'); +const utils = require('../../utils/query-utils'); -const countInfinitiesQueryTpl = ctx => ` - SELECT count(1) FROM (${ctx.query}) __cdb_formula_infinities - WHERE ${ctx.column} = 'infinity'::float OR ${ctx.column} = '-infinity'::float -`; - -const countNansQueryTpl = ctx => ` - SELECT count(1) FROM (${ctx.query}) __cdb_formula_nans - WHERE ${ctx.column} = 'NaN'::float -`; - -const filterOutSpecialNumericValuesTpl = ctx => ` - WHERE - ${ctx.column} != 'infinity'::float - AND - ${ctx.column} != '-infinity'::float - AND - ${ctx.column} != 'NaN'::float -`; - -const formulaQueryTpl = ctx => ` - SELECT - ${ctx.operation}(${ctx.column}) AS result, - (SELECT count(1) FROM (${ctx.query}) _cdb_formula_nulls WHERE ${ctx.column} IS NULL) AS nulls_count - ${ctx.isFloatColumn ? `,(${countInfinitiesQueryTpl(ctx)}) AS infinities_count` : ''} - ${ctx.isFloatColumn ? `,(${countNansQueryTpl(ctx)}) AS nans_count` : ''} - FROM (${ctx.query}) __cdb_formula - ${ctx.isFloatColumn && ctx.operation !== 'count' ? `${filterOutSpecialNumericValuesTpl(ctx)}` : ''} -`; +const formulaQueryTpl = ctx => +`SELECT + ${ctx.operation}(${utils.handleFloatColumn(ctx)}) AS result, + ${utils.countNULLs(ctx)} AS nulls_count + ${ctx.isFloatColumn ? `,${utils.countInfinites(ctx)} AS infinities_count,` : ``} + ${ctx.isFloatColumn ? `${utils.countNaNs(ctx)} AS nans_count` : ``} +FROM (${ctx.query}) __cdb_formula`; const VALID_OPERATIONS = { count: true, diff --git a/lib/cartodb/models/dataview/overviews/formula.js b/lib/cartodb/models/dataview/overviews/formula.js index 64d612c9..2a97061e 100644 --- a/lib/cartodb/models/dataview/overviews/formula.js +++ b/lib/cartodb/models/dataview/overviews/formula.js @@ -1,55 +1,38 @@ var BaseOverviewsDataview = require('./base'); var BaseDataview = require('../formula'); var debug = require('debug')('windshaft:widget:formula:overview'); +const utils = require('../../../utils/query-utils'); var dot = require('dot'); dot.templateSettings.strip = false; -var formulaQueryTpls = { - 'count': dot.template([ - 'SELECT', - 'sum(_feature_count) AS result,', - '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', - '{{?it._isFloatColumn}},(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_infinities', - ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count,', - '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nans', - ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count{{?}}', - 'FROM ({{=it._query}}) _cdb_formula' - ].join('\n')), - 'sum': dot.template([ - 'SELECT', - 'sum({{=it._column}}*_feature_count) AS result,', - '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', - '{{?it._isFloatColumn}},(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_infinities', - ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count', - ',(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nans', - ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count{{?}}', - 'FROM ({{=it._query}}) _cdb_formula', - '{{?it._isFloatColumn}}WHERE', - ' {{=it._column}} != \'infinity\'::float', - 'AND', - ' {{=it._column}} != \'-infinity\'::float', - 'AND', - ' {{=it._column}} != \'NaN\'::float{{?}}' - ].join('\n')), - 'avg': dot.template([ - 'SELECT', - 'sum({{=it._column}}*_feature_count)/sum(_feature_count) AS result,', - '(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', - '{{?it._isFloatColumn}},(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_infinities', - ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count', - ',(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nans', - ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count{{?}}', - 'FROM ({{=it._query}}) _cdb_formula', - '{{?it._isFloatColumn}}WHERE', - ' {{=it._column}} != \'infinity\'::float', - 'AND', - ' {{=it._column}} != \'-infinity\'::float', - 'AND', - ' {{=it._column}} != \'NaN\'::float{{?}}' - ].join('\n')), +const VALID_OPERATIONS = { + count: true, + sum: true, + avg: true }; +/** Formulae to calculate the end result using _feature_count from the overview table*/ +function dataviewResult(ctx) { + switch (ctx.operation) { + case 'count': + return `sum(_feature_count)`; + case 'sum': + return `sum(${utils.handleFloatColumn(ctx)}*_feature_count)`; + case 'avg': + return `sum(${utils.handleFloatColumn(ctx)}*_feature_count)/sum(_feature_count) `; + } + return `${ctx.operation}(${utils.handleFloatColumn(ctx)})`; +} + +const formulaQueryTpl = ctx => +`SELECT + ${dataviewResult(ctx)} AS result, + ${utils.countNULLs(ctx)} AS nulls_count + ${ctx.isFloatColumn ? `,${utils.countInfinites(ctx)} AS infinities_count,` : ``} + ${ctx.isFloatColumn ? `${utils.countNaNs(ctx)} AS nans_count` : ``} +FROM (${ctx.query}) __cdb_formula`; + function Formula(query, options, queryRewriter, queryRewriteData, params, queries) { BaseOverviewsDataview.call(this, query, options, BaseDataview, queryRewriter, queryRewriteData, params, queries); this.column = options.column || '1'; @@ -65,36 +48,31 @@ module.exports = Formula; Formula.prototype.sql = function (psql, override, callback) { var self = this; - var formulaQueryTpl = formulaQueryTpls[this.operation]; - - if (formulaQueryTpl) { - // supported formula for use with overviews - if (this._isFloatColumn === null) { - this._isFloatColumn = false; - this.getColumnType(psql, this.column, this.queries.no_filters, function (err, type) { - if (!err && !!type) { - self._isFloatColumn = type.float; - } - self.sql(psql, override, callback); - }); - return null; - } - - var formulaSql = formulaQueryTpl({ - _isFloatColumn: this._isFloatColumn, - _query: this.rewrittenQuery(this.query), - _operation: this.operation, - _column: this.column - }); - - callback = callback || override; - - debug(formulaSql); - - return callback(null, formulaSql); + if (!VALID_OPERATIONS[this.operation]) { + return this.defaultSql(psql, override, callback); } + if (this._isFloatColumn === null) { + this._isFloatColumn = false; + this.getColumnType(psql, this.column, this.queries.no_filters, function (err, type) { + if (!err && !!type) { + self._isFloatColumn = type.float; + } + self.sql(psql, override, callback); + }); + return null; + } - // default behaviour - return this.defaultSql(psql, override, callback); + var formulaSql = formulaQueryTpl({ + isFloatColumn: this._isFloatColumn, + query: this.rewrittenQuery(this.query), + operation: this.operation, + column: this.column + }); + + callback = callback || override; + + debug(formulaSql); + + return callback(null, formulaSql); }; From 088a8b81a64a51c54b24a5f6105baa8ac6f20420 Mon Sep 17 00:00:00 2001 From: Raul Marin Date: Mon, 4 Dec 2017 12:44:51 +0100 Subject: [PATCH 24/30] Timeseries tests: Set timestamps to the start of the aggregations Also, increase the end of the range check so it includes some data --- .../dataview/histograms/date-histogram.js | 4 +-- test/acceptance/dataviews/histogram.js | 34 +++++++++---------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/lib/cartodb/models/dataview/histograms/date-histogram.js b/lib/cartodb/models/dataview/histograms/date-histogram.js index 34a42f9a..f1790b15 100644 --- a/lib/cartodb/models/dataview/histograms/date-histogram.js +++ b/lib/cartodb/models/dataview/histograms/date-histogram.js @@ -264,8 +264,8 @@ module.exports = class DateHistogram extends BaseHistogram { offset: this._getOffset(override), timestamp_start: firstRow.timestamp_start, - bin_width: firstRow.bin_width, - bins_count: firstRow.bins_number, + bin_width: firstRow.bin_width || 0, + bins_count: firstRow.bins_number || 0, bins_start: firstRow.timestamp, nulls: firstRow.nulls_count, infinities: firstRow.infinities_count, diff --git a/test/acceptance/dataviews/histogram.js b/test/acceptance/dataviews/histogram.js index c9e16489..5331c443 100644 --- a/test/acceptance/dataviews/histogram.js +++ b/test/acceptance/dataviews/histogram.js @@ -186,7 +186,7 @@ describe('histogram-dataview for date column type', function() { }, minute_histogram: { source: { - id: 'minute-histogram-source' + id: 'minute-histogram-source-tz' }, type: 'histogram', options: { @@ -215,7 +215,7 @@ describe('histogram-dataview for date column type', function() { "query": [ "select null::geometry the_geom_webmercator, date AS d", "from generate_series(", - "'2007-02-15 01:00:00'::timestamptz, '2008-04-09 01:00:00'::timestamptz, '1 day'::interval", + "'2007-02-15 01:00:00+00'::timestamptz, '2008-04-09 01:00:00+00'::timestamptz, '1 day'::interval", ") date" ].join(' ') } @@ -233,13 +233,13 @@ describe('histogram-dataview for date column type', function() { } }, { - "id": "minute-histogram-source", + "id": "minute-histogram-source-tz", "type": "source", "params": { "query": [ "select null::geometry the_geom_webmercator, date AS d", "from generate_series(", - "'2007-02-15 23:50:00'::timestamp, '2007-02-16 00:10:00'::timestamp, '1 minute'::interval", + "'2007-02-15 23:50:00+00'::timestamptz, '2007-02-16 00:10:00+00'::timestamptz, '1 minute'::interval", ") date" ].join(' ') } @@ -323,7 +323,7 @@ describe('histogram-dataview for date column type', function() { assert.ok(!err, err); assert.equal(dataview.type, 'histogram'); assert.ok(dataview.bin_width > 0, 'Unexpected bin width: ' + dataview.bin_width); - assert.equal(dataview.bins.length, 6); + assert.equal(dataview.bins_count, 6); dataview.bins.forEach(function (bin) { assert.ok(bin.min <= bin.max, 'bin min < bin max: ' + JSON.stringify(bin)); }); @@ -335,7 +335,7 @@ describe('histogram-dataview for date column type', function() { it('should cast overridden start and end to float to avoid out of range errors ' + test.desc, function (done) { var params = { start: -2145916800, - end: 1009843199 + end: 1193792400 }; this.testClient = new TestClient(mapConfig, 1234); @@ -640,7 +640,7 @@ describe('histogram-dataview for date column type', function() { var dataviewWithDailyAggFixture = { aggregation: 'day', - bin_width: 600, + bin_width: 86400, bins_count: 2, bins_start: 1171497600, timestamp_start: 1171497600, @@ -650,17 +650,17 @@ describe('histogram-dataview for date column type', function() { [{ bin: 0, timestamp: 1171497600, - min: 1171583400, - max: 1171583940, - avg: 1171583670, + min: 1171497600, + max: 1171497600, + avg: 1171497600, freq: 10 }, { bin: 1, timestamp: 1171584000, min: 1171584000, - max: 1171584600, - avg: 1171584300, + max: 1171584000, + avg: 1171584000, freq: 11 }], type: 'histogram' @@ -687,19 +687,19 @@ describe('histogram-dataview for date column type', function() { var dataviewWithDailyAggAndOffsetFixture = { aggregation: 'day', - bin_width: 1200, + bin_width: 86400, bins_count: 1, bins_start: 1171501200, - timestamp_start: 1171497600, + timestamp_start: 1171501200, nulls: 0, offset: -3600, bins: [{ bin: 0, timestamp: 1171501200, - min: 1171583400, - max: 1171584600, - avg: 1171584000, + min: 1171501200, + max: 1171501200, + avg: 1171501200, freq: 21 }], type: 'histogram' From bb5bfd10eeed20825ea0854cbf27811879547dd0 Mon Sep 17 00:00:00 2001 From: Raul Marin Date: Mon, 4 Dec 2017 17:02:40 +0100 Subject: [PATCH 25/30] Timeseries tests: Make them work with any DB setup --- test/acceptance/dataviews/histogram.js | 42 +++++++++++++------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/test/acceptance/dataviews/histogram.js b/test/acceptance/dataviews/histogram.js index 5331c443..002d8617 100644 --- a/test/acceptance/dataviews/histogram.js +++ b/test/acceptance/dataviews/histogram.js @@ -256,6 +256,7 @@ describe('histogram-dataview for date column type', function() { }]; dateHistogramsUseCases.forEach(function (test) { + it('should create a date histogram aggregated in months (EDT) ' + test.desc, function (done) { var OFFSET_EDT_IN_MINUTES = -4 * 60; // EDT Eastern Daylight Time (GMT-4) in minutes @@ -348,27 +349,6 @@ describe('histogram-dataview for date column type', function() { }); }); - it('should return same histogram ' + test.desc, function (done) { - var params = { - start: 1171501200, // 2007-02-15 01:00:00 = min(date_colum) - end: 1207702800 // 2008-04-09 01:00:00 = max(date_colum) - }; - - this.testClient = new TestClient(mapConfig, 1234); - this.testClient.getDataview(test.dataviewId, {}, function (err, dataview) { - assert.ok(!err, err); - - this.testClient = new TestClient(mapConfig, 1234); - this.testClient.getDataview(test.dataviewId, params, function (err, filteredDataview) { - assert.ok(!err, err); - - assert.deepEqual(dataview, filteredDataview); - done(); - }); - }); - }); - - it('should aggregate histogram overriding default offset to CEST ' + test.desc, function (done) { var OFFSET_CEST_IN_SECONDS = 2 * 3600; // Central European Summer Time (Daylight Saving Time) var OFFSET_CEST_IN_MINUTES = 2 * 60; // Central European Summer Time (Daylight Saving Time) @@ -533,6 +513,26 @@ describe('histogram-dataview for date column type', function() { }); }); + it('should return same histogram ', function (done) { + var params = { + start: 1171501200, // 2007-02-15 01:00:00 = min(date_colum) + end: 1207702800 // 2008-04-09 01:00:00 = max(date_colum) + }; + + this.testClient = new TestClient(mapConfig, 1234); + this.testClient.getDataview('datetime_histogram_tz', {}, function (err, dataview) { + assert.ok(!err, err); + + this.testClient = new TestClient(mapConfig, 1234); + this.testClient.getDataview('datetime_histogram_tz', params, function (err, filteredDataview) { + assert.ok(!err, err); + + assert.deepEqual(dataview, filteredDataview); + done(); + }); + }); + }); + it('should find the best aggregation (automatic mode) to build the histogram', function (done) { var params = {}; this.testClient = new TestClient(mapConfig, 1234); From 1b405e42c2804f2ab85ae200be077c9f3ad4a0d5 Mon Sep 17 00:00:00 2001 From: Raul Marin Date: Mon, 4 Dec 2017 17:03:31 +0100 Subject: [PATCH 26/30] Date histogram optimizations --- .../dataview/histograms/date-histogram.js | 229 +++++++++--------- 1 file changed, 121 insertions(+), 108 deletions(-) diff --git a/lib/cartodb/models/dataview/histograms/date-histogram.js b/lib/cartodb/models/dataview/histograms/date-histogram.js index f1790b15..785d132a 100644 --- a/lib/cartodb/models/dataview/histograms/date-histogram.js +++ b/lib/cartodb/models/dataview/histograms/date-histogram.js @@ -1,5 +1,102 @@ const BaseHistogram = require('./base-histogram'); const debug = require('debug')('windshaft:dataview:date-histogram'); +const utils = require('../../../utils/query-utils'); + +/** + * Gets the name of a timezone with the same offset as the required + * using the pg_timezone_names table. We do this because it's simpler to pass + * the name than to pass the offset itself as PostgreSQL uses different + * sign convention. For example: TIME ZONE 'CET' is equal to TIME ZONE 'UTC-1', + * not 'UTC+1' which would be expected. + * Gives priority to Etc/GMT±N timezones but still support odd offsets like 8.5 + * hours for Asia/Pyongyang. + * It also makes it easier to, in the future, support the input of expected timezone + * instead of the offset; that is using 'Europe/Madrid' instead of + * '+3600' or '+7200'. The daylight saving status can be handled by postgres. + */ +const offsetNameQueryTpl = ctx => ` +WITH __wd_tz AS +( + SELECT name + FROM pg_timezone_names + WHERE utc_offset = interval '${ctx.offset} hours' + ORDER BY CASE WHEN name LIKE 'Etc/GMT%' THEN 0 ELSE 1 END + LIMIT 1 +),`; + +/** + * Function to get the subquery that places each row in its bin depending on + * the aggregation. Since the data stored is in epoch we need to adapt it to + * our timezone so when calling date_trunc it falls into the correct bin + */ +function dataBucketsQuery(ctx) { + var condition_str = ''; + + if (ctx.start !== 0) { + condition_str = `WHERE ${ctx.column} >= to_timestamp(${ctx.start})`; + } + if (ctx.end !== 0) { + if (condition_str === '') { + condition_str = `WHERE ${ctx.column} <= to_timestamp(${ctx.end})`; + } + else { + condition_str += ` and ${ctx.column} <= to_timestamp(${ctx.end})`; + } + } + + return ` +__wd_buckets AS +( + SELECT + date_trunc('${ctx.aggregation}', timezone(__wd_tz.name, ${ctx.column}::timestamptz)) as timestamp, + count(*) as freq, + ${utils.countNULLs(ctx)} as nulls_count + FROM + ( + ${ctx.query} + ) __source, __wd_tz + ${condition_str} + GROUP BY timestamp +),`; +} + +/** + * Function that generates an array with all the possible bins between the + * start and end date. If not provided we use the min and max generated from + * the dataBucketsQuery + */ +function allBucketsArrayQuery(ctx) { + var extra_from = ``; + var series_start = ``; + var series_end = ``; + + if (ctx.start === 0) { + extra_from = `, __wd_buckets GROUP BY __wd_tz.name`; + series_start = `min(__wd_buckets.timestamp)`; + } else { + series_start = `date_trunc('${ctx.aggregation}', timezone(__wd_tz.name, to_timestamp(${ctx.start})))`; + } + + if (ctx.end === 0) { + extra_from = `, __wd_buckets GROUP BY __wd_tz.name`; + series_end = `max(__wd_buckets.timestamp)`; + } else { + series_end = `date_trunc('${ctx.aggregation}', timezone(__wd_tz.name, to_timestamp(${ctx.end})))`; + } + + return ` +__wd_all_buckets AS +( + SELECT ARRAY( + SELECT + generate_series( + ${series_start}, + ${series_end}, + interval '${ctx.interval}') as bin_start + FROM __wd_tz${extra_from} + ) as bins +)`; +} const dateIntervalQueryTpl = ctx => ` WITH @@ -41,107 +138,6 @@ const dateIntervalQueryTpl = ctx => ` FROM __cdb_interval_in_days, __cdb_interval_in_hours, __cdb_interval_in_minutes, __cdb_interval_in_seconds `; -const nullsQueryTpl = ctx => ` - __cdb_nulls AS ( - SELECT - count(*) AS __cdb_nulls_count - FROM (${ctx.query}) __cdb_histogram_nulls - WHERE ${ctx.column} IS NULL - ) -`; - -const dateBasicsQueryTpl = ctx => ` - __cdb_basics AS ( - SELECT - max(date_part('epoch', ${ctx.column})) AS __cdb_max_val, - min(date_part('epoch', ${ctx.column})) AS __cdb_min_val, - avg(date_part('epoch', ${ctx.column})) AS __cdb_avg_val, - min( - date_trunc( - '${ctx.aggregation}', ${ctx.column}::timestamp AT TIME ZONE '${ctx.offset}' - ) - ) AS __cdb_start_date, - max(${ctx.column}::timestamp AT TIME ZONE '${ctx.offset}') AS __cdb_end_date, - count(1) AS __cdb_total_rows - FROM (${ctx.query}) __cdb_basics_query - ) -`; - -const dateOverrideBasicsQueryTpl = ctx => ` - __cdb_basics AS ( - SELECT - max(${ctx.end})::float AS __cdb_max_val, - min(${ctx.start})::float AS __cdb_min_val, - avg(date_part('epoch', ${ctx.column})) AS __cdb_avg_val, - min( - date_trunc( - '${ctx.aggregation}', - TO_TIMESTAMP(${ctx.start})::timestamp AT TIME ZONE '${ctx.offset}' - ) - ) AS __cdb_start_date, - max( - TO_TIMESTAMP(${ctx.end})::timestamp AT TIME ZONE '${ctx.offset}' - ) AS __cdb_end_date, - count(1) AS __cdb_total_rows - FROM (${ctx.query}) __cdb_basics_query - ) -`; - -const dateBinsQueryTpl = ctx => ` - __cdb_bins AS ( - SELECT - __cdb_bins_array, - ARRAY_LENGTH(__cdb_bins_array, 1) AS __cdb_bins_number - FROM ( - SELECT - ARRAY( - SELECT GENERATE_SERIES( - __cdb_start_date::timestamptz, - __cdb_end_date::timestamptz, - ${ctx.aggregation === 'quarter' ? `'3 month'::interval` : `'1 ${ctx.aggregation}'::interval`} - ) - ) AS __cdb_bins_array - FROM __cdb_basics - ) __cdb_bins_array_query - ) -`; - -const dateHistogramQueryTpl = ctx => ` - SELECT - (__cdb_max_val - __cdb_min_val) / cast(__cdb_bins_number as float) AS bin_width, - __cdb_bins_number AS bins_number, - __cdb_nulls_count AS nulls_count, - CASE WHEN __cdb_min_val = __cdb_max_val - THEN 0 - ELSE GREATEST( - 1, - LEAST( - WIDTH_BUCKET( - ${ctx.column}::timestamp AT TIME ZONE '${ctx.offset}', - __cdb_bins_array - ), - __cdb_bins_number - ) - ) - 1 - END AS bin, - min( - date_part( - 'epoch', - date_trunc( - '${ctx.aggregation}', ${ctx.column}::timestamp AT TIME ZONE '${ctx.offset}' - ) AT TIME ZONE '${ctx.offset}' - ) - )::numeric AS timestamp, - date_part('epoch', __cdb_start_date)::numeric AS timestamp_start, - min(date_part('epoch', ${ctx.column}))::numeric AS min, - max(date_part('epoch', ${ctx.column}))::numeric AS max, - avg(date_part('epoch', ${ctx.column}))::numeric AS avg, - count(*) AS freq - FROM (${ctx.query}) __cdb_histogram, __cdb_basics, __cdb_bins, __cdb_nulls - WHERE date_part('epoch', ${ctx.column}) IS NOT NULL - GROUP BY bin, bins_number, bin_width, nulls_count, timestamp_start - ORDER BY bin -`; const MAX_INTERVAL_VALUE = 366; @@ -176,12 +172,21 @@ module.exports = class DateHistogram extends BaseHistogram { _buildQueryTpl (ctx) { return ` - WITH - ${this._hasOverridenRange(ctx.override) ? dateOverrideBasicsQueryTpl(ctx) : dateBasicsQueryTpl(ctx)}, - ${dateBinsQueryTpl(ctx)}, - ${nullsQueryTpl(ctx)} - ${dateHistogramQueryTpl(ctx)} - `; +${offsetNameQueryTpl(ctx)} +${dataBucketsQuery(ctx)} +${allBucketsArrayQuery(ctx)} +SELECT + array_position(__wd_all_buckets.bins, __wd_buckets.timestamp) - 1 as bin, + date_part('epoch', timezone(__wd_tz.name, __wd_buckets.timestamp)) AS timestamp, + __wd_buckets.freq as freq, + date_part('epoch', timezone(__wd_tz.name, (__wd_all_buckets.bins)[1])) as timestamp_start, + array_length(__wd_all_buckets.bins, 1) as bins_number, + date_part('epoch', interval '${ctx.interval}') as bin_width, + __wd_buckets.nulls_count as nulls_count +FROM __wd_buckets, __wd_all_buckets, __wd_tz +GROUP BY __wd_tz.name, __wd_all_buckets.bins, __wd_buckets.timestamp, __wd_buckets.nulls_count, __wd_buckets.freq +ORDER BY bin ASC; +`; } _buildQuery (psql, override, callback) { @@ -204,6 +209,9 @@ module.exports = class DateHistogram extends BaseHistogram { return null; } + var interval = this._getAggregation(override) === 'quarter' ? + '3 months' : '1 ' + this._getAggregation(override); + const histogramSql = this._buildQueryTpl({ override: override, query: this.query, @@ -211,7 +219,8 @@ module.exports = class DateHistogram extends BaseHistogram { aggregation: this._getAggregation(override), start: this._getBinStart(override), end: this._getBinEnd(override), - offset: this._parseOffset(override) + offset: this._parseOffset(override), + interval: interval }); debug(histogramSql); @@ -275,6 +284,10 @@ module.exports = class DateHistogram extends BaseHistogram { } _getBuckets (result) { + result.rows.forEach(function(row) { + row.min = row.max = row.avg = row.timestamp; + }); + return result.rows.map(({ bin, min, max, avg, freq, timestamp }) => ({ bin, min, max, avg, freq, timestamp })); } From 88f02458db675aeedf1e03122cff468f6704a3a7 Mon Sep 17 00:00:00 2001 From: Raul Marin Date: Mon, 4 Dec 2017 17:15:06 +0100 Subject: [PATCH 27/30] Timeseries tests: Make jshint happy --- test/acceptance/dataviews/histogram.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/acceptance/dataviews/histogram.js b/test/acceptance/dataviews/histogram.js index 002d8617..af8ce59d 100644 --- a/test/acceptance/dataviews/histogram.js +++ b/test/acceptance/dataviews/histogram.js @@ -214,8 +214,8 @@ describe('histogram-dataview for date column type', function() { "params": { "query": [ "select null::geometry the_geom_webmercator, date AS d", - "from generate_series(", - "'2007-02-15 01:00:00+00'::timestamptz, '2008-04-09 01:00:00+00'::timestamptz, '1 day'::interval", + "from generate_series('2007-02-15 01:00:00+00'::timestamptz,", + "'2008-04-09 01:00:00+00'::timestamptz, '1 day'::interval", ") date" ].join(' ') } @@ -238,8 +238,8 @@ describe('histogram-dataview for date column type', function() { "params": { "query": [ "select null::geometry the_geom_webmercator, date AS d", - "from generate_series(", - "'2007-02-15 23:50:00+00'::timestamptz, '2007-02-16 00:10:00+00'::timestamptz, '1 minute'::interval", + "from generate_series('2007-02-15 23:50:00+00'::timestamptz,", + "'2007-02-16 00:10:00+00'::timestamptz, '1 minute'::interval", ") date" ].join(' ') } From eb4a49ec9256ba34cc30e7df680b38c1265d9385 Mon Sep 17 00:00:00 2001 From: Raul Marin Date: Mon, 4 Dec 2017 17:56:11 +0100 Subject: [PATCH 28/30] Update NEWs file with the changes --- NEWS.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/NEWS.md b/NEWS.md index 84cfb4bf..4c737e7c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -3,6 +3,13 @@ ## 4.2.1 Released 2017-mm-dd +Announcements: +- Optimize Formula queries. +- Optimize Formula queries in overviews. +- Optimize Numeric Histogram queries. +- Optimize Date Histogram queries. +- Date Histograms: Now returns the same value for max/min/avg/timestamp per bin. +- Date Histograms: Now it should return the same no matter the DB/Client time zone. ## 4.2.0 Released 2017-12-04 From 7c5b7641d8ba0328737b61008be859f8a615ad08 Mon Sep 17 00:00:00 2001 From: Raul Marin Date: Tue, 5 Dec 2017 16:25:36 +0100 Subject: [PATCH 29/30] Date histogram: Add missing GROUP BY column --- lib/cartodb/models/dataview/histograms/date-histogram.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/models/dataview/histograms/date-histogram.js b/lib/cartodb/models/dataview/histograms/date-histogram.js index 785d132a..af151a72 100644 --- a/lib/cartodb/models/dataview/histograms/date-histogram.js +++ b/lib/cartodb/models/dataview/histograms/date-histogram.js @@ -56,7 +56,7 @@ __wd_buckets AS ${ctx.query} ) __source, __wd_tz ${condition_str} - GROUP BY timestamp + GROUP BY timestamp, __wd_tz.name ),`; } From aa57cdefb370ef577bad6dacf26b1a88d82feb3a Mon Sep 17 00:00:00 2001 From: Raul Marin Date: Mon, 11 Dec 2017 11:21:16 +0100 Subject: [PATCH 30/30] Release 4.3.0 --- NEWS.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 4c737e7c..e70b8df2 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,8 +1,13 @@ # Changelog -## 4.2.1 +## 4.3.1 Released 2017-mm-dd +Announcements: + +## 4.3.0 +Released 2017-12-11 + Announcements: - Optimize Formula queries. - Optimize Formula queries in overviews.