From fa19f90a6a41042e02c836389df8b07aea40ab02 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 11 May 2016 18:16:18 +0200 Subject: [PATCH 01/38] Apply overviews query rewriter to dataviews This requires the QueryRewriter to handle a filters parametes in its data (with Camshaft filter definitions) and a final options parameters with a bounding_box parameter. --- lib/cartodb/backends/dataview.js | 37 ++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/lib/cartodb/backends/dataview.js b/lib/cartodb/backends/dataview.js index 510709c4..946ea0d8 100644 --- a/lib/cartodb/backends/dataview.js +++ b/lib/cartodb/backends/dataview.js @@ -9,6 +9,10 @@ var Timer = require('../stats/timer'); var BBoxFilter = require('../models/filter/bbox'); var DataviewFactory = require('../models/dataview/factory'); +var OverviewsQueryRewriter = require('../utils/overviews_query_rewriter'); +var overviewsQueryRewriter = new OverviewsQueryRewriter({ + zoom_level: 'CDB_ZoomFromScale(!scale_denominator!)' +}); function DataviewBackend(analysisBackend) { this.analysisBackend = analysisBackend; @@ -101,12 +105,31 @@ DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, param ownFilter = !!ownFilter; var query; - if (ownFilter) { - query = node.getQuery(); - } else { - var applyFilters = {}; - applyFilters[dataviewName] = false; - query = node.getQuery(applyFilters); + + var sourceId = dataviewDefinition.source.id; // node.id + var layer = _.find(mapConfig.obj().layers, function(l){ return l.options.source && (l.options.source.id === sourceId); }) + var queryRewriteData = layer.options.query_rewrite_data; + if ( queryRewriteData ) { + if ( node.type == 'source' ) { + var filters = node.filters; // TODO: node.getFilters() when available in camshaft + var filters_disabler = _.keys(filters).reduce(function(disabler, filter_id){ disabler[filter_id] = false; return disabler; }, {}); + var unfiltered_query = node.getQuery(filters_disabler); + queryRewriteData = _.extend({}, queryRewriteData, { filters: filters }); + var rewritten_query = overviewsQueryRewriter.query(unfiltered_query, queryRewriteData, { bounding_box: params.bbox }); + if ( rewritten_query !== unfiltered_query ) { + query = rewritten_query; + } + } + } + + if ( !query ) { + if (ownFilter) { + query = node.getQuery(); + } else { + var applyFilters = {}; + applyFilters[dataviewName] = false; + query = node.getQuery(applyFilters); + } } if (params.bbox) { @@ -277,4 +300,4 @@ function dbParamsFromReqParams(params) { dbParams.dbname = params.dbname; } return dbParams; -} \ No newline at end of file +} From 65612f0109575f3094b574e0d1e6a748de87670b Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 11 May 2016 19:24:13 +0200 Subject: [PATCH 02/38] Add filters information at map instantion time to the query rewriter data --- lib/cartodb/backends/dataview.js | 2 +- lib/cartodb/controllers/map.js | 1 + .../models/mapconfig/named_map_provider.js | 2 +- .../models/mapconfig_overviews_adapter.js | 17 +++++++++++++++-- 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/lib/cartodb/backends/dataview.js b/lib/cartodb/backends/dataview.js index 946ea0d8..6c9f8a15 100644 --- a/lib/cartodb/backends/dataview.js +++ b/lib/cartodb/backends/dataview.js @@ -114,7 +114,7 @@ DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, param var filters = node.filters; // TODO: node.getFilters() when available in camshaft var filters_disabler = _.keys(filters).reduce(function(disabler, filter_id){ disabler[filter_id] = false; return disabler; }, {}); var unfiltered_query = node.getQuery(filters_disabler); - queryRewriteData = _.extend({}, queryRewriteData, { filters: filters }); + queryRewriteData = _.extend({}, queryRewriteData, { filters: filters, unfiltered_query: unfiltered_query }); var rewritten_query = overviewsQueryRewriter.query(unfiltered_query, queryRewriteData, { bounding_box: params.bbox }); if ( rewritten_query !== unfiltered_query ) { query = rewritten_query; diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 442079e0..d5307d12 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -186,6 +186,7 @@ MapController.prototype.create = function(req, res, prepareConfigFn) { assert.ifError(err); var next = this; self.overviewsAdapter.getLayers(req.context.user, requestMapConfig.layers, function(err, layers) { + self.overviewsAdapter.getLayers(req.context.user, requestMapConfig.layers, analysesResults, function(err, layers) { if (err) { return next(err); } diff --git a/lib/cartodb/models/mapconfig/named_map_provider.js b/lib/cartodb/models/mapconfig/named_map_provider.js index 9c8c218d..e285b2b7 100644 --- a/lib/cartodb/models/mapconfig/named_map_provider.js +++ b/lib/cartodb/models/mapconfig/named_map_provider.js @@ -142,7 +142,7 @@ NamedMapMapConfigProvider.prototype.getMapConfig = function(callback) { assert.ifError(err); var next = this; - self.overviewsAdapter.getLayers(self.owner, _mapConfig.layers, function(err, layers) { + self.overviewsAdapter.getLayers(self.owner, _mapConfig.layers, self.analysesResults, function(err, layers) { if (err) { return next(err); } diff --git a/lib/cartodb/models/mapconfig_overviews_adapter.js b/lib/cartodb/models/mapconfig_overviews_adapter.js index 70a02dc5..f0587d1c 100644 --- a/lib/cartodb/models/mapconfig_overviews_adapter.js +++ b/lib/cartodb/models/mapconfig_overviews_adapter.js @@ -7,7 +7,7 @@ function MapConfigOverviewsAdapter(overviewsMetadataApi) { module.exports = MapConfigOverviewsAdapter; -MapConfigOverviewsAdapter.prototype.getLayers = function(username, layers, callback) { +MapConfigOverviewsAdapter.prototype.getLayers = function(username, layers, analysesResults, callback) { var self = this; if (!layers || layers.length === 0) { @@ -24,9 +24,22 @@ MapConfigOverviewsAdapter.prototype.getLayers = function(username, layers, callb if (err) { done(err, layer); } else { + var query_rewrite_data = { overviews: metadata }; + if ( layer.options.source && analysesResults ) { + var sourceId = layer.options.source.id; + var node = _.find(analysesResults, function(a){ a.rootNode.params.id === sourceId }); + if ( node ) { + node = node.roorNode; + var filters = node.filters; // TODO: node.getFilters() when available in camshaft + var filters_disabler = _.keys(filters).reduce(function(disabler, filter_id){ disabler[filter_id] = false; return disabler; }, {}); + var unfiltered_query = node.getQuery(filters_disabler); + query_rewrite_data.filters = filters; + query_rewrite_data.unfiltered_query = unfiltered_query; + } + } if ( !_.isEmpty(metadata) ) { layer = _.extend({}, layer); - layer.options = _.extend({}, layer.options, { query_rewrite_data: { overviews: metadata } }); + layer.options = _.extend({}, layer.options, { query_rewrite_data: query_rewrite_data }); } done(null, layer); } From 3890014250aa5ae9ae3892ba49d2004230c49e3c Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 12 May 2016 10:25:09 +0200 Subject: [PATCH 03/38] Fix QueryRewriter use QueryRewriter should be passed the query that would be used otherwise. If QR cannot handle it, it will be returned unmodified. So QR must be used when a query has been prepared and the result of QR should be used to replace it. --- lib/cartodb/backends/dataview.js | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/lib/cartodb/backends/dataview.js b/lib/cartodb/backends/dataview.js index 6c9f8a15..fc22804b 100644 --- a/lib/cartodb/backends/dataview.js +++ b/lib/cartodb/backends/dataview.js @@ -106,6 +106,14 @@ DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, param var query; + if (ownFilter) { + query = node.getQuery(); + } else { + var applyFilters = {}; + applyFilters[dataviewName] = false; + query = node.getQuery(applyFilters); + } + var sourceId = dataviewDefinition.source.id; // node.id var layer = _.find(mapConfig.obj().layers, function(l){ return l.options.source && (l.options.source.id === sourceId); }) var queryRewriteData = layer.options.query_rewrite_data; @@ -115,20 +123,7 @@ DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, param var filters_disabler = _.keys(filters).reduce(function(disabler, filter_id){ disabler[filter_id] = false; return disabler; }, {}); var unfiltered_query = node.getQuery(filters_disabler); queryRewriteData = _.extend({}, queryRewriteData, { filters: filters, unfiltered_query: unfiltered_query }); - var rewritten_query = overviewsQueryRewriter.query(unfiltered_query, queryRewriteData, { bounding_box: params.bbox }); - if ( rewritten_query !== unfiltered_query ) { - query = rewritten_query; - } - } - } - - if ( !query ) { - if (ownFilter) { - query = node.getQuery(); - } else { - var applyFilters = {}; - applyFilters[dataviewName] = false; - query = node.getQuery(applyFilters); + query = overviewsQueryRewriter.query(query, queryRewriteData, { bounding_box: params.bbox }); } } From 64a87690eeec6506afdfd04fed81ec79baf493a1 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 12 May 2016 16:20:34 +0200 Subject: [PATCH 04/38] :lipstick: Fix line lengths, etc. --- lib/cartodb/backends/dataview.js | 17 ++++++++++---- lib/cartodb/controllers/map.js | 22 ++++++++++--------- .../models/mapconfig_overviews_adapter.js | 7 ++++-- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/lib/cartodb/backends/dataview.js b/lib/cartodb/backends/dataview.js index fc22804b..b5e57f0e 100644 --- a/lib/cartodb/backends/dataview.js +++ b/lib/cartodb/backends/dataview.js @@ -115,14 +115,23 @@ DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, param } var sourceId = dataviewDefinition.source.id; // node.id - var layer = _.find(mapConfig.obj().layers, function(l){ return l.options.source && (l.options.source.id === sourceId); }) + var layer = _.find( + mapConfig.obj().layers, + function(l){ return l.options.source && (l.options.source.id === sourceId); } + ); var queryRewriteData = layer.options.query_rewrite_data; if ( queryRewriteData ) { - if ( node.type == 'source' ) { + if ( node.type === 'source' ) { var filters = node.filters; // TODO: node.getFilters() when available in camshaft - var filters_disabler = _.keys(filters).reduce(function(disabler, filter_id){ disabler[filter_id] = false; return disabler; }, {}); + var filters_disabler = _.keys(filters).reduce( + function(disabler, filter_id){ disabler[filter_id] = false; return disabler; }, + {} + ); var unfiltered_query = node.getQuery(filters_disabler); - queryRewriteData = _.extend({}, queryRewriteData, { filters: filters, unfiltered_query: unfiltered_query }); + queryRewriteData = _.extend( + {}, + queryRewriteData, { filters: filters, unfiltered_query: unfiltered_query } + ); query = overviewsQueryRewriter.query(query, queryRewriteData, { bounding_box: params.bbox }); } } diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index d5307d12..feaf6fa5 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -185,18 +185,20 @@ MapController.prototype.create = function(req, res, prepareConfigFn) { function addOverviewsInformation(err, requestMapConfig, datasource) { assert.ifError(err); var next = this; - self.overviewsAdapter.getLayers(req.context.user, requestMapConfig.layers, function(err, layers) { - self.overviewsAdapter.getLayers(req.context.user, requestMapConfig.layers, analysesResults, function(err, layers) { - if (err) { - return next(err); - } + self.overviewsAdapter.getLayers( + req.context.user, requestMapConfig.layers, analysesResults, + function(err, layers) { + if (err) { + return next(err); + } - if (layers) { - requestMapConfig.layers = layers; - } + if (layers) { + requestMapConfig.layers = layers; + } - return next(null, requestMapConfig, datasource); - }); + return next(null, requestMapConfig, datasource); + } + ); }, function parseTurboCarto(err, requestMapConfig, datasource) { assert.ifError(err); diff --git a/lib/cartodb/models/mapconfig_overviews_adapter.js b/lib/cartodb/models/mapconfig_overviews_adapter.js index f0587d1c..ca16957e 100644 --- a/lib/cartodb/models/mapconfig_overviews_adapter.js +++ b/lib/cartodb/models/mapconfig_overviews_adapter.js @@ -27,11 +27,14 @@ MapConfigOverviewsAdapter.prototype.getLayers = function(username, layers, analy var query_rewrite_data = { overviews: metadata }; if ( layer.options.source && analysesResults ) { var sourceId = layer.options.source.id; - var node = _.find(analysesResults, function(a){ a.rootNode.params.id === sourceId }); + var node = _.find(analysesResults, function(a){ return a.rootNode.params.id === sourceId; }); if ( node ) { node = node.roorNode; var filters = node.filters; // TODO: node.getFilters() when available in camshaft - var filters_disabler = _.keys(filters).reduce(function(disabler, filter_id){ disabler[filter_id] = false; return disabler; }, {}); + var filters_disabler = _.keys(filters).reduce( + function(disabler, filter_id){ disabler[filter_id] = false; return disabler; }, + {} + ); var unfiltered_query = node.getQuery(filters_disabler); query_rewrite_data.filters = filters; query_rewrite_data.unfiltered_query = unfiltered_query; From 55cf0a84477af6e2057c10c47b08cd4100cb4efc Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 12 May 2016 16:43:09 +0200 Subject: [PATCH 05/38] Fix typo --- lib/cartodb/models/mapconfig_overviews_adapter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/models/mapconfig_overviews_adapter.js b/lib/cartodb/models/mapconfig_overviews_adapter.js index ca16957e..47185e00 100644 --- a/lib/cartodb/models/mapconfig_overviews_adapter.js +++ b/lib/cartodb/models/mapconfig_overviews_adapter.js @@ -29,7 +29,7 @@ MapConfigOverviewsAdapter.prototype.getLayers = function(username, layers, analy var sourceId = layer.options.source.id; var node = _.find(analysesResults, function(a){ return a.rootNode.params.id === sourceId; }); if ( node ) { - node = node.roorNode; + node = node.rootNode; var filters = node.filters; // TODO: node.getFilters() when available in camshaft var filters_disabler = _.keys(filters).reduce( function(disabler, filter_id){ disabler[filter_id] = false; return disabler; }, From fd44b62f260acdad5d2ab18155b0b80d9bcef763 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 12 May 2016 17:52:39 +0200 Subject: [PATCH 06/38] Fix tests for new MapConfigOverviewsAdapter interface --- test/integration/mapconfig_overviews_adapter.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/mapconfig_overviews_adapter.js b/test/integration/mapconfig_overviews_adapter.js index ebc70635..5c425ec1 100644 --- a/test/integration/mapconfig_overviews_adapter.js +++ b/test/integration/mapconfig_overviews_adapter.js @@ -36,7 +36,7 @@ describe('MapConfigOverviewsAdapter', function() { } }; - mapConfigOverviewsAdapter.getLayers('localhost', [layer_without_overviews], function(err, layers) { + mapConfigOverviewsAdapter.getLayers('localhost', [layer_without_overviews], [], function(err, layers) { assert.ok(!err); assert.equal(layers.length, 1); assert.equal(layers[0].type, 'cartodb'); @@ -64,7 +64,7 @@ describe('MapConfigOverviewsAdapter', function() { } }; - mapConfigOverviewsAdapter.getLayers('localhost', [layer_without_overviews], function(err, layers) { + mapConfigOverviewsAdapter.getLayers('localhost', [layer_without_overviews], [], function(err, layers) { assert.ok(!err); assert.equal(layers.length, 1); assert.equal(layers[0].type, 'cartodb'); From 5fb7f07498f38d3ecab2da46b744e7473db2eee6 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 12 May 2016 18:29:30 +0200 Subject: [PATCH 07/38] Prevent problems with missing layers in mapconfig --- lib/cartodb/backends/dataview.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/backends/dataview.js b/lib/cartodb/backends/dataview.js index b5e57f0e..a6b5f549 100644 --- a/lib/cartodb/backends/dataview.js +++ b/lib/cartodb/backends/dataview.js @@ -119,7 +119,7 @@ DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, param mapConfig.obj().layers, function(l){ return l.options.source && (l.options.source.id === sourceId); } ); - var queryRewriteData = layer.options.query_rewrite_data; + var queryRewriteData = layer && layer.options.query_rewrite_data; if ( queryRewriteData ) { if ( node.type === 'source' ) { var filters = node.filters; // TODO: node.getFilters() when available in camshaft From 85788f42a65df0083840fc6da9e1db5034def736 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 12 May 2016 18:30:10 +0200 Subject: [PATCH 08/38] Adapt QueryRewriter to new requirements --- lib/cartodb/utils/overviews_query_rewriter.js | 146 ++++++++++++++---- 1 file changed, 120 insertions(+), 26 deletions(-) diff --git a/lib/cartodb/utils/overviews_query_rewriter.js b/lib/cartodb/utils/overviews_query_rewriter.js index b5b35503..4a23975c 100644 --- a/lib/cartodb/utils/overviews_query_rewriter.js +++ b/lib/cartodb/utils/overviews_query_rewriter.js @@ -1,5 +1,10 @@ +var _ = require('underscore'); var TableNameParser = require('./table_name_parser'); +// this is meant as a hack for development (otherwise we'd be coupling +// this to Camshaft impolementation) +var queryBuilder = require('camshaft/lib/filter/query-builder'); + function OverviewsQueryRewriter(options) { this.options = options; @@ -119,26 +124,34 @@ function replace_table_in_query(sql, old_table_name, replacement) { return sql.replace(new RegExp(regexp, 'g'), replacement); } -function overviews_query(query, overviews, zoom_level_expression) { + +function replace_table_in_query_with_schema(query, table, schema, replacement) { + if ( replacement ) { + query = replace_table_in_query(query, table, replacement); + var parsed_table = TableNameParser.parse(table); + if (!parsed_table.schema && schema) { + // replace also the qualified table name, if the table wasn't qualified + parsed_table.schema = schema; + table = TableNameParser.table_identifier(parsed_table); + query = replace_table_in_query(query, table, replacement); + } + } + return query; +} + +// Build query to use overviews for a variant zoom level (given by a expression to +// be evaluated by the database server) +function overviews_query_with_zoom_expression(query, overviews, zoom_level_expression) { var replaced_query = query; var sql = "WITH\n _vovw_scale AS ( SELECT " + zoom_level_expression + " AS _vovw_z )"; var replacement; - for ( var table in overviews ) { - if (overviews.hasOwnProperty(table)) { - var table_overviews = overviews[table]; - var table_view = overviews_view_name(table); - var schema = table_overviews.schema; - replacement = "(\n" + overviews_view_for_table(table, table_overviews) + "\n ) AS " + table_view; - replaced_query = replace_table_in_query(replaced_query, table, replacement); - var parsed_table = TableNameParser.parse(table); - if (!parsed_table.schema && schema) { - // replace also the qualified table name, if the table wasn't qualified - parsed_table.schema = schema; - table = TableNameParser.table_identifier(parsed_table); - replaced_query = replace_table_in_query(replaced_query, table, replacement); - } - } - } + _.each(_.keys(overviews), function(table) { + var table_overviews = overviews[table]; + var table_view = overviews_view_name(table); + var schema = table_overviews.schema; + replacement = "(\n" + overviews_view_for_table(table, table_overviews) + "\n ) AS " + table_view; + replaced_query = replace_table_in_query_with_schema(replaced_query, table, schema, replacement); + }); if ( replaced_query !== query ) { sql += "\n"; sql += replaced_query; @@ -148,6 +161,54 @@ function overviews_query(query, overviews, zoom_level_expression) { return sql; } +// Build query to use overviews for a specific zoom level value +function overviews_query_with_definite_zoom(query, overviews, zoom_level) { + var replaced_query = query; + var replacement; + _.each(_.keys(overviews), function(table) { + var table_overviews = overviews[table]; + var schema = table_overviews.schema; + replacement = overview_table_for_zoom_level(table_overviews, zoom_level); + replaced_query = replace_table_in_query_with_schema(replaced_query, table, schema, replacement); + }); + return replaced_query; +} + +// Find a suitable overview table for a specific zoom_level +function overview_table_for_zoom_level(table_overviews, zoom_level) { + var overview_table; + if ( table_overviews ) { + overview_table = table_overviews[zoom_level]; + if ( !overview_table ) { + _.every(_.keys(table_overviews).sort(function(x,y){ return x-y; }), function(overview_zoom) { + if ( +overview_zoom > +zoom_level ) { + overview_table = table_overviews[overview_zoom]; + return false; + } else { + return true; + } + }); + } + } + if ( overview_table ) { + overview_table = overview_table.table; + } + return overview_table; +} + +function apply_filters_to_query(query, filters) { + // TODO: it'd be nice to have a public API in camshaft to apply filters! + if ( filters && !_.isEmpty(filters)) { + query = queryBuilder.getSql(query, filters); + } + return query; +} + +function zoom_level_for_bounding_box(query, bounding_box) { + // TODO: implement + return '100'; // force use of base table +} + // Transform an SQL query so that it uses overviews. // overviews contains metadata about the overviews to be used: // { 'table-name': {1: { table: 'overview-table-1' }, ... }, ... } @@ -166,13 +227,50 @@ function overviews_query(query, overviews, zoom_level_expression) { // doesnn't prevent substitution inside literals). // But the transformation will currently only be applied to simple queries // of the form detected by the overviews_supported_query function. -OverviewsQueryRewriter.prototype.query = function(query, data) { - var overviews = this.overviews_metadata(data); - if ( !overviews || !this.is_supported_query(query)) { +OverviewsQueryRewriter.prototype.query = function(query, data, options) { + options = options || {}; + + var overviews = data && data.overviews; + var unfiltered_query = data && data.unfiltered_query; + var filters = data && data.filters; + + if ( !unfiltered_query ) { + unfiltered_query = query; + } + + if ( !overviews || !this.is_supported_query(unfiltered_query)) { return query; } - var zoom_level_expression = this.options.zoom_level || '0'; - return overviews_query(query, overviews, zoom_level_expression); + + var rewritten_query; + + var zoom_level_expression = this.options.zoom_level; + var zoom_level; + if ( _.has(options, 'bounding_box') ) { + // The presence of this key, even if it has a null value indicates the + // query cannot handle dynamic zoom expressinos (is not for the tiler) + zoom_level = zoom_level_for_bounding_box(unfiltered_query, options.bounding_box); + } else if ( options.zoom_level ) { + zoom_level = options.zoom_level; + } + if ( !zoom_level && !zoom_level_expression ) { + zoom_level = '0'; + } + + if ( zoom_level || zoom_level === '0' || zoom_level === 0 ) { + rewritten_query = overviews_query_with_definite_zoom(unfiltered_query, overviews, zoom_level); + } else { + rewritten_query = overviews_query_with_zoom_expression(unfiltered_query, overviews, zoom_level_expression); + } + + if ( rewritten_query === unfiltered_query ) { + // could not or didn't need to alter the query + rewritten_query = query; + } else { + rewritten_query = apply_filters_to_query(rewritten_query, filters); + } + + return rewritten_query; }; OverviewsQueryRewriter.prototype.is_supported_query = function(sql) { @@ -188,7 +286,3 @@ OverviewsQueryRewriter.prototype.is_supported_query = function(sql) { ); return !!(sql.match(unwrapped_query) || sql.match(wrapped_query)); }; - -OverviewsQueryRewriter.prototype.overviews_metadata = function(data) { - return data && data.overviews; -}; From b574489950348bdff2813315653231b2de72b841 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 12 May 2016 18:47:24 +0200 Subject: [PATCH 09/38] Refactor to reduce cyclomatic complexity --- lib/cartodb/utils/overviews_query_rewriter.js | 43 ++++++++++++------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/lib/cartodb/utils/overviews_query_rewriter.js b/lib/cartodb/utils/overviews_query_rewriter.js index 4a23975c..bf20d6c9 100644 --- a/lib/cartodb/utils/overviews_query_rewriter.js +++ b/lib/cartodb/utils/overviews_query_rewriter.js @@ -206,6 +206,7 @@ function apply_filters_to_query(query, filters) { function zoom_level_for_bounding_box(query, bounding_box) { // TODO: implement + // jshint unused:false return '100'; // force use of base table } @@ -227,6 +228,7 @@ function zoom_level_for_bounding_box(query, bounding_box) { // doesnn't prevent substitution inside literals). // But the transformation will currently only be applied to simple queries // of the form detected by the overviews_supported_query function. +// TODO: document recent changes (options bounding_box, zoom_level, filters...) OverviewsQueryRewriter.prototype.query = function(query, data, options) { options = options || {}; @@ -245,23 +247,9 @@ OverviewsQueryRewriter.prototype.query = function(query, data, options) { var rewritten_query; var zoom_level_expression = this.options.zoom_level; - var zoom_level; - if ( _.has(options, 'bounding_box') ) { - // The presence of this key, even if it has a null value indicates the - // query cannot handle dynamic zoom expressinos (is not for the tiler) - zoom_level = zoom_level_for_bounding_box(unfiltered_query, options.bounding_box); - } else if ( options.zoom_level ) { - zoom_level = options.zoom_level; - } - if ( !zoom_level && !zoom_level_expression ) { - zoom_level = '0'; - } + var zoom_level = zoom_level_for_query(unfiltered_query, zoom_level_expression, options); - if ( zoom_level || zoom_level === '0' || zoom_level === 0 ) { - rewritten_query = overviews_query_with_definite_zoom(unfiltered_query, overviews, zoom_level); - } else { - rewritten_query = overviews_query_with_zoom_expression(unfiltered_query, overviews, zoom_level_expression); - } + rewritten_query = overviews_query(unfiltered_query, overviews, zoom_level, zoom_level_expression); if ( rewritten_query === unfiltered_query ) { // could not or didn't need to alter the query @@ -273,6 +261,29 @@ OverviewsQueryRewriter.prototype.query = function(query, data, options) { return rewritten_query; }; +function zoom_level_for_query(query, zoom_level_expression, options) { + var zoom_level = null; + if ( _.has(options, 'bounding_box') ) { + // The presence of this key, even if it has a null value indicates the + // query cannot handle dynamic zoom expressinos (is not for the tiler) + zoom_level = zoom_level_for_bounding_box(query, options.bounding_box); + } else if ( _.has(options, 'zoom_level') ) { + zoom_level = options.zoom_level; + } + if ( zoom_level === null && !zoom_level_expression ) { + zoom_level = '0'; + } + return zoom_level; +} + +function overviews_query(query, overviews, zoom_level, zoom_level_expression) { + if ( zoom_level || zoom_level === '0' || zoom_level === 0 ) { + return overviews_query_with_definite_zoom(query, overviews, zoom_level); + } else { + return overviews_query_with_zoom_expression(query, overviews, zoom_level_expression); + } +} + OverviewsQueryRewriter.prototype.is_supported_query = function(sql) { var basic_query = /\s*SELECT\s+[\*a-z0-9_,\s]+?\s+FROM\s+((\"[^"]+\"|[a-z0-9_]+)\.)?(\"[^"]+\"|[a-z0-9_]+)\s*;?\s*/i; From 224eb392bacb82730994cf74a1e37e09c95ef20a Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Fri, 13 May 2016 18:46:58 +0200 Subject: [PATCH 10/38] Add overviews-dependent dataviews behaviour Now QueryRewriter is used in dataview objects they can decide whether overviews are applicable, have the oportunity to adapt queries for overviews, etc. This is done by having overviews-related behaviour in models/dataview/overviews and falling back to the regular models/dataview. --- lib/cartodb/backends/dataview.js | 9 +- .../models/dataview/overviews/factory.js | 29 +++++++ .../models/dataview/overviews/formula.js | 84 +++++++++++++++++++ .../models/dataview/overviews/index.js | 3 + lib/cartodb/utils/overviews_query_rewriter.js | 39 ++++----- 5 files changed, 138 insertions(+), 26 deletions(-) create mode 100644 lib/cartodb/models/dataview/overviews/factory.js create mode 100644 lib/cartodb/models/dataview/overviews/formula.js create mode 100644 lib/cartodb/models/dataview/overviews/index.js diff --git a/lib/cartodb/backends/dataview.js b/lib/cartodb/backends/dataview.js index a6b5f549..84a47cf5 100644 --- a/lib/cartodb/backends/dataview.js +++ b/lib/cartodb/backends/dataview.js @@ -8,7 +8,9 @@ var step = require('step'); var Timer = require('../stats/timer'); var BBoxFilter = require('../models/filter/bbox'); + var DataviewFactory = require('../models/dataview/factory'); +var DataviewFactoryWithOverviews = require('../models/dataview/overviews/factory'); var OverviewsQueryRewriter = require('../utils/overviews_query_rewriter'); var overviewsQueryRewriter = new OverviewsQueryRewriter({ zoom_level: 'CDB_ZoomFromScale(!scale_denominator!)' @@ -132,16 +134,15 @@ DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, param {}, queryRewriteData, { filters: filters, unfiltered_query: unfiltered_query } ); - query = overviewsQueryRewriter.query(query, queryRewriteData, { bounding_box: params.bbox }); } } + var dataviewFactory = DataviewFactoryWithOverviews.getFactory(overviewsQueryRewriter, queryRewriteData); if (params.bbox) { var bboxFilter = new BBoxFilter({column: 'the_geom', srid: 4326}, {bbox: params.bbox}); query = bboxFilter.sql(query); } - var overrideParams = _.reduce(_.pick(params, 'start', 'end', 'bins'), function castNumbers(overrides, val, k) { overrides[k] = Number.isFinite(+val) ? +val : val; @@ -150,7 +151,7 @@ DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, param {ownFilter: ownFilter} ); - var dataview = DataviewFactory.getDataview(query, dataviewDefinition); + var dataview = dataviewFactory.getDataview(query, dataviewDefinition); dataview.getResult(pg, overrideParams, this); }, function returnCallback(err, result) { @@ -253,6 +254,8 @@ DataviewBackend.prototype.search = function (mapConfigProvider, user, params, ca query = node.getQuery(applyFilters); } + // TODO: should handle overviews as getDataview ? + if (params.bbox) { var bboxFilter = new BBoxFilter({column: 'the_geom', srid: 4326}, {bbox: params.bbox}); query = bboxFilter.sql(query); diff --git a/lib/cartodb/models/dataview/overviews/factory.js b/lib/cartodb/models/dataview/overviews/factory.js new file mode 100644 index 00000000..b5ce8dd3 --- /dev/null +++ b/lib/cartodb/models/dataview/overviews/factory.js @@ -0,0 +1,29 @@ +var parentFactory = require('../factory'); +var dataviews = require('./'); + +function OverviewsDataviewFactory(queryRewriter, queryRewriteData) { + this.queryRewriter = queryRewriter; + this.queryRewriteData = queryRewriteData; +} + +OverviewsDataviewFactory.prototype.getDataview = function(query, dataviewDefinition) { + var type = dataviewDefinition.type; + var dataviews = OverviewsDataviewMetaFactory.dataviews; + if ( !this.queryRewriter || !this.queryRewriteData || !dataviews[type] ) { + return parentFactory.getDataview(query, dataviewDefinition); + } + return new dataviews[type](query, dataviewDefinition.options, this.queryRewriter, this.queryRewriteData); +}; + +var OverviewsDataviewMetaFactory = { + dataviews: Object.keys(dataviews).reduce(function(allDataviews, dataviewClassName) { + allDataviews[dataviewClassName.toLowerCase()] = dataviews[dataviewClassName]; + return allDataviews; + }, {}), + + getFactory: function(queryRewriter, queryRewriteData) { + return new OverviewsDataviewFactory(queryRewriter, queryRewriteData); + }, +}; + +module.exports = OverviewsDataviewMetaFactory; diff --git a/lib/cartodb/models/dataview/overviews/formula.js b/lib/cartodb/models/dataview/overviews/formula.js new file mode 100644 index 00000000..1144f5dd --- /dev/null +++ b/lib/cartodb/models/dataview/overviews/formula.js @@ -0,0 +1,84 @@ +var BaseWidget = require('../base'); +var BaseDataview = require('../formula'); + +var debug = require('debug')('windshaft:widget:formula:overviews'); + +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', + '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', + 'FROM ({{=it._query}}) _cdb_formula' + ].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', + 'FROM ({{=it._query}}) _cdb_formula' + ].join('\n')), +}; + +function Formula(query, options, queryRewriter, queryRewriteData) { + this.base_dataview = new BaseDataview(query, options); + this.query = query; + this.column = options.column || '1'; + this.operation = options.operation; + this.queryRewriter = queryRewriter; + this.queryRewriteData = queryRewriteData; +} + +Formula.prototype = new BaseWidget(); +Formula.prototype.constructor = Formula; + +module.exports = Formula; + +Formula.prototype.sql = function(psql, filters, override, callback) { + var _query = this.query; + var formulaQueryTpl = formulaQueryTpls[this.operation]; + + if ( formulaQueryTpl ) { + // supported formula for use with overviews + + // TODO: determine zoom level using bounding box so that the resolution + // (grid size) of the overview much smaller thatn the bounding box. + // This could be left to be computed by the queryRewriter passing the + // bounding box to it. + // The bounding box could be passed to the dataview constructor, or + // it could b extracted from the query. + var zoom_level = 0; + + _query = this.queryRewriter.query(_query, this.queryRewriteData, { zoom_level: zoom_level }); + var formulaSql = formulaQueryTpl({ + _query: _query, + _operation: this.operation, + _column: this.column + }); + debug(formulaSql); + callback = callback || override; + return callback(null, formulaSql); + } + + // For non supported operations (min, max) we're not using overviews. + return this.base_dataview.sql(psql, filters, override, callback); +}; + +Formula.prototype.format = function(result) { + return this.base_dataview.format(result); +}; + +Formula.prototype.getType = function() { + return this.base_dataview.getType(); +}; + +Formula.prototype.toString = function() { + return this.base_dataview.toString(); +}; diff --git a/lib/cartodb/models/dataview/overviews/index.js b/lib/cartodb/models/dataview/overviews/index.js new file mode 100644 index 00000000..775ceca1 --- /dev/null +++ b/lib/cartodb/models/dataview/overviews/index.js @@ -0,0 +1,3 @@ +module.exports = { + Formula: require('./formula') +}; diff --git a/lib/cartodb/utils/overviews_query_rewriter.js b/lib/cartodb/utils/overviews_query_rewriter.js index bf20d6c9..4999073b 100644 --- a/lib/cartodb/utils/overviews_query_rewriter.js +++ b/lib/cartodb/utils/overviews_query_rewriter.js @@ -1,9 +1,15 @@ var _ = require('underscore'); var TableNameParser = require('./table_name_parser'); -// this is meant as a hack for development (otherwise we'd be coupling -// this to Camshaft impolementation) +// this is meant as a hack for development var queryBuilder = require('camshaft/lib/filter/query-builder'); +function apply_filters_to_query(query, filters) { + // TODO: implement filter application here + if ( filters && !_.isEmpty(filters)) { + query = queryBuilder.getSql(query, filters); + } + return query; +} function OverviewsQueryRewriter(options) { @@ -196,20 +202,6 @@ function overview_table_for_zoom_level(table_overviews, zoom_level) { return overview_table; } -function apply_filters_to_query(query, filters) { - // TODO: it'd be nice to have a public API in camshaft to apply filters! - if ( filters && !_.isEmpty(filters)) { - query = queryBuilder.getSql(query, filters); - } - return query; -} - -function zoom_level_for_bounding_box(query, bounding_box) { - // TODO: implement - // jshint unused:false - return '100'; // force use of base table -} - // Transform an SQL query so that it uses overviews. // overviews contains metadata about the overviews to be used: // { 'table-name': {1: { table: 'overview-table-1' }, ... }, ... } @@ -228,7 +220,7 @@ function zoom_level_for_bounding_box(query, bounding_box) { // doesnn't prevent substitution inside literals). // But the transformation will currently only be applied to simple queries // of the form detected by the overviews_supported_query function. -// TODO: document recent changes (options bounding_box, zoom_level, filters...) +// TODO: document recent changes (zoom_level option, filters...) OverviewsQueryRewriter.prototype.query = function(query, data, options) { options = options || {}; @@ -255,6 +247,11 @@ OverviewsQueryRewriter.prototype.query = function(query, data, options) { // could not or didn't need to alter the query rewritten_query = query; } else { + // TODO: determine if overviews can be used for the filters. + // Otherwise use the base table. + // Applicability of the filters should depend both on the expected + // row count of the result and on the columns to be filter and the + // way they're agregated in overviews. rewritten_query = apply_filters_to_query(rewritten_query, filters); } @@ -263,12 +260,8 @@ OverviewsQueryRewriter.prototype.query = function(query, data, options) { function zoom_level_for_query(query, zoom_level_expression, options) { var zoom_level = null; - if ( _.has(options, 'bounding_box') ) { - // The presence of this key, even if it has a null value indicates the - // query cannot handle dynamic zoom expressinos (is not for the tiler) - zoom_level = zoom_level_for_bounding_box(query, options.bounding_box); - } else if ( _.has(options, 'zoom_level') ) { - zoom_level = options.zoom_level; + if ( _.has(options, 'zoom_level') ) { + zoom_level = options.zoom_level || '0'; } if ( zoom_level === null && !zoom_level_expression ) { zoom_level = '0'; From 9d82e8c27cd92b492bfed545383b835ef89fd878 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Fri, 13 May 2016 20:47:36 +0200 Subject: [PATCH 11/38] Use bounding box of dataviews to select overviews level --- lib/cartodb/backends/dataview.js | 6 ++- .../models/dataview/overviews/factory.js | 11 +++-- .../models/dataview/overviews/formula.js | 41 ++++++++++++++----- lib/cartodb/utils/overviews_query_rewriter.js | 8 +++- 4 files changed, 49 insertions(+), 17 deletions(-) diff --git a/lib/cartodb/backends/dataview.js b/lib/cartodb/backends/dataview.js index 84a47cf5..d18e1529 100644 --- a/lib/cartodb/backends/dataview.js +++ b/lib/cartodb/backends/dataview.js @@ -136,13 +136,17 @@ DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, param ); } } - var dataviewFactory = DataviewFactoryWithOverviews.getFactory(overviewsQueryRewriter, queryRewriteData); if (params.bbox) { var bboxFilter = new BBoxFilter({column: 'the_geom', srid: 4326}, {bbox: params.bbox}); query = bboxFilter.sql(query); + queryRewriteData = _.extend(queryRewriteData, { bbox_filter: bboxFilter }); } + var dataviewFactory = DataviewFactoryWithOverviews.getFactory( + overviewsQueryRewriter, queryRewriteData, { bbox: params.bbox } + ); + var overrideParams = _.reduce(_.pick(params, 'start', 'end', 'bins'), function castNumbers(overrides, val, k) { overrides[k] = Number.isFinite(+val) ? +val : val; diff --git a/lib/cartodb/models/dataview/overviews/factory.js b/lib/cartodb/models/dataview/overviews/factory.js index b5ce8dd3..b63b004a 100644 --- a/lib/cartodb/models/dataview/overviews/factory.js +++ b/lib/cartodb/models/dataview/overviews/factory.js @@ -1,9 +1,10 @@ var parentFactory = require('../factory'); var dataviews = require('./'); -function OverviewsDataviewFactory(queryRewriter, queryRewriteData) { +function OverviewsDataviewFactory(queryRewriter, queryRewriteData, options) { this.queryRewriter = queryRewriter; this.queryRewriteData = queryRewriteData; + this.options = options; } OverviewsDataviewFactory.prototype.getDataview = function(query, dataviewDefinition) { @@ -12,7 +13,9 @@ OverviewsDataviewFactory.prototype.getDataview = function(query, dataviewDefinit if ( !this.queryRewriter || !this.queryRewriteData || !dataviews[type] ) { return parentFactory.getDataview(query, dataviewDefinition); } - return new dataviews[type](query, dataviewDefinition.options, this.queryRewriter, this.queryRewriteData); + return new dataviews[type]( + query, dataviewDefinition.options, this.queryRewriter, this.queryRewriteData, this.options + ); }; var OverviewsDataviewMetaFactory = { @@ -21,8 +24,8 @@ var OverviewsDataviewMetaFactory = { return allDataviews; }, {}), - getFactory: function(queryRewriter, queryRewriteData) { - return new OverviewsDataviewFactory(queryRewriter, queryRewriteData); + getFactory: function(queryRewriter, queryRewriteData, options) { + return new OverviewsDataviewFactory(queryRewriter, queryRewriteData, options); }, }; diff --git a/lib/cartodb/models/dataview/overviews/formula.js b/lib/cartodb/models/dataview/overviews/formula.js index 1144f5dd..db305e55 100644 --- a/lib/cartodb/models/dataview/overviews/formula.js +++ b/lib/cartodb/models/dataview/overviews/formula.js @@ -1,3 +1,4 @@ +var _ = require('underscore'); var BaseWidget = require('../base'); var BaseDataview = require('../formula'); @@ -27,13 +28,14 @@ var formulaQueryTpls = { ].join('\n')), }; -function Formula(query, options, queryRewriter, queryRewriteData) { +function Formula(query, options, queryRewriter, queryRewriteData, params) { this.base_dataview = new BaseDataview(query, options); this.query = query; this.column = options.column || '1'; this.operation = options.operation; this.queryRewriter = queryRewriter; this.queryRewriteData = queryRewriteData; + this.options = params; } Formula.prototype = new BaseWidget(); @@ -41,21 +43,39 @@ Formula.prototype.constructor = Formula; module.exports = Formula; +var zoom_level_factor = 100.0; + +// Compute zoom level so that the the resolution grid size of the +// selected overview is smaller (zoom_level_factor times smaller at least) +// than the bounding box size. +function zoom_level_for_bbox(bbox) { + var px_per_tile = 256.0; + var earth_width = 360.0; + // TODO: now we assume overviews are computed for 1-pixel tolerance; + // should use extended overviews metadata to compute this properly. + if ( bbox ) { + var bbox_values = _.map(bbox.split(','), function(v) { return +v; }); + // TODO: look at possible crossing-180º issues + var w = Math.abs(bbox_values[2]-bbox_values[0]); + var h = Math.abs(bbox_values[3]-bbox_values[1]); + var max_dim = Math.min(w, h); + + // Find minimum suitable z + // note that the QueryRewirter will use the minimum level overview + // of level >= z if it exists, and otherwise the base table + var z = Math.ceil(-Math.log(max_dim*px_per_tile/earth_width/zoom_level_factor)/Math.log(2.0)); + return Math.max(z, 0); + } + return 0; +} + Formula.prototype.sql = function(psql, filters, override, callback) { var _query = this.query; var formulaQueryTpl = formulaQueryTpls[this.operation]; if ( formulaQueryTpl ) { // supported formula for use with overviews - - // TODO: determine zoom level using bounding box so that the resolution - // (grid size) of the overview much smaller thatn the bounding box. - // This could be left to be computed by the queryRewriter passing the - // bounding box to it. - // The bounding box could be passed to the dataview constructor, or - // it could b extracted from the query. - var zoom_level = 0; - + var zoom_level = zoom_level_for_bbox(this.options.bbox); _query = this.queryRewriter.query(_query, this.queryRewriteData, { zoom_level: zoom_level }); var formulaSql = formulaQueryTpl({ _query: _query, @@ -64,6 +84,7 @@ Formula.prototype.sql = function(psql, filters, override, callback) { }); debug(formulaSql); callback = callback || override; + return callback(null, formulaSql); } diff --git a/lib/cartodb/utils/overviews_query_rewriter.js b/lib/cartodb/utils/overviews_query_rewriter.js index 4999073b..7efff2ed 100644 --- a/lib/cartodb/utils/overviews_query_rewriter.js +++ b/lib/cartodb/utils/overviews_query_rewriter.js @@ -3,11 +3,14 @@ var TableNameParser = require('./table_name_parser'); // this is meant as a hack for development var queryBuilder = require('camshaft/lib/filter/query-builder'); -function apply_filters_to_query(query, filters) { +function apply_filters_to_query(query, filters, bbox_filter) { // TODO: implement filter application here if ( filters && !_.isEmpty(filters)) { query = queryBuilder.getSql(query, filters); } + if ( bbox_filter ) { + query = bbox_filter.sql(query); + } return query; } @@ -227,6 +230,7 @@ OverviewsQueryRewriter.prototype.query = function(query, data, options) { var overviews = data && data.overviews; var unfiltered_query = data && data.unfiltered_query; var filters = data && data.filters; + var bbox_filter = data && data.bbox_filter; if ( !unfiltered_query ) { unfiltered_query = query; @@ -252,7 +256,7 @@ OverviewsQueryRewriter.prototype.query = function(query, data, options) { // Applicability of the filters should depend both on the expected // row count of the result and on the columns to be filter and the // way they're agregated in overviews. - rewritten_query = apply_filters_to_query(rewritten_query, filters); + rewritten_query = apply_filters_to_query(rewritten_query, filters, bbox_filter); } return rewritten_query; From df63fbbd0488e24fcdb0a06167e433a8530f53fb Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Tue, 17 May 2016 13:55:00 +0200 Subject: [PATCH 12/38] Refactor filter application into own model This also avoids storing an object in the overviews query rewriter for the bbox filter (a plain data structure is used instead). --- lib/cartodb/backends/dataview.js | 12 +++++++++++- lib/cartodb/models/filter/camshaft.js | 13 +++++++++++++ lib/cartodb/utils/overviews_query_rewriter.js | 12 +++++++----- 3 files changed, 31 insertions(+), 6 deletions(-) create mode 100644 lib/cartodb/models/filter/camshaft.js diff --git a/lib/cartodb/backends/dataview.js b/lib/cartodb/backends/dataview.js index d18e1529..6861a02c 100644 --- a/lib/cartodb/backends/dataview.js +++ b/lib/cartodb/backends/dataview.js @@ -140,7 +140,17 @@ DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, param if (params.bbox) { var bboxFilter = new BBoxFilter({column: 'the_geom', srid: 4326}, {bbox: params.bbox}); query = bboxFilter.sql(query); - queryRewriteData = _.extend(queryRewriteData, { bbox_filter: bboxFilter }); + var bbox_filter_definition = { + type: 'bbox', + options: { + column: 'the_geom', + srid: 4326, + }, + params: { + bbox: params.bbox + } + }; + queryRewriteData = _.extend(queryRewriteData, { bbox_filter: bbox_filter_definition }); } var dataviewFactory = DataviewFactoryWithOverviews.getFactory( diff --git a/lib/cartodb/models/filter/camshaft.js b/lib/cartodb/models/filter/camshaft.js new file mode 100644 index 00000000..4fca3b8f --- /dev/null +++ b/lib/cartodb/models/filter/camshaft.js @@ -0,0 +1,13 @@ +// this is meant as a hack for development +// TODO: reproduce here the filter application of Camshaft +var queryBuilder = require('camshaft/lib/filter/query-builder'); + +function CamshaftFilters(filters) { + this.filters = filters; +} + +CamshaftFilters.prototype.sql = function(rawSql) { + return queryBuilder.getSQL(rawSql, filters); +}; + +module.exports = CamshaftFilters; diff --git a/lib/cartodb/utils/overviews_query_rewriter.js b/lib/cartodb/utils/overviews_query_rewriter.js index 7efff2ed..e38f53bd 100644 --- a/lib/cartodb/utils/overviews_query_rewriter.js +++ b/lib/cartodb/utils/overviews_query_rewriter.js @@ -1,15 +1,17 @@ var _ = require('underscore'); var TableNameParser = require('./table_name_parser'); -// this is meant as a hack for development -var queryBuilder = require('camshaft/lib/filter/query-builder'); +var BBoxFilter = require('../models/filter/bbox'); +var CamshaftFilter = require('../models/filter/camshaft'); + function apply_filters_to_query(query, filters, bbox_filter) { - // TODO: implement filter application here if ( filters && !_.isEmpty(filters)) { - query = queryBuilder.getSql(query, filters); + var camshaftFilter = new CamshaftFilter(filters) + query = camshaftFilter.sql(query); } if ( bbox_filter ) { - query = bbox_filter.sql(query); + var bboxFilter = new BBoxFilter(bbox_filter.options, bbox_filter.params); + query = bboxFilter.sql(query); } return query; } From 3c6d930434afcc9f29da843253d685b0baf42dc9 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Tue, 17 May 2016 15:39:32 +0200 Subject: [PATCH 13/38] Fix bug --- lib/cartodb/models/filter/camshaft.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/models/filter/camshaft.js b/lib/cartodb/models/filter/camshaft.js index 4fca3b8f..744b9fcd 100644 --- a/lib/cartodb/models/filter/camshaft.js +++ b/lib/cartodb/models/filter/camshaft.js @@ -7,7 +7,7 @@ function CamshaftFilters(filters) { } CamshaftFilters.prototype.sql = function(rawSql) { - return queryBuilder.getSQL(rawSql, filters); + return queryBuilder.getSql(rawSql, this.filters); }; module.exports = CamshaftFilters; From 7f7204df6cc993d7e84b08b38ecde6873f568547 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Tue, 17 May 2016 15:41:31 +0200 Subject: [PATCH 14/38] Add filter stats information to query rewriter data --- lib/cartodb/api/filter_stats_api.js | 63 +++++++++++++++++++ .../models/mapconfig_overviews_adapter.js | 63 +++++++++++++------ lib/cartodb/server.js | 4 +- lib/cartodb/utils/overviews_query_rewriter.js | 28 +++++++-- 4 files changed, 133 insertions(+), 25 deletions(-) create mode 100644 lib/cartodb/api/filter_stats_api.js diff --git a/lib/cartodb/api/filter_stats_api.js b/lib/cartodb/api/filter_stats_api.js new file mode 100644 index 00000000..5196d2fa --- /dev/null +++ b/lib/cartodb/api/filter_stats_api.js @@ -0,0 +1,63 @@ +var _ = require('underscore'); +var step = require('step'); +var CamshaftFilter = require('../models/filter/camshaft'); + +function FilterStatsApi(pgQueryRunner) { + this.pgQueryRunner = pgQueryRunner; +} + +module.exports = FilterStatsApi; + +function getEstimatedRows(pgQueryRunner, username, query, callback) { + pgQueryRunner.run(username, "EXPLAIN "+query, function(err, result_rows) { + if (err){ + callback(err); + return; + } + var rows; + var query_plan = result_rows[0]['QUERY PLAN']; + var match; + if ( query_plan ) { + match = query_plan.match(/rows=(\d+)/); + } + if ( match ) { + rows = +match[1]; + } + return callback(null, rows); + }); +} + +FilterStatsApi.prototype.getFilterStats = function (username, unfiltered_query, filters, callback) { + var stats = {}; + var self = this; + step( + function getUnfilteredRows() { + getEstimatedRows(self.pgQueryRunner, username, unfiltered_query, this); + }, + function receiveUnfilteredRows(err, rows) { + if (err){ + callback(err); + return; + } + stats.unfiltered_rows = rows; + this(null, rows); + }, + function getFilteredRows() { + if ( filters && !_.isEmpty(filters)) { + var camshaftFilter = new CamshaftFilter(filters); + var query = camshaftFilter.sql(unfiltered_query); + getEstimatedRows(self.pgQueryRunner, username, query, this); + } else { + this(null, null); + } + }, + function receiveFilteredRows(err, rows) { + if (err){ + callback(err); + return; + } + stats.filtered_rows = rows; + callback(null, stats); + } + ); +}; diff --git a/lib/cartodb/models/mapconfig_overviews_adapter.js b/lib/cartodb/models/mapconfig_overviews_adapter.js index 47185e00..d3fcedc0 100644 --- a/lib/cartodb/models/mapconfig_overviews_adapter.js +++ b/lib/cartodb/models/mapconfig_overviews_adapter.js @@ -1,8 +1,10 @@ +var step = require('step'); var queue = require('queue-async'); var _ = require('underscore'); -function MapConfigOverviewsAdapter(overviewsMetadataApi) { +function MapConfigOverviewsAdapter(overviewsMetadataApi, filterStatsApi) { this.overviewsMetadataApi = overviewsMetadataApi; + this.filterStatsApi = filterStatsApi; } module.exports = MapConfigOverviewsAdapter; @@ -25,26 +27,47 @@ MapConfigOverviewsAdapter.prototype.getLayers = function(username, layers, analy done(err, layer); } else { var query_rewrite_data = { overviews: metadata }; - if ( layer.options.source && analysesResults ) { - var sourceId = layer.options.source.id; - var node = _.find(analysesResults, function(a){ return a.rootNode.params.id === sourceId; }); - if ( node ) { - node = node.rootNode; - var filters = node.filters; // TODO: node.getFilters() when available in camshaft - var filters_disabler = _.keys(filters).reduce( - function(disabler, filter_id){ disabler[filter_id] = false; return disabler; }, - {} - ); - var unfiltered_query = node.getQuery(filters_disabler); - query_rewrite_data.filters = filters; - query_rewrite_data.unfiltered_query = unfiltered_query; + step( + function collectFiltersData() { + var filters, unfiltered_query; + if ( layer.options.source && analysesResults ) { + var sourceId = layer.options.source.id; + var node = _.find(analysesResults, function(a){ return a.rootNode.params.id === sourceId; }); + if ( node ) { + node = node.rootNode; + filters = node.filters; // TODO: node.getFilters() when available in camshaft + var filters_disabler = _.keys(filters).reduce( + function(disabler, filter_id){ disabler[filter_id] = false; return disabler; }, + {} + ); + unfiltered_query = node.getQuery(filters_disabler); + query_rewrite_data.filters = filters; + query_rewrite_data.unfiltered_query = unfiltered_query; + } + } + this(null, filters, unfiltered_query); + }, + function collectStatsData(err, filters, unfiltered_query) { + var next_step = this; + if ( filters ) { + self.filterStatsApi.getFilterStats(username, unfiltered_query, filters, function(err, stats) { + if ( !err ) { + query_rewrite_data.filter_stats = stats; + } + return next_step(err); + }); + } else { + return next_step(null); + } + }, + function addDataToLayer(err) { + if ( !err && !_.isEmpty(metadata) ) { + layer = _.extend({}, layer); + layer.options = _.extend({}, layer.options, { query_rewrite_data: query_rewrite_data }); + } + done(err, layer); } - } - if ( !_.isEmpty(metadata) ) { - layer = _.extend({}, layer); - layer.options = _.extend({}, layer.options, { query_rewrite_data: query_rewrite_data }); - } - done(null, layer); + ); } }); } diff --git a/lib/cartodb/server.js b/lib/cartodb/server.js index ed24f651..3feee602 100644 --- a/lib/cartodb/server.js +++ b/lib/cartodb/server.js @@ -21,6 +21,7 @@ var mapnik = windshaft.mapnik; var TemplateMaps = require('./backends/template_maps.js'); var OverviewsMetadataApi = require('./api/overviews_metadata_api'); +var FilterStatsApi = require('./api/filter_stats_api'); var UserLimitsApi = require('./api/user_limits_api'); var AuthApi = require('./api/auth_api'); var LayergroupAffectedTablesCache = require('./cache/layergroup_affected_tables'); @@ -59,6 +60,7 @@ module.exports = function(serverOptions) { var pgConnection = new PgConnection(metadataBackend); var pgQueryRunner = new PgQueryRunner(pgConnection); var overviewsMetadataApi = new OverviewsMetadataApi(pgQueryRunner); + var filterStatsApi = new FilterStatsApi(pgQueryRunner); var userLimitsApi = new UserLimitsApi(metadataBackend, { limits: { cacheOnTimeout: serverOptions.renderer.mapnik.limits.cacheOnTimeout || false, @@ -148,7 +150,7 @@ module.exports = function(serverOptions) { var layergroupAffectedTablesCache = new LayergroupAffectedTablesCache(); app.layergroupAffectedTablesCache = layergroupAffectedTablesCache; - var overviewsAdapter = new MapConfigOverviewsAdapter(overviewsMetadataApi); + var overviewsAdapter = new MapConfigOverviewsAdapter(overviewsMetadataApi, filterStatsApi); var turboCartoParser = new TurboCartoParser(pgQueryRunner); var turboCartoAdapter = new TurboCartoAdapter(turboCartoParser); diff --git a/lib/cartodb/utils/overviews_query_rewriter.js b/lib/cartodb/utils/overviews_query_rewriter.js index e38f53bd..4025e3f0 100644 --- a/lib/cartodb/utils/overviews_query_rewriter.js +++ b/lib/cartodb/utils/overviews_query_rewriter.js @@ -4,9 +4,14 @@ var TableNameParser = require('./table_name_parser'); var BBoxFilter = require('../models/filter/bbox'); var CamshaftFilter = require('../models/filter/camshaft'); +// Minimim number of filtered rows to use overviews +var FILTER_MIN_ROWS = 65536; +// Maximum filtered fraction to not apply overviews +var FILTER_MAX_FRACTION = 0.2; + function apply_filters_to_query(query, filters, bbox_filter) { if ( filters && !_.isEmpty(filters)) { - var camshaftFilter = new CamshaftFilter(filters) + var camshaftFilter = new CamshaftFilter(filters); query = camshaftFilter.sql(query); } if ( bbox_filter ) { @@ -238,7 +243,7 @@ OverviewsQueryRewriter.prototype.query = function(query, data, options) { unfiltered_query = query; } - if ( !overviews || !this.is_supported_query(unfiltered_query)) { + if ( !should_use_overviews(unfiltered_query, data) ) { return query; } @@ -283,7 +288,22 @@ function overviews_query(query, overviews, zoom_level, zoom_level_expression) { } } -OverviewsQueryRewriter.prototype.is_supported_query = function(sql) { +function should_use_overviews(query, data) { + data = data || {}; + var use_overviews = data.overviews && is_supported_query(query); + if ( use_overviews ) { + if ( data.filters && data.filter_stats ) { + var filtered_rows = data.filter_stats.filtered_rows; + var unfiltered_rows = data.filter_stats.unfiltered_rows; + if ( unfiltered_rows && (filtered_rows || filtered_rows === 0) ) { + use_overviews = filtered_rows >= FILTER_MIN_ROWS || (filtered_rows/unfiltered_rows) > FILTER_MAX_FRACTION; + } + } + } + return use_overviews; +} + +function is_supported_query(sql) { var basic_query = /\s*SELECT\s+[\*a-z0-9_,\s]+?\s+FROM\s+((\"[^"]+\"|[a-z0-9_]+)\.)?(\"[^"]+\"|[a-z0-9_]+)\s*;?\s*/i; var unwrapped_query = new RegExp("^"+basic_query.source+"$", 'i'); @@ -295,4 +315,4 @@ OverviewsQueryRewriter.prototype.is_supported_query = function(sql) { 'i' ); return !!(sql.match(unwrapped_query) || sql.match(wrapped_query)); -}; +} From 42ef40282bb065144c38b2bfc19bdf97416694e3 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Tue, 17 May 2016 15:46:13 +0200 Subject: [PATCH 15/38] :lipstick: shorten long lines --- lib/cartodb/models/mapconfig_overviews_adapter.js | 14 +++++++++----- lib/cartodb/utils/overviews_query_rewriter.js | 13 ++++++------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/lib/cartodb/models/mapconfig_overviews_adapter.js b/lib/cartodb/models/mapconfig_overviews_adapter.js index d3fcedc0..f9f580a8 100644 --- a/lib/cartodb/models/mapconfig_overviews_adapter.js +++ b/lib/cartodb/models/mapconfig_overviews_adapter.js @@ -50,12 +50,16 @@ MapConfigOverviewsAdapter.prototype.getLayers = function(username, layers, analy function collectStatsData(err, filters, unfiltered_query) { var next_step = this; if ( filters ) { - self.filterStatsApi.getFilterStats(username, unfiltered_query, filters, function(err, stats) { - if ( !err ) { - query_rewrite_data.filter_stats = stats; + self.filterStatsApi.getFilterStats( + username, + unfiltered_query, filters, + function(err, stats) { + if ( !err ) { + query_rewrite_data.filter_stats = stats; + } + return next_step(err); } - return next_step(err); - }); + ); } else { return next_step(null); } diff --git a/lib/cartodb/utils/overviews_query_rewriter.js b/lib/cartodb/utils/overviews_query_rewriter.js index 4025e3f0..faad459d 100644 --- a/lib/cartodb/utils/overviews_query_rewriter.js +++ b/lib/cartodb/utils/overviews_query_rewriter.js @@ -291,13 +291,12 @@ function overviews_query(query, overviews, zoom_level, zoom_level_expression) { function should_use_overviews(query, data) { data = data || {}; var use_overviews = data.overviews && is_supported_query(query); - if ( use_overviews ) { - if ( data.filters && data.filter_stats ) { - var filtered_rows = data.filter_stats.filtered_rows; - var unfiltered_rows = data.filter_stats.unfiltered_rows; - if ( unfiltered_rows && (filtered_rows || filtered_rows === 0) ) { - use_overviews = filtered_rows >= FILTER_MIN_ROWS || (filtered_rows/unfiltered_rows) > FILTER_MAX_FRACTION; - } + if ( use_overviews && data.filters && data.filter_stats ) { + var filtered_rows = data.filter_stats.filtered_rows; + var unfiltered_rows = data.filter_stats.unfiltered_rows; + if ( unfiltered_rows && (filtered_rows || filtered_rows === 0) ) { + use_overviews = filtered_rows >= FILTER_MIN_ROWS || + (filtered_rows/unfiltered_rows) > FILTER_MAX_FRACTION; } } return use_overviews; From 7a6b1ec8719230e2213b5a5003a1eff96f711f26 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Tue, 17 May 2016 16:01:10 +0200 Subject: [PATCH 16/38] Fix tests for MapConfigOverviewsAdaptar changes --- test/integration/mapconfig_overviews_adapter.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/integration/mapconfig_overviews_adapter.js b/test/integration/mapconfig_overviews_adapter.js index 5c425ec1..d168c02f 100644 --- a/test/integration/mapconfig_overviews_adapter.js +++ b/test/integration/mapconfig_overviews_adapter.js @@ -6,6 +6,7 @@ var cartodbRedis = require('cartodb-redis'); var PgConnection = require(__dirname + '/../../lib/cartodb/backends/pg_connection'); var PgQueryRunner = require('../../lib/cartodb/backends/pg_query_runner'); var OverviewsMetadataApi = require('../../lib/cartodb/api/overviews_metadata_api'); +var FilterStatsApi = require('../../lib/cartodb/api/filter_stats_api'); var MapConfigOverviewsAdapter = require('../../lib/cartodb/models/mapconfig_overviews_adapter'); // configure redis pool instance to use in tests @@ -17,9 +18,9 @@ var metadataBackend = cartodbRedis({pool: redisPool}); var pgConnection = new PgConnection(metadataBackend); var pgQueryRunner = new PgQueryRunner(pgConnection); var overviewsMetadataApi = new OverviewsMetadataApi(pgQueryRunner); +var filterStatsApi = new FilterStatsApi(pgQueryRunner); - -var mapConfigOverviewsAdapter = new MapConfigOverviewsAdapter(overviewsMetadataApi); +var mapConfigOverviewsAdapter = new MapConfigOverviewsAdapter(overviewsMetadataApi, filterStatsApi); describe('MapConfigOverviewsAdapter', function() { From 24f7bc65966a0441d6a539caf1e5c4b158395444 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 07:47:30 +0200 Subject: [PATCH 17/38] Add tests for dataviews with overviews --- test/support/sql/windshaft.test.sql | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/test/support/sql/windshaft.test.sql b/test/support/sql/windshaft.test.sql index 4690f081..7920a88d 100644 --- a/test/support/sql/windshaft.test.sql +++ b/test/support/sql/windshaft.test.sql @@ -247,8 +247,10 @@ CREATE TABLE test_table_overviews ( cartodb_id integer NOT NULL, name character varying, address character varying, + value float8, the_geom geometry, the_geom_webmercator geometry, + _feature_count integer, CONSTRAINT enforce_dims_the_geom CHECK ((st_ndims(the_geom) = 2)), CONSTRAINT enforce_dims_the_geom_webmercator CHECK ((st_ndims(the_geom_webmercator) = 2)), CONSTRAINT enforce_geotype_the_geom CHECK (((geometrytype(the_geom) = 'POINT'::text) OR (the_geom IS NULL))), @@ -274,11 +276,11 @@ SELECT pg_catalog.setval('test_table_overviews_cartodb_id_seq', 60, true); ALTER TABLE test_table_overviews ALTER COLUMN cartodb_id SET DEFAULT nextval('test_table_overviews_cartodb_id_seq'::regclass); INSERT INTO test_table_overviews VALUES -('2011-09-21 14:02:21.358706', '2011-09-21 14:02:21.314252', 1, 'Hawai', 'Calle de Pérez Galdós 9, Madrid, Spain', '0101000020E6100000A6B73F170D990DC064E8D84125364440', '0101000020110F000076491621312319C122D4663F1DCC5241'), -('2011-09-21 14:02:21.358706', '2011-09-21 14:02:21.319101', 2, 'El Estocolmo', 'Calle de la Palma 72, Madrid, Spain', '0101000020E6100000C90567F0F7AB0DC0AB07CC43A6364440', '0101000020110F0000C4356B29423319C15DD1092DADCC5241'), -('2011-09-21 14:02:21.358706', '2011-09-21 14:02:21.324', 3, 'El Rey del Tallarín', 'Plaza Conde de Toreno 2, Madrid, Spain', '0101000020E610000021C8410933AD0DC0CB0EF10F5B364440', '0101000020110F000053E71AC64D3419C10F664E4659CC5241'), -('2011-09-21 14:02:21.358706', '2011-09-21 14:02:21.329509', 4, 'El Lacón', 'Manuel Fernández y González 8, Madrid, Spain', '0101000020E6100000BC5983F755990DC07D923B6C22354440', '0101000020110F00005DACDB056F2319C1EC41A980FCCA5241'), -('2011-09-21 14:02:21.358706', '2011-09-21 14:02:21.334931', 5, 'El Pico', 'Calle Divino Pastor 12, Madrid, Spain', '0101000020E61000003B6D8D08C6A10DC0371B2B31CF364440', '0101000020110F00005F716E91992A19C17DAAA4D6DACC5241'); +('2011-09-21 14:02:21.358706', '2011-09-21 14:02:21.314252', 1, 'Hawai', 'Calle de Pérez Galdós 9, Madrid, Spain', 1.0, '0101000020E6100000A6B73F170D990DC064E8D84125364440', '0101000020110F000076491621312319C122D4663F1DCC5241', 1), +('2011-09-21 14:02:21.358706', '2011-09-21 14:02:21.319101', 2, 'El Estocolmo', 'Calle de la Palma 72, Madrid, Spain', 2.0, '0101000020E6100000C90567F0F7AB0DC0AB07CC43A6364440', '0101000020110F0000C4356B29423319C15DD1092DADCC5241', 1), +('2011-09-21 14:02:21.358706', '2011-09-21 14:02:21.324', 3, 'El Rey del Tallarín', 'Plaza Conde de Toreno 2, Madrid, Spain', 3.0, '0101000020E610000021C8410933AD0DC0CB0EF10F5B364440', '0101000020110F000053E71AC64D3419C10F664E4659CC5241', 1), +('2011-09-21 14:02:21.358706', '2011-09-21 14:02:21.329509', 4, 'El Lacón', 'Manuel Fernández y González 8, Madrid, Spain', 4.0, '0101000020E6100000BC5983F755990DC07D923B6C22354440', '0101000020110F00005DACDB056F2319C1EC41A980FCCA5241', 1), +('2011-09-21 14:02:21.358706', '2011-09-21 14:02:21.334931', 5, 'El Pico', 'Calle Divino Pastor 12, Madrid, Spain', 5.0, '0101000020E61000003B6D8D08C6A10DC0371B2B31CF364440', '0101000020110F00005F716E91992A19C17DAAA4D6DACC5241', 1); ALTER TABLE ONLY test_table_overviews ADD CONSTRAINT test_table_overviews_pkey PRIMARY KEY (cartodb_id); @@ -294,8 +296,10 @@ CREATE TABLE _vovw_1_test_table_overviews ( cartodb_id integer NOT NULL, name character varying, address character varying, + value float8, the_geom geometry, the_geom_webmercator geometry, + _feature_count integer, CONSTRAINT enforce_dims_the_geom CHECK ((st_ndims(the_geom) = 2)), CONSTRAINT enforce_dims_the_geom_webmercator CHECK ((st_ndims(the_geom_webmercator) = 2)), CONSTRAINT enforce_geotype_the_geom CHECK (((geometrytype(the_geom) = 'POINT'::text) OR (the_geom IS NULL))), @@ -313,8 +317,10 @@ CREATE TABLE _vovw_2_test_table_overviews ( cartodb_id integer NOT NULL, name character varying, address character varying, + value float8, the_geom geometry, the_geom_webmercator geometry, + _feature_count integer, CONSTRAINT enforce_dims_the_geom CHECK ((st_ndims(the_geom) = 2)), CONSTRAINT enforce_dims_the_geom_webmercator CHECK ((st_ndims(the_geom_webmercator) = 2)), CONSTRAINT enforce_geotype_the_geom CHECK (((geometrytype(the_geom) = 'POINT'::text) OR (the_geom IS NULL))), @@ -327,11 +333,11 @@ GRANT ALL ON TABLE _vovw_2_test_table_overviews TO :TESTUSER; GRANT SELECT ON TABLE _vovw_2_test_table_overviews TO :PUBLICUSER; INSERT INTO _vovw_2_test_table_overviews VALUES -('2011-09-21 14:02:21.358706', '2011-09-21 14:02:21.314252', 1, 'Hawai', 'Calle de Pérez Galdós 9, Madrid, Spain', '0101000020E610000000000000000020C00000000000004440', '0101000020110F000076491621312319C122D4663F1DCC5241'), -('2011-09-21 14:02:21.358706', '2011-09-21 14:02:21.319101', 2, 'El Estocolmo', 'Calle de la Palma 72, Madrid, Spain', '0101000020E610000000000000009431C026043C75E7224340', '0101000020110F0000C4356B29423319C15DD1092DADCC5241'); +('2011-09-21 14:02:21.358706', '2011-09-21 14:02:21.314252', 1, 'Hawai', 'Calle de Pérez Galdós 9, Madrid, Spain', 8.0/3.0, '0101000020E610000000000000000020C00000000000004440', '0101000020110F000076491621312319C122D4663F1DCC5241', 3), +('2011-09-21 14:02:21.358706', '2011-09-21 14:02:21.319101', 2, 'El Estocolmo', 'Calle de la Palma 72, Madrid, Spain', 7.0/2.0, '0101000020E610000000000000009431C026043C75E7224340', '0101000020110F0000C4356B29423319C15DD1092DADCC5241', 2); INSERT INTO _vovw_1_test_table_overviews VALUES -('2011-09-21 14:02:21.358706', '2011-09-21 14:02:21.314252', 1, 'Hawai', 'Calle de Pérez Galdós 9, Madrid, Spain', '0101000020E610000000000000000020C00000000000004440', '0101000020110F000076491621312319C122D4663F1DCC5241'); +('2011-09-21 14:02:21.358706', '2011-09-21 14:02:21.314252', 1, 'Hawai', 'Calle de Pérez Galdós 9, Madrid, Spain', 3.0, '0101000020E610000000000000000020C00000000000004440', '0101000020110F000076491621312319C122D4663F1DCC5241', 5); -- analysis tables ----------------------------------------------- From 3d8f6576aa3a98ff0ce1f67382304b720d7826e5 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 07:48:11 +0200 Subject: [PATCH 18/38] Implement category and range filters --- lib/cartodb/models/filter/camshaft.js | 29 ++++++- .../models/filter/camshaft/category.js | 79 +++++++++++++++++++ lib/cartodb/models/filter/camshaft/range.js | 43 ++++++++++ 3 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 lib/cartodb/models/filter/camshaft/category.js create mode 100644 lib/cartodb/models/filter/camshaft/range.js diff --git a/lib/cartodb/models/filter/camshaft.js b/lib/cartodb/models/filter/camshaft.js index 744b9fcd..0eae2681 100644 --- a/lib/cartodb/models/filter/camshaft.js +++ b/lib/cartodb/models/filter/camshaft.js @@ -1,13 +1,38 @@ // this is meant as a hack for development // TODO: reproduce here the filter application of Camshaft -var queryBuilder = require('camshaft/lib/filter/query-builder'); + +var filters = { + category: require('./camshaft/category'), + range: require('./camshaft/range') +}; + +function createFilter(filterDefinition) { + var filterType = filterDefinition.type.toLowerCase(); + if (!filters.hasOwnProperty(filterType)) { + throw new Error('Unknown filter type: ' + filterType); + } + return new filters[filterType](filterDefinition.column, filterDefinition.params); +} function CamshaftFilters(filters) { this.filters = filters; } CamshaftFilters.prototype.sql = function(rawSql) { - return queryBuilder.getSql(rawSql, this.filters); + var filters = this.filters || {}; + var applyFilters = {}; + + return Object.keys(filters) + .filter(function(filterName) { + return applyFilters.hasOwnProperty(filterName) ? applyFilters[filterName] : true; + }) + .map(function(filterName) { + var filterDefinition = filters[filterName]; + return createFilter(filterDefinition); + }) + .reduce(function(sql, filter) { + return filter.sql(sql); + }, rawSql); }; module.exports = CamshaftFilters; diff --git a/lib/cartodb/models/filter/camshaft/category.js b/lib/cartodb/models/filter/camshaft/category.js new file mode 100644 index 00000000..6181de7f --- /dev/null +++ b/lib/cartodb/models/filter/camshaft/category.js @@ -0,0 +1,79 @@ +'use strict'; + +var debug = require('debug')('windshaft:filter:category'); +var dot = require('dot'); +dot.templateSettings.strip = false; + +var filterQueryTpl = dot.template([ + 'SELECT *', + 'FROM ({{=it._sql}}) _camshaft_category_filter', + 'WHERE {{=it._filters}}' +].join('\n')); +var escapeStringTpl = dot.template('$escape_{{=it._i}}${{=it._value}}$escape_{{=it._i}}$'); +var inConditionTpl = dot.template('{{=it._column}} IN ({{=it._values}})'); +var notInConditionTpl = dot.template('{{=it._column}} NOT IN ({{=it._values}})'); + +function Category(column, filterParams) { + this.column = column; + + if (!Array.isArray(filterParams.accept) && !Array.isArray(filterParams.reject)) { + throw new Error('Category filter expects at least one array in accept or reject params'); + } + + if (Array.isArray(filterParams.accept) && Array.isArray(filterParams.reject)) { + if (filterParams.accept.length === 0 && filterParams.reject.length === 0) { + throw new Error( + 'Category filter expects one value either in accept or reject params when both are provided' + ); + } + } + + this.accept = filterParams.accept; + this.reject = filterParams.reject; +} + +module.exports = Category; + +/* + - accept: [] => reject all + - reject: [] => accept all + */ +Category.prototype.sql = function(rawSql) { + var valueFilters = []; + + if (Array.isArray(this.accept)) { + if (this.accept.length > 0) { + valueFilters.push(inConditionTpl({ + _column: this.column, + _values: this.accept.map(function(value, i) { + return Number.isFinite(value) ? value : escapeStringTpl({_i: i, _value: value}); + }).join(',') + })); + } else { + valueFilters.push('0 = 1'); + } + } + + if (Array.isArray(this.reject)) { + if (this.reject.length > 0) { + valueFilters.push(notInConditionTpl({ + _column: this.column, + _values: this.reject.map(function (value, i) { + return Number.isFinite(value) ? value : escapeStringTpl({_i: i, _value: value}); + }).join(',') + })); + } else { + valueFilters.push('1 = 1'); + } + } + + debug(filterQueryTpl({ + _sql: rawSql, + _filters: valueFilters.join(' AND ') + })); + + return filterQueryTpl({ + _sql: rawSql, + _filters: valueFilters.join(' AND ') + }); +}; diff --git a/lib/cartodb/models/filter/camshaft/range.js b/lib/cartodb/models/filter/camshaft/range.js new file mode 100644 index 00000000..f894ca32 --- /dev/null +++ b/lib/cartodb/models/filter/camshaft/range.js @@ -0,0 +1,43 @@ +'use strict'; + +var dot = require('dot'); +dot.templateSettings.strip = false; + +var betweenFilterTpl = dot.template('{{=it._column}} BETWEEN {{=it._min}} AND {{=it._max}}'); +var minFilterTpl = dot.template('{{=it._column}} >= {{=it._min}}'); +var maxFilterTpl = dot.template('{{=it._column}} <= {{=it._max}}'); +var filterQueryTpl = dot.template('SELECT * FROM ({{=it._sql}}) _camshaft_range_filter WHERE {{=it._filter}}'); + +function Range(column, filterParams) { + this.column = column; + + if (!Number.isFinite(filterParams.min) && !Number.isFinite(filterParams.max)) { + throw new Error('Range filter expect to have at least one value in min or max numeric params'); + } + + this.min = filterParams.min; + this.max = filterParams.max; + this.columnType = filterParams.columnType; +} + +module.exports = Range; + +Range.prototype.sql = function(rawSql) { + var minMaxFilter; + if (Number.isFinite(this.min) && Number.isFinite(this.max)) { + minMaxFilter = betweenFilterTpl({ + _column: this.column, + _min: this.min, + _max: this.max + }); + } else if (Number.isFinite(this.min)) { + minMaxFilter = minFilterTpl({ _column: this.column, _min: this.min }); + } else { + minMaxFilter = maxFilterTpl({ _column: this.column, _max: this.max }); + } + + return filterQueryTpl({ + _sql: rawSql, + _filter: minMaxFilter + }); +}; From cb3706e5cfd15ca3574bb0fb4679112036cce87a Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 08:04:11 +0200 Subject: [PATCH 19/38] Update Query Rewriter comments --- lib/cartodb/utils/overviews_query_rewriter.js | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/lib/cartodb/utils/overviews_query_rewriter.js b/lib/cartodb/utils/overviews_query_rewriter.js index faad459d..e2c7e29d 100644 --- a/lib/cartodb/utils/overviews_query_rewriter.js +++ b/lib/cartodb/utils/overviews_query_rewriter.js @@ -213,24 +213,24 @@ function overview_table_for_zoom_level(table_overviews, zoom_level) { } // Transform an SQL query so that it uses overviews. -// overviews contains metadata about the overviews to be used: -// { 'table-name': {1: { table: 'overview-table-1' }, ... }, ... } // // For a given query `SELECT * FROM table`, if any of tables in it // has overviews as defined by the provided metadat, the query will // be transform into something similar to this: // // WITH _vovw_scale AS ( ... ), -- define scale level -// WITH _vovw_table AS ( ... ), -- define union of overviews and base table -// SELECT * FROM _vovw_table -- query with table replaced by _vovw_table +// SELECT * FROM -- in the query the table is replaced by: +// ( ... ) AS _vovw_table -- a union of overviews and base table // -// This transformation can in principle be applied to arbitrary queries -// (except for the case of queries that include the name of tables with -// overviews inside text literals: at the current table name substitution -// doesnn't prevent substitution inside literals). -// But the transformation will currently only be applied to simple queries -// of the form detected by the overviews_supported_query function. -// TODO: document recent changes (zoom_level option, filters...) +// The data argument has the form: +// { +// overviews: // overview tables metadata +// { 'table-name': {1: { table: 'overview-table-1' }, ... }, ... }, +// zoom_level: ..., // optional zoom level +// filters: ..., // filters definition +// unfiltered_query: ..., // query without the filters +// bbox_filter: ... // bounding-box filter +// } OverviewsQueryRewriter.prototype.query = function(query, data, options) { options = options || {}; @@ -258,11 +258,6 @@ OverviewsQueryRewriter.prototype.query = function(query, data, options) { // could not or didn't need to alter the query rewritten_query = query; } else { - // TODO: determine if overviews can be used for the filters. - // Otherwise use the base table. - // Applicability of the filters should depend both on the expected - // row count of the result and on the columns to be filter and the - // way they're agregated in overviews. rewritten_query = apply_filters_to_query(rewritten_query, filters, bbox_filter); } From aa0ddaae95447292d6f0f45b0cdbf904bb30bd7c Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 08:07:48 +0200 Subject: [PATCH 20/38] Remove comment --- lib/cartodb/models/filter/camshaft.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/cartodb/models/filter/camshaft.js b/lib/cartodb/models/filter/camshaft.js index 0eae2681..97cb25b6 100644 --- a/lib/cartodb/models/filter/camshaft.js +++ b/lib/cartodb/models/filter/camshaft.js @@ -1,6 +1,3 @@ -// this is meant as a hack for development -// TODO: reproduce here the filter application of Camshaft - var filters = { category: require('./camshaft/category'), range: require('./camshaft/range') From bbb1b4a7b947801709533a8d5871dd9d5db3ffa3 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 08:11:52 +0200 Subject: [PATCH 21/38] Add tests missing file --- test/acceptance/dataviews/overviews.js | 163 +++++++++++++++++++++++++ 1 file changed, 163 insertions(+) create mode 100644 test/acceptance/dataviews/overviews.js diff --git a/test/acceptance/dataviews/overviews.js b/test/acceptance/dataviews/overviews.js new file mode 100644 index 00000000..14e14362 --- /dev/null +++ b/test/acceptance/dataviews/overviews.js @@ -0,0 +1,163 @@ +require('../../support/test_helper'); + +var assert = require('../../support/assert'); +var TestClient = require('../../support/test-client'); + +describe('dataviews using tables without overviews', function() { + + var countMapConfig = { + version: '1.5.0', + analyses: [ + { id: 'data-source', + type: 'source', + params: { + query: 'select * from populated_places_simple_reduced' + } + } + ], + dataviews: { + country_places_count: { + type: 'formula', + source: {id: 'data-source'}, + options: { + column: 'adm0_a3', + operation: 'count' + } + }, + country_categories: { + type: 'aggregation', + source: {id: 'data-source'}, + options: { + column: 'adm0_a3', + aggregation: 'count' + } + } + }, + layers: [ + { + type: 'mapnik', + options: { + sql: 'select * from populated_places_simple_reduced', + cartocss: '#layer { marker-fill: red; marker-width: 32; marker-allow-overlap: true; }', + cartocss_version: '2.3.0', + source: { id: 'data-source' } + } + } + ] + }; + + it("should expose a formula", function(done) { + var testClient = new TestClient(countMapConfig); + testClient.getDataview('country_places_count', { own_filter: 0 }, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, { operation: 'count', result: 7313, nulls: 0, type: 'formula' }); + + testClient.drain(done); + }); + }); + + describe('filters', function() { + + describe('category', function () { + + it("should expose a filtered formula", function (done) { + var params = { + filters: { + dataviews: {country_categories: {accept: ['CAN']}} + } + }; + var testClient = new TestClient(countMapConfig); + testClient.getDataview('country_places_count', params, function (err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, { operation: 'count', result: 256, nulls: 0, type: 'formula' }); + testClient.drain(done); + }); + }); + }); + + }); +}); + +describe('dataviews using tables with overviews', function() { + + var sumMapConfig = { + version: '1.5.0', + analyses: [ + { id: 'data-source', + type: 'source', + params: { + query: 'select * from test_table_overviews' + } + } + ], + dataviews: { + test_sum: { + type: 'formula', + source: {id: 'data-source'}, + options: { + column: 'value', + operation: 'sum' + } + }, + test_categories: { + type: 'aggregation', + source: {id: 'data-source'}, + options: { + column: 'name', + aggregation: 'count', + aggregationColumn: 'name', + } + } + }, + layers: [ + { + type: 'mapnik', + options: { + sql: 'select * from test_table_overviews', + cartocss: '#layer { marker-fill: red; marker-width: 32; marker-allow-overlap: true; }', + cartocss_version: '2.3.0', + source: { id: 'data-source' } + } + } + ] + }; + + it("should expose a formula", function(done) { + var testClient = new TestClient(sumMapConfig); + testClient.getDataview('test_sum', { own_filter: 0 }, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"sum","result":15,"nulls":0,"type":"formula"}); + + testClient.drain(done); + }); + }); + + describe('filters', function() { + + describe('category', function () { + + it("should expose a filtered formula", function (done) { + var params = { + filters: { + dataviews: {test_categories: {accept: ['Hawai']}} + } + }; + var testClient = new TestClient(sumMapConfig); + testClient.getDataview('test_sum', params, function (err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"sum","result":1,"nulls":0,"type":"formula"}); + testClient.drain(done); + }); + }); + }); + + }); +}); From 1872fbd021cce410261ac48325f40eaa8a21b894 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 10:48:13 +0200 Subject: [PATCH 22/38] Add test cases for dataview formulae Check the overriden (sum,avg,count) and non-overriden (min, max) cases. --- test/acceptance/dataviews/overviews.js | 143 +++++++++++++++++++++++-- 1 file changed, 136 insertions(+), 7 deletions(-) diff --git a/test/acceptance/dataviews/overviews.js b/test/acceptance/dataviews/overviews.js index 14e14362..9acda126 100644 --- a/test/acceptance/dataviews/overviews.js +++ b/test/acceptance/dataviews/overviews.js @@ -5,7 +5,7 @@ var TestClient = require('../../support/test-client'); describe('dataviews using tables without overviews', function() { - var countMapConfig = { + var nonOverviewsMapConfig = { version: '1.5.0', analyses: [ { id: 'data-source', @@ -47,7 +47,7 @@ describe('dataviews using tables without overviews', function() { }; it("should expose a formula", function(done) { - var testClient = new TestClient(countMapConfig); + var testClient = new TestClient(nonOverviewsMapConfig); testClient.getDataview('country_places_count', { own_filter: 0 }, function(err, formula_result) { if (err) { return done(err); @@ -68,7 +68,7 @@ describe('dataviews using tables without overviews', function() { dataviews: {country_categories: {accept: ['CAN']}} } }; - var testClient = new TestClient(countMapConfig); + var testClient = new TestClient(nonOverviewsMapConfig); testClient.getDataview('country_places_count', params, function (err, formula_result) { if (err) { return done(err); @@ -84,7 +84,7 @@ describe('dataviews using tables without overviews', function() { describe('dataviews using tables with overviews', function() { - var sumMapConfig = { + var overviewsMapConfig = { version: '1.5.0', analyses: [ { id: 'data-source', @@ -111,6 +111,38 @@ describe('dataviews using tables with overviews', function() { aggregation: 'count', aggregationColumn: 'name', } + }, + test_avg: { + type: 'formula', + source: {id: 'data-source'}, + options: { + column: 'value', + operation: 'avg' + } + }, + test_count: { + type: 'formula', + source: {id: 'data-source'}, + options: { + column: 'value', + operation: 'count' + } + }, + test_min: { + type: 'formula', + source: {id: 'data-source'}, + options: { + column: 'value', + operation: 'min' + } + }, + test_max: { + type: 'formula', + source: {id: 'data-source'}, + options: { + column: 'value', + operation: 'max' + } } }, layers: [ @@ -126,8 +158,8 @@ describe('dataviews using tables with overviews', function() { ] }; - it("should expose a formula", function(done) { - var testClient = new TestClient(sumMapConfig); + it("should expose a sum formula", function(done) { + var testClient = new TestClient(overviewsMapConfig); testClient.getDataview('test_sum', { own_filter: 0 }, function(err, formula_result) { if (err) { return done(err); @@ -138,6 +170,54 @@ describe('dataviews using tables with overviews', function() { }); }); + it("should expose an avg formula", function(done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_avg', { own_filter: 0 }, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"avg","result":3,"nulls":0,"type":"formula"}); + + testClient.drain(done); + }); + }); + + it("should expose a count formula", function(done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_count', { own_filter: 0 }, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"count","result":5,"nulls":0,"type":"formula"}); + + testClient.drain(done); + }); + }); + + it("should expose a max formula", function(done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_max', { own_filter: 0 }, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"max","result":5,"nulls":0,"type":"formula"}); + + testClient.drain(done); + }); + }); + + it("should expose a min formula", function(done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_min', { own_filter: 0 }, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"min","result":1,"nulls":0,"type":"formula"}); + + testClient.drain(done); + }); + }); + describe('filters', function() { describe('category', function () { @@ -148,7 +228,7 @@ describe('dataviews using tables with overviews', function() { dataviews: {test_categories: {accept: ['Hawai']}} } }; - var testClient = new TestClient(sumMapConfig); + var testClient = new TestClient(overviewsMapConfig); testClient.getDataview('test_sum', params, function (err, formula_result) { if (err) { return done(err); @@ -156,6 +236,55 @@ describe('dataviews using tables with overviews', function() { assert.deepEqual(formula_result, {"operation":"sum","result":1,"nulls":0,"type":"formula"}); testClient.drain(done); }); + + it("should expose an avg formula", function(done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_avg', { own_filter: 0 }, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"avg","result":1,"nulls":0,"type":"formula"}); + + testClient.drain(done); + }); + }); + + it("should expose a count formula", function(done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_count', { own_filter: 0 }, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"count","result":1,"nulls":0,"type":"formula"}); + + testClient.drain(done); + }); + }); + + it("should expose a max formula", function(done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_max', { own_filter: 0 }, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"max","result":1,"nulls":0,"type":"formula"}); + + testClient.drain(done); + }); + }); + + it("should expose a min formula", function(done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_min', { own_filter: 0 }, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"min","result":1,"nulls":0,"type":"formula"}); + + testClient.drain(done); + }); + }); + }); }); From 48d2978997882f08b0e56ba47aca4ce387fb5a9f Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 11:45:14 +0200 Subject: [PATCH 23/38] Test filters query rewrite data --- test/acceptance/overviews_metadata.js | 115 ++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) diff --git a/test/acceptance/overviews_metadata.js b/test/acceptance/overviews_metadata.js index 08b9cb59..0822cfe8 100644 --- a/test/acceptance/overviews_metadata.js +++ b/test/acceptance/overviews_metadata.js @@ -109,3 +109,118 @@ describe('overviews metadata', function() { ); }); }); + +describe('overviews metadata with filters', function() { + // configure redis pool instance to use in tests + var redisPool = new RedisPool(global.environment.redis); + + var keysToDelete; + + beforeEach(function() { + keysToDelete = {}; + }); + + afterEach(function(done) { + test_helper.deleteRedisKeys(keysToDelete, done); + }); + + it("layers with overviews", function(done) { + + var layergroup = { + version: '1.5.0', + layers: [ + { + type: 'cartodb', + options: { + sql: 'SELECT * FROM test_table_overviews', + source: { id: 'with_overviews' }, + cartocss: '#layer { marker-fill: black; }', + cartocss_version: '2.3.0' + } + } + ], + dataviews: { + test_names: { + type: 'aggregation', + source: {id: 'with_overviews'}, + options: { + column: 'name', + aggregation: 'count' + } + } + }, + analyses: [ + { id: 'with_overviews', + type: 'source', + params: { + query: 'select * from test_table_overviews' + } + } + ] + }; + + var filters = { + dataviews: { + test_names: { accept: ['Hawai'] } + } + }; + + var layergroup_url = '/api/v1/map'; + + var expected_token; + step( + function do_post() + { + var next = this; + assert.response(server, { + url: layergroup_url + '?filters=' + JSON.stringify(filters), + method: 'POST', + headers: {host: 'localhost', 'Content-Type': 'application/json' }, + data: JSON.stringify(layergroup) + }, {}, function(res) { + assert.equal(res.statusCode, 200, res.body); + var parsedBody = JSON.parse(res.body); + assert.equal(res.headers['x-layergroup-id'], parsedBody.layergroupid); + expected_token = parsedBody.layergroupid; + next(null, res); + }); + }, + function do_get_mapconfig(err) + { + assert.ifError(err); + var next = this; + + var mapStore = new windshaft.storage.MapStore({ + pool: redisPool, + expire_time: 500000 + }); + mapStore.load(LayergroupToken.parse(expected_token).token, function(err, mapConfig) { + assert.ifError(err); + assert.equal(mapConfig._cfg.layers[0].type, 'cartodb'); + assert.ok(mapConfig._cfg.layers[0].options.query_rewrite_data); + var expected_data = { + overviews: { + test_table_overviews: { + schema: 'public', + 1: { table: '_vovw_1_test_table_overviews' }, + 2: { table: '_vovw_2_test_table_overviews' } + } + }, + filters: { test_names: { type: 'category', column: 'name', params: { accept: [ 'Hawai' ] } } }, + unfiltered_query: 'select * from test_table_overviews', + filter_stats: { unfiltered_rows: 5, filtered_rows: 1 } + }; + assert.deepEqual(mapConfig._cfg.layers[0].options.query_rewrite_data, expected_data); + + }); + + next(err); + }, + function finish(err) { + keysToDelete['map_cfg|' + LayergroupToken.parse(expected_token).token] = 0; + keysToDelete['user:localhost:mapviews:global'] = 5; + done(err); + } + ); + }); +}); From 858d976637b9863357175954eb3e3cbb4244309e Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 11:53:30 +0200 Subject: [PATCH 24/38] Add tests for query rewriter using specific zoom level --- test/unit/cartodb/overviews_query_rewriter.js | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/test/unit/cartodb/overviews_query_rewriter.js b/test/unit/cartodb/overviews_query_rewriter.js index 0e40ef92..ac3039c6 100644 --- a/test/unit/cartodb/overviews_query_rewriter.js +++ b/test/unit/cartodb/overviews_query_rewriter.js @@ -475,4 +475,52 @@ describe('Overviews query rewriter', function() { "; assertSameSql(overviews_sql, expected_sql); }); + + it('generates query for specific Z level', function(){ + var sql = "SELECT * FROM table1"; + var data = { + overviews: { + table1: { + 0: { table: 'table1_ov0' }, + 2: { table: 'table1_ov2' }, + 3: { table: 'table1_ov3' } + } + } + }; + var overviews_sql = overviewsQueryRewriter.query(sql, data, { zoom_level: 3 }); + var expected_sql = "SELECT * FROM table1_ov3"; + assertSameSql(overviews_sql, expected_sql); + }); + + it('generates query for specific nonpresent Z level', function(){ + var sql = "SELECT * FROM table1"; + var data = { + overviews: { + table1: { + 0: { table: 'table1_ov0' }, + 2: { table: 'table1_ov2' }, + 3: { table: 'table1_ov3' } + } + } + }; + var overviews_sql = overviewsQueryRewriter.query(sql, data, { zoom_level: 1 }); + var expected_sql = "SELECT * FROM table1_ov2"; + assertSameSql(overviews_sql, expected_sql); + }); + + it('does not use overviews for specific out-of-range Z level', function(){ + var sql = "SELECT * FROM table1"; + var data = { + overviews: { + table1: { + 0: { table: 'table1_ov0' }, + 2: { table: 'table1_ov2' }, + 3: { table: 'table1_ov3' } + } + } + }; + var overviews_sql = overviewsQueryRewriter.query(sql, data, { zoom_level: 4 }); + var expected_sql = "SELECT * FROM table1"; + assertSameSql(overviews_sql, expected_sql); + }); }); From 3987e83b7a967d9a3df28641c8fd9c2c3aed462d Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 12:34:51 +0200 Subject: [PATCH 25/38] Add tests for query rewriter with filters --- test/unit/cartodb/overviews_query_rewriter.js | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/test/unit/cartodb/overviews_query_rewriter.js b/test/unit/cartodb/overviews_query_rewriter.js index ac3039c6..dd6b0798 100644 --- a/test/unit/cartodb/overviews_query_rewriter.js +++ b/test/unit/cartodb/overviews_query_rewriter.js @@ -523,4 +523,89 @@ describe('Overviews query rewriter', function() { var expected_sql = "SELECT * FROM table1"; assertSameSql(overviews_sql, expected_sql); }); + + it('generates query with filters', function(){ + var sql = "SELECT ST_Transform(the_geom, 3857) the_geom_webmercator, cartodb_id, name\ + FROM (SELECT *\ + FROM (select * from table1) _camshaft_category_filter\ + WHERE name IN ($escape_0$X$escape_0$)) _cdb_analysis_query"; + var data = { + overviews: { + table1: { + 0: { table: 'table1_ov0' }, + 1: { table: 'table1_ov1' }, + 2: { table: 'table1_ov2' }, + 3: { table: 'table1_ov3' } + } + }, + filters: { name_filter: { type: 'category', column: 'name', params: { accept: [ 'X' ] } } }, + unfiltered_query: 'SELECT * FROM table1' + }; + var overviews_sql = overviewsQueryRewriter.query(sql, data); + var expected_sql = "\ + SELECT * FROM (WITH\ + _vovw_scale AS ( SELECT ZoomLevel() AS _vovw_z )\ + SELECT * FROM (\ + SELECT * FROM table1_ov0, _vovw_scale WHERE _vovw_z = 0\ + UNION ALL\ + SELECT * FROM table1_ov1, _vovw_scale WHERE _vovw_z = 1\ + UNION ALL\ + SELECT * FROM table1_ov2, _vovw_scale WHERE _vovw_z = 2\ + UNION ALL\ + SELECT * FROM table1_ov3, _vovw_scale WHERE _vovw_z = 3\ + UNION ALL\ + SELECT * FROM table1, _vovw_scale WHERE _vovw_z > 3\ + ) AS _vovw_table1) _camshaft_category_filter\ + WHERE name IN ($escape_0$X$escape_0$)\ + "; + assertSameSql(overviews_sql, expected_sql); + }); + + it('generates query with filters for specific zoom level', function(){ + var sql = "SELECT ST_Transform(the_geom, 3857) the_geom_webmercator, cartodb_id, name\ + FROM (SELECT *\ + FROM (select * from table1) _camshaft_category_filter\ + WHERE name IN ($escape_0$X$escape_0$)) _cdb_analysis_query"; + var data = { + overviews: { + table1: { + 0: { table: 'table1_ov0' }, + 1: { table: 'table1_ov1' }, + 2: { table: 'table1_ov2' }, + 3: { table: 'table1_ov3' } + } + }, + filters: { name_filter: { type: 'category', column: 'name', params: { accept: [ 'X' ] } } }, + unfiltered_query: 'SELECT * FROM table1', + filter_stats: { unfiltered_rows: 1000, filtered_rows: 900 } + }; + var overviews_sql = overviewsQueryRewriter.query(sql, data, { zoom_level: 2 }); + var expected_sql = "\ + SELECT * FROM (SELECT * FROM table1_ov2) _camshaft_category_filter\ + WHERE name IN ($escape_0$X$escape_0$)\ + "; + assertSameSql(overviews_sql, expected_sql); + }); + + it('does not generates query with aggressive filtering', function(){ + var sql = "SELECT ST_Transform(the_geom, 3857) the_geom_webmercator, cartodb_id, name\ + FROM (SELECT *\ + FROM (select * from table1) _camshaft_category_filter\ + WHERE name IN ($escape_0$X$escape_0$)) _cdb_analysis_query"; + var data = { + overviews: { + table1: { + 0: { table: 'table1_ov0' }, + 1: { table: 'table1_ov1' }, + 2: { table: 'table1_ov2' }, + 3: { table: 'table1_ov3' } + } + }, + filters: { name_filter: { type: 'category', column: 'name', params: { accept: [ 'X' ] } } }, + unfiltered_query: 'SELECT * FROM table1', + filter_stats: { unfiltered_rows: 1000, filtered_rows: 10 } + }; + var overviews_sql = overviewsQueryRewriter.query(sql, data); + assert.equal(overviews_sql, sql); + }); }); From e1aa0bc7ae302ea16d6766988b3d929ed85c1ab3 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 13:09:55 +0200 Subject: [PATCH 26/38] Use JSON format for EXPLAIN --- lib/cartodb/api/filter_stats_api.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/cartodb/api/filter_stats_api.js b/lib/cartodb/api/filter_stats_api.js index 5196d2fa..d872ad8c 100644 --- a/lib/cartodb/api/filter_stats_api.js +++ b/lib/cartodb/api/filter_stats_api.js @@ -9,19 +9,15 @@ function FilterStatsApi(pgQueryRunner) { module.exports = FilterStatsApi; function getEstimatedRows(pgQueryRunner, username, query, callback) { - pgQueryRunner.run(username, "EXPLAIN "+query, function(err, result_rows) { + pgQueryRunner.run(username, "EXPLAIN (FORMAT JSON)"+query, function(err, result_rows) { if (err){ callback(err); return; } var rows; - var query_plan = result_rows[0]['QUERY PLAN']; - var match; - if ( query_plan ) { - match = query_plan.match(/rows=(\d+)/); - } - if ( match ) { - rows = +match[1]; + if ( result_rows[0] && result_rows[0]['QUERY PLAN'] && + result_rows[0]['QUERY PLAN'][0] && result_rows[0]['QUERY PLAN'][0].Plan ) { + rows = result_rows[0]['QUERY PLAN'][0].Plan['Plan Rows']; } return callback(null, rows); }); From ba30f460ee17f86f301918bdbd58ee1b12d7086e Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 13:42:58 +0200 Subject: [PATCH 27/38] Remove comment Overviews will not be used for dataview search --- lib/cartodb/backends/dataview.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/cartodb/backends/dataview.js b/lib/cartodb/backends/dataview.js index 6861a02c..f541cac6 100644 --- a/lib/cartodb/backends/dataview.js +++ b/lib/cartodb/backends/dataview.js @@ -268,8 +268,6 @@ DataviewBackend.prototype.search = function (mapConfigProvider, user, params, ca query = node.getQuery(applyFilters); } - // TODO: should handle overviews as getDataview ? - if (params.bbox) { var bboxFilter = new BBoxFilter({column: 'the_geom', srid: 4326}, {bbox: params.bbox}); query = bboxFilter.sql(query); From 8da7cf73c1dea52da5265a92390647cb65633165 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 13:55:09 +0200 Subject: [PATCH 28/38] Remove comment --- lib/cartodb/models/dataview/overviews/formula.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/cartodb/models/dataview/overviews/formula.js b/lib/cartodb/models/dataview/overviews/formula.js index db305e55..aa57792e 100644 --- a/lib/cartodb/models/dataview/overviews/formula.js +++ b/lib/cartodb/models/dataview/overviews/formula.js @@ -55,7 +55,6 @@ function zoom_level_for_bbox(bbox) { // should use extended overviews metadata to compute this properly. if ( bbox ) { var bbox_values = _.map(bbox.split(','), function(v) { return +v; }); - // TODO: look at possible crossing-180º issues var w = Math.abs(bbox_values[2]-bbox_values[0]); var h = Math.abs(bbox_values[3]-bbox_values[1]); var max_dim = Math.min(w, h); From 4c375780c7897005b8d40648d8fe254f421513f2 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 15:43:20 +0200 Subject: [PATCH 29/38] replace underscore functions by standard (ES5) equivalents Note: _.find(a,...) is not replaced by a.find(...) because it is not available for all the collections we need it for. --- lib/cartodb/backends/dataview.js | 3 +-- lib/cartodb/models/mapconfig_overviews_adapter.js | 2 +- lib/cartodb/utils/overviews_query_rewriter.js | 6 +++--- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/cartodb/backends/dataview.js b/lib/cartodb/backends/dataview.js index f541cac6..44db254e 100644 --- a/lib/cartodb/backends/dataview.js +++ b/lib/cartodb/backends/dataview.js @@ -22,7 +22,6 @@ function DataviewBackend(analysisBackend) { module.exports = DataviewBackend; - DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, params, callback) { var self = this; @@ -125,7 +124,7 @@ DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, param if ( queryRewriteData ) { if ( node.type === 'source' ) { var filters = node.filters; // TODO: node.getFilters() when available in camshaft - var filters_disabler = _.keys(filters).reduce( + var filters_disabler = Object.keys(filters).reduce( function(disabler, filter_id){ disabler[filter_id] = false; return disabler; }, {} ); diff --git a/lib/cartodb/models/mapconfig_overviews_adapter.js b/lib/cartodb/models/mapconfig_overviews_adapter.js index f9f580a8..f5cfde9e 100644 --- a/lib/cartodb/models/mapconfig_overviews_adapter.js +++ b/lib/cartodb/models/mapconfig_overviews_adapter.js @@ -36,7 +36,7 @@ MapConfigOverviewsAdapter.prototype.getLayers = function(username, layers, analy if ( node ) { node = node.rootNode; filters = node.filters; // TODO: node.getFilters() when available in camshaft - var filters_disabler = _.keys(filters).reduce( + var filters_disabler = Object.keys(filters).reduce( function(disabler, filter_id){ disabler[filter_id] = false; return disabler; }, {} ); diff --git a/lib/cartodb/utils/overviews_query_rewriter.js b/lib/cartodb/utils/overviews_query_rewriter.js index e2c7e29d..95a02378 100644 --- a/lib/cartodb/utils/overviews_query_rewriter.js +++ b/lib/cartodb/utils/overviews_query_rewriter.js @@ -161,7 +161,7 @@ function overviews_query_with_zoom_expression(query, overviews, zoom_level_expre var replaced_query = query; var sql = "WITH\n _vovw_scale AS ( SELECT " + zoom_level_expression + " AS _vovw_z )"; var replacement; - _.each(_.keys(overviews), function(table) { + _.each(Object.keys(overviews), function(table) { var table_overviews = overviews[table]; var table_view = overviews_view_name(table); var schema = table_overviews.schema; @@ -181,7 +181,7 @@ function overviews_query_with_zoom_expression(query, overviews, zoom_level_expre function overviews_query_with_definite_zoom(query, overviews, zoom_level) { var replaced_query = query; var replacement; - _.each(_.keys(overviews), function(table) { + _.each(Object.keys(overviews), function(table) { var table_overviews = overviews[table]; var schema = table_overviews.schema; replacement = overview_table_for_zoom_level(table_overviews, zoom_level); @@ -196,7 +196,7 @@ function overview_table_for_zoom_level(table_overviews, zoom_level) { if ( table_overviews ) { overview_table = table_overviews[zoom_level]; if ( !overview_table ) { - _.every(_.keys(table_overviews).sort(function(x,y){ return x-y; }), function(overview_zoom) { + _.every(Object.keys(table_overviews).sort(function(x,y){ return x-y; }), function(overview_zoom) { if ( +overview_zoom > +zoom_level ) { overview_table = table_overviews[overview_zoom]; return false; From e98a5aeff0ff3db47108f578bc6fd522acf6fc5c Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 15:48:30 +0200 Subject: [PATCH 30/38] Small code clean-up --- lib/cartodb/utils/overviews_query_rewriter.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/cartodb/utils/overviews_query_rewriter.js b/lib/cartodb/utils/overviews_query_rewriter.js index 95a02378..ebb91fb0 100644 --- a/lib/cartodb/utils/overviews_query_rewriter.js +++ b/lib/cartodb/utils/overviews_query_rewriter.js @@ -233,11 +233,12 @@ function overview_table_for_zoom_level(table_overviews, zoom_level) { // } OverviewsQueryRewriter.prototype.query = function(query, data, options) { options = options || {}; + data = data || {}; - var overviews = data && data.overviews; - var unfiltered_query = data && data.unfiltered_query; - var filters = data && data.filters; - var bbox_filter = data && data.bbox_filter; + var overviews = data.overviews; + var unfiltered_query = data.unfiltered_query; + var filters = data.filters; + var bbox_filter = data.bbox_filter; if ( !unfiltered_query ) { unfiltered_query = query; From 7ce4104d2fd15dce6f35aff554f27ab30fc79d95 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 16:11:30 +0200 Subject: [PATCH 31/38] Release version 2.43.0 --- NEWS.md | 7 +++++-- package.json | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/NEWS.md b/NEWS.md index 19ad5960..1aeb6c78 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,8 +1,11 @@ # Changelog -## 2.42.3 +## 2.43.0 -Released 2016-mm-dd +Released 2016-05-18 + +New features: + - Overviews now support dataviews and filtering #449 ## 2.42.2 diff --git a/package.json b/package.json index 5ce84b37..b801eb6f 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "private": true, "name": "windshaft-cartodb", - "version": "2.42.3", + "version": "2.43.0", "description": "A map tile server for CartoDB", "keywords": [ "cartodb" From 8628d3b671a0c9536b394963935e33b96572ae35 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 16:15:27 +0200 Subject: [PATCH 32/38] Stub next version --- NEWS.md | 5 +++++ package.json | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 1aeb6c78..fde7f461 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,10 @@ # Changelog +## 2.43.1 + +Released 2016-mm-dd + + ## 2.43.0 Released 2016-05-18 diff --git a/package.json b/package.json index b801eb6f..ecf28729 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "private": true, "name": "windshaft-cartodb", - "version": "2.43.0", + "version": "2.43.1", "description": "A map tile server for CartoDB", "keywords": [ "cartodb" From a1e024e228973b4d6b4fa2260e9e4e485d54847d Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 17:49:09 +0200 Subject: [PATCH 33/38] Fix dataview problem for bbox with no query rewrite data Fixes #457 --- lib/cartodb/backends/dataview.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/backends/dataview.js b/lib/cartodb/backends/dataview.js index 44db254e..89b811bb 100644 --- a/lib/cartodb/backends/dataview.js +++ b/lib/cartodb/backends/dataview.js @@ -149,7 +149,9 @@ DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, param bbox: params.bbox } }; - queryRewriteData = _.extend(queryRewriteData, { bbox_filter: bbox_filter_definition }); + if ( queryRewriteData ) { + queryRewriteData = _.extend(queryRewriteData, { bbox_filter: bbox_filter_definition }); + } } var dataviewFactory = DataviewFactoryWithOverviews.getFactory( From 5989ab344d450155f71885e8e32d6a6b59b156ac Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 18:02:08 +0200 Subject: [PATCH 34/38] Add test to detect problem #457 --- test/acceptance/dataviews/overviews.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/acceptance/dataviews/overviews.js b/test/acceptance/dataviews/overviews.js index 9acda126..0ab7e8bd 100644 --- a/test/acceptance/dataviews/overviews.js +++ b/test/acceptance/dataviews/overviews.js @@ -58,6 +58,21 @@ describe('dataviews using tables without overviews', function() { }); }); + it("should admit a bbox", function(done) { + var params = { + bbox: "-170,-80,170,80" + }; + var testClient = new TestClient(nonOverviewsMapConfig); + testClient.getDataview('country_places_count', params, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, { operation: 'count', result: 7253, nulls: 0, type: 'formula' }); + + testClient.drain(done); + }); + }); + describe('filters', function() { describe('category', function () { From 9206b1a1b502bf0d032de8cfc0c740d4a273233f Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 18:16:32 +0200 Subject: [PATCH 35/38] Fix dataviews/overviews tests and add some new cases --- test/acceptance/dataviews/overviews.js | 161 ++++++++++++++++--------- 1 file changed, 106 insertions(+), 55 deletions(-) diff --git a/test/acceptance/dataviews/overviews.js b/test/acceptance/dataviews/overviews.js index 0ab7e8bd..601124a7 100644 --- a/test/acceptance/dataviews/overviews.js +++ b/test/acceptance/dataviews/overviews.js @@ -92,6 +92,23 @@ describe('dataviews using tables without overviews', function() { testClient.drain(done); }); }); + + it("should expose a filtered formula and admit a bbox", function (done) { + var params = { + filters: { + dataviews: {country_categories: {accept: ['CAN']}} + }, + bbox: "-170,-80,170,80" + }; + var testClient = new TestClient(nonOverviewsMapConfig); + testClient.getDataview('country_places_count', params, function (err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, { operation: 'count', result: 254, nulls: 0, type: 'formula' }); + testClient.drain(done); + }); + }); }); }); @@ -233,16 +250,32 @@ describe('dataviews using tables with overviews', function() { }); }); + it("should admit a bbox", function(done) { + var params = { + bbox: "-170,-80,170,80" + }; + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_sum', params, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"sum","result":15,"nulls":0,"type":"formula"}); + + testClient.drain(done); + }); + }); + describe('filters', function() { describe('category', function () { - it("should expose a filtered formula", function (done) { - var params = { - filters: { - dataviews: {test_categories: {accept: ['Hawai']}} - } - }; + var params = { + filters: { + dataviews: {test_categories: {accept: ['Hawai']}} + } + }; + + it("should expose a filtered sum formula", function (done) { var testClient = new TestClient(overviewsMapConfig); testClient.getDataview('test_sum', params, function (err, formula_result) { if (err) { @@ -251,56 +284,74 @@ describe('dataviews using tables with overviews', function() { assert.deepEqual(formula_result, {"operation":"sum","result":1,"nulls":0,"type":"formula"}); testClient.drain(done); }); - - it("should expose an avg formula", function(done) { - var testClient = new TestClient(overviewsMapConfig); - testClient.getDataview('test_avg', { own_filter: 0 }, function(err, formula_result) { - if (err) { - return done(err); - } - assert.deepEqual(formula_result, {"operation":"avg","result":1,"nulls":0,"type":"formula"}); - - testClient.drain(done); - }); - }); - - it("should expose a count formula", function(done) { - var testClient = new TestClient(overviewsMapConfig); - testClient.getDataview('test_count', { own_filter: 0 }, function(err, formula_result) { - if (err) { - return done(err); - } - assert.deepEqual(formula_result, {"operation":"count","result":1,"nulls":0,"type":"formula"}); - - testClient.drain(done); - }); - }); - - it("should expose a max formula", function(done) { - var testClient = new TestClient(overviewsMapConfig); - testClient.getDataview('test_max', { own_filter: 0 }, function(err, formula_result) { - if (err) { - return done(err); - } - assert.deepEqual(formula_result, {"operation":"max","result":1,"nulls":0,"type":"formula"}); - - testClient.drain(done); - }); - }); - - it("should expose a min formula", function(done) { - var testClient = new TestClient(overviewsMapConfig); - testClient.getDataview('test_min', { own_filter: 0 }, function(err, formula_result) { - if (err) { - return done(err); - } - assert.deepEqual(formula_result, {"operation":"min","result":1,"nulls":0,"type":"formula"}); - - testClient.drain(done); - }); - }); - }); + + it("should expose a filtered avg formula", function(done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_avg', params, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"avg","result":1,"nulls":0,"type":"formula"}); + + testClient.drain(done); + }); + }); + + it("should expose a filtered count formula", function(done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_count', params, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"count","result":1,"nulls":0,"type":"formula"}); + + testClient.drain(done); + }); + }); + + it("should expose a filterd max formula", function(done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_max', params, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"max","result":1,"nulls":0,"type":"formula"}); + + testClient.drain(done); + }); + }); + + it("should expose a filterd min formula", function(done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_min', params, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"min","result":1,"nulls":0,"type":"formula"}); + + testClient.drain(done); + }); + }); + + it("should expose a filtered sum formula with bbox", function (done) { + var bboxparams = { + filters: { + dataviews: {test_categories: {accept: ['Hawai']}} + }, + bbox: "-170,-80,170,80" + }; + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_sum', bboxparams, function (err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"sum","result":1,"nulls":0,"type":"formula"}); + testClient.drain(done); + }); + }); + + }); }); From 2a06405a58b7fae3fa2b4d72d7df739112c2f5d3 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 18:21:17 +0200 Subject: [PATCH 36/38] Move definition to the scope where it's needed --- lib/cartodb/backends/dataview.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/cartodb/backends/dataview.js b/lib/cartodb/backends/dataview.js index 89b811bb..b7bb6d0b 100644 --- a/lib/cartodb/backends/dataview.js +++ b/lib/cartodb/backends/dataview.js @@ -139,17 +139,17 @@ DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, param if (params.bbox) { var bboxFilter = new BBoxFilter({column: 'the_geom', srid: 4326}, {bbox: params.bbox}); query = bboxFilter.sql(query); - var bbox_filter_definition = { - type: 'bbox', - options: { - column: 'the_geom', - srid: 4326, - }, - params: { - bbox: params.bbox - } - }; if ( queryRewriteData ) { + var bbox_filter_definition = { + type: 'bbox', + options: { + column: 'the_geom', + srid: 4326, + }, + params: { + bbox: params.bbox + } + }; queryRewriteData = _.extend(queryRewriteData, { bbox_filter: bbox_filter_definition }); } } From 289ffbbedcd3e579d5a510e2a681c9499294a6f6 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 19 May 2016 12:33:41 +0200 Subject: [PATCH 37/38] Release 2.43.1 --- NEWS.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index fde7f461..7e495c7d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,7 +2,10 @@ ## 2.43.1 -Released 2016-mm-dd +Released 2016-05-19 + +Bug fixes: + - Dataview error when bbox present without query rewrite data #458 ## 2.43.0 From 2e79781711d14627a1dba97fc87bfb7c3f5976ca Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Thu, 19 May 2016 13:32:32 +0200 Subject: [PATCH 38/38] Adds support for sql wrap in all layers Previously it was only working for analyses ones. --- NEWS.md | 7 +++ lib/cartodb/controllers/map.js | 6 ++ .../adapter/sql-wrap-mapconfig-adapter.js | 25 +++++++++ .../models/mapconfig/named_map_provider.js | 6 ++ npm-shrinkwrap.json | 2 +- package.json | 2 +- test/acceptance/sql-wrap.js | 52 ++++++++++++++++++ test/fixtures/sql-wrap-usa-filter.png | Bin 0 -> 4197 bytes 8 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 lib/cartodb/models/mapconfig/adapter/sql-wrap-mapconfig-adapter.js create mode 100644 test/acceptance/sql-wrap.js create mode 100644 test/fixtures/sql-wrap-usa-filter.png diff --git a/NEWS.md b/NEWS.md index 7e495c7d..9eeb06e3 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,12 @@ # Changelog +## 2.44.0 + +Released 2016-mm-dd + +Adds support for sql wrap in all layers + + ## 2.43.1 Released 2016-05-19 diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index feaf6fa5..1f8f25bb 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -19,6 +19,7 @@ var MapConfigNamedLayersAdapter = require('../models/mapconfig_named_layers_adap var AnalysisMapConfigAdapter = require('../models/analysis-mapconfig-adapter'); var NamedMapMapConfigProvider = require('../models/mapconfig/named_map_provider'); var CreateLayergroupMapConfigProvider = require('../models/mapconfig/create_layergroup_provider'); +var SqlWrapMapConfigAdapter = require('../models/mapconfig/adapter/sql-wrap-mapconfig-adapter'); /** * @param {AuthApi} authApi @@ -52,6 +53,7 @@ function MapController(authApi, pgConnection, templateMaps, mapBackend, metadata this.analysisMapConfigAdapter = new AnalysisMapConfigAdapter(analysisBackend); this.namedLayersAdapter = new MapConfigNamedLayersAdapter(templateMaps); this.overviewsAdapter = overviewsAdapter; + this.sqlWrapMapConfigAdapter = new SqlWrapMapConfigAdapter(); } util.inherits(MapController, BaseController); @@ -139,6 +141,10 @@ MapController.prototype.create = function(req, res, prepareConfigFn) { self.req2params(req, this); }, prepareConfigFn, + function prepareSqlWrap(err, requestMapConfig) { + assert.ifError(err); + self.sqlWrapMapConfigAdapter.getMapConfig(requestMapConfig, this); + }, function prepareAnalysisLayers(err, requestMapConfig) { assert.ifError(err); var analysisConfiguration = { diff --git a/lib/cartodb/models/mapconfig/adapter/sql-wrap-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/sql-wrap-mapconfig-adapter.js new file mode 100644 index 00000000..6b9a5130 --- /dev/null +++ b/lib/cartodb/models/mapconfig/adapter/sql-wrap-mapconfig-adapter.js @@ -0,0 +1,25 @@ +function SqlWrapMapConfigAdapter() { +} + +module.exports = SqlWrapMapConfigAdapter; + + +SqlWrapMapConfigAdapter.prototype.getMapConfig = function(requestMapConfig, callback) { + if (requestMapConfig && Array.isArray(requestMapConfig.layers)) { + requestMapConfig.layers = requestMapConfig.layers.map(function(layer) { + if (layer.options) { + var sqlQueryWrap = layer.options.sql_wrap; + if (sqlQueryWrap) { + var layerSql = layer.options.sql; + if (layerSql) { + layer.options.sql_raw = layerSql; + layer.options.sql = sqlQueryWrap.replace(/<%=\s*sql\s*%>/g, layerSql); + } + } + } + return layer; + }); + } + + return callback(null, requestMapConfig); +}; diff --git a/lib/cartodb/models/mapconfig/named_map_provider.js b/lib/cartodb/models/mapconfig/named_map_provider.js index e285b2b7..e1ad6fea 100644 --- a/lib/cartodb/models/mapconfig/named_map_provider.js +++ b/lib/cartodb/models/mapconfig/named_map_provider.js @@ -6,6 +6,7 @@ var step = require('step'); var MapConfig = require('windshaft').model.MapConfig; var templateName = require('../../backends/template_maps').templateName; var QueryTables = require('cartodb-query-tables'); +var SqlWrapMapConfigAdapter = require('./adapter/sql-wrap-mapconfig-adapter'); /** * @constructor @@ -22,6 +23,7 @@ function NamedMapMapConfigProvider(templateMaps, pgConnection, metadataBackend, this.turboCartoAdapter = turboCartoAdapter; this.analysisMapConfigAdapter = analysisMapConfigAdapter; this.overviewsAdapter = overviewsAdapter; + this.sqlWrapMapConfigAdapter = new SqlWrapMapConfigAdapter(); this.owner = owner; this.templateName = templateName(templateId); @@ -95,6 +97,10 @@ NamedMapMapConfigProvider.prototype.getMapConfig = function(callback) { assert.ifError(err); return self.templateMaps.instance(self.template, templateParams); }, + function prepareSqlWrap(err, requestMapConfig) { + assert.ifError(err); + self.sqlWrapMapConfigAdapter.getMapConfig(requestMapConfig, this); + }, function prepareAnalysisLayers(err, requestMapConfig) { assert.ifError(err); var analysisConfiguration = { diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index c3b9529d..04f931c5 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1,6 +1,6 @@ { "name": "windshaft-cartodb", - "version": "2.42.3", + "version": "2.44.0", "dependencies": { "body-parser": { "version": "1.14.2", diff --git a/package.json b/package.json index ecf28729..c426011b 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "private": true, "name": "windshaft-cartodb", - "version": "2.43.1", + "version": "2.44.0", "description": "A map tile server for CartoDB", "keywords": [ "cartodb" diff --git a/test/acceptance/sql-wrap.js b/test/acceptance/sql-wrap.js new file mode 100644 index 00000000..7b3c25d8 --- /dev/null +++ b/test/acceptance/sql-wrap.js @@ -0,0 +1,52 @@ +require('../support/test_helper'); + +var assert = require('../support/assert'); +var TestClient = require('../support/test-client'); + +describe('sql-wrap', function() { + + afterEach(function(done) { + if (this.testClient) { + this.testClient.drain(done); + } else { + return done(); + } + }); + + it('should use sql_wrap from layer options', function(done) { + var mapConfig = { + version: '1.5.0', + layers: [ + { + "type": "cartodb", + "options": { + "sql": "SELECT * FROM populated_places_simple_reduced", + "sql_wrap": "SELECT * FROM (<%= sql %>) _w WHERE adm0_a3 = 'USA'", + "cartocss": [ + "#points {", + " marker-fill-opacity: 1;", + " marker-line-color: #FFF;", + " marker-line-width: 0.5;", + " marker-line-opacity: 1;", + " marker-placement: point;", + " marker-type: ellipse;", + " marker-width: 8;", + " marker-fill: red;", + " marker-allow-overlap: true;", + "}" + ].join('\n'), + "cartocss_version": "2.3.0" + } + } + ] + }; + + this.testClient = new TestClient(mapConfig, 1234); + this.testClient.getTile(0, 0, 0, function(err, tile, img) { + assert.ok(!err, err); + var fixtureImg = './test/fixtures/sql-wrap-usa-filter.png'; + assert.imageIsSimilarToFile(img, fixtureImg, 20, done); + }); + }); + +}); diff --git a/test/fixtures/sql-wrap-usa-filter.png b/test/fixtures/sql-wrap-usa-filter.png new file mode 100644 index 0000000000000000000000000000000000000000..30bd4eb341b814a73a88b99177b2f0461d811189 GIT binary patch literal 4197 zcmb_g`9Bkm8{fv9b0l}mEkuZk-1nKI)Q2KRE6gp_oD-i)j$FCIC?=7soaM}!Ym6K@ zGg>2aHDfdT`X9cZ*Xwz`p693c>v?{8AI}R1I}0ujQ4RnAz-47=dIJCeo{m5O8_Q{P zeOTcI0PreVnHt?h7H%!tr`-B}zH2wob+$M41;TaeDW~HVW!LfEr<~QCwlLqy7qX%n z{Ws6qLMD3oTzLZ^*M&}!g&XoYUR>|3V3JpTp=z1lea7e{QT?s7GrPm3_#3KMW0v=i z^+>Lym{R#r)+I{T!pKMgh190`ZM$&!s3;Um9}SYR%KVQ?yBd>`VNgz4qB-4Z^1m%A z8hPcs9`$+oa#mTE66Wtbbmx=Tmc4gj1|LsR%p>bG-H(a^OS+{*iX2hKYQEV$e<{)r zrnh5g9r~*w?YVd$s(mH?k#t=Yr4fNd+KqAd_ty;7mQMJbx9vn($;q=aM)GNsZ4nBi zMFqIq-i=YHm+Dx5gM&l7!YDdMz9=eiXs$ckNzBS7Cps4${m>~8`p*Ys!5fpwVly1} zRgQ}INOuP^WTWh)FJXX6eTgc3wn}`L)?Qv4Bojl_?0tFgTMM(2@xxe0JY-myMxFZPLntc!)me|6E<+3%+(b>gMr0Rchia= zFCET|7I!uhu@~Ma!*q2u7g3B(9e7MU>gU%3=gt}+a;JOaXSZx_S=iC~K{c+|A%h}1#==5zET}1Y~grQ&5QB|#7 zwWraSI*akMvy{bLT;(msX3Bo)p#|J<+Tj|*d-TU>it%Le^&6>wSCcm{CKTTjYo`e~ zH6#?fosehdZv(+!D`(nvjIEoOew_SRHo%GASy=i@$eJ>X+-f9WjY$I=P!=Y+PYQzc z^}B8ZOoL}0KwyGXfWz4C_yjwkwWI5gI!`O^SiwV6fnu;>u3My19YR8wJ6IK3;RmP? zN(Y*9dHVcLE;lGH6GdRrM8vNVJZBvAJ`=NkYWG?DhUZN8S&pOeso%K(w|mTlQpNco z5PvHgh0#I8UVJaZ#T|R|YPL{*SLLlONgx*xAIlMaA20&g$xf&=5MlNMAq$_y8%q*a zqSTTF6sL6rTLivIs0Fj zSWi!A!O24AoxM9KP~*I@c+Szs1+jZ0fU z$nVC!ieEek#~-b(igAl3d?t2Qi1cm_^7=rM)Y*q~F8NVE+32aHrYyi}yf1hpKe67) z9Ls%2L%KEp1mWp<9BqfI2Ppu;i^#A2sXQ9=ISe{ib2qV-sx=65Ug?qTfR=6FJV^$B z{jwd%x@Rf^0C%;dCz?OG70-X}63BXVSm&UPy73F&PlEw2zmzj60eTFnOq2IDB9Hcr zB-1rOFyXcf<=puK&aHbM)Su6nTOHG3W@SA3E&A>lq3_XNOi4mJz1{p0TueY)uevbe zAz?4_8P8v8v^|s2w!MiVQDwLBg6JW9Ea%=7*&! zUw|-epvnCvjolYiB*HApstmYD5)iI^kTIe;Q5kREn}+;a`6o zZj*X-dvLJF78U!R(1_dZe5bRm3sW{!Sj_>6>6-W&Y=E*QEP# zD}jEjd6IP!@)|13oN=pAvkv*4Gb`&Qsboyr8zNSjtQ$zqPRRAu%cg(M2Xa4RNty#DMc-%JoNj-G?MaW<0go0uR@LNPH?s)g zJ>KFne_3aD${{M`GHBh0Dfl(eb;^w?dGsS)L_?$Lp z{OUYCCYDLf6$#bvD=%l6*FNWqocK3?N8Zrzp`ppyM?6Q!+}k3u*enz*I+8kMVQYhPlJ=7Us*s{|}8 z7_YsB1XbMfo7HKDcrzs#YgXt75B1Nd=XVGZQjE5shW}Bp>!gQBoC0USV!_^5*pUq2i1x z$$|cw4XW5T*Omxt_hn1vl0Ab4;KciiPnLwH`HnH5{q^k}7w$E5MneJoR7~Wy=;{5R zekZ0RfFwdjZ6jRaj)8uns2oR zoqspB9+b=rBwhcMSPaM9`!33&1{URX>skIadnyyL?h8umJv;`!;3Lhe_!3FUb3Y{# z5(BW!fiVp)7zJT>d^7+vYi@Z#>|KHFI^7nAGXXYY#2DANBi#=1Qr;0XWlgtq;7xvUn5Errursl0z6CWBgs`7WjVdTBzRcsdArE}ZFh*8%~*1K@1q zmzWa4Hq3I^OWW>)Z!xQTtCnO1pS|qReLbuslDG(WQTGy+YA5Kz)R^_a*OQVVM#z}@ zGbQ@nn-}CItScut(5PQ-MF{=)fVWFU60$q&*63k46vj5it!3dF#7j{wYkt?%<(F&( zRfI;^h5U5n=cp1RXdEy4L?3>k9o(J%%ebyBiy19g{==+hWCb@|5sae9<0hiiWrewz z=Kw~wb^DQHjvasULbM5`tlnCNCWf`=!I+~UqCOw1&n4JVv-Y97Y_j-<>;*F_Y@RsU#vRoYgs@IK1_#tkKTHCTz{ zn^o+QkB3k25CI>uq%zmv6kxAGH+W@tyLHv>wQY&nnRK!IIE;l#8nHC@g0%d>LiiUx z3vGAw)s=1jT!X9!Si+7VF zl;^;y024Mbd?Q4?LXmfT@rv@g#uNuz?3fAs`}kJ{V#C+$IK;rq17ze|w=YOla}|=K z;gd9vnoW9Xh(`slj=(^sXY5Qh(4)erNNO7VXEwxrlA%d#nAouGtc9Y^R`ez3B(Y{6 zKAbkZ8z&H1Mq$=SHT^KFIL%ba5e4H5rzo#LVNlpj1GmpgM5oLHQoFJfttu(z??_+0 zzGD!Ci;a{gxE3Xv3p=82r2+_vWM=o+2&YMgBC+9>qaWxCG_H@# zU{+opFvlE26%P;fM1ZL*CJFay-EX;6_zL@&`Z(N+0UC){f%7whht-H{md|me8J9Ftn=NoE-CT&Tn*4VnPGrwGA!jyHTg>&;Zr>k7ZvrTqGXDuakH}? zfy|a$R_nwV_Px&%%7Uarw7J9q=L+-|Ibl$ya!iCX?sV~>Nx;N@RuS@CC~<9g6Xe4z z^)#(s-@vg^t_#8MtnQ!$>fg{6<>q#?c#}RL!WWDP`V%^MH01Jh7h2Azpu3l?By|N+ zzawXhZiu2}F0k>|l^t%y9RC=O$D^OtKU4*2MWRGmaR)wZQY*>HDT4r{vOAp7m3J%g z(!`ugI=WK>hM+Pe$*^wzZ zPTSW?1rvNVv}_sTWeSo(i+yj4tUmm^-@F(51j5{KyH#lgU+G#-w9~4|}8~EK0uL|8@z0cIPJ+Jj3J6 zX%<2CBGT_Gxr>KYM;Kf zDQ3bj>t86if_`YkQOQF)`!CT)9+TVT+`;?=$5G=?lL@lHw&KY;MG-LTW9U$-3Q3>H+H7d+WoBnuW9$+CKd9CZH~;_u literal 0 HcmV?d00001