Skip to content

Commit

Permalink
Merge pull request sequelize#1624 from seegno/fix-bulk-operations-result
Browse files Browse the repository at this point in the history 
Change bulkUpdate to return the affected rows
  • Loading branch information
@mickhansen
mickhansen committed Jun 25, 2014
2 parents ce35cca + 8198aa0 commit 0e53c4b
Show file tree
Hide file tree
Showing 9 changed files with 192 additions and 115 deletions.
27 changes: 8 additions & 19 deletions lib/data-types.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,13 @@ module.exports = {
UUIDV4: 'UUIDV4',

/**
* A virtual value that is not stored in the DB. This could for example be useful if you want to provide a default value in your model
* A key / value column. Only available in postgres.
* @property HSTORE
*/
HSTORE: 'HSTORE',

/**
* A virtual value that is not stored in the DB. This could for example be useful if you want to provide a default value in your model
* that is returned to the user but not stored in the DB.
*
* You could also use it to validate a value before permuting and storing it. Checking password length before hashing it for example:
Expand Down Expand Up @@ -453,22 +459,5 @@ module.exports = {
* An array of `type`, e.g. `DataTypes.ARRAY(DataTypes.DECIMAL)`. Only available in postgres.
* @property ARRAY
*/
ARRAY: function(type) { return type + '[]'; },

/**
* A key / value column. Only available in postgres.
* @property HSTORE
*/
get HSTORE() {
var result = function() {
return {
type: 'HSTORE'
};
};

result.type = 'HSTORE';
result.toString = result.valueOf = function() { return 'HSTORE'; };

return result;
}
ARRAY: function(type) { return type + '[]'; }
};
8 changes: 5 additions & 3 deletions lib/dialects/abstract/query-generator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,9 @@ module.exports = (function() {
/*
Returns an insert into command. Parameters: table name + hash of attribute-value-pairs.
*/
insertQuery: function(table, valueHash, modelAttributes) {
insertQuery: function(table, valueHash, modelAttributes, options) {
options = options || {};

var query
, valueQuery = 'INSERT INTO <%= table %> (<%= attributes %>) VALUES (<%= values %>)'
, emptyQuery = 'INSERT INTO <%= table %>'
Expand All @@ -168,7 +170,7 @@ module.exports = (function() {
emptyQuery += ' VALUES ()';
}

if (this._dialect.supports['RETURNING']) {
if (this._dialect.supports['RETURNING'] && options.returning) {
valueQuery += ' RETURNING *';
emptyQuery += ' RETURNING *';
}
Expand Down Expand Up @@ -235,7 +237,7 @@ module.exports = (function() {
query += ' LIMIT ' + this.escape(options.limit) + ' ';
}

if (this._dialect.supports['RETURNING'] && (options.returning || options.returning === undefined)) {
if (this._dialect.supports['RETURNING'] && options.returning) {
query += ' RETURNING *';
}

Expand Down
12 changes: 10 additions & 2 deletions lib/dialects/postgres/query-generator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,11 +255,17 @@ module.exports = (function() {
},

bulkInsertQuery: function(tableName, attrValueHashes, options, modelAttributes) {
var query = 'INSERT INTO <%= table %> (<%= attributes %>) VALUES <%= tuples %> RETURNING *;'
options = options || {};

var query = 'INSERT INTO <%= table %> (<%= attributes %>) VALUES <%= tuples %>'
, tuples = []
, serials = []
, allAttributes = [];

if (this._dialect.supports['RETURNING'] && options.returning) {
query += ' RETURNING *';
}

Utils._.forEach(attrValueHashes, function(attrValueHash) {
Utils._.forOwn(attrValueHash, function(value, key) {
if (allAttributes.indexOf(key) === -1) {
Expand Down Expand Up @@ -291,6 +297,8 @@ module.exports = (function() {
, tuples: tuples.join(',')
};

query = query + ';';

return Utils._.template(query)(replacements);
},

Expand Down Expand Up @@ -815,7 +823,7 @@ module.exports = (function() {
if (value && value._isSequelizeMethod) {
return value.toString(this);
} else {
if (field && field.type && field.type.toString() === DataTypes.HSTORE.type && Utils._.isObject(value)) {
if (Utils._.isObject(value) && field && (field === DataTypes.HSTORE || field.type === DataTypes.HSTORE)) {
value = hstore.stringify(value);
}

Expand Down
42 changes: 30 additions & 12 deletions lib/dialects/postgres/query.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ var Utils = require('../../utils')
, QueryTypes = require('../../query-types')
, Promise = require('../../promise');

// Parses hstore fields if the model has any hstore fields.
// This cannot be done in the 'pg' lib because hstore is a UDT.
var parseHstoreFields = function(model, row) {
Utils._.keys(row).forEach(function(key) {
if (model._isHstoreAttribute(key)) {
row[key] = hstore.parse(row[key]);
}
});
};

module.exports = (function() {
var Query = function(client, sequelize, callee, options) {
this.client = client;
Expand Down Expand Up @@ -125,36 +135,44 @@ module.exports = (function() {
});
}

// Parse hstore fields if the model has any hstore fields.
// This cannot be done in the 'pg' lib because hstore is a UDT.
if (!!self.callee && !!self.callee._hasHstoreAttributes) {
rows.forEach(function(row) {
Utils._.keys(row).forEach(function(key) {
if (self.callee._isHstoreAttribute(key)) {
row[key] = hstore.parse(row[key]);
}
});
parseHstoreFields(self.callee, row);
});
}

return self.send('handleSelectQuery', rows);
}
} else if (self.send('isShowOrDescribeQuery')) {
return results;
} else if ([QueryTypes.BULKUPDATE, QueryTypes.BULKDELETE].indexOf(self.options.type) !== -1) {
} else if (QueryTypes.BULKUPDATE === self.options.type) {
if (!self.options.returning) {
return result.rowCount;
}

if (!!self.callee && !!self.callee._hasHstoreAttributes) {
rows.forEach(function(row) {
parseHstoreFields(self.callee, row);
});
}

return self.send('handleSelectQuery', rows);
} else if (QueryTypes.BULKDELETE === self.options.type) {
return result.rowCount;
} else if (self.send('isInsertQuery') || self.send('isUpdateQuery')) {
if (self.callee !== null) { // may happen for bulk inserts or bulk updates
if (!!self.callee && self.callee.dataValues) {
if (!!self.callee.Model && !!self.callee.Model._hasHstoreAttributes) {
parseHstoreFields(self.callee.Model, rows[0]);
}

for (var key in rows[0]) {
if (rows[0].hasOwnProperty(key)) {
var record = rows[0][key];
if (!!self.callee.Model && !!self.callee.Model.rawAttributes && !!self.callee.Model.rawAttributes[key] && !!self.callee.Model.rawAttributes[key].type && self.callee.Model.rawAttributes[key].type.toString() === DataTypes.HSTORE.toString()) {
record = hstore.parse(record);
}

var attr = Utils._.find(self.callee.Model.rawAttributes, function (attribute) {
return attribute.fieldName === key || attribute.field === key;
});

self.callee.dataValues[attr && attr.fieldName || key] = record;
}
}
Expand Down
11 changes: 9 additions & 2 deletions lib/model.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,12 +262,13 @@ module.exports = (function() {
this.Instance.prototype.validators = {};

Utils._.each(this.rawAttributes, function(definition, name) {
var type = definition.originalType || definition.type || definition;
var type = definition.originalType || definition.type || definition;

if (type === DataTypes.BOOLEAN) {
self._booleanAttributes.push(name);
} else if (type === DataTypes.DATE) {
self._dateAttributes.push(name);
} else if (type === DataTypes.HSTORE.type) {
} else if (type === DataTypes.HSTORE) {
self._hstoreAttributes.push(name);
} else if (type === DataTypes.VIRTUAL) {
self._virtualAttributes.push(name);
Expand Down Expand Up @@ -1341,6 +1342,7 @@ module.exports = (function() {
* @param {Boolean} [options.validate=true] Should each row be subject to validation before it is inserted. The whole insert will fail if one row fails validation
* @param {Boolean} [options.hooks=true] Run before / after bulk update hooks?
* @param {Boolean} [options.individualHooks=false] Run before / after update hooks?
* @param {Boolean} [options.returning=false] Return the affected rows (only for postgres)
* @param {Number} [options.limit] How many rows to update (only for mysql and mariadb)
* @deprecated The syntax is due for change, in order to make `where` more consistent with the rest of the API
*
Expand All @@ -1353,6 +1355,7 @@ module.exports = (function() {
validate: true,
hooks: true,
individualHooks: false,
returning: false,
force: false
}, options || {});

Expand Down Expand Up @@ -1458,6 +1461,10 @@ module.exports = (function() {

// Run query to update all rows
return self.QueryInterface.bulkUpdate(self.getTableName(), attrValueHashUse, where, options, self.tableAttributes).then(function(affectedRows) {
if (options.returning) {
return [affectedRows.length, affectedRows];
}

return [affectedRows];
});
}).tap(function(result) {
Expand Down
8 changes: 5 additions & 3 deletions lib/query-interface.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ module.exports = (function() {
};

QueryInterface.prototype.insert = function(dao, tableName, values, options) {
var sql = this.QueryGenerator.insertQuery(tableName, values, dao.Model.rawAttributes);
var sql = this.QueryGenerator.insertQuery(tableName, values, dao.Model.rawAttributes, options);

return this.sequelize.query(sql, dao, options).then(function(result) {
result.isNewRecord = false;
Expand Down Expand Up @@ -448,9 +448,11 @@ module.exports = (function() {
};

QueryInterface.prototype.bulkUpdate = function(tableName, values, identifier, options, attributes) {
var sql = this.QueryGenerator.updateQuery(tableName, values, identifier, options, attributes);
var sql = this.QueryGenerator.updateQuery(tableName, values, identifier, options, attributes)
, table = Utils._.isObject(tableName) ? tableName : { tableName: tableName }
, daoTable = Utils._.find(this.sequelize.daoFactoryManager.daos, { tableName: table.tableName });

return this.sequelize.query(sql, null, options);
return this.sequelize.query(sql, daoTable, options);
};

QueryInterface.prototype.delete = function(dao, tableName, identifier, options) {
Expand Down
26 changes: 26 additions & 0 deletions test/dao-factory.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -805,6 +805,32 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
})
})

if (dialect === "postgres") {
it('returns the affected rows if `options.returning` is true', function(_done) {
var self = this
, data = [{ username: 'Peter', secretValue: '42' },
{ username: 'Paul', secretValue: '42' },
{ username: 'Bob', secretValue: '43' }]
, done = _.after(2, _done)

this.User.bulkCreate(data).success(function() {
self.User.update({ username: 'Bill' }, { secretValue: '42' }, { returning: true }).spread(function(count, rows) {
expect(count).to.equal(2)
expect(rows).to.have.length(2)

done()
})

self.User.update({ username: 'Bill'}, { secretValue: '44' }, { returning: true }).spread(function(count, rows) {
expect(count).to.equal(0)
expect(rows).to.have.length(0)

done()
})
})
})
}

if(Support.dialectIsMySQL()) {
it('supports limit clause', function (done) {
var self = this
Expand Down
18 changes: 17 additions & 1 deletion test/postgres/dao.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ if (dialect.match(/^postgres/)) {
.create({ username: 'user', email: ['[email protected]'], settings: { created: { test: '"value"' }}})
.success(function(newUser) {
// Check to see if the default value for an hstore field works
expect(newUser.document).to.deep.equal({default: 'value'})
expect(newUser.document).to.deep.equal({ default: 'value' })
expect(newUser.settings).to.deep.equal({ created: { test: '"value"' }})

// Check to see if updating an hstore field works
Expand Down Expand Up @@ -295,6 +295,22 @@ if (dialect.match(/^postgres/)) {
.error(console.log)
})

it("should update hstore correctly and return the affected rows", function(done) {
var self = this

this.User
.create({ username: 'user', email: ['[email protected]'], settings: { created: { test: '"value"' }}})
.success(function(oldUser) {
// Update the user and check that the returned object's fields have been parsed by the hstore library
self.User.update({settings: {should: 'update', to: 'this', first: 'place'}}, oldUser.identifiers, { returning: true }).spread(function(count, users) {
expect(count).to.equal(1);
expect(users[0].settings).to.deep.equal({should: 'update', to: 'this', first: 'place'})
done()
})
})
.error(console.log)
})

it("should read hstore correctly", function(done) {
var self = this
var data = { username: 'user', email: ['[email protected]'], settings: { created: { test: '"value"' }}}
Expand Down
Loading

0 comments on commit 0e53c4b

Please sign in to comment.