diff --git a/.travis.yml b/.travis.yml index 45a2ba3c..6d6f2893 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,13 +1,19 @@ -before_install: - #- sudo apt-add-repository --yes ppa:ubuntugis/ppa - - sudo apt-get update -q - # Removal of postgresql-9.1-postgis-scripts is needed due to a - # bug in some upstream package. - # See http://trac.osgeo.org/ubuntugis/ticket/39 - - sudo apt-get remove --purge -q postgresql-9.1-postgis-scripts - - sudo apt-get install -q postgresql-9.1-postgis gdal-bin - - createdb template_postgis - - psql -c "CREATE EXTENSION postgis" template_postgis +before_script: + - lsb_release -a + - sudo mv /etc/apt/sources.list.d/pgdg-source.list* /tmp + - sudo apt-get -qq purge postgis* postgresql* + - sudo rm -Rf /var/lib/postgresql /etc/postgresql + - sudo apt-add-repository --yes ppa:cartodb/postgresql-9.3 + - sudo apt-add-repository --yes ppa:cartodb/gis + - sudo apt-get update + - sudo apt-get install -q postgresql-9.3-postgis-2.1 + - sudo apt-get install -q postgresql-contrib-9.3 + - sudo apt-get install -q postgis + - sudo apt-get install -q gdal-bin + - echo -e "local\tall\tall\ttrust\nhost\tall\tall\t127.0.0.1/32\ttrust\nhost\tall\tall\t::1/128\ttrust" |sudo tee /etc/postgresql/9.3/main/pg_hba.conf + - sudo service postgresql restart + - psql -c 'create database template_postgis;' -U postgres + - psql -c 'CREATE EXTENSION postgis;' -U postgres -d template_postgis - ./configure language: node_js diff --git a/HOWTO_RELEASE b/HOWTO_RELEASE index 99eebf60..1d65b876 100644 --- a/HOWTO_RELEASE +++ b/HOWTO_RELEASE @@ -1,11 +1,20 @@ -1. Ensure proper version in package.json -2. Ensure NEWS section exists for the new version, review it, add release date -3. Drop npm-shrinkwrap.json -4. Run npm install -5. Test (make check or npm test), fix if broken before proceeding -6. Run npm shrinkwrap -7. Commit package.json, npm-shrinwrap.json, NEWS -8. Tag Major.Minor.Patch -9. Announce -10. Stub NEWS/package for next version +1. Test (make clean all check), fix if broken before proceeding +2. Ensure proper version in package.json +3. Ensure NEWS section exists for the new version, review it, add release date +4. Drop npm-shrinkwrap.json +5. Run npm shrinkwrap to recreate npm-shrinkwrap.json +6. Commit package.json, npm-shrinwrap.json, NEWS +7. git tag -a Major.Minor.Patch # use NEWS section as content +8. Announce on cartodb@googlegroups.com +9. Stub NEWS/package for next version +Versions: + +Bugfix releases increment Patch component of version. +Feature releases increment Minor and set Patch to zero. +If backward compatibility is broken, increment Major and +set to zero Minor and Patch. + +Branches named 'b.' are kept for any critical +fix that might need to be shipped before next feature release +is ready. diff --git a/NEWS.md b/NEWS.md index 4875036a..314097b9 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,27 @@ +1.10.1 - 2014-mm-dd +------------------- +Bug fixes: + + * Backing out Stream JSON responses + +1.10.0 - 2014-06-04 +------------------- + +New features: + + * Order by and sort order through http query params + * Cancelling queries in Postgresql when HTTP request is aborted/closed + +Enhancements: + + * Stream JSON responses + * Pre-compiling may write regex + * Set default PostgreSQL application name to "cartodb_sqlapi" + +Bug fixes: + + * Support trailing semicolons (#147) + 1.9.1 - 2014-03-27 ------------------ diff --git a/app/controllers/app.js b/app/controllers/app.js index 6fe84a93..86502144 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -15,6 +15,9 @@ // // +if ( ! process.env['PGAPPNAME'] ) + process.env['PGAPPNAME']='cartodb_sqlapi'; + function App() { var path = require('path'); @@ -27,7 +30,6 @@ var express = require('express') , os = require('os') , zlib = require('zlib') , util = require('util') - , spawn = require('child_process').spawn , Profiler = require('step-profiler') , StatsD = require('node-statsd').StatsD , Meta = require('cartodb-redis')({ @@ -37,6 +39,7 @@ var express = require('express') // global.settings.app_root + '/app/models/metadata') , oAuth = require(global.settings.app_root + '/app/models/oauth') , PSQL = require(global.settings.app_root + '/app/models/psql') + , PSQLWrapper = require(global.settings.app_root + '/app/sql/psql_wrapper') , CdbRequest = require(global.settings.app_root + '/app/models/cartodb_request') , ApiKeyAuth = require(global.settings.app_root + '/app/models/apikey_auth') , _ = require('underscore') @@ -71,7 +74,7 @@ Date.prototype.toJSON = function() { s += ( offset < 0 ? '+' : '-' ) + pad(Math.abs(offset / 60)) + pad(Math.abs(offset % 60)) - + } return s; } @@ -165,19 +168,16 @@ app.get(global.settings.base_url+'/version', function(req, res) { res.send(getVersion()); }); -// Return true of the given query may write to the database -// -// NOTE: this is a fuzzy check, the return could be true even -// if the query doesn't really write anything. -// But you can be pretty sure of a false return. -// +var sqlQueryMayWriteRegex = new RegExp("\\b(alter|insert|update|delete|create|drop|reindex|truncate)\\b", "i"); +/** + * This is a fuzzy check, the return could be true even if the query doesn't really write anything. But you can be + * pretty sure of a false return. + * + * @param sql The SQL statement to check against + * @returns {boolean} Return true of the given query may write to the database + */ function queryMayWrite(sql) { - var mayWrite = false; - var pattern = RegExp("\\b(alter|insert|update|delete|create|drop|reindex|truncate)\\b", "i"); - if ( pattern.test(sql) ) { - mayWrite = true; - } - return mayWrite; + return sqlQueryMayWriteRegex.test(sql); } function sanitize_filename(filename) { @@ -198,6 +198,8 @@ function handleQuery(req, res) { var database = params.database; // TODO: Deprecate var limit = parseInt(params.rows_per_page); var offset = parseInt(params.page); + var orderBy = params.order_by; + var sortOrder = params.sort_order; var requestedFormat = params.format; var format = _.isArray(requestedFormat) ? _.last(requestedFormat) : requestedFormat; var requestedFilename = params.filename; @@ -215,8 +217,10 @@ function handleQuery(req, res) { req.aborted = false; req.on("close", function() { - console.log("Request closed unexpectedly (aborted?)"); - req.aborted = true; // TODO: there must be a builtin way to check this + if (req.formatter && _.isFunction(req.formatter.cancel)) { + req.formatter.cancel(); + } + req.aborted = true; // TODO: there must be a builtin way to check this }); function checkAborted(step) { @@ -402,8 +406,9 @@ function handleQuery(req, res) { } - var fClass = formats[format] + var fClass = formats[format]; formatter = new fClass(); + req.formatter = formatter; // configure headers for given format @@ -450,7 +455,7 @@ function handleQuery(req, res) { checkAborted('generateFormat'); // TODO: drop this, fix UI! - sql = PSQL.window_sql(sql,limit,offset); + sql = new PSQLWrapper(sql).orderBy(orderBy, sortOrder).window(limit, offset).query(); var opts = { dbopts: dbopts, diff --git a/app/models/formats/pg.js b/app/models/formats/pg.js index 2002287e..fa2acdef 100644 --- a/app/models/formats/pg.js +++ b/app/models/formats/pg.js @@ -15,7 +15,7 @@ pg.prototype = { getFileExtension: function() { return this.id; - }, + } }; @@ -40,6 +40,8 @@ pg.prototype.handleNotice = function(msg, result) { }; pg.prototype.handleQueryEnd = function(result) { + this.queryCanceller = undefined; + if ( this.error ) { this.callback(this.error); return; @@ -111,7 +113,8 @@ pg.prototype.sendResponse = function(opts, callback) { this.start_time = Date.now(); this.client = new PSQL(opts.dbopts); - this.client.eventedQuery(sql, function(err, query) { + this.client.eventedQuery(sql, function(err, query, queryCanceller) { + that.queryCanceller = queryCanceller; if (err) { callback(err); return; @@ -127,4 +130,10 @@ pg.prototype.sendResponse = function(opts, callback) { }); }; +pg.prototype.cancel = function() { + if (this.queryCanceller) { + this.queryCanceller.call(); + } +}; + module.exports = pg; diff --git a/app/models/psql.js b/app/models/psql.js index 043a61da..d96ce567 100644 --- a/app/models/psql.js +++ b/app/models/psql.js @@ -1,6 +1,7 @@ -var _ = require('underscore') - , Step = require('step') - , pg = require('pg');//.native; // disabled for now due to: https://github.com/brianc/node-postgres/issues/48 +var _ = require('underscore'), + PSQLWrapper = require('../sql/psql_wrapper'), + Step = require('step'), + pg = require('pg');//.native; // disabled for now due to: https://github.com/brianc/node-postgres/issues/48 _.mixin(require('underscore.string')); // Max database connections in the pool @@ -181,6 +182,7 @@ var PSQL = function(dbopts) { that.connect(this); }, function(err, client, done){ + var next = this; if (err) throw err; var query = client.query(sql); @@ -197,10 +199,13 @@ var PSQL = function(dbopts) { client.removeListener('notice', noticeListener); done(); }); - return query; + next(null, query, client); }, - function(err, query){ - callback(err, query) + function(err, query, client){ + var queryCanceller = function() { + pg.cancel(undefined, client, query); + }; + callback(err, query, queryCanceller); } ); }; @@ -261,81 +266,19 @@ var PSQL = function(dbopts) { return me; }; -// little hack for UI -// -// TODO:drop, fix in the UI (it's not documented in doc/API) -// + +/** + * Little hack for UI + * TODO: drop, fix in the UI (it's not documented in doc/API) + * + * @param {string} sql + * @param {number} limit + * @param {number} offset + * @returns {string} The wrapped SQL query with the limit window + */ PSQL.window_sql = function(sql, limit, offset) { - // only window select functions (NOTE: "values" will be broken, "with" will be broken) - if (!_.isNumber(limit) || !_.isNumber(offset) ) return sql; - - // Strip comments - sql = sql.replace(/(^|\n)\s*--.*\n/g, ''); - - var cte = ''; - - if ( sql.match(/^\s*WITH\s/i) ) { - - var rem = sql; // analyzed portion of sql - var q; // quote char - var n = 0; // nested parens level - var s = 0; // 0:outQuote, 1:inQuote - var l; - while (1) { - l = rem.search(/[("')]/); -//console.log("REM Is " + rem); - if ( l < 0 ) { - console.log("Malformed SQL"); - return sql; - } - var f = rem.charAt(l); -//console.log("n:" + n + " s:" + s + " l:" + l + " charAt(l):" + f + " charAt(l+1):" + rem.charAt(l+1)); - if ( s == 0 ) { - if ( f == '(' ) ++n; - else if ( f == ')' ) { - if ( ! --n ) { // end of CTE - cte += rem.substr(0, l+1); - rem = rem.substr(l+1); - //console.log("Out of cte, rem is " + rem); - if ( rem.search(/^s*,/) < 0 ) break; - else continue; // cte and rem already updated - } - } - else { // enter quoting - s = 1; q = f; - } - } - else if ( f == q ) { - if ( rem.charAt(l+1) == f ) ++l; // escaped - else s = 0; // exit quoting - } - cte += rem.substr(0, l+1); - rem = rem.substr(l+1); - } -/* -console.log("cte: " + cte); -console.log("rem: " + rem); -*/ - sql = rem; //sql.substr(l+1); - } - - - var re_SELECT = RegExp(/^\s*SELECT\s/i); - var re_INTO = RegExp(/\sINTO\s+([^\s]+|"([^"]|"")*")\s*$/i); - - //console.log("SQL " + sql); - //console.log(" does " + ( sql.match(re_SELECT) ? '' : 'not ' ) + "match re_SELECT " + re_SELECT); - //console.log(" does " + ( sql.match(re_INTO) ? '' : 'not ' ) + "match re_INTO " + re_INTO); - - if ( - sql.match(re_SELECT) && - ! sql.match(re_INTO) - ) - { - return cte + "SELECT * FROM (" + sql + ") AS cdbq_1 LIMIT " + limit + " OFFSET " + offset; - } - - return cte + sql; -} + // keeping it here for backwards compatibility + return new PSQLWrapper(sql).window(limit, offset).query(); +}; module.exports = PSQL; diff --git a/app/sql/psql_wrapper.js b/app/sql/psql_wrapper.js new file mode 100644 index 00000000..f2691b5c --- /dev/null +++ b/app/sql/psql_wrapper.js @@ -0,0 +1,132 @@ +'use strict'; + +var _ = require('underscore'), + util = require('util'), + SORT_ORDER_OPTIONS = {ASC: 1, DESC: 1}, + REGEX_SELECT = /^\s*SELECT\s/i, + REGEX_INTO = /\sINTO\s+([^\s]+|"([^"]|"")*")\s*$/i; + +function PSQLWrapper(sql) { + this.sqlQuery = sql.replace(/;\s*$/, ''); + this.sqlClauses = { + orderBy: '', + limit: '' + }; +} + +/** + * Only window select functions (NOTE: "values" will be broken, "with" will be broken) + * + * @param {number} limit + * @param {number} offset + * @returns {PSQLWrapper} + */ +PSQLWrapper.prototype.window = function (limit, offset) { + if (!_.isNumber(limit) || !_.isNumber(offset)) { + return this; + } + this.sqlClauses.limit = util.format(' LIMIT %d OFFSET %d', limit, offset); + return this; +}; + +/** + * + * @param {string} column The name of the column to sort by + * @param {string} sortOrder Whether it's ASC or DESC ordering + * @returns {PSQLWrapper} + */ +PSQLWrapper.prototype.orderBy = function (column, sortOrder) { + if (!_.isString(column) || _.isEmpty(column)) { + return this; + } + this.sqlClauses.orderBy = util.format(' ORDER BY "%s"', column); + + if (!_.isUndefined(sortOrder)) { + sortOrder = sortOrder.toUpperCase(); + if (SORT_ORDER_OPTIONS[sortOrder]) { + this.sqlClauses.orderBy += util.format(' %s', sortOrder); + } + } + return this; +}; + +/** + * Builds an SQL query with extra clauses based on the builder calls. + * + * @returns {string} The SQL query with the extra clauses + */ +PSQLWrapper.prototype.query = function () { + if (_.isEmpty(this.sqlClauses.orderBy) && _.isEmpty(this.sqlClauses.limit)) { + return this.sqlQuery; + } + // Strip comments + this.sqlQuery = this.sqlQuery.replace(/(^|\n)\s*--.*\n/g, ''); + + var cte = ''; + + if (this.sqlQuery.match(/^\s*WITH\s/i)) { + + var rem = this.sqlQuery, // analyzed portion of sql + q, // quote char + n = 0, // nested parens level + s = 0, // 0:outQuote, 1:inQuote + l; + while (1) { + l = rem.search(/[("')]/); + // console.log("REM Is " + rem); + if (l < 0) { + // console.log("Malformed SQL"); + return this.sqlQuery; + } + var f = rem.charAt(l); + // console.log("n:" + n + " s:" + s + " l:" + l + " charAt(l):" + f + " charAt(l+1):" + rem.charAt(l+1)); + if (s == 0) { + if (f == '(') { + ++n; + } else if (f == ')') { + if (!--n) { // end of CTE + cte += rem.substr(0, l + 1); + rem = rem.substr(l + 1); + //console.log("Out of cte, rem is " + rem); + if (rem.search(/^s*,/) < 0) { + break; + } else { + continue; // cte and rem already updated + } + } + } else { // enter quoting + s = 1; + q = f; + } + } else if (f == q) { + if (rem.charAt(l + 1) == f) { + ++l; // escaped + } else { + s = 0; // exit quoting + } + } + cte += rem.substr(0, l + 1); + rem = rem.substr(l + 1); + } + /* + console.log("cte: " + cte); + console.log("rem: " + rem); + */ + this.sqlQuery = rem; //sql.substr(l+1); + } + + //console.log("SQL " + sql); + //console.log(" does " + ( sql.match(REGEX_SELECT) ? '' : 'not ' ) + "match REGEX_SELECT " + REGEX_SELECT); + //console.log(" does " + ( sql.match(REGEX_INTO) ? '' : 'not ' ) + "match REGEX_INTO " + REGEX_INTO); + + if (this.sqlQuery.match(REGEX_SELECT) && !this.sqlQuery.match(REGEX_INTO)) { + return util.format( + '%sSELECT * FROM (%s) AS cdbq_1%s%s', + cte, this.sqlQuery, this.sqlClauses.orderBy, this.sqlClauses.limit + ); + } + + return cte + this.sqlQuery; +}; + +module.exports = PSQLWrapper; diff --git a/doc/API.md b/doc/API.md index 7f1bba84..a808ebe0 100644 --- a/doc/API.md +++ b/doc/API.md @@ -43,6 +43,16 @@ Supported query string parameters: used to specify which page to start returning rows from. First page has index 0. + 'order_by': + Causes the result rows to be sorted according to the specified + case sensitive column name. See also 'sort_order'. + + 'sort_order': + Optional param combined with 'order_by' one. Values are limited + to ASC (ascending) and DESC (descending), case insensitive. If + not specified or wrongly specified, ASC is assumed by default. + + Response formats ---------------- diff --git a/doc/metrics.md b/doc/metrics.md new file mode 100644 index 00000000..5dfe7b9c --- /dev/null +++ b/doc/metrics.md @@ -0,0 +1,31 @@ +CartoDB-SQL-API metrics +======================= + +## Timers +- **sqlapi.query**: time to return a query resultset from the API, splitted into: + + **sqlapi.query.init**: time to prepare params from the request + + **sqlapi.query.getDatabaseName**: time to retrieve the database associated to the query + + **sqlapi.query.verifyRequest_apikey**: time to retrieve user and verify access with api key + + **sqlapi.query.verifyRequest_oauth**: time to retrieve user and verify access with oauht + + **sqlapi.query.getUserDBHost**: time to retrieve the host for the database + + **sqlapi.query.getUserDBPass**: time to retrieve the user password for the database connection + + **sqlapi.query.queryExplain**: time to retrieve affected tables from the query + + **sqlapi.query.setHeaders**: time to set the headers + + **sqlapi.query.sendResponse**: time to start sending the response. + + **sqlapi.query.finish**: time to handle an exception + + **sqlapi.query.startStreaming**: (json) time to start streaming, from the moment the query it was requested. + * It's not getting into graphite right now. + + **sqlapi.query.gotRows**: (json) Time until it finished processing all rows in the resultset. + * It's sharing the key with pg so stats in graphite can have mixed numbers. + + **sqlapi.query.endStreaming** (json) Time to finish the preparation of the response data. + * It's not getting into graphite right now. + + **sqlapi.query.generate**: (ogr) Time to prepare and generate a response from ogr + + **sqlapi.query.gotRows**: (pg) Time until it finished processing all rows in the resultset. + * It's sharing the key with json so stats in graphite can have mixed numbers. + + **sqlapi.query.packageResult**: (pg) Time to transform between different formats + * It's not getting into graphite right now. + + **sqlapi.query.eventedQuery**: (pg) Time to prepare and execute the query + +## Counters +- **sqlapi.query.success**: number of successful queries +- **sqlapi.query.error**: number of failed queries diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 027dd512..536102b0 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1,6 +1,6 @@ { "name": "cartodb_sql_api", - "version": "1.9.1", + "version": "1.10.0", "dependencies": { "underscore": { "version": "1.3.3" @@ -66,7 +66,7 @@ "version": "0.1.16", "dependencies": { "bindings": { - "version": "1.1.1" + "version": "1.2.0" } } } @@ -102,7 +102,7 @@ "version": "2.2.4" }, "log4js": { - "version": "0.6.12", + "version": "0.6.14", "dependencies": { "async": { "version": "0.1.15" @@ -111,20 +111,32 @@ "version": "1.1.4" }, "readable-stream": { - "version": "1.0.26-2", + "version": "1.0.27-1", "dependencies": { + "core-util-is": { + "version": "1.0.1" + }, + "isarray": { + "version": "0.0.1" + }, "string_decoder": { "version": "0.10.25-1" + }, + "inherits": { + "version": "2.0.1" } } } } }, "rollbar": { - "version": "0.3.1", + "version": "0.3.6", "dependencies": { "node-uuid": { "version": "1.4.1" + }, + "json-stringify-safe": { + "version": "5.0.0" } } }, @@ -170,7 +182,7 @@ "version": "1.0.7" }, "debug": { - "version": "0.7.4" + "version": "0.8.1" }, "mkdirp": { "version": "0.3.5" @@ -179,7 +191,7 @@ "version": "3.2.3", "dependencies": { "minimatch": { - "version": "0.2.12", + "version": "0.2.14", "dependencies": { "sigmund": { "version": "1.0.0" @@ -187,7 +199,7 @@ } }, "graceful-fs": { - "version": "2.0.1" + "version": "2.0.3" }, "inherits": { "version": "2.0.1" @@ -197,10 +209,10 @@ } }, "zipfile": { - "version": "0.5.0", + "version": "0.5.2", "dependencies": { "node-pre-gyp": { - "version": "0.5.5", + "version": "0.5.8", "dependencies": { "nopt": { "version": "2.2.0", @@ -318,7 +330,7 @@ "version": "0.1.25", "dependencies": { "graceful-fs": { - "version": "2.0.2" + "version": "2.0.3" } } } @@ -340,7 +352,7 @@ "version": "0.1.25", "dependencies": { "graceful-fs": { - "version": "2.0.2" + "version": "2.0.3" }, "inherits": { "version": "2.0.1" @@ -353,6 +365,9 @@ "minimatch": { "version": "0.2.14", "dependencies": { + "lru-cache": { + "version": "2.5.0" + }, "sigmund": { "version": "1.0.0" } @@ -364,10 +379,19 @@ } }, "readable-stream": { - "version": "1.0.26-2", + "version": "1.0.26-4", "dependencies": { + "core-util-is": { + "version": "1.0.1" + }, + "isarray": { + "version": "0.0.1" + }, "string_decoder": { "version": "0.10.25-1" + }, + "inherits": { + "version": "2.0.1" } } }, diff --git a/package.json b/package.json index 55637018..f545f932 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,7 @@ "keywords": [ "cartodb" ], - "version": "1.9.1", + "version": "1.10.1", "repository": { "type": "git", "url": "git://github.com/CartoDB/CartoDB-SQL-API.git" @@ -37,7 +37,7 @@ "libxmljs": "~0.8.1" }, "scripts": { - "test": "test/run_tests.sh ${RUNTESTFLAGS} test/unit/*.js test/unit/model/*.js test/acceptance/*.js test/acceptance/export/*.js" + "test": "test/run_tests.sh ${RUNTESTFLAGS} test/unit/*.js test/unit/model/*.js test/unit/sql/*.js test/acceptance/*.js test/acceptance/export/*.js" }, "engines": { "node": ">= 0.4.1 < 0.9" } } diff --git a/test/acceptance/export/kml.js b/test/acceptance/export/kml.js index 1d4c7f1f..2cc7cc22 100644 --- a/test/acceptance/export/kml.js +++ b/test/acceptance/export/kml.js @@ -271,8 +271,11 @@ test('GET /api/v1/sql as kml with no rows', function(done){ method: 'GET' },{ }, function(res){ assert.equal(res.statusCode, 200, res.body); - var body = 'cartodb_query'; - assert.equal(res.body.replace(/\n/g,''), body); + // NOTE: GDAL-1.11+ added 'id="root_doc"' attribute to the output + var pat = RegExp('^<\\?xml version="1.0" encoding="utf-8" \\?>cartodb_query$'); + var body = res.body.replace(/\n/g,''); + assert.ok(body.match(pat), + "Response:\n" + body + '\ndoes not match pattern:\n' + pat); done(); }); }); @@ -288,8 +291,11 @@ test('GET /api/v1/sql as kml with ending semicolon', function(done){ method: 'GET' },{ }, function(res){ assert.equal(res.statusCode, 200, res.body); - var body = 'cartodb_query'; - assert.equal(res.body.replace(/\n/g,''), body); + // NOTE: GDAL-1.11+ added 'id="root_doc"' attribute to the output + var pat = RegExp('^<\\?xml version="1.0" encoding="utf-8" \\?>cartodb_query$'); + var body = res.body.replace(/\n/g,''); + assert.ok(body.match(pat), + "Response:\n" + body + '\ndoes not match pattern:\n' + pat); done(); }); }); diff --git a/test/prepare_db.sh b/test/prepare_db.sh index 324bdc46..3c130510 100755 --- a/test/prepare_db.sh +++ b/test/prepare_db.sh @@ -65,18 +65,18 @@ export PGHOST PGPORT if test x"$PREPARE_PGSQL" = xyes; then echo "preparing postgres..." - dropdb ${TEST_DB} # 2> /dev/null # error expected if doesn't exist, but not otherwise - createdb -Ttemplate_postgis -EUTF8 ${TEST_DB} || die "Could not create test database" + dropdb -U postgres ${TEST_DB} # 2> /dev/null # error expected if doesn't exist, but not otherwise + createdb -U postgres -Ttemplate_postgis -EUTF8 ${TEST_DB} || die "Could not create test database" cat test.sql | sed "s/:PUBLICUSER/${PUBLICUSER}/" | sed "s/:PUBLICPASS/${PUBLICPASS}/" | sed "s/:TESTUSER/${TESTUSER}/" | sed "s/:TESTPASS/${TESTPASS}/" | - psql -v ON_ERROR_STOP=1 ${TEST_DB} || exit 1 + psql -U postgres -v ON_ERROR_STOP=1 ${TEST_DB} || exit 1 # TODO: send in a single run, togheter with test.sql - psql -f support/CDB_QueryStatements.sql ${TEST_DB} - psql -f support/CDB_QueryTables.sql ${TEST_DB} + psql -U postgres -f support/CDB_QueryStatements.sql ${TEST_DB} + psql -U postgres -f support/CDB_QueryTables.sql ${TEST_DB} fi diff --git a/test/run_tests.sh b/test/run_tests.sh index 5306b65e..a1a3ceab 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -2,6 +2,7 @@ # To make output dates deterministic export TZ='Europe/Rome' +export PGAPPNAME='cartodb_sqlapi_tester' OPT_CREATE_PGSQL=yes # create/prepare the postgresql test database OPT_CREATE_REDIS=yes # create/prepare the redis test databases diff --git a/test/unit/psql.test.js b/test/unit/psql.test.js index ce44a7a6..4cfd21e0 100644 --- a/test/unit/psql.test.js +++ b/test/unit/psql.test.js @@ -86,40 +86,6 @@ test('test public user cannot execute INSERT on db', function(done){ }); }); -test('Windowed SQL with simple select', function(){ - // NOTE: intentionally mixed-case and space-padded - var sql = "\n \tSEleCT * from table1"; - var out = PSQL.window_sql(sql, 1, 0); - assert.equal(out, "SELECT * FROM (" + sql + ") AS cdbq_1 LIMIT 1 OFFSET 0"); -}); - -test('Windowed SQL with CTE select', function(){ - // NOTE: intentionally mixed-case and space-padded - var cte = "\n \twiTh x as( update test set x=x+1)"; - var select = "\n \tSEleCT * from x"; - var sql = cte + select; - var out = PSQL.window_sql(sql, 1, 0); - assert.equal(out, cte + "SELECT * FROM (" + select + ") AS cdbq_1 LIMIT 1 OFFSET 0"); -}); - -test('Windowed SQL with CTE update', function(){ - // NOTE: intentionally mixed-case and space-padded - var cte = "\n \twiTh a as( update test set x=x+1)"; - var upd = "\n \tupdate tost set y=x from x"; - var sql = cte + upd; - var out = PSQL.window_sql(sql, 1, 0); - assert.equal(out, sql); -}); - -test('Windowed SQL with complex CTE and insane quoting', function(){ - // NOTE: intentionally mixed-case and space-padded - var cte = "\n \twiTh \"('a\" as( update \"\"\"test)\" set x='x'+1), \")b(\" as ( select ')))\"' from z )"; - var sel = "\n \tselect '\"' from x"; - var sql = cte + sel; - var out = PSQL.window_sql(sql, 1, 0); - assert.equal(out, cte + "SELECT * FROM (" + sel + ") AS cdbq_1 LIMIT 1 OFFSET 0"); -}); - test('dbkey depends on dbopts', function(){ var opt1 = _.clone(dbopts_anon); opt1.dbname = 'dbname1'; @@ -135,4 +101,12 @@ test('dbkey depends on dbopts', function(){ assert.ok(_.isString(pg1.dbkey()), "pg1 dbkey is " + pg1.dbkey()); }); + test('eventedQuery provisions a cancel mechanism to abort queries', function (done) { + var psql = new PSQL(dbopts_auth); + psql.eventedQuery("SELECT 1 as foo", function(err, query, queryCanceller) { + assert.ok(_.isFunction(queryCanceller)); + done(); + }); + }); + }); diff --git a/test/unit/sql/psql_wrapper.test.js b/test/unit/sql/psql_wrapper.test.js new file mode 100644 index 00000000..9b6d0def --- /dev/null +++ b/test/unit/sql/psql_wrapper.test.js @@ -0,0 +1,115 @@ +var _ = require('underscore'), + PSQLWrapper = require('../../../app/sql/psql_wrapper'), + assert = require('assert'); + + +// NOTE: intentionally mixed-case and space-padded +var simpleSql = "\n \tSEleCT * from table1"; + +suite('psql_wrapper', function() { + + test('Windowed SQL with simple select', function(){ + var out = new PSQLWrapper(simpleSql).window(1, 0).query(); + + assert.equal(out, "SELECT * FROM (" + simpleSql + ") AS cdbq_1 LIMIT 1 OFFSET 0"); + }); + + test('Windowed SQL with CTE select', function(){ + // NOTE: intentionally mixed-case and space-padded + var cte = "\n \twiTh x as( update test set x=x+1)"; + var select = "\n \tSEleCT * from x"; + var sql = cte + select; + + var out = new PSQLWrapper(sql).window(1, 0).query(); + + assert.equal(out, cte + "SELECT * FROM (" + select + ") AS cdbq_1 LIMIT 1 OFFSET 0"); + }); + + test('Windowed SQL with CTE update', function(){ + // NOTE: intentionally mixed-case and space-padded + var cte = "\n \twiTh a as( update test set x=x+1)"; + var upd = "\n \tupdate tost set y=x from x"; + var sql = cte + upd; + + var out = new PSQLWrapper(sql).window(1, 0).query(); + + assert.equal(out, sql); + }); + + test('Windowed SQL with complex CTE and insane quoting', function(){ + // NOTE: intentionally mixed-case and space-padded + var cte = "\n \twiTh \"('a\" as( update \"\"\"test)\" set x='x'+1), \")b(\" as ( select ')))\"' from z )"; + var sel = "\n \tselect '\"' from x"; + var sql = cte + sel; + + var out = new PSQLWrapper(sql).window(1, 0).query(); + + assert.equal(out, cte + "SELECT * FROM (" + sel + ") AS cdbq_1 LIMIT 1 OFFSET 0"); + }); + + test('Different instances return different queries', function() { + var aWrapper = new PSQLWrapper('select 1'); + var bWrapper = new PSQLWrapper('select * from databaseB'); + + assert.notEqual(aWrapper, bWrapper); + assert.notEqual(aWrapper.query(), bWrapper.query(), 'queries should be different'); + }); + + test('Order by SQL with simple select and empty column name returns original query', function() { + var expectedSql = simpleSql; + + var outputSql = new PSQLWrapper(simpleSql).orderBy('').query(); + + assert.equal(outputSql, expectedSql); + }); + + test('Order by SQL with simple select and no sort order', function() { + var expectedSql = 'SELECT * FROM (' + simpleSql + ') AS cdbq_1 ORDER BY "foo"'; + + var outputSql = new PSQLWrapper(simpleSql).orderBy('foo').query(); + + assert.equal(outputSql, expectedSql); + }); + + test('Order by SQL with simple select and invalid sort order use no sort order', function() { + var expectedSql = 'SELECT * FROM (' + simpleSql + ') AS cdbq_1 ORDER BY "foo"'; + + var outputSql = new PSQLWrapper(simpleSql).orderBy('foo', "BAD_SORT_ORDER").query(); + + assert.equal(outputSql, expectedSql); + }); + + test('Order by SQL with simple select and asc order', function() { + var expectedSql = 'SELECT * FROM (' + simpleSql + ') AS cdbq_1 ORDER BY "foo" ASC'; + + var outputSql = new PSQLWrapper(simpleSql).orderBy('foo', "asc").query(); + + assert.equal(outputSql, expectedSql); + }); + + test('Order by SQL with simple select and DESC order', function() { + var expectedSql = 'SELECT * FROM (' + simpleSql + ') AS cdbq_1 ORDER BY "foo" DESC'; + + var outputSql = new PSQLWrapper(simpleSql).orderBy('foo', "DESC").query(); + + assert.equal(outputSql, expectedSql); + }); + + test('Query with ending semicolon returns without it', function() { + var expectedSql = 'select a, ( a - min(a) over() ) / ( ( max(a) over () - min(a) over () ) / 4 ) as interval from ( select test as a from quantile_test ) as f', + query = expectedSql + ';'; + + var outputSql = new PSQLWrapper(query).query(); + + assert.equal(outputSql, expectedSql); + }); + + test('Several queries with semicolon get only last semicolon removed', function() { + var expectedSql = 'SELECT 1; SELECT 2; SELECT 3', + query = expectedSql + ';'; + + var outputSql = new PSQLWrapper(query).query(); + + assert.equal(outputSql, expectedSql); + }); +});