SERVER-39567 Change find min/max options to require hint

13 files changed, 134 insertions, 153 deletions

diff --git a/src/mongo/db/exec/cached_plan.cpp b/src/mongo/db/exec/cached_plan.cpp
index b824b0a9e66..e426a0ad340 100644
--- a/src/mongo/db/exec/cached_plan.cpp
+++ b/src/mongo/db/exec/cached_plan.cpp
@@ -198,10 +198,9 @@ Status CachedPlanStage::replan(PlanYieldPolicy* yieldPolicy, bool shouldCache) {
     // Use the query planning module to plan the whole query.
     auto statusWithSolutions = QueryPlanner::plan(*_canonicalQuery, _plannerParams);
     if (!statusWithSolutions.isOK()) {
-        return Status(ErrorCodes::BadValue,
-                      str::stream() << "error processing query: " << _canonicalQuery->toString()
-                                    << " planner returned error: "
-                                    << statusWithSolutions.getStatus().reason());
+        return statusWithSolutions.getStatus().withContext(
+            str::stream() << "error processing query: " << _canonicalQuery->toString()
+                          << " planner returned error");
     }
 
     auto solutions = std::move(statusWithSolutions.getValue());
diff --git a/src/mongo/db/exec/subplan.cpp b/src/mongo/db/exec/subplan.cpp
index dade23b7b5e..168fc342d3d 100644
--- a/src/mongo/db/exec/subplan.cpp
+++ b/src/mongo/db/exec/subplan.cpp
@@ -372,9 +372,9 @@ Status SubplanStage::choosePlanWholeQuery(PlanYieldPolicy* yieldPolicy) {
     // Use the query planning module to plan the whole query.
     auto statusWithSolutions = QueryPlanner::plan(*_query, _plannerParams);
     if (!statusWithSolutions.isOK()) {
-        return Status(ErrorCodes::BadValue,
-                      "error processing query: " + _query->toString() +
-                          " planner returned error: " + statusWithSolutions.getStatus().reason());
+        return statusWithSolutions.getStatus().withContext(
+            str::stream() << "error processing query: " << _query->toString()
+                          << " planner returned error");
     }
 
     auto solutions = std::move(statusWithSolutions.getValue());
diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp
index 727f6d52449..339074d502e 100644
--- a/src/mongo/db/query/get_executor.cpp
+++ b/src/mongo/db/query/get_executor.cpp
@@ -469,9 +469,9 @@ StatusWith<PrepareExecutionResult> prepareExecution(OperationContext* opCtx,
 
     auto statusWithSolutions = QueryPlanner::plan(*canonicalQuery, plannerParams);
     if (!statusWithSolutions.isOK()) {
-        return Status(ErrorCodes::BadValue,
-                      "error processing query: " + canonicalQuery->toString() +
-                          " planner returned error: " + statusWithSolutions.getStatus().reason());
+        return statusWithSolutions.getStatus().withContext(
+            str::stream() << "error processing query: " << canonicalQuery->toString()
+                          << " planner returned error");
     }
     auto solutions = std::move(statusWithSolutions.getValue());
 
diff --git a/src/mongo/db/query/index_entry.h b/src/mongo/db/query/index_entry.h
index 89b14ffd609..ede83b8e700 100644
--- a/src/mongo/db/query/index_entry.h
+++ b/src/mongo/db/query/index_entry.h
@@ -71,6 +71,8 @@ struct CoreIndexInfo {
         invariant((type == IndexType::INDEX_WILDCARD) == (projExec != nullptr));
     }
 
+    virtual ~CoreIndexInfo() = default;
+
     /**
      * This struct is used to uniquely identify an index. The index "Identifier" has two
      * components: catalog name, and "disambiguator". The catalog name is just the name of the
@@ -164,6 +166,12 @@ struct IndexEntry : CoreIndexInfo {
         invariant(multikeyPaths.empty() || multikeyPathSet.empty());
     }
 
+    IndexEntry(const IndexEntry&) = default;
+    IndexEntry(IndexEntry&&) = default;
+
+    IndexEntry& operator=(const IndexEntry&) = default;
+    IndexEntry& operator=(IndexEntry&&) = default;
+
     ~IndexEntry() {
         // An IndexEntry should never have both formats of multikey metadata simultaneously.
         invariant(multikeyPaths.empty() || multikeyPathSet.empty());
diff --git a/src/mongo/db/query/plan_cache_test.cpp b/src/mongo/db/query/plan_cache_test.cpp
index 6cf50b688a1..5614137b90a 100644
--- a/src/mongo/db/query/plan_cache_test.cpp
+++ b/src/mongo/db/query/plan_cache_test.cpp
@@ -394,7 +394,8 @@ TEST(PlanCacheTest, ShouldNotCacheQueryWithHint) {
  * Min queries are a specialized case of hinted queries
  */
 TEST(PlanCacheTest, ShouldNotCacheQueryWithMin) {
-    unique_ptr<CanonicalQuery> cq(canonicalize("{a: 1}", "{}", "{}", 0, 0, "{}", "{a: 100}", "{}"));
+    unique_ptr<CanonicalQuery> cq(
+        canonicalize("{a: 1}", "{}", "{}", 0, 0, "{a: 1}", "{a: 100}", "{}"));
     assertShouldNotCacheQuery(*cq);
 }
 
@@ -402,7 +403,8 @@ TEST(PlanCacheTest, ShouldNotCacheQueryWithMin) {
  *  Max queries are non-cacheable for the same reasons as min queries.
  */
 TEST(PlanCacheTest, ShouldNotCacheQueryWithMax) {
-    unique_ptr<CanonicalQuery> cq(canonicalize("{a: 1}", "{}", "{}", 0, 0, "{}", "{}", "{a: 100}"));
+    unique_ptr<CanonicalQuery> cq(
+        canonicalize("{a: 1}", "{}", "{}", 0, 0, "{a: 1}", "{}", "{a: 100}"));
     assertShouldNotCacheQuery(*cq);
 }
 
@@ -1659,7 +1661,7 @@ TEST_F(CachePlanSelectionTest, GeoNear2DNotCached) {
 
 TEST_F(CachePlanSelectionTest, MinNotCached) {
     addIndex(BSON("a" << 1), "a_1");
-    runQueryHintMinMax(BSONObj(), BSONObj(), fromjson("{a: 1}"), BSONObj());
+    runQueryHintMinMax(BSONObj(), fromjson("{a: 1}"), fromjson("{a: 1}"), BSONObj());
     assertNotCached(
         "{fetch: {filter: null, "
         "node: {ixscan: {filter: null, pattern: {a: 1}}}}}");
@@ -1667,7 +1669,7 @@ TEST_F(CachePlanSelectionTest, MinNotCached) {
 
 TEST_F(CachePlanSelectionTest, MaxNotCached) {
     addIndex(BSON("a" << 1), "a_1");
-    runQueryHintMinMax(BSONObj(), BSONObj(), BSONObj(), fromjson("{a: 1}"));
+    runQueryHintMinMax(BSONObj(), fromjson("{a: 1}"), BSONObj(), fromjson("{a: 1}"));
     assertNotCached(
         "{fetch: {filter: null, "
         "node: {ixscan: {filter: null, pattern: {a: 1}}}}}");
diff --git a/src/mongo/db/query/planner_ixselect.cpp b/src/mongo/db/query/planner_ixselect.cpp
index 1f314787fdb..77f662a4701 100644
--- a/src/mongo/db/query/planner_ixselect.cpp
+++ b/src/mongo/db/query/planner_ixselect.cpp
@@ -321,14 +321,13 @@ std::vector<IndexEntry> QueryPlannerIXSelect::findRelevantIndices(
 }
 
 std::vector<IndexEntry> QueryPlannerIXSelect::expandIndexes(
-    const stdx::unordered_set<std::string>& fields,
-    const std::vector<IndexEntry>& relevantIndices) {
+    const stdx::unordered_set<std::string>& fields, std::vector<IndexEntry> relevantIndices) {
     std::vector<IndexEntry> out;
     for (auto&& entry : relevantIndices) {
         if (entry.type == IndexType::INDEX_WILDCARD) {
             wcp::expandWildcardIndexEntry(entry, fields, &out);
         } else {
-            out.push_back(entry);
+            out.push_back(std::move(entry));
         }
     }
 
diff --git a/src/mongo/db/query/planner_ixselect.h b/src/mongo/db/query/planner_ixselect.h
index b4269f9b64d..1b7fed5eed0 100644
--- a/src/mongo/db/query/planner_ixselect.h
+++ b/src/mongo/db/query/planner_ixselect.h
@@ -133,7 +133,7 @@ public:
      * "expanded" indexes (where the $** indexes in the given list have been expanded).
      */
     static std::vector<IndexEntry> expandIndexes(const stdx::unordered_set<std::string>& fields,
-                                                 const std::vector<IndexEntry>& relevantIndices);
+                                                 std::vector<IndexEntry> relevantIndices);
 
     /**
      * Check if this match expression is a leaf and is supported by a wildcard index.
diff --git a/src/mongo/db/query/query_planner.cpp b/src/mongo/db/query/query_planner.cpp
index fb819ce9c36..655a6816194 100644
--- a/src/mongo/db/query/query_planner.cpp
+++ b/src/mongo/db/query/query_planner.cpp
@@ -151,6 +151,7 @@ static BSONObj getKeyFromQuery(const BSONObj& keyPattern, const BSONObj& query)
 static bool indexCompatibleMaxMin(const BSONObj& obj,
                                   const CollatorInterface* queryCollator,
                                   const IndexEntry& indexEntry) {
+    // Wildcard indexes should have been filtered out by the time this is called.
     if (indexEntry.type == IndexType::INDEX_WILDCARD) {
         return false;
     }
@@ -596,6 +597,8 @@ StatusWith<std::vector<std::unique_ptr<QuerySolution>>> QueryPlanner::plan(
     // indexes.
     std::vector<IndexEntry> fullIndexList;
 
+    // Will hold a copy of the index entry chosen by the hint.
+    boost::optional<IndexEntry> hintedIndexEntry;
     if (hintedIndex.isEmpty()) {
         fullIndexList = params.indices;
     } else {
@@ -613,6 +616,8 @@ StatusWith<std::vector<std::unique_ptr<QuerySolution>>> QueryPlanner::plan(
                                         << " and "
                                         << fullIndexList[1].toString());
         }
+
+        hintedIndexEntry.emplace(fullIndexList.front());
     }
 
     // Figure out what fields we care about.
@@ -625,7 +630,7 @@ StatusWith<std::vector<std::unique_ptr<QuerySolution>>> QueryPlanner::plan(
     fullIndexList = QueryPlannerIXSelect::expandIndexes(fields, std::move(fullIndexList));
     std::vector<IndexEntry> relevantIndices;
 
-    if (hintedIndex.isEmpty()) {
+    if (!hintedIndexEntry) {
         relevantIndices = QueryPlannerIXSelect::findRelevantIndices(fields, fullIndexList);
     } else {
         relevantIndices = fullIndexList;
@@ -639,60 +644,45 @@ StatusWith<std::vector<std::unique_ptr<QuerySolution>>> QueryPlanner::plan(
         }
     }
 
-    // TODO SERVER-35335 Ensure min/max can generate bounds over $** index.
     // Deal with the .min() and .max() query options.  If either exist we can only use an index
     // that matches the object inside.
     if (!query.getQueryRequest().getMin().isEmpty() ||
         !query.getQueryRequest().getMax().isEmpty()) {
+
+        if (!hintedIndexEntry) {
+            return Status(ErrorCodes::Error(51173),
+                          "When using min()/max() a hint of which index to use must be provided");
+        }
+
         BSONObj minObj = query.getQueryRequest().getMin();
         BSONObj maxObj = query.getQueryRequest().getMax();
 
-        // The unfinished siblings of these objects may not be proper index keys because they
-        // may be empty objects or have field names. When an index is picked to use for the
-        // min/max query, these "finished" objects will always be valid index keys for the
-        // index's key pattern.
-        BSONObj finishedMinObj;
-        BSONObj finishedMaxObj;
-
-        // Index into the 'fulledIndexList' vector indicating the index that we will use to answer
-        // this min/max query.
-        size_t idxNo = numeric_limits<size_t>::max();
-
-        for (size_t i = 0; i < fullIndexList.size(); ++i) {
-            const auto& indexEntry = fullIndexList[i];
-
-            const BSONObj toUse = minObj.isEmpty() ? maxObj : minObj;
-            if (indexCompatibleMaxMin(toUse, query.getCollator(), indexEntry)) {
-                // In order to be fully compatible, the min has to be less than the max
-                // according to the index key pattern ordering. The first step in verifying
-                // this is "finish" the min and max by replacing empty objects and stripping
-                // field names.
-                finishedMinObj = finishMinObj(indexEntry, minObj, maxObj);
-                finishedMaxObj = finishMaxObj(indexEntry, minObj, maxObj);
-
-                // Now we have the final min and max. This index is only relevant for
-                // the min/max query if min < max.
-                if (0 > finishedMinObj.woCompare(finishedMaxObj, indexEntry.keyPattern, false)) {
-                    // Found a relevant index.
-                    idxNo = i;
-                    break;
-                }
-
-                // This index is not relevant; move on to the next.
-            }
+        if (!indexCompatibleMaxMin(
+                minObj.isEmpty() ? maxObj : minObj, query.getCollator(), *hintedIndexEntry)) {
+            return Status(ErrorCodes::Error(51174),
+                          "The index chosen is not compatible with min/max");
         }
+        // Be sure that index expansion didn't do anything. As wildcard indexes are banned for
+        // min/max, we expect to find a single hinted index entry.
+        invariant(fullIndexList.size() == 1);
+        invariant(*hintedIndexEntry == fullIndexList.front());
 
-        if (idxNo == numeric_limits<size_t>::max()) {
-            LOG(5) << "Can't find relevant index to use for max/min query";
-            // Can't find an index to use, bail out.
-            return Status(ErrorCodes::BadValue, "unable to find relevant index for max/min query");
-        }
+        // In order to be fully compatible, the min has to be less than the max according to the
+        // index key pattern ordering. The first step in verifying this is "finish" the min and max
+        // by replacing empty objects and stripping field names.
+        BSONObj finishedMinObj = finishMinObj(*hintedIndexEntry, minObj, maxObj);
+        BSONObj finishedMaxObj = finishMaxObj(*hintedIndexEntry, minObj, maxObj);
 
-        LOG(5) << "Max/min query using index " << fullIndexList[idxNo].toString();
+        // Now we have the final min and max. This index is only relevant for the min/max query if
+        // min < max.
+        if (finishedMinObj.woCompare(finishedMaxObj, hintedIndexEntry->keyPattern, false) >= 0) {
+            return Status(ErrorCodes::Error(51175),
+                          "The value provided for min() does not come before the value provided "
+                          "for max() in the hinted index");
+        }
 
-        // Make our scan and output.
         std::unique_ptr<QuerySolutionNode> solnRoot(QueryPlannerAccess::makeIndexScan(
-            fullIndexList[idxNo], query, params, finishedMinObj, finishedMaxObj));
+            *hintedIndexEntry, query, params, finishedMinObj, finishedMaxObj));
         invariant(solnRoot);
 
         auto soln = QueryPlannerAnalysis::analyzeDataAccess(query, params, std::move(solnRoot));
diff --git a/src/mongo/db/query/query_planner_collation_test.cpp b/src/mongo/db/query/query_planner_collation_test.cpp
index 9606b62f4a0..5cefee957c1 100644
--- a/src/mongo/db/query/query_planner_collation_test.cpp
+++ b/src/mongo/db/query/query_planner_collation_test.cpp
@@ -211,9 +211,9 @@ TEST_F(QueryPlannerTest, MinMaxWithStringBoundsCannotBeCoveredWithCollator) {
     CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
     addIndex(fromjson("{a: 1, b: 1}"), &collator);
 
-    runQueryAsCommand(
-        fromjson("{find: 'testns', min: {a: 1, b: 2}, max: {a: 2, b: 1}, "
-                 "projection: {_id: 0, a: 1, b: 1}, collation: {locale: 'reverse'}}"));
+    runQueryAsCommand(fromjson(
+        "{find: 'testns', min: {a: 1, b: 2}, max: {a: 2, b: 1}, "
+        "projection: {_id: 0, a: 1, b: 1}, hint: {a: 1, b: 1}, collation: {locale: 'reverse'}}"));
 
     assertNumSolutions(1U);
     assertSolutionExists(
@@ -446,7 +446,8 @@ TEST_F(QueryPlannerTest, SuccessfullyPlanWhenMinMaxHaveNumberBoundariesAndCollat
     addIndex(fromjson("{a: 1, b: 1, c: 1}"), &indexCollator);
 
     runQueryAsCommand(
-        fromjson("{find: 'testns', min: {a: 1, b: 1, c: 1}, max: {a: 3, b: 3, c: 3}}"));
+        fromjson("{find: 'testns', min: {a: 1, b: 1, c: 1}, max: {a: 3, b: 3, c: 3},"
+                 "hint: {a:1, b: 1, c: 1}}"));
 
     assertNumSolutions(1U);
     assertSolutionExists("{fetch: {node: {ixscan: {pattern: {a: 1, b: 1, c: 1}}}}}");
@@ -455,37 +456,48 @@ TEST_F(QueryPlannerTest, SuccessfullyPlanWhenMinMaxHaveNumberBoundariesAndCollat
 TEST_F(QueryPlannerTest, FailToPlanWhenMinHasStringBoundaryAndCollationsDontMatch) {
     CollatorInterfaceMock indexCollator(CollatorInterfaceMock::MockType::kToLowerString);
     addIndex(fromjson("{a: 1, b: 1, c: 1}"), &indexCollator);
-    runInvalidQueryAsCommand(fromjson("{find: 'testns', min: {a: 1, b: 'foo', c: 1}}"));
+    runInvalidQueryAsCommand(
+        fromjson("{find: 'testns', min: {a: 1, b: 'foo', c: 1}, "
+                 "hint: {a: 1, b: 1, c: 1}}"));
 }
 
 TEST_F(QueryPlannerTest, FailToPlanWhenMaxHasStringBoundaryAndCollationsDontMatch) {
     CollatorInterfaceMock indexCollator(CollatorInterfaceMock::MockType::kToLowerString);
     addIndex(fromjson("{a: 1, b: 1, c: 1}"), &indexCollator);
-    runInvalidQueryAsCommand(fromjson("{find: 'testns', max: {a: 1, b: 'foo', c: 1}}"));
+    runInvalidQueryAsCommand(
+        fromjson("{find: 'testns', max: {a: 1, b: 'foo', c: 1}, hint: {a: 1, b: 1, c: 1}}"));
 }
 
 TEST_F(QueryPlannerTest, FailToPlanWhenMinHasObjectBoundaryAndCollationsDontMatch) {
     CollatorInterfaceMock indexCollator(CollatorInterfaceMock::MockType::kToLowerString);
     addIndex(fromjson("{a: 1, b: 1, c: 1}"), &indexCollator);
-    runInvalidQueryAsCommand(fromjson("{find: 'testns', min: {a: 1, b: {d: 'foo'}, c: 1}}"));
+    runInvalidQueryAsCommand(
+        fromjson("{find: 'testns', min: {a: 1, b: {d: 'foo'}, c: 1}, "
+                 "hint: {a: 1, b: 1, c: 1}}"));
 }
 
 TEST_F(QueryPlannerTest, FailToPlanWhenMaxHasObjectBoundaryAndCollationsDontMatch) {
     CollatorInterfaceMock indexCollator(CollatorInterfaceMock::MockType::kToLowerString);
     addIndex(fromjson("{a: 1, b: 1, c: 1}"), &indexCollator);
-    runInvalidQueryAsCommand(fromjson("{find: 'testns', max: {a: 1, b: {d: 'foo'}, c: 1}}"));
+    runInvalidQueryAsCommand(
+        fromjson("{find: 'testns', max: {a: 1, b: {d: 'foo'}, c: 1}, "
+                 "hint: {a: 1, b: 1, c: 1}}"));
 }
 
 TEST_F(QueryPlannerTest, FailToPlanWhenMinHasArrayBoundaryAndCollationsDontMatch) {
     CollatorInterfaceMock indexCollator(CollatorInterfaceMock::MockType::kToLowerString);
     addIndex(fromjson("{a: 1, b: 1, c: 1}"), &indexCollator);
-    runInvalidQueryAsCommand(fromjson("{find: 'testns', min: {a: 1, b: 1, c: [1, 'foo']}}"));
+    runInvalidQueryAsCommand(
+        fromjson("{find: 'testns', min: {a: 1, b: 1, c: [1, 'foo']}, "
+                 "hint: {a: 1, b: 1, c: 1}}"));
 }
 
 TEST_F(QueryPlannerTest, FailToPlanWhenMaxHasArrayBoundaryAndCollationsDontMatch) {
     CollatorInterfaceMock indexCollator(CollatorInterfaceMock::MockType::kToLowerString);
     addIndex(fromjson("{a: 1, b: 1, c: 1}"), &indexCollator);
-    runInvalidQueryAsCommand(fromjson("{find: 'testns', max: {a: 1, b: 1, c: [1, 'foo']}}"));
+    runInvalidQueryAsCommand(
+        fromjson("{find: 'testns', max: {a: 1, b: 1, c: [1, 'foo']}, "
+                 "hint: {a: 1, b: 1, c: 1}}"));
 }
 
 TEST_F(QueryPlannerTest, FailToPlanWhenHintingIndexIncompatibleWithMinDueToCollation) {
@@ -502,35 +514,30 @@ TEST_F(QueryPlannerTest, FailToPlanWhenHintingIndexIncompatibleWithMaxDueToColla
     runInvalidQueryAsCommand(fromjson("{find: 'testns', max: {a: 'foo'}, hint: 'indexToHint'}"));
 }
 
-TEST_F(QueryPlannerTest, SelectIndexWithMatchingSimpleCollationWhenMinHasStringBoundary) {
-    CollatorInterfaceMock indexCollator(CollatorInterfaceMock::MockType::kToLowerString);
-    addIndex(fromjson("{a: 1}"), &indexCollator, "withCollation"_sd);
+TEST_F(QueryPlannerTest, SuccessWithIndexWithMatchingSimpleCollationWhenMinHasStringBoundary) {
     addIndex(fromjson("{a: 1}"), nullptr, "noCollation"_sd);
 
-    runQueryAsCommand(fromjson("{find: 'testns', min: {a: 'foo'}}"));
+    runQueryAsCommand(fromjson("{find: 'testns', min: {a: 'foo'}, hint: {a: 1}}"));
 
     assertNumSolutions(1U);
     assertSolutionExists("{fetch: {node: {ixscan: {pattern: {a: 1}, name: 'noCollation'}}}}");
 }
 
-TEST_F(QueryPlannerTest, SelectIndexWithMatchingNonSimpleCollationWhenMinHasStringBoundary) {
+TEST_F(QueryPlannerTest, SuccessWithIndexWithMatchingNonSimpleCollationWhenMinHasStringBoundary) {
     CollatorInterfaceMock indexCollator(CollatorInterfaceMock::MockType::kReverseString);
     addIndex(fromjson("{a: 1}"), &indexCollator, "withCollation"_sd);
-    addIndex(fromjson("{a: 1}"), nullptr, "noCollation"_sd);
 
-    runQueryAsCommand(
-        fromjson("{find: 'testns', min: {a: 'foo'}, collation: {locale: 'reverse'}}"));
+    runQueryAsCommand(fromjson(
+        "{find: 'testns', min: {a: 'foo'}, hint: {a: 1}, collation: {locale: 'reverse'}}"));
 
     assertNumSolutions(1U);
     assertSolutionExists("{fetch: {node: {ixscan: {pattern: {a: 1}, name: 'withCollation'}}}}");
 }
 
-TEST_F(QueryPlannerTest, SelectIndexWithMatchingSimpleCollationWhenMaxHasStringBoundary) {
-    CollatorInterfaceMock indexCollator(CollatorInterfaceMock::MockType::kToLowerString);
-    addIndex(fromjson("{a: 1}"), &indexCollator, "withCollation"_sd);
+TEST_F(QueryPlannerTest, SuccessWithIndexWithMatchingSimpleCollationWhenMaxHasStringBoundary) {
     addIndex(fromjson("{a: 1}"), nullptr, "noCollation"_sd);
 
-    runQueryAsCommand(fromjson("{find: 'testns', max: {a: 'foo'}}"));
+    runQueryAsCommand(fromjson("{find: 'testns', max: {a: 'foo'}, hint: {a: 1}}"));
 
     assertNumSolutions(1U);
     assertSolutionExists("{fetch: {node: {ixscan: {pattern: {a: 1}, name: 'noCollation'}}}}");
@@ -540,8 +547,8 @@ TEST_F(QueryPlannerTest, MustSortInMemoryWhenMinMaxQueryHasCollationAndIndexDoes
     addIndex(fromjson("{a: 1, b: 1}"));
 
     runQueryAsCommand(
-        fromjson("{find: 'testns', min: {a: 1, b: 1}, max: {a: 2, b: 1}, collation: {locale: "
-                 "'reverse'}, sort: {a: 1, b: 1}}"));
+        fromjson("{find: 'testns', min: {a: 1, b: 1}, max: {a: 2, b: 1}, hint: {a:1, b:1}, "
+                 "collation: {locale: 'reverse'}, sort: {a: 1, b: 1}}"));
 
     assertNumSolutions(1U);
 
@@ -555,7 +562,8 @@ TEST_F(QueryPlannerTest, MustSortInMemoryWhenMinMaxIndexHasCollationAndQueryDoes
     addIndex(fromjson("{a: 1, b:1}"), &collator);
 
     runQueryAsCommand(
-        fromjson("{find: 'testns', min: {a: 1, b: 1}, max: {a: 2, b: 1}, sort: {a: 1, b: 1}}"));
+        fromjson("{find: 'testns', min: {a: 1, b: 1}, max: {a: 2, b: 1}, sort: {a: 1, b: 1}, "
+                 "hint: {a: 1, b: 1}}"));
 
     assertNumSolutions(1U);
 
@@ -569,7 +577,7 @@ TEST_F(QueryPlannerTest, CanProduceCoveredSortPlanWhenQueryHasCollationButIndexD
 
     runQueryAsCommand(fromjson(
         "{find: 'testns', projection: {a: 1, b: 1, _id: 0}, min: {a: 1, b: 1}, max: {a: 2, "
-        "b: 1}, collation: {locale: 'reverse'}, sort: {a: 1, b: 1}}"));
+        "b: 1}, hint: {a: 1, b: 1}, collation: {locale: 'reverse'}, sort: {a: 1, b: 1}}"));
 
     assertNumSolutions(1U);
     assertSolutionExists(
@@ -631,8 +639,9 @@ TEST_F(QueryPlannerTest, NoSortStageWhenMinMaxIndexCollationDoesNotMatchButBound
     addIndex(fromjson("{a: 1, b: 1, c: 1}"));
 
     runQueryAsCommand(
-        fromjson("{find: 'testns', min: {a: 1, b: 8, c: 1}, max: {a: 1, b: 8, c: 100}, collation: "
-                 "{locale: 'reverse'}, sort: {a: 1, b: 1, c: 1}}"));
+        fromjson("{find: 'testns', min: {a: 1, b: 8, c: 1}, max: {a: 1, b: 8, c: 100}, "
+                 "hint: {a: 1, b: 1, c: 1}, collation: {locale: 'reverse'}, "
+                 "sort: {a: 1, b: 1, c: 1}}"));
 
     assertNumSolutions(1U);
     assertSolutionExists("{fetch: {node: {ixscan: {pattern: {a: 1, b: 1, c: 1}}}}}");
diff --git a/src/mongo/db/query/query_planner_test.cpp b/src/mongo/db/query/query_planner_test.cpp
index fe55792abfe..3c38b272fd1 100644
--- a/src/mongo/db/query/query_planner_test.cpp
+++ b/src/mongo/db/query/query_planner_test.cpp
@@ -1358,7 +1358,7 @@ TEST_F(QueryPlannerTest, OrWithExactAndInexact3) {
 
 TEST_F(QueryPlannerTest, MinValid) {
     addIndex(BSON("a" << 1));
-    runQueryHintMinMax(BSONObj(), BSONObj(), fromjson("{a: 1}"), BSONObj());
+    runQueryHintMinMax(BSONObj(), BSONObj(fromjson("{a: 1}")), fromjson("{a: 1}"), BSONObj());
 
     assertNumSolutions(1U);
     assertSolutionExists(
@@ -1367,7 +1367,8 @@ TEST_F(QueryPlannerTest, MinValid) {
 }
 
 TEST_F(QueryPlannerTest, MinWithoutIndex) {
-    runInvalidQueryHintMinMax(BSONObj(), BSONObj(), fromjson("{a: 1}"), BSONObj());
+    runInvalidQueryHintMinMax(
+        BSONObj(), BSONObj(fromjson("{a: 1}")), fromjson("{a: 1}"), BSONObj());
 }
 
 TEST_F(QueryPlannerTest, MinBadHint) {
@@ -1377,7 +1378,7 @@ TEST_F(QueryPlannerTest, MinBadHint) {
 
 TEST_F(QueryPlannerTest, MaxValid) {
     addIndex(BSON("a" << 1));
-    runQueryHintMinMax(BSONObj(), BSONObj(), BSONObj(), fromjson("{a: 1}"));
+    runQueryHintMinMax(BSONObj(), BSONObj(fromjson("{a: 1}")), BSONObj(), fromjson("{a: 1}"));
 
     assertNumSolutions(1U);
     assertSolutionExists(
@@ -1387,11 +1388,13 @@ TEST_F(QueryPlannerTest, MaxValid) {
 
 TEST_F(QueryPlannerTest, MinMaxSameValue) {
     addIndex(BSON("a" << 1));
-    runInvalidQueryHintMinMax(BSONObj(), BSONObj(), fromjson("{a: 1}"), fromjson("{a: 1}"));
+    runInvalidQueryHintMinMax(
+        BSONObj(), BSONObj(fromjson("{a: 1}")), fromjson("{a: 1}"), fromjson("{a: 1}"));
 }
 
 TEST_F(QueryPlannerTest, MaxWithoutIndex) {
-    runInvalidQueryHintMinMax(BSONObj(), BSONObj(), BSONObj(), fromjson("{a: 1}"));
+    runInvalidQueryHintMinMax(
+        BSONObj(), BSONObj(fromjson("{a: 1}")), BSONObj(), fromjson("{a: 1}"));
 }
 
 TEST_F(QueryPlannerTest, MaxBadHint) {
@@ -1408,7 +1411,7 @@ TEST_F(QueryPlannerTest, MaxMinSort) {
                  BSONObj(),
                  0,
                  0,
-                 BSONObj(),
+                 fromjson("{a: 1}"),
                  fromjson("{a: 2}"),
                  fromjson("{a: 8}"));
 
@@ -1425,7 +1428,7 @@ TEST_F(QueryPlannerTest, MaxMinSortEqualityFirstSortSecond) {
                  BSONObj(),
                  0,
                  0,
-                 BSONObj(),
+                 fromjson("{a: 1, b: 1}"),
                  fromjson("{a: 1, b: 1}"),
                  fromjson("{a: 1, b: 2}"));
 
@@ -1442,7 +1445,7 @@ TEST_F(QueryPlannerTest, MaxMinSortInequalityFirstSortSecond) {
                  BSONObj(),
                  0,
                  0,
-                 BSONObj(),
+                 fromjson("{a: 1, b: 1}"),
                  fromjson("{a: 1, b: 1}"),
                  fromjson("{a: 2, b: 2}"));
 
@@ -1461,7 +1464,7 @@ TEST_F(QueryPlannerTest, MaxMinReverseSort) {
                  BSONObj(),
                  0,
                  0,
-                 BSONObj(),
+                 fromjson("{a: 1}"),
                  fromjson("{a: 2}"),
                  fromjson("{a: 8}"));
 
@@ -1478,7 +1481,7 @@ TEST_F(QueryPlannerTest, MaxMinReverseIndexDir) {
                  BSONObj(),
                  0,
                  0,
-                 BSONObj(),
+                 fromjson("{a: -1}"),
                  fromjson("{a: 8}"),
                  fromjson("{a: 2}"));
 
@@ -1496,7 +1499,7 @@ TEST_F(QueryPlannerTest, MaxMinReverseIndexDirSort) {
                  BSONObj(),
                  0,
                  0,
-                 BSONObj(),
+                 fromjson("{a: -1}"),
                  fromjson("{a: 8}"),
                  fromjson("{a: 2}"));
 
@@ -1508,34 +1511,13 @@ TEST_F(QueryPlannerTest, MaxMinReverseIndexDirSort) {
 
 TEST_F(QueryPlannerTest, MaxMinNoMatchingIndexDir) {
     addIndex(BSON("a" << -1));
-    runInvalidQueryHintMinMax(BSONObj(), fromjson("{a: 2}"), BSONObj(), fromjson("{a: 8}"));
+    runInvalidQueryHintMinMax(
+        BSONObj(), fromjson("{a: 1}"), BSONObj(fromjson("{a: 2}")), fromjson("{a: 8}"));
 }
 
-TEST_F(QueryPlannerTest, MaxMinSelectCorrectlyOrderedIndex) {
+TEST_F(QueryPlannerTest, MaxMinBadHintIfIndexOrderDoesNotMatch) {
     // There are both ascending and descending indices on 'a'.
     addIndex(BSON("a" << 1));
-    addIndex(BSON("a" << -1));
-
-    // The ordering of min and max means that we *must* use the descending index.
-    runQueryFull(
-        BSONObj(), BSONObj(), BSONObj(), 0, 0, BSONObj(), fromjson("{a: 8}"), fromjson("{a: 2}"));
-
-    assertNumSolutions(1);
-    assertSolutionExists("{fetch: {node: {ixscan: {filter: null, dir: 1, pattern: {a: -1}}}}}");
-
-    // If we switch the ordering, then we use the ascending index.
-    // The ordering of min and max means that we *must* use the descending index.
-    runQueryFull(
-        BSONObj(), BSONObj(), BSONObj(), 0, 0, BSONObj(), fromjson("{a: 2}"), fromjson("{a: 8}"));
-
-    assertNumSolutions(1);
-    assertSolutionExists("{fetch: {node: {ixscan: {filter: null, dir: 1, pattern: {a: 1}}}}}");
-}
-
-TEST_F(QueryPlannerTest, MaxMinBadHintSelectsReverseIndex) {
-    // There are both ascending and descending indices on 'a'.
-    addIndex(BSON("a" << 1));
-    addIndex(BSON("a" << -1));
 
     // A query hinting on {a: 1} is bad if min is {a: 8} and {a: 2} because this
     // min/max pairing requires a descending index.
diff --git a/src/mongo/db/query/query_planner_wildcard_index_test.cpp b/src/mongo/db/query/query_planner_wildcard_index_test.cpp
index a4841718b28..d0fd0def30e 100644
--- a/src/mongo/db/query/query_planner_wildcard_index_test.cpp
+++ b/src/mongo/db/query/query_planner_wildcard_index_test.cpp
@@ -1691,12 +1691,14 @@ TEST_F(QueryPlannerWildcardTest, PathSpecifiedWildcardIndexDoesNotSupportMinMax)
         fromjson("{x: {$eq: 5}}"), BSON("x.$**" << 1), fromjson("{x: 1}"), fromjson("{x: 10}"));
 }
 
-TEST_F(QueryPlannerWildcardTest, WildcardIndexDoesNotEffectValidMinMax) {
+TEST_F(QueryPlannerWildcardTest, WildcardIndexDoesNotAffectValidMinMax) {
     addWildcardIndex(BSON("$**" << 1));
     addIndex(BSON("x" << 1));
 
-    runQueryHintMinMax(
-        fromjson("{x: {$eq: 5}}"), BSONObj(), fromjson("{x: 1}"), fromjson("{x: 10}"));
+    runQueryHintMinMax(fromjson("{x: {$eq: 5}}"),
+                       BSONObj(fromjson("{x: 1}")),
+                       fromjson("{x: 1}"),
+                       fromjson("{x: 10}"));
 
     assertNumSolutions(1U);
     assertSolutionExists(
@@ -1704,13 +1706,6 @@ TEST_F(QueryPlannerWildcardTest, WildcardIndexDoesNotEffectValidMinMax) {
         "node: {ixscan: {filter: null, pattern: {x: 1}}}}}");
 }
 
-TEST_F(QueryPlannerWildcardTest, WildcardIndexDoesNotSupportMinMaxWithoutHint) {
-    addWildcardIndex(BSON("$**" << 1));
-
-    runInvalidQueryHintMinMax(
-        fromjson("{x: {$eq: 5}}"), BSONObj(), fromjson("{x: 1}"), fromjson("{x: 10}"));
-}
-
 TEST_F(QueryPlannerWildcardTest, CanAnswerEqualityToEmptyObject) {
     addWildcardIndex(BSON("$**" << 1));
     runQuery(fromjson("{a: {}}"));
diff --git a/src/mongo/db/query/query_request.cpp b/src/mongo/db/query/query_request.cpp
index ca26724a64b..4dddbb9d361 100644
--- a/src/mongo/db/query/query_request.cpp
+++ b/src/mongo/db/query/query_request.cpp
@@ -583,7 +583,7 @@ Status QueryRequest::validate() const {
     // Min and Max objects must have the same fields.
     if (!_min.isEmpty() && !_max.isEmpty()) {
         if (!_min.isFieldNamePrefixOf(_max) || (_min.nFields() != _max.nFields())) {
-            return Status(ErrorCodes::BadValue, "min and max must have the same field names");
+            return Status(ErrorCodes::Error(51176), "min and max must have the same field names");
         }
     }
 
diff --git a/src/mongo/dbtests/querytests.cpp b/src/mongo/dbtests/querytests.cpp
index 6c178e4f11e..880d94913db 100644
--- a/src/mongo/dbtests/querytests.cpp
+++ b/src/mongo/dbtests/querytests.cpp
@@ -1058,30 +1058,27 @@ public:
         _client.insert(ns, BSON("a" << 2 << "b" << 2));
 
         ASSERT_EQUALS(4, count(_client.query(NamespaceString(ns), BSONObj())));
-        BSONObj hints[] = {BSONObj(), BSON("a" << 1 << "b" << 1)};
-        for (int i = 0; i < 2; ++i) {
-            check(0, 0, 3, 3, 4, hints[i]);
-            check(1, 1, 2, 2, 3, hints[i]);
-            check(1, 2, 2, 2, 2, hints[i]);
-            check(1, 2, 2, 1, 1, hints[i]);
-
-            unique_ptr<DBClientCursor> c = query(1, 2, 2, 2, hints[i]);
-            BSONObj obj = c->next();
-            ASSERT_EQUALS(1, obj.getIntField("a"));
-            ASSERT_EQUALS(2, obj.getIntField("b"));
-            obj = c->next();
-            ASSERT_EQUALS(2, obj.getIntField("a"));
-            ASSERT_EQUALS(1, obj.getIntField("b"));
-            ASSERT(!c->more());
-        }
+        BSONObj hint = BSON("a" << 1 << "b" << 1);
+        check(0, 0, 3, 3, 4, hint);
+        check(1, 1, 2, 2, 3, hint);
+        check(1, 2, 2, 2, 2, hint);
+        check(1, 2, 2, 1, 1, hint);
+
+        unique_ptr<DBClientCursor> c = query(1, 2, 2, 2, hint);
+        BSONObj obj = c->next();
+        ASSERT_EQUALS(1, obj.getIntField("a"));
+        ASSERT_EQUALS(2, obj.getIntField("b"));
+        obj = c->next();
+        ASSERT_EQUALS(2, obj.getIntField("a"));
+        ASSERT_EQUALS(1, obj.getIntField("b"));
+        ASSERT(!c->more());
     }
 
 private:
     unique_ptr<DBClientCursor> query(int minA, int minB, int maxA, int maxB, const BSONObj& hint) {
         Query q;
         q = q.minKey(BSON("a" << minA << "b" << minB)).maxKey(BSON("a" << maxA << "b" << maxB));
-        if (!hint.isEmpty())
-            q.hint(hint);
+        q.hint(hint);
         return _client.query(NamespaceString(ns), q);
     }
     void check(