diff options
-rw-r--r-- | src/mongo/db/geo/hash.cpp | 12 | ||||
-rw-r--r-- | src/mongo/db/geo/hash_test.cpp | 38 |
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); +} } |