fix problems with mssql constraints

temporarily disable on update foreign key constraints, mssql dialect now passes all current test cases
sarme · Dec 9, 2014 · 381751a · 381751a
1 parent bba8702
commit 381751a
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 28 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -41,3 +41,9 @@ matrix:
   allow_failures:
     - node_js: "0.10"
       env: COVERAGE=true
+
+notifications:
+  hipchat:
+    - 40e8850aaba9854ac4c9963bd33f8b@253477
+  irc:
+    - "chat.freenode.net#sequelizejs"
diff --git a/lib/dialects/mssql/query-generator.js b/lib/dialects/mssql/query-generator.js
@@ -46,15 +46,15 @@ module.exports = (function() {
             primaryKeys.push(attr);
 
             if (Utils._.includes(dataType, 'REFERENCES')) {
-               // MySQL doesn't support inline REFERENCES declarations: move to the end
+               // MSSQL doesn't support inline REFERENCES declarations: move to the end
               match = dataType.match(/^(.+) (REFERENCES.*)$/);
               attrStr.push(this.quoteIdentifier(attr) + ' ' + match[1].replace(/PRIMARY KEY/, ''));
               foreignKeys[attr] = match[2];
             } else {
               attrStr.push(this.quoteIdentifier(attr) + ' ' + dataType.replace(/PRIMARY KEY/, ''));
             }
           } else if (Utils._.includes(dataType, 'REFERENCES')) {
-            // MySQL doesn't support inline REFERENCES declarations: move to the end
+            // MSSQL doesn't support inline REFERENCES declarations: move to the end
             match = dataType.match(/^(.+) (REFERENCES.*)$/);
             attrStr.push(this.quoteIdentifier(attr) + ' ' + match[1]);
             foreignKeys[attr] = match[2];
@@ -381,42 +381,28 @@ module.exports = (function() {
     attributesToSQL: function(attributes, options) {
       var result = {}
         , key
-        , attribute;
+        , attribute
+        , existingConstraints = [];
 
-      //detect multiple cascades associated with the same id
-      var cascadeCheck = [];
       for (key in attributes) {
         attribute = attributes[key];
-        if (attribute.onDelete || attribute.onUpdate) {
-          //handles self referencial keys, first it doesnt make sense, second, what?
-          if (attribute.Model && attribute.Model.tableName === attribute.references) {
-            this.sequelize.log('MSSQL does not support self referencial constraints, '
-              + 'we will remove it but we recommend restructuring your query');
+
+        if (attribute.references) {
+          if (existingConstraints.indexOf(attribute.references.toString()) !== -1) {
+            // no cascading constraints to a table more than once
             attribute.onDelete = '';
             attribute.onUpdate = '';
           } else {
-            cascadeCheck.push(key);
-          }
-        }
-      }
+            existingConstraints.push(attribute.references.toString());
 
-      for (var i = 0; i < cascadeCheck.length-1; i++) {
-        var casKey = cascadeCheck[i];
-        for (var j = i+1; j < cascadeCheck.length; j++) {
-          var casKey2 = cascadeCheck[j];
-          if (attributes[casKey].referencesKey === attributes[casKey2].referencesKey) {
-            this.sequelize.log('MSSQL does not support multiple cascade keys on the same reference, '
-              + 'we will remove them to make this work but we recommend restructuring your query.');
-            attributes[casKey].onDelete = '';
-            attributes[casKey].onUpdate = '';
-            attributes[casKey2].onDelete = '';
-            attributes[casKey2].onUpdate = '';
+            // NOTE: this really just disables cascading updates for all
+            //       definitions. Can be made more robust to support the
+            //       few cases where MSSQL actually supports them
+            attribute.onUpdate = '';
           }
+
         }
-      }
 
-      for (key in attributes) {
-        attribute = attributes[key];
         if (key && !attribute.field) attribute.field = key;
         result[attribute.field || key] = this.attributeToSQL(attribute, options);
       }

diff --git a/lib/dialects/mssql/query.js b/lib/dialects/mssql/query.js
@@ -167,6 +167,8 @@ module.exports = (function() {
     match = err.message.match(/Violation of UNIQUE KEY constraint '(.*)'. Cannot insert duplicate key in object '?(.*?)$/);
     match = match || err.message.match(/Cannot insert duplicate key row in object .* with unique index '(.*)'/);
     if (match && match.length > 1) {
+      console.log(err);
+
       return new sequelizeErrors.UniqueConstraintError({
         name: 'SequelizeUniqueConstraintError',
         fields: null,
@@ -179,13 +181,15 @@ module.exports = (function() {
     match = err.message.match(/Failed on step '(.*)'.Could not create constraint. See previous errors./);
     match = err.message.match(/The DELETE statement conflicted with the REFERENCE constraint "(.*)". The conflict occurred in database "(.*)", table "(.*)", column '(.*)'./);
     if (match && match.length > 0) {
+      console.log(err);
       return new sequelizeErrors.ForeignKeyConstraintError({
         fields: null,
         index: match[1],
         parent: err
       });
     }
 
+    console.log(err);
     return new sequelizeErrors.DatabaseError(err);
   };