Merge pull request #4990 from MichaelKearnsSoftwire/mssql_can_remove_column_with_default

Mssql can remove column with default

Makefile

@@ -22,7 +22,7 @@ endif
 
 
 # Unit tests
 # Unit tests
 test-unit:
 test-unit:
-	$(MOCHA) --globals setImmediate,clearImmediate --ui tdd --check-leaks --colors -t 15000 --reporter $(REPORTER) ./test/unit/*.js ./test/unit/**/*.js
+	$(MOCHA) --globals setImmediate,clearImmediate --ui tdd --check-leaks --colors -t 15000 --reporter $(REPORTER) ./test/unit/*.js "./test/unit/**/*.js"
 
 
 test-unit-all: test-unit-sqlite test-unit-mysql test-unit-postgres test-unit-postgres-native test-unit-mariadb test-unit-mssql
 test-unit-all: test-unit-sqlite test-unit-mysql test-unit-postgres test-unit-postgres-native test-unit-mariadb test-unit-mssql
 
 

lib/dialects/abstract/query-generator.js

@@ -24,7 +24,7 @@ var QueryGenerator = {
     tableName = tableName || {};
     tableName = tableName || {};
     return {
     return {
       schema: tableName.schema || options.schema || 'public',
       schema: tableName.schema || options.schema || 'public',
-      tableName: _.isPlainObject(tableName) ? tableName.table : tableName,
+      tableName: _.isPlainObject(tableName) ? tableName.tableName : tableName,
       delimiter: tableName.delimiter || options.delimiter || '.'
       delimiter: tableName.delimiter || options.delimiter || '.'
     };
     };
   },
   },

lib/dialects/mssql/query-generator.js

@@ -532,6 +532,25 @@ var QueryGenerator = {
     });
     });
   },
   },
 
 
+  getDefaultConstraintQuery: function (tableName, attributeName) {
+    var sql = "SELECT name FROM SYS.DEFAULT_CONSTRAINTS " +
+      "WHERE PARENT_OBJECT_ID = OBJECT_ID('<%= table %>', 'U') " +
+      "AND PARENT_COLUMN_ID = (SELECT column_id FROM sys.columns WHERE NAME = ('<%= column %>') " +
+      "AND object_id = OBJECT_ID('<%= table %>', 'U'));";
+    return Utils._.template(sql)({
+      table: this.quoteTable(tableName),
+      column: attributeName
+    });
+  },
+
+  dropConstraintQuery: function (tableName, constraintName) {
+    var sql = 'ALTER TABLE <%= table %> DROP CONSTRAINT <%= constraint %>;';
+    return Utils._.template(sql)({
+      table: this.quoteTable(tableName),
+      constraint: this.quoteIdentifier(constraintName)
+    });
+  },
+
   setAutocommitQuery: function(value) {
   setAutocommitQuery: function(value) {
     return '';
     return '';
     // return 'SET IMPLICIT_TRANSACTIONS ' + (!!value ? 'OFF' : 'ON') + ';';
     // return 'SET IMPLICIT_TRANSACTIONS ' + (!!value ? 'OFF' : 'ON') + ';';

lib/dialects/mssql/query-interface.js

+'use strict';
+
+/**
+ Returns an object that treats MSSQL's inabilities to do certain queries.
+
+ @class QueryInterface
+ @static
+ */
+
+/**
+  A wrapper that fixes MSSQL's inability to cleanly remove columns from existing tables if they have a default constraint.
+
+  @method removeColumn
+  @for    QueryInterface
+
+  @param  {String} tableName     The name of the table.
+  @param  {String} attributeName The name of the attribute that we want to remove.
+  @param  {Object} options
+  @param  {Boolean|Function} [options.logging] A function that logs the sql queries, or false for explicitly not logging these queries
+ */
+var removeColumn = function (tableName, attributeName, options) {
+  var self = this;
+  options = options || {};
+
+  var findConstraintSql = self.QueryGenerator.getDefaultConstraintQuery(tableName, attributeName);
+  return self.sequelize.query(findConstraintSql, { raw: true, logging: options.logging})
+    .spread(function (results) {
+      if (!results.length) {
+        // No default constraint found -- we can cleanly remove the column
+        return;
+      }
+      var dropConstraintSql = self.QueryGenerator.dropConstraintQuery(tableName, results[0].name);
+      return self.sequelize.query(dropConstraintSql, { raw: true, logging: options.logging});
+    })
+    .then(function () {
+      var removeSql = self.QueryGenerator.removeColumnQuery(tableName, attributeName);
+      return self.sequelize.query(removeSql, { raw: true, logging: options.logging});
+    });
+};
+
+module.exports = {
+  removeColumn: removeColumn
+};

lib/dialects/postgres/query-generator.js

@@ -193,7 +193,7 @@ var QueryGenerator = {
     });
     });
 
 
     return Utils._.template(query)({
     return Utils._.template(query)({
-      table: this.quoteTable(table),
+      table: this.quoteTable(this.extractTableDetails(table)),
       attribute: attribute
       attribute: attribute
     });
     });
   },
   },
@@ -201,7 +201,7 @@ var QueryGenerator = {
   removeColumnQuery: function(tableName, attributeName) {
   removeColumnQuery: function(tableName, attributeName) {
     var query = 'ALTER TABLE <%= tableName %> DROP COLUMN <%= attributeName %>;';
     var query = 'ALTER TABLE <%= tableName %> DROP COLUMN <%= attributeName %>;';
     return Utils._.template(query)({
     return Utils._.template(query)({
-      tableName: this.quoteTable(tableName),
+      tableName: this.quoteTable(this.extractTableDetails(tableName)),
       attributeName: this.quoteIdentifier(attributeName)
       attributeName: this.quoteIdentifier(attributeName)
     });
     });
   },
   },

lib/dialects/sqlite/query-interface.js

@@ -22,8 +22,6 @@ var Utils = require('../../utils')
   @param  {String} attributeName The name of the attribute that we want to remove.
   @param  {String} attributeName The name of the attribute that we want to remove.
   @param  {Object} options
   @param  {Object} options
   @param  {Boolean|Function} [options.logging] A function that logs the sql queries, or false for explicitly not logging these queries
   @param  {Boolean|Function} [options.logging] A function that logs the sql queries, or false for explicitly not logging these queries
-  @param  {CustomEventEmitter} emitter       The EventEmitter from outside.
-  @param  {Function} queryAndEmit  The function from outside that triggers some events to get triggered.
 
 
   @since 1.6.0
   @since 1.6.0
  */
  */
@@ -55,8 +53,6 @@ var removeColumn = function(tableName, attributeName, options) {
   @param  {Object} attributes An object with the attribute's name as key and it's options as value object.
   @param  {Object} attributes An object with the attribute's name as key and it's options as value object.
   @param  {Object} options
   @param  {Object} options
   @param  {Boolean|Function} [options.logging] A function that logs the sql queries, or false for explicitly not logging these queries
   @param  {Boolean|Function} [options.logging] A function that logs the sql queries, or false for explicitly not logging these queries
-  @param  {CustomEventEmitter} emitter The EventEmitter from outside.
-  @param  {Function} queryAndEmit The function from outside that triggers some events to get triggered.
 
 
   @since 1.6.0
   @since 1.6.0
  */
  */
@@ -90,8 +86,6 @@ var changeColumn = function(tableName, attributes, options) {
   @param  {String} attrNameAfter The name of the attribute after it was renamed.
   @param  {String} attrNameAfter The name of the attribute after it was renamed.
   @param  {Object} options
   @param  {Object} options
   @param  {Boolean|Function} [options.logging] A function that logs the sql queries, or false for explicitly not logging these queries
   @param  {Boolean|Function} [options.logging] A function that logs the sql queries, or false for explicitly not logging these queries
-  @param  {CustomEventEmitter} emitter The EventEmitter from outside.
-  @param  {Function} queryAndEmit The function from outside that triggers some events to get triggered.
 
 
   @since 1.6.0
   @since 1.6.0
  */
  */

lib/query-interface.js

@@ -4,6 +4,7 @@ var Utils = require('./utils')
   , _ = require('lodash')
   , _ = require('lodash')
   , DataTypes = require('./data-types')
   , DataTypes = require('./data-types')
   , SQLiteQueryInterface = require('./dialects/sqlite/query-interface')
   , SQLiteQueryInterface = require('./dialects/sqlite/query-interface')
+  , MSSSQLQueryInterface = require('./dialects/mssql/query-interface')
   , Transaction = require('./transaction')
   , Transaction = require('./transaction')
   , Promise = require('./promise')
   , Promise = require('./promise')
   , QueryTypes = require('./query-types');
   , QueryTypes = require('./query-types');
@@ -354,12 +355,16 @@ QueryInterface.prototype.addColumn = function(table, key, attribute, options) {
 
 
 QueryInterface.prototype.removeColumn = function(tableName, attributeName, options) {
 QueryInterface.prototype.removeColumn = function(tableName, attributeName, options) {
   options = options || {};
   options = options || {};
-  if (this.sequelize.options.dialect === 'sqlite') {
-    // sqlite needs some special treatment as it cannot drop a column
-    return SQLiteQueryInterface.removeColumn.call(this, tableName, attributeName, options);
-  } else {
-    var sql = this.QueryGenerator.removeColumnQuery(tableName, attributeName);
-    return this.sequelize.query(sql, options);
+  switch (this.sequelize.options.dialect) {
+    case 'sqlite':
+      // sqlite needs some special treatment as it cannot drop a column
+      return SQLiteQueryInterface.removeColumn.call(this, tableName, attributeName, options);
+    case 'mssql':
+      // mssql needs special treatment as it cannot drop a column with a default constraint
+      return MSSSQLQueryInterface.removeColumn.call(this, tableName, attributeName, options);
+    default:
+        var sql = this.QueryGenerator.removeColumnQuery(tableName, attributeName);
+        return this.sequelize.query(sql, options);
   }
   }
 };
 };
 
 

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

+'use strict';
+
+/* jshint -W110 */
+var Support = require(__dirname + '/../../support')
+  , expectsql = Support.expectsql
+  , current = Support.sequelize
+  , QueryGenerator = require('../../../../lib/dialects/mssql/query-generator');
+
+if (current.dialect.name === 'mssql') {
+  suite('[MSSQL Specific] QueryGenerator', function () {
+    // Dialect would normally be set by the query interface that instantiates the query-generator, but here we specify it explicitly
+    QueryGenerator._dialect = current.dialect;
+
+    test('getDefaultConstraintQuery', function () {
+      expectsql(QueryGenerator.getDefaultConstraintQuery({tableName: 'myTable', schema: 'mySchema'}, 'myColumn'), {
+        mssql: "SELECT name FROM SYS.DEFAULT_CONSTRAINTS WHERE PARENT_OBJECT_ID = OBJECT_ID('[mySchema].[myTable]', 'U') AND PARENT_COLUMN_ID = (SELECT column_id FROM sys.columns WHERE NAME = ('myColumn') AND object_id = OBJECT_ID('[mySchema].[myTable]', 'U'));"
+      });
+    });
+
+    test('dropConstraintQuery', function () {
+      expectsql(QueryGenerator.dropConstraintQuery({tableName: 'myTable', schema: 'mySchema'}, 'myConstraint'), {
+        mssql: "ALTER TABLE [mySchema].[myTable] DROP CONSTRAINT [myConstraint];"
+      });
+    });
+  });
+}

test/integration/dialects/postgres/query-generator.test.js → test/unit/dialects/postgres/query-generator.test.js

@@ -13,17 +13,6 @@ var chai = require('chai')
 
 
 if (dialect.match(/^postgres/)) {
 if (dialect.match(/^postgres/)) {
   describe('[POSTGRES Specific] QueryGenerator', function() {
   describe('[POSTGRES Specific] QueryGenerator', function() {
-    beforeEach(function() {
-      this.User = this.sequelize.define('User', {
-        username: DataTypes.STRING,
-        email: { type: DataTypes.ARRAY(DataTypes.TEXT) },
-        numbers: { type: DataTypes.ARRAY(DataTypes.FLOAT) },
-        document: { type: DataTypes.HSTORE, defaultValue: { default: '"value"' } }
-      });
-
-      return this.User.sync({ force: true });
-    });
-
     var suites = {
     var suites = {
       attributesToSQL: [
       attributesToSQL: [
         {
         {