Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improving support for SQL Server 2008 #5616

Merged
merged 17 commits into from May 13, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions Dockerfile
@@ -1,9 +1,9 @@
FROM iojs:1.6
FROM node:5

RUN apt-get install libpq-dev

COPY package.json /
RUN npm install

WORKDIR /sequelize
VOLUME /sequelize
VOLUME /sequelize
130 changes: 74 additions & 56 deletions lib/dialects/abstract/query-generator.js
Expand Up @@ -1408,10 +1408,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
Expand All @@ -1420,8 +1417,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;
Expand All @@ -1439,12 +1435,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(''));
}
Expand All @@ -1457,6 +1450,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);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens here exactly? You attempt to find the first query item containg a select statement and then replace? Shouldn't there be an easier way of doing this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct, more or less about what this is for. It was originally designed to handle nesting queries, but since that's handled already by the recursive subquery calls, I'll simplify it.

}.bind(this));
}
}
}
Expand All @@ -1482,53 +1481,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(', '));
}
}

Expand Down Expand Up @@ -1575,6 +1534,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
Expand Down
1 change: 1 addition & 0 deletions lib/dialects/mssql/index.js
Expand Up @@ -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 = '"';
Expand Down
96 changes: 70 additions & 26 deletions lib/dialects/mssql/query-generator.js
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -609,38 +614,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);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/dialects/mssql/query.js
Expand Up @@ -191,7 +191,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 = {}
Expand Down
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -90,6 +90,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",
Expand Down
5 changes: 4 additions & 1 deletion test/integration/model/findAll.test.js
Expand Up @@ -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);
});
});
Expand Down