diff --git a/NEWS.md b/NEWS.md index 2190f597..ea9f57c0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -5,6 +5,7 @@ Released 2019-XX-XX Announcements: +- Improved efficiency of query samples while instatiating a map (#1120). - Cache control header fine tuning. Set a shorter value for "max-age" directive if there is no way to know when to trigger the invalidation. - Update deps: - Update `cartodb-query-tables` to version [`0.5.0`](https://github.com/CartoDB/node-cartodb-query-tables/releases/tag/0.5.0) diff --git a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js index a8a181cc..3d229799 100644 --- a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js +++ b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js @@ -125,22 +125,43 @@ 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.num_rows / numRows, 1); - // We'll use a safety limit just in case numRows is a bad estimate - 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 queryUtils.queryPromise(ctx.dbConnection, _getSQL( - ctx, - sql => queryUtils.getQuerySample(sql, sampleProb, limit, SAMPLE_SEED, columns) - )).then(res => ({ sample: res.rows })); +function _sample(ctx) { + if (!ctx.metaOptions.sample) { + return Promise.resolve(); } - return Promise.resolve(); + + const limit = ctx.metaOptions.sample.num_rows || DEFAULT_SAMPLE_ROWS; + const columns = ctx.metaOptions.sample.include_columns; + + const sqlMaxMin = _getSQL(ctx, sql => queryUtils.getMaxMinSpanColumnQuery(sql)); + return queryUtils.queryPromise(ctx.dbConnection, sqlMaxMin) + .then(maxMinRes => { + const { min_id: min, id_span: span } = maxMinRes.rows[0]; + + if (!min || !span) { + return { rows: {} }; + } + + const values = _getSampleValuesFromRange(min, span, limit); + const sqlSample = _getSQL(ctx, sql => queryUtils.getSampleFromIdsQuery(sql, values, columns)); + + return queryUtils.queryPromise(ctx.dbConnection, sqlSample); + }) + .then(res => ({ sample: res.rows })); +} + +function _getSampleValuesFromRange (min, span, limit) { + const sample = new Set(); + + limit = limit < span ? limit : span; + + while (sample.size < limit) { + sample.add(Math.floor(min + Math.random() * span)); + } + + return Array.from(sample); } function _columnsMetadataRequired(options) { @@ -294,8 +315,8 @@ function (layer, dbConnection, callback) { Promise.all([ _estimatedFeatureCount(ctx).then( - ({ estimatedFeatureCount }) => _sample(ctx, estimatedFeatureCount) - .then(sampleResults => mergeResults([sampleResults, { estimatedFeatureCount }])) + ({ estimatedFeatureCount }) => _sample(ctx) + .then(sampleResults => mergeResults([ sampleResults, { estimatedFeatureCount }] )) ), _featureCount(ctx), _aggrFeatureCount(ctx), diff --git a/lib/cartodb/utils/query-utils.js b/lib/cartodb/utils/query-utils.js index f5daf0e1..8ef03c34 100644 --- a/lib/cartodb/utils/query-utils.js +++ b/lib/cartodb/utils/query-utils.js @@ -98,58 +98,24 @@ function columnSelector(columns) { 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, columns || singleTable.columns, sampleProb, limit, randomSeed); - } - const limitClause = limit ? `LIMIT ${limit}` : ''; +module.exports.getMaxMinSpanColumnQuery = function (query, column = 'cartodb_id') { return ` - WITH __cdb_rndseed AS ( - SELECT setseed(${randomSeed}) - ) - SELECT ${columnSelector(columns)} - FROM (${substituteDummyTokens(query)}) AS __cdb_query - WHERE random() < ${sampleProb} - ${limitClause} + SELECT + min(${column}) AS min_id, + max(${column}) AS max_id, + (max(${column}) - min(${column})) AS id_span + FROM (${substituteDummyTokens(query)}) _cdb_metadata_max_min_span; `; }; -function getTableSample(table, columns, sampleProb, limit = null, randomSeed = 0.5) { - const limitClause = limit ? `LIMIT ${limit}` : ''; - sampleProb *= 100; - randomSeed *= Math.pow(2, 31) - 1; +module.exports.getSampleFromIdsQuery = function (query, ids, columns, column = 'cartodb_id') { return ` - SELECT ${columnSelector(columns)} - FROM ${table} - TABLESAMPLE BERNOULLI (${sampleProb}) REPEATABLE (${randomSeed}) ${limitClause} + SELECT + ${columnSelector(columns)} + FROM (${substituteDummyTokens(query)}) _cdb_metadata_sample + WHERE ${column} IN (${ids.join(',')}) `; -} - -function simpleQueryTable(sql) { - const basicQuery = - /\s*SELECT\s+([\*a-z0-9_,\s]+?)\s+FROM\s+((\"[^"]+\"|[a-z0-9_]+)\.)?(\"[^"]+\"|[a-z0-9_]+)\s*;?\s*/i; - const unwrappedQuery = new RegExp('^' + basicQuery.source + '$', 'i'); - // queries for named maps are wrapped like this: - var wrappedQuery = new RegExp( - '^\\s*SELECT\\s+\\*\\s+FROM\\s+\\(' + - basicQuery.source + - '\\)\\s+AS\\s+wrapped_query\\s+WHERE\\s+\\d+=1\\s*$', - 'i' - ); - let match = sql.match(unwrappedQuery); - if (!match) { - match = sql.match(wrappedQuery); - } - if (match) { - const columns = match[1]; - const schema = match[3]; - const table = match[4]; - return { table: schema ? `${schema}.${table}` : table, columns }; - } - return false; -} - +}; function getQueryLimited(query, limit = 0) { return ` diff --git a/test/acceptance/stats/mapnik_stats_layergroup.js b/test/acceptance/stats/mapnik_stats_layergroup.js index dde4f059..55ca7a64 100644 --- a/test/acceptance/stats/mapnik_stats_layergroup.js +++ b/test/acceptance/stats/mapnik_stats_layergroup.js @@ -583,6 +583,33 @@ describe(`[${desc}] Create mapnik layergroup`, function() { }); }); + it('should not provide a sample when the source table is empty', function (done) { + var testClient = new TestClient({ + version: "1.4.0", + layers: [ + { + type: "mapnik", + options: { + sql: "SELECT * FROM test_table_100 limit 0", + cartocss_version: "2.3.0", + cartocss: "#layer { line-width:16; }", + metadata: { + sample: { + num_rows: 30 + } + } + } + } + ] + }); + + testClient.getLayergroup(function(err, layergroup) { + assert.ifError(err); + assert.deepStrictEqual(layergroup.metadata.layers[0].meta.stats.sample, {}); + testClient.drain(done); + }); + }); + it('can specify sample columns', function(done) { var testClient = new TestClient({ version: '1.4.0',