diff --git a/.travis.yml b/.travis.yml index 8492e564..db5cd52f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,6 +13,7 @@ addons: before_install: - npm install -g npm@2 - createdb template_postgis + - createuser publicuser - psql -c "CREATE EXTENSION postgis" template_postgis env: diff --git a/lib/cartodb/api/query_tables_api.js b/lib/cartodb/api/query_tables_api.js index af2fc978..a94d6411 100644 --- a/lib/cartodb/api/query_tables_api.js +++ b/lib/cartodb/api/query_tables_api.js @@ -29,32 +29,26 @@ QueryTablesApi.prototype.getAffectedTablesInQuery = function (username, sql, cal }; QueryTablesApi.prototype.getAffectedTablesAndLastUpdatedTime = function (username, sql, callback) { - var query = [ - 'WITH querytables AS (', - 'SELECT * FROM CDB_QueryTablesText($windshaft$' + prepareSql(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 ((SELECT tablenames from querytables)::regclass[])' - ].join(' '); + var query = + 'SELECT * FROM CDB_QueryTables_Updated_At($windshaft$' + prepareSql(sql) + '$windshaft$)'; this.pgQueryRunner.run(username, query, function handleAffectedTablesAndLastUpdatedTimeRows (err, rows) { - if (err || rows.length === 0) { + if (err) { var msg = err.message ? err.message : err; callback(new Error('could not fetch affected tables or last updated time: ' + msg)); return; } - var result = rows[0]; + var affectedTables = rows; - // This is an Array, so no need to split into parts - var tableNames = result.tablenames; - - var lastUpdatedTime = result.max || 0; + var updatedTimes = affectedTables.map(function getUpdateDate(table) { + return table.updated_at; + }); + var lastUpdatedTime = (affectedTables.length === 0 ? 0 : Math.max.apply(null, updatedTimes)) || 0; callback(null, { - affectedTables: tableNames, - lastUpdatedTime: lastUpdatedTime * 1000 + affectedTables: affectedTables, + lastUpdatedTime: lastUpdatedTime }); }); }; diff --git a/lib/cartodb/api/tables_extent_api.js b/lib/cartodb/api/tables_extent_api.js index d4293ed7..7b534a3f 100644 --- a/lib/cartodb/api/tables_extent_api.js +++ b/lib/cartodb/api/tables_extent_api.js @@ -13,13 +13,9 @@ module.exports = TablesExtentApi; * `table_name` format as valid input * @param {Function} callback function(err, result) {Object} result with `west`, `south`, `east`, `north` */ -TablesExtentApi.prototype.getBounds = function (username, tableNames, callback) { - var estimatedExtentSQLs = tableNames.map(function(tableName) { - var schemaTable = tableName.split('.'); - if (schemaTable.length > 1) { - return "ST_EstimatedExtent('" + schemaTable[0] + "', '" + schemaTable[1] + "', 'the_geom_webmercator')"; - } - return "ST_EstimatedExtent('" + schemaTable[0] + "', 'the_geom_webmercator')"; +TablesExtentApi.prototype.getBounds = function (username, tables, callback) { + var estimatedExtentSQLs = tables.map(function(table) { + return "ST_EstimatedExtent('" + table.schema_name + "', '" + table.table_name + "', 'the_geom_webmercator')"; }); var query = [ diff --git a/lib/cartodb/cache/model/database_tables_entry.js b/lib/cartodb/cache/model/database_tables_entry.js index 4d269137..60629714 100644 --- a/lib/cartodb/cache/model/database_tables_entry.js +++ b/lib/cartodb/cache/model/database_tables_entry.js @@ -1,22 +1,24 @@ var crypto = require('crypto'); -function DatabaseTables(dbName, tableNames) { +function DatabaseTables(tables) { this.namespace = 't'; - this.dbName = dbName; - this.tableNames = tableNames; + this.tables = tables; } module.exports = DatabaseTables; DatabaseTables.prototype.key = function() { - return this.tableNames.map(function(tableName) { - return this.namespace + ':' + shortHashKey(this.dbName + ':' + tableName); + return this.tables.map(function(table) { + return this.namespace + ':' + shortHashKey(table.dbname + ':' + table.table_name + '.' + table.schema_name); }.bind(this)); }; DatabaseTables.prototype.getCacheChannel = function() { - return this.dbName + ':' + this.tableNames.join(','); + var key = this.tables.map(function(table) { + return table.dbname + ':' + table.schema_name + "." + table.table_name; + }).join(";;"); + return key; }; function shortHashKey(target) { diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 0a7e7698..e647a046 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -320,7 +320,7 @@ LayergroupController.prototype.sendResponse = function(req, res, body, status, h global.logger.warn('ERROR generating cache channel: ' + err); } if (!!affectedTables) { - var tablesCacheEntry = new TablesCacheEntry(dbName, affectedTables); + var tablesCacheEntry = new TablesCacheEntry(affectedTables); res.set('X-Cache-Channel', tablesCacheEntry.getCacheChannel()); self.surrogateKeysCache.tag(res, tablesCacheEntry); } @@ -366,17 +366,20 @@ LayergroupController.prototype.getAffectedTables = function(user, dbName, layerg throw new Error("this request doesn't need an X-Cache-Channel generated"); } - self.queryTablesApi.getAffectedTablesInQuery(user, sql, this); // in addCacheChannel + self.queryTablesApi.getAffectedTablesAndLastUpdatedTime(user, sql, this); // in addCacheChannel }, - function buildCacheChannel(err, tableNames) { + function buildCacheChannel(err, tables) { assert.ifError(err); + self.layergroupAffectedTables.set(dbName, layergroupId, tables.affectedTables); - self.layergroupAffectedTables.set(dbName, layergroupId, tableNames); - - return tableNames; + return tables; }, - function finish(err, affectedTables) { - callback(err, affectedTables); + function finish(err, tables) { + if(tables === undefined){ + callback(err); + }else{ + callback(err, tables.affectedTables); + } } ); }; diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index a09b9572..1d05a120 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -323,20 +323,20 @@ MapController.prototype.afterLayergroupCreate = function(req, res, mapconfig, la function checkCachedAffectedTables() { return self.layergroupAffectedTables.hasAffectedTables(dbName, layergroupId); }, - function getAffectedTablesAndLastUpdatedTime(err, hasCache) { + function getAffectedTablesAndLastUpdatedTime(err) { assert.ifError(err); - if (hasCache) { - var next = this; - var affectedTables = self.layergroupAffectedTables.get(dbName, layergroupId); - self.queryTablesApi.getLastUpdatedTime(username, affectedTables, function(err, lastUpdatedTime) { - if (err) { - return next(err); - } - return next(null, { affectedTables: affectedTables, lastUpdatedTime: lastUpdatedTime }); - }); - } else { + // if (hasCache) { + // var next = this; + // var affectedTables = self.layergroupAffectedTables.get(dbName, layergroupId); + // self.queryTablesApi.getLastUpdatedTime(username, affectedTables, function(err, lastUpdatedTime) { + // if (err) { + // return next(err); + // } + // return next(null, { affectedTables: affectedTables, lastUpdatedTime: lastUpdatedTime }); + // }); + // } else { self.queryTablesApi.getAffectedTablesAndLastUpdatedTime(username, sql, this); - } + //} }, function handleAffectedTablesAndLastUpdatedTime(err, result) { if (req.profiler) { @@ -353,7 +353,7 @@ MapController.prototype.afterLayergroupCreate = function(req, res, mapconfig, la addWidgetsUrl(username, layergroup); if (req.method === 'GET') { - var tableCacheEntry = new TablesCacheEntry(dbName, result.affectedTables); + var tableCacheEntry = new TablesCacheEntry(result.affectedTables); var ttl = global.environment.varnish.layergroupTtl || 86400; res.set('Cache-Control', 'public,max-age='+ttl+',must-revalidate'); res.set('Last-Modified', (new Date()).toUTCString()); diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 004282c9..3687dc39 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -44,7 +44,6 @@ NamedMapsController.prototype.sendResponse = function(req, res, resource, header var self = this; - var dbName = req.params.dbname; step( function getAffectedTablesAndLastUpdatedTime() { namedMapProvider.getAffectedTablesAndLastUpdatedTime(this); @@ -66,7 +65,7 @@ NamedMapsController.prototype.sendResponse = function(req, res, resource, header } res.set('Last-Modified', lastModifiedDate.toUTCString()); - var tablesCacheEntry = new TablesCacheEntry(dbName, result.affectedTables); + var tablesCacheEntry = new TablesCacheEntry(result.affectedTables); res.set('X-Cache-Channel', tablesCacheEntry.getCacheChannel()); if (result.affectedTables.length > 0) { self.surrogateKeysCache.tag(res, tablesCacheEntry); diff --git a/test/acceptance/multilayer.js b/test/acceptance/multilayer.js index ededd1b7..d4e06e52 100644 --- a/test/acceptance/multilayer.js +++ b/test/acceptance/multilayer.js @@ -262,9 +262,9 @@ describe(suiteName, function() { var parsedBody = JSON.parse(res.body); expected_token = parsedBody.layergroupid.split(':')[0]; helper.checkCache(res); - helper.checkSurrogateKey(res, new TablesCacheEntry('test_windshaft_cartodb_user_1_db', [ - 'public.test_table', - 'public.test_table_2' + helper.checkSurrogateKey(res, new TablesCacheEntry([ + {dbname: "test_windshaft_cartodb_user_1_db", table_name: "test_table", schema_name: "public"}, + {dbname: "test_windshaft_cartodb_user_1_db", table_name: "test_table_2", schema_name: "public"}, ]).key().join(' ')); diff --git a/test/acceptance/templates.js b/test/acceptance/templates.js index d4eb304b..137f5ce5 100644 --- a/test/acceptance/templates.js +++ b/test/acceptance/templates.js @@ -1393,7 +1393,8 @@ describe('template_api', function() { // See https://github.com/CartoDB/Windshaft-cartodb/issues/176 helper.checkCache(res); var expectedSurrogateKey = [ - new TablesCacheEntry('test_windshaft_cartodb_user_1_db', ['public.test_table_private_1']).key(), + new TablesCacheEntry([{dbname: 'test_windshaft_cartodb_user_1_db', schema_name: 'public', + table_name: 'test_table_private_1'}]).key(), new NamedMapsCacheEntry('localhost', template_acceptance_open.name).key() ].join(' '); helper.checkSurrogateKey(res, expectedSurrogateKey); @@ -1476,7 +1477,8 @@ describe('template_api', function() { // See https://github.com/CartoDB/Windshaft-cartodb/issues/176 helper.checkCache(res); var expectedSurrogateKey = [ - new TablesCacheEntry('test_windshaft_cartodb_user_1_db', ['public.test_table_private_1']).key(), + new TablesCacheEntry([{dbname: 'test_windshaft_cartodb_user_1_db', schema_name: 'public', + table_name: 'test_table_private_1'}]).key(), new NamedMapsCacheEntry('localhost', template_acceptance_open.name).key() ].join(' '); helper.checkSurrogateKey(res, expectedSurrogateKey); diff --git a/test/integration/query-tables-api.js b/test/integration/query-tables-api.js index d05b1ed0..93ca0a07 100644 --- a/test/integration/query-tables-api.js +++ b/test/integration/query-tables-api.js @@ -28,9 +28,9 @@ describe('QueryTablesApi', function() { var query = 'select * from test_table'; queryTablesApi.getAffectedTablesAndLastUpdatedTime('localhost', query, function(err, result) { assert.ok(!err, err); - assert.deepEqual(result, { - affectedTables: [ 'public.test_table' ], + affectedTables: [{dbname: "test_windshaft_cartodb_user_1_db", schema_name: "public", + "table_name": 'test_table', updated_at: new Date(1234567890123)}], lastUpdatedTime: 1234567890123 }); @@ -44,7 +44,8 @@ describe('QueryTablesApi', function() { assert.ok(!err, err); assert.deepEqual(result, { - affectedTables: [ 'public.test_table_private_1' ], + affectedTables: [{dbname: "test_windshaft_cartodb_user_1_db", schema_name: "public", + "table_name": 'test_table_private_1', updated_at: new Date(1234567890123)}], lastUpdatedTime: 1234567890123 }); diff --git a/test/support/prepare_db.sh b/test/support/prepare_db.sh index b4b6a1a5..10b34830 100755 --- a/test/support/prepare_db.sh +++ b/test/support/prepare_db.sh @@ -79,10 +79,12 @@ if test x"$PREPARE_PGSQL" = xyes; then psql -v ON_ERROR_STOP=1 ${TEST_DB} || exit 1 psql -c "CREATE LANGUAGE plpythonu;" ${TEST_DB} - curl -L -s https://github.com/CartoDB/cartodb-postgresql/raw/cdb/scripts-available/CDB_QueryStatements.sql -o sql/CDB_QueryStatements.sql - curl -L -s https://github.com/CartoDB/cartodb-postgresql/raw/cdb/scripts-available/CDB_QueryTables.sql -o sql/CDB_QueryTables.sql - cat sql/CDB_QueryStatements.sql sql/CDB_QueryTables.sql sql/CDB_Overviews.sql | - psql -v ON_ERROR_STOP=1 ${TEST_DB} || exit 1 + for i in CDB_QueryStatements CDB_QueryTables CDB_CartodbfyTable CDB_TableMetadata CDB_ForeignTable CDB_UserTables CDB_ColumnNames CDB_ZoomFromScale CDB_Overviews + do + curl -L -s https://github.com/CartoDB/cartodb-postgresql/raw/master/scripts-available/$i.sql -o sql/$i.sql + cat sql/$i.sql | sed -e 's/cartodb\./public./g' -e "s/''cartodb''/''public''/g" \ + | psql -v ON_ERROR_STOP=1 ${TEST_DB} || exit 1 + done fi diff --git a/test/support/sql/CDB_Overviews.sql b/test/support/sql/CDB_Overviews.sql deleted file mode 100644 index 654bd9d2..00000000 --- a/test/support/sql/CDB_Overviews.sql +++ /dev/null @@ -1,46 +0,0 @@ --- Mockup for CDB_Overviews -CREATE OR REPLACE FUNCTION CDB_Overviews(table_names regclass[]) -RETURNS TABLE(base_table regclass, z integer, overview_table regclass) -AS $$ - BEGIN - IF (SELECT 'test_table_overviews'::regclass = ANY (table_names)) THEN - RETURN QUERY - SELECT 'test_table_overviews'::regclass AS base_table, 1 AS z, '_vovw_1_test_table_overviews'::regclass AS overview_table - UNION ALL - SELECT 'test_table_overviews'::regclass AS base_table, 2 AS z, '_vovw_2_test_table_overviews'::regclass AS overview_table; - ELSE - RETURN; - END IF; - END -$$ LANGUAGE PLPGSQL; - -CREATE OR REPLACE FUNCTION CDB_ZoomFromScale(scaleDenominator numeric) RETURNS int AS $$ -BEGIN - CASE - WHEN scaleDenominator > 500000000 THEN RETURN 0; - WHEN scaleDenominator <= 500000000 AND scaleDenominator > 200000000 THEN RETURN 1; - WHEN scaleDenominator <= 200000000 AND scaleDenominator > 100000000 THEN RETURN 2; - WHEN scaleDenominator <= 100000000 AND scaleDenominator > 50000000 THEN RETURN 3; - WHEN scaleDenominator <= 50000000 AND scaleDenominator > 25000000 THEN RETURN 4; - WHEN scaleDenominator <= 25000000 AND scaleDenominator > 12500000 THEN RETURN 5; - WHEN scaleDenominator <= 12500000 AND scaleDenominator > 6500000 THEN RETURN 6; - WHEN scaleDenominator <= 6500000 AND scaleDenominator > 3000000 THEN RETURN 7; - WHEN scaleDenominator <= 3000000 AND scaleDenominator > 1500000 THEN RETURN 8; - WHEN scaleDenominator <= 1500000 AND scaleDenominator > 750000 THEN RETURN 9; - WHEN scaleDenominator <= 750000 AND scaleDenominator > 400000 THEN RETURN 10; - WHEN scaleDenominator <= 400000 AND scaleDenominator > 200000 THEN RETURN 11; - WHEN scaleDenominator <= 200000 AND scaleDenominator > 100000 THEN RETURN 12; - WHEN scaleDenominator <= 100000 AND scaleDenominator > 50000 THEN RETURN 13; - WHEN scaleDenominator <= 50000 AND scaleDenominator > 25000 THEN RETURN 14; - WHEN scaleDenominator <= 25000 AND scaleDenominator > 12500 THEN RETURN 15; - WHEN scaleDenominator <= 12500 AND scaleDenominator > 5000 THEN RETURN 16; - WHEN scaleDenominator <= 5000 AND scaleDenominator > 2500 THEN RETURN 17; - WHEN scaleDenominator <= 2500 AND scaleDenominator > 1500 THEN RETURN 18; - WHEN scaleDenominator <= 1500 AND scaleDenominator > 750 THEN RETURN 19; - WHEN scaleDenominator <= 750 AND scaleDenominator > 500 THEN RETURN 20; - WHEN scaleDenominator <= 500 AND scaleDenominator > 250 THEN RETURN 21; - WHEN scaleDenominator <= 250 AND scaleDenominator > 100 THEN RETURN 22; - WHEN scaleDenominator <= 100 THEN RETURN 23; - END CASE; -END -$$ LANGUAGE plpgsql IMMUTABLE; diff --git a/test/support/sql/CDB_QueryStatements.sql b/test/support/sql/CDB_QueryStatements.sql deleted file mode 100644 index bb4d10cf..00000000 --- a/test/support/sql/CDB_QueryStatements.sql +++ /dev/null @@ -1,14 +0,0 @@ --- Return an array of statements found in the given query text --- --- Regexp curtesy of Hubert Lubaczewski (depesz) --- Implemented in plpython for performance reasons --- -CREATE OR REPLACE FUNCTION CDB_QueryStatements(query text) -RETURNS SETOF TEXT AS $$ - import re - pat = re.compile( r'''((?:[^'"$;]+|"[^"]*"|'[^']*'|(\$[^$]*\$).*?\2)+)''', re.DOTALL ) - for match in pat.findall(query): - cleaned = match[0].strip() - if ( cleaned ): - yield cleaned -$$ language 'plpythonu' IMMUTABLE STRICT; diff --git a/test/support/sql/CDB_QueryTables.sql b/test/support/sql/CDB_QueryTables.sql deleted file mode 100644 index c7cfa64b..00000000 --- a/test/support/sql/CDB_QueryTables.sql +++ /dev/null @@ -1,78 +0,0 @@ --- Return an array of table names scanned by a given query --- --- Requires PostgreSQL 9.x+ --- -CREATE OR REPLACE FUNCTION CDB_QueryTablesText(query text) -RETURNS text[] -AS $$ -DECLARE - exp XML; - tables text[]; - rec RECORD; - rec2 RECORD; -BEGIN - - tables := '{}'; - - FOR rec IN SELECT CDB_QueryStatements(query) q LOOP - - IF NOT ( rec.q ilike 'select%' or rec.q ilike 'with%' ) THEN - --RAISE WARNING 'Skipping %', rec.q; - CONTINUE; - END IF; - - BEGIN - EXECUTE 'EXPLAIN (FORMAT XML, VERBOSE) ' || rec.q INTO STRICT exp; - EXCEPTION WHEN others THEN - -- TODO: if error is 'relation "xxxxxx" does not exist', take xxxxxx as - -- the affected table ? - RAISE WARNING 'CDB_QueryTables cannot explain query: % (%: %)', rec.q, SQLSTATE, SQLERRM; - RAISE EXCEPTION '%', SQLERRM; - CONTINUE; - END; - - -- Now need to extract all values of - - -- RAISE DEBUG 'Explain: %', exp; - - FOR rec2 IN WITH - inp AS ( - SELECT - xpath('//x:Relation-Name/text()', exp, ARRAY[ARRAY['x', 'http://www.postgresql.org/2009/explain']]) as x, - xpath('//x:Relation-Name/../x:Schema/text()', exp, ARRAY[ARRAY['x', 'http://www.postgresql.org/2009/explain']]) as s - ) - SELECT unnest(x)::text as p, unnest(s)::text as sc from inp - LOOP - -- RAISE DEBUG 'tab: %', rec2.p; - -- RAISE DEBUG 'sc: %', rec2.sc; - tables := array_append(tables, format('%s.%s', quote_ident(rec2.sc), quote_ident(rec2.p))); - END LOOP; - - -- RAISE DEBUG 'Tables: %', tables; - - END LOOP; - - -- RAISE DEBUG 'Tables: %', tables; - - -- Remove duplicates and sort by name - IF array_upper(tables, 1) > 0 THEN - WITH dist as ( SELECT DISTINCT unnest(tables)::text as p ORDER BY p ) - SELECT array_agg(p) from dist into tables; - END IF; - - --RAISE DEBUG 'Tables: %', tables; - - return tables; -END -$$ LANGUAGE 'plpgsql' VOLATILE STRICT; - - --- Keep CDB_QueryTables with same signature for backwards compatibility. --- It should probably be removed in the future. -CREATE OR REPLACE FUNCTION CDB_QueryTables(query text) -RETURNS name[] -AS $$ -BEGIN - RETURN CDB_QueryTablesText(query)::name[]; -END -$$ LANGUAGE 'plpgsql' VOLATILE STRICT;