SERVER-29618 fix serialization of geo match expressionsr3.4.6-rc0r3.4.6

Explicitly blacklist lookup_absorb_match from aggregation_sharded_collections_passthrough.

(cherry picked from commit 18bc61d22123da5897d275eb92576522a1bab4de)

5 files changed, 116 insertions, 21 deletions

diff --git a/buildscripts/resmokeconfig/suites/aggregation_sharded_collections_passthrough.yml b/buildscripts/resmokeconfig/suites/aggregation_sharded_collections_passthrough.yml
index d179184abc8..fa367432dc5 100644
--- a/buildscripts/resmokeconfig/suites/aggregation_sharded_collections_passthrough.yml
+++ b/buildscripts/resmokeconfig/suites/aggregation_sharded_collections_passthrough.yml
@@ -24,6 +24,8 @@ selector:
     - jstests/aggregation/bugs/cursor_timeout.js
     - jstests/aggregation/bugs/lookup_unwind_getmore.js
     - jstests/aggregation/bugs/lookup_unwind_killcursor.js
+    # The following tests assume that accessed collections are unsharded.
+    - jstests/aggregation/sources/lookup/lookup_absorb_match.js
 
 executor:
   js_test:
diff --git a/jstests/aggregation/sources/lookup/lookup_absorb_match.js b/jstests/aggregation/sources/lookup/lookup_absorb_match.js
new file mode 100644
index 00000000000..b00e86aaa98
--- /dev/null
+++ b/jstests/aggregation/sources/lookup/lookup_absorb_match.js
@@ -0,0 +1,97 @@
+/**
+ * Tests that a $match with a geo expression still returns the correct results if it has been
+ * absorbed by a $lookup.
+ */
+(function() {
+    "use strict";
+
+    let testDB = db.getSiblingDB("lookup_absorb_match");
+    testDB.dropDatabase();
+
+    let locations = testDB.getCollection("locations");
+    assert.writeOK(locations.insert({_id: "doghouse", coordinates: [25.0, 60.0]}));
+    assert.writeOK(locations.insert({_id: "bullpen", coordinates: [-25.0, -60.0]}));
+
+    let animals = testDB.getCollection("animals");
+    assert.writeOK(animals.insert({_id: "dog", locationId: "doghouse"}));
+    assert.writeOK(animals.insert({_id: "bull", locationId: "bullpen"}));
+
+    // Test that a $match with $geoWithin works properly when performed directly on an absorbed
+    // lookup field.
+    let result = testDB.animals
+                     .aggregate([
+                         {
+                           $lookup: {
+                               from: "locations",
+                               localField: "locationId",
+                               foreignField: "_id",
+                               as: "location"
+                           }
+                         },
+                         {$unwind: "$location"},
+                         {
+                           $match: {
+                               "location.coordinates": {
+                                   $geoWithin: {
+                                       $geometry: {
+                                           type: "MultiPolygon",
+                                           coordinates: [[[
+                                               [20.0, 70.0],
+                                               [30.0, 70.0],
+                                               [30.0, 50.0],
+                                               [20.0, 50.0],
+                                               [20.0, 70.0]
+                                           ]]]
+                                       }
+                                   }
+                               }
+                           }
+                         }
+                     ])
+                     .toArray();
+    let expected = [{
+        _id: "dog",
+        locationId: "doghouse",
+        location: {_id: "doghouse", coordinates: [25.0, 60.0]}
+    }];
+    assert.eq(result, expected);
+
+    // Test that a $match with $geoIntersects works as expected when absorbed by a $lookup.
+    result = testDB.animals
+                 .aggregate([
+                     {
+                       $lookup: {
+                           from: "locations",
+                           localField: "locationId",
+                           foreignField: "_id",
+                           as: "location"
+                       }
+                     },
+                     {$unwind: "$location"},
+                     {
+                       $match: {
+                           "location.coordinates": {
+                               $geoIntersects: {
+                                   $geometry: {
+                                       type: "MultiPolygon",
+                                       coordinates: [[[
+                                           [-20.0, -70.0],
+                                           [-30.0, -70.0],
+                                           [-30.0, -50.0],
+                                           [-20.0, -50.0],
+                                           [-20.0, -70.0]
+                                       ]]]
+                                   }
+                               }
+                           }
+                       }
+                     }
+                 ])
+                 .toArray();
+    expected = [{
+        _id: "bull",
+        locationId: "bullpen",
+        location: {_id: "bullpen", coordinates: [-25.0, -60.0]}
+    }];
+    assert.eq(result, expected);
+}());
diff --git a/src/mongo/db/matcher/expression_geo.cpp b/src/mongo/db/matcher/expression_geo.cpp
index f229eec42c2..e73efdca1db 100644
--- a/src/mongo/db/matcher/expression_geo.cpp
+++ b/src/mongo/db/matcher/expression_geo.cpp
@@ -364,7 +364,11 @@ bool GeoMatchExpression::matchesSingleElement(const BSONElement& e) const {
 
 void GeoMatchExpression::debugString(StringBuilder& debug, int level) const {
     _debugAddSpace(debug, level);
-    debug << "GEO raw = " << _rawObj.toString();
+
+    BSONObjBuilder builder;
+    serialize(&builder);
+    debug << "GEO raw = " << builder.obj().toString();
+
     MatchExpression::TagData* td = getTag();
     if (NULL != td) {
         debug << " ";
@@ -374,7 +378,9 @@ void GeoMatchExpression::debugString(StringBuilder& debug, int level) const {
 }
 
 void GeoMatchExpression::serialize(BSONObjBuilder* out) const {
-    out->appendElements(_rawObj);
+    BSONObjBuilder subobj(out->subobjStart(path()));
+    subobj.appendElements(_rawObj);
+    subobj.doneFast();
 }
 
 bool GeoMatchExpression::equivalent(const MatchExpression* other) const {
@@ -431,7 +437,9 @@ void GeoNearMatchExpression::debugString(StringBuilder& debug, int level) const
 }
 
 void GeoNearMatchExpression::serialize(BSONObjBuilder* out) const {
-    out->appendElements(_rawObj);
+    BSONObjBuilder subobj(out->subobjStart(path()));
+    subobj.appendElements(_rawObj);
+    subobj.doneFast();
 }
 
 bool GeoNearMatchExpression::equivalent(const MatchExpression* other) const {
diff --git a/src/mongo/db/matcher/expression_geo.h b/src/mongo/db/matcher/expression_geo.h
index 856b31045f8..60ae66f3cb6 100644
--- a/src/mongo/db/matcher/expression_geo.h
+++ b/src/mongo/db/matcher/expression_geo.h
@@ -108,12 +108,11 @@ public:
     const GeoExpression& getGeoExpression() const {
         return *_query;
     }
-    const BSONObj getRawObj() const {
-        return _rawObj;
-    }
 
 private:
+    // The original geo specification provided by the user.
     BSONObj _rawObj;
+
     // Share ownership of our query with all of our clones
     std::shared_ptr<const GeoExpression> _query;
     bool _canSkipValidation;
@@ -183,12 +182,11 @@ public:
     const GeoNearExpression& getData() const {
         return *_query;
     }
-    const BSONObj getRawObj() const {
-        return _rawObj;
-    }
 
 private:
+    // The original geo specification provided by the user.
     BSONObj _rawObj;
+
     // Share ownership of our query with all of our clones
     std::shared_ptr<const GeoNearExpression> _query;
 };
diff --git a/src/mongo/db/matcher/expression_parser_geo.cpp b/src/mongo/db/matcher/expression_parser_geo.cpp
index d706ef65193..433af63ed3d 100644
--- a/src/mongo/db/matcher/expression_parser_geo.cpp
+++ b/src/mongo/db/matcher/expression_parser_geo.cpp
@@ -53,12 +53,7 @@ StatusWithMatchExpression expressionParserGeoCallbackReal(const char* name,
 
         unique_ptr<GeoMatchExpression> e = make_unique<GeoMatchExpression>();
 
-        // Until the index layer accepts non-BSON predicates, or special indices are moved into
-        // stages, we have to clean up the raw object so it can be passed down to the index
-        // layer.
-        BSONObjBuilder bob;
-        bob.append(name, section);
-        Status s = e->init(name, gq.release(), bob.obj());
+        Status s = e->init(name, gq.release(), section);
         if (!s.isOK())
             return StatusWithMatchExpression(s);
         return {std::move(e)};
@@ -70,12 +65,7 @@ StatusWithMatchExpression expressionParserGeoCallbackReal(const char* name,
             return StatusWithMatchExpression(s);
         }
         unique_ptr<GeoNearMatchExpression> e = make_unique<GeoNearMatchExpression>();
-        // Until the index layer accepts non-BSON predicates, or special indices are moved into
-        // stages, we have to clean up the raw object so it can be passed down to the index
-        // layer.
-        BSONObjBuilder bob;
-        bob.append(name, section);
-        s = e->init(name, nq.release(), bob.obj());
+        s = e->init(name, nq.release(), section);
         if (!s.isOK())
             return StatusWithMatchExpression(s);
         return {std::move(e)};