Browse Source

Merge branch 'release/v0.6.0+2013.11.04'

* release/v0.6.0+2013.11.04:
  Refs #3836. Fixed null compared to null bug in Value.compare
  Refs #3836. Fixed bug in Value.compare
  Refs #3645: update package.json to use release branch for version v0.6.0+2013.11.04.
  refs #3571 #brought code closer to c src
  Refs #3488: update package.json to new version 0.6.0+2013.11.04.

Conflicts:
	package.json
Charles Ezell 12 years ago
parent
commit
ad972b1dce
4 changed files with 100 additions and 30 deletions
  1. 75 24
      lib/pipeline/Value.js
  2. 2 2
      package.json
  3. 21 0
      test/lib/aggregate.js
  4. 2 4
      test/lib/pipeline/expressions/FieldRangeExpression.js

+ 75 - 24
lib/pipeline/Value.js

@@ -69,6 +69,64 @@ klass.coerceToString = function coerceToString(value) {
 		throw new Error("can't convert from BSON type " + typeof(value) + " to String; uassert code 16007");
 	}
 };
+
+
+klass.canonicalize = function canonicalize(x) {
+	var xType = typeof(x);
+	if(xType == "object") xType = x === null ? "null" : x.constructor.name;
+	switch (xType) {
+		case "MinKey":
+			return -1;
+		case "MaxKey":
+			return 127;
+		case "EOO":
+		case "undefined":
+		case undefined:
+			return 0;
+		case "jstNULL":
+		case "null":
+		case "Null":
+			return 5;
+		case "NumberDouble":
+		case "NumberInt":
+		case "NumberLong":
+		case "number":
+			return 10;
+		case "Symbol":
+		case "string":
+			return 15;
+		case "Object":
+			return 20;
+		case "Array":
+			return 25;
+		case "BinData":
+			return 30;
+		case "jstOID":
+			return 35;
+		case "boolean":
+		case "Boolean":
+			return 40;
+		case "Date":
+		case "Timestamp":
+			return 45;
+		case "RegEx":
+		case "RegExp":
+			return 50;
+		case "DBRef":
+			return 55;
+		case "Code":
+			return 60;
+		case "CodeWScope":
+			return 65;
+		default:
+			throw new Error("Unexpected type in mongodb-aggregate canonicalize");
+	}
+};
+
+klass.cmp = function cmp(l, r){
+	return l < r ? -1 : l > r ? 1 : 0;
+};
+
 //TODO:	klass.coerceToTimestamp = ...?
 
 /**
@@ -82,34 +140,24 @@ klass.coerceToString = function coerceToString(value) {
  **/
 var Document;  // loaded lazily below //TODO: a dirty hack; need to investigate and clean up
 klass.compare = function compare(l, r) {
-	var lt = typeof(l),
-		rt = typeof(r);
+	//NOTE: deviation from mongo code: we have to do some coercing for null "types" because of javascript
+	var lt = l === null ? "null" : typeof(l),
+		rt = r === null ? "null" : typeof(r),
+		ret;
+
+	// NOTE: deviation from mongo code: javascript types do not work quite the same, so for proper results we always canonicalize, and we don't need the "speed" hack
+	ret = (klass.cmp(klass.canonicalize(l), klass.canonicalize(r)));
+
+	if(ret !== 0) return ret;
 
-	// Special handling for Undefined and NULL values ...
-	if (lt === "undefined") {
-		if (rt === "undefined") return 0;
-		return -1;
-	}
-	if (l === null) {
-		if (rt === "undefined") return 1;
-		if (r === null) return 0;
-		return -1;
-	}
-	// We know the left value isn't Undefined, because of the above. Count a NULL value as greater than an undefined one.
-	if (rt === "undefined" || r === null) return 1;
 	// Numbers
 	if (lt === "number" && rt === "number"){
 		//NOTE: deviation from Mongo code: they handle NaN a bit differently
 		if (isNaN(l)) return isNaN(r) ? 0 : -1;
 		if (isNaN(r)) return 1;
-		return l < r ? -1 : l > r ? 1 : 0;
-	}
-	// hack: These should really get converted to their BSON type ids and then compared, we use int vs object in queries
-	if (lt === "number" && rt === "object"){
-		return -1;
-	} else if (lt === "object" && rt === "number") {
-		return 1;
+		return klass.cmp(l,r);
 	}
+
 	// CW TODO for now, only compare like values
 	if (lt !== rt) throw new Error("can't compare values of BSON types [" + lt + " " + l.constructor.name + "] and [" + rt + ":" + r.constructor.name + "]; code 16016");
 	// Compare everything else
@@ -117,9 +165,12 @@ klass.compare = function compare(l, r) {
 	case "number":
 		throw new Error("number types should have been handled earlier!");
 	case "string":
-		return l < r ? -1 : l > r ? 1 : 0;
+		return klass.cmp(l,r);
 	case "boolean":
 		return l == r ? 0 : l ? 1 : -1;
+	case "undefined": //NOTE: deviation from mongo code: we are comparing null to null or undefined to undefined (otherwise the ret stuff above would have caught it)
+	case "null":
+		return 0;
 	case "object":
 		if (l instanceof Array) {
 			for (var i = 0, ll = l.length, rl = r.length; true ; ++i) {
@@ -134,8 +185,8 @@ klass.compare = function compare(l, r) {
 
 			throw new Error("logic error in Value.compare for Array types!");
 		}
-		if (l instanceof Date) return l < r ? -1 : l > r ? 1 : 0;
-		if (l instanceof RegExp) return l < r ? -1 : l > r ? 1 : 0;
+		if (l instanceof Date) return klass.cmp(l,r);
+		if (l instanceof RegExp) return klass.cmp(l,r);
 		if (Document === undefined) Document = require("./Document");	//TODO: a dirty hack; need to investigate and clean up
 		return Document.compare(l, r);
 	default:

+ 2 - 2
package.json

@@ -1,6 +1,6 @@
 {
   "name": "mungedb-aggregate",
-  "version": "0.5.9+2013.10.21",
+  "version": "0.6.0+2013.11.04",
   "description": "A JavaScript data aggregation pipeline based on the MongoDB aggregation framework.",
   "author": "Rivera Group <support@riverainc.com>",
   "contributors": [
@@ -15,7 +15,7 @@
     "test": "npm_scripts/test/test.sh"
   },
   "repository": {
-    "url": "git+https://source.rd.rcg.local/git/private/mungedb-aggregate.git#master"
+    "url": "git+https://source.rd.rcg.local/git/private/mungedb-aggregate.git#release/v0.6.0+2013.11.04"
   },
   "keywords": [
     "manipulation",

+ 21 - 0
test/lib/aggregate.js

@@ -245,6 +245,27 @@ module.exports = {
 			});
 		}
 
+		"should be able to successfully compare a null to a null": function(next){
+			testAggregate({
+				inputs: [
+					{
+						cond:null,
+						value:"Authorized"
+					}
+				],
+				pipeline: [
+					{$project:{
+						retValue:{$cond:[
+							{$eq:["$cond", null]},
+							"$value",
+							null
+						]}
+					}}
+				],
+				expected: [{"retValue":"Authorized"}],
+				next: next
+			});
+		},
 	}
 
 };

+ 2 - 4
test/lib/pipeline/expressions/FieldRangeExpression.js

@@ -102,10 +102,8 @@ module.exports = {
 			},
 
 			"should throw Error if given multikey values": function testMultikey(){
-				assert.throws(function(){
-					new FieldRangeExpression(new FieldPathExpression("a"), "$eq", 0).evaluate({a:[1,0,2]});
-				});
-			}
+				assert.strictEqual(new FieldRangeExpression(new FieldPathExpression("a"), "$eq", 0).evaluate({a:[1,0,2]}), false);
+            }
 
 		},