SERVER-83119 Qualify a ClusteredIndexscan as an allowed operation when notablescan is set (#18507)r6.0.14-rc0

GitOrigin-RevId: f156446935a1f21a001a975ff1ece88dd50a80ae

9 files changed, 186 insertions, 41 deletions

diff --git a/etc/backports_required_for_multiversion_tests.yml b/etc/backports_required_for_multiversion_tests.yml
index c5beabaad03..cbb28d8de36 100644
--- a/etc/backports_required_for_multiversion_tests.yml
+++ b/etc/backports_required_for_multiversion_tests.yml
@@ -315,6 +315,10 @@ last-continuous:
     ticket: SERVER-84013
   - test_file: jstests/sharding/multi_collection_transaction_placement_conflict_workaround.js
     ticket: SERVER-82353
+  - test_file: jstests/core/notablescan.js
+    ticket: SERVER-83119
+  - test_file: jstests/sharding/clustered_coll_scan.js
+    ticket: SERVER-83119
   suites: null
 last-lts:
   all:
@@ -676,4 +680,8 @@ last-lts:
     ticket: SERVER-84013
   - test_file: jstests/sharding/multi_collection_transaction_placement_conflict_workaround.js
     ticket: SERVER-82353
+  - test_file: jstests/core/notablescan.js
+    ticket: SERVER-83119
+  - test_file: jstests/sharding/clustered_coll_scan.js
+    ticket: SERVER-83119
   suites: null
diff --git a/jstests/core/notablescan.js b/jstests/core/notablescan.js
index baef5d56ae6..e99ecd013e8 100644
--- a/jstests/core/notablescan.js
+++ b/jstests/core/notablescan.js
@@ -1,6 +1,8 @@
 // check notablescan mode
 //
 // @tags: [
+//   # The test runs commands that are not allowed with security token: setParameter.
+//   not_allowed_with_security_token,
 //   assumes_against_mongod_not_mongos,
 //   # This test attempts to perform read operations after having enabled the notablescan server
 //   # parameter. The former operations may be routed to a secondary in the replica set, whereas the
@@ -14,48 +16,71 @@
 //   tenant_migration_incompatible,
 // ]
 
-t = db.test_notablescan;
-t.drop();
+(function() {
+load("jstests/libs/analyze_plan.js");
+load("jstests/libs/collection_drop_recreate.js");
 
-try {
-    assert.commandWorked(db._adminCommand({setParameter: 1, notablescan: true}));
-    // commented lines are SERVER-2222
-    if (0) {  // SERVER-2222
-        assert.throws(function() {
-            t.find({a: 1}).toArray();
-        });
-    }
-    t.save({a: 1});
-    assert.throws(function() {
-        t.count({a: 1});
-    });
+function checkError(err) {
+    assert.includes(err.toString(), "'notablescan'");
+}
+
+const colName = jsTestName();
+let coll = db.getCollection(colName);
+coll.drop();
+
+assert.commandWorked(db.adminCommand({setParameter: 1, notablescan: true}));
+
+{
     if (0) {
+        // TODO: SERVER-2222 This should actually throw an error as it performs a collection
+        // scan.
         assert.throws(function() {
-            t.find({}).toArray();
+            coll.find({a: 1}).toArray();
         });
     }
-    assert.eq(1, t.find({}).itcount());  // SERVER-274
 
+    coll.insert({a: 1});
     let err = assert.throws(function() {
-        t.find({a: 1}).toArray();
+        coll.count({a: 1});
     });
-    assert.includes(err.toString(), "No indexed plans available, and running with 'notablescan'");
+    checkError(err);
+
+    // TODO: SERVER-2222 This should actually throw an error as it performs a collection scan.
+    assert.eq(1, coll.find({}).itcount());
+
+    err = assert.throws(function() {
+        coll.find({a: 1}).toArray();
+    });
+    checkError(err);
 
     err = assert.throws(function() {
-        t.find({a: 1}).hint({$natural: 1}).toArray();
+        coll.find({a: 1}).hint({$natural: 1}).toArray();
     });
-    assert.includes(err.toString(),
-                    "hint $natural is not allowed, because 'notablescan' is enabled");
-
-    t.createIndex({a: 1});
-    assert.eq(0, t.find({a: 1, b: 1}).itcount());
-    assert.eq(1, t.find({a: 1, b: null}).itcount());
-
-    // SERVER-4327
-    assert.eq(0, t.find({a: {$in: []}}).itcount());
-    assert.eq(0, t.find({a: {$in: []}, b: 0}).itcount());
-} finally {
-    // We assume notablescan was false before this test started and restore that
-    // expected value.
-    assert.commandWorked(db._adminCommand({setParameter: 1, notablescan: false}));
+    assert.includes(err.toString(), "$natural");
+    checkError(err);
+
+    coll.createIndex({a: 1});
+    assert.eq(0, coll.find({a: 1, b: 1}).itcount());
+    assert.eq(1, coll.find({a: 1, b: null}).itcount());
+}
+
+{  // Run the testcase with a clustered index.
+    assertDropAndRecreateCollection(db, colName, {clusteredIndex: {key: {_id: 1}, unique: true}});
+    coll = db.getCollection(colName);
+    assert.commandWorked(coll.insert({_id: 22}));
+    assert.eq(1, coll.find({_id: 22}).itcount());
+    let plan = coll.find({_id: 22}).explain();
+    // Make sure the plan has a clustered index scan.
+    assert(isClusteredIxscan(db, plan));
+
+    // Make sure the same works with an aggregate.
+    assert.eq(1, coll.aggregate([{$match: {_id: 22}}]).itcount());
+    plan = coll.explain().aggregate([{$match: {_id: 22}}]);
+    // Make sure the plan has a clustered index scan.
+    assert(isClusteredIxscan(db, plan));
+    assert.commandWorked(
+        db.runCommand({aggregate: colName, pipeline: [{$match: {_id: 22}}], cursor: {}}));
 }
+// Set it back to the original value.
+assert.commandWorked(db.adminCommand({setParameter: 1, notablescan: false}));
+})();
diff --git a/jstests/sharding/clustered_coll_scan.js b/jstests/sharding/clustered_coll_scan.js
new file mode 100644
index 00000000000..b96e50b065d
--- /dev/null
+++ b/jstests/sharding/clustered_coll_scan.js
@@ -0,0 +1,46 @@
+/*
+ * Testing if mongos can deal with clustered collections and the related clustered IDX scan bounds
+ * (SERVER-83119)
+ */
+(function() {
+load("jstests/libs/analyze_plan.js");
+load("jstests/libs/collection_drop_recreate.js");
+
+const st = new ShardingTest({
+    shards: 2,
+    mongos: 1,
+});
+
+st.s.adminCommand({enableSharding: "test"});
+
+const db = st.getDB("test");
+// Create the collection as a clustered collection.
+const coll = assertDropAndRecreateCollection(
+    db, jsTestName(), {clusteredIndex: {key: {_id: 1}, unique: true}});
+st.shardColl(coll, {a: 1});
+// First of all check that we can execute the query.
+assert.commandWorked(coll.insertMany([...Array(10).keys()].map(i => {
+    return {_id: i, a: i};
+})));
+
+{
+    var explain = coll.find({_id: 2}).explain();
+    // Make sure that we have a clusteredIDXScan in the plan.
+
+    assert(isClusteredIxscan(db, explain));
+    assert.commandWorked(
+        st.getPrimaryShard("test").adminCommand({setParameter: 1, notablescan: 1}));
+    // Do the same thing only with notablescan enabled.
+    explain = coll.find({_id: 2}).explain();
+    assert(isClusteredIxscan(db, explain));
+    // Sanity count check.
+    assert.eq(1, coll.find({_id: 2}).itcount());
+}
+// Test the same with aggregate.
+{
+    var explain = coll.explain().aggregate([{$match: {_id: 22}}]);
+    assert(isClusteredIxscan(db, explain));
+}
+
+st.stop();
+})();
diff --git a/src/mongo/db/query/planner_access.cpp b/src/mongo/db/query/planner_access.cpp
index ef0956dc56f..2e8e77c44b5 100644
--- a/src/mongo/db/query/planner_access.cpp
+++ b/src/mongo/db/query/planner_access.cpp
@@ -430,6 +430,9 @@ void handleRIDRangeMinMax(const CanonicalQuery& query,
 
 std::unique_ptr<QuerySolutionNode> QueryPlannerAccess::makeCollectionScan(
     const CanonicalQuery& query, bool tailable, const QueryPlannerParams& params, int direction) {
+    // The following are expensive to look up, so only do it once for each.
+    const mongo::NamespaceString nss = query.nss();
+    const bool isOplog = nss.isOplog();
     // Make the (only) node, a collection scan.
     auto csn = std::make_unique<CollectionScanNode>();
     csn->name = query.ns();
@@ -440,6 +443,7 @@ std::unique_ptr<QuerySolutionNode> QueryPlannerAccess::makeCollectionScan(
     csn->shouldWaitForOplogVisibility =
         params.options & QueryPlannerParams::OPLOG_SCAN_WAIT_FOR_VISIBLE;
     csn->direction = direction;
+    csn->isOplog = isOplog;
 
     if (params.clusteredInfo) {
         csn->clusteredIndex = params.clusteredInfo->getIndexSpec();
diff --git a/src/mongo/db/query/query_planner.cpp b/src/mongo/db/query/query_planner.cpp
index 117db492183..32af6762a17 100644
--- a/src/mongo/db/query/query_planner.cpp
+++ b/src/mongo/db/query/query_planner.cpp
@@ -339,6 +339,9 @@ string optionString(size_t options) {
             case QueryPlannerParams::RETURN_OWNED_DATA:
                 ss << "RETURN_OWNED_DATA ";
                 break;
+            case QueryPlannerParams::STRICT_NO_TABLE_SCAN:
+                ss << "STRICT_NO_TABLE_SCAN ";
+                break;
             case QueryPlannerParams::DEFAULT:
                 MONGO_UNREACHABLE;
                 break;
@@ -782,6 +785,38 @@ StatusWith<std::unique_ptr<QuerySolution>> QueryPlanner::planFromCache(
     return {std::move(soln)};
 }
 
+// If no table scan option is set the planner may not return any plan containing a collection scan.
+// Yet clusteredIdxScans are still allowed as they are not a full collection scan but a bounded
+// collection scan.
+bool noTableScan(const QueryPlannerParams& params) {
+    return (params.options & QueryPlannerParams::NO_TABLE_SCAN);
+}
+
+// Used internally if the planner should also avoid retruning a plan containing a clusteredIDX scan.
+bool noTableAndClusteredIDXScan(const QueryPlannerParams& params) {
+    return (params.options & QueryPlannerParams::STRICT_NO_TABLE_SCAN);
+}
+
+bool isClusteredScan(QuerySolutionNode* node) {
+    if (node->getType() == STAGE_COLLSCAN) {
+        auto collectionScanSolnNode = dynamic_cast<CollectionScanNode*>(node);
+        return (collectionScanSolnNode->doClusteredCollectionScan());
+    }
+    return false;
+}
+
+// Check if this is a real coll scan or a hidden ClusteredIDX scan.
+bool isColusteredIDXScanSoln(QuerySolution* collscanSoln) {
+    if (collscanSoln->root()->getType() == STAGE_SHARDING_FILTER) {
+        auto child = collscanSoln->root()->children.begin();
+        return isClusteredScan(*child);
+    }
+    if (collscanSoln->root()->getType() == STAGE_COLLSCAN) {
+        return isClusteredScan(collscanSoln->root());
+    }
+    return false;
+}
+
 StatusWith<std::vector<std::unique_ptr<QuerySolution>>> QueryPlanner::plan(
     const CanonicalQuery& query, const QueryPlannerParams& params) {
     // It's a little silly to ask for a count and for owned data. This could indicate a bug
@@ -835,7 +870,6 @@ StatusWith<std::vector<std::unique_ptr<QuerySolution>>> QueryPlanner::plan(
             // any $natural sort to have been normalized to a $natural hint upstream.
             // Additionally, if the hint matches the collection's cluster key, we also output a
             // collscan utilizing the cluster key.
-
             if (naturalHint) {
                 // Perform validation specific to $natural.
                 LOGV2_DEBUG(20969, 5, "Forcing a table scan due to hinted $natural");
@@ -895,7 +929,7 @@ StatusWith<std::vector<std::unique_ptr<QuerySolution>>> QueryPlanner::plan(
             out.push_back(std::move(soln));
             return {std::move(out)};
         }
-    }
+    }  // namespace mongo
 
     // Hints require us to only consider the hinted index. If index filters in the query
     // settings were used to override the allowed indices for planning, we should not use the
@@ -1349,7 +1383,7 @@ StatusWith<std::vector<std::unique_ptr<QuerySolution>>> QueryPlanner::plan(
 
     // No indexed plans?  We must provide a collscan if possible or else we can't run the query.
     bool collScanRequired = 0 == out.size();
-    if (collScanRequired && !canTableScan) {
+    if (collScanRequired && noTableAndClusteredIDXScan(params)) {
         return Status(ErrorCodes::NoQueryExecutionPlans,
                       "No indexed plans available, and running with 'notablescan'");
     }
@@ -1365,6 +1399,7 @@ StatusWith<std::vector<std::unique_ptr<QuerySolution>>> QueryPlanner::plan(
         return Status(ErrorCodes::NoQueryExecutionPlans, "No query solutions");
     }
 
+    bool isClusteredIDXScan = false;
     if (possibleToCollscan && (collscanRequested || collScanRequired || clusteredCollection)) {
         auto clusteredScanDirection = determineClusteredScanDirection(query, params);
         auto direction = clusteredScanDirection.value_or(1);
@@ -1374,7 +1409,7 @@ StatusWith<std::vector<std::unique_ptr<QuerySolution>>> QueryPlanner::plan(
             return Status(ErrorCodes::NoQueryExecutionPlans,
                           "Failed to build collection scan soln");
         }
-
+        isClusteredIDXScan = isColusteredIDXScanSoln(collscanSoln.get());
         // We consider collection scan in the following cases:
         // 1. collScanRequested - specifically requested by caller.
         // 2. collScanRequired - there are no other possible plans, so we fallback to full scan.
@@ -1396,13 +1431,21 @@ StatusWith<std::vector<std::unique_ptr<QuerySolution>>> QueryPlanner::plan(
         }
     }
 
+    // Make sure to respect the notablescan option. A clustered IDX scan is allowed even under a
+    // NOTABLE option. Only in the case of a strict NOTABLE scan option a clustered IDX scan is not
+    // allowed. This option is used in mongoS for shardPruning.
     invariant(out.size() > 0);
+    if (collScanRequired && noTableScan(params) && !isClusteredIDXScan) {
+        return Status(ErrorCodes::NoQueryExecutionPlans,
+                      "No indexed plans available, and running with 'notablescan'");
+    }
     return {std::move(out)};
 }
 
 /**
- * The 'query' might contain parts of aggregation pipeline. For now, we plan those separately and
- * later attach the agg portion of the plan to the solution(s) for the "find" part of the query.
+ * The 'query' might contain parts of aggregation pipeline. For now, we plan those separately
+ * and later attach the agg portion of the plan to the solution(s) for the "find" part of the
+ * query.
  */
 std::unique_ptr<QuerySolution> QueryPlanner::extendWithAggPipeline(
     const CanonicalQuery& query,
diff --git a/src/mongo/db/query/query_planner_params.h b/src/mongo/db/query/query_planner_params.h
index c8542cda90e..ab07388710f 100644
--- a/src/mongo/db/query/query_planner_params.h
+++ b/src/mongo/db/query/query_planner_params.h
@@ -153,6 +153,11 @@ struct QueryPlannerParams {
         // Ensure that any plan generated returns data that is "owned." That is, all BSONObjs are
         // in an "owned" state and are not pointing to data that belongs to the storage engine.
         RETURN_OWNED_DATA = 1 << 12,
+
+        // This is an extension to the NO_TABLE_SCAN parameter. This more stricter option will also
+        // avoid a CLUSTEREDIDX_SCAN which comes built into a collection scan when the collection is
+        // clustered.
+        STRICT_NO_TABLE_SCAN = 1 << 13,
     };
 
     // See Options enum above.
diff --git a/src/mongo/db/query/query_solution.cpp b/src/mongo/db/query/query_solution.cpp
index 7b24c1c08a3..669777b61ae 100644
--- a/src/mongo/db/query/query_solution.cpp
+++ b/src/mongo/db/query/query_solution.cpp
@@ -316,7 +316,11 @@ void CollectionScanNode::computeProperties() {
 
 void CollectionScanNode::appendToString(str::stream* ss, int indent) const {
     addIndent(ss, indent);
-    *ss << "COLLSCAN\n";
+    if (doClusteredCollectionScan()) {
+        *ss << "CLUSTERED_IDXSCAN\n";
+    } else {
+        *ss << "COLLSCAN\n";
+    }
     addIndent(ss, indent + 1);
     *ss << "ns = " << name << '\n';
     if (nullptr != filter) {
@@ -333,6 +337,9 @@ QuerySolutionNode* CollectionScanNode::clone() const {
     copy->name = this->name;
     copy->tailable = this->tailable;
     copy->direction = this->direction;
+    copy->minRecord = this->minRecord;
+    copy->maxRecord = this->maxRecord;
+    copy->clusteredIndex = this->clusteredIndex;
     copy->shouldTrackLatestOplogTimestamp = this->shouldTrackLatestOplogTimestamp;
     copy->assertTsHasNotFallenOffOplog = this->assertTsHasNotFallenOffOplog;
     copy->shouldWaitForOplogVisibility = this->shouldWaitForOplogVisibility;
diff --git a/src/mongo/db/query/query_solution.h b/src/mongo/db/query/query_solution.h
index 9a50b62c71b..8b04e9f0bf3 100644
--- a/src/mongo/db/query/query_solution.h
+++ b/src/mongo/db/query/query_solution.h
@@ -463,6 +463,10 @@ struct CollectionScanNode : public QuerySolutionNodeWithSortSet {
 
     QuerySolutionNode* clone() const;
 
+    bool doClusteredCollectionScan() const {
+        return (!isOplog && (minRecord || maxRecord));
+    }
+
     // Name of the namespace.
     std::string name;
 
@@ -502,6 +506,9 @@ struct CollectionScanNode : public QuerySolutionNodeWithSortSet {
 
     int direction{1};
 
+    // Tells whether the collection is an oplog.
+    bool isOplog = false;
+
     // By default, includes the minRecord and maxRecord when present.
     CollectionScanParams::ScanBoundInclusion boundInclusion =
         CollectionScanParams::ScanBoundInclusion::kIncludeBothStartAndEndRecords;
diff --git a/src/mongo/s/chunk_manager.cpp b/src/mongo/s/chunk_manager.cpp
index 5d4dbab248d..1d3ed85e925 100644
--- a/src/mongo/s/chunk_manager.cpp
+++ b/src/mongo/s/chunk_manager.cpp
@@ -922,7 +922,7 @@ IndexBounds ChunkManager::getIndexBoundsForQuery(const BSONObj& key,
     // Use query framework to generate index bounds
     QueryPlannerParams plannerParams;
     // Must use "shard key" index
-    plannerParams.options = QueryPlannerParams::NO_TABLE_SCAN;
+    plannerParams.options = QueryPlannerParams::STRICT_NO_TABLE_SCAN;
     IndexEntry indexEntry(key,
                           indexType,
                           IndexDescriptor::kLatestIndexVersion,