SERVER-63604 Optimize detection of the need to produce/propagate RecordId slots in SBE

19 files changed, 129 insertions, 113 deletions

diff --git a/src/mongo/db/dbhelpers.cpp b/src/mongo/db/dbhelpers.cpp
index 89f71139f07..dd00b6366e9 100644
--- a/src/mongo/db/dbhelpers.cpp
+++ b/src/mongo/db/dbhelpers.cpp
@@ -119,14 +119,13 @@ RecordId Helpers::findOne(OperationContext* opCtx,
 
     massertStatusOK(statusWithCQ.getStatus());
     unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue());
+    cq->setForceGenerateRecordId(true);
 
-    auto exec = uassertStatusOK(
-        getExecutor(opCtx,
-                    &collection,
-                    std::move(cq),
-                    nullptr /* extractAndAttachPipelineStages */,
-                    PlanYieldPolicy::YieldPolicy::NO_YIELD,
-                    QueryPlannerParams::DEFAULT | QueryPlannerParams::PRESERVE_RECORD_ID));
+    auto exec = uassertStatusOK(getExecutor(opCtx,
+                                            &collection,
+                                            std::move(cq),
+                                            nullptr /* extractAndAttachPipelineStages */,
+                                            PlanYieldPolicy::YieldPolicy::NO_YIELD));
 
     PlanExecutor::ExecState state;
     BSONObj obj;
diff --git a/src/mongo/db/query/canonical_query.h b/src/mongo/db/query/canonical_query.h
index 1c0213d0132..e939c248a9d 100644
--- a/src/mongo/db/query/canonical_query.h
+++ b/src/mongo/db/query/canonical_query.h
@@ -245,6 +245,14 @@ public:
         _explain = explain;
     }
 
+    bool getForceGenerateRecordId() const {
+        return _forceGenerateRecordId;
+    }
+
+    void setForceGenerateRecordId(bool value) {
+        _forceGenerateRecordId = value;
+    }
+
     OperationContext* getOpCtx() const {
         tassert(6508300, "'CanonicalQuery' does not have an 'ExpressionContext'", _expCtx);
         return _expCtx->opCtx;
@@ -310,6 +318,11 @@ private:
     // True if this query can be executed by the SBE.
     bool _sbeCompatible = false;
 
+    // True if this query must produce a RecordId output in addition to the BSON objects that
+    // constitute the result set of the query. Any generated query solution must not discard record
+    // ids, even if the optimizer detects that they are not going to be consumed downstream.
+    bool _forceGenerateRecordId = false;
+
     // A map from assigned InputParamId's to parameterised MatchExpression's.
     std::vector<const MatchExpression*> _inputParamIdToExpressionMap;
 };
diff --git a/src/mongo/db/query/canonical_query_encoder.cpp b/src/mongo/db/query/canonical_query_encoder.cpp
index 348e196d7f9..52344ab35f8 100644
--- a/src/mongo/db/query/canonical_query_encoder.cpp
+++ b/src/mongo/db/query/canonical_query_encoder.cpp
@@ -1119,6 +1119,7 @@ std::string encodeSBE(const CanonicalQuery& cq) {
 
     bufBuilder.appendBuf(proj.objdata(), proj.objsize());
     bufBuilder.appendStr(strBuilderEncoded, false /* includeEndingNull */);
+    bufBuilder.appendChar(cq.getForceGenerateRecordId() ? 1 : 0);
 
     encodeFindCommandRequest(cq.getFindCommandRequest(), &bufBuilder);
 
diff --git a/src/mongo/db/query/cqf_get_executor.cpp b/src/mongo/db/query/cqf_get_executor.cpp
index f2e13faebbd..f1843613b1d 100644
--- a/src/mongo/db/query/cqf_get_executor.cpp
+++ b/src/mongo/db/query/cqf_get_executor.cpp
@@ -484,9 +484,7 @@ std::unique_ptr<PlanExecutor, PlanExecutor::Deleter> getSBEExecutorViaCascadesOp
     const CollectionPtr& collection,
     const boost::optional<BSONObj>& indexHint,
     std::unique_ptr<Pipeline, PipelineDeleter> pipeline,
-    std::unique_ptr<CanonicalQuery> canonicalQuery,
-    const bool requireRID) {
-
+    std::unique_ptr<CanonicalQuery> canonicalQuery) {
     // Ensure that either pipeline or canonicalQuery is set.
     tassert(624070,
             "getSBEExecutorViaCascadesOptimizer expects exactly one of the following to be set: "
@@ -503,6 +501,7 @@ std::unique_ptr<PlanExecutor, PlanExecutor::Deleter> getSBEExecutorViaCascadesOp
     auto curOp = CurOp::get(opCtx);
     curOp->debug().cqfUsed = true;
 
+    const bool requireRID = canonicalQuery ? canonicalQuery->getForceGenerateRecordId() : false;
     const bool collectionExists = collection != nullptr;
     const std::string uuidStr = collectionExists ? collection->uuid().toString() : "<missing_uuid>";
     const std::string collNameStr = nss.coll().toString();
@@ -624,8 +623,7 @@ std::unique_ptr<PlanExecutor, PlanExecutor::Deleter> getSBEExecutorViaCascadesOp
 }
 
 std::unique_ptr<PlanExecutor, PlanExecutor::Deleter> getSBEExecutorViaCascadesOptimizer(
-    const CollectionPtr& collection, std::unique_ptr<CanonicalQuery> query, const bool requireRID) {
-
+    const CollectionPtr& collection, std::unique_ptr<CanonicalQuery> query) {
     boost::optional<BSONObj> indexHint = query->getFindCommandRequest().getHint().isEmpty()
         ? boost::none
         : boost::make_optional(query->getFindCommandRequest().getHint());
@@ -635,14 +633,8 @@ std::unique_ptr<PlanExecutor, PlanExecutor::Deleter> getSBEExecutorViaCascadesOp
     auto expCtx = query->getExpCtx();
     auto nss = query->nss();
 
-    return getSBEExecutorViaCascadesOptimizer(opCtx,
-                                              expCtx,
-                                              nss,
-                                              collection,
-                                              indexHint,
-                                              nullptr /* pipeline */,
-                                              std::move(query),
-                                              requireRID);
+    return getSBEExecutorViaCascadesOptimizer(
+        opCtx, expCtx, nss, collection, indexHint, nullptr /* pipeline */, std::move(query));
 }
 
 }  // namespace mongo
diff --git a/src/mongo/db/query/cqf_get_executor.h b/src/mongo/db/query/cqf_get_executor.h
index a2eb34081dc..81454fad196 100644
--- a/src/mongo/db/query/cqf_get_executor.h
+++ b/src/mongo/db/query/cqf_get_executor.h
@@ -49,15 +49,12 @@ std::unique_ptr<PlanExecutor, PlanExecutor::Deleter> getSBEExecutorViaCascadesOp
     const CollectionPtr& collection,
     const boost::optional<BSONObj>& indexHint,
     std::unique_ptr<Pipeline, PipelineDeleter> pipeline,
-    std::unique_ptr<CanonicalQuery> = nullptr,
-    bool requireRID = false);
+    std::unique_ptr<CanonicalQuery> = nullptr);
 
 /**
  * Returns a PlanExecutor for the given CanonicalQuery.
  */
 std::unique_ptr<PlanExecutor, PlanExecutor::Deleter> getSBEExecutorViaCascadesOptimizer(
-    const CollectionPtr& collection,
-    std::unique_ptr<CanonicalQuery> query,
-    bool requireRID = false);
+    const CollectionPtr& collection, std::unique_ptr<CanonicalQuery> query);
 
 }  // namespace mongo
diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp
index e2f0fc91726..3e8653eb229 100644
--- a/src/mongo/db/query/get_executor.cpp
+++ b/src/mongo/db/query/get_executor.cpp
@@ -1437,10 +1437,7 @@ StatusWith<std::unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutor(
         sbe::isQuerySbeCompatible(&mainColl, canonicalQuery.get(), plannerParams.options));
 
     if (isEligibleForBonsai(*canonicalQuery, opCtx, mainColl)) {
-        return getSBEExecutorViaCascadesOptimizer(mainColl,
-                                                  std::move(canonicalQuery),
-                                                  plannerParams.options &
-                                                      QueryPlannerParams::PRESERVE_RECORD_ID);
+        return getSBEExecutorViaCascadesOptimizer(mainColl, std::move(canonicalQuery));
     }
 
     // Use SBE if 'canonicalQuery' is SBE compatible.
@@ -1716,8 +1713,9 @@ StatusWith<std::unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorDele
 
     // The underlying query plan must preserve the record id, since it will be needed in order to
     // identify the record to update.
-    const size_t defaultPlannerOptions = QueryPlannerParams::PRESERVE_RECORD_ID;
+    cq->setForceGenerateRecordId(true);
 
+    const size_t defaultPlannerOptions = QueryPlannerParams::DEFAULT;
     ClassicPrepareExecutionHelper helper{
         opCtx, collection, ws.get(), cq.get(), nullptr, defaultPlannerOptions};
     auto executionResult = helper.prepare();
@@ -1907,8 +1905,9 @@ StatusWith<std::unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorUpda
 
     // The underlying query plan must preserve the record id, since it will be needed in order to
     // identify the record to update.
-    const size_t defaultPlannerOptions = QueryPlannerParams::PRESERVE_RECORD_ID;
+    cq->setForceGenerateRecordId(true);
 
+    const size_t defaultPlannerOptions = QueryPlannerParams::DEFAULT;
     ClassicPrepareExecutionHelper helper{
         opCtx, collection, ws.get(), cq.get(), nullptr, defaultPlannerOptions};
     auto executionResult = helper.prepare();
diff --git a/src/mongo/db/query/planner_analysis.cpp b/src/mongo/db/query/planner_analysis.cpp
index 4b19c1fac59..7cbc7116a92 100644
--- a/src/mongo/db/query/planner_analysis.cpp
+++ b/src/mongo/db/query/planner_analysis.cpp
@@ -585,7 +585,7 @@ bool canUseSimpleSort(const QuerySolutionNode& solnRoot,
         // record ids along through the sorting process is wasted work when these ids will never be
         // consumed later in the execution of the query. If the record ids are needed, however, then
         // we can't use the simple sort stage.
-        !(plannerParams.options & QueryPlannerParams::PRESERVE_RECORD_ID);
+        !cq.getForceGenerateRecordId();
 }
 
 boost::optional<const projection_ast::Projection*> attemptToGetProjectionFromQuerySolution(
diff --git a/src/mongo/db/query/query_planner.cpp b/src/mongo/db/query/query_planner.cpp
index c0e787c6d99..b330ad4bedf 100644
--- a/src/mongo/db/query/query_planner.cpp
+++ b/src/mongo/db/query/query_planner.cpp
@@ -463,9 +463,6 @@ string optionString(size_t options) {
             case QueryPlannerParams::STRICT_DISTINCT_ONLY:
                 ss << "STRICT_DISTINCT_ONLY ";
                 break;
-            case QueryPlannerParams::PRESERVE_RECORD_ID:
-                ss << "PRESERVE_RECORD_ID ";
-                break;
             case QueryPlannerParams::ASSERT_MIN_TS_HAS_NOT_FALLEN_OFF_OPLOG:
                 ss << "ASSERT_MIN_TS_HAS_NOT_FALLEN_OFF_OPLOG ";
                 break;
diff --git a/src/mongo/db/query/query_planner_options_test.cpp b/src/mongo/db/query/query_planner_options_test.cpp
index 7471af9e7a0..7f4cb9e3a1b 100644
--- a/src/mongo/db/query/query_planner_options_test.cpp
+++ b/src/mongo/db/query/query_planner_options_test.cpp
@@ -859,7 +859,7 @@ TEST_F(QueryPlannerTest, DollarResumeAfterFieldPropagatedFromQueryRequestToStage
 }
 
 TEST_F(QueryPlannerTest, PreserveRecordIdOptionPrecludesSimpleSort) {
-    params.options |= QueryPlannerParams::PRESERVE_RECORD_ID;
+    forceRecordId = true;
 
     runQueryAsCommand(fromjson("{find: 'testns', sort: {a:1}}"));
 
diff --git a/src/mongo/db/query/query_planner_params.h b/src/mongo/db/query/query_planner_params.h
index c826a92eb12..d055a1ef832 100644
--- a/src/mongo/db/query/query_planner_params.h
+++ b/src/mongo/db/query/query_planner_params.h
@@ -125,15 +125,9 @@ struct QueryPlannerParams {
         // declaration of getExecutorDistinct() for more detail.
         STRICT_DISTINCT_ONLY = 1 << 8,
 
-        // Instruct the planner that the caller is expecting to consume the record ids associated
-        // with documents returned by the plan. Any generated query solution must not discard record
-        // ids. In some cases, record ids can be discarded as an optimization when they will not be
-        // consumed downstream.
-        PRESERVE_RECORD_ID = 1 << 9,
-
         // Set this on an oplog scan to uassert that the oplog has not already rolled over the
         // minimum 'ts' timestamp specified in the query.
-        ASSERT_MIN_TS_HAS_NOT_FALLEN_OFF_OPLOG = 1 << 10,
+        ASSERT_MIN_TS_HAS_NOT_FALLEN_OFF_OPLOG = 1 << 9,
 
         // Instruct the plan enumerator to enumerate contained $ors in a special order. $or
         // enumeration can generate an exponential number of plans, and is therefore limited at some
@@ -150,16 +144,16 @@ struct QueryPlannerParams {
         // order, we would get assignments [a_b, a_b], [a_c, a_c], [a_c, a_b], then [a_b, a_c]. This
         // is thought to be helpful in general, but particularly in cases where all children of the
         // $or use the same fields and have the same indexes available, as in this example.
-        ENUMERATE_OR_CHILDREN_LOCKSTEP = 1 << 11,
+        ENUMERATE_OR_CHILDREN_LOCKSTEP = 1 << 10,
 
         // 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,
+        RETURN_OWNED_DATA = 1 << 11,
 
         // When generating column scan queries, splits match expressions so that the filters can be
         // applied per-column. This is off by default, since the execution side doesn't support it
         // yet.
-        GENERATE_PER_COLUMN_FILTERS = 1 << 13,
+        GENERATE_PER_COLUMN_FILTERS = 1 << 12,
     };
 
     // See Options enum above.
diff --git a/src/mongo/db/query/query_planner_test_fixture.cpp b/src/mongo/db/query/query_planner_test_fixture.cpp
index 6db2512950e..e31cc6ecd98 100644
--- a/src/mongo/db/query/query_planner_test_fixture.cpp
+++ b/src/mongo/db/query/query_planner_test_fixture.cpp
@@ -368,6 +368,7 @@ void QueryPlannerTest::runQueryFull(
     ASSERT_OK(statusWithCQ.getStatus());
     cq = std::move(statusWithCQ.getValue());
     cq->setSbeCompatible(markQueriesSbeCompatible);
+    cq->setForceGenerateRecordId(forceRecordId);
 
     auto statusWithMultiPlanSolns = QueryPlanner::plan(*cq, params);
     ASSERT_OK(statusWithMultiPlanSolns.getStatus());
@@ -445,6 +446,7 @@ void QueryPlannerTest::runInvalidQueryFull(const BSONObj& query,
     ASSERT_OK(statusWithCQ.getStatus());
     cq = std::move(statusWithCQ.getValue());
     cq->setSbeCompatible(markQueriesSbeCompatible);
+    cq->setForceGenerateRecordId(forceRecordId);
 
     auto statusWithMultiPlanSolns = QueryPlanner::plan(*cq, params);
     plannerStatus = statusWithMultiPlanSolns.getStatus();
@@ -474,6 +476,7 @@ void QueryPlannerTest::runQueryAsCommand(const BSONObj& cmdObj) {
     ASSERT_OK(statusWithCQ.getStatus());
     cq = std::move(statusWithCQ.getValue());
     cq->setSbeCompatible(markQueriesSbeCompatible);
+    cq->setForceGenerateRecordId(forceRecordId);
 
     auto statusWithMultiPlanSolns = QueryPlanner::plan(*cq, params);
     ASSERT_OK(statusWithMultiPlanSolns.getStatus());
@@ -502,6 +505,7 @@ void QueryPlannerTest::runInvalidQueryAsCommand(const BSONObj& cmdObj) {
     ASSERT_OK(statusWithCQ.getStatus());
     cq = std::move(statusWithCQ.getValue());
     cq->setSbeCompatible(markQueriesSbeCompatible);
+    cq->setForceGenerateRecordId(forceRecordId);
 
     auto statusWithMultiPlanSolns = QueryPlanner::plan(*cq, params);
     plannerStatus = statusWithMultiPlanSolns.getStatus();
diff --git a/src/mongo/db/query/query_planner_test_fixture.h b/src/mongo/db/query/query_planner_test_fixture.h
index 44f51282d55..35cc0d0c294 100644
--- a/src/mongo/db/query/query_planner_test_fixture.h
+++ b/src/mongo/db/query/query_planner_test_fixture.h
@@ -269,7 +269,12 @@ protected:
     std::vector<std::unique_ptr<QuerySolution>> solns;
 
     bool relaxBoundsCheck = false;
+    // Value used for the sbeCompatible flag in the CanonicalQuery objects created by the
+    // test.
     bool markQueriesSbeCompatible = false;
+    // Value used for the forceGenerateRecordId flag in the CanonicalQuery objects created by the
+    // test.
+    bool forceRecordId = false;
 };
 
 }  // namespace mongo
diff --git a/src/mongo/db/query/sbe_stage_builder.cpp b/src/mongo/db/query/sbe_stage_builder.cpp
index ab20b24f502..e21b72d0c0c 100644
--- a/src/mongo/db/query/sbe_stage_builder.cpp
+++ b/src/mongo/db/query/sbe_stage_builder.cpp
@@ -473,26 +473,6 @@ SlotBasedStageBuilder::SlotBasedStageBuilder(OperationContext* opCtx,
         _data.shouldTrackResumeToken = csn->requestResumeToken;
         _data.shouldUseTailableScan = csn->tailable;
     }
-
-    for (const auto& node : getAllNodesByType(solution.root(), STAGE_VIRTUAL_SCAN)) {
-        auto vsn = static_cast<const VirtualScanNode*>(node);
-        if (!vsn->hasRecordId) {
-            _shouldProduceRecordIdSlot = false;
-            break;
-        }
-    }
-
-    const auto [lookupNode, lookupCount] = getFirstNodeByType(solution.root(), STAGE_EQ_LOOKUP);
-    if (lookupCount) {
-        // TODO: SERVER-63604 optimize _shouldProduceRecordIdSlot maintenance
-        _shouldProduceRecordIdSlot = false;
-    }
-
-    const auto [groupNode, groupCount] = getFirstNodeByType(solution.root(), STAGE_GROUP);
-    if (groupCount) {
-        // TODO: SERVER-63604 optimize _shouldProduceRecordIdSlot maintenance
-        _shouldProduceRecordIdSlot = false;
-    }
 }
 
 std::unique_ptr<sbe::PlanStage> SlotBasedStageBuilder::build(const QuerySolutionNode* root) {
@@ -500,11 +480,15 @@ std::unique_ptr<sbe::PlanStage> SlotBasedStageBuilder::build(const QuerySolution
     invariant(!_buildHasStarted);
     _buildHasStarted = true;
 
-    // We always produce a 'resultSlot' and conditionally produce a 'recordIdSlot' based on the
-    // 'shouldProduceRecordIdSlot'.
+    // We always produce a 'resultSlot'.
     PlanStageReqs reqs;
     reqs.set(kResult);
-    reqs.setIf(kRecordId, _shouldProduceRecordIdSlot);
+    // We force the root stage to produce a 'recordId' if the iteration can be
+    // resumed (via a resume token or a tailable cursor) or if the caller simply expects to be able
+    // to read it.
+    reqs.setIf(kRecordId,
+               (_data.shouldUseTailableScan || _data.shouldTrackResumeToken ||
+                _cq.getForceGenerateRecordId()));
 
     // Set the target namespace to '_mainNss'. This is necessary as some QuerySolutionNodes that
     // require a collection when stage building do not explicitly name which collection they are
@@ -514,11 +498,10 @@ std::unique_ptr<sbe::PlanStage> SlotBasedStageBuilder::build(const QuerySolution
     // Build the SBE plan stage tree.
     auto [stage, outputs] = build(root, reqs);
 
-    // Assert that we produced a 'resultSlot' and that we prouced a 'recordIdSlot' if the
-    // 'shouldProduceRecordIdSlot' flag was set. Also assert that we produced an 'oplogTsSlot' if
-    // it's needed.
+    // Assert that we produced a 'resultSlot' and that we produced a 'recordIdSlot' only if it was
+    // needed.
     invariant(outputs.has(kResult));
-    invariant(!_shouldProduceRecordIdSlot || outputs.has(kRecordId));
+    invariant(reqs.has(kRecordId) == outputs.has(kRecordId));
 
     _data.outputs = std::move(outputs);
 
@@ -544,6 +527,10 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder
                                       outputs.get(kReturnKey),
                                       sbe::makeE<sbe::EFunction>("newObj", sbe::makeEs()));
     }
+    // Don't advertize the RecordId output if none of our ancestors are going to use it.
+    if (!reqs.has(kRecordId)) {
+        outputs.clear(kRecordId);
+    }
 
     return {std::move(stage), std::move(outputs)};
 }
@@ -659,6 +646,10 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder
                                                     iamMap,
                                                     reqs.has(kIndexKeyPattern));
 
+    // Remove the RecordId from the output if we were not requested to produce it.
+    if (!reqs.has(PlanStageSlots::kRecordId) && outputs.has(kRecordId)) {
+        outputs.clear(kRecordId);
+    }
     if (reqs.has(PlanStageSlots::kReturnKey)) {
         sbe::EExpression::Vector mkObjArgs;
 
@@ -984,10 +975,15 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder
                              _slotIdGenerator);
 
     outputs.set(kResult, fetchResultSlot);
-    outputs.set(kRecordId, fetchRecordIdSlot);
+    // Propagate the RecordId output only if requested.
+    if (reqs.has(kRecordId)) {
+        outputs.set(kRecordId, fetchRecordIdSlot);
+    } else {
+        outputs.clear(kRecordId);
+    }
 
     if (fn->filter) {
-        forwardingReqs = reqs.copy().set(kResult).set(kRecordId);
+        forwardingReqs = reqs.copy().set(kResult);
 
         relevantSlots = sbe::makeSV();
         outputs.forEachSlot(forwardingReqs, [&](auto&& slot) { relevantSlots.push_back(slot); });
@@ -1676,6 +1672,10 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder
     if (mergeSortNode->dedup) {
         stage = sbe::makeS<sbe::UniqueStage>(
             std::move(stage), sbe::makeSV(outputs.get(kRecordId)), root->nodeId());
+        // Stop propagating the RecordId output if none of our ancestors are going to use it.
+        if (!reqs.has(kRecordId)) {
+            outputs.clear(kRecordId);
+        }
     }
 
     return {std::move(stage), std::move(outputs)};
@@ -1894,6 +1894,10 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder
     if (orn->dedup) {
         stage = sbe::makeS<sbe::UniqueStage>(
             std::move(stage), sbe::makeSV(outputs.get(kRecordId)), root->nodeId());
+        // Stop propagating the RecordId output if none of our ancestors are going to use it.
+        if (!reqs.has(kRecordId)) {
+            outputs.clear(kRecordId);
+        }
     }
 
     if (orn->filter) {
@@ -2086,6 +2090,10 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder
                                                        collatorSlot,
                                                        root->nodeId());
     }
+    // Stop propagating the RecordId output if none of our ancestors are going to use it.
+    if (!reqs.has(kRecordId)) {
+        outputs.clear(kRecordId);
+    }
 
     return {std::move(hashJoinStage), std::move(outputs)};
 }
@@ -2195,6 +2203,10 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder
                                                          sortDirs,
                                                          root->nodeId());
     }
+    // Stop propagating the RecordId output if none of our ancestors are going to use it.
+    if (!reqs.has(kRecordId)) {
+        outputs.clear(kRecordId);
+    }
 
     return {std::move(mergeJoinStage), std::move(outputs)};
 }
@@ -2619,6 +2631,10 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder
     tassert(
         5851600, "should have one and only one child for GROUP", groupNode->children.size() == 1);
     tassert(5851601, "GROUP should have had group-by key expression", idExpr);
+    tassert(
+        6360401,
+        "GROUP cannot propagate a record id slot, but the record id was requested by the parent",
+        !reqs.has(kRecordId));
 
     const auto& childNode = groupNode->children[0].get();
     const auto& accStmts = groupNode->accumulators;
@@ -2634,7 +2650,6 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder
 
     // Builds the child and gets the child result slot.
     auto [childStage, childOutputs] = build(childNode, childReqs);
-    _shouldProduceRecordIdSlot = false;
 
     tassert(6075900,
             "Expected no optimized expressions but got: {}"_format(_state.preGeneratedExprs.size()),
diff --git a/src/mongo/db/query/sbe_stage_builder.h b/src/mongo/db/query/sbe_stage_builder.h
index ab928f14329..1507069fe90 100644
--- a/src/mongo/db/query/sbe_stage_builder.h
+++ b/src/mongo/db/query/sbe_stage_builder.h
@@ -541,7 +541,6 @@ private:
     PlanStageData _data;
 
     bool _buildHasStarted{false};
-    bool _shouldProduceRecordIdSlot{true};
 
     // Common parameters to SBE stage builder functions.
     StageBuilderState _state;
diff --git a/src/mongo/db/query/sbe_stage_builder_lookup.cpp b/src/mongo/db/query/sbe_stage_builder_lookup.cpp
index 3b35fb71ef6..aa4ae7fbde3 100644
--- a/src/mongo/db/query/sbe_stage_builder_lookup.cpp
+++ b/src/mongo/db/query/sbe_stage_builder_lookup.cpp
@@ -1063,9 +1063,6 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder
         _state.data->foreignHashJoinCollections.emplace(eqLookupNode->foreignCollection);
     }
 
-    // $lookup creates its own output documents.
-    _shouldProduceRecordIdSlot = false;
-
     auto localReqs = reqs.copy().set(kResult);
     auto [localStage, localOutputs] = build(eqLookupNode->children[0].get(), localReqs);
     SlotId localDocumentSlot = localOutputs.get(PlanStageSlots::kResult);
diff --git a/src/mongo/db/query/sbe_stage_builder_test_fixture.cpp b/src/mongo/db/query/sbe_stage_builder_test_fixture.cpp
index c6b0525dea9..91042b7b096 100644
--- a/src/mongo/db/query/sbe_stage_builder_test_fixture.cpp
+++ b/src/mongo/db/query/sbe_stage_builder_test_fixture.cpp
@@ -60,6 +60,10 @@ SbeStageBuilderTestFixture::buildPlanStage(
     auto statusWithCQ =
         CanonicalQuery::canonicalize(operationContext(), std::move(findCommand), false, expCtx);
     ASSERT_OK(statusWithCQ.getStatus());
+    if (hasRecordId) {
+        // Force the builder to generate the RecordId output even if it isn't needed by the plan.
+        statusWithCQ.getValue()->setForceGenerateRecordId(true);
+    }
 
     CollectionMock coll(_nss);
     CollectionPtr collPtr(&coll);
diff --git a/src/mongo/db/test_output/query/canonical_query_encoder_test/compute_key_s_b_e.txt b/src/mongo/db/test_output/query/canonical_query_encoder_test/compute_key_s_b_e.txt
index f7f248ddb74..8d7a52033c4 100644
--- a/src/mongo/db/test_output/query/canonical_query_encoder_test/compute_key_s_b_e.txt
+++ b/src/mongo/db/test_output/query/canonical_query_encoder_test/compute_key_s_b_e.txt
@@ -1,48 +1,48 @@
 ==== VARIATION: sbe, query={}, sort={}, proj={}
-YW4ABQAAAAAAAAAAAAAAAG5ubm4FAAAAAGZe
+YW4ABQAAAAAAAAAAAAAAAABubm5uBQAAAABmXg==
 ==== VARIATION: sbe, query={$or: [{a: 1}, {b: 2}]}, sort={}, proj={}
-b3IAW2VxAGE/AAAAACxlcQBiPwEAAABdBQAAAAAAAAAAAAAAAG5ubm4FAAAAAGZe
+b3IAW2VxAGE/AAAAACxlcQBiPwEAAABdBQAAAAAAAAAAAAAAAABubm5uBQAAAABmXg==
 ==== VARIATION: sbe, query={a: 1}, sort={}, proj={}
-ZXEAYT8AAAAABQAAAAAAAAAAAAAAAG5ubm4FAAAAAGZe
+ZXEAYT8AAAAABQAAAAAAAAAAAAAAAABubm5uBQAAAABmXg==
 ==== VARIATION: sbe, query={b: 1}, sort={}, proj={}
-ZXEAYj8AAAAABQAAAAAAAAAAAAAAAG5ubm4FAAAAAGZe
+ZXEAYj8AAAAABQAAAAAAAAAAAAAAAABubm5uBQAAAABmXg==
 ==== VARIATION: sbe, query={a: 1, b: 1, c: 1}, sort={}, proj={}
-YW4AW2VxAGE/AAAAACxlcQBiPwEAAAAsZXEAYz8CAAAAXQUAAAAAAAAAAAAAAABubm5uBQAAAABmXg==
+YW4AW2VxAGE/AAAAACxlcQBiPwEAAAAsZXEAYz8CAAAAXQUAAAAAAAAAAAAAAAAAbm5ubgUAAAAAZl4=
 ==== VARIATION: sbe, query={}, sort={a: 1}, proj={}
-YW4ABQAAAAB+YWEAAAAAAAAAAG5ubm4FAAAAAGZe
+YW4ABQAAAAB+YWEAAAAAAAAAAABubm5uBQAAAABmXg==
 ==== VARIATION: sbe, query={}, sort={a: -1}, proj={}
-YW4ABQAAAAB+ZGEAAAAAAAAAAG5ubm4FAAAAAGZe
+YW4ABQAAAAB+ZGEAAAAAAAAAAABubm5uBQAAAABmXg==
 ==== VARIATION: sbe, query={a: 1}, sort={a: 1}, proj={}
-ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAG5ubm4FAAAAAGZe
+ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAABubm5uBQAAAABmXg==
 ==== VARIATION: sbe, query={a: 1}, sort={a: 1}, proj={a: 1}
-ZXEAYT8AAAAADAAAABBhAAEAAAAAfmFhAAAAAAAAAABubm5uBQAAAABmXg==
+ZXEAYT8AAAAADAAAABBhAAEAAAAAfmFhAAAAAAAAAAAAbm5ubgUAAAAAZl4=
 ==== VARIATION: sbe, query={}, sort={a: 1}, proj={a: 1}
-YW4ADAAAABBhAAEAAAAAfmFhAAAAAAAAAABubm5uBQAAAABmXg==
+YW4ADAAAABBhAAEAAAAAfmFhAAAAAAAAAAAAbm5ubgUAAAAAZl4=
 ==== VARIATION: sbe, query={}, sort={a: 1}, proj={a: 1}
-YW4ADAAAABBhAAEAAAAAfmFhAAAAAAAAAABubm5uBQAAAABmXg==
+YW4ADAAAABBhAAEAAAAAfmFhAAAAAAAAAAAAbm5ubgUAAAAAZl4=
 ==== VARIATION: sbe, query={}, sort={}, proj={a: 1}
-YW4ADAAAABBhAAEAAAAAAAAAAAAAAABubm5uBQAAAABmXg==
+YW4ADAAAABBhAAEAAAAAAAAAAAAAAAAAbm5ubgUAAAAAZl4=
 ==== VARIATION: sbe, query={}, sort={}, proj={a: true}
-YW4ACQAAAAhhAAEAAAAAAAAAAABubm5uBQAAAABmXg==
+YW4ACQAAAAhhAAEAAAAAAAAAAAAAbm5ubgUAAAAAZl4=
 ==== VARIATION: sbe, query={}, sort={}, proj={a: false}
-YW4ACQAAAAhhAAAAAAAAAAAAAABubm5uBQAAAABmXg==
+YW4ACQAAAAhhAAAAAAAAAAAAAAAAbm5ubgUAAAAAZl4=
 ==== VARIATION: sbe, query={a: 1}, sort={a: 1}, proj={}, allowDiskUse=0, returnKey=0, requestResumeToken=0
-ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAG5ubm4FAAAAAGZe
+ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAABubm5uBQAAAABmXg==
 ==== VARIATION: sbe, query={a: 1}, sort={a: 1}, proj={}, allowDiskUse=1, returnKey=0, requestResumeToken=0
-ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAHRubm4FAAAAAGZe
+ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAAB0bm5uBQAAAABmXg==
 ==== VARIATION: sbe, query={a: 1}, sort={a: 1}, proj={}, allowDiskUse=0, returnKey=0, requestResumeToken=0
-ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAGZubm4FAAAAAGZe
+ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAABmbm5uBQAAAABmXg==
 ==== VARIATION: sbe, query={a: 1}, sort={a: 1}, proj={}, allowDiskUse=0, returnKey=1, requestResumeToken=0
-ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAG50bm4FAAAAAGZe
+ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAABudG5uBQAAAABmXg==
 ==== VARIATION: sbe, query={a: 1}, sort={a: 1}, proj={}, allowDiskUse=0, returnKey=0, requestResumeToken=0
-ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAG5uZm4FAAAAAGZe
+ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAABubmZuBQAAAABmXg==
 ==== VARIATION: sbe, query={a: 1}, sort={a: 1}, proj={}, allowDiskUse=0, returnKey=0, requestResumeToken=0
-ZXEAYT8AAAAABQAAAAB+YWEKAAAAAAAAAAAAAABubm5uBQAAAABmXg==
+ZXEAYT8AAAAABQAAAAB+YWEACgAAAAAAAAAAAAAAbm5ubgUAAAAAZl4=
 ==== VARIATION: sbe, query={a: 1}, sort={a: 1}, proj={}, allowDiskUse=0, returnKey=0, requestResumeToken=0
-ZXEAYT8AAAAABQAAAAB+YWEAAAAACgAAAAAAAABubm5uBQAAAABmXg==
+ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAoAAAAAAAAAbm5ubgUAAAAAZl4=
 ==== VARIATION: sbe, query={a: 1}, sort={a: 1}, proj={}, allowDiskUse=0, returnKey=0, requestResumeToken=0
-ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAG5ubm4FAAAAAGZe
+ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAABubm5uBQAAAABmXg==
 ==== VARIATION: sbe, query={a: 1}, sort={a: 1}, proj={}, allowDiskUse=0, returnKey=0, requestResumeToken=0
-ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAG5ubm4FAAAAAGZe
+ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAABubm5uBQAAAABmXg==
 ==== VARIATION: sbe, query={a: 1}, sort={}, proj={}, allowDiskUse=0, returnKey=0, requestResumeToken=1
-ZXEAYT8AAAAABQAAAAAAAAAAAAAAAG5udG4YAAAAEiRyZWNvcmRJZAABAAAAAAAAAABmXg==
+ZXEAYT8AAAAABQAAAAAAAAAAAAAAAABubnRuGAAAABIkcmVjb3JkSWQAAQAAAAAAAAAAZl4=
diff --git a/src/mongo/db/test_output/query/canonical_query_encoder_test/compute_key_s_b_e_with_pipeline.txt b/src/mongo/db/test_output/query/canonical_query_encoder_test/compute_key_s_b_e_with_pipeline.txt
index 15084d41d0e..532045bed16 100644
--- a/src/mongo/db/test_output/query/canonical_query_encoder_test/compute_key_s_b_e_with_pipeline.txt
+++ b/src/mongo/db/test_output/query/canonical_query_encoder_test/compute_key_s_b_e_with_pipeline.txt
@@ -1,12 +1,12 @@
 ==== VARIATION: sbe, query={a: 1}, sort={}, proj={}
-ZXEAYT8AAAAABQAAAAAAAAAAAAAAAG5ubm4FAAAAAGZe
+ZXEAYT8AAAAABQAAAAAAAAAAAAAAAABubm5uBQAAAABmXg==
 ==== VARIATION: sbe, query={a: 1}, sort={}, proj={}
-ZXEAYT8AAAAABQAAAAAAAAAAAAAAAG5ubm4FAAAAAGZeWgAAAAMkbG9va3VwAEwAAAACZnJvbQAMAAAAZm9yZWlnbmNvbGwAAmFzAAMAAABhcwACbG9jYWxGaWVsZAACAAAAYQACZm9yZWlnbkZpZWxkAAIAAABiAAAA
+ZXEAYT8AAAAABQAAAAAAAAAAAAAAAABubm5uBQAAAABmXloAAAADJGxvb2t1cABMAAAAAmZyb20ADAAAAGZvcmVpZ25jb2xsAAJhcwADAAAAYXMAAmxvY2FsRmllbGQAAgAAAGEAAmZvcmVpZ25GaWVsZAACAAAAYgAAAA==
 ==== VARIATION: sbe, query={a: 1}, sort={}, proj={}
-ZXEAYT8AAAAABQAAAAAAAAAAAAAAAG5ubm4FAAAAAGZeWwAAAAMkbG9va3VwAE0AAAACZnJvbQAMAAAAZm9yZWlnbmNvbGwAAmFzAAMAAABhcwACbG9jYWxGaWVsZAADAAAAYTEAAmZvcmVpZ25GaWVsZAACAAAAYgAAAA==
+ZXEAYT8AAAAABQAAAAAAAAAAAAAAAABubm5uBQAAAABmXlsAAAADJGxvb2t1cABNAAAAAmZyb20ADAAAAGZvcmVpZ25jb2xsAAJhcwADAAAAYXMAAmxvY2FsRmllbGQAAwAAAGExAAJmb3JlaWduRmllbGQAAgAAAGIAAAA=
 ==== VARIATION: sbe, query={a: 1}, sort={}, proj={}
-ZXEAYT8AAAAABQAAAAAAAAAAAAAAAG5ubm4FAAAAAGZeWwAAAAMkbG9va3VwAE0AAAACZnJvbQAMAAAAZm9yZWlnbmNvbGwAAmFzAAMAAABhcwACbG9jYWxGaWVsZAACAAAAYQACZm9yZWlnbkZpZWxkAAMAAABiMQAAAA==
+ZXEAYT8AAAAABQAAAAAAAAAAAAAAAABubm5uBQAAAABmXlsAAAADJGxvb2t1cABNAAAAAmZyb20ADAAAAGZvcmVpZ25jb2xsAAJhcwADAAAAYXMAAmxvY2FsRmllbGQAAgAAAGEAAmZvcmVpZ25GaWVsZAADAAAAYjEAAAA=
 ==== VARIATION: sbe, query={a: 1}, sort={}, proj={}
-ZXEAYT8AAAAABQAAAAAAAAAAAAAAAG5ubm4FAAAAAGZeWwAAAAMkbG9va3VwAE0AAAACZnJvbQAMAAAAZm9yZWlnbmNvbGwAAmFzAAQAAABhczEAAmxvY2FsRmllbGQAAgAAAGEAAmZvcmVpZ25GaWVsZAACAAAAYgAAAA==
+ZXEAYT8AAAAABQAAAAAAAAAAAAAAAABubm5uBQAAAABmXlsAAAADJGxvb2t1cABNAAAAAmZyb20ADAAAAGZvcmVpZ25jb2xsAAJhcwAEAAAAYXMxAAJsb2NhbEZpZWxkAAIAAABhAAJmb3JlaWduRmllbGQAAgAAAGIAAAA=
 ==== VARIATION: sbe, query={a: 1}, sort={}, proj={}
-ZXEAYT8AAAAABQAAAAAAAAAAAAAAAG5ubm4FAAAAAGZeWgAAAAMkbG9va3VwAEwAAAACZnJvbQAMAAAAZm9yZWlnbmNvbGwAAmFzAAMAAABhcwACbG9jYWxGaWVsZAACAAAAYQACZm9yZWlnbkZpZWxkAAIAAABiAAAAXQAAAAMkbG9va3VwAE8AAAACZnJvbQAMAAAAZm9yZWlnbmNvbGwAAmFzAAQAAABhczEAAmxvY2FsRmllbGQAAwAAAGExAAJmb3JlaWduRmllbGQAAwAAAGIxAAAA
+ZXEAYT8AAAAABQAAAAAAAAAAAAAAAABubm5uBQAAAABmXloAAAADJGxvb2t1cABMAAAAAmZyb20ADAAAAGZvcmVpZ25jb2xsAAJhcwADAAAAYXMAAmxvY2FsRmllbGQAAgAAAGEAAmZvcmVpZ25GaWVsZAACAAAAYgAAAF0AAAADJGxvb2t1cABPAAAAAmZyb20ADAAAAGZvcmVpZ25jb2xsAAJhcwAEAAAAYXMxAAJsb2NhbEZpZWxkAAMAAABhMQACZm9yZWlnbkZpZWxkAAMAAABiMQAAAA==
diff --git a/src/mongo/db/test_output/query/canonical_query_encoder_test/compute_key_s_b_e_with_read_concern.txt b/src/mongo/db/test_output/query/canonical_query_encoder_test/compute_key_s_b_e_with_read_concern.txt
index 5e494c83f72..8177ce2bbc8 100644
--- a/src/mongo/db/test_output/query/canonical_query_encoder_test/compute_key_s_b_e_with_read_concern.txt
+++ b/src/mongo/db/test_output/query/canonical_query_encoder_test/compute_key_s_b_e_with_read_concern.txt
@@ -1,6 +1,6 @@
 ==== VARIATION: sbe, query={a: 1}, sort={a: 1}, proj={}, allowDiskUse=0, returnKey=0, requestResumeToken=0
-ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAG5ubm4FAAAAAGZe
+ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAABubm5uBQAAAABmXg==
 ==== VARIATION: sbe, query={a: 1}, sort={a: 1}, proj={}, allowDiskUse=0, returnKey=0, requestResumeToken=0
-ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAG5ubm4FAAAAAGZe
+ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAABubm5uBQAAAABmXg==
 ==== VARIATION: sbe, query={a: 1}, sort={a: 1}, proj={}, allowDiskUse=0, returnKey=0, requestResumeToken=0
-ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAG5ubm4FAAAAAHRe
+ZXEAYT8AAAAABQAAAAB+YWEAAAAAAAAAAABubm5uBQAAAAB0Xg==