Merge pull request #5534 from sushantdhiman/fix-count-yarando

Fix findAndCountAll issues

changelog.md

 # Future
 # Future
 - [FIXED] `addColumn` with reference in mysql [#5592](https://github.com/sequelize/sequelize/issues/5592)
 - [FIXED] `addColumn` with reference in mysql [#5592](https://github.com/sequelize/sequelize/issues/5592)
 - [ADDED] rejectOnEmpty mode [#272](https://github.com/sequelize/sequelize/issues/272) [#5480](https://github.com/sequelize/sequelize/issues/5480)
 - [ADDED] rejectOnEmpty mode [#272](https://github.com/sequelize/sequelize/issues/272) [#5480](https://github.com/sequelize/sequelize/issues/5480)
+- [FIXED] `findAndCountAll` generates invalid SQL, subQuery moves to LEFT OUTER JOIN [#5445](https://github.com/sequelize/sequelize/issues/5445)
+- [FIXED] `count` methods pollute the options.includes [#4191](https://github.com/sequelize/sequelize/issues/4191)
+- [FIXED] Invalid SQL generated when using group option along with attributes [#3009](https://github.com/sequelize/sequelize/issues/3009)
 - [ADDED] `beforeCount` hook [#5209](https://github.com/sequelize/sequelize/pull/5209)
 - [ADDED] `beforeCount` hook [#5209](https://github.com/sequelize/sequelize/pull/5209)
 - [ADDED] `validationFailed` hook [#1626](https://github.com/sequelize/sequelize/issues/1626)
 - [ADDED] `validationFailed` hook [#1626](https://github.com/sequelize/sequelize/issues/1626)
 - [FIXED] Mark index as `unique: true` when `type: 'UNIQUE'`. Fixes [#5351](https://github.com/sequelize/sequelize/issues/5351)
 - [FIXED] Mark index as `unique: true` when `type: 'UNIQUE'`. Fixes [#5351](https://github.com/sequelize/sequelize/issues/5351)

lib/model.js

@@ -486,6 +486,7 @@ var validateIncludedElements = function(options, tableNames) {
         include.subQuery = false;
         include.subQuery = false;
       } else {
       } else {
         include.subQueryFilter = false;
         include.subQueryFilter = false;
+        include.subQuery = include.subQuery || (include.hasParentRequired && include.hasRequired);
       }
       }
     }
     }
 
 

test/integration/include/findAndCountAll.test.js

@@ -207,5 +207,78 @@ describe(Support.getTestDialectTeaser('Include'), function() {
         expect(result.rows.length).to.equal(1);
         expect(result.rows.length).to.equal(1);
       });
       });
     });
     });
+
+    it('should correctly filter, limit and sort when multiple includes and types of associations are present.', function() {
+      var TaskTag = this.sequelize.define('TaskTag', {
+        id: { type: DataTypes.INTEGER, allowNull: false, primaryKey: true, autoIncrement: true },
+        name: { type: DataTypes.STRING}
+      });
+
+      var Tag = this.sequelize.define('Tag', {
+        id: { type: DataTypes.INTEGER, allowNull: false, primaryKey: true, autoIncrement: true },
+        name: { type: DataTypes.STRING}
+      });
+
+      var Task = this.sequelize.define('Task', {
+        id: { type: DataTypes.INTEGER, allowNull: false, primaryKey: true, autoIncrement: true },
+        name: { type: DataTypes.STRING}
+      });
+      var Project = this.sequelize.define('Project', {
+        id: { type: DataTypes.INTEGER, allowNull: false, primaryKey: true, autoIncrement: true },
+        m: { type: DataTypes.STRING}
+      });
+
+      var User = this.sequelize.define('User', {
+        id: { type: DataTypes.INTEGER, allowNull: false, primaryKey: true, autoIncrement: true },
+        name: { type: DataTypes.STRING }
+      });
+
+      Project.belongsTo(User);
+      Task.belongsTo(Project);
+      Task.belongsToMany(Tag, {through: TaskTag});
+      // Sync them
+      return this.sequelize.sync({ force: true }).then(function() {
+        // Create an enviroment
+        return User.bulkCreate([
+          { name: 'user-name-1' },
+          { name: 'user-name-2' }
+        ]).then(function(u){
+          return Project.bulkCreate([
+            { m: 'A', UserId: 1},
+            { m: 'A', UserId: 2},
+          ]);
+        }).then(function(p){
+          return Task.bulkCreate([
+            { ProjectId: 1, name: 'Just' },
+            { ProjectId: 1, name: 'for' },
+            { ProjectId: 2, name: 'testing' },
+            { ProjectId: 2, name: 'proposes' }
+          ]);
+        })
+        .then(function() {
+          // Find All Tasks with Project(m=a) and User(name=user-name-2)
+          return Task.findAndCountAll({
+            limit: 1,
+            offset: 0,
+            order: [[ 'id', 'DESC' ]],
+            include: [
+              {
+                model: Project,
+                where: { '$and': [ { m: 'A' } ] } ,
+                include: [ {
+                    model: User,
+                    where: { '$and': [ { name: 'user-name-2' } ] }
+                  }
+                ]
+              },
+              { model : Tag }
+            ]
+          });
+        });
+      }).then(function(result) {
+          expect(result.count).to.equal(2);
+          expect(result.rows.length).to.equal(1);
+      });
+    });
   });
   });
 });
 });

test/integration/model.test.js

@@ -2081,9 +2081,7 @@ describe(Support.getTestDialectTeaser('Model'), function() {
       return this.UserWithFields.bulkCreate([
       return this.UserWithFields.bulkCreate([
         {age: 2, gender: 'male'},
         {age: 2, gender: 'male'},
         {age: 3, gender: 'female'}
         {age: 3, gender: 'female'}
-      ], {
-        logging: console.log
-      }).bind(this).then(function() {
+      ]).bind(this).then(function() {
         return expect(this.UserWithFields.sum('age', {
         return expect(this.UserWithFields.sum('age', {
           where: { 'gender': 'male' }
           where: { 'gender': 'male' }
         })).to.eventually.equal(2);
         })).to.eventually.equal(2);

test/integration/model/findAll/group.test.js

+'use strict';
+
+/* jshint -W030 */
+var chai = require('chai')
+, expect = chai.expect
+, Support = require(__dirname + '/../../support')
+, Sequelize = Support.Sequelize
+, DataTypes = require(__dirname + '/../../../../lib/data-types')
+, current = Support.sequelize;
+
+describe(Support.getTestDialectTeaser('Model'), function() {
+  describe('findAll', function () {
+    describe('group', function () {
+      it('should correctly group with attributes, #3009', function() {
+
+        var Post = current.define('Post', {
+          id: { type: DataTypes.INTEGER, autoIncrement: true, primaryKey: true },
+          name: { type: DataTypes.STRING, allowNull: false }
+        });
+
+        var Comment = current.define('Comment', {
+          id: { type: DataTypes.INTEGER, autoIncrement: true, primaryKey: true },
+          text: { type: DataTypes.STRING, allowNull: false }
+        });
+
+        Post.hasMany(Comment);
+
+        return current.sync({ force: true }).then(function() {
+          // Create an enviroment
+          return Post.bulkCreate([
+            { name: 'post-1' },
+            { name: 'post-2' }
+          ]);
+        }).then(function(u) {
+          return Comment.bulkCreate([
+            { text: 'Market', PostId: 1},
+            { text: 'Text', PostId: 2},
+            { text: 'Abc', PostId: 2},
+            { text: 'Semaphor', PostId: 1},
+            { text: 'Text', PostId: 1},
+          ]);
+        }).then(function(p) {
+          return Post.findAll({
+            attributes: [ [ Sequelize.fn('COUNT', Sequelize.col('Comments.id')), 'comment_count' ] ],
+            include: [
+              { model: Comment, attributes: [] }
+            ],
+            group: [ 'Post.id' ]
+          });
+        }).then(function(posts) {
+          expect(parseInt(posts[0].get('comment_count'))).to.be.equal(3);
+          expect(parseInt(posts[1].get('comment_count'))).to.be.equal(2);
+        });
+      });
+    });
+  });
+});

test/unit/model/findAndCountAll.test.js

+'use strict';
+
+/* jshint -W030 */
+var chai = require('chai')
+, expect = chai.expect
+, Support = require(__dirname + '/../support')
+, current = Support.sequelize
+, sinon = require('sinon')
+, DataTypes = require(__dirname + '/../../../lib/data-types')
+, _ = require('lodash');
+
+describe(Support.getTestDialectTeaser('Model'), function() {
+  describe('method findAndCountAll', function () {
+    var Model = current.define('model', {
+      name: DataTypes.STRING
+    }, { timestamps: false });
+
+    var Model2 = current.define('model2', {
+      name: DataTypes.STRING
+    }, { timestamps: false });
+
+    Model.hasMany(Model2);
+    Model2.belongsTo(Model);
+
+    before(function () {
+      this.stub = sinon.stub(current.getQueryInterface(), 'select', function () {
+        return Model.build({});
+      });
+      this.stubRaw = sinon.stub(current.getQueryInterface(), 'rawSelect', function () {
+        return Model.build({});
+      });
+    });
+
+    beforeEach(function () {
+      this.stub.reset();
+      this.stubRaw.reset();
+    });
+
+    after(function () {
+      this.stub.restore();
+      this.stubRaw.restore();
+    });
+
+    it('properly clones options values', function() {
+      var options = {
+        includes: [
+          { model: 'model2', where: {
+            name: 'hello'
+          }}
+        ]
+      };
+      var optionsClones = _.cloneDeep(options);
+      return Model.findAndCountAll(options).bind(this).then(function () {
+        expect(options).to.deep.equal(optionsClones);
+      });
+    });
+
+  });
+});