SERVER-58789 Make GeometryContainer copyable

11 files changed, 216 insertions, 72 deletions

diff --git a/src/mongo/db/geo/geometry_container.cpp b/src/mongo/db/geo/geometry_container.cpp
index 2bf9dbba5c7..c3c3adaa1b2 100644
--- a/src/mongo/db/geo/geometry_container.cpp
+++ b/src/mongo/db/geo/geometry_container.cpp
@@ -230,6 +230,46 @@ Box GeometryContainer::R2BoxRegion::buildBounds(const GeometryContainer& geometr
     return bounds;
 }
 
+GeometryContainer::GeometryContainer(const GeometryContainer& other)
+    : _point{other._point},
+      _line{other._line},
+      _box{other._box},
+      _polygon{other._polygon},
+      _cap{other._cap},
+      _multiPoint{other._multiPoint},
+      _multiLine{other._multiLine},
+      _multiPolygon{other._multiPolygon},
+      _geometryCollection{other._geometryCollection} {
+    if (other._s2Region) {
+        _s2Region.reset(other._s2Region->Clone());
+    }
+    if (hasR2Region()) {
+        _r2Region.reset(new R2BoxRegion(this));
+    }
+}
+
+GeometryContainer& GeometryContainer::operator=(const GeometryContainer& other) {
+    if (&other != this) {
+        _point = other._point;
+        _line = other._line;
+        _box = other._box;
+        _polygon = other._polygon;
+        _cap = other._cap;
+        _multiPoint = other._multiPoint;
+        _multiLine = other._multiLine;
+        _multiPolygon = other._multiPolygon;
+        _geometryCollection = other._geometryCollection;
+
+        if (other._s2Region) {
+            _s2Region.reset(other._s2Region->Clone());
+        }
+        if (hasR2Region()) {
+            _r2Region.reset(new R2BoxRegion(this));
+        }
+    }
+    return *this;
+}
+
 const R2Region& GeometryContainer::getR2Region() const {
     return *_r2Region;
 }
diff --git a/src/mongo/db/geo/geometry_container.h b/src/mongo/db/geo/geometry_container.h
index aa5bc3cf81e..bc73a6d9bd5 100644
--- a/src/mongo/db/geo/geometry_container.h
+++ b/src/mongo/db/geo/geometry_container.h
@@ -31,6 +31,7 @@
 
 #include <string>
 
+#include "mongo/base/clonable_ptr.h"
 #include "mongo/db/bson/dotted_path_support.h"
 #include "mongo/db/geo/shapes.h"
 #include "third_party/s2/s2regionunion.h"
@@ -38,15 +39,15 @@
 namespace mongo {
 
 class GeometryContainer {
-    GeometryContainer(const GeometryContainer&) = delete;
-    GeometryContainer& operator=(const GeometryContainer&) = delete;
-
 public:
     /**
      * Creates an empty geometry container which may then be loaded from BSON or directly.
      */
     GeometryContainer() = default;
 
+    GeometryContainer(const GeometryContainer&);
+    GeometryContainer& operator=(const GeometryContainer&);
+
     /**
      * Loads an empty GeometryContainer from query.
      */
@@ -154,18 +155,18 @@ private:
     bool contains(const S2Polyline& otherLine) const;
     bool contains(const S2Polygon& otherPolygon) const;
 
-    // Only one of these shared_ptrs should be non-NULL.  S2Region is a
+    // Only one of these clonable_ptrs should be non-NULL.  S2Region is a
     // superclass but it only supports testing against S2Cells.  We need
     // the most specific class we can get.
-    std::unique_ptr<PointWithCRS> _point;
-    std::unique_ptr<LineWithCRS> _line;
-    std::unique_ptr<BoxWithCRS> _box;
-    std::unique_ptr<PolygonWithCRS> _polygon;
-    std::unique_ptr<CapWithCRS> _cap;
-    std::unique_ptr<MultiPointWithCRS> _multiPoint;
-    std::unique_ptr<MultiLineWithCRS> _multiLine;
-    std::unique_ptr<MultiPolygonWithCRS> _multiPolygon;
-    std::unique_ptr<GeometryCollection> _geometryCollection;
+    clonable_ptr<PointWithCRS> _point;
+    clonable_ptr<LineWithCRS> _line;
+    clonable_ptr<BoxWithCRS> _box;
+    clonable_ptr<PolygonWithCRS> _polygon;
+    clonable_ptr<CapWithCRS> _cap;
+    clonable_ptr<MultiPointWithCRS> _multiPoint;
+    clonable_ptr<MultiLineWithCRS> _multiLine;
+    clonable_ptr<MultiPolygonWithCRS> _multiPolygon;
+    clonable_ptr<GeometryCollection> _geometryCollection;
 
     // Cached for use during covering calculations
     // TODO: _s2Region is currently generated immediately - don't necessarily need to do this
diff --git a/src/mongo/db/geo/shapes.cpp b/src/mongo/db/geo/shapes.cpp
index d0a062c5e2c..147147833df 100644
--- a/src/mongo/db/geo/shapes.cpp
+++ b/src/mongo/db/geo/shapes.cpp
@@ -440,6 +440,101 @@ string R2Annulus::toString() const {
                          << " outer: " << _outer;
 }
 
+std::unique_ptr<PointWithCRS> PointWithCRS::clone() const {
+    auto cloned = std::make_unique<PointWithCRS>();
+    cloned->crs = crs;
+
+    cloned->point = point;
+    cloned->cell = cell;
+    cloned->oldPoint = oldPoint;
+
+    return cloned;
+}
+
+std::unique_ptr<LineWithCRS> LineWithCRS::clone() const {
+    auto cloned = std::make_unique<LineWithCRS>();
+    cloned->crs = crs;
+
+    std::vector<S2Point> vertices;
+    for (int i = 0; i < line.num_vertices(); ++i) {
+        vertices.emplace_back(line.vertex(i));
+    }
+    cloned->line.Init(vertices);
+
+    return cloned;
+}
+
+std::unique_ptr<CapWithCRS> CapWithCRS::clone() const {
+    auto cloned = std::make_unique<CapWithCRS>();
+    cloned->crs = crs;
+
+    cloned->cap = cap;
+
+    return cloned;
+}
+
+std::unique_ptr<BoxWithCRS> BoxWithCRS::clone() const {
+    auto cloned = std::make_unique<BoxWithCRS>();
+    cloned->crs = crs;
+
+    cloned->box = box;
+
+    return cloned;
+}
+
+std::unique_ptr<PolygonWithCRS> PolygonWithCRS::clone() const {
+    auto cloned = std::make_unique<PolygonWithCRS>();
+    cloned->crs = crs;
+
+    if (s2Polygon) {
+        cloned->s2Polygon.reset(s2Polygon->Clone());
+    }
+    if (bigPolygon) {
+        cloned->bigPolygon.reset(bigPolygon->Clone());
+    }
+    cloned->oldPolygon.init(oldPolygon);
+
+    return cloned;
+}
+
+std::unique_ptr<MultiPointWithCRS> MultiPointWithCRS::clone() const {
+    auto cloned = std::make_unique<MultiPointWithCRS>();
+    cloned->crs = crs;
+
+    cloned->points = points;
+    cloned->cells = cells;
+
+    return cloned;
+}
+
+std::unique_ptr<MultiLineWithCRS> MultiLineWithCRS::clone() const {
+    auto cloned = std::make_unique<MultiLineWithCRS>();
+    cloned->crs = crs;
+
+    for (const auto& line : lines) {
+        invariant(line);
+        cloned->lines.emplace_back(line->Clone());
+    }
+
+    return cloned;
+}
+
+std::unique_ptr<MultiPolygonWithCRS> MultiPolygonWithCRS::clone() const {
+    auto cloned = std::make_unique<MultiPolygonWithCRS>();
+    cloned->crs = crs;
+
+    for (const auto& polygon : polygons) {
+        invariant(polygon);
+        cloned->polygons.emplace_back(polygon->Clone());
+    }
+
+    return cloned;
+}
+
+std::unique_ptr<GeometryCollection> GeometryCollection::clone() const {
+    return std::make_unique<GeometryCollection>(*this);
+}
+
 /////// Other methods
 
 double S2Distance::distanceRad(const S2Point& pointA, const S2Point& pointB) {
diff --git a/src/mongo/db/geo/shapes.h b/src/mongo/db/geo/shapes.h
index 56f1849304a..9bfa711adb4 100644
--- a/src/mongo/db/geo/shapes.h
+++ b/src/mongo/db/geo/shapes.h
@@ -33,6 +33,7 @@
 #include <string>
 #include <vector>
 
+#include "mongo/base/clonable_ptr.h"
 #include "mongo/db/geo/big_polygon.h"
 #include "mongo/db/geo/s2.h"
 #include "mongo/db/jsobj.h"
@@ -260,6 +261,7 @@ enum CRS {
 
 struct PointWithCRS {
     PointWithCRS() : crs(UNSET) {}
+    std::unique_ptr<PointWithCRS> clone() const;
 
     S2Point point;
     S2Cell cell;
@@ -269,6 +271,7 @@ struct PointWithCRS {
 
 struct LineWithCRS {
     LineWithCRS() : crs(UNSET) {}
+    std::unique_ptr<LineWithCRS> clone() const;
 
     S2Polyline line;
     CRS crs;
@@ -276,6 +279,7 @@ struct LineWithCRS {
 
 struct CapWithCRS {
     CapWithCRS() : crs(UNSET) {}
+    std::unique_ptr<CapWithCRS> clone() const;
 
     S2Cap cap;
     Circle circle;
@@ -284,6 +288,7 @@ struct CapWithCRS {
 
 struct BoxWithCRS {
     BoxWithCRS() : crs(UNSET) {}
+    std::unique_ptr<BoxWithCRS> clone() const;
 
     Box box;
     CRS crs;
@@ -291,6 +296,7 @@ struct BoxWithCRS {
 
 struct PolygonWithCRS {
     PolygonWithCRS() : crs(UNSET) {}
+    std::unique_ptr<PolygonWithCRS> clone() const;
 
     std::unique_ptr<S2Polygon> s2Polygon;
     // Simple polygons with strict winding order may be bigger or smaller than a hemisphere.
@@ -303,6 +309,7 @@ struct PolygonWithCRS {
 
 struct MultiPointWithCRS {
     MultiPointWithCRS() : crs(UNSET) {}
+    std::unique_ptr<MultiPointWithCRS> clone() const;
 
     std::vector<S2Point> points;
     std::vector<S2Cell> cells;
@@ -311,6 +318,7 @@ struct MultiPointWithCRS {
 
 struct MultiLineWithCRS {
     MultiLineWithCRS() : crs(UNSET) {}
+    std::unique_ptr<MultiLineWithCRS> clone() const;
 
     std::vector<std::unique_ptr<S2Polyline>> lines;
     CRS crs;
@@ -318,20 +326,23 @@ struct MultiLineWithCRS {
 
 struct MultiPolygonWithCRS {
     MultiPolygonWithCRS() : crs(UNSET) {}
+    std::unique_ptr<MultiPolygonWithCRS> clone() const;
 
     std::vector<std::unique_ptr<S2Polygon>> polygons;
     CRS crs;
 };
 
 struct GeometryCollection {
+    std::unique_ptr<GeometryCollection> clone() const;
+
     std::vector<PointWithCRS> points;
 
-    // The amount of indirection here is painful but we can't operator= unique_ptr.
-    std::vector<std::unique_ptr<LineWithCRS>> lines;
-    std::vector<std::unique_ptr<PolygonWithCRS>> polygons;
-    std::vector<std::unique_ptr<MultiPointWithCRS>> multiPoints;
-    std::vector<std::unique_ptr<MultiLineWithCRS>> multiLines;
-    std::vector<std::unique_ptr<MultiPolygonWithCRS>> multiPolygons;
+    // The amount of indirection here is painful but we can't assign these geometric types.
+    std::vector<clonable_ptr<LineWithCRS>> lines;
+    std::vector<clonable_ptr<PolygonWithCRS>> polygons;
+    std::vector<clonable_ptr<MultiPointWithCRS>> multiPoints;
+    std::vector<clonable_ptr<MultiLineWithCRS>> multiLines;
+    std::vector<clonable_ptr<MultiPolygonWithCRS>> multiPolygons;
 
     bool supportsContains() {
         // Only polygons (and multiPolygons) support containment.
diff --git a/src/mongo/db/matcher/expression_algo.cpp b/src/mongo/db/matcher/expression_algo.cpp
index a453d07b50e..29e1d25a04b 100644
--- a/src/mongo/db/matcher/expression_algo.cpp
+++ b/src/mongo/db/matcher/expression_algo.cpp
@@ -698,38 +698,19 @@ bool isSubsetOf(const MatchExpression* lhs, const MatchExpression* rhs) {
 
     if (lhs->matchType() == MatchExpression::INTERNAL_BUCKET_GEO_WITHIN &&
         rhs->matchType() == MatchExpression::INTERNAL_BUCKET_GEO_WITHIN) {
-        auto indexMatchExpression = static_cast<const InternalBucketGeoWithinMatchExpression*>(rhs);
+        const auto* queryMatchExpression =
+            static_cast<const InternalBucketGeoWithinMatchExpression*>(lhs);
+        const auto* indexMatchExpression =
+            static_cast<const InternalBucketGeoWithinMatchExpression*>(rhs);
 
-        // {$_internalBucketGeoWithin: {$withinRegion: {$geometry: ...}, field: 'loc' }}
-        auto queryInternalBucketGeoWithinObj = lhs->serialize();
-        // '$_internalBucketGeoWithin: {$withinRegion: ... , field: 'loc' }'
-        auto queryInternalBucketGeoWithinElement = queryInternalBucketGeoWithinObj.firstElement();
         // Confirm that the "field" arguments match before continuing.
-        if (queryInternalBucketGeoWithinElement["field"].type() != mongo::String ||
-            queryInternalBucketGeoWithinElement["field"].valueStringData() !=
-                indexMatchExpression->getField()) {
+        if (queryMatchExpression->getField() != indexMatchExpression->getField()) {
             return false;
         }
-        // {$withinRegion: {$geometry: {type: "Polygon", coords:[...]}}
-        auto queryWithinRegionObj = queryInternalBucketGeoWithinElement.Obj();
-        // '$withinRegion: {$geometry: {type: "Polygon", coords:[...]}'
-        auto queryWithinRegionElement = queryWithinRegionObj.firstElement();
-        // {$geometry: {type: "Polygon", coordinates: [...]}}
-        auto queryGeometryObj = queryWithinRegionElement.Obj();
-
-        // We only handle $_internalBucketGeoWithin queries that use the $geometry operator.
-        if (!queryGeometryObj.hasField("$geometry"))
-            return false;
-
-        // geometryElement is '$geometry: {type: ... }'
-        auto queryGeometryElement = queryGeometryObj.firstElement();
-        MatchDetails* details = nullptr;
 
-        if (GeoMatchExpression::contains(*indexMatchExpression->getGeoContainer(),
-                                         GeoExpression::WITHIN,
-                                         false,
-                                         queryGeometryElement,
-                                         details)) {
+        GeometryContainer geometry = queryMatchExpression->getGeoContainer();
+        if (GeoMatchExpression::contains(
+                indexMatchExpression->getGeoContainer(), GeoExpression::WITHIN, &geometry)) {
             // The region described by query is within the region captured by the index.
             // For example, a query over the $geometry for the city of Houston is covered by an
             // index over the $geometry for the entire state of texas. Therefore this index can be
@@ -745,26 +726,15 @@ bool isSubsetOf(const MatchExpression* lhs, const MatchExpression* rhs) {
         // [...]}}' geometryObj is  {$geometry: {type: "Polygon", coordinates: [...]}}
         // geometryElement '$geometry: {type: "Polygon", coordinates: [...]}'
 
-        const GeoMatchExpression* queryMatchExpression =
-            static_cast<const GeoMatchExpression*>(lhs);
+        const auto* queryMatchExpression = static_cast<const GeoMatchExpression*>(lhs);
         // We only handle geoWithin queries
         if (queryMatchExpression->getGeoExpression().getPred() != GeoExpression::WITHIN) {
             return false;
         }
-        const GeoMatchExpression* indexMatchExpression =
-            static_cast<const GeoMatchExpression*>(rhs);
-        auto geoWithinObj = queryMatchExpression->getSerializedRightHandSide();
-        auto geoWithinElement = geoWithinObj.firstElement();
-        auto geometryObj = geoWithinElement.Obj();
-
-        // More specifically, we only handle geoWithin queries that use the $geometry operator.
-        if (!geometryObj.hasField("$geometry")) {
-            return false;
-        }
-        auto geometryElement = geometryObj.firstElement();
-        MatchDetails* details = nullptr;
+        const auto* indexMatchExpression = static_cast<const GeoMatchExpression*>(rhs);
 
-        if (indexMatchExpression->matchesSingleElement(geometryElement, details)) {
+        auto geometryContainer = queryMatchExpression->getGeoExpression().getGeometry();
+        if (indexMatchExpression->matchesGeoContainer(geometryContainer)) {
             // The region described by query is within the region captured by the index.
             // Therefore this index can be used in a potential solution for this query.
             return true;
diff --git a/src/mongo/db/matcher/expression_geo.cpp b/src/mongo/db/matcher/expression_geo.cpp
index 84129edfd3a..fe592df742e 100644
--- a/src/mongo/db/matcher/expression_geo.cpp
+++ b/src/mongo/db/matcher/expression_geo.cpp
@@ -368,7 +368,7 @@ bool GeoMatchExpression::contains(const GeometryContainer& queryGeom,
                                   const GeoExpression::Predicate& queryPredicate,
                                   bool skipValidation,
                                   const BSONElement& e,
-                                  MatchDetails* details) {
+                                  MatchDetails*) {
     if (!e.isABSONObj())
         return false;
 
@@ -389,16 +389,39 @@ bool GeoMatchExpression::contains(const GeometryContainer& queryGeom,
     if (!geometry.supportsProject(queryGeom.getNativeCRS()))
         return false;
 
-    geometry.projectInto(queryGeom.getNativeCRS());
+    return contains(queryGeom, queryPredicate, &geometry);
+}
 
+bool GeoMatchExpression::contains(const GeometryContainer& queryGeom,
+                                  const GeoExpression::Predicate& queryPredicate,
+                                  GeometryContainer* geometry) {
+    geometry->projectInto(queryGeom.getNativeCRS());
     if (GeoExpression::WITHIN == queryPredicate) {
-        return queryGeom.contains(geometry);
+        return queryGeom.contains(*geometry);
     } else {
         verify(GeoExpression::INTERSECT == queryPredicate);
-        return queryGeom.intersects(geometry);
+        return queryGeom.intersects(*geometry);
     }
 }
 
+bool GeoMatchExpression::matchesGeoContainer(const GeometryContainer& input) const {
+    // Never match big polygon
+    if (input.getNativeCRS() == STRICT_SPHERE)
+        return false;
+
+    // Project this geometry into the CRS of the larger geometry.
+
+    // In the case of index validation, we are projecting the geometry of the query
+    // into the CRS of the index to confirm that the index region convers/includes
+    // the region described by the predicate.
+
+    if (!input.supportsProject(_query->getGeometry().getNativeCRS()))
+        return false;
+
+    GeometryContainer geometry{input};
+    return contains(_query->getGeometry(), _query->getPred(), &geometry);
+}
+
 void GeoMatchExpression::debugString(StringBuilder& debug, int indentationLevel) const {
     _debugAddSpace(debug, indentationLevel);
 
diff --git a/src/mongo/db/matcher/expression_geo.h b/src/mongo/db/matcher/expression_geo.h
index 394eeb9cfb7..bd3752399c8 100644
--- a/src/mongo/db/matcher/expression_geo.h
+++ b/src/mongo/db/matcher/expression_geo.h
@@ -97,8 +97,12 @@ public:
                          const GeoExpression::Predicate& queryPredicate,
                          bool skipValidation,
                          const BSONElement& e,
-                         MatchDetails* details);
+                         MatchDetails*);
+    static bool contains(const GeometryContainer& queryGeom,
+                         const GeoExpression::Predicate& queryPredicate,
+                         GeometryContainer* geometry);
     bool matchesSingleElement(const BSONElement&, MatchDetails* details = nullptr) const final;
+    bool matchesGeoContainer(const GeometryContainer&) const;
 
     virtual void debugString(StringBuilder& debug, int indentationLevel = 0) const;
 
diff --git a/src/mongo/db/matcher/expression_internal_bucket_geo_within.cpp b/src/mongo/db/matcher/expression_internal_bucket_geo_within.cpp
index 2b24baa1672..5bf75f8218f 100644
--- a/src/mongo/db/matcher/expression_internal_bucket_geo_within.cpp
+++ b/src/mongo/db/matcher/expression_internal_bucket_geo_within.cpp
@@ -72,7 +72,7 @@ bool InternalBucketGeoWithinMatchExpression::equivalent(const MatchExpression* e
 
     return SimpleBSONObjComparator::kInstance.evaluate(
                _geoContainer->getGeoElement().Obj() ==
-               other->getGeoContainer()->getGeoElement().Obj()) &&
+               other->getGeoContainer().getGeoElement().Obj()) &&
         _field == other->getField();
 }
 
diff --git a/src/mongo/db/matcher/expression_internal_bucket_geo_within.h b/src/mongo/db/matcher/expression_internal_bucket_geo_within.h
index 2864c71a05e..b46b6c5076c 100644
--- a/src/mongo/db/matcher/expression_internal_bucket_geo_within.h
+++ b/src/mongo/db/matcher/expression_internal_bucket_geo_within.h
@@ -118,8 +118,8 @@ public:
         return _field;
     }
 
-    const std::shared_ptr<GeometryContainer> getGeoContainer() const {
-        return _geoContainer;
+    const GeometryContainer& getGeoContainer() const {
+        return *_geoContainer;
     }
 
     const StringData path() const final {
diff --git a/src/mongo/db/query/index_bounds_builder.cpp b/src/mongo/db/query/index_bounds_builder.cpp
index 145cd901c97..c347944fb3c 100644
--- a/src/mongo/db/query/index_bounds_builder.cpp
+++ b/src/mongo/db/query/index_bounds_builder.cpp
@@ -1155,8 +1155,8 @@ void IndexBoundsBuilder::_translatePredicate(const MatchExpression* expr,
         if ("2dsphere_bucket"_sd == elt.valueStringDataSafe()) {
             tassert(5837101,
                     "A geo query on a sphere must have an S2 region",
-                    ibgwme->getGeoContainer()->hasS2Region());
-            const S2Region& region = ibgwme->getGeoContainer()->getS2Region();
+                    ibgwme->getGeoContainer().hasS2Region());
+            const S2Region& region = ibgwme->getGeoContainer().getS2Region();
             S2IndexingParams indexParams;
             ExpressionParams::initialize2dsphereParams(index.infoObj, index.collator, &indexParams);
             ExpressionMapping::cover2dsphere(region, indexParams, oilOut);
diff --git a/src/mongo/db/query/planner_ixselect.cpp b/src/mongo/db/query/planner_ixselect.cpp
index e4368c7b174..518da370750 100644
--- a/src/mongo/db/query/planner_ixselect.cpp
+++ b/src/mongo/db/query/planner_ixselect.cpp
@@ -577,7 +577,7 @@ bool QueryPlannerIXSelect::_compatible(const BSONElement& keyPatternElt,
             const InternalBucketGeoWithinMatchExpression* ibgwme =
                 static_cast<InternalBucketGeoWithinMatchExpression*>(node);
             auto gc = ibgwme->getGeoContainer();
-            return gc->hasS2Region();
+            return gc.hasS2Region();
         }
         return false;
     } else if (IndexNames::GEO_2D == indexedFieldType) {