Merge pull request #1200 from xdarklight/master

Improved supported data-types in min() and max()

lib/dao-factory.js

@@ -535,14 +535,37 @@ module.exports = (function() {
     }, queryOptions))
   }
 
-  DAOFactory.prototype.count = function(options) {
+  DAOFactory.prototype.aggregate = function(field, aggregateFunction, options) {
+    var tableField;
+
+    if (field == '*') {
+      tableField = field
+    } else {
+      tableField = this.QueryInterface.QueryGenerator.quoteIdentifier(field)
+    }
+
     options = Utils._.extend({ attributes: [] }, options || {})
-    options.attributes.push(['count(*)', 'count'])
-    options.parseInt = true
+    options.attributes.push([aggregateFunction + '(' + tableField + ')', aggregateFunction])
+
+    if (!options.dataType) {
+      if (this.rawAttributes[field]) {
+        options.dataType = this.rawAttributes[field]
+      } else {
+        // Use FLOAT as fallback
+        options.dataType = DataTypes.FLOAT
+      }
+    }
 
     options = paranoidClause.call(this, options)
 
-    return this.QueryInterface.rawSelect(this.getTableName(), options, 'count')
+    return this.QueryInterface.rawSelect(this.getTableName(), options, aggregateFunction)
+  }
+
+  DAOFactory.prototype.count = function(options) {
+    options = Utils._.extend({ attributes: [] }, options || {})
+    options.dataType = DataTypes.INTEGER
+
+    return this.aggregate('*', 'count', options)
   }
 
   DAOFactory.prototype.findAndCountAll = function(options, queryOptions) {
@@ -586,18 +609,11 @@ module.exports = (function() {
   }
 
   DAOFactory.prototype.max = function(field, options) {
-    options = Utils._.extend({ attributes: [] }, options || {})
-    options.attributes.push(['max(' + this.QueryInterface.QueryGenerator.quoteIdentifier(field) + ')', 'max'])
-    options.parseFloat = true
-
-    return this.QueryInterface.rawSelect(this.getTableName(), options, 'max')
+    return this.aggregate(field, 'max', options)
   }
-  DAOFactory.prototype.min = function(field, options) {
-    options = Utils._.extend({ attributes: [] }, options || {})
-    options.attributes.push(['min(' + this.QueryInterface.QueryGenerator.quoteIdentifier(field) + ')', 'min'])
-    options.parseFloat = true
 
-    return this.QueryInterface.rawSelect(this.getTableName(), options, 'min')
+  DAOFactory.prototype.min = function(field, options) {
+    return this.aggregate(field, 'min', options)
   }
 
   DAOFactory.prototype.build = function(values, options) {

lib/data-types.js

@@ -46,6 +46,10 @@ var BLOB = function() {
   return BLOB.prototype.construct.apply(this, [BLOB].concat(Array.prototype.slice.apply(arguments)))
 }
 
+var DECIMAL = function() {
+  return DECIMAL.prototype.construct.apply(this, [DECIMAL].concat(Array.prototype.slice.apply(arguments)))
+}
+
 FLOAT._type = FLOAT
 FLOAT._typeName = 'FLOAT'
 INTEGER._type = INTEGER
@@ -56,8 +60,11 @@ STRING._type = STRING
 STRING._typeName = 'VARCHAR'
 BLOB._type = BLOB
 BLOB._typeName = 'BLOB'
+DECIMAL._type = DECIMAL
+DECIMAL._typeName = 'DECIMAL'
+
 
-BLOB.toString = STRING.toString = INTEGER.toString = FLOAT.toString = BIGINT.toString = function() {
+BLOB.toString = STRING.toString = INTEGER.toString = FLOAT.toString = BIGINT.toString = DECIMAL.toString = function() {
   return new this._type().toString()
 }
 
@@ -145,6 +152,47 @@ FLOAT.prototype = BIGINT.prototype = INTEGER.prototype = {
   }
 }
 
+DECIMAL.prototype = {
+
+  construct: function(RealType, precision, scale) {
+    if (this instanceof RealType) {
+      this._typeName = RealType._typeName
+      if (typeof precision === 'number') {
+        this._precision = precision
+      } else {
+        this._precision = 0
+      }
+      if (typeof scale === 'number') {
+        this._scale = scale
+      } else {
+        this._scale = 0
+      }
+    } else {
+      return new RealType(precision, scale)
+    }
+  },
+
+  get type() {
+    return this.toString()
+  },
+
+  get PRECISION() {
+    return this._precision
+  },
+
+  get SCALE() {
+    return this._scale
+  },
+
+  toString: function() {
+    if (this._precision || this._scale) {
+      return 'DECIMAL(' + this._precision + ',' + this._scale + ')'
+    }
+
+    return 'DECIMAL'
+  }
+}
+
 var unsignedDesc = {
   get: function() {
     return new this._type(undefined, undefined, true)
@@ -163,11 +211,18 @@ var typeDesc = {
   }
 }
 
+var decimalDesc = {
+  get: function() {
+    return new this._type(undefined, undefined, undefined)
+  }
+}
+
 Object.defineProperty(STRING,  'type', typeDesc)
 Object.defineProperty(INTEGER, 'type', typeDesc)
 Object.defineProperty(BIGINT,  'type', typeDesc)
 Object.defineProperty(FLOAT,   'type', typeDesc)
 Object.defineProperty(BLOB,    'type', typeDesc)
+Object.defineProperty(DECIMAL, 'type', typeDesc)
 
 Object.defineProperty(INTEGER, 'UNSIGNED', unsignedDesc)
 Object.defineProperty(BIGINT,  'UNSIGNED', unsignedDesc)
@@ -177,6 +232,9 @@ Object.defineProperty(INTEGER, 'ZEROFILL', zerofillDesc)
 Object.defineProperty(BIGINT,  'ZEROFILL', zerofillDesc)
 Object.defineProperty(FLOAT,   'ZEROFILL', zerofillDesc)
 
+Object.defineProperty(DECIMAL, 'PRECISION', decimalDesc)
+Object.defineProperty(DECIMAL, 'SCALE', decimalDesc)
+
 module.exports = {
   STRING: STRING,
 
@@ -188,6 +246,7 @@ module.exports = {
   FLOAT: FLOAT,
   NOW: 'NOW',
   BLOB: BLOB,
+  DECIMAL: DECIMAL,
   UUID: 'UUID',
   UUIDV1: 'UUIDV1',
   UUIDV4: 'UUIDV4',
@@ -207,16 +266,6 @@ module.exports = {
     return result
   },
 
-  get DECIMAL() {
-    var result = function(precision, scale) {
-      return 'DECIMAL(' + precision + ',' + scale + ')'
-    }
-
-    result.toString = result.valueOf = function() { return 'DECIMAL' }
-
-    return result
-  },
-
   ARRAY: function(type) { return type + '[]' },
 
   get HSTORE() {

lib/query-interface.js

@@ -686,12 +686,18 @@ module.exports = (function() {
         .success(function(data) {
           var result = data ? data[attributeSelector] : null
 
-          if (options && options.parseInt) {
-            result = parseInt(result, 10)
-          }
-
-          if (options && options.parseFloat) {
-            result = parseFloat(result)
+          if (options && options.dataType) {
+            var dataType = options.dataType;
+
+            if (dataType instanceof DataTypes.DECIMAL || dataType instanceof DataTypes.FLOAT) {
+              result = parseFloat(result);
+            } else if (dataType === DataTypes.INTEGER || dataType instanceof DataTypes.BIGINT) {
+              result = parseInt(result, 10);
+            } else if (dataType === DataTypes.DATE) {
+              result = new Date(result + 'Z');
+            } else if (dataType === DataTypes.STRING) {
+              // Nothing to do, result is already a string.
+            }
           }
 
           self.emit('rawSelect', null)

test/dao-factory.test.js

@@ -1036,6 +1036,27 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
         })
       })
     })
+
+    it("should allow strings in min", function(done) {
+      var self = this
+      this.User.bulkCreate([{username: 'bbb'}, {username: 'yyy'}]).success(function(){
+        self.User.min('username').success(function(min){
+          expect(min).to.equal('bbb')
+          done()
+        })
+      })
+    })
+
+    it("should allow dates in min", function(done){
+      var self = this
+      this.User.bulkCreate([{theDate: new Date(2000, 01, 01)}, {theDate: new Date(1990, 01, 01)}]).success(function(){
+        self.User.min('theDate').success(function(min){
+          expect(min).to.be.a('Date');
+          expect(new Date(1990, 01, 01)).to.equalDate(min)
+          done()
+        })
+      })
+    })
   })
 
   describe('max', function() {
@@ -1107,6 +1128,27 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
       })
     })
 
+    it("should allow dates in max", function(done) {
+      var self = this
+      this.User.bulkCreate([{theDate: new Date(2013, 12, 31)}, {theDate: new Date(2000, 01, 01)}]).success(function(){
+        self.User.max('theDate').success(function(max){
+          expect(max).to.be.a('Date');
+          expect(max).to.equalDate(new Date(2013, 12, 31))
+          done()
+        })
+      })
+    })
+
+    it("should allow strings in max", function(done) {
+      var self = this
+      this.User.bulkCreate([{username: 'aaa'}, {username: 'zzz'}]).success(function(){
+        self.User.max('username').success(function(max){
+          expect(max).to.equal('zzz')
+          done()
+        })
+      })
+    })
+
     it('allows sql logging', function(done) {
       this.UserWithAge.max('age').on('sql', function(sql) {
         expect(sql).to.exist

test/data-types.test.js

@@ -6,13 +6,43 @@ var chai      = require('chai')
 chai.Assertion.includeStack = true
 
 describe(Support.getTestDialectTeaser('DataTypes'), function() {
-  it('should return DECIMAL for the default decimal type', function(done) {
-    expect(Sequelize.DECIMAL.toString()).to.equal('DECIMAL')
+  it('should return false when comparing DECIMAL and DECIMAL(10,2)', function(done) {
+    expect(Sequelize.DECIMAL).to.not.equal(Sequelize.DECIMAL(10,2))
     done()
   })
 
-  it('should return DECIMAL(10,2) for the default decimal type with arguments', function(done) {
-    expect(Sequelize.DECIMAL(10, 2)).to.equal('DECIMAL(10,2)')
+  it('DECIMAL(10,2) should be an instance of DECIMAL', function(done) {
+    expect(Sequelize.DECIMAL(10,2)).to.be.an.instanceof(Sequelize.DECIMAL)
+    done()
+  })
+
+  it('should return false when comparing FLOAT and FLOAT(11)', function(done) {
+    expect(Sequelize.FLOAT).to.not.equal(Sequelize.FLOAT(11))
+    done()
+  })
+
+  it('FLOAT(11) should be an instance of FLOAT', function(done) {
+    expect(Sequelize.FLOAT(11)).to.be.an.instanceof(Sequelize.FLOAT)
+    done()
+  })
+
+  it('should return false when comparing STRING and STRING(4096)', function(done) {
+    expect(Sequelize.STRING).to.not.equal(Sequelize.STRING(4096))
+    done()
+  })
+
+  it('STRING(4096) should be an instance of STRING', function(done) {
+    expect(Sequelize.STRING(4096)).to.be.an.instanceof(Sequelize.STRING)
+    done()
+  })
+
+  it('should return false when comparing BIGINT and BIGINT(11)', function(done) {
+    expect(Sequelize.BIGINT).to.not.equal(Sequelize.BIGINT(11))
+    done()
+  })
+
+  it('BIGINT(11) should be an instance of BIGINT', function(done) {
+    expect(Sequelize.BIGINT(11)).to.be.an.instanceof(Sequelize.BIGINT)
     done()
   })
 
@@ -61,7 +91,10 @@ describe(Support.getTestDialectTeaser('DataTypes'), function() {
     [Sequelize.FLOAT(11, 12).UNSIGNED, 'FLOAT(11,12).UNSIGNED', 'FLOAT(11,12) UNSIGNED'],
     [Sequelize.FLOAT(11, 12).UNSIGNED.ZEROFILL,'FLOAT(11,12).UNSIGNED.ZEROFILL','FLOAT(11,12) UNSIGNED ZEROFILL'],
     [Sequelize.FLOAT(11, 12).ZEROFILL,'FLOAT(11,12).ZEROFILL', 'FLOAT(11,12) ZEROFILL'],
-    [Sequelize.FLOAT(11, 12).ZEROFILL.UNSIGNED,'FLOAT(11,12).ZEROFILL.UNSIGNED', 'FLOAT(11,12) UNSIGNED ZEROFILL']
+    [Sequelize.FLOAT(11, 12).ZEROFILL.UNSIGNED,'FLOAT(11,12).ZEROFILL.UNSIGNED', 'FLOAT(11,12) UNSIGNED ZEROFILL'],
+
+    [Sequelize.DECIMAL, 'DECIMAL', 'DECIMAL'],
+    [Sequelize.DECIMAL(10,2), 'DECIMAL(10,2)','DECIMAL(10,2)']
   ]
 
   tests.forEach(function(test) {