Merge branch 'master' into unify-connection-pool-config

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

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) {
+function queryPromise(dbConnection, query) {
-    return new Promise(function(resolve, reject) {
+    return new Promise((resolve, reject) => {
-        dbConnection.query(query, function (err, res) {
+      dbConnection.query(query, (err, res) => err ? reject(err) : resolve(res));
-            if (err) {
-                if (errorHandler) {
-                    resolve(errorHandler(err));
-                }
-                else {
-                    reject(err);
-                }
-            }
-            else {
-                resolve(adaptResults(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(
+    return queryPromise(ctx.dbConnection, _getSQL(ctx, queryUtils.getQueryRowEstimation))
-        ctx.dbConnection,
+        .then(res => ({ estimatedFeatureCount: res.rows[0].rows }))
-        _getSQL(ctx, queryUtils.getQueryRowEstimation),
+        .catch(() => ({ estimatedFeatureCount: -1 }));
-        res => ({ estimatedFeatureCount: res.rows[0].rows }),
-        () => ({ estimatedFeatureCount: -1 })
-    );
 }
 
 function _featureCount(ctx) {
     if (ctx.metaOptions.featureCount) {
         // TODO: if ctx.metaOptions.columnStats we can combine this with column stats query
-        return queryPromise(
+        return queryPromise(ctx.dbConnection, _getSQL(ctx, queryUtils.getQueryActualRowCount))
-            ctx.dbConnection,
+            .then(res => ({ featureCount: res.rows[0].rows }));
-            _getSQL(ctx, queryUtils.getQueryActualRowCount),
-            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),
+            _getSQL(ctx, queryUtils.getQueryActualRowCount, 'post',  ctx.metaOptions.aggrFeatureCount)
-            res => ({ aggrFeatureCount: res.rows[0].rows })
+        ).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(
+        return queryPromise(ctx.dbConnection, _getSQL(ctx, sql => queryUtils.getQueryGeometryType(sql, geometryColumn)))
-            ctx.dbConnection,
+            .then(res => ({ geometryType: res.rows[0].geom_type }));
-            _getSQL(ctx, sql => queryUtils.getQueryGeometryType(sql, geometryColumn)),
-            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(
+        return queryPromise(ctx.dbConnection, _getSQL(ctx, sql => queryUtils.getQueryLimited(sql, 0)))
-            ctx.dbConnection,
+            .then(res => formatResultFields(ctx.dbConnection, res.fields));
-            _getSQL(ctx, sql => queryUtils.getQueryLimited(sql, 0)),
-            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);
+        const requestedRows = ctx.metaOptions.sample.num_rows || DEFAULT_SAMPLE_ROWS;
-        return queryPromise(
+        const limit = Math.ceil(requestedRows * 1.5);
-            ctx.dbConnection,
+        let columns = ctx.metaOptions.sample.include_columns;
-            _getSQL(ctx, sql => queryUtils.getQuerySample(sql, sampleProb, limit)),
+        return queryPromise(ctx.dbConnection, _getSQL(
-            res => ({ sample: res.rows })
+            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)),
+                        _getSQL(ctx, sql => queryUtils.getQueryTopCategories(sql, name, topN, includeNulls))
-                        res => ({ [name]: { categories: res.rows } })
+                    ).then(res => ({ [name]: { categories: res.rows } }))
-                    )
                 );
             }
         });
         queries.push(
             queryPromise(
                 ctx.dbConnection,
-                _getSQL(ctx, sql => `SELECT ${aggr.join(',')} FROM (${sql}) AS __cdb_query`),
+                _getSQL(ctx, sql => `SELECT ${aggr.join(',')} FROM (${sql}) AS __cdb_query`)
-                res => {
+            ).then(res => {
-                    let stats = {};
+                let stats = {};
-                    Object.keys(columns).forEach(name => {
+                Object.keys(columns).forEach(name => {
-                        stats[name] = {};
+                    stats[name] = {};
-                        columnAggregations(columns[name]).forEach(fn => {
+                    columnAggregations(columns[name]).forEach(fn => {
-                            stats[name][fn] = res.rows[0][`${name}_${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(

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}
     `;
 }
 

package.json

@@ -26,8 +26,8 @@
   "dependencies": {
     "basic-auth": "2.0.0",
     "body-parser": "1.18.3",
-    "camshaft": "0.61.9",
+    "camshaft": "0.61.10",
-    "cartodb-psql": "0.10.2",
+    "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": {

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',

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:
+camshaft@0.61.10:
-  version "0.61.9"
+  version "0.61.10"
-  resolved "https://registry.yarnpkg.com/camshaft/-/camshaft-0.61.9.tgz#f3d399dfacf51b6a492c579e925c8b1141b22974"
+  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:
+cartodb-psql@0.11.0:
-  version "0.10.2"
+  version "0.11.0"
-  resolved "https://registry.yarnpkg.com/cartodb-psql/-/cartodb-psql-0.10.2.tgz#8c505066e4a635cfa0ee4c603769c83f6e2187dd"
+  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:
+packet-reader@0.3.1:
-  version "0.2.0"
+  version "0.3.1"
-  resolved "https://registry.yarnpkg.com/packet-reader/-/packet-reader-0.2.0.tgz#819df4d010b82d5ea5671f8a1a3acf039bcd7700"
+  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:
+"pg@github:CartoDB/node-postgres#6.4.2-cdb1":
-  version "6.1.6"
+  version "6.4.2"
-  resolved "https://codeload.github.com/cartodb/node-postgres/tar.gz/3eef52dd1e655f658a4ee8ac5697688b3ecfed44"
+  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:
+windshaft@4.8.1:
-  version "4.8.0"
+  version "4.8.1"
-  resolved "https://registry.yarnpkg.com/windshaft/-/windshaft-4.8.0.tgz#79ea03ee78fa68288e07c7aa01a9fdabe3cc5af0"
+  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"