SERVER-8564 An index hint inconsistent with requested natural order sort should be an error

3 files changed, 40 insertions, 32 deletions

diff --git a/src/mongo/db/query/canonical_query.cpp b/src/mongo/db/query/canonical_query.cpp
index ddddc3fa932..fa90a487b5c 100644
--- a/src/mongo/db/query/canonical_query.cpp
+++ b/src/mongo/db/query/canonical_query.cpp
@@ -555,6 +555,19 @@ Status CanonicalQuery::isValid(MatchExpression* root, const LiteParsedQuery& par
         return Status(ErrorCodes::BadValue, "text and snapshot not allowed in same query");
     }
 
+    // $natural sort order must agree with hint.
+    if (sortNaturalElt) {
+        if (!hintObj.isEmpty() && !hintNaturalElt) {
+            return Status(ErrorCodes::BadValue, "index hint not allowed with $natural sort order");
+        }
+        if (hintNaturalElt) {
+            if (hintNaturalElt.numberInt() != sortNaturalElt.numberInt()) {
+                return Status(ErrorCodes::BadValue,
+                              "$natural hint must be in the same direction as $natural sort order");
+            }
+        }
+    }
+
     return Status::OK();
 }
 
diff --git a/src/mongo/db/query/canonical_query_test.cpp b/src/mongo/db/query/canonical_query_test.cpp
index 0e9a0a60d04..d56c1814ebd 100644
--- a/src/mongo/db/query/canonical_query_test.cpp
+++ b/src/mongo/db/query/canonical_query_test.cpp
@@ -425,6 +425,33 @@ TEST(CanonicalQueryTest, IsValidSortKeyMetaProjection) {
     }
 }
 
+TEST(CanonicalQueryTest, IsValidNaturalSortIndexHint) {
+    const bool isExplain = false;
+    auto lpq = assertGet(LiteParsedQuery::makeFromFindCommand(
+        nss, fromjson("{find: 'testcoll', sort: {$natural: 1}, hint: {a: 1}}"), isExplain));
+
+    // Invalid: {$natural: 1} sort order and index hint.
+    ASSERT_NOT_OK(isValid("{}", *lpq));
+}
+
+TEST(CanonicalQueryTest, IsValidNaturalSortNaturalHint) {
+    const bool isExplain = false;
+    auto lpq = assertGet(LiteParsedQuery::makeFromFindCommand(
+        nss, fromjson("{find: 'testcoll', sort: {$natural: 1}, hint: {$natural: 1}}"), isExplain));
+
+    // Valid: {$natural: 1} sort order and {$natural: 1} hint.
+    ASSERT_OK(isValid("{}", *lpq));
+}
+
+TEST(CanonicalQueryTest, IsValidNaturalSortNaturalHintDifferentDirections) {
+    const bool isExplain = false;
+    auto lpq = assertGet(LiteParsedQuery::makeFromFindCommand(
+        nss, fromjson("{find: 'testcoll', sort: {$natural: 1}, hint: {$natural: -1}}"), isExplain));
+
+    // Invalid: {$natural: 1} sort order and {$natural: -1} hint.
+    ASSERT_NOT_OK(isValid("{}", *lpq));
+}
+
 //
 // Tests for CanonicalQuery::sortTree
 //
diff --git a/src/mongo/db/query/query_planner_test.cpp b/src/mongo/db/query/query_planner_test.cpp
index 2f9b4d619ed..1a779a4961c 100644
--- a/src/mongo/db/query/query_planner_test.cpp
+++ b/src/mongo/db/query/query_planner_test.cpp
@@ -2199,18 +2199,6 @@ TEST_F(QueryPlannerTest, NaturalSortAndHint) {
     assertNumSolutions(1U);
     assertSolutionExists("{cscan: {dir: -1}}");
 
-    // Non-empty query, 1 sort, -1 hint.
-    runQuerySortHint(
-        fromjson("{x: {$exists: true}}"), BSON("$natural" << 1), BSON("$natural" << -1));
-    assertNumSolutions(1U);
-    assertSolutionExists("{cscan: {dir: 1}}");
-
-    // Non-empty query, -1 sort, 1 hint.
-    runQuerySortHint(
-        fromjson("{x: {$exists: true}}"), BSON("$natural" << -1), BSON("$natural" << 1));
-    assertNumSolutions(1U);
-    assertSolutionExists("{cscan: {dir: -1}}");
-
     // Non-empty query, 1 sort, 1 hint.
     runQuerySortHint(
         fromjson("{x: {$exists: true}}"), BSON("$natural" << 1), BSON("$natural" << 1));
@@ -2232,32 +2220,12 @@ TEST_F(QueryPlannerTest, NaturalSortAndHint) {
     assertNumSolutions(1U);
     assertSolutionExists("{cscan: {dir: -1}}");
 
-    // Empty query, 1 sort, -1 hint.
-    runQuerySortHint(BSONObj(), BSON("$natural" << 1), BSON("$natural" << -1));
-    assertNumSolutions(1U);
-    assertSolutionExists("{cscan: {dir: 1}}");
-
-    // Empty query, -1 sort, 1 hint.
-    runQuerySortHint(BSONObj(), BSON("$natural" << -1), BSON("$natural" << 1));
-    assertNumSolutions(1U);
-    assertSolutionExists("{cscan: {dir: -1}}");
-
     // Empty query, 1 sort, 1 hint.
     runQuerySortHint(BSONObj(), BSON("$natural" << 1), BSON("$natural" << 1));
     assertNumSolutions(1U);
     assertSolutionExists("{cscan: {dir: 1}}");
 }
 
-TEST_F(QueryPlannerTest, HintOverridesNaturalSort) {
-    addIndex(BSON("x" << 1));
-    runQuerySortHint(fromjson("{x: {$exists: true}}"), BSON("$natural" << -1), BSON("x" << 1));
-
-    assertNumSolutions(1U);
-    assertSolutionExists(
-        "{fetch: {filter: {x:{$exists:true}}, node: "
-        "{ixscan: {filter: null, pattern: {x: 1}}}}}");
-}
-
 TEST_F(QueryPlannerTest, HintValid) {
     addIndex(BSON("a" << 1));
     runQueryHint(BSONObj(), fromjson("{a: 1}"));