SERVER-76895 Micro-optimizations for the hot find-by-indexed-key query path

3 files changed, 15 insertions, 62 deletions

diff --git a/src/mongo/db/query/canonical_query.cpp b/src/mongo/db/query/canonical_query.cpp
index 3a411c6925b..d632f9141ba 100644
--- a/src/mongo/db/query/canonical_query.cpp
+++ b/src/mongo/db/query/canonical_query.cpp
@@ -52,13 +52,6 @@
 namespace mongo {
 namespace {
 
-bool parsingCanProduceNoopMatchNodes(const ExtensionsCallback& extensionsCallback,
-                                     MatchExpressionParser::AllowedFeatureSet allowedFeatures) {
-    return extensionsCallback.hasNoopExtensions() &&
-        (allowedFeatures & MatchExpressionParser::AllowedFeatures::kText ||
-         allowedFeatures & MatchExpressionParser::AllowedFeatures::kJavascript);
-}
-
 boost::optional<size_t> loadMaxParameterCount() {
     auto value = internalQueryAutoParameterizationMaxParameterCount.load();
     if (value > 0) {
@@ -116,38 +109,25 @@ StatusWith<std::unique_ptr<CanonicalQuery>> CanonicalQuery::canonicalize(
     std::unique_ptr<CanonicalQuery> cq(new CanonicalQuery());
     cq->setExplain(explain);
 
-    StatusWithMatchExpression statusWithMatcher = [&]() -> StatusWithMatchExpression {
-        if (getTestCommandsEnabled() && internalQueryEnableCSTParser.load()) {
-            try {
-                return cst::parseToMatchExpression(
-                    findCommand->getFilter(), newExpCtx, extensionsCallback);
-            } catch (const DBException& ex) {
-                return ex.toStatus();
-            }
-        } else {
-            return MatchExpressionParser::parse(
-                findCommand->getFilter(), newExpCtx, extensionsCallback, allowedFeatures);
-        }
-    }();
+    auto statusWithMatcher = MatchExpressionParser::parse(
+        findCommand->getFilter(), newExpCtx, extensionsCallback, allowedFeatures);
     if (!statusWithMatcher.isOK()) {
         return statusWithMatcher.getStatus();
     }
 
+    std::unique_ptr<MatchExpression> me = std::move(statusWithMatcher.getValue());
+
     // Stop counting expressions after they have been parsed to exclude expressions created
     // during optimization and other processing steps.
     newExpCtx->stopExpressionCounters();
 
-    std::unique_ptr<MatchExpression> me = std::move(statusWithMatcher.getValue());
-
-    Status initStatus =
-        cq->init(opCtx,
-                 std::move(newExpCtx),
-                 std::move(findCommand),
-                 parsingCanProduceNoopMatchNodes(extensionsCallback, allowedFeatures),
-                 std::move(me),
-                 projectionPolicies,
-                 std::move(pipeline),
-                 isCountLike);
+    Status initStatus = cq->init(opCtx,
+                                 std::move(newExpCtx),
+                                 std::move(findCommand),
+                                 std::move(me),
+                                 projectionPolicies,
+                                 std::move(pipeline),
+                                 isCountLike);
 
     if (!initStatus.isOK()) {
         return initStatus;
@@ -174,7 +154,6 @@ StatusWith<std::unique_ptr<CanonicalQuery>> CanonicalQuery::canonicalize(
     Status initStatus = cq->init(opCtx,
                                  baseQuery.getExpCtx(),
                                  std::move(findCommand),
-                                 baseQuery.canHaveNoopMatchNodes(),
                                  root->clone(),
                                  ProjectionPolicies::findProjectionPolicies(),
                                  {} /* an empty pipeline */,
@@ -189,7 +168,6 @@ StatusWith<std::unique_ptr<CanonicalQuery>> CanonicalQuery::canonicalize(
 Status CanonicalQuery::init(OperationContext* opCtx,
                             boost::intrusive_ptr<ExpressionContext> expCtx,
                             std::unique_ptr<FindCommandRequest> findCommand,
-                            bool canHaveNoopMatchNodes,
                             std::unique_ptr<MatchExpression> root,
                             const ProjectionPolicies& projectionPolicies,
                             std::vector<std::unique_ptr<InnerPipelineStageInterface>> pipeline,
@@ -197,7 +175,6 @@ Status CanonicalQuery::init(OperationContext* opCtx,
     _expCtx = expCtx;
     _findCommand = std::move(findCommand);
 
-    _canHaveNoopMatchNodes = canHaveNoopMatchNodes;
     _forceClassicEngine = ServerParameterSet::getNodeParameterSet()
                               ->get<QueryFrameworkControl>("internalQueryFrameworkControl")
                               ->_data.get() == QueryFrameworkControlEnum::kForceClassicEngine;
@@ -262,7 +239,9 @@ Status CanonicalQuery::init(OperationContext* opCtx,
 
     // If there is a sort, parse it and add any metadata dependencies it induces.
     try {
-        initSortPattern(unavailableMetadata);
+        if (!_findCommand->getSort().isEmpty()) {
+            initSortPattern(unavailableMetadata);
+        }
     } catch (const DBException& ex) {
         return ex.toStatus();
     }
@@ -276,10 +255,6 @@ Status CanonicalQuery::init(OperationContext* opCtx,
 }
 
 void CanonicalQuery::initSortPattern(QueryMetadataBitSet unavailableMetadata) {
-    if (_findCommand->getSort().isEmpty()) {
-        return;
-    }
-
     // A $natural sort is really a hint, and should be handled as such. Furthermore, the downstream
     // sort handling code may not expect a $natural sort.
     //
@@ -291,11 +266,7 @@ void CanonicalQuery::initSortPattern(QueryMetadataBitSet unavailableMetadata) {
         _findCommand->setSort(BSONObj{});
     }
 
-    if (getTestCommandsEnabled() && internalQueryEnableCSTParser.load()) {
-        _sortPattern = cst::parseToSortPattern(_findCommand->getSort(), _expCtx);
-    } else {
-        _sortPattern = SortPattern{_findCommand->getSort(), _expCtx};
-    }
+    _sortPattern = SortPattern{_findCommand->getSort(), _expCtx};
     _metadataDeps |= _sortPattern->metadataDeps(unavailableMetadata);
 
     // If the results of this query might have to be merged on a remote node, then that node might
diff --git a/src/mongo/db/query/canonical_query.h b/src/mongo/db/query/canonical_query.h
index c59c1c6736f..f53b70562af 100644
--- a/src/mongo/db/query/canonical_query.h
+++ b/src/mongo/db/query/canonical_query.h
@@ -212,19 +212,6 @@ public:
      */
     static size_t countNodes(const MatchExpression* root, MatchExpression::MatchType type);
 
-    /**
-     * Returns true if this canonical query may have converted extensions such as $where and $text
-     * into no-ops during parsing. This will be the case if it allowed $where and $text in parsing,
-     * but parsed using an ExtensionsCallbackNoop. This does not guarantee that a $where or $text
-     * existed in the query.
-     *
-     * Queries with a no-op extension context are special because they can be parsed and planned,
-     * but they cannot be executed.
-     */
-    bool canHaveNoopMatchNodes() const {
-        return _canHaveNoopMatchNodes;
-    }
-
     bool getExplain() const {
         return _explain;
     }
@@ -311,7 +298,6 @@ private:
     Status init(OperationContext* opCtx,
                 boost::intrusive_ptr<ExpressionContext> expCtx,
                 std::unique_ptr<FindCommandRequest> findCommand,
-                bool canHaveNoopMatchNodes,
                 std::unique_ptr<MatchExpression> root,
                 const ProjectionPolicies& projectionPolicies,
                 std::vector<std::unique_ptr<InnerPipelineStageInterface>> pipeline,
@@ -340,8 +326,6 @@ private:
     // Keeps track of what metadata has been explicitly requested.
     QueryMetadataBitSet _metadataDeps;
 
-    bool _canHaveNoopMatchNodes = false;
-
     bool _explain = false;
 
     // Determines whether the classic engine must be used.
diff --git a/src/mongo/db/query/stage_builder_util.cpp b/src/mongo/db/query/stage_builder_util.cpp
index 96becfae481..a90a4881ae6 100644
--- a/src/mongo/db/query/stage_builder_util.cpp
+++ b/src/mongo/db/query/stage_builder_util.cpp
@@ -46,7 +46,6 @@ std::unique_ptr<PlanStage> buildClassicExecutableTree(OperationContext* opCtx,
     // queries that disallow extensions, can be properly executed. If the query does not have
     // $text/$where context (and $text/$where are allowed), then no attempt should be made to
     // execute the query.
-    invariant(!cq.canHaveNoopMatchNodes());
     invariant(solution.root());
     invariant(ws);
     auto builder = std::make_unique<ClassicStageBuilder>(opCtx, collection, cq, solution, ws);
@@ -63,7 +62,6 @@ buildSlotBasedExecutableTree(OperationContext* opCtx,
     // queries that disallow extensions, can be properly executed. If the query does not have
     // $text/$where context (and $text/$where are allowed), then no attempt should be made to
     // execute the query.
-    invariant(!cq.canHaveNoopMatchNodes());
     invariant(solution.root());
 
     auto sbeYieldPolicy = dynamic_cast<PlanYieldPolicySBE*>(yieldPolicy);