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

7 files changed, 288 insertions, 37 deletions

diff --git a/jstests/core/mod.js b/jstests/core/mod.js
new file mode 100644
index 00000000000..bf68927f905
--- /dev/null
+++ b/jstests/core/mod.js
@@ -0,0 +1,175 @@
+// Tests the behavior of $mod for match expressions.
+
+(function() {
+    "use strict";
+
+    const coll = db.mod;
+    coll.drop();
+
+    function assertDocumentsFromMod(divider, remainder, expectedDocuments) {
+        const actualDocuments =
+            coll.find({a: {$mod: [divider, remainder]}}).sort({_id: 1}).toArray();
+        assert.eq(expectedDocuments, actualDocuments);
+    }
+
+    // Check mod with divisor and remainder which do not fit into int type.
+    assert.writeOK(coll.insert([
+        {_id: 1, a: 4000000000},
+        {_id: 2, a: 15000000000},
+        {_id: 3, a: -4000000000},
+        {_id: 4, a: -15000000000},
+        {_id: 5, a: 4000000000.12345},
+        {_id: 6, a: 15000000000.12345},
+        {_id: 7, a: -4000000000.12345},
+        {_id: 8, a: -15000000000.12345},
+        {_id: 9, a: NumberDecimal(4000000000.12345)},
+        {_id: 10, a: NumberDecimal(15000000000.12345)},
+        {_id: 11, a: NumberDecimal(-4000000000.12345)},
+        {_id: 12, a: NumberDecimal(-15000000000.12345)},
+    ]));
+
+    assertDocumentsFromMod(4000000000, 0, [
+        {_id: 1, a: 4000000000},
+        {_id: 3, a: -4000000000},
+        {_id: 5, a: 4000000000.12345},
+        {_id: 7, a: -4000000000.12345},
+        {_id: 9, a: NumberDecimal(4000000000.12345)},
+        {_id: 11, a: NumberDecimal(-4000000000.12345)},
+    ]);
+
+    assertDocumentsFromMod(-4000000000, 0, [
+        {_id: 1, a: 4000000000},
+        {_id: 3, a: -4000000000},
+        {_id: 5, a: 4000000000.12345},
+        {_id: 7, a: -4000000000.12345},
+        {_id: 9, a: NumberDecimal(4000000000.12345)},
+        {_id: 11, a: NumberDecimal(-4000000000.12345)},
+    ]);
+
+    assertDocumentsFromMod(10000000000, 5000000000, [
+        {_id: 2, a: 15000000000},
+        {_id: 6, a: 15000000000.12345},
+        {_id: 10, a: NumberDecimal(15000000000.12345)},
+    ]);
+
+    assertDocumentsFromMod(10000000000, -5000000000, [
+        {_id: 4, a: -15000000000},
+        {_id: 8, a: -15000000000.12345},
+        {_id: 12, a: NumberDecimal(-15000000000.12345)},
+    ]);
+
+    assertDocumentsFromMod(-10000000000, 5000000000, [
+        {_id: 2, a: 15000000000},
+        {_id: 6, a: 15000000000.12345},
+        {_id: 10, a: NumberDecimal(15000000000.12345)},
+    ]);
+
+    assertDocumentsFromMod(-10000000000, -5000000000, [
+        {_id: 4, a: -15000000000},
+        {_id: 8, a: -15000000000.12345},
+        {_id: 12, a: NumberDecimal(-15000000000.12345)},
+    ]);
+
+    assert(coll.drop());
+
+    // Check truncation of input argument for mod operator.
+    assert.writeOK(coll.insert([
+        {_id: 1, a: 4.2},
+        {_id: 2, a: 4.5},
+        {_id: 3, a: 4.7},
+        {_id: 4, a: NumberDecimal(4.2)},
+        {_id: 5, a: NumberDecimal(4.5)},
+        {_id: 6, a: NumberDecimal(4.7)},
+    ]));
+
+    assertDocumentsFromMod(4, 0, [
+        {_id: 1, a: 4.2},
+        {_id: 2, a: 4.5},
+        {_id: 3, a: 4.7},
+        {_id: 4, a: NumberDecimal(4.2)},
+        {_id: 5, a: NumberDecimal(4.5)},
+        {_id: 6, a: NumberDecimal(4.7)},
+    ]);
+
+    assert(coll.drop());
+
+    // Check more basic mod usage.
+    assert.writeOK(coll.insert([
+        {_id: 1, str: "abc123", a: 0},
+        {_id: 2, str: "xyz123", a: 5},
+        {_id: 3, str: "ijk123", a: 12},
+        {_id: 4, s: "array", a: [0, 7]},
+        {_id: 5, s: "array", a: [1, 7]},
+        {_id: 6, s: "array", a: [-5]},
+    ]));
+
+    assert.eq(1, coll.find({a: {$mod: [-10, -5]}}).itcount());
+    assert.eq(1, coll.find({a: {$mod: [4, -1]}}).itcount());
+    assert.eq(4, coll.find({a: {$mod: [5, 0]}}).itcount());
+    assert.eq(3, coll.find({a: {$mod: [12, 0]}}).itcount());
+    assert.eq(1, coll.find({a: {$mod: [12, 1]}}).itcount());
+
+    assert.writeOK(coll.insert([
+        {_id: 7, arr: [1, [2, 3], [[4]]]},
+        {_id: 8, arr: [{b: [1, 2, 3]}, {b: [5]}]},
+        {_id: 9, arr: [{b: [-1]}, {b: [5]}]},
+        {_id: 10, arr: "string"},
+    ]));
+
+    // Check nested arrays and dotted paths.
+    assert.eq(1, coll.find({arr: {$mod: [1, 0]}}).itcount());
+    assert.eq(0, coll.find({arr: {$mod: [4, 0]}}).itcount());
+    assert.eq(1, coll.find({"arr.b": {$mod: [3, 0]}}).itcount());
+    assert.eq(2, coll.find({"arr.b": {$mod: [5, 0]}}).itcount());
+    assert.eq(0, coll.find({"arr.b.c": {$mod: [5, 0]}}).itcount());
+
+    // Check with different data types.
+    assert.eq(6, coll.find({a: {$mod: [1, NumberLong(0)]}}).itcount());
+    assert.eq(3, coll.find({a: {$mod: [NumberLong(5), NumberDecimal(2.1)]}}).itcount());
+    assert.eq(3, coll.find({a: {$mod: [NumberDecimal(5.001), NumberDecimal(2.1)]}}).itcount());
+    assert.eq(1, coll.find({a: {$mod: [NumberInt(7), NumberInt(1)]}}).itcount());
+
+    // Check on fields that are not numbers or do not exist.
+    assert.eq(0, coll.find({str: {$mod: [10, 1]}}).itcount());
+    assert.eq(0, coll.find({s: {$mod: [10, 1]}}).itcount());
+    assert.eq(0, coll.find({noField: {$mod: [10, 1]}}).itcount());
+
+    // Check divide by zero.
+    assert.commandFailedWithCode(db.runCommand({find: coll.getName(), filter: {a: {$mod: [0, 1]}}}),
+                                 ErrorCodes.BadValue);
+    assert.commandFailedWithCode(db.runCommand({find: coll.getName(), filter: {a: {$mod: [0, 0]}}}),
+                                 ErrorCodes.BadValue);
+
+    // Check failures with different data types.
+    assert.commandFailedWithCode(db.runCommand({
+        find: coll.getName(),
+        filter: {a: {$mod: [NumberDecimal(0.001), NumberDecimal(1.001)]}}
+    }),
+                                 ErrorCodes.BadValue);
+    assert.commandFailedWithCode(db.runCommand({
+        find: coll.getName(),
+        filter: {a: {$mod: [NumberInt(1), NumberInt(0), NumberInt(0)]}}
+    }),
+                                 ErrorCodes.BadValue);
+    assert.commandFailedWithCode(
+        db.runCommand({find: coll.getName(), filter: {a: {$mod: [NumberDecimal(0.1)]}}}),
+        ErrorCodes.BadValue);
+    assert.commandFailedWithCode(
+        db.runCommand(
+            {find: coll.getName(), filter: {a: {$mod: [NumberDecimal(0), NumberDecimal(0)]}}}),
+        ErrorCodes.BadValue);
+
+    // Check incorrect arity.
+    assert.commandFailedWithCode(
+        db.runCommand({find: coll.getName(), filter: {a: {$mod: [10, 1, 1]}}}),
+        ErrorCodes.BadValue);
+    assert.commandFailedWithCode(db.runCommand({find: coll.getName(), filter: {a: {$mod: [10]}}}),
+                                 ErrorCodes.BadValue);
+
+    // Check remainder, divisor not a number.
+    assert.commandFailedWithCode(
+        db.runCommand({find: coll.getName(), filter: {a: {$mod: ["a", 0]}}}), ErrorCodes.BadValue);
+    assert.commandFailedWithCode(
+        db.runCommand({find: coll.getName(), filter: {a: {$mod: ["a", "b"]}}}),
+        ErrorCodes.BadValue);
+}());
diff --git a/jstests/core/mod1.js b/jstests/core/mod1.js
deleted file mode 100644
index 11be6b1b293..00000000000
--- a/jstests/core/mod1.js
+++ /dev/null
@@ -1,29 +0,0 @@
-
-t = db.mod1;
-t.drop();
-
-t.save({a: 1});
-t.save({a: 2});
-t.save({a: 11});
-t.save({a: 20});
-t.save({a: "asd"});
-t.save({a: "adasdas"});
-
-assert.eq(2, t.find("this.a % 10 == 1").itcount(), "A1");
-assert.eq(2, t.find({a: {$mod: [10, 1]}}).itcount(), "A2");
-assert.eq(0,
-          t.find({a: {$mod: [10, 1]}}).explain("executionStats").executionStats.totalKeysExamined,
-          "A3");
-
-t.ensureIndex({a: 1});
-
-assert.eq(2, t.find("this.a % 10 == 1").itcount(), "B1");
-assert.eq(2, t.find({a: {$mod: [10, 1]}}).itcount(), "B2");
-
-assert.eq(1, t.find("this.a % 10 == 0").itcount(), "B3");
-assert.eq(1, t.find({a: {$mod: [10, 0]}}).itcount(), "B4");
-assert.eq(4,
-          t.find({a: {$mod: [10, 1]}}).explain("executionStats").executionStats.totalKeysExamined,
-          "B5");
-
-assert.eq(1, t.find({a: {$gt: 5, $mod: [10, 1]}}).itcount());
diff --git a/jstests/core/mod_with_where.js b/jstests/core/mod_with_where.js
new file mode 100644
index 00000000000..5f60ec8d0e7
--- /dev/null
+++ b/jstests/core/mod_with_where.js
@@ -0,0 +1,38 @@
+// Tests the behavior of $mod for match expressions.
+// @tags: [
+//   assumes_balancer_off,
+//   # Uses $where operator
+//   requires_scripting,
+// ]
+
+(function() {
+    "use strict";
+
+    const coll = db.mod_with_where;
+    coll.drop();
+
+    assert.writeOK(coll.insert([{a: 1}, {a: 2}, {a: 11}, {a: 20}, {a: "asd"}, {a: "adasdas"}]));
+
+    // Check basic mod usage.
+    assert.eq(2, coll.find("this.a % 10 == 1").itcount(), "A1");
+    assert.eq(2, coll.find({a: {$mod: [10, 1]}}).itcount(), "A2");
+    assert.eq(
+        0,
+        coll.find({a: {$mod: [10, 1]}}).explain("executionStats").executionStats.totalKeysExamined,
+        "A3");
+
+    assert.commandWorked(coll.createIndex({a: 1}));
+
+    // Check mod with an index.
+    assert.eq(2, coll.find("this.a % 10 == 1").itcount(), "B1");
+    assert.eq(2, coll.find({a: {$mod: [10, 1]}}).itcount(), "B2");
+    assert.eq(1, coll.find("this.a % 10 == 0").itcount(), "B3");
+    assert.eq(1, coll.find({a: {$mod: [10, 0]}}).itcount(), "B4");
+    assert.eq(
+        4,
+        coll.find({a: {$mod: [10, 1]}}).explain("executionStats").executionStats.totalKeysExamined,
+        "B5");
+    assert.eq(1, coll.find({a: {$gt: 5, $mod: [10, 1]}}).itcount());
+
+    assert(coll.drop());
+}());
diff --git a/src/mongo/db/matcher/expression_leaf.cpp b/src/mongo/db/matcher/expression_leaf.cpp
index fd9d082c518..eb1e77d88d9 100644
--- a/src/mongo/db/matcher/expression_leaf.cpp
+++ b/src/mongo/db/matcher/expression_leaf.cpp
@@ -289,7 +289,7 @@ void RegexMatchExpression::shortDebugString(StringBuilder& debug) const {
 
 // ---------
 
-Status ModMatchExpression::init(StringData path, int divisor, int remainder) {
+Status ModMatchExpression::init(StringData path, long long divisor, long long remainder) {
     if (divisor == 0)
         return Status(ErrorCodes::BadValue, "divisor cannot be 0");
     _divisor = divisor;
@@ -300,7 +300,7 @@ Status ModMatchExpression::init(StringData path, int divisor, int remainder) {
 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 f7e1ea078dc..90e8463d196 100644
--- a/src/mongo/db/matcher/expression_leaf.h
+++ b/src/mongo/db/matcher/expression_leaf.h
@@ -335,7 +335,7 @@ class ModMatchExpression : public LeafMatchExpression {
 public:
     ModMatchExpression() : LeafMatchExpression(MOD) {}
 
-    Status init(StringData path, int divisor, int remainder);
+    Status init(StringData path, long long divisor, long long remainder);
 
     virtual std::unique_ptr<MatchExpression> shallowClone() const {
         std::unique_ptr<ModMatchExpression> m = stdx::make_unique<ModMatchExpression>();
@@ -354,20 +354,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 e1068520e2e..68b1702c7c5 100644
--- a/src/mongo/db/matcher/expression_parser.cpp
+++ b/src/mongo/db/matcher/expression_parser.cpp
@@ -565,7 +565,8 @@ StatusWithMatchExpression parseMOD(StringData name, BSONElement e) {
         return {Status(ErrorCodes::BadValue, "malformed mod, too many elements")};
 
     auto temp = stdx::make_unique<ModMatchExpression>();
-    auto s = temp->init(name, d.numberInt(), r.numberInt());
+    auto s = temp->init(
+        name, ModMatchExpression::truncateToLong(d), ModMatchExpression::truncateToLong(r));
     if (!s.isOK())
         return s;
     return {std::move(temp)};
diff --git a/src/mongo/db/matcher/expression_parser_leaf_test.cpp b/src/mongo/db/matcher/expression_parser_leaf_test.cpp
index a1a79f8f450..18a25a410ae 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 = static_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");