SERVER-20536 Ignore comment field when planning counts.

3 files changed, 50 insertions, 26 deletions

diff --git a/src/mongo/db/exec/count.cpp b/src/mongo/db/exec/count.cpp
index c71d87cdba6..b1a15f756e6 100644
--- a/src/mongo/db/exec/count.cpp
+++ b/src/mongo/db/exec/count.cpp
@@ -46,42 +46,42 @@ const char* CountStage::kStageType = "COUNT";
 
 CountStage::CountStage(OperationContext* txn,
                        Collection* collection,
-                       const CountRequest& request,
+                       CountStageParams params,
                        WorkingSet* ws,
                        PlanStage* child)
     : PlanStage(kStageType, txn),
       _collection(collection),
-      _request(request),
-      _leftToSkip(request.getSkip()),
+      _params(std::move(params)),
+      _leftToSkip(_params.skip),
       _ws(ws) {
     if (child)
         _children.emplace_back(child);
 }
 
 bool CountStage::isEOF() {
-    if (_specificStats.trivialCount) {
+    if (_specificStats.recordStoreCount) {
         return true;
     }
 
-    if (_request.getLimit() > 0 && _specificStats.nCounted >= _request.getLimit()) {
+    if (_params.limit > 0 && _specificStats.nCounted >= _params.limit) {
         return true;
     }
 
     return !_children.empty() && child()->isEOF();
 }
 
-void CountStage::trivialCount() {
+void CountStage::recordStoreCount() {
     invariant(_collection);
     long long nCounted = _collection->numRecords(getOpCtx());
 
-    if (0 != _request.getSkip()) {
-        nCounted -= _request.getSkip();
+    if (0 != _params.skip) {
+        nCounted -= _params.skip;
         if (nCounted < 0) {
             nCounted = 0;
         }
     }
 
-    long long limit = _request.getLimit();
+    long long limit = _params.limit;
     if (limit < 0) {
         limit = -limit;
     }
@@ -91,18 +91,17 @@ void CountStage::trivialCount() {
     }
 
     _specificStats.nCounted = nCounted;
-    _specificStats.nSkipped = _request.getSkip();
-    _specificStats.trivialCount = true;
+    _specificStats.nSkipped = _params.skip;
+    _specificStats.recordStoreCount = true;
 }
 
 PlanStage::StageState CountStage::doWork(WorkingSetID* out) {
     // This stage never returns a working set member.
     *out = WorkingSet::INVALID_ID;
 
-    // If we don't have a query and we have a non-NULL collection, then we can execute this
-    // as a trivial count (just ask the collection for how many records it has).
-    if (_request.getQuery().isEmpty() && NULL != _collection) {
-        trivialCount();
+    if (_params.useRecordStoreCount) {
+        invariant(_collection);
+        recordStoreCount();
         return PlanStage::IS_EOF;
     }
 
@@ -111,8 +110,8 @@ PlanStage::StageState CountStage::doWork(WorkingSetID* out) {
         return PlanStage::IS_EOF;
     }
 
-    // For non-trivial counts, we should always have a child stage from which we can retrieve
-    // results.
+    // For cases where we can't ask the record store directly, we should always have a child stage
+    // from which we can retrieve results.
     invariant(child());
     WorkingSetID id = WorkingSet::INVALID_ID;
     PlanStage::StageState state = child()->work(&id);
diff --git a/src/mongo/db/exec/count.h b/src/mongo/db/exec/count.h
index e083eb59aff..56cdbe6614c 100644
--- a/src/mongo/db/exec/count.h
+++ b/src/mongo/db/exec/count.h
@@ -34,6 +34,30 @@
 
 namespace mongo {
 
+struct CountStageParams {
+    CountStageParams(const CountRequest& request, bool useRecordStoreCount)
+        : nss(request.getNs()),
+          limit(request.getLimit()),
+          skip(request.getSkip()),
+          useRecordStoreCount(useRecordStoreCount) {}
+
+    // Namespace to operate on (e.g. "foo.bar").
+    NamespaceString nss;
+
+    // An integer limiting the number of documents to count. 0 means no limit.
+    long long limit;
+
+    // An integer indicating to not include the first n documents in the count. 0 means no skip.
+    long long skip;
+
+    // True if this count stage should just ask the record store for a count instead of computing
+    // one itself.
+    //
+    // Note: This strategy can lead to inaccurate counts on certain storage engines (including
+    // WiredTiger).
+    bool useRecordStoreCount;
+};
+
 /**
  * Stage used by the count command. This stage sits at the root of a plan tree
  * and counts the number of results returned by its child stage.
@@ -49,7 +73,7 @@ class CountStage final : public PlanStage {
 public:
     CountStage(OperationContext* txn,
                Collection* collection,
-               const CountRequest& request,
+               CountStageParams params,
                WorkingSet* ws,
                PlanStage* child);
 
@@ -68,15 +92,17 @@ public:
 
 private:
     /**
-     * Computes the count in the case of an empty query, applying the skip and
-     * limit if necessary. The result is stored in '_specificStats'.
+     * Asks the record store for the count, applying the skip and limit if necessary. The result is
+     * stored in '_specificStats'.
+     *
+     * This is only valid if the query and hint are both empty.
      */
-    void trivialCount();
+    void recordStoreCount();
 
     // The collection over which we are counting.
     Collection* _collection;
 
-    CountRequest _request;
+    CountStageParams _params;
 
     // The number of documents that we still need to skip.
     long long _leftToSkip;
diff --git a/src/mongo/db/exec/plan_stats.h b/src/mongo/db/exec/plan_stats.h
index 01af3cd0198..e7cfbf03f6f 100644
--- a/src/mongo/db/exec/plan_stats.h
+++ b/src/mongo/db/exec/plan_stats.h
@@ -209,7 +209,7 @@ struct CollectionScanStats : public SpecificStats {
 };
 
 struct CountStats : public SpecificStats {
-    CountStats() : nCounted(0), nSkipped(0), trivialCount(false) {}
+    CountStats() : nCounted(0), nSkipped(0), recordStoreCount(false) {}
 
     SpecificStats* clone() const final {
         CountStats* specific = new CountStats(*this);
@@ -222,9 +222,8 @@ struct CountStats : public SpecificStats {
     // The number of results we skipped over.
     long long nSkipped;
 
-    // A "trivial count" is one that we can answer by calling numRecords() on the
-    // collection, without actually going through any query logic.
-    bool trivialCount;
+    // True if we computed the count via Collection::numRecords().
+    bool recordStoreCount;
 };
 
 struct CountScanStats : public SpecificStats {