summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorMinji <minjikim0202@gmail.com>2018-06-20 11:24:30 -0400
committerMinji <minjikim0202@gmail.com>2018-07-02 17:05:40 -0400
commitb5291b87ab3350bff9bd9ef4464a63dcda81ac21 (patch)
tree43f6bfe87e4a51419466c14a1ced26553bab2d25 /src
parenta9eab0049a47799e310853e9cb21812c77e4a13c (diff)
downloadmongo-b5291b87ab3350bff9bd9ef4464a63dcda81ac21.tar.gz
SERVER-18341 Matcher returns false positives in comparison predicates with MinKey/MaxKey
Diffstat (limited to 'src')
-rw-r--r--src/mongo/db/matcher/expression_leaf.cpp29
-rw-r--r--src/mongo/db/matcher/expression_leaf_test.cpp140
2 files changed, 134 insertions, 35 deletions
diff --git a/src/mongo/db/matcher/expression_leaf.cpp b/src/mongo/db/matcher/expression_leaf.cpp
index 7226d9111ae..b2c92ed892d 100644
--- a/src/mongo/db/matcher/expression_leaf.cpp
+++ b/src/mongo/db/matcher/expression_leaf.cpp
@@ -115,14 +115,34 @@ ComparisonMatchExpression::ComparisonMatchExpression(MatchType type,
bool ComparisonMatchExpression::matchesSingleElement(const BSONElement& e,
MatchDetails* details) const {
if (e.canonicalType() != _rhs.canonicalType()) {
- // some special cases
- // jstNULL and undefined are treated the same
+ // We can't call 'compareElements' on elements of different canonical types. Usually
+ // elements with different canonical types should never match any comparison, but there are
+ // a few exceptions, handled here.
+
+ // jstNULL and undefined are treated the same
if (e.canonicalType() + _rhs.canonicalType() == 5) {
return matchType() == EQ || matchType() == LTE || matchType() == GTE;
}
-
if (_rhs.type() == MaxKey || _rhs.type() == MinKey) {
- return matchType() != EQ;
+ switch (matchType()) {
+ // LT and LTE need no distinction here because the two elements that we are
+ // comparing do not even have the same canonical type and are thus not equal
+ // (i.e.the case where we compare MinKey against MinKey would not reach this switch
+ // statement at all). The same reasoning follows for the lack of distinction
+ // between GTE and GT.
+ case LT:
+ case LTE:
+ return _rhs.type() == MaxKey;
+ case EQ:
+ return false;
+ case GT:
+ case GTE:
+ return _rhs.type() == MinKey;
+ default:
+ // This is a comparison match expression, so it must be either
+ // a $lt, $lte, $gt, $gte, or equality expression.
+ MONGO_UNREACHABLE;
+ }
}
return false;
}
@@ -151,7 +171,6 @@ bool ComparisonMatchExpression::matchesSingleElement(const BSONElement& e,
int x = BSONElement::compareElements(
e, _rhs, BSONElement::ComparisonRules::kConsiderFieldName, _collator);
-
switch (matchType()) {
case LT:
return x < 0;
diff --git a/src/mongo/db/matcher/expression_leaf_test.cpp b/src/mongo/db/matcher/expression_leaf_test.cpp
index 762ffa21279..235e2af641f 100644
--- a/src/mongo/db/matcher/expression_leaf_test.cpp
+++ b/src/mongo/db/matcher/expression_leaf_test.cpp
@@ -164,18 +164,34 @@ TEST(EqOp, MatchesNestedNull) {
TEST(EqOp, MatchesMinKey) {
BSONObj operand = BSON("a" << MinKey);
EqualityMatchExpression eq("a", operand["a"]);
- ASSERT(eq.matchesBSON(BSON("a" << MinKey), NULL));
- ASSERT(!eq.matchesBSON(BSON("a" << MaxKey), NULL));
- ASSERT(!eq.matchesBSON(BSON("a" << 4), NULL));
+ BSONObj minKeyObj = BSON("a" << MinKey);
+ BSONObj maxKeyObj = BSON("a" << MaxKey);
+ BSONObj numObj = BSON("a" << 4);
+
+ ASSERT(eq.matchesBSON(minKeyObj, NULL));
+ ASSERT(!eq.matchesBSON(maxKeyObj, NULL));
+ ASSERT(!eq.matchesBSON(numObj, NULL));
+
+ ASSERT(eq.matchesSingleElement(minKeyObj.firstElement()));
+ ASSERT(!eq.matchesSingleElement(maxKeyObj.firstElement()));
+ ASSERT(!eq.matchesSingleElement(numObj.firstElement()));
}
TEST(EqOp, MatchesMaxKey) {
BSONObj operand = BSON("a" << MaxKey);
EqualityMatchExpression eq("a", operand["a"]);
- ASSERT(eq.matchesBSON(BSON("a" << MaxKey), NULL));
- ASSERT(!eq.matchesBSON(BSON("a" << MinKey), NULL));
- ASSERT(!eq.matchesBSON(BSON("a" << 4), NULL));
+ BSONObj minKeyObj = BSON("a" << MinKey);
+ BSONObj maxKeyObj = BSON("a" << MaxKey);
+ BSONObj numObj = BSON("a" << 4);
+
+ ASSERT(!eq.matchesBSON(minKeyObj, NULL));
+ ASSERT(eq.matchesBSON(maxKeyObj, NULL));
+ ASSERT(!eq.matchesBSON(numObj, NULL));
+
+ ASSERT(!eq.matchesSingleElement(minKeyObj.firstElement()));
+ ASSERT(eq.matchesSingleElement(maxKeyObj.firstElement()));
+ ASSERT(!eq.matchesSingleElement(numObj.firstElement()));
}
TEST(EqOp, MatchesFullArray) {
@@ -312,17 +328,33 @@ TEST(LtOp, MatchesDotNotationNull) {
TEST(LtOp, MatchesMinKey) {
BSONObj operand = BSON("a" << MinKey);
LTMatchExpression lt("a", operand["a"]);
- ASSERT(!lt.matchesBSON(BSON("a" << MinKey), NULL));
- ASSERT(!lt.matchesBSON(BSON("a" << MaxKey), NULL));
- ASSERT(!lt.matchesBSON(BSON("a" << 4), NULL));
+ BSONObj minKeyObj = BSON("a" << MinKey);
+ BSONObj maxKeyObj = BSON("a" << MaxKey);
+ BSONObj numObj = BSON("a" << 4);
+
+ ASSERT(!lt.matchesBSON(minKeyObj, NULL));
+ ASSERT(!lt.matchesBSON(maxKeyObj, NULL));
+ ASSERT(!lt.matchesBSON(numObj, NULL));
+
+ ASSERT(!lt.matchesSingleElement(minKeyObj.firstElement()));
+ ASSERT(!lt.matchesSingleElement(maxKeyObj.firstElement()));
+ ASSERT(!lt.matchesSingleElement(numObj.firstElement()));
}
TEST(LtOp, MatchesMaxKey) {
BSONObj operand = BSON("a" << MaxKey);
LTMatchExpression lt("a", operand["a"]);
- ASSERT(!lt.matchesBSON(BSON("a" << MaxKey), NULL));
- ASSERT(lt.matchesBSON(BSON("a" << MinKey), NULL));
- ASSERT(lt.matchesBSON(BSON("a" << 4), NULL));
+ BSONObj minKeyObj = BSON("a" << MinKey);
+ BSONObj maxKeyObj = BSON("a" << MaxKey);
+ BSONObj numObj = BSON("a" << 4);
+
+ ASSERT(lt.matchesBSON(minKeyObj, NULL));
+ ASSERT(!lt.matchesBSON(maxKeyObj, NULL));
+ ASSERT(lt.matchesBSON(numObj, NULL));
+
+ ASSERT(lt.matchesSingleElement(minKeyObj.firstElement()));
+ ASSERT(!lt.matchesSingleElement(maxKeyObj.firstElement()));
+ ASSERT(lt.matchesSingleElement(numObj.firstElement()));
}
TEST(LtOp, ElemMatchKey) {
@@ -410,17 +442,33 @@ TEST(LteOp, MatchesDotNotationNull) {
TEST(LteOp, MatchesMinKey) {
BSONObj operand = BSON("a" << MinKey);
LTEMatchExpression lte("a", operand["a"]);
- ASSERT(lte.matchesBSON(BSON("a" << MinKey), NULL));
- ASSERT(!lte.matchesBSON(BSON("a" << MaxKey), NULL));
- ASSERT(!lte.matchesBSON(BSON("a" << 4), NULL));
+ BSONObj minKeyObj = BSON("a" << MinKey);
+ BSONObj maxKeyObj = BSON("a" << MaxKey);
+ BSONObj numObj = BSON("a" << 4);
+
+ ASSERT(lte.matchesBSON(minKeyObj, NULL));
+ ASSERT(!lte.matchesBSON(maxKeyObj, NULL));
+ ASSERT(!lte.matchesBSON(numObj, NULL));
+
+ ASSERT(lte.matchesSingleElement(minKeyObj.firstElement()));
+ ASSERT(!lte.matchesSingleElement(maxKeyObj.firstElement()));
+ ASSERT(!lte.matchesSingleElement(numObj.firstElement()));
}
TEST(LteOp, MatchesMaxKey) {
BSONObj operand = BSON("a" << MaxKey);
LTEMatchExpression lte("a", operand["a"]);
- ASSERT(lte.matchesBSON(BSON("a" << MaxKey), NULL));
- ASSERT(lte.matchesBSON(BSON("a" << MinKey), NULL));
- ASSERT(lte.matchesBSON(BSON("a" << 4), NULL));
+ BSONObj minKeyObj = BSON("a" << MinKey);
+ BSONObj maxKeyObj = BSON("a" << MaxKey);
+ BSONObj numObj = BSON("a" << 4);
+
+ ASSERT(lte.matchesBSON(minKeyObj, NULL));
+ ASSERT(lte.matchesBSON(maxKeyObj, NULL));
+ ASSERT(lte.matchesBSON(numObj, NULL));
+
+ ASSERT(lte.matchesSingleElement(minKeyObj.firstElement()));
+ ASSERT(lte.matchesSingleElement(maxKeyObj.firstElement()));
+ ASSERT(lte.matchesSingleElement(numObj.firstElement()));
}
@@ -497,17 +545,33 @@ TEST(GtOp, MatchesDotNotationNull) {
TEST(GtOp, MatchesMinKey) {
BSONObj operand = BSON("a" << MinKey);
GTMatchExpression gt("a", operand["a"]);
- ASSERT(!gt.matchesBSON(BSON("a" << MinKey), NULL));
- ASSERT(gt.matchesBSON(BSON("a" << MaxKey), NULL));
- ASSERT(gt.matchesBSON(BSON("a" << 4), NULL));
+ BSONObj minKeyObj = BSON("a" << MinKey);
+ BSONObj maxKeyObj = BSON("a" << MaxKey);
+ BSONObj numObj = BSON("a" << 4);
+
+ ASSERT(!gt.matchesBSON(minKeyObj, NULL));
+ ASSERT(gt.matchesBSON(maxKeyObj, NULL));
+ ASSERT(gt.matchesBSON(numObj, NULL));
+
+ ASSERT(!gt.matchesSingleElement(minKeyObj.firstElement()));
+ ASSERT(gt.matchesSingleElement(maxKeyObj.firstElement()));
+ ASSERT(gt.matchesSingleElement(numObj.firstElement()));
}
TEST(GtOp, MatchesMaxKey) {
BSONObj operand = BSON("a" << MaxKey);
GTMatchExpression gt("a", operand["a"]);
- ASSERT(!gt.matchesBSON(BSON("a" << MaxKey), NULL));
- ASSERT(!gt.matchesBSON(BSON("a" << MinKey), NULL));
- ASSERT(!gt.matchesBSON(BSON("a" << 4), NULL));
+ BSONObj minKeyObj = BSON("a" << MinKey);
+ BSONObj maxKeyObj = BSON("a" << MaxKey);
+ BSONObj numObj = BSON("a" << 4);
+
+ ASSERT(!gt.matchesBSON(minKeyObj, NULL));
+ ASSERT(!gt.matchesBSON(maxKeyObj, NULL));
+ ASSERT(!gt.matchesBSON(numObj, NULL));
+
+ ASSERT(!gt.matchesSingleElement(minKeyObj.firstElement()));
+ ASSERT(!gt.matchesSingleElement(maxKeyObj.firstElement()));
+ ASSERT(!gt.matchesSingleElement(numObj.firstElement()));
}
TEST(GtOp, ElemMatchKey) {
@@ -596,17 +660,33 @@ TEST(GteOp, MatchesDotNotationNull) {
TEST(GteOp, MatchesMinKey) {
BSONObj operand = BSON("a" << MinKey);
GTEMatchExpression gte("a", operand["a"]);
- ASSERT(gte.matchesBSON(BSON("a" << MinKey), NULL));
- ASSERT(gte.matchesBSON(BSON("a" << MaxKey), NULL));
- ASSERT(gte.matchesBSON(BSON("a" << 4), NULL));
+ BSONObj minKeyObj = BSON("a" << MinKey);
+ BSONObj maxKeyObj = BSON("a" << MaxKey);
+ BSONObj numObj = BSON("a" << 4);
+
+ ASSERT(gte.matchesBSON(minKeyObj, NULL));
+ ASSERT(gte.matchesBSON(maxKeyObj, NULL));
+ ASSERT(gte.matchesBSON(numObj, NULL));
+
+ ASSERT(gte.matchesSingleElement(minKeyObj.firstElement()));
+ ASSERT(gte.matchesSingleElement(maxKeyObj.firstElement()));
+ ASSERT(gte.matchesSingleElement(numObj.firstElement()));
}
TEST(GteOp, MatchesMaxKey) {
BSONObj operand = BSON("a" << MaxKey);
GTEMatchExpression gte("a", operand["a"]);
- ASSERT(gte.matchesBSON(BSON("a" << MaxKey), NULL));
- ASSERT(!gte.matchesBSON(BSON("a" << MinKey), NULL));
- ASSERT(!gte.matchesBSON(BSON("a" << 4), NULL));
+ BSONObj minKeyObj = BSON("a" << MinKey);
+ BSONObj maxKeyObj = BSON("a" << MaxKey);
+ BSONObj numObj = BSON("a" << 4);
+
+ ASSERT(!gte.matchesBSON(minKeyObj, NULL));
+ ASSERT(gte.matchesBSON(maxKeyObj, NULL));
+ ASSERT(!gte.matchesBSON(numObj, NULL));
+
+ ASSERT(!gte.matchesSingleElement(minKeyObj.firstElement()));
+ ASSERT(gte.matchesSingleElement(maxKeyObj.firstElement()));
+ ASSERT(!gte.matchesSingleElement(numObj.firstElement()));
}
TEST(GteOp, ElemMatchKey) {