Improving support for SQL Server 2008 (#5616)

* Updating MSSQL version query to return semver compatible string

* Altering query builder to handle sql server 2008

Use TOP instead of LIMIT for queries against SQL server <11.0

* Refactor abstract query builder to support overriding select fragments

* Adding OFFSET support for simple queries in SQL Server 2008

Nested queries still fail due to how WITH() queries must be defined

* Altering OFFSET support for SQL Server 2008 to enable complex queries

* Updating Docker to Node v5

Adding shortcut for running integration tests in containers

* Updating legacy SQL server OFFSET query to support WHERE clauses

* Adding rebuild of WHERE clauses for OFFSET

* Add proper handling for nested ORDER BY for SQL Server

* Moving field includes to outer query for SQL server OFFSET

* Changing semver comparison

* Making semver.valid check before using version

* Extracting query orders and deleting duplicate code

* Fixing lint problems

* Adding unit tests and documentation for SQL limit/offset

* Fixing lint problems

* Tweaking query build loop and renaming sql fragment builder

Dockerfile

-FROM iojs:1.6
+FROM node:5
 
 RUN apt-get install libpq-dev
 
@@ -6,4 +6,4 @@ COPY package.json /
 RUN npm install
 
 WORKDIR /sequelize
-VOLUME /sequelize
\ No newline at end of file
+VOLUME /sequelize

lib/dialects/abstract/query-generator.js

@@ -1412,10 +1412,7 @@ var QueryGenerator = {
 
     // If using subQuery select defined subQuery attributes and join subJoinQueries
     if (subQuery) {
-      subQueryItems.push('SELECT ' + subQueryAttributes.join(', ') + ' FROM ' + table);
-      if (mainTableAs) {
-        subQueryItems.push(' AS ' + mainTableAs);
-      }
+      subQueryItems.push(this.selectFromTableFragment(options, model, subQueryAttributes, table, mainTableAs));
       subQueryItems.push(subJoinQueries.join(''));
 
     // Else do it the reguar way
@@ -1424,8 +1421,7 @@ var QueryGenerator = {
         if (!mainTableAs) {
           mainTableAs = table;
         }
-
-        mainQueryItems.push('SELECT '+mainAttributes.join(', ')+' FROM ('+
+        mainQueryItems.push(this.selectFromTableFragment(options, model, mainAttributes, '('+
           options.groupedLimit.values.map(function (value) {
             var where = _.assign({}, options.where);
             where[options.groupedLimit.on] = value;
@@ -1443,12 +1439,9 @@ var QueryGenerator = {
           }).join(
             self._dialect.supports['UNION ALL'] ?' UNION ALL ' : ' UNION '
           )
-        +')');
+        +')', mainTableAs));
       } else {
-        mainQueryItems.push('SELECT ' + mainAttributes.join(', ') + ' FROM ' + table);
-      }
-      if (mainTableAs) {
-        mainQueryItems.push(' AS ' + mainTableAs);
+        mainQueryItems.push(this.selectFromTableFragment(options, model, mainAttributes, table, mainTableAs));
       }
       mainQueryItems.push(mainJoinQueries.join(''));
     }
@@ -1461,6 +1454,12 @@ var QueryGenerator = {
           subQueryItems.push(' WHERE ' + options.where);
         } else {
           mainQueryItems.push(' WHERE ' + options.where);
+          // Walk the main query to update all selects
+          _.each(mainQueryItems, function(value, key) {
+            if(value.match(/^SELECT/)) {
+              mainQueryItems[key] = this.selectFromTableFragment(options, model, mainAttributes, table, mainTableAs, options.where);
+            }
+          }.bind(this));
         }
       }
     }
@@ -1486,53 +1485,13 @@ var QueryGenerator = {
     }
     // Add ORDER to sub or main query
     if (options.order && !options.groupedLimit) {
-      var mainQueryOrder = [];
-      var subQueryOrder = [];
-
-      var validateOrder = function(order) {
-        if (order instanceof Utils.literal) return;
-
-        if (!_.includes([
-          'ASC',
-          'DESC',
-          'ASC NULLS LAST',
-          'DESC NULLS LAST',
-          'ASC NULLS FIRST',
-          'DESC NULLS FIRST',
-          'NULLS FIRST',
-          'NULLS LAST'
-        ], order.toUpperCase())) {
-          throw new Error(util.format('Order must be \'ASC\' or \'DESC\', \'%s\' given', order));
-        }
-      };
-
-      if (Array.isArray(options.order)) {
-        options.order.forEach(function(t) {
-          if (Array.isArray(t) && _.size(t) > 1) {
-            if (t[0] instanceof Model || t[0].model instanceof Model) {
-              if (typeof t[t.length - 2] === 'string') {
-                validateOrder(_.last(t));
-              }
-            } else {
-              validateOrder(_.last(t));
-            }
-          }
-
-          if (subQuery && (Array.isArray(t) && !(t[0] instanceof Model) && !(t[0].model instanceof Model))) {
-            subQueryOrder.push(this.quote(t, model));
-          }
+      var orders = this.getQueryOrders(options, model, subQuery);
 
-          mainQueryOrder.push(this.quote(t, model));
-        }.bind(this));
-      } else {
-        mainQueryOrder.push(this.quote(typeof options.order === 'string' ? new Utils.literal(options.order) : options.order, model));
+      if (orders.mainQueryOrder.length) {
+        mainQueryItems.push(' ORDER BY ' + orders.mainQueryOrder.join(', '));
       }
-
-      if (mainQueryOrder.length) {
-        mainQueryItems.push(' ORDER BY ' + mainQueryOrder.join(', '));
-      }
-      if (subQueryOrder.length) {
-        subQueryItems.push(' ORDER BY ' + subQueryOrder.join(', '));
+      if (orders.subQueryOrder.length) {
+        subQueryItems.push(' ORDER BY ' + orders.subQueryOrder.join(', '));
       }
     }
 
@@ -1579,6 +1538,65 @@ var QueryGenerator = {
     return query;
   },
 
+  getQueryOrders: function(options, model, subQuery) {
+    var mainQueryOrder = [];
+    var subQueryOrder = [];
+
+    var validateOrder = function(order) {
+      if (order instanceof Utils.literal) return;
+
+      if (!_.includes([
+        'ASC',
+        'DESC',
+        'ASC NULLS LAST',
+        'DESC NULLS LAST',
+        'ASC NULLS FIRST',
+        'DESC NULLS FIRST',
+        'NULLS FIRST',
+        'NULLS LAST'
+      ], order.toUpperCase())) {
+        throw new Error(util.format('Order must be \'ASC\' or \'DESC\', \'%s\' given', order));
+      }
+    };
+
+    if (Array.isArray(options.order)) {
+      options.order.forEach(function(t) {
+        if (Array.isArray(t) && _.size(t) > 1) {
+          if (t[0] instanceof Model || t[0].model instanceof Model) {
+            if (typeof t[t.length - 2] === 'string') {
+              validateOrder(_.last(t));
+            }
+          } else {
+            validateOrder(_.last(t));
+          }
+        }
+
+        if (subQuery && (Array.isArray(t) && !(t[0] instanceof Model) && !(t[0].model instanceof Model))) {
+          subQueryOrder.push(this.quote(t, model));
+        }
+
+        mainQueryOrder.push(this.quote(t, model));
+      }.bind(this));
+    } else {
+      mainQueryOrder.push(this.quote(typeof options.order === 'string' ? new Utils.literal(options.order) : options.order, model));
+    }
+
+    return {
+      mainQueryOrder: mainQueryOrder,
+      subQueryOrder: subQueryOrder
+    };
+  },
+
+  selectFromTableFragment: function(options, model, attributes, tables, mainTableAs, whereClause) {
+    var fragment = 'SELECT ' + attributes.join(', ') + ' FROM ' + tables;
+
+    if(mainTableAs) {
+      fragment += ' AS ' + mainTableAs;
+    }
+
+    return fragment;
+  },
+
   joinIncludeQuery: function(options) {
     var subQuery = options.subQuery
       , include = options.include

lib/dialects/mssql/index.js

@@ -50,6 +50,7 @@ MssqlDialect.prototype.supports = _.merge(_.cloneDeep(Abstract.prototype.support
   tmpTableTrigger: true
 });
 
+MssqlDialect.prototype.defaultVersion = '12.0.2000'; // SQL Server 2014 Express
 MssqlDialect.prototype.Query = Query;
 MssqlDialect.prototype.name = 'mssql';
 MssqlDialect.prototype.TICK_CHAR = '"';

lib/dialects/mssql/query-generator.js

@@ -3,8 +3,8 @@
 /* jshint -W110 */
 var Utils = require('../../utils')
   , DataTypes = require('../../data-types')
-  , Model = require('../../model')
-  , AbstractQueryGenerator = require('../abstract/query-generator');
+  , AbstractQueryGenerator = require('../abstract/query-generator')
+  , semver = require('semver');
 
 /* istanbul ignore next */
 var throwMethodUndefined = function(methodName) {
@@ -38,7 +38,12 @@ var QueryGenerator = {
   },
 
   versionQuery: function() {
-    return "SELECT @@VERSION as 'version'";
+    // Uses string manipulation to convert the MS Maj.Min.Patch.Build to semver Maj.Min.Patch
+    return [
+      'DECLARE @ms_ver NVARCHAR(20);',
+      "SET @ms_ver = REVERSE(CONVERT(NVARCHAR(20), SERVERPROPERTY('ProductVersion')));",
+      "SELECT REVERSE(SUBSTRING(@ms_ver, CHARINDEX('.', @ms_ver)+1, 20)) AS 'version'"
+    ].join(' ');
   },
 
   createTableQuery: function(tableName, attributes, options) {
@@ -610,38 +615,77 @@ var QueryGenerator = {
     return 'ROLLBACK TRANSACTION;';
   },
 
+  selectFromTableFragment: function(options, model, attributes, tables, mainTableAs, where) {
+    var topFragment = '';
+    var mainFragment = 'SELECT ' + attributes.join(', ') + ' FROM ' + tables;
+
+    // Handle SQL Server 2008 with TOP instead of LIMIT
+    if (semver.valid(this.sequelize.options.databaseVersion) && semver.lt(this.sequelize.options.databaseVersion, '11.0.0')) {
+      if (options.limit) {
+        topFragment = 'TOP ' + options.limit + ' ';
+      }
+      if (options.offset) {
+        var offset = options.offset || 0
+          , isSubQuery = options.hasIncludeWhere || options.hasIncludeRequired || options.hasMultiAssociation
+          , orders = { mainQueryOrder: [] };
+        if (options.order) {
+          orders = this.getQueryOrders(options, model, isSubQuery);
+        }
+
+        if(!orders.mainQueryOrder.length) {
+          orders.mainQueryOrder.push(this.quoteIdentifier(model.primaryKeyField));
+        }
+
+        var tmpTable = (mainTableAs) ? mainTableAs : 'OffsetTable';
+        var whereFragment = (where) ? ' WHERE ' + where : '';
+
+        /*
+         * For earlier versions of SQL server, we need to nest several queries
+         * in order to emulate the OFFSET behavior.
+         *
+         * 1. The outermost query selects all items from the inner query block.
+         *    This is due to a limitation in SQL server with the use of computed
+         *    columns (e.g. SELECT ROW_NUMBER()...AS x) in WHERE clauses.
+         * 2. The next query handles the LIMIT and OFFSET behavior by getting
+         *    the TOP N rows of the query where the row number is > OFFSET
+         * 3. The innermost query is the actual set we want information from
+         */
+        var fragment = 'SELECT TOP 100 PERCENT ' + attributes.join(', ') + ' FROM ' +
+                        '(SELECT ' + topFragment + '*' +
+                          ' FROM (SELECT ROW_NUMBER() OVER (ORDER BY ' + orders.mainQueryOrder.join(', ') + ') as row_num, * ' +
+                            ' FROM ' + tables + ' AS ' + tmpTable + whereFragment + ')' +
+                          ' AS ' + tmpTable + ' WHERE row_num > ' + offset + ')' +
+                        ' AS ' + tmpTable;
+        return fragment;
+      } else {
+        mainFragment = 'SELECT ' + topFragment + attributes.join(', ') + ' FROM ' + tables;
+      }
+    }
+
+    if(mainTableAs) {
+      mainFragment += ' AS ' + mainTableAs;
+    }
+
+    return mainFragment;
+  },
+
   addLimitAndOffset: function(options, model) {
+    // Skip handling of limit and offset as postfixes for older SQL Server versions
+    if(semver.valid(this.sequelize.options.databaseVersion) && semver.lt(this.sequelize.options.databaseVersion, '11.0.0')) {
+      return '';
+    }
+
     var fragment = '';
     var offset = options.offset || 0
       , isSubQuery = options.hasIncludeWhere || options.hasIncludeRequired || options.hasMultiAssociation;
 
-    // FIXME: This is ripped from selectQuery to determine whether there is already
-    //        an ORDER BY added for a subquery. Should be refactored so we dont' need
-    //        the duplication. Also consider moving this logic inside the options.order
-    //        check, so that we aren't compiling this twice for every invocation.
-    var mainQueryOrder = [];
-    var subQueryOrder = [];
+    var orders = {};
     if (options.order) {
-      if (Array.isArray(options.order)) {
-        options.order.forEach(function(t) {
-          if (!Array.isArray(t)) {
-            if (isSubQuery && !(t instanceof Model) && !(t.model instanceof Model)) {
-              subQueryOrder.push(this.quote(t, model));
-            }
-          } else {
-            if (isSubQuery && !(t[0] instanceof Model) && !(t[0].model instanceof Model)) {
-              subQueryOrder.push(this.quote(t, model));
-            }
-            mainQueryOrder.push(this.quote(t, model));
-          }
-        }.bind(this));
-      } else {
-        mainQueryOrder.push(options.order);
-      }
+      orders = this.getQueryOrders(options, model, isSubQuery);
     }
 
     if (options.limit || options.offset) {
-      if (!options.order || (options.include && !subQueryOrder.length)) {
+      if (!options.order || (options.include && !orders.subQueryOrder.length)) {
         fragment += (options.order && !isSubQuery) ? ', ' : ' ORDER BY ';
         fragment += this.quoteIdentifier(model.primaryKeyField);
       }

lib/dialects/mssql/query.js

@@ -200,7 +200,7 @@ Query.prototype.handleShowTablesQuery = function(results) {
 
 Query.prototype.formatError = function (err) {
   var match;
-  match = err.message.match(/Violation of UNIQUE KEY constraint '((.|\s)*)'. Cannot insert duplicate key in object '.*'. The duplicate key value is \((.*)\)./);
+  match = err.message.match(/Violation of UNIQUE KEY constraint '((.|\s)*)'. Cannot insert duplicate key in object '.*'.(:? The duplicate key value is \((.*)\).)?/);
   match = match || err.message.match(/Cannot insert duplicate key row in object .* with unique index '(.*)'/);
   if (match && match.length > 1) {
     var fields = {}

package.json

@@ -91,6 +91,7 @@
     "test": "if [ $COVERAGE ]; then npm run codeclimate; else npm run jshint && npm run teaser && npm run test-unit && npm run test-integration; fi",
     "test-docker": "docker-compose run sequelize /bin/sh -c \"npm run test-all\"",
     "test-docker-unit": "docker-compose run sequelize /bin/sh -c \"npm run test-unit-all\"",
+    "test-docker-integration": "docker-compose run sequelize /bin/sh -c \"npm run test-integration-all\"",
     "build-docker": "docker-compose build",
     "docs": "node docs/docs-generator.js",
     "jshint": "./node_modules/.bin/jshint lib test",

test/integration/model/findAll.test.js

@@ -1313,13 +1313,16 @@ describe(Support.getTestDialectTeaser('Model'), function() {
                     var criteria = {
                       offset: 5,
                       limit: 1,
+                      where: {
+                        name: 'Some election'
+                      },
                       include: [
                         Citizen, // Election creator
                         { model: Citizen, as: 'Voters' } // Election voters
                       ]
                     };
                     return Election.findAndCountAll(criteria).then(function(elections) {
-                      expect(elections.count).to.equal(2);
+                      expect(elections.count).to.equal(1);
                       expect(elections.rows.length).to.equal(0);
                     });
                   });

test/unit/dialects/mssql/query-generator.test.js

@@ -4,7 +4,8 @@
 var Support = require(__dirname + '/../../support')
   , expectsql = Support.expectsql
   , current = Support.sequelize
-  , QueryGenerator = require('../../../../lib/dialects/mssql/query-generator');
+  , QueryGenerator = require('../../../../lib/dialects/mssql/query-generator')
+  , _ = require('lodash');
 
 if (current.dialect.name === 'mssql') {
   suite('[MSSQL Specific] QueryGenerator', function () {
@@ -22,5 +23,60 @@ if (current.dialect.name === 'mssql') {
         mssql: "ALTER TABLE [mySchema].[myTable] DROP CONSTRAINT [myConstraint];"
       });
     });
+
+    test('selectFromTableFragment', function() {
+      var modifiedGen = _.clone(QueryGenerator);
+      // Test newer versions first
+      // Should be all the same since handling is done in addLimitAndOffset
+      // for SQL Server 2012 and higher (>= v11.0.0)
+      modifiedGen.sequelize = {
+        options: {
+          databaseVersion: '11.0.0'
+        }
+      };
+
+      // Base case
+      expectsql(modifiedGen.selectFromTableFragment({}, { primaryKeyField: 'id' }, ['id', 'name'], 'myTable', 'myOtherName', 'WHERE id=1'), {
+        mssql: "SELECT id, name FROM myTable AS myOtherName"
+      });
+
+      // With limit
+      expectsql(modifiedGen.selectFromTableFragment({ limit: 10 }, { primaryKeyField: 'id' }, ['id', 'name'], 'myTable', 'myOtherName'), {
+        mssql: "SELECT id, name FROM myTable AS myOtherName"
+      });
+
+      // With offset
+      expectsql(modifiedGen.selectFromTableFragment({ offset: 10 }, { primaryKeyField: 'id' }, ['id', 'name'], 'myTable', 'myOtherName'), {
+        mssql: "SELECT id, name FROM myTable AS myOtherName"
+      });
+
+      // With both limit and offset
+      expectsql(modifiedGen.selectFromTableFragment({ limit: 10, offset: 10 }, { primaryKeyField: 'id' }, ['id', 'name'], 'myTable', 'myOtherName'), {
+        mssql: "SELECT id, name FROM myTable AS myOtherName"
+      });
+
+      // Test older version (< v11.0.0)
+      modifiedGen.sequelize.options.databaseVersion = '10.0.0';
+
+      // Base case
+      expectsql(modifiedGen.selectFromTableFragment({}, { primaryKeyField: 'id' }, ['id', 'name'], 'myTable', 'myOtherName', 'WHERE id=1'), {
+        mssql: "SELECT id, name FROM myTable AS myOtherName"
+      });
+
+      // With limit
+      expectsql(modifiedGen.selectFromTableFragment({ limit: 10 }, { primaryKeyField: 'id' }, ['id', 'name'], 'myTable', 'myOtherName'), {
+        mssql: "SELECT TOP 10 id, name FROM myTable AS myOtherName"
+      });
+
+      // With offset
+      expectsql(modifiedGen.selectFromTableFragment({ offset: 10 }, { primaryKeyField: 'id' }, ['id', 'name'], 'myTable', 'myOtherName'), {
+        mssql: "SELECT TOP 100 PERCENT id, name FROM (SELECT * FROM (SELECT ROW_NUMBER() OVER (ORDER BY [id]) as row_num, *  FROM myTable AS myOtherName) AS myOtherName WHERE row_num > 10) AS myOtherName"
+      });
+
+      // With both limit and offset
+      expectsql(modifiedGen.selectFromTableFragment({ limit: 10, offset: 10 }, { primaryKeyField: 'id' }, ['id', 'name'], 'myTable', 'myOtherName'), {
+        mssql: "SELECT TOP 100 PERCENT id, name FROM (SELECT TOP 10 * FROM (SELECT ROW_NUMBER() OVER (ORDER BY [id]) as row_num, *  FROM myTable AS myOtherName) AS myOtherName WHERE row_num > 10) AS myOtherName"
+      });
+    });
   });
 }