Merge pull request #2163 from ThePoloQ/1.7.0

Fix ORDER BY with INNER, OUTER JOIN and LIMIT, issue #2145

lib/dialects/abstract/query-generator.js

@@ -407,6 +407,38 @@ module.exports = (function() {
       }
     },
 
+    getTableNameOrder: function (obj){
+      if (Utils._.isString(obj)) {
+        return obj
+      } else if (Array.isArray(obj)) {
+        return this.getTableNameOrder(obj[0])
+      } else if (obj instanceof Utils.fn) {
+        return this.getTableNameOrder(obj.args)
+      } else if (obj instanceof Utils.col) {
+        return obj.col.split('.')[0]
+      } else if (Utils._.isObject(obj) && 'raw' in obj){
+        return obj.raw
+      } else{
+        return ''
+      }
+    },
+    
+    getSpecificQuoteOrder: function (obj){
+      if (Utils._.isString(obj)) {
+        return this.quoteIdentifier(obj)
+      } else if (Array.isArray(obj)) {
+        return this.getSpecificQuoteOrder(obj[0]) + (obj.length > 1 ? ' ' + obj[1] : '')
+      } else if (obj instanceof Utils.fn) {
+        return obj.fn + '(' + this.getSpecificQuoteOrder(obj.args) + ')'
+      } else if (obj instanceof Utils.col) {
+        return this.quoteIdentifier(obj.col)
+      } else if (Utils._.isObject(obj) && 'raw' in obj){
+        return obj.raw
+      } else{
+        return ''
+      }
+    },
+
     /*
      Create a trigger
      */
@@ -527,6 +559,7 @@ module.exports = (function() {
         // We'll use a subquery if we have hasMany associations and a limit and a filtered/required association
         , subQuery            = limit && (options.hasIncludeWhere || options.hasIncludeRequired || options.hasMultiAssociation)
         , subQueryItems       = []
+        , subQueryAs          = []
         , subQueryAttributes  = null
         , subJoinQueries      = []
 
@@ -583,6 +616,7 @@ module.exports = (function() {
       if (subQuery) {
         // We need primary keys
         subQueryAttributes = mainAttributes
+        !Array.isArray(tableName) ? subQueryAs.push(tableName) : subQueryAs.concat(tableName)
         mainAttributes = [options.table+'.*']
       }
 
@@ -607,6 +641,10 @@ module.exports = (function() {
           if (tableName !== parentTable) {
             as = parentTable+'.'+include.as
           }
+          
+          if (include.subQuery && subQuery){
+            subQueryAs.push(as);
+          }
 
           // includeIgnoreAttributes is used by aggregate functions
           if (options.includeIgnoreAttributes !== false) {
@@ -795,10 +833,19 @@ module.exports = (function() {
 
         if (Array.isArray(options.order)) {
           options.order.forEach(function (t) {
+            var strOrder = this.quote(t, factory)
+            var tableName = this.getTableNameOrder(t)
             if (subQuery && !(t[0] instanceof daoFactory) && !(t[0].model instanceof daoFactory)) {
-              subQueryOrder.push(this.quote(t, factory))
+              if(tableName && subQueryAs.indexOf(tableName) > -1){
+                subQueryOrder.push(strOrder)
+              }              
+            }
+            
+            if(subQuery && tableName !== subQueryAs[0] && subQueryAs.indexOf(tableName) > -1){
+              mainQueryOrder.push(this.getSpecificQuoteOrder(t))
+            }else{
+              mainQueryOrder.push(strOrder)
             }
-            mainQueryOrder.push(this.quote(t, factory))
           }.bind(this))
         } else {
           mainQueryOrder.push(options.order)

test/associations/has-many.test.js

@@ -1557,4 +1557,66 @@ describe(Support.getTestDialectTeaser("HasMany"), function() {
       })
     })
   })
+  
+  describe("issue #2145 : Test for fix ORDER BY with INNER, OUTER JOIN and LIMIT", function() {
+    it("can order colum and limit records with outer join", function(done) {
+      var Task = this.sequelize.define('Task', { title: DataTypes.STRING })
+        , User = this.sequelize.define('User', { username: DataTypes.STRING })
+        , Subtask = this.sequelize.define('Subtask', { title: DataTypes.STRING })
+
+      User.hasMany(Task);
+      Task.belongsTo(User);
+      
+      Task.hasMany(Subtask);
+      Subtask.belongsTo(Task);
+
+      User.sync({ force: true }).success(function() {
+        Task.sync({ force: true }).success(function() {
+          Subtask.sync({ force: true }).success(function() {
+            User.create({ username: 'myuser' }).success(function(myuser) {
+              var bulkSubtasks = [{title: 'subtask1'},{title: 'another subtask1'},{title: 'mysubtask1'}]
+              Subtask.bulkCreate(bulkSubtasks).success(function() {
+                Task.bulkCreate([{title: 'myTask1'},{title: 'ztask'}]).success(function() {
+                  Task.create({title: 'task1'}).success(function(task1) {
+                    User.findAll().success(function(users){
+                      expect(users).to.have.length(1);
+                      Subtask.findAll().success(function(subtasks){
+                        expect(subtasks).to.have.length(3);
+                        task1.setSubtasks(subtasks).success(function(){
+                          Task.findAll().success(function(tasks){
+                            expect(tasks).to.have.length(3);
+                            myuser.setTasks(tasks).success(function(){
+                              Task.findAll({
+                                include: [
+                                  {model:User, attributes: ['username'], required: true},
+                                  {model:Subtask, attributes: ['title']},
+                                  ],
+                                order: [
+                                  [Sequelize.col('Tasks.title')],
+                                  [Sequelize.col('Subtasks.title')],
+                                  ],
+                                limit: 2
+                              }).success(function(ftasks){
+                                expect(ftasks).to.have.length(2);
+                                var count = 0;
+                                ftasks.forEach(function(task){
+                                  count += task.subtasks.length;
+                                });
+                                expect(count).to.equal(3);
+                                done();
+                              })
+                            });
+                          });
+                        });
+                      });
+                    });
+                  });
+                })
+              })
+            })
+          })
+        })
+      })
+    })
+  })  
 })