summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/mongo/db/geo/hash.cpp12
-rw-r--r--src/mongo/db/geo/hash_test.cpp38
2 files changed, 40 insertions, 10 deletions
diff --git a/src/mongo/db/geo/hash.cpp b/src/mongo/db/geo/hash.cpp
index 281a8b9544a..9b96a98a957 100644
--- a/src/mongo/db/geo/hash.cpp
+++ b/src/mongo/db/geo/hash.cpp
@@ -471,10 +471,9 @@ GeoHash GeoHash::operator+(const std::string& s) const {
return operator+(s.c_str());
}
-/*
- * Keep the upper _bits*2 bits of _hash, clear the lower bits.
- * Maybe there's junk in there? Not sure why this is done.
- */
+// Keep the most significant _bits*2 bits of _hash, clear the least significant bits. If shorter
+// than 64 bits, the hash occupies the higher order bits, so we ensure that the lower order bits are
+// zeroed.
void GeoHash::clearUnusedBits() {
// Left shift count should be less than 64
if (_bits == 0) {
@@ -482,9 +481,8 @@ void GeoHash::clearUnusedBits() {
return;
}
- static long long FULL = 0xFFFFFFFFFFFFFFFFLL;
- long long mask = FULL << (64 - (_bits * 2));
- _hash &= mask;
+ unsigned long long mask = (1LL << (64U - (_bits * 2U))) - 1LL;
+ _hash &= ~mask;
}
static void appendHashToBuilder(long long hash, BSONObjBuilder* builder, const char* fieldName) {
diff --git a/src/mongo/db/geo/hash_test.cpp b/src/mongo/db/geo/hash_test.cpp
index 93fffb2fbe3..bfe6050fd9d 100644
--- a/src/mongo/db/geo/hash_test.cpp
+++ b/src/mongo/db/geo/hash_test.cpp
@@ -42,15 +42,17 @@
#include "mongo/platform/random.h"
#include "mongo/unittest/unittest.h"
#include "mongo/util/assert_util.h"
+#include "mongo/util/mongoutils/str.h"
+
+namespace {
+
+using namespace mongo;
-using mongo::GeoHash;
-using mongo::GeoHashConverter;
using std::cout;
using std::endl;
using std::string;
using std::stringstream;
-namespace {
TEST(GeoHash, MakeZeroHash) {
unsigned x = 0, y = 0;
GeoHash hash(x, y);
@@ -516,4 +518,34 @@ TEST(GeoHash, NeighborsAtFinestLevel) {
ASSERT_EQUALS(neighbors.size(), (size_t)1);
ASSERT_EQUALS(neighbors[0], GeoHash("00"));
}
+
+TEST(GeoHash, ClearUnusedBitsClearsSomeBits) {
+ GeoHash geoHash("10110010");
+ // 'parent' should have the four higher order bits from the original hash (1011, or the
+ // hexidecimal digit 'b').
+ GeoHash parent = geoHash.parent(2);
+ ASSERT_EQUALS(parent, GeoHash("1011"));
+ const long long expectedHash = 0xb000000000000000LL;
+ ASSERT_EQUALS(expectedHash, parent.getHash());
+}
+
+TEST(GeoHash, ClearUnusedBitsOnLengthZeroHashClearsAllBits) {
+ GeoHash geoHash("11");
+ const long long expectedHash = 0xc000000000000000LL;
+ ASSERT_EQUALS(expectedHash, geoHash.getHash());
+ GeoHash parent = geoHash.parent();
+ ASSERT_EQUALS(GeoHash(), parent);
+ ASSERT_EQUALS(0LL, parent.getHash());
+}
+
+TEST(GeoHash, ClearUnusedBitsIsNoopIfNoBitsAreUnused) {
+ // 64 pairs of "10" repeated.
+ str::stream ss;
+ for (int i = 0; i < 32; ++i) {
+ ss << "10";
+ }
+ GeoHash geoHash(ss);
+ GeoHash other = geoHash.parent(32);
+ ASSERT_EQUALS(geoHash, other);
+}
}