From 11cdcc65ad0e570e57ede263d65d82c0dd062899 Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Mon, 21 May 2018 12:45:16 +0200
Subject: [PATCH] Add safety limit to sample metadata

The sampling probability is now being computed using an estimate of the table row count
This could led to too high probabilities (to large samples) if the estimate is not accurate.
To avoid potential problems with large samples we've added a LIMIT to the sampling queries.
---
 lib/cartodb/backends/layer-stats/mapnik-layer-stats.js | 5 +++--
 lib/cartodb/utils/query-utils.js                       | 9 ++++++---
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js
index 689edb9b..0390536b 100644
--- a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js
+++ b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js
@@ -172,13 +172,14 @@ function mergeColumns(results) {
     }
 }
 
-
 function _sample(ctx, numRows) {
     if (ctx.metaOptions.sample) {
         const sampleProb = Math.min(ctx.metaOptions.sample / 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)),
+            _getSQL(ctx, sql => queryUtils.getQuerySample(sql, sampleProb, limit)),
             res => ({ sample: res.rows })
         );
     }
diff --git a/lib/cartodb/utils/query-utils.js b/lib/cartodb/utils/query-utils.js
index 0bbd38c0..a1276f5e 100644
--- a/lib/cartodb/utils/query-utils.js
+++ b/lib/cartodb/utils/query-utils.js
@@ -88,11 +88,12 @@ module.exports.getQueryTopCategories = function(query, column, topN, includeNull
     `;
 };
 
-module.exports.getQuerySample = function(query, sampleProb, randomSeed = 0.5) {
+module.exports.getQuerySample = function(query, sampleProb, limit = null, randomSeed = 0.5) {
     const singleTable = simpleQueryTable(query);
     if (singleTable) {
         return getTableSample(singleTable.table, singleTable.columns, sampleProb, randomSeed);
     }
+    const limitClause = limit ? `LIMIT ${limit}` : '';
     return `
         WITH __cdb_rndseed AS (
             SELECT setseed(${randomSeed})
@@ -100,14 +101,16 @@ module.exports.getQuerySample = function(query, sampleProb, randomSeed = 0.5) {
         SELECT *
             FROM (${query}) AS __cdb_query
             WHERE random() < ${sampleProb}
+            ${limitClause}
     `;
 };
 
-function getTableSample(table, columns, sampleProb, randomSeed) {
+function getTableSample(table, columns, sampleProb, limit = null, randomSeed = 0.5) {
+    const limitClause = limit ? `LIMIT ${limit}` : '';
     sampleProb *= 100;
     randomSeed *= Math.pow(2, 31) -1;
     return `
-        SELECT ${columns} FROM ${table} TABLESAMPLE BERNOULLI (${sampleProb}) REPEATABLE (${randomSeed})
+        SELECT ${columns} FROM ${table} TABLESAMPLE BERNOULLI (${sampleProb}) REPEATABLE (${randomSeed}) ${limitClause}
     `;
 }