SERVER-36993 Fix crash due to incorrect $or pushdown for indexed $expr.

4 files changed, 71 insertions, 1 deletions

diff --git a/jstests/core/expr_or_pushdown.js b/jstests/core/expr_or_pushdown.js
new file mode 100644
index 00000000000..431e2932ae7
--- /dev/null
+++ b/jstests/core/expr_or_pushdown.js
@@ -0,0 +1,25 @@
+/**
+ * Test that an $expr predicated which is eligible for being indexed with an $or pushdown executes
+ * as expected.
+ */
+(function() {
+    "use strict";
+
+    const coll = db.expr_or_pushdown;
+    coll.drop();
+    assert.commandWorked(coll.createIndex({"a": 1, "b": 1}));
+    assert.commandWorked(coll.insert({_id: 0, a: "a", b: "b", d: "d"}));
+    assert.commandWorked(coll.insert({_id: 1, a: "a", b: "c", d: "d"}));
+    assert.commandWorked(coll.insert({_id: 2, a: "a", b: "x", d: "d"}));
+    assert.commandWorked(coll.insert({_id: 3, a: "x", b: "b", d: "d"}));
+    assert.commandWorked(coll.insert({_id: 4, a: "a", b: "b", d: "x"}));
+
+    const results = coll.find({
+                            $expr: {$and: [{$eq: ["$d", "d"]}, {$eq: ["$a", "a"]}]},
+                            $or: [{"b": "b"}, {"b": "c"}]
+                        })
+                        .sort({_id: 1})
+                        .toArray();
+
+    assert.eq(results, [{_id: 0, a: "a", b: "b", d: "d"}, {_id: 1, a: "a", b: "c", d: "d"}]);
+}());
diff --git a/src/mongo/db/matcher/expression_expr.cpp b/src/mongo/db/matcher/expression_expr.cpp
index 18883040f95..3b265364033 100644
--- a/src/mongo/db/matcher/expression_expr.cpp
+++ b/src/mongo/db/matcher/expression_expr.cpp
@@ -115,7 +115,8 @@ MatchExpression::ExpressionOptimizerFunc ExprMatchExpression::getOptimizer() con
             auto andMatch = stdx::make_unique<AndMatchExpression>();
             andMatch->add(exprMatchExpr._rewriteResult->releaseMatchExpression().release());
             andMatch->add(expression.release());
-            expression = std::move(andMatch);
+            // Re-optimize the new AND in order to make sure that any AND children are absorbed.
+            expression = MatchExpression::optimize(std::move(andMatch));
         }
 
         return expression;
diff --git a/src/mongo/db/matcher/expression_expr_test.cpp b/src/mongo/db/matcher/expression_expr_test.cpp
index 4fa86044d98..552ce3eee18 100644
--- a/src/mongo/db/matcher/expression_expr_test.cpp
+++ b/src/mongo/db/matcher/expression_expr_test.cpp
@@ -689,5 +689,28 @@ TEST(ExprMatchTest, ShallowClonedExpressionIsEquivalentToOriginal) {
     ASSERT_TRUE(pipelineExpr.equivalent(shallowClone.get()));
 }
 
+TEST(ExprMatchTest, OptimizingExprAbsorbsAndOfAnd) {
+    BSONObj exprBson = fromjson("{$expr: {$and: [{$eq: ['$a', 1]}, {$eq: ['$b', 2]}]}}");
+
+    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));
+
+    // The optimized match expression should not have and AND children of AND nodes. This should be
+    // collapsed during optimization.
+    BSONObj serialized;
+    {
+        BSONObjBuilder builder;
+        optimized->serialize(&builder);
+        serialized = builder.obj();
+    }
+
+    BSONObj expectedSerialization = fromjson(
+        "{$and: [{$expr: {$and: [{$eq: ['$a', {$const: 1}]}, {$eq: ['$b', {$const: 2}]}]}},"
+        "{a: {$_internalExprEq: 1}}, {b: {$_internalExprEq: 2}}]}");
+    ASSERT_BSONOBJ_EQ(serialized, expectedSerialization);
+}
+
 }  // namespace
 }  // namespace mongo
diff --git a/src/mongo/db/query/query_planner_test.cpp b/src/mongo/db/query/query_planner_test.cpp
index cdb09a63d32..8acccd4da85 100644
--- a/src/mongo/db/query/query_planner_test.cpp
+++ b/src/mongo/db/query/query_planner_test.cpp
@@ -5363,6 +5363,27 @@ TEST_F(QueryPlannerTest, ContainedOrPathLevelMultikeyCannotCompoundTrailingField
     assertSolutionExists("{cscan: {dir: 1}}}}");
 }
 
+TEST_F(QueryPlannerTest, ContainedOrPushdownIndexedExpr) {
+    addIndex(BSON("a" << 1 << "b" << 1));
+
+    runQuery(
+        fromjson("{$expr: {$and: [{$eq: ['$d', 'd']}, {$eq: ['$a', 'a']}]},"
+                 "$or: [{b: 'b'}, {b: 'c'}]}"));
+    assertNumSolutions(3);
+    // When we have path-level multikey info, we ensure that predicates are assigned in order of
+    // index position.
+    assertSolutionExists(
+        "{fetch: {node: {or: {nodes: ["
+        "{ixscan: {pattern: {a: 1, b: 1}, filter: null, bounds: {a: [['a', 'a', true, true]], b: "
+        "[['b', 'b', true, true]]}}},"
+        "{ixscan: {pattern: {a: 1, b: 1}, filter: null, bounds: {a: [['a', 'a', true, true]], b: "
+        "[['c', 'c', true, true]]}}}]}}}}");
+    assertSolutionExists(
+        "{fetch: {node: {ixscan: {pattern: {a: 1, b: 1}, filter: null,"
+        "bounds: {a: [['a', 'a', true, true]], b: [['MinKey', 'MaxKey', true, true]]}}}}}");
+    assertSolutionExists("{cscan: {dir: 1}}}}");
+}
+
 TEST_F(QueryPlannerTest, EmptyQueryWithoutProjectionUsesCollscan) {
     addIndex(BSON("a" << 1));
     runQuery(BSONObj());