fix(includes): do not use subquery if required is false

lib/dialects/abstract/query-generator.js

@@ -789,7 +789,7 @@ module.exports = (function() {
             if (include.where) {
               targetWhere = self.getWhereConditions(include.where, self.sequelize.literal(self.quoteIdentifier(as)), include.model, whereOptions);
               joinQueryItem += ' AND ' + targetWhere;
-              if (subQuery) {
+              if (subQuery && include.required) {
                 if (!options.where) options.where = {};
 
                 // Creating the as-is where for the subQuery, checks that the required association exists

test/include/find.test.js

@@ -11,33 +11,29 @@ chai.config.includeStack = true
 
 describe(Support.getTestDialectTeaser("Include"), function () {
   describe('find', function () {
-
-    it( 'Try to include a non required model, with conditions and two includes N:M 1:M', function ( done ) {
-      var DT = DataTypes,
-          S = this.sequelize,
-          A = S.define('A', { name: DT.STRING(40) }, { paranoid: true }),
-          B = S.define('B', { name: DT.STRING(40) }, { paranoid: true }),
-          C = S.define('C', { name: DT.STRING(40) }, { paranoid: true }),
-          D = S.define('D', { name: DT.STRING(40) }, { paranoid: true })
+    it('should include a non required model, with conditions and two includes N:M 1:M', function ( done ) {
+      var A = this.sequelize.define('A', { name: DataTypes.STRING(40) }, { paranoid: true })
+        , B = this.sequelize.define('B', { name: DataTypes.STRING(40) }, { paranoid: true })
+        , C = this.sequelize.define('C', { name: DataTypes.STRING(40) }, { paranoid: true })
+        , D = this.sequelize.define('D', { name: DataTypes.STRING(40) }, { paranoid: true });
 
       // Associations
-      A.hasMany( B )
+      A.hasMany(B);
 
-      B.belongsTo( B )
-      B.belongsTo( D )
-      B.hasMany( C, {
+      B.belongsTo(B);
+      B.belongsTo(D);
+      B.hasMany(C, {
         through: 'BC',
-        })
+      });
 
-      C
-        .hasMany( B, {
+      C.hasMany(B, {
         through: 'BC',
-        })
+      });
 
-      D
-        .hasMany( B )
+      D.hasMany(B);
 
-      S.sync({ force: true }).done( function ( err ) { expect( err ).not.to.be.ok
+      this.sequelize.sync({ force: true }).done(function ( err ) {
+        expect( err ).not.to.be.ok;
 
         A.find({
           include: [
@@ -47,24 +43,25 @@ describe(Support.getTestDialectTeaser("Include"), function () {
             ]}
           ]
         }).done( function ( err ) {
-          expect( err ).not.to.be.ok
-          done()
-        })
-
-      })
+          expect( err ).not.to.be.ok;
+          done();
+        });
+      });
 
     });
 
     it("should still pull the main record when an included model is not required and has where restrictions without matches", function () {
-      var DT = DataTypes,
-          S  = this.sequelize,
-          A  = S.define( 'A', {name: DT.STRING(40)} ),
-          B  = S.define( 'B', {name: DT.STRING(40)} );
+      var A = this.sequelize.define('A', {
+          name: DataTypes.STRING(40)
+        })
+        , B = this.sequelize.define('B', {
+          name: DataTypes.STRING(40)
+        });
 
       A.hasMany(B);
       B.hasMany(A);
 
-      return S
+      return this.sequelize
         .sync({force: true})
         .then(function () {
           return A.create({
@@ -75,7 +72,7 @@ describe(Support.getTestDialectTeaser("Include"), function () {
           return A.find({
             where: {name: 'Foobar'},
             include: [
-              {model: B, where: {name: 'idontexist'}, require: false}
+              {model: B, where: {name: 'idontexist'}, required: false}
             ]
           });
         })
@@ -84,7 +81,5 @@ describe(Support.getTestDialectTeaser("Include"), function () {
           expect(a.get('bs')).to.deep.equal([]);
         });
     });
-
   });
-
-})
\ No newline at end of file
+});
\ No newline at end of file

test/include/findAll.test.js

@@ -1681,15 +1681,14 @@ describe(Support.getTestDialectTeaser("Include"), function () {
     })
 
     it("should still pull the main record(s) when an included model is not required and has where restrictions without matches", function () {
-      var DT = DataTypes,
-          S  = this.sequelize,
-          A  = S.define( 'A', {name: DT.STRING(40)} ),
-          B  = S.define( 'B', {name: DT.STRING(40)} );
+      var self = this
+        , A = this.sequelize.define( 'A', {name: DataTypes.STRING(40)} )
+        , B = this.sequelize.define( 'B', {name: DataTypes.STRING(40)} );
 
       A.hasMany(B);
       B.hasMany(A);
 
-      return S
+      return this.sequelize
         .sync({force: true})
         .then(function () {
           return A.create({
@@ -1700,7 +1699,7 @@ describe(Support.getTestDialectTeaser("Include"), function () {
           return A.findAll({
             where: {name: 'Foobar'},
             include: [
-              {model: B, where: {name: 'idontexist'}, require: false}
+              {model: B, where: {name: 'idontexist'}, required: false}
             ]
           });
         })