Merge branch 'master' of https://github.com/CartoDB/CartoDB-SQL-API into cdb

.travis.yml

@@ -1,13 +1,19 @@
-before_install:
+before_script:
-  #- sudo apt-add-repository --yes ppa:ubuntugis/ppa
+  - lsb_release -a
-  - sudo apt-get update -q
+  - sudo mv /etc/apt/sources.list.d/pgdg-source.list* /tmp
-  # Removal of postgresql-9.1-postgis-scripts is needed due to a
+  - sudo apt-get -qq purge postgis* postgresql*
-  # bug in some upstream package.
+  - sudo rm -Rf /var/lib/postgresql /etc/postgresql
-  # See http://trac.osgeo.org/ubuntugis/ticket/39
+  - sudo apt-add-repository --yes ppa:cartodb/postgresql-9.3
-  - sudo apt-get remove --purge -q postgresql-9.1-postgis-scripts 
+  - sudo apt-add-repository --yes ppa:cartodb/gis
-  - sudo apt-get install -q postgresql-9.1-postgis gdal-bin
+  - sudo apt-get update
-  - createdb template_postgis
+  - sudo apt-get install -q postgresql-9.3-postgis-2.1
-  - psql -c "CREATE EXTENSION postgis" template_postgis
+  - 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

HOWTO_RELEASE

@@ -1,11 +1,20 @@
-1. Ensure proper version in package.json 
+1. Test (make clean all check), fix if broken before proceeding
-2. Ensure NEWS section exists for the new version, review it, add release date
+2. Ensure proper version in package.json 
-3. Drop npm-shrinkwrap.json
+3. Ensure NEWS section exists for the new version, review it, add release date
-4. Run npm install
+4. Drop npm-shrinkwrap.json
-5. Test (make check or npm test), fix if broken before proceeding
+5. Run npm shrinkwrap to recreate npm-shrinkwrap.json
-6. Run npm shrinkwrap
+6. Commit package.json, npm-shrinwrap.json, NEWS
-7. Commit package.json, npm-shrinwrap.json, NEWS
+7. git tag -a Major.Minor.Patch # use NEWS section as content
-8. Tag Major.Minor.Patch
+8. Announce on cartodb@googlegroups.com
-9. Announce
+9. Stub NEWS/package for next version
-10. 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<Major>.<Minor>' are kept for any critical
+fix that might need to be shipped before next feature release
+is ready.

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
 ------------------
 

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
+var sqlQueryMayWriteRegex = new RegExp("\\b(alter|insert|update|delete|create|drop|reindex|truncate)\\b", "i");
-//
+/**
-// NOTE: this is a fuzzy check, the return could be true even
+ * This is a fuzzy check, the return could be true even if the query doesn't really write anything. But you can be
-//       if the query doesn't really write anything.
+ * pretty sure of a false return.
-//       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;
+    return sqlQueryMayWriteRegex.test(sql);
-  var pattern = RegExp("\\b(alter|insert|update|delete|create|drop|reindex|truncate)\\b", "i");
-  if ( pattern.test(sql) ) {
-    mayWrite = true;
-  }
-  return mayWrite;
 }
 
 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?)");
+        if (req.formatter && _.isFunction(req.formatter.cancel)) {
-      req.aborted = true; // TODO: there must be a builtin way to check this
+            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,

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;

app/models/psql.js

@@ -1,6 +1,7 @@
-var _      = require('underscore')
+var _      = require('underscore'),
-    , Step   = require('step')
+    PSQLWrapper = require('../sql/psql_wrapper'),
-    , pg     = require('pg');//.native; // disabled for now due to: https://github.com/brianc/node-postgres/issues/48
+    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){
+            function(err, query, client){
-                callback(err, query)
+                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)
+    // keeping it here for backwards compatibility
-  if (!_.isNumber(limit) || !_.isNumber(offset) ) return sql;
+    return new PSQLWrapper(sql).window(limit, offset).query();
-
+};
-  // 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;
-}
 
 module.exports = PSQL;

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;

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
 ----------------
 

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

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"
                     }
                   }
                 },

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" }
 }

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 = '<?xml version="1.0" encoding="utf-8" ?><kml xmlns="http://www.opengis.net/kml/2.2"><Document><Folder><name>cartodb_query</name></Folder></Document></kml>';
+        // NOTE: GDAL-1.11+ added 'id="root_doc"' attribute to the output
-        assert.equal(res.body.replace(/\n/g,''), body);
+        var pat = RegExp('^<\\?xml version="1.0" encoding="utf-8" \\?><kml xmlns="http://www.opengis.net/kml/2.2"><Document( id="root_doc")?><Folder><name>cartodb_query</name></Folder></Document></kml>$');
+        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 = '<?xml version="1.0" encoding="utf-8" ?><kml xmlns="http://www.opengis.net/kml/2.2"><Document><Folder><name>cartodb_query</name></Folder></Document></kml>';
+        // NOTE: GDAL-1.11+ added 'id="root_doc"' attribute to the output
-        assert.equal(res.body.replace(/\n/g,''), body);
+        var pat = RegExp('^<\\?xml version="1.0" encoding="utf-8" \\?><kml xmlns="http://www.opengis.net/kml/2.2"><Document( id="root_doc")?><Folder><name>cartodb_query</name></Folder></Document></kml>$');
+        var body = res.body.replace(/\n/g,'');
+        assert.ok(body.match(pat),
+          "Response:\n" + body + '\ndoes not match pattern:\n' + pat);
         done();
     });
 });

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
+  dropdb -U postgres ${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"
+  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 -U postgres -f support/CDB_QueryStatements.sql ${TEST_DB}
-  psql -f support/CDB_QueryTables.sql ${TEST_DB} 
+  psql -U postgres -f support/CDB_QueryTables.sql ${TEST_DB}
 
 fi
 

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

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();
+        });
+    });
+
 });

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);
+    });
+});