First subQuery refactoring

Mick Hansen
Commit 7df710b8 authored Jan 05, 2014 by Mick Hansen
Showing with 110 additions and 33 deletions
lib/dao-factory.js
lib/dialects/abstract/query-generator.js
lib/dialects/mysql/query-generator.js
lib/dialects/postgres/query-generator.js
lib/dialects/sqlite/query-generator.js
test/associations/multiple-level-filters.test.js
test/include.test.js
--- a/lib/dao-factory.js
+++ b/lib/dao-factory.js
@@ -1280,11 +1280,15 @@ module.exports = (function() {
    options.include = options.include.map(function(include) {
      include = validateIncludedElement.call(this, include, options.daoFactory)
+      options.includeMap[include.as] = include
+      options.includeNames.push(include.as)
      if (include.association.isMultiAssociation) options.hasMultiAssociation = true
      if (include.association.isSingleAssociation) options.hasSingleAssociation = true
-      options.includeMap[include.as] = include
+      options.hasIncludeWhere = options.hasIncludeWhere || include.hasIncludeWhere || !!include.where
-      options.includeNames.push(include.as)
+      options.hasIncludeRequired = options.hasIncludeRequired || include.hasIncludeRequired || !!include.required
      return include
    }.bind(this))
  };

--- a/lib/dialects/abstract/query-generator.js
+++ b/lib/dialects/abstract/query-generator.js
@@ -463,12 +463,23 @@ module.exports = (function() {
    selectQuery: function(tableName, options, factory) {
      var table               = null
        , self                = this
-        , joinQuery = ""
+        , query
+        , limit               = options.limit
+        , mainQueryItems      = []
+        , mainAttributes      = options.attributes
+        , mainJoinQueries     = []
+        // We'll use a subquery if there's a limit, if we have hasMany associations and if any of them are filtered
+        , subQuery            = limit && options && options.hasIncludeWhere && options.hasMultiAssociation
+        , subQueryItems       = []
+        , subQueryAttributes  = null
+        , subJoinQueries      = []
      options       = options || {}
-      options.table      = table = Array.isArray(tableName) ? tableName.map(function(t) { return this.quoteIdentifiers(t) }.bind(this)).join(", ") : this.quoteIdentifiers(tableName)
+      options.table = table = !Array.isArray(tableName) ? this.quoteIdentifiers(tableName) : tableName.map(function(t) {
+        return this.quoteIdentifiers(t)
+      }.bind(this)).join(", ")
-      options.attributes = options.attributes && options.attributes.map(function(attr){
+      mainAttributes = mainAttributes && mainAttributes.map(function(attr){
        var addTable = true
        if (attr instanceof Utils.fn || attr instanceof Utils.col) {
@@ -490,12 +501,15 @@ module.exports = (function() {
        }
        return attr
-      }.bind(this)).join(", ")
+      })
-      options.attributes = options.attributes || '*'
+      mainAttributes = mainAttributes || [options.table+'.*']
-      if (options.include) {
+      if (subQuery) {
-        var optAttributes = options.attributes === '*' ? [options.table + '.*'] : [options.attributes]
+        subQueryAttributes = mainAttributes
+        mainAttributes = [options.table+'.*']
+      }
+      if (options.include) {
        var generateJoinQuery = function(include, parentTable) {
          var table         = include.daoFactory.tableName
            , as            = include.as
@@ -524,7 +538,11 @@ module.exports = (function() {
              return self.quoteIdentifier(as) + "." + self.quoteIdentifier(attr) + " AS " + self.quoteIdentifier(as + "." + attr)
            })
-            optAttributes = optAttributes.concat(attributes)
+            if (include.hasIncludeRequired || include.required) {
+              subQueryAttributes = subQueryAttributes.concat(attributes)
+            } else {
+              mainAttributes = mainAttributes.concat(attributes)
+            }
          }
          if (through) {
@@ -534,7 +552,13 @@ module.exports = (function() {
                return self.quoteIdentifier(throughAs) + "." + self.quoteIdentifier(attr) + " AS " + self.quoteIdentifier(throughAs + "." + attr)
              })
-            if (options.includeIgnoreAttributes !== false) optAttributes = optAttributes.concat(throughAttributes)
+            if (options.includeIgnoreAttributes !== false) {
+              if (include.hasIncludeRequired || include.required) {
+                subQueryAttributes = subQueryAttributes.concat(attributes)
+              } else {
+                mainAttributes = mainAttributes.concat(throughAttributes)
+              }
+            }
            var primaryKeysSource = Object.keys(association.source.primaryKeys)
              , tableSource       = parentTable
@@ -584,46 +608,90 @@ module.exports = (function() {
        }
        options.include.forEach(function(include) {
-          joinQuery += generateJoinQuery(include, tableName)
+          var joinQueryItem = generateJoinQuery(include, tableName)
-        }.bind(this))
-        options.attributes = optAttributes.join(', ')
+          if (include.hasIncludeWhere || include.where) {
+            subJoinQueries.push(joinQueryItem)
+          } else {
+            mainJoinQueries.push(joinQueryItem)
+          }
+        }.bind(this))
      }
-      var conditionalJoins = ((options.hasMultiAssociation && (options.limit || options.offset)) || !options.include) && this.getConditionalJoins(options, factory),
+      //var conditionalJoins = ((options.hasMultiAssociation && (options.limit || options.offset)) || !options.include) && this.getConditionalJoins(options, factory),
-        query;
-      if (conditionalJoins) {
+      /*if (conditionalJoins) {
        query = "SELECT " + options.attributes + " FROM ( "
          + "SELECT " + options.table + ".* FROM " + options.table + this.getConditionalJoins(options, factory)
+      } else {*/
+        //query += "SELECT " + mainAttributes.join(',') + " FROM " + options.table
+        //query += mainJoinQueries.concat(subJoinQueries).join('')
+      //}
+      if (subQuery) {
+        subQueryItems.push("SELECT " + subQueryAttributes.join(',') + " FROM " + options.table)
+        subQueryItems.push(subJoinQueries.join(''))
      } else {
-        query = "SELECT " + options.attributes + " FROM " + options.table
+        mainQueryItems.push("SELECT " + mainAttributes.join(',') + " FROM " + options.table)
-        query += joinQuery
+        mainQueryItems.push(mainJoinQueries.join(''))
      }
      if (options.hasOwnProperty('where')) {
        options.where = this.getWhereConditions(options.where, tableName, factory, options)
-        query += " WHERE " + options.where
+        if (subQuery) {
+          subQueryItems.push(" WHERE " + options.where)
+        } else {
+          mainQueryItems.push(" WHERE " + options.where)
+        }
      }
      if (options.group) {
        options.group = Array.isArray(options.group) ? options.group.map(function (t) { return this.quote(t) }.bind(this)).join(', ') : options.group
-        query += " GROUP BY " + options.group
+        if (subQuery) {
+          subQueryItems.push(" GROUP BY " + options.group)
+        } else {
+          mainQueryItems.push(" GROUP BY " + options.group)
+        }
      }
      if (options.order) {
        options.order = Array.isArray(options.order) ? options.order.map(function (t) { return this.quote(t) }.bind(this)).join(', ') : options.order
-        query += " ORDER BY " + options.order
+        if (subQuery) {
+          subQueryItems.push(" ORDER BY " + options.order)
+        } else {
+          mainQueryItems.push(" ORDER BY " + options.order)
        }
+      }
-      query = this.addLimitAndOffset(options, query)
+      var limitOrder = this.addLimitAndOffset(options, query)
-      if (conditionalJoins) {
+      if (limitOrder) {
+        if (subQuery) {
+          subQueryItems.push(limitOrder)
+        } else {
+          mainQueryItems.push(limitOrder)
+        }
+      }
+      //console.log(subQueryItems)
+      /*if (conditionalJoins) {
        query += ") AS " + options.table
        query += joinQuery
+      }*/
+      if (subQuery) {
+        //console.log(mainQueryItems)
+        //" + mainAttributes.join(',') + "
+        query = "SELECT " + mainAttributes.join(',') + " FROM ("
+          query += subQueryItems.join('')
+        query += ") AS "+options.table
+        query += mainJoinQueries.join('')
+      } else {
+        query = mainQueryItems.join('')
      }
-      query += ";"
+      //console.log(query)
      return query
    },
@@ -673,6 +741,8 @@ module.exports = (function() {
    },
    addLimitAndOffset: function(options, query){
+      query = query || ""
      if (options.offset && !options.limit) {
        query += " LIMIT " + options.offset + ", " + 10000000000000;
      } else if (options.limit && !(options.include && (options.limit === 1))) {

--- a/lib/dialects/mysql/query-generator.js
+++ b/lib/dialects/mysql/query-generator.js
@@ -374,6 +374,7 @@ module.exports = (function() {
    },
    addLimitAndOffset: function(options, query){
+      query = query || ""
      if (options.offset && !options.limit) {
        query += " LIMIT " + options.offset + ", " + 18440000000000000000;
      } else if (options.limit && !(options.include && (options.limit === 1))) {

--- a/lib/dialects/postgres/query-generator.js
+++ b/lib/dialects/postgres/query-generator.js
@@ -389,6 +389,7 @@ module.exports = (function() {
    },
    addLimitAndOffset: function(options, query){
+      query = query || ""
      if (!(options.include && (options.limit === 1))) {
        if (options.limit) {
          query += " LIMIT " + options.limit

--- a/lib/dialects/sqlite/query-generator.js
+++ b/lib/dialects/sqlite/query-generator.js
@@ -132,6 +132,7 @@ module.exports = (function() {
    },
    addLimitAndOffset: function(options, query){
+      query = query || ""
      if (options.offset && !options.limit) {
        query += " LIMIT " + options.offset + ", " + 10000000000000;
      } else if (options.limit && !(options.include && (options.limit === 1))) {

--- a/test/associations/multiple-level-filters.test.js
+++ b/test/associations/multiple-level-filters.test.js
@@ -47,12 +47,12 @@ describe(Support.getTestDialectTeaser("Multiple Level Filters"), function() {
            Task.findAll({
              where: {
                'project.user.username': 'leia'
-              }/*,
+              },
              include: [
                {model: Project, include: [
                  User
                ]}
-              ]*/
+              ]
            }).done(function(err, tasks){
              expect(err).not.to.be.ok
@@ -113,12 +113,12 @@ describe(Support.getTestDialectTeaser("Multiple Level Filters"), function() {
              where: {
                'project.user.username': 'leia',
                'project.user.id': 1
-              }/*,
+              },
              include: [
                {model: Project, include: [
                  User
                ]}
-              ]*/
+              ]
            }).success(function(tasks){
              try{
                expect(tasks.length).to.be.equal(2);
@@ -175,12 +175,12 @@ describe(Support.getTestDialectTeaser("Multiple Level Filters"), function() {
            User.findAll({
              where: {
                'projects.tasks.title': 'fight empire'
-              }/*,
+              },
              include: [
                {model: Project, include: [
                  Task
                ]}
-              ]*/
+              ]
            }).done(function(err, users){
              try{
                expect(users.length).to.be.equal(1);
@@ -224,10 +224,10 @@ describe(Support.getTestDialectTeaser("Multiple Level Filters"), function() {
                      User.findAll({
                        where: {
                          'projects.title': 'republic'
-                        }/*,
+                        },
                        include: [
                          {model: Project}
-                        ]*/
+                        ]
                      }).success(function(users){
                        try{
                          expect(users.length).to.be.equal(1);

--- a/test/include.test.js
+++ b/test/include.test.js