diff options
| -rw-r--r-- | jstests/aggregation/match_swapping_renamed_fields.js | 13 | ||||
| -rw-r--r-- | src/mongo/db/matcher/expression_algo_test.cpp | 19 | ||||
| -rw-r--r-- | src/mongo/db/matcher/expression_path.h | 11 |
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: |