SERVER-14761 only allow NumberLong as split key in hashed shard patterns

When a shard uses a hashed key pattern, only NumberLong types may be used
as split keys.

A small bug in hash_basic.js regarding 'middle' syntax was also fixed.
Instead of 10000 to the document as a whole, 10000 is now added to the
actual number in the document.

A small bug in read_only_test.js regarding the shardColl(...) function
was also found. It was fixed to now do what I believe the author originally
intended it to do.

5 files changed, 98 insertions, 30 deletions

diff --git a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml
index 44f46244ea7..d9cca2eaeee 100644
--- a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml
+++ b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml
@@ -44,6 +44,8 @@ selector:
   # SERVER-19318: added new $currentOp agg stage in 3.6
   - jstests/sharding/aggregation_currentop.js
   - jstests/sharding/read_pref_cmd.js
+  # SERVER-14761 only allow NumberLong as split key in hashed shard patterns
+  - jstests/sharding/hash_number_long_split.js
 
 executor:
   config:
diff --git a/jstests/readonly/lib/read_only_test.js b/jstests/readonly/lib/read_only_test.js
index 5b2775e697a..b34d7470ad5 100644
--- a/jstests/readonly/lib/read_only_test.js
+++ b/jstests/readonly/lib/read_only_test.js
@@ -58,7 +58,7 @@ var StandaloneFixture, ShardedFixture, runReadOnlyTest, zip2, cycleN;
         jsTest.log("sharding test collection...");
 
         // Use a hashed shard key so we actually hit multiple shards.
-        this.shardingTest.shardColl(test.name, {_id: "hashed"});
+        this.shardingTest.shardColl(test.name, {_id: "hashed"}, false);
 
         test.load(this.shardingTest.getDB("test")[test.name]);
     };
diff --git a/jstests/sharding/hash_basic.js b/jstests/sharding/hash_basic.js
index 4e3d6806a30..1bd3376eecb 100644
--- a/jstests/sharding/hash_basic.js
+++ b/jstests/sharding/hash_basic.js
@@ -27,7 +27,7 @@
            'split on bounds failed on chunk[' + tojson(chunkDoc) + ']: ' + tojson(cmdRes));
 
     chunkDoc = configDB.chunks.find().sort({min: 1}).skip(1).next();
-    var middle = chunkDoc.min + 1000000;
+    var middle = NumberLong(chunkDoc.min.x + 1000000);
 
     cmdRes = testDB.adminCommand({split: 'test.user', middle: {x: middle}});
     assert(cmdRes.ok, 'split failed with middle [' + middle + ']: ' + tojson(cmdRes));
diff --git a/jstests/sharding/hash_number_long_split.js b/jstests/sharding/hash_number_long_split.js
new file mode 100644
index 00000000000..a67c5badf74
--- /dev/null
+++ b/jstests/sharding/hash_number_long_split.js
@@ -0,0 +1,46 @@
+// Hash sharding on a non empty collection should should not allow non-long type split keys.
+
+(function() {
+    'use strict';
+
+    var st = new ShardingTest({shards: 2, chunkSize: 1});
+
+    var testDB = st.s.getDB('test');
+    assert.commandWorked(testDB.adminCommand({enableSharding: 'test'}));
+    st.ensurePrimaryShard('test', 'shard0001');
+    assert.commandWorked(testDB.adminCommand({shardCollection: 'test.user', key: {x: 'hashed'}}));
+
+    var configDB = st.s.getDB('config');
+    var chunkCountBefore = configDB.chunks.count();
+    assert.gt(chunkCountBefore, 1);
+
+    for (var x = 0; x < 100; x++) {
+        testDB.user.insert({x: x});
+    }
+
+    // double-precision floating-point values
+    var doubles = [47.21230129, 1.0, 0.0, -0.001];
+
+    // integer values
+    var ints = [NumberInt(-1), NumberInt(0), NumberInt(1), NumberInt(42)];
+
+    // long values
+    var longs =
+        [NumberLong(5), NumberLong(-2147483647), NumberLong(2147483647), NumberLong(2147483648)];
+
+    for (var i = 0; i < 4; i++) {
+        // Split on 'middle' should fail on double value keys.
+        var cmdRes = testDB.adminCommand({split: 'test.user', middle: {x: doubles[i]}});
+        assert(!cmdRes.ok, 'split on middle double succeeded on {x: ' + doubles[i] + '}');
+
+        // Split on 'middle' should fail on int value keys.
+        var cmdRes = testDB.adminCommand({split: 'test.user', middle: {x: ints[i]}});
+        assert(!cmdRes.ok, 'split on middle int succeeded on {x: ' + ints[i] + '}');
+
+        // Split on 'middle' should succeed on long value keys.
+        var cmdRes = testDB.adminCommand({split: 'test.user', middle: {x: longs[i]}});
+        assert(cmdRes.ok, 'split on middle long failed on {x: ' + longs[i] + '}');
+    }
+
+    st.stop();
+})();
diff --git a/src/mongo/db/s/split_chunk_command.cpp b/src/mongo/db/s/split_chunk_command.cpp
index cfc2c04ad55..cc43284a7ef 100644
--- a/src/mongo/db/s/split_chunk_command.cpp
+++ b/src/mongo/db/s/split_chunk_command.cpp
@@ -202,22 +202,6 @@ public:
         const BSONObj min = chunkRange.getMin();
         const BSONObj max = chunkRange.getMax();
 
-        vector<BSONObj> splitKeys;
-        {
-            BSONElement splitKeysElem;
-            auto splitKeysElemStatus =
-                bsonExtractTypedField(cmdObj, "splitKeys", mongo::Array, &splitKeysElem);
-
-            if (!splitKeysElemStatus.isOK()) {
-                errmsg = "need to provide the split points to chunk over";
-                return false;
-            }
-            BSONObjIterator it(splitKeysElem.Obj());
-            while (it.more()) {
-                splitKeys.push_back(it.next().Obj().getOwned());
-            }
-        }
-
         string shardName;
         auto parseShardNameStatus = bsonExtractStringField(cmdObj, "from", &shardName);
         if (!parseShardNameStatus.isOK())
@@ -265,6 +249,54 @@ public:
             return false;
         }
 
+        ScopedCollectionMetadata collMetadata;
+        {
+            AutoGetCollection autoColl(opCtx, nss, MODE_IS);
+
+            // Get collection metadata
+            collMetadata = CollectionShardingState::get(opCtx, nss.ns())->getMetadata();
+        }
+
+        // With nonzero shard version, we must have metadata
+        invariant(collMetadata);
+
+        KeyPattern shardKeyPattern(collMetadata->getKeyPattern());
+
+        vector<BSONObj> splitKeys;
+        {
+            BSONElement splitKeysElem;
+            auto splitKeysElemStatus =
+                bsonExtractTypedField(cmdObj, "splitKeys", mongo::Array, &splitKeysElem);
+
+            if (!splitKeysElemStatus.isOK()) {
+                errmsg = "need to provide the split points to chunk over";
+                return false;
+            }
+
+            BSONObjIterator it(splitKeysElem.Obj());
+
+            // If the shard uses a hashed key, then we must make sure that the split point is of
+            // type NumberLong.
+            if (KeyPattern::isHashedKeyPattern(shardKeyPattern.toBSON())) {
+                while (it.more()) {
+                    BSONObj splitKeyObj = it.next().Obj();
+                    BSONObjIterator keyIt(splitKeyObj);
+                    while (keyIt.more()) {
+                        BSONElement splitKey = keyIt.next();
+                        if (splitKey.type() != NumberLong) {
+                            errmsg = "split point must be of type NumberLong";
+                            return false;
+                        }
+                    }
+                    splitKeys.push_back(splitKeyObj.getOwned());
+                }
+            } else {
+                while (it.more()) {
+                    splitKeys.push_back(it.next().Obj().getOwned());
+                }
+            }
+        }
+
         OID expectedCollectionEpoch;
         if (cmdObj.hasField("epoch")) {
             auto epochStatus = bsonExtractOIDField(cmdObj, "epoch", &expectedCollectionEpoch);
@@ -293,17 +325,6 @@ public:
             return appendCommandStatus(result, {ErrorCodes::StaleEpoch, msg});
         }
 
-        ScopedCollectionMetadata collMetadata;
-        {
-            AutoGetCollection autoColl(opCtx, nss, MODE_IS);
-
-            // Get collection metadata
-            collMetadata = CollectionShardingState::get(opCtx, nss.ns())->getMetadata();
-        }
-
-        // With nonzero shard version, we must have metadata
-        invariant(collMetadata);
-
         ChunkVersion collVersion = collMetadata->getCollVersion();
         // With nonzero shard version, we must have a coll version >= our shard version
         invariant(collVersion >= shardVersion);
@@ -389,7 +410,6 @@ public:
         }
 
         // Select chunk to move out for "top chunk optimization".
-        KeyPattern shardKeyPattern(collMetadata->getKeyPattern());
 
         AutoGetCollection autoColl(opCtx, nss, MODE_IS);