From d1373bec6614d91ea93f1a0acc8b3afe73d701a6 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Mon, 4 Aug 2014 17:48:59 +0200 Subject: [PATCH 1/5] Improves SQL query for affected tables and last updated time --- lib/cartodb/api/query_tables_api.js | 7 +++---- test/acceptance/multilayer.js | 29 +++++++++++++---------------- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/lib/cartodb/api/query_tables_api.js b/lib/cartodb/api/query_tables_api.js index 18920b59..52e3a989 100644 --- a/lib/cartodb/api/query_tables_api.js +++ b/lib/cartodb/api/query_tables_api.js @@ -66,11 +66,10 @@ QueryTablesApi.prototype.getAffectedTablesAndLastUpdatedTime = function (usernam ; var query = [ - 'SELECT', - 'CDB_QueryTables($windshaft$' + sql + '$windshaft$) as tablenames,', - 'EXTRACT(EPOCH FROM max(updated_at)) as max', + 'WITH querytables AS (SELECT * FROM CDB_QueryTables($windshaft$' + sql + '$windshaft$) as tablenames)', + 'SELECT (SELECT tablenames FROM querytables), EXTRACT(EPOCH FROM max(updated_at)) as max', 'FROM CDB_TableMetadata m', - 'WHERE m.tabname = any (CDB_QueryTables($windshaft$' + sql + '$windshaft$)::regclass[])' + 'WHERE m.tabname = any ((SELECT tablenames from querytables)::regclass[])' ].join(' '); sqlApi.query(username, api_key, query, function(err, rows){ diff --git a/test/acceptance/multilayer.js b/test/acceptance/multilayer.js index 89e4c25d..fd95edd7 100644 --- a/test/acceptance/multilayer.js +++ b/test/acceptance/multilayer.js @@ -112,13 +112,12 @@ suite('multilayer', function() { var jsonquery = cc.substring(dbname.length+1); var sentquery = JSON.parse(jsonquery); var expectedQuery = [layergroup.layers[0].options.sql, ';', layergroup.layers[1].options.sql].join(''); - assert.equal(sentquery.q, 'SELECT CDB_QueryTables($windshaft$' + assert.equal(sentquery.q, 'WITH querytables AS (SELECT * FROM CDB_QueryTables($windshaft$' + expectedQuery - + '$windshaft$) as tablenames, EXTRACT(EPOCH FROM max(updated_at)) as max' + + '$windshaft$) as tablenames)' + + ' SELECT (SELECT tablenames FROM querytables), EXTRACT(EPOCH FROM max(updated_at)) as max' + ' FROM CDB_TableMetadata m' - + ' WHERE m.tabname = any (CDB_QueryTables($windshaft$' - + expectedQuery - + '$windshaft$)::regclass[])'); + + ' WHERE m.tabname = any ((SELECT tablenames from querytables)::regclass[])'); assert.imageEqualsFile(res.body, 'test/fixtures/test_table_0_0_0_multilayer1.png', IMAGE_EQUALS_HIGHER_TOLERANCE_PER_MIL, function(err, similarity) { @@ -395,13 +394,12 @@ suite('multilayer', function() { .replace(/!bbox!/g, 'ST_MakeEnvelope(0,0,0,0)') .replace(/!pixel_width!/g, '1') .replace(/!pixel_height!/g, '1'); - assert.equal(sentquery.q, 'SELECT CDB_QueryTables($windshaft$' + assert.equal(sentquery.q, 'WITH querytables AS (SELECT * FROM CDB_QueryTables($windshaft$' + expectedQuery - + '$windshaft$) as tablenames, EXTRACT(EPOCH FROM max(updated_at)) as max' + + '$windshaft$) as tablenames)' + + ' SELECT (SELECT tablenames FROM querytables), EXTRACT(EPOCH FROM max(updated_at)) as max' + ' FROM CDB_TableMetadata m' - + ' WHERE m.tabname = any (CDB_QueryTables($windshaft$' - + expectedQuery - + '$windshaft$)::regclass[])'); + + ' WHERE m.tabname = any ((SELECT tablenames from querytables)::regclass[])'); assert.imageEqualsFile(res.body, 'test/fixtures/test_multilayer_bbox.png', IMAGE_EQUALS_TOLERANCE_PER_MIL, function(err, similarity) { @@ -433,13 +431,12 @@ suite('multilayer', function() { .replace('!bbox!', 'ST_MakeEnvelope(0,0,0,0)') .replace('!pixel_width!', '1') .replace('!pixel_height!', '1'); - assert.equal(sentquery.q, 'SELECT CDB_QueryTables($windshaft$' - + expectedQuery - + '$windshaft$) as tablenames, EXTRACT(EPOCH FROM max(updated_at)) as max' - + ' FROM CDB_TableMetadata m' - + ' WHERE m.tabname = any (CDB_QueryTables($windshaft$' + assert.equal(sentquery.q, 'WITH querytables AS (SELECT * FROM CDB_QueryTables($windshaft$' + expectedQuery - + '$windshaft$)::regclass[])'); + + '$windshaft$) as tablenames)' + + ' SELECT (SELECT tablenames FROM querytables), EXTRACT(EPOCH FROM max(updated_at)) as max' + + ' FROM CDB_TableMetadata m' + + ' WHERE m.tabname = any ((SELECT tablenames from querytables)::regclass[])'); assert.imageEqualsFile(res.body, 'test/fixtures/test_multilayer_bbox.png', IMAGE_EQUALS_TOLERANCE_PER_MIL, function(err, similarity) { From 5cf79c82bb8ef6c95153bae88a6b2d59ea8e6c80 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Wed, 6 Aug 2014 21:48:08 +0200 Subject: [PATCH 2/5] Configurable QueryTablesAPI to call directly postgresql using cartodb-psql or to keep using a request to the SQL API --- lib/cartodb/api/query_tables_api.js | 96 +++++++++++++++++++---------- lib/cartodb/server_options.js | 18 +++++- npm-shrinkwrap.json | 23 +++++++ package.json | 1 + test/acceptance/multilayer.js | 7 ++- test/acceptance/server.js | 8 ++- test/acceptance/templates.js | 7 ++- 7 files changed, 123 insertions(+), 37 deletions(-) diff --git a/lib/cartodb/api/query_tables_api.js b/lib/cartodb/api/query_tables_api.js index 52e3a989..61ef8d92 100644 --- a/lib/cartodb/api/query_tables_api.js +++ b/lib/cartodb/api/query_tables_api.js @@ -1,4 +1,5 @@ -var sqlApi = require('../sql/sql_api'); +var sqlApi = require('../sql/sql_api'), + PSQL = require('cartodb-psql'); function QueryTablesApi() { } @@ -33,7 +34,7 @@ QueryTablesApi.prototype.getLastUpdatedTime = function (username, api_key, table }); }; -QueryTablesApi.prototype.getAffectedTablesInQuery = function (username, api_key, sql, callback) { +QueryTablesApi.prototype.getAffectedTablesInQuery = function (username, options, sql, callback) { // Replace mapnik tokens sql = sql .replace(affectedTableRegexCache.bbox, 'ST_MakeEnvelope(0,0,0,0)') @@ -45,20 +46,32 @@ QueryTablesApi.prototype.getAffectedTablesInQuery = function (username, api_key, sql = 'SELECT CDB_QueryTables($windshaft$' + sql + '$windshaft$)'; // call sql api - sqlApi.query(username, api_key, sql, function(err, rows){ - if (err){ - var msg = err.message ? err.message : err; - callback(new Error('could not fetch source tables: ' + msg)); - return; - } - var qtables = rows[0].cdb_querytables; - var tableNames = qtables.split(/^\{(.*)\}$/)[1]; - tableNames = tableNames ? tableNames.split(',') : []; - callback(null, tableNames); - }); + if (shouldQueryPostgresDirectly()) { + var psql = new PSQL(options); + psql.query(sql, function(err, resultSet) { + var rows = resultSet.rows || []; + handleAffectedTablesInQueryRows(err, rows, callback); + }); + } else { + sqlApi.query(username, options.api_key, sql, function(err, rows) { + handleAffectedTablesInQueryRows(err, rows, callback); + }); + } }; -QueryTablesApi.prototype.getAffectedTablesAndLastUpdatedTime = function (username, api_key, sql, callback) { +function handleAffectedTablesInQueryRows(err, rows, callback) { + if (err){ + var msg = err.message ? err.message : err; + callback(new Error('could not fetch source tables: ' + msg)); + return; + } + var qtables = rows[0].cdb_querytables; + var tableNames = qtables.split(/^\{(.*)\}$/)[1]; + tableNames = tableNames ? tableNames.split(',') : []; + callback(null, tableNames); +} + +QueryTablesApi.prototype.getAffectedTablesAndLastUpdatedTime = function (username, options, sql, callback) { sql = sql .replace(affectedTableRegexCache.bbox, 'ST_MakeEnvelope(0,0,0,0)') .replace(affectedTableRegexCache.pixel_width, '1') @@ -72,23 +85,42 @@ QueryTablesApi.prototype.getAffectedTablesAndLastUpdatedTime = function (usernam 'WHERE m.tabname = any ((SELECT tablenames from querytables)::regclass[])' ].join(' '); - sqlApi.query(username, api_key, query, function(err, rows){ - if (err || rows.length === 0) { - var msg = err.message ? err.message : err; - callback(new Error('could not fetch affected tables and last updated time: ' + msg)); - return; - } - - var result = rows[0]; - - var tableNames = result.tablenames.split(/^\{(.*)\}$/)[1]; - tableNames = tableNames ? tableNames.split(',') : []; - - var lastUpdatedTime = result.max || 0; - - callback(null, { - affectedTables: tableNames, - lastUpdatedTime: lastUpdatedTime * 1000 + if (shouldQueryPostgresDirectly()) { + var psql = new PSQL(options); + psql.query(query, function(err, resultSet) { + var rows = resultSet.rows || []; + handleAffectedTablesAndLastUpdatedTimeRows(err, rows, callback); }); - }); + } else { + sqlApi.query(username, options.api_key, query, function(err, rows) { + handleAffectedTablesAndLastUpdatedTimeRows(err, rows, callback); + }); + } }; + +function handleAffectedTablesAndLastUpdatedTimeRows(err, rows, callback) { + if (err || rows.length === 0) { + var msg = err.message ? err.message : err; + callback(new Error('could not fetch affected tables and last updated time: ' + msg)); + return; + } + + var result = rows[0]; + + var tableNames = result.tablenames.split(/^\{(.*)\}$/)[1]; + tableNames = tableNames ? tableNames.split(',') : []; + + var lastUpdatedTime = result.max || 0; + + callback(null, { + affectedTables: tableNames, + lastUpdatedTime: lastUpdatedTime * 1000 + }); +} + + +function shouldQueryPostgresDirectly() { + return global.environment + && global.environment.enabledFeatures + && global.environment.enabledFeatures.cdbQueryTablesFromPostgres; +} diff --git a/lib/cartodb/server_options.js b/lib/cartodb/server_options.js index cd8098ad..751a8218 100644 --- a/lib/cartodb/server_options.js +++ b/lib/cartodb/server_options.js @@ -191,7 +191,14 @@ module.exports = function(){ if ( req.profiler ) req.profiler.done('getSignerMapKey'); key = data; } - queryTablesApi.getAffectedTablesInQuery(user, key, sql, this); // in addCacheChannel + queryTablesApi.getAffectedTablesInQuery(user, { + user: req.params.dbuser, + pass: req.params.dbpass, + host: req.params.dbhost, + port: req.params.dbport, + dbname: req.params.dbname, + api_key: key + }, sql, this); // in addCacheChannel }, function finish(err, data) { next(err,data); @@ -316,7 +323,14 @@ module.exports = function(){ Step( function getAffectedTablesAndLastUpdatedTime() { - queryTablesApi.getAffectedTablesAndLastUpdatedTime(usr, key, sql, this); + queryTablesApi.getAffectedTablesAndLastUpdatedTime(usr, { + user: req.params.dbuser, + pass: req.params.dbpass, + host: req.params.dbhost, + port: req.params.dbport, + dbname: req.params.dbname, + api_key: key + }, sql, this); }, function handleAffectedTablesAndLastUpdatedTime(err, result) { if (req.profiler) req.profiler.done('queryTablesAndLastUpdated'); diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index a331beb6..42e552a6 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -937,6 +937,29 @@ "version": "0.5.0", "from": "git://github.com/CartoDB/node-cartodb-redis.git#0.5.0" }, + "cartodb-psql": { + "version": "0.2.0", + "from": "git://github.com/CartoDB/node-cartodb-psql.git#0.2.0", + "dependencies": { + "pg": { + "version": "2.6.2", + "dependencies": { + "generic-pool": { + "version": "2.0.3" + }, + "buffer-writer": { + "version": "1.0.0" + } + } + }, + "step": { + "version": "0.0.5" + }, + "underscore": { + "version": "1.6.0" + } + } + }, "redis-mpool": { "version": "0.0.4", "from": "http://github.com/CartoDB/node-redis-mpool/tarball/0.0.4", diff --git a/package.json b/package.json index 74daefb7..58f98aa8 100644 --- a/package.json +++ b/package.json @@ -28,6 +28,7 @@ "step": "0.0.x", "request": "2.9.202", "cartodb-redis": "git://github.com/CartoDB/node-cartodb-redis.git#0.5.0", + "cartodb-psql": "git://github.com/CartoDB/node-cartodb-psql.git#0.2.0", "redis-mpool": "http://github.com/CartoDB/node-redis-mpool/tarball/0.0.4", "mapnik": "http://github.com/Vizzuality/node-mapnik/tarball/0.7.26-cdb1", "lzma": "~1.2.3", diff --git a/test/acceptance/multilayer.js b/test/acceptance/multilayer.js index fd95edd7..f685dad6 100644 --- a/test/acceptance/multilayer.js +++ b/test/acceptance/multilayer.js @@ -23,7 +23,11 @@ serverOptions = ServerOptions(); var server = new CartodbWindshaft(serverOptions); server.setMaxListeners(0); -suite('multilayer', function() { +[true, false].forEach(function(cdbQueryTablesFromPostgresEnabledValue) { + +global.environment.enabledFeatures = {cdbQueryTablesFromPostgres: cdbQueryTablesFromPostgresEnabledValue}; + +suite('multilayer:postgres=' + cdbQueryTablesFromPostgresEnabledValue, function() { var redis_client = redis.createClient(global.environment.redis.port); var sqlapi_server; @@ -1342,3 +1346,4 @@ suite('multilayer', function() { }); +}); diff --git a/test/acceptance/server.js b/test/acceptance/server.js index 0a430dd1..e98a68d4 100644 --- a/test/acceptance/server.js +++ b/test/acceptance/server.js @@ -19,7 +19,12 @@ var serverOptions = require(__dirname + '/../../lib/cartodb/server_options')(); var server = new CartodbWindshaft(serverOptions); server.setMaxListeners(0); -suite('server', function() { +[true, false].forEach(function(cdbQueryTablesFromPostgresEnabledValue) { + +global.environment.enabledFeatures = {cdbQueryTablesFromPostgres: cdbQueryTablesFromPostgresEnabledValue}; + +suite('multilayer:postgres=' + cdbQueryTablesFromPostgresEnabledValue, function() { + var redis_client = redis.createClient(global.environment.redis.port); var sqlapi_server; @@ -1389,3 +1394,4 @@ suite('server', function() { }); +}); diff --git a/test/acceptance/templates.js b/test/acceptance/templates.js index 085afad1..f169a914 100644 --- a/test/acceptance/templates.js +++ b/test/acceptance/templates.js @@ -25,7 +25,11 @@ var serverOptions = ServerOptions(); var server = new CartodbWindshaft(serverOptions); server.setMaxListeners(0); -suite('template_api', function() { +[true, false].forEach(function(cdbQueryTablesFromPostgresEnabledValue) { + +global.environment.enabledFeatures = {cdbQueryTablesFromPostgres: cdbQueryTablesFromPostgresEnabledValue}; + +suite('template_api:postgres=' + cdbQueryTablesFromPostgresEnabledValue, function() { var redis_client = redis.createClient(global.environment.redis.port); var sqlapi_server; @@ -1948,3 +1952,4 @@ suite('template_api', function() { }); +}); From 8255f3eb5177cee103591dd5161d303f15e97a0e Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Mon, 11 Aug 2014 20:18:27 +0200 Subject: [PATCH 3/5] Upgrades cartodb-psql dependency --- npm-shrinkwrap.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 23b18559..84d5c5aa 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -938,8 +938,8 @@ "from": "git://github.com/CartoDB/node-cartodb-redis.git#0.5.0" }, "cartodb-psql": { - "version": "0.2.0", - "from": "git://github.com/CartoDB/node-cartodb-psql.git#0.2.0", + "version": "0.3.1", + "from": "git://github.com/CartoDB/node-cartodb-psql.git#0.3.1", "dependencies": { "pg": { "version": "2.6.2", diff --git a/package.json b/package.json index b8a0f7c1..645ae643 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,7 @@ "step": "0.0.x", "request": "2.9.202", "cartodb-redis": "git://github.com/CartoDB/node-cartodb-redis.git#0.5.0", - "cartodb-psql": "git://github.com/CartoDB/node-cartodb-psql.git#0.2.0", + "cartodb-psql": "git://github.com/CartoDB/node-cartodb-psql.git#0.3.1", "redis-mpool": "http://github.com/CartoDB/node-redis-mpool/tarball/0.0.4", "mapnik": "http://github.com/Vizzuality/node-mapnik/tarball/0.7.26-cdb1", "lzma": "~1.2.3", From 49b0120c6d99baf21f0b572b154c72d0df1d2ceb Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Thu, 14 Aug 2014 13:32:04 +0200 Subject: [PATCH 4/5] Formats NEWS --- NEWS.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index af75e5df..102f7b21 100644 --- a/NEWS.md +++ b/NEWS.md @@ -18,8 +18,10 @@ Enhancements: - SQL API requests moved to its own entity New features: - - Affected tables and last updated time for a query are performed in a single request to the SQL API - - Allow specifying the tile format, upgrades windshaft and grainstore dependencies for this matter + - Affected tables and last updated time for a query are performed in a single + request to the SQL API + - Allow specifying the tile format, upgrades windshaft and grainstore + dependencies for this matter 1.13.1 -- 2014-08-04 From 4555b2107ae0ecbb2a9e61f04391807fd426c837 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Thu, 14 Aug 2014 13:32:22 +0200 Subject: [PATCH 5/5] Updates news --- NEWS.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS.md b/NEWS.md index 102f7b21..41568228 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,10 @@ 1.15.1 -- 2014-mm-dd -------------------- +New features: + - Configurable QueryTablesAPI to call directly postgresql using cartodb-psql + or to keep using a request to the SQL API + 1.15.0 -- 2014-08-13 -------------------- Enhancements: