From f8294a13108b43feb7de435f045b5b363c29faac Mon Sep 17 00:00:00 2001 From: David Storch Date: Thu, 5 Jun 2014 11:47:08 -0400 Subject: SERVER-14176 query planner should obey $natural sort (cherry picked from commit 89383b224f061d306240d621745ba082a4ecb23b) --- src/mongo/db/query/planner_access.cpp | 15 +++--- src/mongo/db/query/query_planner.cpp | 16 +++++-- src/mongo/db/query/query_planner_test.cpp | 78 +++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 11 deletions(-) diff --git a/src/mongo/db/query/planner_access.cpp b/src/mongo/db/query/planner_access.cpp index ef9669cd604..705f25d6aad 100644 --- a/src/mongo/db/query/planner_access.cpp +++ b/src/mongo/db/query/planner_access.cpp @@ -69,18 +69,19 @@ namespace mongo { csn->tailable = tailable; csn->maxScan = query.getParsed().getMaxScan(); - // If the sort is {$natural: +-1} this changes the direction of the collection scan. - const BSONObj& sortObj = query.getParsed().getSort(); - if (!sortObj.isEmpty()) { - BSONElement natural = sortObj.getFieldDotted("$natural"); + // If the hint is {$natural: +-1} this changes the direction of the collection scan. + if (!query.getParsed().getHint().isEmpty()) { + BSONElement natural = query.getParsed().getHint().getFieldDotted("$natural"); if (!natural.eoo()) { csn->direction = natural.numberInt() >= 0 ? 1 : -1; } } - // The hint can specify $natural as well. - if (!query.getParsed().getHint().isEmpty()) { - BSONElement natural = query.getParsed().getHint().getFieldDotted("$natural"); + // The sort can specify $natural as well. The sort direction should override the hint + // direction if both are specified. + const BSONObj& sortObj = query.getParsed().getSort(); + if (!sortObj.isEmpty()) { + BSONElement natural = sortObj.getFieldDotted("$natural"); if (!natural.eoo()) { csn->direction = natural.numberInt() >= 0 ? 1 : -1; } diff --git a/src/mongo/db/query/query_planner.cpp b/src/mongo/db/query/query_planner.cpp index 72964ff90ce..8a97a2ae02b 100644 --- a/src/mongo/db/query/query_planner.cpp +++ b/src/mongo/db/query/query_planner.cpp @@ -410,10 +410,18 @@ namespace mongo { return Status::OK(); } - // The hint can be $natural: 1. If this happens, output a collscan. - if (!query.getParsed().getHint().isEmpty()) { - BSONElement natural = query.getParsed().getHint().getFieldDotted("$natural"); - if (!natural.eoo()) { + // The hint or sort can be $natural: 1. If this happens, output a collscan. If both + // a $natural hint and a $natural sort are specified, then the direction of the collscan + // is determined by the sign of the sort (not the sign of the hint). + if (!query.getParsed().getHint().isEmpty() || !query.getParsed().getSort().isEmpty()) { + BSONObj hintObj = query.getParsed().getHint(); + BSONObj sortObj = query.getParsed().getSort(); + BSONElement naturalHint = hintObj.getFieldDotted("$natural"); + BSONElement naturalSort = sortObj.getFieldDotted("$natural"); + + // A hint overrides a $natural sort. This means that we don't force a table + // scan if there is a $natural sort with a non-$natural hint. + if (!naturalHint.eoo() || (!naturalSort.eoo() && hintObj.isEmpty())) { QLOG() << "Forcing a table scan due to hinted $natural\n"; // min/max are incompatible with $natural. if (canTableScan && query.getParsed().getMin().isEmpty() diff --git a/src/mongo/db/query/query_planner_test.cpp b/src/mongo/db/query/query_planner_test.cpp index 41fa983c56b..eae67fb9330 100644 --- a/src/mongo/db/query/query_planner_test.cpp +++ b/src/mongo/db/query/query_planner_test.cpp @@ -2468,6 +2468,84 @@ namespace { "{cscan: {filter: {a: 1}, dir: 1}}}}"); } + // Test $natural sort and its interaction with $natural hint. + TEST_F(QueryPlannerTest, NaturalSortAndHint) { + addIndex(BSON("x" << 1)); + + // Non-empty query, -1 sort, no hint. + runQuerySortHint(fromjson("{x: {$exists: true}}"), BSON("$natural" << -1), BSONObj()); + assertNumSolutions(1U); + assertSolutionExists("{cscan: {dir: -1}}"); + + // Non-empty query, 1 sort, no hint. + runQuerySortHint(fromjson("{x: {$exists: true}}"), BSON("$natural" << 1), BSONObj()); + 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)); + 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}}"); + + // Empty query, -1 sort, no hint. + runQuerySortHint(BSONObj(), BSON("$natural" << -1), BSONObj()); + assertNumSolutions(1U); + assertSolutionExists("{cscan: {dir: -1}}"); + + // Empty query, 1 sort, no hint. + runQuerySortHint(BSONObj(), BSON("$natural" << 1), BSONObj()); + 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}}"); + + // 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}")); -- cgit v1.2.1