fix(indexes): fix composite index issue (#7854)

lib/dialects/mssql/query-generator.js

@@ -322,7 +322,7 @@ const QueryGenerator = {
     return sql;
   },
 
-  upsertQuery(tableName, insertValues, updateValues, where, rawAttributes) {
+  upsertQuery(tableName, insertValues, updateValues, where, model) {
     const targetTableAlias = this.quoteTable(`${tableName}_target`);
     const sourceTableAlias = this.quoteTable(`${tableName}_source`);
     const primaryKeysAttrs = [];
@@ -333,15 +333,27 @@ const QueryGenerator = {
 
 
     //Obtain primaryKeys, uniquekeys and identity attrs from rawAttributes as model is not passed
-    for (const key in rawAttributes) {
-      if (rawAttributes[key].primaryKey) {
-        primaryKeysAttrs.push(rawAttributes[key].field || key);
+    for (const key in model.rawAttributes) {
+      if (model.rawAttributes[key].primaryKey) {
+        primaryKeysAttrs.push(model.rawAttributes[key].field || key);
       }
-      if (rawAttributes[key].unique) {
-        uniqueAttrs.push(rawAttributes[key].field || key);
+      if (model.rawAttributes[key].unique) {
+        uniqueAttrs.push(model.rawAttributes[key].field || key);
       }
-      if (rawAttributes[key].autoIncrement) {
-        identityAttrs.push(rawAttributes[key].field || key);
+      if (model.rawAttributes[key].autoIncrement) {
+        identityAttrs.push(model.rawAttributes[key].field || key);
+      }
+    }
+
+    //Add unique indexes defined by indexes option to uniqueAttrs
+    for (const index of model.options.indexes) {
+      if (index.unique && index.fields) {
+        for (const field of index.fields) {
+          const fieldName = typeof field === 'string' ? field : field.name || field.attribute;
+          if (uniqueAttrs.indexOf(fieldName) === -1 && model.rawAttributes[fieldName]) {
+            uniqueAttrs.push(fieldName);
+          }
+        }
       }
     }
 

lib/dialects/mysql/query-generator.js

@@ -150,7 +150,7 @@ const QueryGenerator = {
     return `ALTER TABLE ${this.quoteTable(tableName)} CHANGE ${attrString.join(', ')};`;
   },
 
-  upsertQuery(tableName, insertValues, updateValues, where, rawAttributes, options) {
+  upsertQuery(tableName, insertValues, updateValues, where, model, options) {
     options.onDuplicate = 'UPDATE ';
 
     options.onDuplicate += Object.keys(updateValues).map(key => {
@@ -158,7 +158,7 @@ const QueryGenerator = {
       return key + '=VALUES(' + key +')';
     }).join(', ');
 
-    return this.insertQuery(tableName, insertValues, rawAttributes, options);
+    return this.insertQuery(tableName, insertValues, model.rawAttributes, options);
   },
 
   deleteQuery(tableName, where, options) {

lib/dialects/postgres/query-generator.js

@@ -355,9 +355,9 @@ const QueryGenerator = {
     return this.fn(fnName, tableName, body, returns, language);
   },
 
-  upsertQuery(tableName, insertValues, updateValues, where, rawAttributes, options) {
-    const insert = this.insertQuery(tableName, insertValues, rawAttributes, options);
-    const update = this.updateQuery(tableName, updateValues, where, options, rawAttributes);
+  upsertQuery(tableName, insertValues, updateValues, where, model, options) {
+    const insert = this.insertQuery(tableName, insertValues, model.rawAttributes, options);
+    const update = this.updateQuery(tableName, updateValues, where, options, model.rawAttributes);
 
     // The numbers here are selected to match the number of affected rows returned by MySQL
     return this.exceptionFn(

lib/dialects/sqlite/query-generator.js

@@ -208,10 +208,10 @@ const QueryGenerator = {
     return "SELECT name FROM `sqlite_master` WHERE type='table' and name!='sqlite_sequence';";
   },
 
-  upsertQuery(tableName, insertValues, updateValues, where, rawAttributes, options) {
+  upsertQuery(tableName, insertValues, updateValues, where, model, options) {
     options.ignoreDuplicates = true;
 
-    const sql = this.insertQuery(tableName, insertValues, rawAttributes, options) + ' ' + this.updateQuery(tableName, updateValues, where, options, rawAttributes);
+    const sql = this.insertQuery(tableName, insertValues, model.rawAttributes, options) + ' ' + this.updateQuery(tableName, updateValues, where, options, model.rawAttributes);
 
     return sql;
   },

lib/model.js

@@ -843,20 +843,7 @@ class Model {
       }
     });
 
-    this.options.indexes = this.options.indexes.map(index => {
-      index = this._conformIndex(index);
-      //Add unique indexes to rawAttributes
-      if (index.unique && index.fields) {
-        for (const field of index.fields) {
-          const fieldName = typeof field === 'string' ? field : field.name || field.attribute;
-          if (this.rawAttributes[fieldName]) {
-            this.rawAttributes[fieldName].unique = this.attributes[fieldName].unique = index.name || index.unique;
-          }
-        }
-      }
-
-      return index;
-    });
+    this.options.indexes = this.options.indexes.map(this._conformIndex);
 
     this.sequelize.modelManager.addModel(this);
 

lib/query-interface.js

@@ -572,7 +572,7 @@ class QueryInterface {
     options.type = QueryTypes.UPSERT;
     options.raw = true;
 
-    const sql = this.QueryGenerator.upsertQuery(tableName, valuesByField, updateValues, where, model.rawAttributes, options);
+    const sql = this.QueryGenerator.upsertQuery(tableName, valuesByField, updateValues, where, model, options);
     return this.sequelize.query(sql, options).then(rowCount => {
       if (rowCount === undefined) {
         return rowCount;

test/integration/model/sync.test.js

@@ -79,5 +79,37 @@ describe(Support.getTestDialectTeaser('Model'), () => {
           expect(data.dataValues.age).to.eql('1');
         });
     });
+
+    it('should properly create composite index without affecting individual fields', function() {
+      const testSync = this.sequelize.define('testSync', {
+        name: Sequelize.STRING,
+        age: Sequelize.STRING
+      }, {indexes: [{unique: true, fields: ['name', 'age']}]});
+      return this.sequelize.sync()
+        .then(() => testSync.create({name: 'test'}))
+        .then(() => testSync.create({name: 'test2'}))
+        .then(() => testSync.create({name: 'test3'}))
+        .then(() => testSync.create({age: '1'}))
+        .then(() => testSync.create({age: '2'}))
+        .then(() => testSync.create({name: 'test', age: '1'}))
+        .then(() => testSync.create({name: 'test', age: '2'}))
+        .then(() => testSync.create({name: 'test2', age: '2'}))
+        .then(() => testSync.create({name: 'test3', age: '2'}))
+        .then(() => testSync.create({name: 'test3', age: '1'}))
+        .then(data => {
+          expect(data.dataValues.name).to.eql('test3');
+          expect(data.dataValues.age).to.eql('1');
+        });
+    });
+    it('should properly create composite index that fails on constraint violation', function() {
+      const testSync = this.sequelize.define('testSync', {
+        name: Sequelize.STRING,
+        age: Sequelize.STRING
+      }, {indexes: [{unique: true, fields: ['name', 'age']}]});
+      return this.sequelize.sync()
+        .then(() => testSync.create({name: 'test', age: '1'}))
+        .then(() => testSync.create({name: 'test', age: '1'}))
+        .then(data => expect(data).not.to.be.ok, error => expect(error).to.be.ok);
+    });
   });
 });

test/unit/model/indexes.test.js

@@ -39,7 +39,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
       expect(Model.options.indexes[1].unique).to.eql(true);
     });
 
-    it('should set rawAttributes when indexes are defined via options', () => {
+    it('should not set rawAttributes when indexes are defined via options', () => {
       const User = current.define('User', {
         username: DataTypes.STRING
       }, {
@@ -49,11 +49,10 @@ describe(Support.getTestDialectTeaser('Model'), () => {
         }]
       });
 
-      expect(User.rawAttributes.username).to.have.property('unique');
-      expect(User.rawAttributes.username.unique).to.be.true;
+      expect(User.rawAttributes.username.unique).to.be.undefined;
     });
 
-    it('should set rawAttributes when composite unique indexes are defined via options', () => {
+    it('should not set rawAttributes when composite unique indexes are defined via options', () => {
       const User = current.define('User', {
         name: DataTypes.STRING,
         address: DataTypes.STRING
@@ -64,11 +63,8 @@ describe(Support.getTestDialectTeaser('Model'), () => {
         }]
       });
 
-      expect(User.rawAttributes.name).to.have.property('unique');
-      expect(User.rawAttributes.name.unique).to.be.equal('users_name_address');
-
-      expect(User.rawAttributes.address).to.have.property('unique');
-      expect(User.rawAttributes.address.unique).to.be.equal('users_name_address');
+      expect(User.rawAttributes.name.unique).to.be.undefined;
+      expect(User.rawAttributes.address.unique).to.be.undefined;
     });
   });
 });