diff options
-rw-r--r-- | src/mongo/db/exec/geo_near.cpp | 28 | ||||
-rw-r--r-- | src/mongo/db/geo/hash.cpp | 45 | ||||
-rw-r--r-- | src/mongo/db/geo/hash.h | 14 | ||||
-rw-r--r-- | src/mongo/db/geo/hash_test.cpp | 34 | ||||
-rw-r--r-- | src/mongo/db/geo/r2_region_coverer.cpp | 10 | ||||
-rw-r--r-- | src/mongo/db/geo/r2_region_coverer.h | 6 | ||||
-rw-r--r-- | src/mongo/db/geo/r2_region_coverer_test.cpp | 24 | ||||
-rw-r--r-- | src/mongo/db/index/expression_params.cpp | 14 | ||||
-rw-r--r-- | src/mongo/db/query/expression_index.cpp | 11 | ||||
-rw-r--r-- | src/mongo/db/query/planner_ixselect.cpp | 9 |
10 files changed, 109 insertions, 86 deletions
diff --git a/src/mongo/db/exec/geo_near.cpp b/src/mongo/db/exec/geo_near.cpp index c684b0dfe46..c12bd3e11ed 100644 --- a/src/mongo/db/exec/geo_near.cpp +++ b/src/mongo/db/exec/geo_near.cpp @@ -245,13 +245,13 @@ static R2Annulus twoDDistanceBounds(const GeoNearParams& nearParams, const CRS queryCRS = nearParams.nearQuery->centroid->crs; if (FLAT == queryCRS) { - // Reset the full bounds based on our index bounds - GeoHashConverter::Parameters hashParams; - Status status = GeoHashConverter::parseParameters(twoDIndex->infoObj(), &hashParams); - invariantStatusOK(status); // The index status should always be valid + // Reset the full bounds based on our index bounds. + // The index status should always be valid. + auto result = invariantStatusOK(GeoHashConverter::createFromDoc(twoDIndex->infoObj())); // The biggest distance possible in this indexed collection is the diagonal of the // square indexed region. + const GeoHashConverter::Parameters& hashParams = result->getParams(); const double sqrt2Approx = 1.5; const double diagonalDist = sqrt2Approx * (hashParams.max - hashParams.min); @@ -272,18 +272,16 @@ GeoNear2DStage::DensityEstimator::DensityEstimator(PlanStage::Children* children const GeoNearParams* nearParams, const R2Annulus& fullBounds) : _children(children), _nearParams(nearParams), _fullBounds(fullBounds), _currentLevel(0) { - GeoHashConverter::Parameters hashParams; - Status status = GeoHashConverter::parseParameters(std::move(infoObj), &hashParams); // The index status should always be valid. - invariantStatusOK(status); + auto result = invariantStatusOK(GeoHashConverter::createFromDoc(std::move(infoObj))); - _converter.reset(new GeoHashConverter(hashParams)); + _converter = std::move(result); _centroidCell = _converter->hash(_nearParams->nearQuery->centroid->oldPoint); // Since appendVertexNeighbors(level, output) requires level < hash.getBits(), // we have to start to find documents at most GeoHash::kMaxBits - 1. Thus the finest // search area is 16 * finest cell area at GeoHash::kMaxBits. - _currentLevel = std::max(0, hashParams.bits - 1); + _currentLevel = std::max(0, _converter->getParams().bits - 1); } // Initialize the internal states @@ -539,15 +537,13 @@ private: static double min2DBoundsIncrement(const GeoNearExpression& query, const IndexDescriptor* twoDIndex) { - GeoHashConverter::Parameters hashParams; - Status status = GeoHashConverter::parseParameters(twoDIndex->infoObj(), &hashParams); - invariantStatusOK(status); // The index status should always be valid - GeoHashConverter hasher(hashParams); + // The index status should always be valid. + auto result = invariantStatusOK(GeoHashConverter::createFromDoc(twoDIndex->infoObj())); // The hasher error is the diagonal of a 2D hash region - it's generally not helpful // to change region size such that a search radius is smaller than the 2D hash region // max radius. This is slightly conservative for now (box diagonal vs circle radius). - double minBoundsIncrement = hasher.getError() / 2; + const double minBoundsIncrement = result->getError() / 2; const CRS queryCRS = query.centroid->crs; if (FLAT == queryCRS) @@ -697,9 +693,7 @@ GeoNear2DStage::nextInterval(OperationContext* opCtx, &scanParams.bounds.fields[twoDFieldPosition]); // These parameters are stored by the index, and so must be ok - GeoHashConverter::Parameters hashParams; - GeoHashConverter::parseParameters(indexDescriptor()->infoObj(), &hashParams) - .transitional_ignore(); + invariantStatusOK(GeoHashConverter::createFromDoc(indexDescriptor()->infoObj())); // 2D indexes support covered search over additional fields they contain auto scan = std::make_unique<IndexScan>(expCtx(), scanParams, workingSet, _nearParams.filter); diff --git a/src/mongo/db/geo/hash.cpp b/src/mongo/db/geo/hash.cpp index c1170efd57e..25437d51769 100644 --- a/src/mongo/db/geo/hash.cpp +++ b/src/mongo/db/geo/hash.cpp @@ -662,49 +662,64 @@ static BSONField<double> minField("min", -180.0); // (about 1.11e-16) times the magnitude of the result. double const GeoHashConverter::kMachinePrecision = 0.5 * std::numeric_limits<double>::epsilon(); -Status GeoHashConverter::parseParameters(const BSONObj& paramDoc, - GeoHashConverter::Parameters* params) { +StatusWith<std::unique_ptr<GeoHashConverter>> GeoHashConverter::createFromDoc( + const BSONObj& paramDoc) { string errMsg; + Parameters params{}; if (FieldParser::FIELD_INVALID == - FieldParser::extractNumber(paramDoc, bitsField, ¶ms->bits, &errMsg)) { + FieldParser::extractNumber(paramDoc, bitsField, ¶ms.bits, &errMsg)) { return Status(ErrorCodes::InvalidOptions, errMsg); } if (FieldParser::FIELD_INVALID == - FieldParser::extractNumber(paramDoc, maxField, ¶ms->max, &errMsg)) { + FieldParser::extractNumber(paramDoc, maxField, ¶ms.max, &errMsg)) { return Status(ErrorCodes::InvalidOptions, errMsg); } if (FieldParser::FIELD_INVALID == - FieldParser::extractNumber(paramDoc, minField, ¶ms->min, &errMsg)) { + FieldParser::extractNumber(paramDoc, minField, ¶ms.min, &errMsg)) { return Status(ErrorCodes::InvalidOptions, errMsg); } - if (params->bits < 1 || params->bits > 32) { + if (params.bits < 1 || params.bits > 32) { return Status(ErrorCodes::InvalidOptions, str::stream() << "bits for hash must be > 0 and <= 32, " - << "but " << params->bits << " bits were specified"); + << "but " << params.bits << " bits were specified"); } - const bool rangeValid = params->min < params->max; - if (!rangeValid || std::isinf(params->min) || std::isinf(params->max)) { + const bool rangeValid = params.min < params.max; + if (!rangeValid || std::isinf(params.min) || std::isinf(params.max)) { return Status(ErrorCodes::InvalidOptions, str::stream() << "region for hash must be valid and have positive area, " - << "but [" << params->min << ", " << params->max << "] " + << "but [" << params.min << ", " << params.max << "] " << "was specified"); } constexpr double numBuckets = 4.0 * 1024 * 1024 * 1024; - params->scaling = numBuckets / (params->max - params->min); - const bool scalingValid = params->scaling > 0; - if (!scalingValid || std::isinf(params->scaling)) { + params.scaling = numBuckets / (params.max - params.min); + const bool scalingValid = params.scaling > 0; + if (!scalingValid || std::isinf(params.scaling)) { return Status(ErrorCodes::InvalidOptions, str::stream() - << "range [" << params->min << ", " << params->max << "] is too small."); + << "range [" << params.min << ", " << params.max << "] is too small."); } - return Status::OK(); + return createFromParams(params); +} + +StatusWith<std::unique_ptr<GeoHashConverter>> GeoHashConverter::createFromParams( + const Parameters& params) { + std::unique_ptr<GeoHashConverter> converter(new GeoHashConverter(params)); + + const bool errorValid = params.max - params.min >= converter->_error / 2; + if (!errorValid) { + return Status(ErrorCodes::InvalidOptions, + str::stream() << "invalid computed error: " << converter->_error + << " on range [" << params.min << ", " << params.max << "]."); + } + + return {std::move(converter)}; } GeoHashConverter::GeoHashConverter(const Parameters& params) : _params(params) { diff --git a/src/mongo/db/geo/hash.h b/src/mongo/db/geo/hash.h index bfdca2cdffd..6b59034accc 100644 --- a/src/mongo/db/geo/hash.h +++ b/src/mongo/db/geo/hash.h @@ -179,14 +179,18 @@ public: double scaling; }; - GeoHashConverter(const Parameters& params); + /** + * Factory method to return a new instance with status. Uses hashing parameters parsed from a + * BSONObj. + */ + static StatusWith<std::unique_ptr<GeoHashConverter>> createFromDoc(const BSONObj& paramDoc); /** - * Returns hashing parameters parsed from a BSONObj + * Factory method to return a new instance with status. */ - static Status parseParameters(const BSONObj& paramDoc, Parameters* params); + static StatusWith<std::unique_ptr<GeoHashConverter>> createFromParams(const Parameters& params); - static double calcUnhashToBoxError(const GeoHashConverter::Parameters& params); + static double calcUnhashToBoxError(const Parameters& params); /** * Return converter parameterss which can be used to @@ -257,6 +261,8 @@ public: double convertToDoubleHashScale(double in) const; private: + GeoHashConverter(const Parameters& params); + void init(); // Convert from an unsigned in [0, (max-min)*scaling] to [min, max] diff --git a/src/mongo/db/geo/hash_test.cpp b/src/mongo/db/geo/hash_test.cpp index 288a0895d02..d73c983bc06 100644 --- a/src/mongo/db/geo/hash_test.cpp +++ b/src/mongo/db/geo/hash_test.cpp @@ -150,18 +150,19 @@ TEST(GeoHash, UnhashFastMatchesUnhashSlow) { TEST(GeoHashConvertor, EdgeLength) { const double kError = 10E-15; - GeoHashConverter::Parameters params; + GeoHashConverter::Parameters params{}; params.max = 200.0; params.min = 100.0; params.bits = 32; double numBuckets = (1024 * 1024 * 1024 * 4.0); params.scaling = numBuckets / (params.max - params.min); - GeoHashConverter converter(params); + auto converter = GeoHashConverter::createFromParams(params); + ASSERT_OK(converter.getStatus()); - ASSERT_APPROX_EQUAL(100.0, converter.sizeEdge(0), kError); - ASSERT_APPROX_EQUAL(50.0, converter.sizeEdge(1), kError); - ASSERT_APPROX_EQUAL(25.0, converter.sizeEdge(2), kError); + ASSERT_APPROX_EQUAL(100.0, converter.getValue()->sizeEdge(0), kError); + ASSERT_APPROX_EQUAL(50.0, converter.getValue()->sizeEdge(1), kError); + ASSERT_APPROX_EQUAL(25.0, converter.getValue()->sizeEdge(2), kError); } /** @@ -377,7 +378,7 @@ TEST(GeoHashConvertor, EdgeLength) { * We can get the maximum of the error by making max very large and min = -min, x -> max */ TEST(GeoHashConverter, UnhashToBoxError) { - GeoHashConverter::Parameters params; + GeoHashConverter::Parameters params{}; // Test max from 2^-20 to 2^20 for (int times = -20; times <= 20; times += 2) { // Construct parameters @@ -387,7 +388,9 @@ TEST(GeoHashConverter, UnhashToBoxError) { double numBuckets = (1024 * 1024 * 1024 * 4.0); params.scaling = numBuckets / (params.max - params.min); - GeoHashConverter converter(params); + auto converter = GeoHashConverter::createFromParams(params); + ASSERT_OK(converter.getStatus()); + // Assume level == 32, so we ignore the error of edge length here. double delta_box = 7.0 / 8.0 * GeoHashConverter::calcUnhashToBoxError(params); double cellEdge = 1 / params.scaling; @@ -400,8 +403,8 @@ TEST(GeoHashConverter, UnhashToBoxError) { x = params.max; while (x > params.max - cellEdge) { x = nextafter(x, params.min); - double x_prime = - converter.convertDoubleFromHashScale(converter.convertToDoubleHashScale(x)); + double x_prime = converter.getValue()->convertDoubleFromHashScale( + converter.getValue()->convertToDoubleHashScale(x)); double delta = fabs(x - x_prime); ASSERT_LESS_THAN(delta, delta_box); } @@ -410,8 +413,8 @@ TEST(GeoHashConverter, UnhashToBoxError) { x = params.min + cellEdge; while (x > params.min) { x = nextafter(x, params.min); - double x_prime = - converter.convertDoubleFromHashScale(converter.convertToDoubleHashScale(x)); + double x_prime = converter.getValue()->convertDoubleFromHashScale( + converter.getValue()->convertToDoubleHashScale(x)); double delta = fabs(x - x_prime); ASSERT_LESS_THAN(delta, delta_box); } @@ -420,19 +423,20 @@ TEST(GeoHashConverter, UnhashToBoxError) { // SERVER-15576 Verify a point is contained by its GeoHash box. TEST(GeoHashConverter, GeoHashBox) { - GeoHashConverter::Parameters params; + GeoHashConverter::Parameters params{}; params.max = 100000000.3; params.min = -params.max; params.bits = 32; double numBuckets = (1024 * 1024 * 1024 * 4.0); params.scaling = numBuckets / (params.max - params.min); - GeoHashConverter converter(params); + auto converter = GeoHashConverter::createFromParams(params); + ASSERT_OK(converter.getStatus()); // Without expanding the box, the following point is not contained by its GeoHash box. mongo::Point p(-7201198.6497758823, -0.1); - mongo::GeoHash hash = converter.hash(p); - mongo::Box box = converter.unhashToBoxCovering(hash); + mongo::GeoHash hash = converter.getValue()->hash(p); + mongo::Box box = converter.getValue()->unhashToBoxCovering(hash); ASSERT(box.inside(p)); } diff --git a/src/mongo/db/geo/r2_region_coverer.cpp b/src/mongo/db/geo/r2_region_coverer.cpp index 43c0094e04b..155546266b2 100644 --- a/src/mongo/db/geo/r2_region_coverer.cpp +++ b/src/mongo/db/geo/r2_region_coverer.cpp @@ -54,10 +54,8 @@ struct R2RegionCoverer::CompareQueueEntries : public less<QueueEntry> { } }; -// Doesn't take ownership of "hashConverter". The caller should guarantee its life cycle -// is longer than this coverer. -R2RegionCoverer::R2RegionCoverer(GeoHashConverter* hashConverter) - : _hashConverter(hashConverter), +R2RegionCoverer::R2RegionCoverer(std::unique_ptr<GeoHashConverter> hashConverter) + : _hashConverter(std::move(hashConverter)), _minLevel(0u), _maxLevel(GeoHash::kMaxBits), _maxCells(kDefaultMaxCells), @@ -136,6 +134,10 @@ void R2RegionCoverer::getCovering(const R2Region& region, vector<GeoHash>* cover cover->swap(*_results); } +const GeoHashConverter& R2RegionCoverer::getHashConverter() const { + return *_hashConverter; +} + // Caller owns the returned pointer R2RegionCoverer::Candidate* R2RegionCoverer::newCandidate(const GeoHash& cell) { // Exclude the cell that doesn't intersect with the geometry. diff --git a/src/mongo/db/geo/r2_region_coverer.h b/src/mongo/db/geo/r2_region_coverer.h index 86ea19a2177..e1f91624fa7 100644 --- a/src/mongo/db/geo/r2_region_coverer.h +++ b/src/mongo/db/geo/r2_region_coverer.h @@ -46,7 +46,7 @@ class R2RegionCoverer : boost::noncopyable { static const int kDefaultMaxCells; // = 8; public: - R2RegionCoverer(GeoHashConverter* hashConverter); + R2RegionCoverer(std::unique_ptr<GeoHashConverter> hashConverter); ~R2RegionCoverer(); // Set the minimum and maximum cell level to be used. The default is to use @@ -74,6 +74,8 @@ public: void getCovering(const R2Region& region, std::vector<GeoHash>* cover); + const GeoHashConverter& getHashConverter() const; + private: struct Candidate { GeoHash cell; @@ -102,7 +104,7 @@ private: // Computes a set of initial candidates that cover the given region. void getInitialCandidates(); - GeoHashConverter* _hashConverter; // Not owned. + std::unique_ptr<GeoHashConverter> _hashConverter; // min / max level as unsigned so as to be consistent with GeoHash unsigned int _minLevel; unsigned int _maxLevel; diff --git a/src/mongo/db/geo/r2_region_coverer_test.cpp b/src/mongo/db/geo/r2_region_coverer_test.cpp index 0716496a43c..806fc9d4e37 100644 --- a/src/mongo/db/geo/r2_region_coverer_test.cpp +++ b/src/mongo/db/geo/r2_region_coverer_test.cpp @@ -179,8 +179,10 @@ private: }; TEST(R2RegionCoverer, RandomCells) { - GeoHashConverter converter(getConverterParams()); - R2RegionCoverer coverer(&converter); + auto result = GeoHashConverter::createFromParams(getConverterParams()); + ASSERT_OK(result.getStatus()); + + R2RegionCoverer coverer(std::move(result.getValue())); coverer.setMaxCells(1); // Test random cell ids at all levels. for (int i = 0; i < 10000; ++i) { @@ -188,7 +190,7 @@ TEST(R2RegionCoverer, RandomCells) { random(std::numeric_limits<long long>::lowest(), std::numeric_limits<long long>::max()), random(0U, GeoHash::kMaxBits)); vector<GeoHash> covering; - Box box = converter.unhashToBoxCovering(id); + Box box = coverer.getHashConverter().unhashToBoxCovering(id); // Since the unhashed box is expanded by the error 8Mu, we need to shrink it. box.fudge(-GeoHashConverter::kMachinePrecision * MAXBOUND * 20); HashBoxRegion region(box); @@ -288,8 +290,10 @@ GeometryContainer* getRandomCircle(double radius) { // Test the covering for arbitrary random circle. TEST(R2RegionCoverer, RandomCircles) { - GeoHashConverter converter(getConverterParams()); - R2RegionCoverer coverer(&converter); + auto result = GeoHashConverter::createFromParams(getConverterParams()); + ASSERT_OK(result.getStatus()); + + R2RegionCoverer coverer(std::move(result.getValue())); coverer.setMaxCells(8); for (int i = 0; i < 1000; i++) { @@ -305,14 +309,16 @@ TEST(R2RegionCoverer, RandomCircles) { vector<GeoHash> covering; coverer.getCovering(region, &covering); - checkCovering(converter, region, coverer, covering); + checkCovering(coverer.getHashConverter(), region, coverer, covering); } } // Test the covering for very small circles, since the above test doesn't cover finest cells. TEST(R2RegionCoverer, RandomTinyCircles) { - GeoHashConverter converter(getConverterParams()); - R2RegionCoverer coverer(&converter); + auto result = GeoHashConverter::createFromParams(getConverterParams()); + ASSERT_OK(result.getStatus()); + + R2RegionCoverer coverer(std::move(result.getValue())); coverer.setMaxCells(random(1, 20)); // [1, 20] for (int i = 0; i < 10000; i++) { @@ -328,7 +334,7 @@ TEST(R2RegionCoverer, RandomTinyCircles) { vector<GeoHash> covering; coverer.getCovering(region, &covering); - checkCovering(converter, region, coverer, covering); + checkCovering(coverer.getHashConverter(), region, coverer, covering); } } diff --git a/src/mongo/db/index/expression_params.cpp b/src/mongo/db/index/expression_params.cpp index 29977a5baab..519b5f649e9 100644 --- a/src/mongo/db/index/expression_params.cpp +++ b/src/mongo/db/index/expression_params.cpp @@ -48,25 +48,23 @@ void ExpressionParams::parseTwoDParams(const BSONObj& infoObj, TwoDIndexingParam while (i.more()) { BSONElement e = i.next(); if (e.type() == String && IndexNames::GEO_2D == e.valuestr()) { - uassert(16800, "can't have 2 geo fields", out->geo.size() == 0); - uassert(16801, "2d has to be first in index", out->other.size() == 0); + uassert(16800, "can't have 2 geo fields", out->geo.empty()); + uassert(16801, "2d has to be first in index", out->other.empty()); out->geo = e.fieldName(); } else { int order = 1; if (e.isNumber()) { order = static_cast<int>(e.Number()); } - out->other.push_back(std::make_pair(e.fieldName(), order)); + out->other.emplace_back(e.fieldName(), order); } } uassert(16802, "no geo field specified", out->geo.size()); - GeoHashConverter::Parameters hashParams; - Status paramStatus = GeoHashConverter::parseParameters(infoObj, &hashParams); - uassertStatusOK(paramStatus); - - out->geoHashConverter.reset(new GeoHashConverter(hashParams)); + auto result = GeoHashConverter::createFromDoc(infoObj); + uassertStatusOK(result.getStatus()); + out->geoHashConverter.reset(result.getValue().release()); } void ExpressionParams::parseHashParams(const BSONObj& infoObj, diff --git a/src/mongo/db/query/expression_index.cpp b/src/mongo/db/query/expression_index.cpp index c6c784be231..cf9202ea653 100644 --- a/src/mongo/db/query/expression_index.cpp +++ b/src/mongo/db/query/expression_index.cpp @@ -71,13 +71,12 @@ static std::string toCoveringString(const GeoHashConverter& hashConverter, std::vector<GeoHash> ExpressionMapping::get2dCovering(const R2Region& region, const BSONObj& indexInfoObj, int maxCoveringCells) { - GeoHashConverter::Parameters hashParams; - Status paramStatus = GeoHashConverter::parseParameters(indexInfoObj, &hashParams); - verify(paramStatus.isOK()); // We validated the parameters when creating the index + auto result = GeoHashConverter::createFromDoc(indexInfoObj); + verify(result.isOK()); // We validated the parameters when creating the index. - GeoHashConverter hashConverter(hashParams); - R2RegionCoverer coverer(&hashConverter); - coverer.setMaxLevel(hashConverter.getBits()); + const auto bits = result.getValue()->getBits(); + R2RegionCoverer coverer(std::move(result.getValue())); + coverer.setMaxLevel(bits); coverer.setMaxCells(maxCoveringCells); // TODO: Maybe slightly optimize by returning results in order diff --git a/src/mongo/db/query/planner_ixselect.cpp b/src/mongo/db/query/planner_ixselect.cpp index e228153bb72..2f390eec1f0 100644 --- a/src/mongo/db/query/planner_ixselect.cpp +++ b/src/mongo/db/query/planner_ixselect.cpp @@ -185,14 +185,11 @@ static double fieldWithDefault(const BSONObj& infoObj, const string& name, doubl * 2d indices don't handle wrapping so we can't use them for queries that wrap. */ static bool twoDWontWrap(const Circle& circle, const IndexEntry& index) { - GeoHashConverter::Parameters hashParams; - Status paramStatus = GeoHashConverter::parseParameters(index.infoObj, &hashParams); - verify(paramStatus.isOK()); // we validated the params on index creation - - GeoHashConverter conv(hashParams); + auto conv = GeoHashConverter::createFromDoc(index.infoObj); + uassertStatusOK(conv.getStatus()); // We validated the parameters when creating the index. // FYI: old code used flat not spherical error. - double yscandist = rad2deg(circle.radius) + conv.getErrorSphere(); + double yscandist = rad2deg(circle.radius) + conv.getValue()->getErrorSphere(); double xscandist = computeXScanDistance(circle.center.y, yscandist); bool ret = circle.center.x + xscandist < 180 && circle.center.x - xscandist > -180 && circle.center.y + yscandist < 90 && circle.center.y - yscandist > -90; |