SERVER-19388 fix assertion in $or planning

3 files changed, 61 insertions, 15 deletions

diff --git a/src/mongo/db/query/canonical_query.cpp b/src/mongo/db/query/canonical_query.cpp
index 26bbd2946ce..aa5f9762920 100644
--- a/src/mongo/db/query/canonical_query.cpp
+++ b/src/mongo/db/query/canonical_query.cpp
@@ -235,19 +235,14 @@ StatusWith<std::unique_ptr<CanonicalQuery>> CanonicalQuery::canonicalize(
     const CanonicalQuery& baseQuery,
     MatchExpression* root,
     const MatchExpressionParser::WhereCallback& whereCallback) {
-    // Obtain filter for our LPQ by serializing the new match expression root.
-    BSONObjBuilder bob;
-    root->toBSON(&bob);
-    BSONObj filter = bob.obj();
-
-    // Pass empty sort and projection.
+    // TODO: we should be passing the filter corresponding to 'root' to the LPQ rather than the base
+    // query's filter, baseQuery.getParsed().getFilter().
     BSONObj emptyObj;
-
     auto lpqStatus = LiteParsedQuery::makeAsOpQuery(baseQuery.nss(),
                                                     0,  // ntoskip
                                                     0,  // ntoreturn
                                                     0,  // queryOptions
-                                                    filter,
+                                                    baseQuery.getParsed().getFilter(),
                                                     baseQuery.getParsed().getProj(),
                                                     baseQuery.getParsed().getSort(),
                                                     emptyObj,  // hint
diff --git a/src/mongo/db/query/canonical_query_test.cpp b/src/mongo/db/query/canonical_query_test.cpp
index 7d90620e818..022210fb8a6 100644
--- a/src/mongo/db/query/canonical_query_test.cpp
+++ b/src/mongo/db/query/canonical_query_test.cpp
@@ -519,11 +519,10 @@ TEST(CanonicalQueryTest, CanonicalizeFromBaseQuery) {
     MatchExpression* firstClauseExpr = baseCq->root()->getChild(0);
     auto childCq = assertGet(CanonicalQuery::canonicalize(*baseCq, firstClauseExpr));
 
-    BSONObjBuilder expectedFilterBuilder;
-    firstClauseExpr->toBSON(&expectedFilterBuilder);
-    BSONObj expectedFilter = expectedFilterBuilder.obj();
+    // Descriptive test. The childCq's filter should be the relevant $or clause, rather than the
+    // entire query predicate.
+    ASSERT_EQ(childCq->getParsed().getFilter(), baseCq->getParsed().getFilter());
 
-    ASSERT_EQ(childCq->getParsed().getFilter(), expectedFilter);
     ASSERT_EQ(childCq->getParsed().getProj(), baseCq->getParsed().getProj());
     ASSERT_EQ(childCq->getParsed().getSort(), baseCq->getParsed().getSort());
     ASSERT_TRUE(childCq->getParsed().isExplain());
diff --git a/src/mongo/dbtests/query_stage_subplan.cpp b/src/mongo/dbtests/query_stage_subplan.cpp
index ffe592eba90..cde78293033 100644
--- a/src/mongo/dbtests/query_stage_subplan.cpp
+++ b/src/mongo/dbtests/query_stage_subplan.cpp
@@ -377,7 +377,7 @@ public:
 /**
  * Test the subplan stage's ability to answer a contained $or query.
  */
-class QueryStageSubplanPlanRootedOr : public QueryStageSubplanBase {
+class QueryStageSubplanPlanContainedOr : public QueryStageSubplanBase {
 public:
     void run() {
         OldClientWriteContext ctx(&_txn, ns());
@@ -405,7 +405,7 @@ public:
             new SubplanStage(&_txn, collection, &ws, plannerParams, cq.get()));
 
         // Plan selection should succeed due to falling back on regular planning.
-        PlanYieldPolicy yieldPolicy(NULL, PlanExecutor::YIELD_MANUAL);
+        PlanYieldPolicy yieldPolicy(nullptr, PlanExecutor::YIELD_MANUAL);
         ASSERT_OK(subplan->pickBestPlan(&yieldPolicy));
 
         // Work the stage until it produces all results.
@@ -430,6 +430,57 @@ public:
     }
 };
 
+/**
+ * Test the subplan stage's ability to answer a rooted $or query with a $ne and a sort.
+ *
+ * Regression test for SERVER-19388.
+ */
+class QueryStageSubplanPlanRootedOrNE : public QueryStageSubplanBase {
+public:
+    void run() {
+        OldClientWriteContext ctx(&_txn, ns());
+        addIndex(BSON("a" << 1 << "b" << 1));
+        addIndex(BSON("a" << 1 << "c" << 1));
+
+        // Every doc matches.
+        insert(BSON("_id" << 1 << "a" << 1));
+        insert(BSON("_id" << 2 << "a" << 2));
+        insert(BSON("_id" << 3 << "a" << 3));
+        insert(BSON("_id" << 4));
+
+        BSONObj query = fromjson("{$or: [{a: 1}, {a: {$ne:1}}]}");
+        BSONObj sort = BSON("d" << 1);
+        BSONObj projection;
+        auto cq = unittest::assertGet(CanonicalQuery::canonicalize(ns(), query, sort, projection));
+
+        Collection* collection = ctx.getCollection();
+
+        QueryPlannerParams plannerParams;
+        fillOutPlannerParams(&_txn, collection, cq.get(), &plannerParams);
+
+        WorkingSet ws;
+        std::unique_ptr<SubplanStage> subplan(
+            new SubplanStage(&_txn, collection, &ws, plannerParams, cq.get()));
+
+        PlanYieldPolicy yieldPolicy(nullptr, PlanExecutor::YIELD_MANUAL);
+        ASSERT_OK(subplan->pickBestPlan(&yieldPolicy));
+
+        size_t numResults = 0;
+        PlanStage::StageState stageState = PlanStage::NEED_TIME;
+        while (stageState != PlanStage::IS_EOF) {
+            WorkingSetID id = WorkingSet::INVALID_ID;
+            stageState = subplan->work(&id);
+            ASSERT_NE(stageState, PlanStage::DEAD);
+            ASSERT_NE(stageState, PlanStage::FAILURE);
+            if (stageState == PlanStage::ADVANCED) {
+                ++numResults;
+            }
+        }
+
+        ASSERT_EQ(numResults, 4U);
+    }
+};
+
 class All : public Suite {
 public:
     All() : Suite("query_stage_subplan") {}
@@ -439,7 +490,8 @@ public:
         add<QueryStageSubplanPlanFromCache>();
         add<QueryStageSubplanCanUseSubplanning>();
         add<QueryStageSubplanRewriteToRootedOr>();
-        add<QueryStageSubplanPlanRootedOr>();
+        add<QueryStageSubplanPlanContainedOr>();
+        add<QueryStageSubplanPlanRootedOrNE>();
     }
 };