refactor support for return values on insert/update

The dialect support for return values has been refactored
to make detecting it more readable for the two dialects
that currently support it. Tests have also been added for
regular creates as well as bulkCreates for returning these
values

lib/dialects/abstract/index.js

@@ -9,12 +9,13 @@ AbstractDialect.prototype.supports = {
   'DEFAULT VALUES': false,
   'VALUES ()': false,
   'LIMIT ON UPDATE': false,
+  'ON DUPLICATE KEY': true,
 
-  /* does the dialect support returning values for inserted/updated fields using RETURNING */
-  'RETURNING': false,
+  /* What is the dialect's keyword for INSERT IGNORE */
+  'IGNORE': '',
 
-  /* does the dialect support returning values for inserted/updated fields using OUTPUT */
-  'OUTPUT': false,
+  /* does the dialect support returning values for inserted/updated fields */
+  returnValues: false,
 
   /* features specific to autoIncrement values */
   autoIncrement: {
@@ -27,10 +28,7 @@ AbstractDialect.prototype.supports = {
     /* does the dialect support updating autoincrement fields */
     update: true
   },
-  'ON DUPLICATE KEY': true,
 
-  /* What is the dialect's keyword for INSERT IGNORE */
-  'IGNORE': '',
   schemas: false,
   transactions: true,
   migrations: true,

lib/dialects/abstract/query-generator.js

@@ -190,13 +190,13 @@ module.exports = (function() {
         emptyQuery += ' VALUES ()';
       }
 
-      // FIXME: ideally these two can be merged in the future, the only
-      //        difference is placement of the value in the query
-      if (this._dialect.supports['RETURNING'] && options.returning) {
-        valueQuery += ' RETURNING *';
-        emptyQuery += ' RETURNING *';
-      } else if (this._dialect.supports['OUTPUT'] && options.returning) {
-        outputFragment = ' OUTPUT INSERTED.*';
+      if (this._dialect.supports.returnValues && options.returning) {
+        if (!!this._dialect.supports.returnValues.returning) {
+          valueQuery += ' RETURNING *';
+          emptyQuery += ' RETURNING *';
+        } else if (!!this._dialect.supports.returnValues.output) {
+          outputFragment = ' OUTPUT INSERTED.*';
+        }
       }
 
       if (this._dialect.supports['EXCEPTION'] && options.exception) {
@@ -303,10 +303,13 @@ module.exports = (function() {
         query += ' LIMIT ' + this.escape(options.limit) + ' ';
       }
 
-      if (this._dialect.supports['RETURNING'] && options.returning) {
-        query += ' RETURNING *';
-      } else if (this._dialect.supports['OUTPUT']) {
-        outputFragment = ' OUTPUT INSERTED.*';
+      if (this._dialect.supports.returnValues) {
+        if (!!this._dialect.supports.returnValues.output) {
+          // we always need this for mssql
+          outputFragment = ' OUTPUT INSERTED.*';
+        } else if (this._dialect.supports.returnValues && options.returning) {
+          query += ' RETURNING *';
+        }
       }
 
       if (attributes) {
@@ -388,11 +391,16 @@ module.exports = (function() {
       var query
         , key
         , value
-        , values = [];
+        , values = []
+        , outputFragment;
 
-      query = 'UPDATE <%= table %> SET <%= values %> WHERE <%= where %>';
-      if (this._dialect.supports['RETURNING']) {
-        query += ' RETURNING *';
+      query = 'UPDATE <%= table %> SET <%= values %><%= output %> WHERE <%= where %>';
+      if (this._dialect.supports.returnValues) {
+        if (!!this._dialect.supports.returnValues.returning) {
+          query += ' RETURNING *';
+        } else if (!!this._dialect.supports.returnValues.output) {
+          outputFragment = ' OUTPUT INSERTED.*';
+        }
       }
 
       for (key in attrValueHash) {
@@ -409,6 +417,7 @@ module.exports = (function() {
       var replacements = {
         table: this.quoteTable(tableName),
         values: values.join(','),
+        output: outputFragment,
         where: this.getWhereConditions(where)
       };
 

lib/dialects/mssql/index.js

@@ -12,8 +12,6 @@ var MssqlDialect = function(sequelize) {
 };
 
 MssqlDialect.prototype.supports = _.merge(_.cloneDeep(Abstract.prototype.supports), {
-  'RETURNING': false,
-  'OUTPUT': true,
   'DEFAULT': true,
   'DEFAULT VALUES': true,
   'LIMIT ON UPDATE': true,
@@ -21,6 +19,9 @@ MssqlDialect.prototype.supports = _.merge(_.cloneDeep(Abstract.prototype.support
   transactions: false,
   migrations: false,
   upserts: false,
+  returnValues: {
+    output: true
+  },
   autoIncrement: {
     identityInsert: true,
     defaultValue: false,

lib/dialects/mssql/query-generator.js

@@ -196,12 +196,17 @@ module.exports = (function() {
     },
 
     bulkInsertQuery: function(tableName, attrValueHashes, options, attributes) {
-      var query = 'INSERT INTO <%= table %> (<%= attributes %>) VALUES <%= tuples %>;'
-        , emptyQuery = 'INSERT INTO <%= table %> DEFAULT VALUES'
+      var query = 'INSERT INTO <%= table %> (<%= attributes %>)<%= output %> VALUES <%= tuples %>;'
+        , emptyQuery = 'INSERT INTO <%= table %><%= output %> DEFAULT VALUES'
         , tuples = []
         , allAttributes = []
         , needIdentityInsertWrapper = false
-        , allQueries = [];
+        , allQueries = []
+        , outputFragment;
+
+      if (options.returning) {
+        outputFragment = ' OUTPUT INSERTED.*';
+      }
 
       Utils._.forEach(attrValueHashes, function(attrValueHash, i) {
         // special case for empty objects with primary keys
@@ -243,7 +248,8 @@ module.exports = (function() {
         attributes: allAttributes.map(function(attr) {
                       return this.quoteIdentifier(attr);
                     }.bind(this)).join(','),
-        tuples: tuples
+        tuples: tuples,
+        output: outputFragment
       };
 
       var generatedQuery = Utils._.template(allQueries.join(';'))(replacements);

lib/dialects/mssql/query.js

@@ -114,12 +114,16 @@ module.exports = (function() {
       this.handleInsertQuery(data);
 
       if (!this.callee) {
-        // NOTE: super contrived. This just passes the newly added query-interface
-        //       test returning only the PK. There isn't a way in MSSQL to identify
-        //       that a given return value is the PK, and we have no schema information
-        //       because there was no calling Model.
-        var record = data[0];
-        result = record[Object.keys(record)[0]];
+        if (this.options.plain) {
+          // NOTE: super contrived. This just passes the newly added query-interface
+          //       test returning only the PK. There isn't a way in MSSQL to identify
+          //       that a given return value is the PK, and we have no schema information
+          //       because there was no calling Model.
+          var record = data[0];
+          result = record[Object.keys(record)[0]];
+        } else {
+          result = data;
+        }
       }
     }
 

lib/dialects/postgres/index.js

@@ -12,10 +12,12 @@ var PostgresDialect = function(sequelize) {
 };
 
 PostgresDialect.prototype.supports = _.merge(_.cloneDeep(Abstract.prototype.supports), {
-  'RETURNING': true,
   'DEFAULT VALUES': true,
   'EXCEPTION': true,
   'ON DUPLICATE KEY': false,
+  returnValues: {
+    returning: true
+  },
   schemas: true,
   lock: true,
   forShare: 'FOR SHARE',

lib/dialects/postgres/query-generator.js

@@ -354,7 +354,7 @@ module.exports = (function() {
         , serials = []
         , allAttributes = [];
 
-      if (this._dialect.supports['RETURNING'] && options.returning) {
+      if (options.returning) {
         query += ' RETURNING *';
       }
 

lib/query-interface.js

@@ -459,7 +459,7 @@ module.exports = (function() {
   QueryInterface.prototype.insert = function(dao, tableName, values, options) {
     var sql = this.QueryGenerator.insertQuery(tableName, values, dao && dao.Model.rawAttributes, options);
 
-    options.type = 'INSERT';
+    options.type = QueryTypes.INSERT;
     return this.sequelize.query(sql, dao, options).then(function(result) {
       if (dao) result.isNewRecord = false;
       return result;
@@ -536,6 +536,7 @@ module.exports = (function() {
   };
 
   QueryInterface.prototype.bulkInsert = function(tableName, records, options, attributes) {
+    options.type = QueryTypes.INSERT;
     var sql = this.QueryGenerator.bulkInsertQuery(tableName, records, options, attributes);
     return this.sequelize.query(sql, null, options);
   };

test/model/create.test.js

@@ -449,27 +449,20 @@ describe(Support.getTestDialectTeaser('Model'), function() {
       });
     }
 
-    if (current.dialect.supports['RETURNING']) {
-      describe('Autoincrement values', function () {
+    if (current.dialect.supports.returnValues) {
+      describe('return values', function () {
         it('should make the autoincremented values available on the returned instances', function () {
           var User = this.sequelize.define('user', {});
 
           return User.sync({force: true}).then(function () {
-            return User.bulkCreate([
-              {},
-              {},
-              {}
-            ], {returning: true}).then(function (users) {
-              expect(users.length).to.be.ok;
-              users.forEach(function (user, i) {
-                expect(user.get('id')).to.be.ok;
-                expect(user.get('id')).to.equal(i+1);
-              });
+            return User.create({}, {returning: true}).then(function (user) {
+              expect(user.get('id')).to.be.ok;
+              expect(user.get('id')).to.equal(1);
             });
           });
         });
 
-        it('should make the autoincremented values available on the returned instances', function () {
+        it('should make the autoincremented values available on the returned instances with custom fields', function () {
           var User = this.sequelize.define('user', {
             maId: {
               type: DataTypes.INTEGER,
@@ -480,16 +473,9 @@ describe(Support.getTestDialectTeaser('Model'), function() {
           });
 
           return User.sync({force: true}).then(function () {
-            return User.bulkCreate([
-              {},
-              {},
-              {}
-            ], {returning: true}).then(function (users) {
-              expect(users.length).to.be.ok;
-              users.forEach(function (user, i) {
-                expect(user.get('maId')).to.be.ok;
-                expect(user.get('maId')).to.equal(i+1);
-              });
+            return User.create({}, {returning: true}).then(function (user) {
+              expect(user.get('maId')).to.be.ok;
+              expect(user.get('maId')).to.equal(1);
             });
           });
         });
@@ -1506,6 +1492,53 @@ describe(Support.getTestDialectTeaser('Model'), function() {
       });
     }
 
+    if (current.dialect.supports.returnValues) {
+      describe('return values', function () {
+        it('should make the autoincremented values available on the returned instances', function () {
+          var User = this.sequelize.define('user', {});
+
+          return User.sync({force: true}).then(function () {
+            return User.bulkCreate([
+              {},
+              {},
+              {}
+            ], {returning: true}).then(function (users) {
+              expect(users.length).to.be.ok;
+              users.forEach(function (user, i) {
+                expect(user.get('id')).to.be.ok;
+                expect(user.get('id')).to.equal(i+1);
+              });
+            });
+          });
+        });
+
+        it('should make the autoincremented values available on the returned instances with custom fields', function () {
+          var User = this.sequelize.define('user', {
+            maId: {
+              type: DataTypes.INTEGER,
+              primaryKey: true,
+              autoIncrement: true,
+              field: 'yo_id'
+            }
+          });
+
+          return User.sync({force: true}).then(function () {
+            return User.bulkCreate([
+              {},
+              {},
+              {}
+            ], {returning: true}).then(function (users) {
+              expect(users.length).to.be.ok;
+              users.forEach(function (user, i) {
+                expect(user.get('maId')).to.be.ok;
+                expect(user.get('maId')).to.equal(i+1);
+              });
+            });
+          });
+        });
+      });
+    }
+
     describe('enums', function() {
       it('correctly restores enum values', function(done) {
         var self = this