SERVER-19593 snapshot default to collscan except when using MMAPv1

jstests/core/explain_shell_helpers.js

@@ -175,10 +175,8 @@ assert.commandWorked(explain);
 // .snapshot()
 explain = t.explain().find().snapshot().finish();
 assert.commandWorked(explain);
-assert(isIxscan(explain.queryPlanner.winningPlan));
 explain = t.find().snapshot().explain();
 assert.commandWorked(explain);
-assert(isIxscan(explain.queryPlanner.winningPlan));
 
 // .next()
 explain = t.explain().find().next();

jstests/mmap_v1/explain_snapshot.js

@@ -0,0 +1,23 @@
+/**
+ * This test ensures that .snapshot() performs an index scan on mmap_v1.
+ */
+(function() {
+    'use strict';
+
+    load("jstests/libs/analyze_plan.js");
+
+    var coll = db.jstests_explain_snapshot;
+    coll.drop();
+
+    assert.writeOK(coll.insert({}));
+
+    var explain = coll.explain().find().snapshot().finish();
+    assert.commandWorked(explain);
+    assert(isIxscan(explain.queryPlanner.winningPlan));
+
+    explain = coll.find().snapshot().explain();
+    assert.commandWorked(explain);
+    assert(isIxscan(explain.queryPlanner.winningPlan));
+
+    coll.drop();
+})();

src/mongo/db/query/get_executor.cpp

@@ -197,6 +197,13 @@ void fillOutPlannerParams(OperationContext* txn,
     } else {
         plannerParams->options |= QueryPlannerParams::KEEP_MUTATIONS;
     }
+
+    // MMAPv1 storage engine should have snapshot() perform an index scan on _id rather than a
+    // collection scan since a collection scan on the MMAP storage engine can return duplicates
+    // or miss documents.
+    if (isMMAPV1()) {
+        plannerParams->options |= QueryPlannerParams::SNAPSHOT_USE_ID;
+    }
 }
 
 namespace {
@@ -1374,8 +1381,8 @@ StatusWith<unique_ptr<PlanExecutor>> getExecutorDistinct(OperationContext* txn,
                                                      BSONObj(),  // hint
                                                      BSONObj(),  // min
                                                      BSONObj(),  // max
+                                                     false,      // snapshot
                                                      isExplain,
-                                                     false,  // snapshot
                                                      whereCallback);
     if (!statusWithCQ.isOK()) {
         return statusWithCQ.getStatus();

src/mongo/db/query/query_planner.cpp

@@ -436,15 +436,16 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
         LOG(5) << "Index " << i << " is " << params.indices[i].toString() << endl;
     }
 
-    bool canTableScan = !(params.options & QueryPlannerParams::NO_TABLE_SCAN);
+    const bool canTableScan = !(params.options & QueryPlannerParams::NO_TABLE_SCAN);
+    const bool isTailable = query.getParsed().isTailable();
 
     // If the query requests a tailable cursor, the only solution is a collscan + filter with
     // tailable set on the collscan.  TODO: This is a policy departure.  Previously I think you
     // could ask for a tailable cursor and it just tried to give you one.  Now, we fail if we
     // can't provide one.  Is this what we want?
-    if (query.getParsed().isTailable()) {
+    if (isTailable) {
         if (!QueryPlannerCommon::hasNode(query.root(), MatchExpression::GEO_NEAR) && canTableScan) {
-            QuerySolution* soln = buildCollscanSoln(query, true, params);
+            QuerySolution* soln = buildCollscanSoln(query, isTailable, params);
             if (NULL != soln) {
                 out->push_back(soln);
             }
@@ -468,7 +469,7 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
             // min/max are incompatible with $natural.
             if (canTableScan && query.getParsed().getMin().isEmpty() &&
                 query.getParsed().getMax().isEmpty()) {
-                QuerySolution* soln = buildCollscanSoln(query, false, params);
+                QuerySolution* soln = buildCollscanSoln(query, isTailable, params);
                 if (NULL != soln) {
                     out->push_back(soln);
                 }
@@ -497,14 +498,30 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
         hintIndex = query.getParsed().getHint();
     }
 
-    // Snapshot is a form of a hint.  If snapshot is set, try to use _id index to make a real
-    // plan.  If that fails, just scan the _id index.
-    if (query.getParsed().isSnapshot()) {
-        // Find the ID index in indexKeyPatterns.  It's our hint.
-        for (size_t i = 0; i < params.indices.size(); ++i) {
-            if (isIdIndex(params.indices[i].keyPattern)) {
-                hintIndex = params.indices[i].keyPattern;
-                break;
+    // If snapshot is set, default to collscanning. If the query param SNAPSHOT_USE_ID is set,
+    // snapshot is a form of a hint, so try to use _id index to make a real plan. If that fails,
+    // just scan the _id index.
+    //
+    // Don't do this if the query is a geonear or text as as text search queries must be answered
+    // using full text indices and geoNear queries must be answered using geospatial indices.
+    if (query.getParsed().isSnapshot() &&
+        !QueryPlannerCommon::hasNode(query.root(), MatchExpression::GEO_NEAR) &&
+        !QueryPlannerCommon::hasNode(query.root(), MatchExpression::TEXT)) {
+        const bool useIXScan = params.options & QueryPlannerParams::SNAPSHOT_USE_ID;
+
+        if (!useIXScan) {
+            QuerySolution* soln = buildCollscanSoln(query, isTailable, params);
+            if (soln) {
+                out->push_back(soln);
+            }
+            return Status::OK();
+        } else {
+            // Find the ID index in indexKeyPatterns. It's our hint.
+            for (size_t i = 0; i < params.indices.size(); ++i) {
+                if (isIdIndex(params.indices[i].keyPattern)) {
+                    hintIndex = params.indices[i].keyPattern;
+                    break;
+                }
             }
         }
     }
@@ -893,7 +910,7 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
     bool collscanNeeded = (0 == out->size() && canTableScan);
 
     if (possibleToCollscan && (collscanRequested || collscanNeeded)) {
-        QuerySolution* collscan = buildCollscanSoln(query, false, params);
+        QuerySolution* collscan = buildCollscanSoln(query, isTailable, params);
         if (NULL != collscan) {
             SolutionCacheData* scd = new SolutionCacheData();
             scd->solnType = SolutionCacheData::COLLSCAN_SOLN;

src/mongo/db/query/query_planner_params.h

@@ -87,6 +87,11 @@ struct QueryPlannerParams {
         // Set this to prevent the planner from generating plans which answer a predicate
         // implicitly via exact index bounds for index intersection solutions.
         CANNOT_TRIM_IXISECT = 1 << 8,
+
+        // Set this if snapshot() should scan the _id index rather than performing a
+        // collection scan. The MMAPv1 storage engine sets this option since it cannot
+        // guarantee that a collection scan won't miss documents or return duplicates.
+        SNAPSHOT_USE_ID = 1 << 9,
     };
 
     // See Options enum above.

src/mongo/db/query/query_planner_test.cpp

@@ -1195,12 +1195,32 @@ TEST_F(QueryPlannerTest, Snapshot) {
     addIndex(BSON("a" << 1));
     runQuerySnapshot(fromjson("{a: {$gt: 0}}"));
 
+    assertNumSolutions(1U);
+    assertSolutionExists("{cscan: {filter: {a: {$gt: 0}}, dir: 1}}");
+}
+
+TEST_F(QueryPlannerTest, SnapshotUseId) {
+    params.options = QueryPlannerParams::SNAPSHOT_USE_ID;
+
+    addIndex(BSON("a" << 1));
+    runQuerySnapshot(fromjson("{a: {$gt: 0}}"));
+
     assertNumSolutions(1U);
     assertSolutionExists(
         "{fetch: {filter: {a:{$gt:0}}, node: "
         "{ixscan: {filter: null, pattern: {_id: 1}}}}}");
 }
 
+TEST_F(QueryPlannerTest, CannotSnapshotWithGeoNear) {
+    // Snapshot is skipped with geonear queries.
+    addIndex(BSON("a"
+                  << "2d"));
+    runQuerySnapshot(fromjson("{a: {$near: [0,0]}}"));
+
+    ASSERT_EQUALS(getNumSolutions(), 1U);
+    assertSolutionExists("{geoNear2d: {a: '2d'}}");
+}
+
 //
 // Tree operations that require simple tree rewriting.
 //