SERVER-74539 [CQF] Remove some assumptions about Sargable disjunction

8 files changed, 230 insertions, 106 deletions

diff --git a/src/mongo/db/query/optimizer/algebra/operator.h b/src/mongo/db/query/optimizer/algebra/operator.h
index 934fb504397..1dd75272747 100644
--- a/src/mongo/db/query/optimizer/algebra/operator.h
+++ b/src/mongo/db/query/optimizer/algebra/operator.h
@@ -351,6 +351,7 @@ public:
  *
  *      int transport(const NodeType&, int childResult0, int childResult1)
  *
+ * This method guarantees depth-first, left-to-right order.
  */
 template <bool withSlot = false, typename D, typename N, typename... Args>
 auto transport(N&& node, D& domain, Args&&... args) {
diff --git a/src/mongo/db/query/optimizer/bool_expression.h b/src/mongo/db/query/optimizer/bool_expression.h
index ed5cd052c9c..7e0371041b3 100644
--- a/src/mongo/db/query/optimizer/bool_expression.h
+++ b/src/mongo/db/query/optimizer/bool_expression.h
@@ -41,12 +41,14 @@
 namespace mongo::optimizer {
 
 template <class T>
-struct NoOpNegator {
+struct TassertNegator {
     T operator()(const T v) const {
+        tassert(7453909, "No negator specified", false);
         return v;
     }
 };
 
+
 /**
  * Represents a generic boolean expression with arbitrarily nested conjunctions and disjunction
  * elements.
@@ -205,6 +207,19 @@ struct BoolExpr {
         });
     }
 
+    static void visitAnyShape(const Node& node, const AtomVisitorConst& atomVisitor) {
+        struct AtomTransport {
+            void transport(const Conjunction&, const NodeVector&) {}
+            void transport(const Disjunction&, const NodeVector&) {}
+            void transport(const Atom& node) {
+                atomVisitor(node.getExpr());
+            }
+            const AtomVisitorConst& atomVisitor;
+        };
+        AtomTransport impl{atomVisitor};
+        algebra::transport<false>(node, impl);
+    }
+
     static void visitCNF(Node& node, const AtomVisitor& visitor) {
         visitConjuncts(node, [&](Node& child, const size_t) {
             visitDisjuncts(child,
@@ -219,6 +234,19 @@ struct BoolExpr {
         });
     }
 
+    static void visitAnyShape(Node& node, const AtomVisitor& atomVisitor) {
+        struct AtomTransport {
+            void transport(Conjunction&, NodeVector&) {}
+            void transport(Disjunction&, NodeVector&) {}
+            void transport(Atom& node) {
+                atomVisitor(node.getExpr());
+            }
+            const AtomVisitor& atomVisitor;
+        };
+        AtomTransport impl{atomVisitor};
+        algebra::transport<false>(node, impl);
+    }
+
 
     static bool isCNF(const Node& n) {
         if (n.template is<Conjunction>()) {
@@ -242,6 +270,11 @@ struct BoolExpr {
         return false;
     }
 
+    static bool isSingletonDisjunction(const Node& node) {
+        auto* disjunction = node.template cast<Disjunction>();
+        return disjunction && disjunction->nodes().size() == 1;
+    }
+
     static size_t numLeaves(const Node& n) {
         return NumLeavesTransporter().countLeaves(n);
     }
@@ -267,7 +300,7 @@ struct BoolExpr {
      */
     template <bool simplifyEmptyOrSingular = false,
               bool removeDups = false,
-              class Negator = NoOpNegator<T>>
+              class Negator = TassertNegator<T>>
     class Builder {
         enum class NodeType { Conj, Disj };
 
diff --git a/src/mongo/db/query/optimizer/cascades/implementers.cpp b/src/mongo/db/query/optimizer/cascades/implementers.cpp
index 7d96a1e541f..5214ac139ba 100644
--- a/src/mongo/db/query/optimizer/cascades/implementers.cpp
+++ b/src/mongo/db/query/optimizer/cascades/implementers.cpp
@@ -417,9 +417,16 @@ public:
         }
 
         const ProjectionName& scanProjectionName = indexingAvailability.getScanProjection();
-        for (const auto& [key, req] : reqMap.conjuncts()) {
-            if (key._projectionName != scanProjectionName) {
-                // We can only satisfy partial schema requirements using our root projection.
+
+        // We can only satisfy partial schema requirements using our root projection.
+        {
+            bool anyNonRoot = false;
+            PSRExpr::visitAnyShape(reqMap.getRoot(), [&](const PartialSchemaEntry& e) {
+                if (e.first._projectionName != scanProjectionName) {
+                    anyNonRoot = true;
+                }
+            });
+            if (anyNonRoot) {
                 return;
             }
         }
@@ -438,11 +445,8 @@ public:
                 requiresRootProjection = projectionsLeftToSatisfy.erase(scanProjectionName);
             }
 
-            for (const auto& entry : reqMap.conjuncts()) {
-                if (const auto& boundProjName = entry.second.getBoundProjectionName()) {
-                    // Project field only if it required.
-                    projectionsLeftToSatisfy.erase(*boundProjName);
-                }
+            for (const auto& [key, boundProjName] : getBoundProjections(reqMap.getRoot())) {
+                projectionsLeftToSatisfy.erase(boundProjName);
             }
             if (!projectionsLeftToSatisfy.getVector().empty()) {
                 // Unknown projections remain. Reject.
diff --git a/src/mongo/db/query/optimizer/index_bounds.h b/src/mongo/db/query/optimizer/index_bounds.h
index 00e7e171bd6..420c21892b6 100644
--- a/src/mongo/db/query/optimizer/index_bounds.h
+++ b/src/mongo/db/query/optimizer/index_bounds.h
@@ -363,6 +363,16 @@ struct CandidateIndexEntry {
     size_t _intervalPrefixSize;
 };
 
+/**
+ * ScanParams describes a set of predicates and projections to use for a collection scan or fetch.
+ *
+ * The semantics are:
+ * 1. Apply the FieldProjectionMap to introduce some bindings.
+ * 2. Apply the ResidualRequirements (a filter), which can read any of those bindings.
+ *
+ * We represent projections specially because SBE 'ScanStage' is more efficient at handling multiple
+ * fields, compared to doing N separate getField calls.
+ */
 struct ScanParams {
     bool operator==(const ScanParams& other) const;
 
diff --git a/src/mongo/db/query/optimizer/partial_schema_requirements.cpp b/src/mongo/db/query/optimizer/partial_schema_requirements.cpp
index bc78a429709..0659675ccee 100644
--- a/src/mongo/db/query/optimizer/partial_schema_requirements.cpp
+++ b/src/mongo/db/query/optimizer/partial_schema_requirements.cpp
@@ -145,7 +145,9 @@ size_t PartialSchemaRequirements::numConjuncts() const {
 
 boost::optional<ProjectionName> PartialSchemaRequirements::findProjection(
     const PartialSchemaKey& key) const {
-    assertIsSingletonDisjunction();
+    tassert(7453908,
+            "Expected PartialSchemaRequirement to be a singleton disjunction",
+            PSRExpr::isSingletonDisjunction(getRoot()));
 
     boost::optional<ProjectionName> proj;
     PSRExpr::visitDNF(_expr, [&](const Entry& entry) {
@@ -158,7 +160,9 @@ boost::optional<ProjectionName> PartialSchemaRequirements::findProjection(
 
 boost::optional<std::pair<size_t, PartialSchemaRequirement>>
 PartialSchemaRequirements::findFirstConjunct(const PartialSchemaKey& key) const {
-    assertIsSingletonDisjunction();
+    tassert(7453907,
+            "Expected PartialSchemaRequirement to be a singleton disjunction",
+            PSRExpr::isSingletonDisjunction(getRoot()));
 
     size_t i = 0;
     boost::optional<std::pair<size_t, PartialSchemaRequirement>> res;
@@ -185,14 +189,6 @@ void PartialSchemaRequirements::add(PartialSchemaKey key, PartialSchemaRequireme
     normalize();
 }
 
-void PartialSchemaRequirements::assertIsSingletonDisjunction() const {
-    if (auto disjunction = _expr.cast<PSRExpr::Disjunction>();
-        disjunction && disjunction->nodes().size() == 1) {
-        return;
-    }
-    tasserted(7016405, "Expected PartialSchemaRequirement to be a singleton disjunction");
-}
-
 namespace {
 // TODO SERVER-73827: Apply this simplification during BoolExpr building.
 template <bool isCNF,
@@ -261,4 +257,25 @@ bool PartialSchemaRequirements::simplify(
     return simplifyExpr<false /*isCNF*/>(_expr, func);
 }
 
+/**
+ * Returns a vector of ((input binding, path), output binding). The output binding names
+ * are unique and you can think of the vector as a product: every row has all the projections
+ * available.
+ */
+std::vector<std::pair<PartialSchemaKey, ProjectionName>> getBoundProjections(
+    const PartialSchemaRequirements& reqs) {
+    // For now we assume no projections inside a nontrivial disjunction.
+    std::vector<std::pair<PartialSchemaKey, ProjectionName>> result;
+    PSRExpr::visitAnyShape(reqs.getRoot(), [&](const PartialSchemaEntry& e) {
+        const auto& [key, req] = e;
+        if (auto proj = req.getBoundProjectionName()) {
+            result.emplace_back(key, *proj);
+        }
+    });
+    tassert(7453906,
+            "Expected no bound projections in a nontrivial disjunction",
+            result.empty() || PSRExpr::isSingletonDisjunction(reqs.getRoot()));
+    return result;
+}
+
 }  // namespace mongo::optimizer
diff --git a/src/mongo/db/query/optimizer/partial_schema_requirements.h b/src/mongo/db/query/optimizer/partial_schema_requirements.h
index 7f0fb77b1cd..de57d4f7012 100644
--- a/src/mongo/db/query/optimizer/partial_schema_requirements.h
+++ b/src/mongo/db/query/optimizer/partial_schema_requirements.h
@@ -36,9 +36,7 @@ namespace mongo::optimizer {
 
 using PartialSchemaEntry = std::pair<PartialSchemaKey, PartialSchemaRequirement>;
 using PSRExpr = BoolExpr<PartialSchemaEntry>;
-using PSRExprBuilder = PSRExpr::Builder<false /*simplifyEmptyOrSingular*/,
-                                        false /*removeDups*/,
-                                        NoOpNegator<PartialSchemaEntry>>;
+using PSRExprBuilder = PSRExpr::Builder<false /*simplifyEmptyOrSingular*/, false /*removeDups*/>;
 
 /**
  * Represents a set of predicates and projections. Cannot represent all predicates/projections:
@@ -166,7 +164,9 @@ public:
 
     // TODO SERVER-74101: Remove these methods in favor of visitDis/Conjuncts().
     Range<true> conjuncts() const {
-        assertIsSingletonDisjunction();
+        tassert(7453905,
+                "Expected PartialSchemaRequirement to be a singleton disjunction",
+                PSRExpr::isSingletonDisjunction(_expr));
         const auto& atoms = _expr.cast<PSRExpr::Disjunction>()
                                 ->nodes()
                                 .begin()
@@ -176,7 +176,9 @@ public:
     }
 
     Range<false> conjuncts() {
-        assertIsSingletonDisjunction();
+        tassert(7453904,
+                "Expected PartialSchemaRequirement to be a singleton disjunction",
+                PSRExpr::isSingletonDisjunction(_expr));
         auto& atoms = _expr.cast<PSRExpr::Disjunction>()
                           ->nodes()
                           .begin()
@@ -219,11 +221,16 @@ private:
     // TODO SERVER-73827: Consider applying this normalization during BoolExpr building.
     void normalize();
 
-    // Asserts that _expr is in DNF form where the disjunction has a single conjunction child.
-    void assertIsSingletonDisjunction() const;
-
     // _expr is currently always in DNF.
     PSRExpr::Node _expr;
 };
 
+/**
+ * Returns a vector of ((input binding, path), output binding). The output binding names
+ * are unique and you can think of the vector as a product: every row has all the projections
+ * available.
+ */
+std::vector<std::pair<PartialSchemaKey, ProjectionName>> getBoundProjections(
+    const PartialSchemaRequirements& reqs);
+
 }  // namespace mongo::optimizer
diff --git a/src/mongo/db/query/optimizer/utils/utils.cpp b/src/mongo/db/query/optimizer/utils/utils.cpp
index 8049124da6d..2f8b5d49f23 100644
--- a/src/mongo/db/query/optimizer/utils/utils.cpp
+++ b/src/mongo/db/query/optimizer/utils/utils.cpp
@@ -170,7 +170,13 @@ public:
     PartialSchemaReqConverter(const bool isFilterContext, const PathToIntervalFn& pathToInterval)
         : _isFilterContext(isFilterContext), _pathToInterval(pathToInterval) {}
 
+    /**
+     * Handle EvalPath and EvalFilter nodes.
+     */
     ResultType handleEvalContext(ResultType pathResult, ResultType inputResult) {
+        // In a (Eval <path> <input>) expression, we expect the <path> to result only in a path and
+        // intervals: no input binding or output binding. The input binding comes from <input>, and
+        // we don't expect <input> to have any predicates or output bindings.
         if (!pathResult || !inputResult) {
             return {};
         }
@@ -178,22 +184,19 @@ public:
             return {};
         }
 
-        if (auto boundPtr = inputResult->_bound->cast<Variable>(); boundPtr != nullptr) {
-            const ProjectionName& boundVarName = boundPtr->name();
-            PSRExpr::Builder newReqs;
-            newReqs.pushDisj().pushConj();
-
-            for (auto& [key, req] : pathResult->_reqMap.conjuncts()) {
-                if (key._projectionName) {
-                    return {};
-                }
-
-                newReqs.atom(PartialSchemaKey{boundVarName, key._path}, std::move(req));
-            }
+        if (auto* inputVar = inputResult->_bound->cast<Variable>()) {
+            // Every Atom in pathResult has an unknown input binding.
+            // Fill it in with 'inputVar'.
 
-            PartialSchemaReqConversion result{std::move(*newReqs.finish())};
-            result._retainPredicate = pathResult->_retainPredicate;
-            return result;
+            const ProjectionName& inputVarName = inputVar->name();
+            PSRExpr::visitAnyShape(pathResult->_reqMap.getRoot(), [&](PartialSchemaEntry& entry) {
+                tassert(
+                    7453903,
+                    "Expected PartialSchemaReqConversion for a path to have its input left blank",
+                    !entry.first._projectionName);
+                entry.first._projectionName = inputVarName;
+            });
+            return pathResult;
         }
 
         return {};
@@ -203,6 +206,10 @@ public:
                          const EvalPath& evalPath,
                          ResultType pathResult,
                          ResultType inputResult) {
+        if (_isFilterContext) {
+            // 'pathResult' was translated as if it appeared in EvalFilter; we can't use it.
+            return {};
+        }
         return handleEvalContext(std::move(pathResult), std::move(inputResult));
     }
 
@@ -210,6 +217,10 @@ public:
                          const EvalFilter& evalFilter,
                          ResultType pathResult,
                          ResultType inputResult) {
+        if (!_isFilterContext) {
+            // 'pathResult' was translated as if it appeared in EvalPath; we can't use it.
+            return {};
+        }
         return handleEvalContext(std::move(pathResult), std::move(inputResult));
     }
 
@@ -292,6 +303,19 @@ public:
             }
         }
 
+        return createSameFieldDisjunction(leftResult, rightResult);
+    }
+
+    /**
+     * Given two predicates, form their disjunction if we can represent the result as a conjunction
+     * of predicates on the same field. Otherwise return an empty optional.
+     *
+     * When this function returns a nonempty optional, it may modify or move from the arguments.
+     * When it returns boost::none the arguments are unchanged.
+     */
+    static ResultType createSameFieldDisjunction(ResultType& leftResult, ResultType& rightResult) {
+        auto& leftReqMap = leftResult->_reqMap;
+        auto& rightReqMap = rightResult->_reqMap;
         auto leftEntries = leftReqMap.conjuncts();
         auto rightEntries = rightReqMap.conjuncts();
         auto leftEntry = leftEntries.begin();
@@ -355,9 +379,8 @@ public:
         if (leftKey._projectionName != rightKey._projectionName) {
             return {};
         }
-        if (leftReq.getBoundProjectionName() || rightReq.getBoundProjectionName()) {
-            return {};
-        }
+        tassert(7453902, "Unexpected binding in ComposeA", !leftReq.getBoundProjectionName());
+        tassert(7453901, "Unexpected binding in ComposeA", !rightReq.getBoundProjectionName());
 
         auto& leftIntervals = leftReq.getIntervals();
         auto& rightIntervals = rightReq.getIntervals();
@@ -670,9 +693,16 @@ boost::optional<PartialSchemaReqConversion> convertExprToPartialSchemaReq(
         return {};
     }
 
-    for (const auto& [key, req] : reqMap.conjuncts()) {
-        if (key._path.is<PathIdentity>() && isIntervalReqFullyOpenDNF(req.getIntervals())) {
-            // We need to determine either path or interval (or both).
+    // We need to determine either path or interval (or both).
+    {
+        bool trivialAtom = false;
+        PSRExpr::visitAnyShape(reqMap.getRoot(), [&](PartialSchemaEntry& entry) {
+            auto&& [key, req] = entry;
+            if (key._path.is<PathIdentity>() && isIntervalReqFullyOpenDNF(req.getIntervals())) {
+                trivialAtom = true;
+            }
+        });
+        if (trivialAtom) {
             return {};
         }
     }
@@ -694,10 +724,6 @@ bool simplifyPartialSchemaReqPaths(const boost::optional<ProjectionName>& scanPr
                                    PartialSchemaRequirements& reqMap,
                                    ProjectionRenames& projectionRenames,
                                    const ConstFoldFn& constFold) {
-    PSRExpr::Builder resultReqs;
-    resultReqs.pushDisj().pushConj();
-    boost::optional<std::pair<PartialSchemaKey, PartialSchemaRequirement>> prevEntry;
-
     const auto simplifyFn = [&constFold](IntervalReqExpr::Node& intervals) -> bool {
         normalizeIntervals(intervals);
         auto simplified = simplifyDNFIntervals(intervals, constFold);
@@ -707,66 +733,87 @@ bool simplifyPartialSchemaReqPaths(const boost::optional<ProjectionName>& scanPr
         return simplified.has_value();
     };
 
-    const auto nextEntryFn = [&](PartialSchemaKey newKey, const PartialSchemaRequirement& req) {
-        resultReqs.atom(std::move(prevEntry->first), std::move(prevEntry->second));
-        prevEntry.reset({std::move(newKey), req});
-    };
-
-    // Simplify paths by eliminating unnecessary Traverse elements.
-    for (const auto& [key, req] : reqMap.conjuncts()) {
-        PartialSchemaKey newKey = key;
+    PSRExpr::Builder resultReqs;
+    resultReqs.pushDisj();
 
-        bool simplified = false;
-        const bool containedTraverse = checkPathContainsTraverse(newKey._path);
-        if (key._projectionName == scanProjName && containedTraverse) {
-            simplified = simplifyTraverseNonArray(newKey._path, multikeynessTrie);
-        }
-        // At this point we have simplified the path in newKey.
+    PSRExpr::visitDisjuncts(reqMap.getRoot(), [&](const PSRExpr::Node& disjunct, size_t) {
+        resultReqs.pushConj();
+        boost::optional<std::pair<PartialSchemaKey, PartialSchemaRequirement>> prevEntry;
 
-        if (!prevEntry) {
+        const auto nextEntryFn = [&](PartialSchemaKey newKey, const PartialSchemaRequirement& req) {
+            resultReqs.atom(std::move(prevEntry->first), std::move(prevEntry->second));
             prevEntry.reset({std::move(newKey), req});
-            continue;
-        }
-        if (prevEntry->first != newKey) {
-            nextEntryFn(std::move(newKey), req);
-            continue;
-        }
+        };
 
-        auto& prevReq = prevEntry->second;
-        auto resultIntervals = prevReq.getIntervals();
-        combineIntervalsDNF(true /*intersect*/, resultIntervals, req.getIntervals());
+        // Simplify paths by eliminating unnecessary Traverse elements.
 
-        // Ensure that Traverse-less keys appear only once: we can move the conjunction into the
-        // intervals and simplify. For traversing keys, check if interval is subsumed in the other
-        // and if so, then combine.
-        if (containedTraverse && !simplified &&
-            !(resultIntervals == prevReq.getIntervals() || resultIntervals == req.getIntervals())) {
-            // We cannot combine multikey paths where one interval does not subsume the other.
-            nextEntryFn(std::move(newKey), req);
-            continue;
-        }
+        PSRExpr::visitConjuncts(disjunct, [&](const PSRExpr::Node conjunct, size_t) {
+            const auto& [key, req] = conjunct.cast<PSRExpr::Atom>()->getExpr();
 
-        auto resultBoundProjName = prevReq.getBoundProjectionName();
-        if (const auto& boundProjName = req.getBoundProjectionName()) {
-            if (resultBoundProjName) {
-                // The existing name wins (stays in 'reqMap'). We tell the caller that the name
-                // "boundProjName" is available under "resultBoundProjName".
-                projectionRenames.emplace(*boundProjName, *resultBoundProjName);
-            } else {
-                resultBoundProjName = boundProjName;
+            PartialSchemaKey newKey = key;
+
+            bool simplified = false;
+            const bool containedTraverse = checkPathContainsTraverse(newKey._path);
+            if (key._projectionName == scanProjName && containedTraverse) {
+                simplified = simplifyTraverseNonArray(newKey._path, multikeynessTrie);
             }
-        }
+            // At this point we have simplified the path in newKey.
 
-        if (constFold && !simplifyFn(resultIntervals)) {
-            return true;
+            if (!prevEntry) {
+                prevEntry.reset({std::move(newKey), req});
+                return;
+            }
+            if (prevEntry->first != newKey) {
+                nextEntryFn(std::move(newKey), req);
+                return;
+            }
+
+            auto& prevReq = prevEntry->second;
+            auto resultIntervals = prevReq.getIntervals();
+            combineIntervalsDNF(true /*intersect*/, resultIntervals, req.getIntervals());
+
+            // Ensure that Traverse-less keys appear only once: we can move the conjunction into the
+            // intervals and simplify. For traversing keys, check if interval is subsumed in the
+            // other and if so, then combine.
+            if (containedTraverse && !simplified &&
+                !(resultIntervals == prevReq.getIntervals() ||
+                  resultIntervals == req.getIntervals())) {
+                // We cannot combine multikey paths where one interval does not subsume the other.
+                nextEntryFn(std::move(newKey), req);
+                return;
+            }
+
+            auto resultBoundProjName = prevReq.getBoundProjectionName();
+            if (const auto& boundProjName = req.getBoundProjectionName()) {
+                if (resultBoundProjName) {
+                    // The existing name wins (stays in 'reqMap'). We tell the caller that the name
+                    // "boundProjName" is available under "resultBoundProjName".
+                    projectionRenames.emplace(*boundProjName, *resultBoundProjName);
+                } else {
+                    resultBoundProjName = boundProjName;
+                }
+            }
+
+            if (constFold && !simplifyFn(resultIntervals)) {
+                // TODO SERVER-73827 Consider having the BoolExpr builder handle simplifying away
+                // trivial (always-true or always-false) clauses.
+
+                // An always-false conjunct means the whole conjunction is always-false.
+                // However, there can be other disjuncts, so we can't short-circuit the whole tree.
+                // Create an explicit always-false atom.
+                resultIntervals = IntervalReqExpr::makeSingularDNF(
+                    BoundRequirement::makePlusInf(), BoundRequirement::makeMinusInf());
+            }
+            prevReq = {std::move(resultBoundProjName),
+                       std::move(resultIntervals),
+                       req.getIsPerfOnly() && prevReq.getIsPerfOnly()};
+        });
+        if (prevEntry) {
+            resultReqs.atom(std::move(prevEntry->first), std::move(prevEntry->second));
         }
-        prevReq = {std::move(resultBoundProjName),
-                   std::move(resultIntervals),
-                   req.getIsPerfOnly() && prevReq.getIsPerfOnly()};
-    }
-    if (prevEntry) {
-        resultReqs.atom(std::move(prevEntry->first), std::move(prevEntry->second));
-    }
+
+        resultReqs.pop();
+    });
 
     PartialSchemaRequirements newReqs{std::move(*resultReqs.finish())};
 
@@ -1226,6 +1273,11 @@ boost::optional<ScanParams> computeScanParams(PrefixId& prefixId,
     auto& residualReqs = result._residualRequirements;
     auto& fieldProjMap = result._fieldProjectionMap;
 
+    // Expect a DNF with one disjunct; bail out if we have a nontrivial disjunction.
+    if (!PSRExpr::isSingletonDisjunction(reqMap.getRoot())) {
+        return {};
+    }
+
     size_t entryIndex = 0;
     for (const auto& [key, req] : reqMap.conjuncts()) {
         if (req.getIsPerfOnly()) {
diff --git a/src/mongo/db/query/optimizer/utils/utils.h b/src/mongo/db/query/optimizer/utils/utils.h
index 999f18b3c9d..ed0ee68dce9 100644
--- a/src/mongo/db/query/optimizer/utils/utils.h
+++ b/src/mongo/db/query/optimizer/utils/utils.h
@@ -244,8 +244,8 @@ boost::optional<PartialSchemaReqConversion> convertExprToPartialSchemaReq(
  * Schema Requirement structure. Following that the intervals of any remaining non-multikey paths
  * (following simplification) on the same key are intersected. Intervals of multikey paths are
  * checked for subsumption and if one subsumes the other, the subsuming one is retained. Returns
- * true if we have an empty result after simplification. Each redundant binding gets an entry in
- * 'projectionRenames', which maps redundant name to the de-duplicated name.
+ * true if we have an always-false predicate after simplification. Each redundant binding gets an
+ * entry in 'projectionRenames', which maps redundant name to the de-duplicated name.
  */
 [[nodiscard]] bool simplifyPartialSchemaReqPaths(
     const boost::optional<ProjectionName>& scanProjName,