Merge pull request #1202 from mickhansen/findandcountall-include

Findandcountall include

lib/dao-factory.js

@@ -66,6 +66,10 @@ module.exports = (function() {
     }
   })
 
+  Object.defineProperty(DAOFactory.prototype, 'sequelize', {
+    get: function() { return this.daoFactoryManager.sequelize }
+  })
+
   Object.defineProperty(DAOFactory.prototype, 'QueryInterface', {
     get: function() { return this.daoFactoryManager.sequelize.getQueryInterface() }
   })
@@ -562,47 +566,46 @@ module.exports = (function() {
   }
 
   DAOFactory.prototype.count = function(options) {
-    options = Utils._.extend({ attributes: [] }, options || {})
-    options.dataType = DataTypes.INTEGER
+    options = Utils._.clone(options || {})
+    return new Utils.CustomEventEmitter(function (emitter) {
+      options.attributes = [
+        [this.sequelize.fn('COUNT', this.sequelize.col(this.tableName+'.*')), 'count']
+      ]
+      options.includeIgnoreAttributes = false
 
-    return this.aggregate('*', 'count', options)
+      this.find(options, {raw: true, transaction: options.transaction}).proxy(emitter, {events: ['sql', 'error']}).success(function (result) {
+        emitter.emit('success', parseInt(result.count, 10))
+      })
+    }.bind(this)).run()
   }
 
-  DAOFactory.prototype.findAndCountAll = function(options, queryOptions) {
+  DAOFactory.prototype.findAndCountAll = function(findOptions, queryOptions) {
     var self  = this
-      // no limit, offset, order, attributes or include for the options given to count()
-      , copts = Utils._.omit(options || {}, ['offset', 'limit', 'order', 'include', 'attributes'])
+      // no limit, offset, order, attributes for the options given to count()
+      , countOptions = Utils._.omit(findOptions || {}, ['offset', 'limit', 'order', 'attributes'])
 
     return new Utils.CustomEventEmitter(function (emitter) {
       var emit = {
-          err  : function(e) {        // emit error
-            emitter.emit('error', e);
-          }
-        , okay : function(c, r) {     // emit success
-            emitter.emit('success', {
-              count: c || 0,
-              rows : (r && Array.isArray(r) ? r : [])
-            });
-          }
-        , sql  : function(s) {        // emit SQL
-            emitter.emit('sql', s);
-          }
+        okay : function(count, results) {     // emit success
+          emitter.emit('success', {
+            count: count || 0,
+            rows : (results && Array.isArray(results) ? results : [])
+          })
         }
+      }
 
-      self.count(copts)
-          .on('sql', emit.sql)
-          .error(emit.err)
-          .success(function(cnt) {
-            if (cnt === 0) {
-              return emit.okay(cnt) // no records, no need for another query
+      self.count(countOptions)
+          .proxy(emitter, {events: ['sql', 'error']})
+          .success(function(count) {
+            if (count === 0) {
+              return emit.okay(count) // no records, no need for another query
             }
 
-            self.findAll(options, queryOptions)
-                .on('sql', emit.sql)
-                .error(emit.err)
-                .success(function(rows) {
-                  emit.okay(cnt, rows)
-                })
+            self.findAll(findOptions, queryOptions)
+              .proxy(emitter, {events: ['sql', 'error']})
+              .success(function(results) {
+                emit.okay(count, results)
+              })
           })
 
     }).run()

lib/dialects/abstract/query-generator.js

@@ -469,13 +469,23 @@ module.exports = (function() {
       options.table      = table = Array.isArray(tableName) ? tableName.map(function(t) { return this.quoteIdentifiers(t) }.bind(this)).join(", ") : this.quoteIdentifiers(tableName)
 
       options.attributes = options.attributes && options.attributes.map(function(attr){
+        var addTable = true
+
+        if (attr instanceof Utils.fn || attr instanceof Utils.col) {
+          return self.quote(attr)
+        }
+
         if(Array.isArray(attr) && attr.length == 2) {
+          if (attr[0] instanceof Utils.fn || attr[0] instanceof Utils.col) {
+            attr[0] = self.quote(attr[0])
+            addTable = false
+          }
           attr = [attr[0], this.quoteIdentifier(attr[1])].join(' as ')
         } else {
           attr = attr.indexOf(Utils.TICK_CHAR) < 0 && attr.indexOf('"') < 0 ? this.quoteIdentifiers(attr) : attr
         }
 
-        if (options.include && attr.indexOf('.') === -1) {
+        if (options.include && attr.indexOf('.') === -1 && addTable) {
           attr = this.quoteIdentifier(options.table) + '.' + attr
         }
 
@@ -494,7 +504,7 @@ module.exports = (function() {
             , association   = include.association
             , through       = include.through
             , joinType      = include.required ? ' INNER JOIN ' : ' LEFT OUTER JOIN '
-            , where         = {}
+            , includeWhere  = {}
             , whereOptions  = Utils._.clone(options)
 
           if (tableName !== parentTable) as = parentTable+'.'+include.as
@@ -502,19 +512,20 @@ module.exports = (function() {
           if (include.where) {
             for (var key in include.where) {
               if (include.where.hasOwnProperty(key)) {
-                where[self.quoteIdentifier(as)+'.'+self.quoteIdentifiers(key)] = include.where[key]
+                includeWhere[self.quoteIdentifier(as)+'.'+self.quoteIdentifiers(key)] = include.where[key]
               }
             }
-
-            include.where = where
             whereOptions.keysEscaped = true
           }
 
-          attributes  = include.attributes.map(function(attr) {
-            return self.quoteIdentifier(as) + "." + self.quoteIdentifier(attr) + " AS " + self.quoteIdentifier(as + "." + attr)
-          })
+          // includeIgnoreAttributes is used by aggregate functions
+          if (options.includeIgnoreAttributes !== false) {
+            attributes  = include.attributes.map(function(attr) {
+              return self.quoteIdentifier(as) + "." + self.quoteIdentifier(attr) + " AS " + self.quoteIdentifier(as + "." + attr)
+            })
 
-          optAttributes = optAttributes.concat(attributes)
+            optAttributes = optAttributes.concat(attributes)
+          }
 
           if (through) {
             var throughTable      = through.daoFactory.tableName
@@ -523,7 +534,7 @@ module.exports = (function() {
                 return self.quoteIdentifier(throughAs) + "." + self.quoteIdentifier(attr) + " AS " + self.quoteIdentifier(throughAs + "." + attr)
               })
 
-            optAttributes = optAttributes.concat(throughAttributes)
+            if (options.includeIgnoreAttributes !== false) optAttributes = optAttributes.concat(throughAttributes)
 
             var primaryKeysSource = Object.keys(association.source.primaryKeys)
               , tableSource       = parentTable
@@ -544,7 +555,7 @@ module.exports = (function() {
               joinQueryItem += self.quoteIdentifier(throughAs) + "." + self.quoteIdentifier(identTarget)
 
             if (include.where) {
-              joinQueryItem += " AND "+self.hashToWhereConditions(include.where, include.daoFactory, whereOptions)
+              joinQueryItem += " AND "+self.hashToWhereConditions(includeWhere, include.daoFactory, whereOptions)
             }
           } else {
             var primaryKeysLeft = ((association.associationType === 'BelongsTo') ? Object.keys(association.target.primaryKeys) : Object.keys(include.association.source.primaryKeys))
@@ -558,7 +569,7 @@ module.exports = (function() {
               joinQueryItem += self.quoteIdentifier(tableRight) + "." + self.quoteIdentifier(attrRight)
 
             if (include.where) {
-              joinQueryItem += " AND "+self.hashToWhereConditions(include.where, include.daoFactory, whereOptions)
+              joinQueryItem += " AND "+self.hashToWhereConditions(includeWhere, include.daoFactory, whereOptions)
             }
           }
 
@@ -852,6 +863,7 @@ module.exports = (function() {
           ,  _key
           , _value = null
 
+
         if (options.keysEscaped) {
           _key = key
         } else {

lib/dialects/mysql/query-generator.js

@@ -387,6 +387,7 @@ module.exports = (function() {
     },
 
     quoteIdentifier: function(identifier, force) {
+      if (identifier === '*') return identifier
       return Utils.addTicks(identifier, "`")
     },
 

lib/dialects/postgres/query-generator.js

@@ -774,6 +774,7 @@ module.exports = (function() {
     },
 
     quoteIdentifier: function(identifier, force) {
+      if (identifier === '*') return identifier
       if(!force && this.options && this.options.quoteIdentifiers === false) { // default is `true`
         // In Postgres, if tables or attributes are created double-quoted,
         // they are also case sensitive. If they contain any uppercase

lib/dialects/sqlite/query-generator.js

@@ -380,6 +380,7 @@ module.exports = (function() {
     },
 
     quoteIdentifier: function(identifier, force) {
+      if (identifier === '*') return identifier
       return Utils.addTicks(identifier, "`")
     },
 

lib/utils.js

@@ -580,6 +580,7 @@ Utils.fn.prototype.toString = function(queryGenerator) {
 }
 
 Utils.col.prototype.toString = function (queryGenerator) {
+  if (this.col.indexOf('*') !== -1) return '*'
   return queryGenerator.quote(this.col)
 }
 

test/include.test.js

@@ -1345,7 +1345,7 @@ describe(Support.getTestDialectTeaser("Include"), function () {
   })
 
   describe('findAndCountAll', function () {
-    xit('should include associations to findAndCountAll', function(done) {
+    it('should include associations to findAndCountAll', function(done) {
       var User = this.sequelize.define('User', {})
         , Item = this.sequelize.define('Item', {'test': DataTypes.STRING})
 
@@ -1381,12 +1381,17 @@ describe(Support.getTestDialectTeaser("Include"), function () {
             chainer.run().done(callback)
           }]
         }, function() {
-          User.findAndCountAll({'where': {'item.test': 'def'}, 'include': [Item]}).done(function(err, result) {
+          User.findAndCountAll({include: [
+            {model: Item, where: {
+              test: 'def'
+            }}
+          ]}).done(function(err, result) {
             expect(err).not.to.be.ok
             expect(result.count).to.eql(1)
 
             expect(result.rows.length).to.eql(1)
             expect(result.rows[0].item.test).to.eql('def')
+            done()
           })
         })
       })