SERVER-32960 Make $mod truncate float values consistently and use long long for its arguments

4 files changed, 75 insertions, 8 deletions

diff --git a/src/mongo/db/matcher/expression_leaf.cpp b/src/mongo/db/matcher/expression_leaf.cpp
index 9ee9ad27f01..e00eeb4d61c 100644
--- a/src/mongo/db/matcher/expression_leaf.cpp
+++ b/src/mongo/db/matcher/expression_leaf.cpp
@@ -318,7 +318,7 @@ void RegexMatchExpression::shortDebugString(StringBuilder& debug) const {
 
 // ---------
 
-ModMatchExpression::ModMatchExpression(StringData path, int divisor, int remainder)
+ModMatchExpression::ModMatchExpression(StringData path, long long divisor, long long remainder)
     : LeafMatchExpression(MOD, path), _divisor(divisor), _remainder(remainder) {
     uassert(ErrorCodes::BadValue, "divisor cannot be 0", divisor != 0);
 }
@@ -326,7 +326,7 @@ ModMatchExpression::ModMatchExpression(StringData path, int divisor, int remaind
 bool ModMatchExpression::matchesSingleElement(const BSONElement& e, MatchDetails* details) const {
     if (!e.isNumber())
         return false;
-    return mongoSafeMod(e.numberLong(), static_cast<long long>(_divisor)) == _remainder;
+    return mongoSafeMod(truncateToLong(e), _divisor) == _remainder;
 }
 
 void ModMatchExpression::debugString(StringBuilder& debug, int level) const {
diff --git a/src/mongo/db/matcher/expression_leaf.h b/src/mongo/db/matcher/expression_leaf.h
index 1c77d23afb1..0fc78e1c5f3 100644
--- a/src/mongo/db/matcher/expression_leaf.h
+++ b/src/mongo/db/matcher/expression_leaf.h
@@ -333,7 +333,7 @@ private:
 
 class ModMatchExpression : public LeafMatchExpression {
 public:
-    ModMatchExpression(StringData path, int divisor, int remainder);
+    ModMatchExpression(StringData path, long long divisor, long long remainder);
 
     virtual std::unique_ptr<MatchExpression> shallowClone() const {
         std::unique_ptr<ModMatchExpression> m =
@@ -352,20 +352,27 @@ public:
 
     virtual bool equivalent(const MatchExpression* other) const;
 
-    int getDivisor() const {
+    long long getDivisor() const {
         return _divisor;
     }
-    int getRemainder() const {
+    long long getRemainder() const {
         return _remainder;
     }
 
+    static long long truncateToLong(const BSONElement& element) {
+        if (element.type() == BSONType::NumberDecimal) {
+            return element.numberDecimal().toLong(Decimal128::kRoundTowardZero);
+        }
+        return element.numberLong();
+    }
+
 private:
     ExpressionOptimizerFunc getOptimizer() const final {
         return [](std::unique_ptr<MatchExpression> expression) { return expression; };
     }
 
-    int _divisor;
-    int _remainder;
+    long long _divisor;
+    long long _remainder;
 };
 
 class ExistsMatchExpression : public LeafMatchExpression {
diff --git a/src/mongo/db/matcher/expression_parser.cpp b/src/mongo/db/matcher/expression_parser.cpp
index 468e9cc052b..29d1d14393c 100644
--- a/src/mongo/db/matcher/expression_parser.cpp
+++ b/src/mongo/db/matcher/expression_parser.cpp
@@ -530,7 +530,8 @@ StatusWithMatchExpression parseMOD(StringData name, BSONElement e) {
     if (i.more())
         return {Status(ErrorCodes::BadValue, "malformed mod, too many elements")};
 
-    return {stdx::make_unique<ModMatchExpression>(name, d.numberInt(), r.numberInt())};
+    return {std::make_unique<ModMatchExpression>(
+        name, ModMatchExpression::truncateToLong(d), ModMatchExpression::truncateToLong(r))};
 }
 
 StatusWithMatchExpression parseRegexDocument(StringData name, const BSONObj& doc) {
diff --git a/src/mongo/db/matcher/expression_parser_leaf_test.cpp b/src/mongo/db/matcher/expression_parser_leaf_test.cpp
index a1a79f8f450..fd9e81c079f 100644
--- a/src/mongo/db/matcher/expression_parser_leaf_test.cpp
+++ b/src/mongo/db/matcher/expression_parser_leaf_test.cpp
@@ -349,6 +349,65 @@ TEST(MatchExpressionParserLeafTest, SimpleModNotNumber) {
                                                 << "a")));
 }
 
+TEST(MatchExpressionParserLeafTest, ModFloatTruncate) {
+    struct TestCase {
+        BSONObj _query;
+        long long _divider;
+        long long _remainder;
+    };
+
+    const auto positiveLargerThanInt = 3 * static_cast<int64_t>(std::numeric_limits<int>::max());
+    const auto negativeSmallerThanInt = 3 * static_cast<int64_t>(std::numeric_limits<int>::min());
+    std::vector<TestCase> testCases = {
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(3 << 2))), 3, 2},
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(3LL << 2LL))), 3, 2},
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(3.2 << 2.2))), 3, 2},
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(3.7 << 2.7))), 3, 2},
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(Decimal128("3") << Decimal128("2")))), 3, 2},
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(Decimal128("3.2") << Decimal128("2.2")))), 3, 2},
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(Decimal128("3.7") << Decimal128("2.7")))), 3, 2},
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(positiveLargerThanInt << positiveLargerThanInt))),
+         positiveLargerThanInt,
+         positiveLargerThanInt},
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(static_cast<double>(positiveLargerThanInt)
+                                               << static_cast<double>(positiveLargerThanInt)))),
+         positiveLargerThanInt,
+         positiveLargerThanInt},
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(Decimal128(positiveLargerThanInt)
+                                               << Decimal128(positiveLargerThanInt)))),
+         positiveLargerThanInt,
+         positiveLargerThanInt},
+
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(-3 << -2))), -3, -2},
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(-3LL << -2LL))), -3, -2},
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(-3.2 << -2.2))), -3, -2},
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(-3.7 << -2.7))), -3, -2},
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(Decimal128("-3") << Decimal128("-2")))), -3, -2},
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(Decimal128("-3.2") << Decimal128("-2.2")))), -3, -2},
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(Decimal128("-3.7") << Decimal128("-2.7")))), -3, -2},
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(negativeSmallerThanInt << negativeSmallerThanInt))),
+         negativeSmallerThanInt,
+         negativeSmallerThanInt},
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(static_cast<double>(negativeSmallerThanInt)
+                                               << static_cast<double>(negativeSmallerThanInt)))),
+         negativeSmallerThanInt,
+         negativeSmallerThanInt},
+        {BSON("x" << BSON("$mod" << BSON_ARRAY(Decimal128(negativeSmallerThanInt)
+                                               << Decimal128(negativeSmallerThanInt)))),
+         negativeSmallerThanInt,
+         negativeSmallerThanInt},
+    };
+
+    for (const auto& testCase : testCases) {
+        boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
+        StatusWithMatchExpression result = MatchExpressionParser::parse(testCase._query, expCtx);
+        ASSERT_OK(result.getStatus());
+        auto modExpr = checked_cast<ModMatchExpression*>(result.getValue().get());
+        ASSERT_EQ(modExpr->getDivisor(), testCase._divider);
+        ASSERT_EQ(modExpr->getRemainder(), testCase._remainder);
+    }
+}
+
 TEST(MatchExpressionParserLeafTest, IdCollation) {
     BSONObj query = BSON("$id"
                          << "string");