Skip to content

public / sequelize

Go to a project

不要怂，就是干，撸起袖子干！

Commit 37edaeaa by Mick Hansen
Options

Merge pull request #1322 from sequelize/findAndCountAll-limit-1-fix 

Fix bug with limit: 1 and findAndCountAll - Fixes #1281
2 parents 65a1407a 844657a0
Inline Side-by-side
Showing with 118 additions and 40 deletions
lib/dao-factory.js
...@@ -545,7 +545,9 @@ module.exports = (function() { ...@@ -545,7 +545,9 @@ module.exports = (function() {
} }
options = paranoidClause.call(this, options) options = paranoidClause.call(this, options)
options.limit = 1 if (options.limit === undefined) {
options.limit = 1
}
return this.QueryInterface.select(this, this.getTableName(), options, Utils._.defaults({ return this.QueryInterface.select(this, this.getTableName(), options, Utils._.defaults({
plain: true, plain: true,
...@@ -582,11 +584,19 @@ module.exports = (function() { ...@@ -582,11 +584,19 @@ module.exports = (function() {
DAOFactory.prototype.count = function(options) { DAOFactory.prototype.count = function(options) {
options = Utils._.clone(options || {}) options = Utils._.clone(options || {})
return new Utils.CustomEventEmitter(function (emitter) { return new Utils.CustomEventEmitter(function (emitter) {
var col = this.sequelize.col('*')
if (options.include) {
col = this.sequelize.col(this.tableName+'.'+(this.primaryKeyAttributes[0] || 'id'))
}
options.attributes = [ options.attributes = [
[this.sequelize.fn('COUNT', this.sequelize.col(this.tableName+'.*')), 'count'] [this.sequelize.fn('COUNT', col), 'count']
] ]
options.includeIgnoreAttributes = false options.includeIgnoreAttributes = false
options.limit = null
this.find(options, {raw: true, transaction: options.transaction}).proxy(emitter, {events: ['sql', 'error']}).success(function (result) { this.find(options, {raw: true, transaction: options.transaction}).proxy(emitter, {events: ['sql', 'error']}).success(function (result) {
emitter.emit('success', parseInt(result.count, 10)) emitter.emit('success', parseInt(result.count, 10))
...@@ -610,18 +620,18 @@ module.exports = (function() { ...@@ -610,18 +620,18 @@ module.exports = (function() {
} }
self.count(countOptions) self.count(countOptions)
.proxy(emitter, {events: ['sql', 'error']}) .proxy(emitter, {events: ['sql', 'error']})
.success(function(count) { .success(function(count) {
if (count === 0) { if (count === 0) {
return emit.okay(count) // no records, no need for another query return emit.okay(count) // no records, no need for another query
} }
self.findAll(findOptions, queryOptions) self.findAll(findOptions, queryOptions)
.proxy(emitter, {events: ['sql', 'error']}) .proxy(emitter, {events: ['sql', 'error']})
.success(function(results) { .success(function(results) {
emit.okay(count, results) emit.okay(count, results)
}) })
}) })
}).run() }).run()
} }
...@@ -1325,8 +1335,8 @@ module.exports = (function() { ...@@ -1325,8 +1335,8 @@ module.exports = (function() {
options.includeMap[include.as] = include options.includeMap[include.as] = include
options.includeNames.push(include.as) options.includeNames.push(include.as)
if (include.association.isMultiAssociation) options.hasMultiAssociation = true if (include.association.isMultiAssociation || include.hasMultiAssociation) options.hasMultiAssociation = true
if (include.association.isSingleAssociation) options.hasSingleAssociation = true if (include.association.isSingleAssociation || include.hasSingleAssociation) options.hasSingleAssociation = true
options.hasIncludeWhere = options.hasIncludeWhere || include.hasIncludeWhere || !!include.where options.hasIncludeWhere = options.hasIncludeWhere || include.hasIncludeWhere || !!include.where
options.hasIncludeRequired = options.hasIncludeRequired || include.hasIncludeRequired || !!include.required options.hasIncludeRequired = options.hasIncludeRequired || include.hasIncludeRequired || !!include.required
......
lib/dialects/abstract/query-generator.js
...@@ -473,8 +473,8 @@ module.exports = (function() { ...@@ -473,8 +473,8 @@ module.exports = (function() {
, mainQueryItems = [] , mainQueryItems = []
, mainAttributes = options.attributes , mainAttributes = options.attributes
, mainJoinQueries = [] , mainJoinQueries = []
// We'll use a subquery if there's a limit, if we have hasMany associations and if any of them are filtered // We'll use a subquery if we have hasMany associations and a limit and a filtered/required association
, subQuery = limit && options && options.hasIncludeWhere && options.hasIncludeRequired && options.hasMultiAssociation , subQuery = limit && (options.hasIncludeWhere || options.hasIncludeRequired || options.hasMultiAssociation)
, subQueryItems = [] , subQueryItems = []
, subQueryAttributes = null , subQueryAttributes = null
, subJoinQueries = [] , subJoinQueries = []
...@@ -484,6 +484,10 @@ module.exports = (function() { ...@@ -484,6 +484,10 @@ module.exports = (function() {
return this.quoteIdentifiers(t) return this.quoteIdentifiers(t)
}.bind(this)).join(", ") }.bind(this)).join(", ")
if (subQuery && mainAttributes) {
mainAttributes = mainAttributes.concat(factory.hasPrimaryKeys ? factory.primaryKeyAttributes : ['id'])
}
// Escape attributes // Escape attributes
mainAttributes = mainAttributes && mainAttributes.map(function(attr){ mainAttributes = mainAttributes && mainAttributes.map(function(attr){
var addTable = true var addTable = true
...@@ -518,6 +522,7 @@ module.exports = (function() { ...@@ -518,6 +522,7 @@ module.exports = (function() {
// If subquery, we ad the mainAttributes to the subQuery and set the mainAttributes to select * from subquery // If subquery, we ad the mainAttributes to the subQuery and set the mainAttributes to select * from subquery
if (subQuery) { if (subQuery) {
// We need primary keys
subQueryAttributes = mainAttributes subQueryAttributes = mainAttributes
mainAttributes = [options.table+'.*'] mainAttributes = [options.table+'.*']
} }
...@@ -534,7 +539,9 @@ module.exports = (function() { ...@@ -534,7 +539,9 @@ module.exports = (function() {
, includeWhere = {} , includeWhere = {}
, whereOptions = Utils._.clone(options) , whereOptions = Utils._.clone(options)
if (tableName !== parentTable) as = parentTable+'.'+include.as if (tableName !== parentTable) {
as = parentTable+'.'+include.as
}
if (include.where) { if (include.where) {
for (var key in include.where) { for (var key in include.where) {
...@@ -795,12 +802,12 @@ module.exports = (function() { ...@@ -795,12 +802,12 @@ module.exports = (function() {
return "ROLLBACK;" return "ROLLBACK;"
}, },
addLimitAndOffset: function(options, query){ addLimitAndOffset: function(options, query) {
query = query || "" query = query || ""
if (options.offset && !options.limit) { if (options.offset && !options.limit) {
query += " LIMIT " + options.offset + ", " + 10000000000000; query += " LIMIT " + options.offset + ", " + 10000000000000;
} else if (options.limit && !(options.include && (options.limit === 1))) { } else if (options.limit) {
if (options.offset) { if (options.offset) {
query += " LIMIT " + options.offset + ", " + options.limit query += " LIMIT " + options.offset + ", " + options.limit
} else { } else {
......
lib/dialects/mysql/query-generator.js
...@@ -396,7 +396,7 @@ module.exports = (function() { ...@@ -396,7 +396,7 @@ module.exports = (function() {
query = query || "" query = query || ""
if (options.offset && !options.limit) { if (options.offset && !options.limit) {
query += " LIMIT " + options.offset + ", " + 18440000000000000000; query += " LIMIT " + options.offset + ", " + 18440000000000000000;
} else if (options.limit && !(options.include && (options.limit === 1))) { } else if (options.limit) {
if (options.offset) { if (options.offset) {
query += " LIMIT " + options.offset + ", " + options.limit query += " LIMIT " + options.offset + ", " + options.limit
} else { } else {
......
lib/dialects/postgres/query-generator.js
...@@ -410,14 +410,12 @@ module.exports = (function() { ...@@ -410,14 +410,12 @@ module.exports = (function() {
addLimitAndOffset: function(options, query){ addLimitAndOffset: function(options, query){
query = query || "" query = query || ""
if (!(options.include && (options.limit === 1))) { if (options.limit) {
if (options.limit) { query += " LIMIT " + options.limit
query += " LIMIT " + options.limit }
}
if (options.offset) { if (options.offset) {
query += " OFFSET " + options.offset query += " OFFSET " + options.offset
}
} }
return query; return query;
......
lib/dialects/sqlite/query-generator.js
...@@ -153,7 +153,7 @@ module.exports = (function() { ...@@ -153,7 +153,7 @@ module.exports = (function() {
query = query || "" query = query || ""
if (options.offset && !options.limit) { if (options.offset && !options.limit) {
query += " LIMIT " + options.offset + ", " + 10000000000000; query += " LIMIT " + options.offset + ", " + 10000000000000;
} else if (options.limit && !(options.include && (options.limit === 1))) { } else if (options.limit) {
if (options.offset) { if (options.offset) {
query += " LIMIT " + options.offset + ", " + options.limit query += " LIMIT " + options.offset + ", " + options.limit
} else { } else {
......
lib/utils.js
...@@ -599,10 +599,12 @@ Utils.fn.prototype.toString = function(queryGenerator) { ...@@ -599,10 +599,12 @@ Utils.fn.prototype.toString = function(queryGenerator) {
} }
Utils.col.prototype.toString = function (queryGenerator) { Utils.col.prototype.toString = function (queryGenerator) {
if (this.col.indexOf('*') !== -1) return '*' if (this.col.indexOf('*') === 0) {
return '*'
}
return queryGenerator.quote(this.col) return queryGenerator.quote(this.col)
} }
Utils.CustomEventEmitter = require(__dirname + "/emitters/custom-event-emitter") Utils.CustomEventEmitter = require(__dirname + "/emitters/custom-event-emitter")
Utils.QueryChainer = require(__dirname + "/query-chainer") Utils.QueryChainer = require(__dirname + "/query-chainer")
Utils.Lingo = require("lingo") Utils.Lingo = require("lingo")
\ No newline at end of file
test/dao-factory/find.test.js
...@@ -237,8 +237,9 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () { ...@@ -237,8 +237,9 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
self.User.find({ self.User.find({
where: { username: 'JohnXOXOXO' }, where: { username: 'JohnXOXOXO' },
attributes: ['username'] attributes: ['username']
}).success(function(user) { }).done(function(err, user) {
expect(user.selectedValues).to.have.property('username', 'JohnXOXOXO') expect(err).not.to.be.ok
expect(_.omit(user.selectedValues, ['id'])).to.have.property('username', 'JohnXOXOXO')
done() done()
}) })
}) })
...@@ -262,8 +263,9 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () { ...@@ -262,8 +263,9 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
where: { username: 'John DOE' }, where: { username: 'John DOE' },
attributes: ['username'], attributes: ['username'],
include: [self.Mission] include: [self.Mission]
}).success(function(user) { }).done(function(err, user) {
expect(user.selectedValues).to.deep.equal({ username: 'John DOE' }) expect(err).not.to.be.ok
expect(_.omit(user.selectedValues, ['id'])).to.deep.equal({ username: 'John DOE' })
done() done()
}) })
}) })
...@@ -291,7 +293,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () { ...@@ -291,7 +293,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
attributes: ['username'], attributes: ['username'],
include: [{model: self.Mission, as: self.Mission.tableName, attributes: ['title']}] include: [{model: self.Mission, as: self.Mission.tableName, attributes: ['title']}]
}).success(function(user) { }).success(function(user) {
expect(user.selectedValues).to.deep.equal({ username: 'Brain Picker' }) expect(_.omit(user.selectedValues, ['id'])).to.deep.equal({ username: 'Brain Picker' })
expect(user.missions[0].selectedValues).to.deep.equal({ id: 1, title: 'another mission!!'}) expect(user.missions[0].selectedValues).to.deep.equal({ id: 1, title: 'another mission!!'})
expect(user.missions[0].foo).not.to.exist expect(user.missions[0].foo).not.to.exist
done() done()
......
test/dao-factory/findAll.test.js
...@@ -957,6 +957,57 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () { ...@@ -957,6 +957,57 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
}) })
}) })
it("handles offset with includes", function (done) {
var Election = this.sequelize.define('Election', {
name: Sequelize.STRING
})
var Citizen = this.sequelize.define('Citizen', {
name: Sequelize.STRING
})
// Associations
Election.belongsTo(Citizen)
Election.hasMany(Citizen, { as: 'Voters', through: 'ElectionsVotes' })
Citizen.hasMany(Election)
Citizen.hasMany(Election, { as: 'Votes', through: 'ElectionsVotes' })
this.sequelize.sync().done(function (err) {
expect(err).not.be.ok
// Add some data
Citizen.create({ name: 'Alice' }).done(function (err, alice) {
expect(err).not.be.ok
Citizen.create({ name: 'Bob' }).done(function (err, bob) {
expect(err).not.be.ok
Election.create({ name: 'Some election' }).done(function (err, election) {
expect(err).not.be.ok
election.setCitizen(alice).done(function (err) {
expect(err).not.be.ok
election.setVoters([alice, bob]).done(function (err) {
expect(err).not.be.ok
var criteria = {
offset: 5,
limit: 1,
include: [
Citizen, // Election creator
{ model: Citizen, as: 'Voters' } // Election voters
]
}
Election.findAndCountAll(criteria).done(function (err, elections) {
expect(err).not.be.ok
expect(elections.count).to.equal(2)
expect(elections.rows.length).to.equal(0)
done()
})
})
})
})
})
})
})
})
it("handles attributes", function(done) { it("handles attributes", function(done) {
this.User.findAndCountAll({where: "id != " + this.users[0].id, attributes: ['data']}).success(function(info) { this.User.findAndCountAll({where: "id != " + this.users[0].id, attributes: ['data']}).success(function(info) {
expect(info.count).to.equal(2) expect(info.count).to.equal(2)
......
test/include/findAll.test.js
...@@ -895,7 +895,9 @@ describe(Support.getTestDialectTeaser("Include"), function () { ...@@ -895,7 +895,9 @@ describe(Support.getTestDialectTeaser("Include"), function () {
}) })
it('should be possible to extend the on clause with a where option on nested includes', function (done) { it('should be possible to extend the on clause with a where option on nested includes', function (done) {
var User = this.sequelize.define('User', {}) var User = this.sequelize.define('User', {
name: DataTypes.STRING
})
, Product = this.sequelize.define('Product', { , Product = this.sequelize.define('Product', {
title: DataTypes.STRING title: DataTypes.STRING
}) })
...@@ -984,7 +986,7 @@ describe(Support.getTestDialectTeaser("Include"), function () { ...@@ -984,7 +986,7 @@ describe(Support.getTestDialectTeaser("Include"), function () {
async.auto({ async.auto({
user: function (callback) { user: function (callback) {
User.create().done(callback) User.create({name: 'FooBarzz'}).done(callback)
}, },
memberships: ['user', function (callback, results) { memberships: ['user', function (callback, results) {
GroupMember.bulkCreate([ GroupMember.bulkCreate([
...@@ -1123,22 +1125,26 @@ describe(Support.getTestDialectTeaser("Include"), function () { ...@@ -1123,22 +1125,26 @@ describe(Support.getTestDialectTeaser("Include"), function () {
}) })
}) })
it('should be possible use limit and a where on a belongsTo with additional hasMany includes', function (done) { it('should be possible use limit, attributes and a where on a belongsTo with additional hasMany includes', function (done) {
var self = this var self = this
this.fixtureA(function () { this.fixtureA(function () {
self.models.Product.findAll({ self.models.Product.findAll({
attributes: ['title'],
include: [ include: [
{model: self.models.Company, where: {name: 'NYSE'}}, {model: self.models.Company, where: {name: 'NYSE'}},
{model: self.models.Tag}, {model: self.models.Tag},
{model: self.models.Price} {model: self.models.Price}
], ],
limit: 3, limit: 3,
order: 'id ASC' order: [
[self.sequelize.col(self.models.Product.tableName+'.id'), 'ASC']
]
}).done(function (err, products) { }).done(function (err, products) {
expect(err).not.to.be.ok expect(err).not.to.be.ok
expect(products.length).to.equal(3) expect(products.length).to.equal(3)
products.forEach(function (product) { products.forEach(function (product) {
expect(product.company.name).to.equal('NYSE')
expect(product.tags.length).to.be.ok expect(product.tags.length).to.be.ok
expect(product.prices.length).to.be.ok expect(product.prices.length).to.be.ok
}) })
...@@ -1151,6 +1157,7 @@ describe(Support.getTestDialectTeaser("Include"), function () { ...@@ -1151,6 +1157,7 @@ describe(Support.getTestDialectTeaser("Include"), function () {
var self = this var self = this
this.fixtureA(function () { this.fixtureA(function () {
self.models.Product.findAll({ self.models.Product.findAll({
include: [ include: [
{model: self.models.Company}, {model: self.models.Company},
{model: self.models.Tag}, {model: self.models.Tag},
......
Markdown is supported
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!