Merge pull request #69 from RiveraGroup/feature/mongo_2.6.5_expressions_Multiply

Feature/mongo 2.6.5 expressions Multiply

lib/pipeline/expressions/AndExpression.js

@@ -12,9 +12,9 @@
  * @constructor
  **/
 var AndExpression = module.exports = function AndExpression() {
-//	if (arguments.length !== 0) throw new Error("zero args expected");
+	if (arguments.length !== 0) throw new Error("zero args expected");
 	base.call(this);
-}, klass = AndExpression, base = require("./VariadicExpressionT")(AndExpression), proto = klass.prototype = Object.create(base.prototype, {constructor: {value: klass}});
+}, klass = AndExpression, base = require("./VariadicExpressionT")(klass), proto = klass.prototype = Object.create(base.prototype, {constructor: {value: klass}});
 
 // DEPENDENCIES
 var Value = require("../Value"),
@@ -35,11 +35,13 @@ proto.getOpName = function getOpName() {
 proto.evaluateInternal = function evaluateInternal(vars) {
 	for (var i = 0, n = this.operands.length; i < n; ++i) {
 		var value = this.operands[i].evaluateInternal(vars);
-		if (!Value.coerceToBool()) return false;
+		if (!Value.coerceToBool(value)) return false;
 	}
 	return true;
 };
 
+proto.isAssociativeAndCommutative = function isAssociativeAndCommutative() { return true; };
+
 proto.optimize = function optimize() {
 	var expr = base.prototype.optimize.call(this); //optimize the conjunction as much as possible
 
@@ -55,7 +57,7 @@ proto.optimize = function optimize() {
 	if (!(lastExpr instanceof ConstantExpression)) return expr;
 
 	// Evaluate and coerce the last argument to a boolean.  If it's false, then we can replace this entire expression.
-	var last = Value.coerceToBool(lastExpr.evaluate());
+	var last = Value.coerceToBool(lastExpr.evaluateInternal());
 	if (!last) return new ConstantExpression(false);
 
 	// If we got here, the final operand was true, so we don't need it anymore.

lib/pipeline/expressions/MultiplyExpression.js

@@ -7,36 +7,43 @@
  * @namespace mungedb-aggregate.pipeline.expressions
  * @module mungedb-aggregate
  * @constructor
- **/
+ */
 var MultiplyExpression = module.exports = function MultiplyExpression(){
-	//if (arguments.length !== 0) throw new Error("Zero args expected");
+if (arguments.length !== 0) throw new Error(klass.name + ": no args expected");
 	base.call(this);
 }, klass = MultiplyExpression, base = require("./VariadicExpressionT")(MultiplyExpression), proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
 
-// DEPENDENCIES
 var Value = require("../Value"),
- Expression = require("./Expression");
+	Expression = require("./Expression");
 
-// PROTOTYPE MEMBERS
-klass.opName = "$multiply";
-proto.getOpName = function getOpName(){
-	return klass.opName;
-};
+proto.evaluateInternal = function evaluateInternal(vars) {
+	var product = 1; //NOTE: DEVIATION FROM MONGO: no need to track narrowest so just use one var
 
-/**
- * Takes an array of one or more numbers and multiples them, returning the resulting product.
- * @method evaluateInternal
- **/
-proto.evaluateInternal = function evaluateInternal(vars){
-	var product = 1;
-	for(var i = 0, n = this.operands.length; i < n; ++i){
-		var value = this.operands[i].evaluateInternal(vars);
-		if(value instanceof Date) throw new Error("$multiply does not support dates; code 16375");
-		product *= Value.coerceToDouble(value);
+	var n = this.operands.length;
+	for (var i = 0; i < n; ++i) {
+		var val = this.operands[i].evaluateInternal(vars);
+
+		if (typeof val === "number") {
+			product *= Value.coerceToDouble(val);
+		} else if (val === undefined || val === null) {
+			return null;
+		} else {
+			throw new Error("$multiply only supports numeric types, not " +
+			 	Value.getType(val) + "; uasserted code 16555");
+		}
 	}
-	if(typeof(product) != "number") throw new Error("$multiply resulted in a non-numeric type; code 16418");
-	return product;
+
+	if (typeof product === "number")
+		return product;
+	throw new Error("$multiply resulted in a non-numeric type; massert code 16418");
 };
 
-/** Register Expression */
-Expression.registerExpression(klass.opName, base.parse);
+Expression.registerExpression("$multiply", base.parse);
+
+proto.getOpName = function getOpName(){
+	return "$multiply";
+};
+
+proto.isAssociativeAndCommutative = function isAssociativeAndCommutative() {
+	return true;
+};

test/lib/pipeline/expressions/AndExpression_test.js

@@ -1,6 +1,11 @@
 "use strict";
 var assert = require("assert"),
 	AndExpression = require("../../../../lib/pipeline/expressions/AndExpression"),
+	VariablesParseState = require("../../../../lib/pipeline/expressions/VariablesParseState"),
+	VariablesIdGenerator = require("../../../../lib/pipeline/expressions/VariablesIdGenerator"),
+	CoerceToBoolExpression = require("../../../../lib/pipeline/expressions/CoerceToBoolExpression"),
+	ConstantExpression = require("../../../../lib/pipeline/expressions/ConstantExpression"),
+	FieldPathExpression = require("../../../../lib/pipeline/expressions/FieldPathExpression"),
 	Expression = require("../../../../lib/pipeline/expressions/Expression");
 
 
@@ -8,6 +13,10 @@ module.exports = {
 
 	"AndExpression": {
 
+		beforeEach: function() {
+			this.vps = new VariablesParseState(new VariablesIdGenerator());
+		},
+
 		"constructor()": {
 
 			"should not throw Error when constructing without args": function testConstructor(){
@@ -36,51 +45,51 @@ module.exports = {
 		"#evaluate()": {
 
 			"should return true if no operands were given; {$and:[]}": function testEmpty(){
-				assert.equal(Expression.parseOperand({$and:[]}).evaluate(), true);
+				assert.equal(Expression.parseOperand({$and:[]},this.vps).evaluate(), true);
 			},
 
 			"should return true if operands is one true; {$and:[true]}": function testTrue(){
-				assert.equal(Expression.parseOperand({$and:[true]}).evaluate(), true);
+				assert.equal(Expression.parseOperand({$and:[true]},this.vps).evaluate(), true);
 			},
 
 			"should return false if operands is one false; {$and:[false]}": function testFalse(){
-				assert.equal(Expression.parseOperand({$and:[false]}).evaluate(), false);
+				assert.equal(Expression.parseOperand({$and:[false]},this.vps).evaluate(), false);
 			},
 
 			"should return true if operands are true and true; {$and:[true,true]}": function testTrueTrue(){
-				assert.equal(Expression.parseOperand({$and:[true,true]}).evaluate(), true);
+				assert.equal(Expression.parseOperand({$and:[true,true]},this.vps).evaluate(), true);
 			},
 
 			"should return false if operands are true and false; {$and:[true,false]}": function testTrueFalse(){
-				assert.equal(Expression.parseOperand({$and:[true,false]}).evaluate(), false);
+				assert.equal(Expression.parseOperand({$and:[true,false]},this.vps).evaluate(), false);
 			},
 
 			"should return false if operands are false and true; {$and:[false,true]}": function testFalseTrue(){
-				assert.equal(Expression.parseOperand({$and:[false,true]}).evaluate(), false);
+				assert.equal(Expression.parseOperand({$and:[false,true]},this.vps).evaluate(), false);
 			},
 
 			"should return false if operands are false and false; {$and:[false,false]}": function testFalseFalse(){
-				assert.equal(Expression.parseOperand({$and:[false,false]}).evaluate(), false);
+				assert.equal(Expression.parseOperand({$and:[false,false]},this.vps).evaluate(), false);
 			},
 
 			"should return true if operands are true, true, and true; {$and:[true,true,true]}": function testTrueTrueTrue(){
-				assert.equal(Expression.parseOperand({$and:[true,true,true]}).evaluate(), true);
+				assert.equal(Expression.parseOperand({$and:[true,true,true]},this.vps).evaluate(), true);
 			},
 
 			"should return false if operands are true, true, and false; {$and:[true,true,false]}": function testTrueTrueFalse(){
-				assert.equal(Expression.parseOperand({$and:[true,true,false]}).evaluate(), false);
+				assert.equal(Expression.parseOperand({$and:[true,true,false]},this.vps).evaluate(), false);
 			},
 
 			"should return false if operands are 0 and 1; {$and:[0,1]}": function testZeroOne(){
-				assert.equal(Expression.parseOperand({$and:[0,1]}).evaluate(), false);
+				assert.equal(Expression.parseOperand({$and:[0,1]},this.vps).evaluate(), false);
 			},
 
 			"should return false if operands are 1 and 2; {$and:[1,2]}": function testOneTwo(){
-				assert.equal(Expression.parseOperand({$and:[1,2]}).evaluate(), true);
+				assert.equal(Expression.parseOperand({$and:[1,2]},this.vps).evaluate(), true);
 			},
 
 			"should return true if operand is a path String to a truthy value; {$and:['$a']}": function testFieldPath(){
-				assert.equal(Expression.parseOperand({$and:['$a']}).evaluate({a:1}), true);
+				assert.equal(Expression.parseOperand({$and:['$a']},this.vps).evaluate({a:1}), true);
 			}
 
 		},
@@ -88,50 +97,113 @@ module.exports = {
 		"#optimize()": {
 
 			"should optimize a constant expression to a constant; {$and:[1]} == true": function testOptimizeConstantExpression(){
-				assert.deepEqual(Expression.parseOperand({$and:[1]}).optimize().toJSON(true), {$const:true});
+				var a = Expression.parseOperand({$and:[1]}, this.vps).optimize();
+				assert.equal(a.operands.length, 0, "The operands should have been optimized away");
+				assert.equal(a.evaluateInternal(), true);
 			},
 
 			"should not optimize a non-constant expression; {$and:['$a']}": function testNonConstant(){
-				assert.deepEqual(Expression.parseOperand({$and:['$a']}).optimize().toJSON(), {$and:['$a']});
+				var a = Expression.parseOperand({$and:['$a']}, this.vps).optimize();
+				assert.equal(a.operands[0]._fieldPath.fieldNames.length, 2);
+				assert.deepEqual(a.operands[0]._fieldPath.fieldNames[0], "CURRENT");
+				assert.deepEqual(a.operands[0]._fieldPath.fieldNames[1], "a");
 			},
 
-			"optimize an expression beginning with a constant; {$and:[1,'$a']};": function testConstantNonConstant(){
-				assert.deepEqual(Expression.parseOperand({$and:[1,'$a']}).optimize().toJSON(), {$and:[1,'$a']});
-				assert.notEqual(Expression.parseOperand({$and:[1,'$a']}).optimize().toJSON(), {$and:[0,'$a']});
+			"should not optimize an expression ending with a non-constant. {$and:[1,'$a']};": function testConstantNonConstant(){
+				var a = Expression.parseOperand({$and:[1,'$a']}, this.vps).optimize();
+				assert(a instanceof CoerceToBoolExpression);
+				assert(a.expression instanceof FieldPathExpression);
+
+				assert.equal(a.expression._fieldPath.fieldNames.length, 2);
+				assert.equal(a.expression._fieldPath.fieldNames[0], "CURRENT");
+				assert.equal(a.expression._fieldPath.fieldNames[1], "a");
 			},
 
-			"should optimize an expression with a path and a '1' (is entirely constant); {$and:['$a',1]}": function testNonConstantOne(){
-				assert.deepEqual(Expression.parseOperand({$and:['$a',1]}).optimize().toJSON(), {$and:['$a']});
+			"should optimize an expression with a path and a '1'; {$and:['$a',1]}": function testNonConstantOne(){
+				var a = Expression.parseOperand({$and:['$a', 1]}, this.vps).optimize();
+				// The 1 should be removed as it is redundant.
+				assert(a instanceof CoerceToBoolExpression, "The result should be forced to a boolean");
+
+				// This is the '$a' which cannot be optimized.
+				assert.equal(a.expression._fieldPath.fieldNames.length, 2);
+				assert.equal(a.expression._fieldPath.fieldNames[0], "CURRENT");
+				assert.equal(a.expression._fieldPath.fieldNames[1], "a");
 			},
 
 			"should optimize an expression with a field path and a '0'; {$and:['$a',0]}": function testNonConstantZero(){
-				assert.deepEqual(Expression.parseOperand({$and:['$a',0]}).optimize().toJSON(true), {$const:false});
+				var a = Expression.parseOperand({$and:['$a',0]}, this.vps).optimize();
+				assert.equal(a.operands.length, 0, "The operands should have been optimized away");
+				assert.equal(a.evaluateInternal(), false, "The 0 operand should have been converted to false");
 			},
 
 			"should optimize an expression with two field paths and '1'; {$and:['$a','$b',1]}": function testNonConstantNonConstantOne(){
-				assert.deepEqual(Expression.parseOperand({$and:['$a','$b',1]}).optimize().toJSON(), {$and:['$a','$b']});
+				var a = Expression.parseOperand({$and:['$a', '$b', 1]}, this.vps).optimize();
+				assert.equal(a.operands.length, 2, "Two operands should remain.");
+
+				// This is the '$a' which cannot be optimized.
+				assert.deepEqual(a.operands[0]._fieldPath.fieldNames.length, 2);
+				assert.deepEqual(a.operands[0]._fieldPath.fieldNames[0], "CURRENT");
+				assert.deepEqual(a.operands[0]._fieldPath.fieldNames[1], "a");
+
+				// This is the '$b' which cannot be optimized.
+				assert.deepEqual(a.operands[1]._fieldPath.fieldNames.length, 2);
+				assert.deepEqual(a.operands[1]._fieldPath.fieldNames[0], "CURRENT");
+				assert.deepEqual(a.operands[1]._fieldPath.fieldNames[1], "b");
 			},
 
 			"should optimize an expression with two field paths and '0'; {$and:['$a','$b',0]}": function testNonConstantNonConstantZero(){
-				assert.deepEqual(Expression.parseOperand({$and:['$a','$b',0]}).optimize().toJSON(true), {$const:false});
+				var a = Expression.parseOperand({$and:['$a', '$b', 0]}, this.vps).optimize();
+				assert(a instanceof ConstantExpression, "With that trailing false, we know the result...");
+				assert.equal(a.operands.length, 0, "The operands should have been optimized away");
+				assert.equal(a.evaluateInternal(), false);
 			},
 
 			"should optimize an expression with '0', '1', and a field path; {$and:[0,1,'$a']}": function testZeroOneNonConstant(){
-				assert.deepEqual(Expression.parseOperand({$and:[0,1,'$a']}).optimize().toJSON(true), {$const:false});
+				var a = Expression.parseOperand({$and:[0,1,'$a']}, this.vps).optimize();
+				assert(a instanceof ConstantExpression);
+				assert.equal(a.evaluateInternal(), false);
 			},
 
 			"should optimize an expression with '1', '1', and a field path; {$and:[1,1,'$a']}": function testOneOneNonConstant(){
-				assert.deepEqual(Expression.parseOperand({$and:[1,1,'$a']}).optimize().toJSON(), {$and:['$a']});
+				var a = Expression.parseOperand({$and:[1,1,'$a']}, this.vps).optimize();
+				assert(a instanceof CoerceToBoolExpression);
+				assert(a.expression instanceof FieldPathExpression);
+
+				assert.equal(a.expression._fieldPath.fieldNames.length, 2);
+				assert.equal(a.expression._fieldPath.fieldNames[0], "CURRENT");
+				assert.equal(a.expression._fieldPath.fieldNames[1], "a");
 			},
 
 			"should optimize nested $and expressions properly and optimize out values evaluating to true; {$and:[1,{$and:[1]},'$a','$b']}": function testNested(){
-				assert.deepEqual(Expression.parseOperand({$and:[1,{$and:[1]},'$a','$b']}).optimize().toJSON(), {$and:['$a','$b']});
+				var a = Expression.parseOperand({$and:[1,{$and:[1]},'$a','$b']}, this.vps).optimize();
+				assert.equal(a.operands.length, 2)
+				assert(a.operands[0] instanceof FieldPathExpression);
+				assert(a.operands[1] instanceof FieldPathExpression);
 			},
 
 			"should optimize nested $and expressions containing a nested value evaluating to false; {$and:[1,{$and:[1]},'$a','$b']}": function testNested(){
-				assert.deepEqual(Expression.parseOperand({$and:[1,{$and:[{$and:[0]}]},'$a','$b']}).optimize().toJSON(true), {$const:false});
+				//assert.deepEqual(Expression.parseOperand({$and:[1,{$and:[{$and:[0]}]},'$a','$b']}, this.vps).optimize().toJSON(true), {$const:false});
+				var a = Expression.parseOperand({$and:[1,{$and:[{$and:[0]}]},'$a','$b']}, this.vps).optimize();
+				assert(a instanceof ConstantExpression);
+				assert.equal(a.evaluateInternal(), false);
+			},
+
+			"should optimize when the constants are on the right of the operand list. The rightmost is true": function(){
+				// 1, "x", and 1 are all true.  They should be optimized away.
+				var a = Expression.parseOperand({$and:['$a', 1, "x", 1]}, this.vps).optimize();
+				assert(a instanceof CoerceToBoolExpression);
+				assert(a.expression instanceof FieldPathExpression);
+
+				assert.equal(a.expression._fieldPath.fieldNames.length, 2);
+				assert.equal(a.expression._fieldPath.fieldNames[0], "CURRENT");
+				assert.equal(a.expression._fieldPath.fieldNames[1], "a");
+			},
+			"should optimize when the constants are on the right of the operand list. The rightmost is false": function(){
+				// 1, "x", and 1 are all true.  They should be optimized away.
+				var a = Expression.parseOperand({$and:['$a', 1, "x", 0]}, this.vps).optimize();
+				assert(a instanceof ConstantExpression, "The rightmost false kills it all");
+				assert.equal(a.evaluateInternal(), false);
 			}
-
 		}
 
 	}

test/lib/pipeline/expressions/MultiplyExpression_test.js

@@ -1,51 +1,112 @@
 "use strict";
 var assert = require("assert"),
 	MultiplyExpression = require("../../../../lib/pipeline/expressions/MultiplyExpression"),
+	VariablesParseState = require("../../../../lib/pipeline/expressions/VariablesParseState"),
+	VariablesIdGenerator = require("../../../../lib/pipeline/expressions/VariablesIdGenerator"),
+	FieldPathExpression = require("../../../../lib/pipeline/expressions/FieldPathExpression"),
+	ConstantExpression = require("../../../../lib/pipeline/expressions/ConstantExpression"),
 	Expression = require("../../../../lib/pipeline/expressions/Expression");
 
+// Mocha one-liner to make these tests self-hosted
+if(!module.parent)return(require.cache[__filename]=null,(new(require("mocha"))({ui:"exports",reporter:"spec",grep:process.env.TEST_GREP})).addFile(__filename).run(process.exit));
 
-module.exports = {
+exports.MultiplyExpression = {
 
-	"MultiplyExpression": {
+	beforeEach: function(){
+		this.vps = new VariablesParseState(new VariablesIdGenerator());
+	},
 
-		"constructor()": {
+	"constructor()": {
 
-			"should not throw Error when constructing without args": function testConstructor(){
-				assert.doesNotThrow(function(){
-					new MultiplyExpression();
-				});
-			},
+		"should not throw Error when constructing without args": function() {
+			assert.doesNotThrow(function(){
+				new MultiplyExpression();
+			});
+		},
+
+		"should throw Error when constructing with args": function() {
+			assert.throws(function(){
+				new MultiplyExpression(1);
+			});
+		},
+
+	},
 
-			"should throw Error when constructing with args": function testConstructor(){
-				assert.throws(function(){
-					new MultiplyExpression(1);
-				});
-			}
+	"#getOpName()": {
 
+		"should return the correct op name; $multiply": function() {
+			assert.equal(new MultiplyExpression().getOpName(), "$multiply");
 		},
 
-		"#getOpName()": {
+	},
+
+	"#evaluate()": {
+
+		"should multiply constants": function () {
+			assert.strictEqual(Expression.parseOperand({$multiply: [2, 3, 4]}, this.vps).evaluate(), 2 * 3 * 4);
+		},
 
-			"should return the correct op name; $multiply": function testOpName(){
-				assert.equal(new MultiplyExpression().getOpName(), "$multiply");
-			}
+		"should 'splode if an operand is a string": function () {
+			assert.throws(function () {
+				Expression.parseOperand({$multiply: [2, "x", 4]}, this.vps).evaluate();
+			});
+		},
 
+		"should 'splode if an operand is a boolean": function () {
+			assert.throws(function () {
+				Expression.parseOperand({$multiply: [2, "x", 4]}, this.vps).evaluate();
+			});
 		},
 
-		"#evaluate()": {
+		"should 'splode if an operand is a date": function () {
+			assert.throws(function () {
+				Expression.parseOperand({$multiply: [2, "x", 4]}, this.vps).evaluate();
+			});
+		},
 
-			"should return result of multiplying numbers": function testStuff(){
-				assert.strictEqual(Expression.parseOperand({$multiply:["$a", "$b"]}).evaluateInternal({a:1, b:2}), 1*2);
-				assert.strictEqual(Expression.parseOperand({$multiply:["$a", "$b", "$c"]}).evaluateInternal({a:1.345, b:2e45, c:0}), 1.345*2e45*0);
-				assert.strictEqual(Expression.parseOperand({$multiply:["$a"]}).evaluateInternal({a:1}), 1);
-			},
-			"should throw an exception if the result is not a number": function testStuff(){
-				assert.throws(Expression.parseOperand({$multiply:["$a", "$b"]}).evaluateInternal({a:1e199, b:1e199}));
-			}
-		}
+		"should handle a null operand": function(){
+			assert.strictEqual(Expression.parseOperand({$multiply: [2, null]}, this.vps).evaluate(), null);
+		},
 
-	}
+		"should handle an undefined operand": function(){
+			assert.strictEqual(Expression.parseOperand({$multiply: [2, undefined]}, this.vps).evaluate(), null);
+		},
 
-};
+		"should multiply mixed numbers": function () {
+			assert.strictEqual(Expression.parseOperand({$multiply: [2.1, 3, 4.4]}, this.vps).evaluate(), 2.1 * 3 * 4.4);
+		},
 
-if (!module.parent)(new(require("mocha"))()).ui("exports").reporter("spec").addFile(__filename).run(process.exit);
+		"should return result of multiplying simple variables": function () {
+			assert.equal(Expression.parseOperand({$multiply: ["$a", "$b"]}, this.vps).evaluate({a: 1, b: 2}), 1 * 2);
+		},
+
+		"should return result of multiplying large variables": function () {
+			assert.strictEqual(Expression.parseOperand({$multiply: ["$a", "$b", "$c"]}, this.vps).evaluate({a: 1.345, b: 2e45, c: 0}), 1.345 * 2e45 * 0);
+		},
+
+		"should return result of multiplying one number": function () {
+			assert.strictEqual(Expression.parseOperand({$multiply: ["$a"]}, this.vps).evaluate({a: 1}), 1);
+		},
+
+		"should throw an exception if the result is not a number": function () {
+			assert.throws(function() {
+				Expression.parseOperand({$multiply: ["$a", "$b"]}, this.vps).evaluate({a: 1e199, b: 1e199});
+			});
+		},
+
+	},
+
+	"optimize": {
+
+		"should optimize out constants separated by a variable": function () {
+			var a = Expression.parseOperand({$multiply: [2, 3, 4, 5, '$a', 6, 7, 8]}, this.vps).optimize();
+			assert(a instanceof MultiplyExpression);
+			assert.equal(a.operands.length, 2);
+			assert(a.operands[0] instanceof FieldPathExpression);
+			assert(a.operands[1] instanceof ConstantExpression);
+			assert.equal(a.operands[1].evaluateInternal(), 2*3*4*5*6*7*8);
+		},
+
+	},
+
+};