diff --git a/NEWS.md b/NEWS.md index 02c66777..fb2d8ea2 100644 --- a/NEWS.md +++ b/NEWS.md @@ -5,16 +5,17 @@ Released 2018-mm-dd New features: - CI tests with Ubuntu Xenial + PostgreSQL 10.1 and Ubuntu Precise + PostgreSQL 9.5 -- Upgrades Windshaft to [4.8.0](https://github.com/CartoDB/Windshaft/blob/4.8.0/NEWS.md#version-480) which includes: +- Upgrades Windshaft to [4.8.1](https://github.com/CartoDB/Windshaft/blob/4.8.1/NEWS.md#version-481) which includes: - Update internal deps. - A fix in mapnik-vector-tile to avoid grouping together properties with the same value but a different type. - Performance improvements in the marker symbolizer (local cache, avoid building the collision matrix when possible). - MVT: Disable simplify_distance to avoid multiple simplifications. - Fix a bug with zero length lines not being rendered when using the marker symbolizer. -- Upgrades Camshaft to [0.61.9](https://github.com/CartoDB/camshaft/releases/tag/0.61.9): +- Upgrades Camshaft to [0.61.10](https://github.com/CartoDB/camshaft/releases/tag/0.61.10): - Use Dollar-Quoted String Constants to avoid Syntax Error while running moran analyses. - Update other deps: - body-parser: 1.18.3 + - cartodb-psql: 0.11.0 - dot: 1.1.2 - express: 4.16.3 - lru-cache: 4.1.3 diff --git a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js index 129e4937..d8320334 100644 --- a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js +++ b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js @@ -34,24 +34,11 @@ MapnikLayerStats.prototype.is = function (type) { return this._types[type] ? this._types[type] : false; }; -function queryPromise(dbConnection, query, adaptResults, errorHandler) { - return new Promise(function(resolve, reject) { - dbConnection.query(query, function (err, res) { - if (err) { - if (errorHandler) { - resolve(errorHandler(err)); - } - else { - reject(err); - } - } - else { - resolve(adaptResults(res)); - } - }); - +function queryPromise(dbConnection, query) { + return new Promise((resolve, reject) => { + dbConnection.query(query, (err, res) => err ? reject(err) : resolve(res)); }); -} + } function columnAggregations(field) { if (field.type === 'number') { @@ -76,22 +63,16 @@ function _getSQL(ctx, query, type='pre', zoom=0) { } function _estimatedFeatureCount(ctx) { - return queryPromise( - ctx.dbConnection, - _getSQL(ctx, queryUtils.getQueryRowEstimation), - res => ({ estimatedFeatureCount: res.rows[0].rows }), - () => ({ estimatedFeatureCount: -1 }) - ); + return queryPromise(ctx.dbConnection, _getSQL(ctx, queryUtils.getQueryRowEstimation)) + .then(res => ({ estimatedFeatureCount: res.rows[0].rows })) + .catch(() => ({ estimatedFeatureCount: -1 })); } function _featureCount(ctx) { if (ctx.metaOptions.featureCount) { // TODO: if ctx.metaOptions.columnStats we can combine this with column stats query - return queryPromise( - ctx.dbConnection, - _getSQL(ctx, queryUtils.getQueryActualRowCount), - res => ({ featureCount: res.rows[0].rows }) - ); + return queryPromise(ctx.dbConnection, _getSQL(ctx, queryUtils.getQueryActualRowCount)) + .then(res => ({ featureCount: res.rows[0].rows })); } return Promise.resolve(); } @@ -103,9 +84,8 @@ function _aggrFeatureCount(ctx) { // return metadata for multiple levels. return queryPromise( ctx.dbConnection, - _getSQL(ctx, queryUtils.getQueryActualRowCount, 'post', ctx.metaOptions.aggrFeatureCount), - res => ({ aggrFeatureCount: res.rows[0].rows }) - ); + _getSQL(ctx, queryUtils.getQueryActualRowCount, 'post', ctx.metaOptions.aggrFeatureCount) + ).then(res => ({ aggrFeatureCount: res.rows[0].rows })); } return Promise.resolve(); } @@ -113,11 +93,8 @@ function _aggrFeatureCount(ctx) { function _geometryType(ctx) { if (ctx.metaOptions.geometryType) { const geometryColumn = AggregationMapConfig.getAggregationGeometryColumn(); - return queryPromise( - ctx.dbConnection, - _getSQL(ctx, sql => queryUtils.getQueryGeometryType(sql, geometryColumn)), - res => ({ geometryType: res.rows[0].geom_type }) - ); + return queryPromise(ctx.dbConnection, _getSQL(ctx, sql => queryUtils.getQueryGeometryType(sql, geometryColumn))) + .then(res => ({ geometryType: res.rows[0].geom_type })); } return Promise.resolve(); } @@ -125,11 +102,8 @@ function _geometryType(ctx) { function _columns(ctx) { if (ctx.metaOptions.columns || ctx.metaOptions.columnStats) { // note: post-aggregation columns are in layer.options.columns when aggregation is present - return queryPromise( - ctx.dbConnection, - _getSQL(ctx, sql => queryUtils.getQueryLimited(sql, 0)), - res => formatResultFields(ctx.dbConnection, res.fields) - ); + return queryPromise(ctx.dbConnection, _getSQL(ctx, sql => queryUtils.getQueryLimited(sql, 0))) + .then(res => formatResultFields(ctx.dbConnection, res.fields)); } return Promise.resolve(); } @@ -172,16 +146,20 @@ function mergeColumns(results) { } } +const SAMPLE_SEED = 0.5; +const DEFAULT_SAMPLE_ROWS = 100; + function _sample(ctx, numRows) { if (ctx.metaOptions.sample) { - const sampleProb = Math.min(ctx.metaOptions.sample / numRows, 1); + const sampleProb = Math.min(ctx.metaOptions.sample.num_rows / numRows, 1); // We'll use a safety limit just in case numRows is a bad estimate - const limit = Math.ceil(ctx.metaOptions.sample * 1.5); - return queryPromise( - ctx.dbConnection, - _getSQL(ctx, sql => queryUtils.getQuerySample(sql, sampleProb, limit)), - res => ({ sample: res.rows }) - ); + const requestedRows = ctx.metaOptions.sample.num_rows || DEFAULT_SAMPLE_ROWS; + const limit = Math.ceil(requestedRows * 1.5); + let columns = ctx.metaOptions.sample.include_columns; + return queryPromise(ctx.dbConnection, _getSQL( + ctx, + sql => queryUtils.getQuerySample(sql, sampleProb, limit, SAMPLE_SEED, columns) + )).then(res => ({ sample: res.rows })); } return Promise.resolve(); } @@ -210,27 +188,25 @@ function _columnStats(ctx, columns) { queries.push( queryPromise( ctx.dbConnection, - _getSQL(ctx, sql => queryUtils.getQueryTopCategories(sql, name, topN, includeNulls)), - res => ({ [name]: { categories: res.rows } }) - ) + _getSQL(ctx, sql => queryUtils.getQueryTopCategories(sql, name, topN, includeNulls)) + ).then(res => ({ [name]: { categories: res.rows } })) ); } }); queries.push( queryPromise( ctx.dbConnection, - _getSQL(ctx, sql => `SELECT ${aggr.join(',')} FROM (${sql}) AS __cdb_query`), - res => { - let stats = {}; - Object.keys(columns).forEach(name => { - stats[name] = {}; - columnAggregations(columns[name]).forEach(fn => { - stats[name][fn] = res.rows[0][`${name}_${fn}`]; - }); + _getSQL(ctx, sql => `SELECT ${aggr.join(',')} FROM (${sql}) AS __cdb_query`) + ).then(res => { + let stats = {}; + Object.keys(columns).forEach(name => { + stats[name] = {}; + columnAggregations(columns[name]).forEach(fn => { + stats[name][fn] = res.rows[0][`${name}_${fn}`]; }); - return stats; - } - ) + }); + return stats; + }) ); return Promise.all(queries).then(results => ({ columns: mergeColumns(results) })); } @@ -296,6 +272,8 @@ function (layer, dbConnection, callback) { // (if metaOptions.geometryType) from it. // TODO: compute _sample with _featureCount when available + // TODO: add support for sample.exclude option by, in that case, forcing the columns query and + // passing the results to the sample query function. Promise.all([ _estimatedFeatureCount(ctx).then( diff --git a/lib/cartodb/utils/query-utils.js b/lib/cartodb/utils/query-utils.js index 22d99eef..867db0cd 100644 --- a/lib/cartodb/utils/query-utils.js +++ b/lib/cartodb/utils/query-utils.js @@ -88,17 +88,30 @@ module.exports.getQueryTopCategories = function(query, column, topN, includeNull `; }; -module.exports.getQuerySample = function(query, sampleProb, limit = null, randomSeed = 0.5) { +function columnSelector(columns) { + if (!columns) { + return '*'; + } + if (typeof columns === 'string') { + return columns; + } + if (Array.isArray(columns)) { + return columns.map(name => `"${name}"`).join(', '); + } + throw new TypeError(`Bad argument type for columns: ${typeof columns}`); +} + +module.exports.getQuerySample = function(query, sampleProb, limit = null, randomSeed = 0.5, columns = null) { const singleTable = simpleQueryTable(query); if (singleTable) { - return getTableSample(singleTable.table, singleTable.columns, sampleProb, limit, randomSeed); + return getTableSample(singleTable.table, columns || singleTable.columns, sampleProb, limit, randomSeed); } const limitClause = limit ? `LIMIT ${limit}` : ''; return ` WITH __cdb_rndseed AS ( SELECT setseed(${randomSeed}) ) - SELECT * + SELECT ${columnSelector(columns)} FROM (${query}) AS __cdb_query WHERE random() < ${sampleProb} ${limitClause} @@ -110,7 +123,9 @@ function getTableSample(table, columns, sampleProb, limit = null, randomSeed = 0 sampleProb *= 100; randomSeed *= Math.pow(2, 31) -1; return ` - SELECT ${columns} FROM ${table} TABLESAMPLE BERNOULLI (${sampleProb}) REPEATABLE (${randomSeed}) ${limitClause} + SELECT ${columnSelector(columns)} + FROM ${table} + TABLESAMPLE BERNOULLI (${sampleProb}) REPEATABLE (${randomSeed}) ${limitClause} `; } diff --git a/package.json b/package.json index 40d75a38..488b59c3 100644 --- a/package.json +++ b/package.json @@ -26,8 +26,8 @@ "dependencies": { "basic-auth": "2.0.0", "body-parser": "1.18.3", - "camshaft": "0.61.9", - "cartodb-psql": "0.10.2", + "camshaft": "0.61.10", + "cartodb-psql": "0.11.0", "cartodb-query-tables": "0.3.0", "cartodb-redis": "1.0.0", "debug": "3.1.0", @@ -48,7 +48,7 @@ "step-profiler": "0.3.0", "turbo-carto": "0.20.2", "underscore": "1.6.0", - "windshaft": "4.8.0", + "windshaft": "4.8.1", "yargs": "11.1.0" }, "devDependencies": { diff --git a/test/acceptance/stats/mapnik_stats_layergroup.js b/test/acceptance/stats/mapnik_stats_layergroup.js index 46c4368a..4e8413ae 100644 --- a/test/acceptance/stats/mapnik_stats_layergroup.js +++ b/test/acceptance/stats/mapnik_stats_layergroup.js @@ -513,7 +513,7 @@ describe('Create mapnik layergroup', function() { version: '1.4.0', layers: [ layerWithMetadata(mapnikLayer4, { - sample: 3 + sample: { num_rows: 3 } }) ] }); @@ -529,6 +529,31 @@ describe('Create mapnik layergroup', function() { }); }); + it('can specify sample columns', function(done) { + var testClient = new TestClient({ + version: '1.4.0', + layers: [ + layerWithMetadata(mapnikLayer4, { + sample: { + num_rows: 3, + include_columns: [ 'cartodb_id', 'address', 'the_geom' ] + } + }) + ] + }); + + testClient.getLayergroup(function(err, layergroup) { + assert.ifError(err); + assert.equal(layergroup.metadata.layers[0].id, mapnikBasicLayerId(0)); + assert.equal(layergroup.metadata.layers[0].meta.stats.estimatedFeatureCount, 5); + assert(layergroup.metadata.layers[0].meta.stats.sample.length > 0); + const expectedCols = [ 'cartodb_id', 'address', 'the_geom' ].sort(); + assert.deepEqual(Object.keys(layergroup.metadata.layers[0].meta.stats.sample[0]).sort(), expectedCols); + testClient.drain(done); + }); + }); + + it('should only provide requested optional metadata', function(done) { var testClient = new TestClient({ version: '1.4.0', diff --git a/yarn.lock b/yarn.lock index deb13479..4f98787a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -261,13 +261,13 @@ camelcase@^4.1.0: version "4.1.0" resolved "https://registry.yarnpkg.com/camelcase/-/camelcase-4.1.0.tgz#d545635be1e33c542649c69173e5de6acfae34dd" -camshaft@0.61.9: - version "0.61.9" - resolved "https://registry.yarnpkg.com/camshaft/-/camshaft-0.61.9.tgz#f3d399dfacf51b6a492c579e925c8b1141b22974" +camshaft@0.61.10: + version "0.61.10" + resolved "https://registry.yarnpkg.com/camshaft/-/camshaft-0.61.10.tgz#9055bbc577dbd87d38a0e712bf6157afd7704dca" dependencies: async "^1.5.2" bunyan "1.8.1" - cartodb-psql "^0.10.1" + cartodb-psql "0.11.0" debug "^3.1.0" dot "^1.0.3" request "2.85.0" @@ -312,12 +312,12 @@ cartocolor@4.0.0: dependencies: colorbrewer "1.0.0" -cartodb-psql@0.10.2, cartodb-psql@^0.10.1: - version "0.10.2" - resolved "https://registry.yarnpkg.com/cartodb-psql/-/cartodb-psql-0.10.2.tgz#8c505066e4a635cfa0ee4c603769c83f6e2187dd" +cartodb-psql@0.11.0: + version "0.11.0" + resolved "https://registry.yarnpkg.com/cartodb-psql/-/cartodb-psql-0.11.0.tgz#6b4eae0876ee56944a61fe5f4acc6a8b0b11233f" dependencies: debug "^3.1.0" - pg cartodb/node-postgres#6.1.6-cdb1 + pg CartoDB/node-postgres#6.4.2-cdb1 underscore "~1.6.0" cartodb-query-tables@0.3.0: @@ -1804,9 +1804,9 @@ p-try@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/p-try/-/p-try-1.0.0.tgz#cbc79cdbaf8fd4228e13f621f2b1a237c1b207b3" -packet-reader@0.2.0: - version "0.2.0" - resolved "https://registry.yarnpkg.com/packet-reader/-/packet-reader-0.2.0.tgz#819df4d010b82d5ea5671f8a1a3acf039bcd7700" +packet-reader@0.3.1: + version "0.3.1" + resolved "https://registry.yarnpkg.com/packet-reader/-/packet-reader-0.3.1.tgz#cd62e60af8d7fea8a705ec4ff990871c46871f27" parse-json@^2.2.0: version "2.2.0" @@ -1885,20 +1885,20 @@ pg-types@1.*: postgres-date "~1.0.0" postgres-interval "^1.1.0" -pg@cartodb/node-postgres#6.1.6-cdb1: - version "6.1.6" - resolved "https://codeload.github.com/cartodb/node-postgres/tar.gz/3eef52dd1e655f658a4ee8ac5697688b3ecfed44" +"pg@github:CartoDB/node-postgres#6.4.2-cdb1": + version "6.4.2" + resolved "https://codeload.github.com/CartoDB/node-postgres/tar.gz/449fac1d6da711ffcc6694ae3c89f85244f48bdc" dependencies: buffer-writer "1.0.1" js-string-escape "1.0.1" - packet-reader "0.2.0" + packet-reader "0.3.1" pg-connection-string "0.1.3" pg-pool "1.*" pg-types "1.*" - pgpass "1.x" + pgpass "1.*" semver "4.3.2" -pgpass@1.x: +pgpass@1.*: version "1.0.2" resolved "https://registry.yarnpkg.com/pgpass/-/pgpass-1.0.2.tgz#2a7bb41b6065b67907e91da1b07c1847c877b306" dependencies: @@ -2802,16 +2802,16 @@ 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.8.0: - version "4.8.0" - resolved "https://registry.yarnpkg.com/windshaft/-/windshaft-4.8.0.tgz#79ea03ee78fa68288e07c7aa01a9fdabe3cc5af0" +windshaft@4.8.1: + version "4.8.1" + resolved "https://registry.yarnpkg.com/windshaft/-/windshaft-4.8.1.tgz#7c1ef21e4f885643f6347bc5871274c533fce206" dependencies: "@carto/mapnik" "3.6.2-carto.10" "@carto/tilelive-bridge" cartodb/tilelive-bridge#2.5.1-cdb9 abaculus cartodb/abaculus#2.0.3-cdb10 canvas cartodb/node-canvas#1.6.2-cdb2 carto cartodb/carto#0.15.1-cdb3 - cartodb-psql "0.10.2" + cartodb-psql "0.11.0" debug "3.1.0" dot "1.1.2" grainstore "1.9.0"