fix(bulkCreate): updateOnDuplicate doesn't map back to fields (#9162)

lib/dialects/abstract/query-generator.js

@@ -217,9 +217,9 @@ const QueryGenerator = {
     Parameters: table name + list of hashes of attribute-value-pairs.
    @private
   */
-  bulkInsertQuery(tableName, attrValueHashes, options, rawAttributes) {
+  bulkInsertQuery(tableName, fieldValueHashes, options, fieldMappedAttributes) {
     options = options || {};
-    rawAttributes = rawAttributes || {};
+    fieldMappedAttributes = fieldMappedAttributes || {};
 
     const query = 'INSERT<%= ignoreDuplicates %> INTO <%= table %> (<%= attributes %>) VALUES <%= tuples %><%= onDuplicateKeyUpdate %><%= returning %>;';
     const tuples = [];
@@ -227,31 +227,38 @@ const QueryGenerator = {
     const allAttributes = [];
     let onDuplicateKeyUpdate = '';
 
-    for (const attrValueHash of attrValueHashes) {
-      _.forOwn(attrValueHash, (value, key) => {
+    for (const fieldValueHash of fieldValueHashes) {
+      _.forOwn(fieldValueHash, (value, key) => {
         if (allAttributes.indexOf(key) === -1) {
           allAttributes.push(key);
         }
-
-        if (rawAttributes[key] && rawAttributes[key].autoIncrement === true) {
+        if (
+          fieldMappedAttributes[key]
+          && fieldMappedAttributes[key].autoIncrement === true
+        ) {
           serials[key] = true;
         }
       });
     }
 
-    for (const attrValueHash of attrValueHashes) {
-      tuples.push('(' + allAttributes.map(key => {
-        if (this._dialect.supports.bulkDefault && serials[key] === true) {
-          return attrValueHash[key] || 'DEFAULT';
+    for (const fieldValueHash of fieldValueHashes) {
+      const values = allAttributes.map(key => {
+        if (
+          this._dialect.supports.bulkDefault
+          && serials[key] === true
+        ) {
+          return fieldValueHash[key] || 'DEFAULT';
         }
-        return this.escape(attrValueHash[key], rawAttributes[key], { context: 'INSERT' });
-      }).join(',') + ')');
+
+        return this.escape(fieldValueHash[key], fieldMappedAttributes[key], { context: 'INSERT' });
+      });
+
+      tuples.push(`(${values.join(',')})`);
     }
 
     if (this._dialect.supports.updateOnDuplicate && options.updateOnDuplicate) {
-      onDuplicateKeyUpdate += ' ON DUPLICATE KEY UPDATE ' + options.updateOnDuplicate.map(attr => {
-        const field = rawAttributes && rawAttributes[attr] && rawAttributes[attr].field || attr;
-        const key = this.quoteIdentifier(field);
+      onDuplicateKeyUpdate = ' ON DUPLICATE KEY UPDATE ' + options.updateOnDuplicate.map(attr => {
+        const key = this.quoteIdentifier(attr);
         return key + '=VALUES(' + key + ')';
       }).join(',');
     }

lib/model.js

@@ -2345,11 +2345,11 @@ class Model {
 
     if (options.updateOnDuplicate) {
       // By default, all attributes except 'createdAt' can be updated
-      let updatableFields = _.pull(Object.keys(this.tableAttributes), 'createdAt');
+      let updatableAttributes = _.pull(Object.keys(this.tableAttributes), 'createdAt');
       if (_.isArray(options.updateOnDuplicate) && !_.isEmpty(options.updateOnDuplicate)) {
-        updatableFields = _.intersection(updatableFields, options.updateOnDuplicate);
+        updatableAttributes = _.intersection(updatableAttributes, options.updateOnDuplicate);
       }
-      options.updateOnDuplicate = updatableFields;
+      options.updateOnDuplicate = updatableAttributes;
     }
 
     options.model = this;
@@ -2421,13 +2421,18 @@ class Model {
           return _.omit(instance.dataValues, this._virtualAttributes);
         });
 
-        // Map attributes for serial identification
-        const attributes = {};
+        // Map attributes to fields for serial identification
+        const fieldMappedAttributes = {};
         for (const attr in this.tableAttributes) {
-          attributes[this.rawAttributes[attr].field] = this.rawAttributes[attr];
+          fieldMappedAttributes[this.rawAttributes[attr].field || attr] = this.rawAttributes[attr];
         }
 
-        return this.QueryInterface.bulkInsert(this.getTableName(options), records, options, attributes).then(results => {
+        // Map updateOnDuplicate attributes to fields
+        if (options.updateOnDuplicate) {
+          options.updateOnDuplicate = options.updateOnDuplicate.map(attr => this.rawAttributes[attr].field || attr);
+        }
+
+        return this.QueryInterface.bulkInsert(this.getTableName(options), records, options, fieldMappedAttributes).then(results => {
           if (Array.isArray(results)) {
             results.forEach((result, i) => {
               if (instances[i] && !instances[i].get(this.primaryKeyAttribute)) {

lib/query-interface.js

@@ -1001,14 +1001,19 @@ class QueryInterface {
    *
    * @param {String} tableName             Table name to insert record to
    * @param {Array}  records               List of records to insert
+   * @param {Object} options               Various options, please see Model.bulkCreate options
+   * @param {Object} fieldMappedAttributes Various attributes mapped by field name
    *
    * @return {Promise}
    */
   bulkInsert(tableName, records, options, attributes) {
     options = _.clone(options) || {};
     options.type = QueryTypes.INSERT;
-    const sql = this.QueryGenerator.bulkInsertQuery(tableName, records, options, attributes);
-    return this.sequelize.query(sql, options).then(results => results[0]);
+
+    return this.sequelize.query(
+      this.QueryGenerator.bulkInsertQuery(tableName, records, options, attributes),
+      options
+    ).then(results => results[0]);
   }
 
   update(instance, tableName, values, identifier, options) {

test/integration/model/bulk-create.test.js

@@ -17,7 +17,10 @@ describe(Support.getTestDialectTeaser('Model'), () => {
 
       this.User = this.sequelize.define('User', {
         username: DataTypes.STRING,
-        secretValue: DataTypes.STRING,
+        secretValue: {
+          type: DataTypes.STRING,
+          field: 'secret_value'
+        },
         data: DataTypes.STRING,
         intVal: DataTypes.INTEGER,
         theDate: DataTypes.DATE,

test/unit/sql/insert.test.js

@@ -83,11 +83,6 @@ describe(Support.getTestDialectTeaser('SQL'), () => {
 
   describe('bulkCreate', () => {
     it('bulk create with onDuplicateKeyUpdate', () => {
-      // Skip mssql for now, it seems broken
-      if (Support.getTestDialect() === 'mssql') {
-        return;
-      }
-
       const User = Support.sequelize.define('user', {
         username: {
           type: DataTypes.STRING,
@@ -109,10 +104,11 @@ describe(Support.getTestDialectTeaser('SQL'), () => {
         timestamps: true
       });
 
-      expectsql(sql.bulkInsertQuery(User.tableName, [{ user_name: 'testuser', pass_word: '12345' }], { updateOnDuplicate: ['username', 'password', 'updatedAt'] }, User.rawAttributes),
+      expectsql(sql.bulkInsertQuery(User.tableName, [{ user_name: 'testuser', pass_word: '12345' }], { updateOnDuplicate: ['user_name', 'pass_word', 'updated_at'] }, User.fieldRawAttributesMap),
         {
           default: 'INSERT INTO `users` (`user_name`,`pass_word`) VALUES (\'testuser\',\'12345\');',
           postgres: 'INSERT INTO "users" ("user_name","pass_word") VALUES (\'testuser\',\'12345\');',
+          mssql: 'INSERT INTO [users] ([user_name],[pass_word]) VALUES (N\'testuser\',N\'12345\');',
           mysql: 'INSERT INTO `users` (`user_name`,`pass_word`) VALUES (\'testuser\',\'12345\') ON DUPLICATE KEY UPDATE `user_name`=VALUES(`user_name`),`pass_word`=VALUES(`pass_word`),`updated_at`=VALUES(`updated_at`);'
         });
     });