SERVER-32690 Fix invariant failure in agg renamed paths optimization.

3 files changed, 39 insertions, 4 deletions

diff --git a/jstests/aggregation/match_swapping_renamed_fields.js b/jstests/aggregation/match_swapping_renamed_fields.js
index 91afcba8af7..92340a868cb 100644
--- a/jstests/aggregation/match_swapping_renamed_fields.js
+++ b/jstests/aggregation/match_swapping_renamed_fields.js
@@ -198,4 +198,17 @@
     ixscan = getAggPlanStage(explain, "IXSCAN");
     assert.neq(null, ixscan, tojson(explain));
     assert.eq({"a.b.c": 1}, ixscan.keyPattern, tojson(ixscan));
+
+    // Test multiple renames. Designed to reproduce SERVER-32690.
+    pipeline = [
+        {$_internalInhibitOptimization: {}},
+        {$project: {x: "$x", y: "$x"}},
+        {$match: {y: 1, w: 1}}
+    ];
+    assert.eq([], coll.aggregate(pipeline).toArray());
+    explain = coll.explain().aggregate(pipeline);
+    // We expect that the $match stage has been split into two, since one predicate has an
+    // applicable rename that allows swapping, while the other does not.
+    let matchStages = getAggPlanStages(explain, "$match");
+    assert.eq(2, matchStages.length);
 }());
diff --git a/src/mongo/db/matcher/expression_algo_test.cpp b/src/mongo/db/matcher/expression_algo_test.cpp
index ca0825b3185..4b0bacb1b9f 100644
--- a/src/mongo/db/matcher/expression_algo_test.cpp
+++ b/src/mongo/db/matcher/expression_algo_test.cpp
@@ -1186,6 +1186,25 @@ TEST(SplitMatchExpression, ShouldMoveMaxLengthAcrossRename) {
     ASSERT_FALSE(splitExpr.second.get());
 }
 
+TEST(SplitMatchExpression, ShouldMoveIndependentPredicateWhenThereAreMultipleRenames) {
+    // Designed to reproduce SERVER-32690.
+    BSONObj matchPredicate = fromjson("{y: 3}");
+    boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
+    auto matcher = MatchExpressionParser::parse(matchPredicate, std::move(expCtx));
+    ASSERT_OK(matcher.getStatus());
+
+    StringMap<std::string> renames{{"y", "x"}, {"x", "x"}};
+    std::pair<unique_ptr<MatchExpression>, unique_ptr<MatchExpression>> splitExpr =
+        expression::splitMatchExpressionBy(std::move(matcher.getValue()), {}, renames);
+
+    ASSERT_TRUE(splitExpr.first.get());
+    BSONObjBuilder firstBob;
+    splitExpr.first->serialize(&firstBob);
+    ASSERT_BSONOBJ_EQ(firstBob.obj(), fromjson("{x: {$eq: 3}}"));
+
+    ASSERT_FALSE(splitExpr.second.get());
+}
+
 TEST(MapOverMatchExpression, DoesMapOverLogicalNodes) {
     BSONObj matchPredicate = fromjson("{a: {$not: {$eq: 1}}}");
     boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
diff --git a/src/mongo/db/matcher/expression_path.h b/src/mongo/db/matcher/expression_path.h
index 189d9cb4a3b..89cf953fd35 100644
--- a/src/mongo/db/matcher/expression_path.h
+++ b/src/mongo/db/matcher/expression_path.h
@@ -86,11 +86,10 @@ public:
     void applyRename(const StringMap<std::string>& renameList) {
         FieldRef pathFieldRef(_path);
 
-        int renamesFound = 0;
+        size_t renamesFound = 0u;
         for (auto rename : renameList) {
             if (rename.first == _path) {
                 _rewrittenPath = rename.second;
-                setPath(_rewrittenPath);
 
                 ++renamesFound;
             }
@@ -104,14 +103,18 @@ public:
                 // Replace the chopped off components with the component names resulting from the
                 // rename.
                 _rewrittenPath = str::stream() << rename.second << "." << pathTail.toString();
-                setPath(_rewrittenPath);
 
                 ++renamesFound;
             }
         }
 
         // There should never be multiple applicable renames.
-        invariant(renamesFound <= 1);
+        invariant(renamesFound <= 1u);
+        if (renamesFound == 1u) {
+            // There is an applicable rename. Modify the path of this expression to use the new
+            // name.
+            setPath(_rewrittenPath);
+        }
     }
 
 protected: