SERVER-82121: Ignore empty hints in CQF

Author: HanaPearlman

jstests/cqf/optimizer/index_hints.js

@@ -19,6 +19,15 @@ assert.commandWorked(t.insert({_id: 2, b: 2, a: [2]}));
 assert.commandWorked(t.insert({_id: 3, b: 3, a: 2}));
 assert.commandWorked(t.insert({_id: 4, b: 4, a: [1, 3]}));
 
+{
+    // Empty hint is ignored when there are no relevant indexes.
+    let res = t.explain("executionStats").find({a: 2}).hint({}).finish();
+    assertValueOnPlanPath("PhysicalScan", res, "child.child.nodeType");
+
+    res = t.explain("executionStats").aggregate([{$match: {a: 2}}], {hint: {}});
+    assertValueOnPlanPath("PhysicalScan", res, "child.child.nodeType");
+}
+
 assert.commandWorked(t.createIndex({a: 1}));
 assert.commandWorked(t.createIndex({b: 1}));
 
@@ -28,6 +37,15 @@ assert.commandWorked(t.createIndex({b: 1}));
     assertValueOnPlanPath("PhysicalScan", res, "child.child.nodeType");
 }
 
+{
+    // Empty hint is ignored when there are relevant indexes that are not preferable.
+    let res = t.explain("executionStats").find({a: 2}).hint({}).finish();
+    assertValueOnPlanPath("PhysicalScan", res, "child.child.nodeType");
+
+    res = t.explain("executionStats").aggregate([{$match: {a: 2}}], {hint: {}});
+    assertValueOnPlanPath("PhysicalScan", res, "child.child.nodeType");
+}
+
 {
     let res = t.explain("executionStats").find({a: 2}).hint({a: 1}).finish();
     assertValueOnPlanPath("IndexScan", res, "child.leftChild.nodeType");
@@ -64,6 +82,15 @@ for (let i = 0; i < 100; i++) {
     assertValueOnPlanPath("IndexScan", res, "child.leftChild.nodeType");
 }
 
+{
+    // Empty hint is ignored when there are relevant indexes that are preferable.
+    let res = t.explain("executionStats").find({a: 2}).hint({}).finish();
+    assertValueOnPlanPath("IndexScan", res, "child.leftChild.nodeType");
+
+    res = t.explain("executionStats").aggregate([{$match: {a: 2}}], {hint: {}});
+    assertValueOnPlanPath("IndexScan", res, "child.leftChild.nodeType");
+}
+
 {
     let res = t.explain("executionStats").find({a: 2}).hint({a: 1}).finish();
     assertValueOnPlanPath("IndexScan", res, "child.leftChild.nodeType");

src/mongo/db/query/cqf_get_executor.cpp

@@ -158,28 +158,28 @@ static std::pair<IndexDefinitions, MultikeynessTrie> buildIndexSpecsOptimizer(
 
     std::pair<IndexDefinitions, MultikeynessTrie> result;
     std::string indexHintName;
-    bool skipAllIndexes = false;
+
+    // True if the query has a $natural hint, indicating that we must use a collection scan.
+    bool hasNaturalHint = false;
+
     if (indexHint) {
         const BSONElement element = indexHint->firstElement();
         const StringData fieldName = element.fieldNameStringData();
-        if (fieldName == "$natural"_sd) {
+        if (fieldName == query_request_helper::kNaturalSortField) {
             // Do not add indexes.
-            skipAllIndexes = true;
+            hasNaturalHint = true;
         } else if (fieldName == "$hint"_sd && element.type() == BSONType::String) {
             indexHintName = element.valueStringData().toString();
         }
 
-        disableScan = !skipAllIndexes;
+        disableScan = !hasNaturalHint;
     }
 
     const IndexCatalog& indexCatalog = *collection->getIndexCatalog();
 
     auto indexIterator =
         indexCatalog.getIndexIterator(opCtx, IndexCatalog::InclusionPolicy::kReady);
 
-    const bool queryHasNaturalHint = indexHint && !indexHint->isEmpty() &&
-        indexHint->firstElementFieldNameStringData() == query_request_helper::kNaturalSortField;
-
     while (indexIterator->more()) {
         const IndexCatalogEntry& catalogEntry = *indexIterator->next();
         const IndexDescriptor& descriptor = *catalogEntry.descriptor();
@@ -196,32 +196,25 @@ static std::pair<IndexDefinitions, MultikeynessTrie> buildIndexSpecsOptimizer(
             continue;
         }
 
-        // If there is a $natural hint, we should not assert here as we will not use the index.
-        const bool isSpecialIndex =
-            descriptor.infoObj().hasField(IndexDescriptor::kExpireAfterSecondsFieldName) ||
+        // Check for special indexes. We do not want to try to build index metadata for a special
+        // index (since we do not support those yet in CQF) but we should allow the query to go
+        // through CQF if there is a $natural hint.
+        if (descriptor.infoObj().hasField(IndexDescriptor::kExpireAfterSecondsFieldName) ||
             descriptor.isSparse() || descriptor.getIndexType() != IndexType::INDEX_BTREE ||
-            !descriptor.collation().isEmpty();
-        if (!queryHasNaturalHint && isSpecialIndex) {
-            uasserted(ErrorCodes::InternalErrorNotSupported, "Unsupported index type");
-        }
-
-        // We do not want to try to build index metadata for a special index (since we do not
-        // support those yet in CQF) but we should allow the query to go through CQF if there is a
-        // $natural hint.
-        if (queryHasNaturalHint && isSpecialIndex) {
+            !descriptor.collation().isEmpty()) {
+            uassert(
+                ErrorCodes::InternalErrorNotSupported, "Unsupported index type", hasNaturalHint);
             continue;
         }
 
         if (indexHint) {
             if (indexHintName.empty()) {
-                if (!SimpleBSONObjComparator::kInstance.evaluate(descriptor.keyPattern() ==
-                                                                 *indexHint)) {
-                    // Index key pattern does not match hint.
-                    skipIndex = true;
-                }
-            } else if (indexHintName != descriptor.indexName()) {
-                // Index name does not match hint.
-                skipIndex = true;
+                // Index hint is a key pattern. Check if it matches this descriptor's key pattern.
+                skipIndex = SimpleBSONObjComparator::kInstance.evaluate(descriptor.keyPattern() !=
+                                                                        *indexHint);
+            } else {
+                // Index hint is an index name. Check if it matches the descriptor's name.
+                skipIndex = indexHintName != descriptor.indexName();
             }
         }
 
@@ -331,7 +324,7 @@ static std::pair<IndexDefinitions, MultikeynessTrie> buildIndexSpecsOptimizer(
             }
         }
         // For now we assume distribution is Centralized.
-        if (!skipIndex && !skipAllIndexes) {
+        if (!skipIndex && !hasNaturalHint) {
             result.first.emplace(descriptor.indexName(), std::move(indexDef));
         }
     }
@@ -864,7 +857,10 @@ boost::optional<ExecParams> getSBEExecutorViaCascadesOptimizer(
 
     const auto& collection = collections.getMainCollection();
 
-    validateCommandOptions(canonicalQuery, collection, indexHint, involvedCollections);
+    const boost::optional<BSONObj>& hint =
+        (indexHint && !indexHint->isEmpty() ? indexHint : boost::none);
+
+    validateCommandOptions(canonicalQuery, collection, hint, involvedCollections);
 
     const bool requireRID = canonicalQuery ? canonicalQuery->getForceGenerateRecordId() : false;
     const bool collectionExists = static_cast<bool>(collection);
@@ -882,7 +878,7 @@ boost::optional<ExecParams> getSBEExecutorViaCascadesOptimizer(
                                      collection,
                                      involvedCollections,
                                      nss,
-                                     indexHint,
+                                     hint,
                                      scanProjName,
                                      uuidStr,
                                      scanDefName,