SERVER-65049 Optimize away { $expr : true } in $match clauses

5 files changed, 108 insertions, 14 deletions

diff --git a/src/mongo/db/matcher/expression_expr.cpp b/src/mongo/db/matcher/expression_expr.cpp
index 567039995f6..a1d151b8e1c 100644
--- a/src/mongo/db/matcher/expression_expr.cpp
+++ b/src/mongo/db/matcher/expression_expr.cpp
@@ -126,6 +126,11 @@ std::unique_ptr<MatchExpression> ExprMatchExpression::shallowClone() const {
     return clone;
 }
 
+bool ExprMatchExpression::isTriviallyTrue() const {
+    auto exprConst = dynamic_cast<ExpressionConstant*>(_expression.get());
+    return exprConst && exprConst->getValue().coerceToBool();
+}
+
 MatchExpression::ExpressionOptimizerFunc ExprMatchExpression::getOptimizer() const {
     return [](std::unique_ptr<MatchExpression> expression) {
         auto& exprMatchExpr = static_cast<ExprMatchExpression&>(*expression);
@@ -150,6 +155,12 @@ MatchExpression::ExpressionOptimizerFunc ExprMatchExpression::getOptimizer() con
             expression = MatchExpression::optimize(std::move(andMatch));
         }
 
+        // Replace trivially true expression with an empty AND since the planner doesn't always
+        // check for 'isTriviallyTrue()'.
+        if (expression->isTriviallyTrue()) {
+            expression = std::make_unique<AndMatchExpression>();
+        }
+
         return expression;
     };
 }
diff --git a/src/mongo/db/matcher/expression_expr.h b/src/mongo/db/matcher/expression_expr.h
index 0d1110f5bac..5e5be65cbbe 100644
--- a/src/mongo/db/matcher/expression_expr.h
+++ b/src/mongo/db/matcher/expression_expr.h
@@ -77,6 +77,8 @@ public:
 
     void serialize(BSONObjBuilder* out, bool includePath) const final;
 
+    bool isTriviallyTrue() const final;
+
     bool equivalent(const MatchExpression* other) const final;
 
     MatchCategory getCategory() const final {
diff --git a/src/mongo/db/matcher/expression_expr_test.cpp b/src/mongo/db/matcher/expression_expr_test.cpp
index 801b3634813..01bd01818b0 100644
--- a/src/mongo/db/matcher/expression_expr_test.cpp
+++ b/src/mongo/db/matcher/expression_expr_test.cpp
@@ -78,6 +78,10 @@ public:
         return _matchExpression->matchesBSON(doc);
     }
 
+    MatchExpression* getMatchExpression() {
+        return _matchExpression.get();
+    }
+
     ExprMatchExpression* getExprMatchExpression() {
         return checked_cast<ExprMatchExpression*>(_matchExpression.get());
     }
@@ -606,9 +610,13 @@ TEST_F(ExprMatchTest, ReturnsFalseInsteadOfErrorWithFailpointSet) {
 }
 
 TEST(ExprMatchTest, IdenticalPostOptimizedExpressionsAreEquivalent) {
-    BSONObj expression = BSON("$expr" << BSON("$multiply" << BSON_ARRAY(2 << 2)));
-    BSONObj expressionEquiv = BSON("$expr" << BSON("$const" << 4));
-    BSONObj expressionNotEquiv = BSON("$expr" << BSON("$const" << 10));
+    BSONObj expression =
+        BSON("$expr" << BSON("$ifNull" << BSON_ARRAY("$NO_SUCH_FIELD"
+                                                     << BSON("$multiply" << BSON_ARRAY(2 << 2)))));
+    BSONObj expressionEquiv =
+        BSON("$expr" << BSON("$ifNull" << BSON_ARRAY("$NO_SUCH_FIELD" << BSON("$const" << 4))));
+    BSONObj expressionNotEquiv =
+        BSON("$expr" << BSON("$ifNull" << BSON_ARRAY("$NO_SUCH_FIELD" << BSON("$const" << 10))));
 
     // Create and optimize an ExprMatchExpression.
     const boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
@@ -634,11 +642,12 @@ TEST(ExprMatchTest, ExpressionOptimizeRewritesVariableDereferenceAsConstant) {
     const boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
     auto varId = expCtx->variablesParseState.defineVariable("var");
     expCtx->variables.setConstantValue(varId, Value(4));
-
-    BSONObj expression = BSON("$expr"
-                              << "$$var");
-    BSONObj expressionEquiv = BSON("$expr" << BSON("$const" << 4));
-    BSONObj expressionNotEquiv = BSON("$expr" << BSON("$const" << 10));
+    BSONObj expression = BSON("$expr" << BSON("$ifNull" << BSON_ARRAY("$NO_SUCH_FIELD"
+                                                                      << "$$var")));
+    BSONObj expressionEquiv =
+        BSON("$expr" << BSON("$ifNull" << BSON_ARRAY("$NO_SUCH_FIELD" << BSON("$const" << 4))));
+    BSONObj expressionNotEquiv =
+        BSON("$expr" << BSON("$ifNull" << BSON_ARRAY("$NO_SUCH_FIELD" << BSON("$const" << 10))));
 
     // Create and optimize an ExprMatchExpression.
     std::unique_ptr<MatchExpression> matchExpr =
@@ -728,8 +737,54 @@ TEST(ExprMatchTest, OptimizingExprAbsorbsAndOfAnd) {
     ASSERT_BSONOBJ_EQ(serialized, expectedSerialization);
 }
 
+TEST(ExprMatchTest, OptimizingExprRemovesTrueConstantExpression) {
+    auto exprBson = fromjson("{$expr: true}");
+    boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
+
+    auto matchExpr =
+        std::make_unique<ExprMatchExpression>(exprBson.firstElement(), std::move(expCtx));
+    auto optimized = MatchExpression::optimize(std::move(matchExpr));
+
+    auto serialization = optimized->serialize();
+    auto expectedSerialization = fromjson("{}");
+    ASSERT_BSONOBJ_EQ(serialization, expectedSerialization);
+}
+
+TEST(ExprMatchTest, OptimizingExprRemovesTruthyConstantExpression) {
+    auto exprBson = fromjson("{$expr: {$concat: ['a', 'b', 'c']}}");
+    boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
+
+    auto matchExpr =
+        std::make_unique<ExprMatchExpression>(exprBson.firstElement(), std::move(expCtx));
+    auto optimized = MatchExpression::optimize(std::move(matchExpr));
+
+    auto serialization = optimized->serialize();
+    auto expectedSerialization = fromjson("{}");
+    ASSERT_BSONOBJ_EQ(serialization, expectedSerialization);
+}
+
+TEST_F(ExprMatchTest, ExprWithTrueConstantExpressionIsTriviallyTrue) {
+    createMatcher(fromjson("{$expr: true}"));
+    ASSERT_TRUE(getMatchExpression()->isTriviallyTrue());
+}
+
+TEST_F(ExprMatchTest, ExprWithTruthyConstantExpressionIsTriviallyTrue) {
+    createMatcher(fromjson("{$expr: {$concat: ['a', 'b', 'c']}}"));
+    ASSERT_TRUE(getMatchExpression()->isTriviallyTrue());
+}
+
+TEST_F(ExprMatchTest, ExprWithNonConstantExpressionIsNotTriviallyTrue) {
+    createMatcher(fromjson("{$expr: {$concat: ['$a', '$b', '$c']}}"));
+    ASSERT_FALSE(getMatchExpression()->isTriviallyTrue());
+}
+
+TEST_F(ExprMatchTest, ExprWithFalsyConstantExpressionIsNotTriviallyTrue) {
+    createMatcher(fromjson("{$expr: {$sum: [1, -1]}}"));
+    ASSERT_FALSE(getMatchExpression()->isTriviallyTrue());
+}
+
 TEST_F(ExprMatchTest, ExpressionEvaluationReturnsResultsCorrectly) {
-    createMatcher(fromjson("{$expr: -2}"));
+    createMatcher(fromjson("{$expr: {$ifNull: ['$NO_SUCH_FIELD', -2]}}"));
     BSONMatchableDocument document{BSONObj{}};
     auto expressionResult = getExprMatchExpression()->evaluateExpression(&document);
     ASSERT_TRUE(expressionResult.integral());
diff --git a/src/mongo/db/pipeline/document_source_match.cpp b/src/mongo/db/pipeline/document_source_match.cpp
index 038b38100c4..014ce55364b 100644
--- a/src/mongo/db/pipeline/document_source_match.cpp
+++ b/src/mongo/db/pipeline/document_source_match.cpp
@@ -83,6 +83,10 @@ intrusive_ptr<DocumentSource> DocumentSourceMatch::optimize() {
 
     _expression = MatchExpression::optimize(std::move(_expression));
 
+    if (_expression->isTriviallyTrue()) {
+        return nullptr;
+    }
+
     return this;
 }
 
diff --git a/src/mongo/db/pipeline/pipeline_test.cpp b/src/mongo/db/pipeline/pipeline_test.cpp
index 446973aa1ec..90e4193ee8b 100644
--- a/src/mongo/db/pipeline/pipeline_test.cpp
+++ b/src/mongo/db/pipeline/pipeline_test.cpp
@@ -991,15 +991,20 @@ TEST(PipelineOptimizationTest, RemoveEmptyMatch) {
 }
 
 TEST(PipelineOptimizationTest, RemoveMultipleEmptyMatches) {
-    string inputPipe = "[{$match: {}}, {$match: {}}]";
-
-    string outputPipe = "[{$match: {}}]";
-
-    string serializedPipe = "[{$match: {$and: [{}, {}]}}]";
+    assertPipelineOptimizesTo("[{$match: {}}, {$match: {}}]", "[]");
+}
 
+TEST(PipelineOptimizationTest, RemoveEmptyMatchesAndKeepNonEmptyMatches) {
+    string inputPipe = "[{$match: {}}, {$match: {}}, {$match: {a: 1}}]";
+    string outputPipe = "[{$match: {a: {$eq: 1}}}]";
+    string serializedPipe = "[{$match: {$and: [{}, {}, {a: 1}]}}]";
     assertPipelineOptimizesAndSerializesTo(inputPipe, outputPipe, serializedPipe);
 }
 
+TEST(PipelineOptimizationTest, RemoveEmptyMatchesAndKeepOtherStages) {
+    assertPipelineOptimizesTo("[{$match: {}}, {$skip: 1}, {$match: {}}]", "[{$skip: 1}]");
+}
+
 TEST(PipelineOptimizationTest, DoNotRemoveNonEmptyMatch) {
     string inputPipe = "[{$match: {_id: 1}}]";
 
@@ -1010,6 +1015,23 @@ TEST(PipelineOptimizationTest, DoNotRemoveNonEmptyMatch) {
     assertPipelineOptimizesAndSerializesTo(inputPipe, outputPipe, serializedPipe);
 }
 
+TEST(PipelineOptimizationTest, RemoveMatchWithTrueConstExpr) {
+    assertPipelineOptimizesTo("[{$match: {$expr: true}}]", "[]");
+}
+
+TEST(PipelineOptimizationTest, RemoveMultipleMatchesWithTrueConstExpr) {
+    assertPipelineOptimizesTo("[{$match: {$expr: true}}, {$match: {$expr: true}}]", "[]");
+}
+
+TEST(PipelineOptimizationTest, RemoveMatchWithTruthyConstExpr) {
+    assertPipelineOptimizesTo("[{$match: {$expr: {$concat: ['a', 'b']}}}]", "[]");
+}
+
+TEST(PipelineOptimizationTest, KeepMatchWithNonConstExpr) {
+    assertPipelineOptimizesTo("[{$match: {$expr: {$concat: ['$a', '$b']}}}]",
+                              "[{$match: {$expr: {$concat: ['$a', '$b']}}}]");
+}
+
 TEST(PipelineOptimizationTest, MoveMatchBeforeSort) {
     std::string inputPipe = "[{$sort: {b: 1}}, {$match: {a: 2}}]";
     std::string outputPipe = "[{$match: {a: {$eq : 2}}}, {$sort: {sortKey: {b: 1}}}]";