Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

SERVER-6147 fix aggro $ne to behave with constants

  • Loading branch information...
commit 36c78176a97e52874850d38d08ac5f8019055325 1 parent 9bfb82e
Matt Dannenberg dannenberg authored RedBeard0531 committed
59 jstests/aggregation/bugs/server6147.js
View
@@ -0,0 +1,59 @@
+/*
+ * SERVER-6147 : aggregation $ne expression applied to constant returns incorrect result
+ *
+ * This test validates the SERVER-6147 ticket. Return true when comparing a constant to a field
+ * containing a different value using $ne. Previously it would return false when comparing a
+ * constant and a field regardless of whether they were equal or not.
+ */
+
+/*
+ * 1) Clear and create testing db
+ * 2) Run an aggregation with $ne comparing constants and fields in various configurations
+ * 3) Assert that the result is what we expected
+ */
+
+// Load the test utilities
+load('jstests/aggregation/extras/utils.js');
+
+// Clear db
+db.s6147.drop();
+
+// Populate db
+db.s6147.save({a:1});
+db.s6147.save({a:2});
+
+// Aggregate checking various combinations of the constant and the field
+var s6147 = db.runCommand(
+{ aggregate: "s6147", pipeline : [
+ { $project : {
+ _id : 0,
+ constantAndField : { $ne: [1, "$a"] },
+ fieldAndConstant : { $ne: ["$a", 1] },
+ constantAndConstant : { $ne: [1, 1] },
+ fieldAndField : { $ne: ["$a", "$a"] }
+ }}
+]});
+
+/*
+ * In both documents the constantAndConstant and fieldAndField should be false since they compare
+ * something with itself but the constantAndField and fieldAndConstant should be different as
+ * document one contains 1 which should return false and document 2 contains something different so
+ * should return true
+ */
+var s6147result = [
+ {
+ constantAndField : false,
+ fieldAndConstant : false,
+ constantAndConstant : false,
+ fieldAndField : false
+ },
+ {
+ constantAndField : true,
+ fieldAndConstant : true,
+ constantAndConstant : false,
+ fieldAndField : false
+ }
+];
+
+// Assert
+assert(arrayEq(s6147.result, s6147result), 's6147 failed');
1  jstests/aggregation/testbugs.js
View
@@ -17,3 +17,4 @@ load('jstests/aggregation/bugs/server5782.js');
load('jstests/aggregation/bugs/server5973.js');
load('jstests/aggregation/bugs/server6045.js');
load('jstests/aggregation/bugs/server6127.js');
+load('jstests/aggregation/bugs/server6147.js');
5 src/mongo/db/pipeline/expression.cpp
View
@@ -747,8 +747,9 @@ namespace mongo {
/* check to see if optimizing comparison operator is supported */
CmpOp newOp = pCmp->cmpOp;
- if (newOp == CMP)
- return pE; // not reversible: there's nothing more we can do
+ // CMP and NE cannot use ExpressionFieldRange which is what this optimization uses
+ if (newOp == CMP || newOp == NE)
+ return pE;
/*
There's one localized optimization we recognize: a comparison
Please sign in to comment.
Something went wrong with that request. Please try again.