diff options
-rw-r--r-- | jstests/core/minmax.js | 239 | ||||
-rw-r--r-- | jstests/core/minmax_edge.js | 6 | ||||
-rw-r--r-- | src/mongo/db/query/query_planner.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/query/query_planner_collation_test.cpp | 13 | ||||
-rw-r--r-- | src/mongo/db/query/query_planner_common.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/query/query_planner_test.cpp | 7 |
6 files changed, 146 insertions, 125 deletions
diff --git a/jstests/core/minmax.js b/jstests/core/minmax.js index 670c7d2f8b2..68dcdb38992 100644 --- a/jstests/core/minmax.js +++ b/jstests/core/minmax.js @@ -1,97 +1,142 @@ -// test min / max query parameters - -addData = function() { - t.save({a: 1, b: 1}); - t.save({a: 1, b: 2}); - t.save({a: 2, b: 1}); - t.save({a: 2, b: 2}); -}; - -t = db.jstests_minmax; -t.drop(); -t.ensureIndex({a: 1, b: 1}); -addData(); - -printjson(t.find().min({a: 1, b: 2}).max({a: 2, b: 1}).toArray()); -assert.eq(1, t.find().min({a: 1, b: 2}).max({a: 2, b: 1}).toArray().length); -assert.eq(2, t.find().min({a: 1, b: 2}).max({a: 2, b: 1.5}).toArray().length); -assert.eq(2, t.find().min({a: 1, b: 2}).max({a: 2, b: 2}).toArray().length); - -// just one bound -assert.eq(3, t.find().min({a: 1, b: 2}).toArray().length); -assert.eq(3, t.find().max({a: 2, b: 1.5}).toArray().length); -assert.eq(3, t.find().min({a: 1, b: 2}).hint({a: 1, b: 1}).toArray().length); -assert.eq(3, t.find().max({a: 2, b: 1.5}).hint({a: 1, b: 1}).toArray().length); - -t.drop(); -t.ensureIndex({a: 1, b: -1}); -addData(); -assert.eq(4, t.find().min({a: 1, b: 2}).toArray().length); -assert.eq(4, t.find().max({a: 2, b: 0.5}).toArray().length); -assert.eq(1, t.find().min({a: 2, b: 1}).toArray().length); -assert.eq(1, t.find().max({a: 1, b: 1.5}).toArray().length); -assert.eq(4, t.find().min({a: 1, b: 2}).hint({a: 1, b: -1}).toArray().length); -assert.eq(4, t.find().max({a: 2, b: 0.5}).hint({a: 1, b: -1}).toArray().length); -assert.eq(1, t.find().min({a: 2, b: 1}).hint({a: 1, b: -1}).toArray().length); -assert.eq(1, t.find().max({a: 1, b: 1.5}).hint({a: 1, b: -1}).toArray().length); - -// hint doesn't match -assert.throws(function() { - t.find().min({a: 1}).hint({a: 1, b: -1}).toArray(); -}); -assert.throws(function() { - t.find().min({a: 1, b: 1}).max({a: 1}).hint({a: 1, b: -1}).toArray(); -}); -assert.throws(function() { - t.find().min({b: 1}).max({a: 1, b: 2}).hint({a: 1, b: -1}).toArray(); -}); -assert.throws(function() { - t.find().min({a: 1}).hint({$natural: 1}).toArray(); -}); -assert.throws(function() { - t.find().max({a: 1}).hint({$natural: 1}).toArray(); -}); - -// Reverse direction scan of the a:1 index between a:6 (inclusive) and a:3 (exclusive). -t.drop(); -t.ensureIndex({a: 1}); -for (i = 0; i < 10; ++i) { - t.save({_id: i, a: i}); -} -if (0) { // SERVER-3766 - reverseResult = t.find().min({a: 6}).max({a: 3}).sort({a: -1}).hint({a: 1}).toArray(); - assert.eq([{_id: 6, a: 6}, {_id: 5, a: 5}, {_id: 4, a: 4}], reverseResult); -} - -// -// SERVER-15015. -// - -// Test ascending index. -t.drop(); -t.ensureIndex({a: 1}); -t.insert({a: 3}); -t.insert({a: 4}); -t.insert({a: 5}); - -var cursor = t.find().min({a: 4}); -assert.eq(4, cursor.next()["a"]); -assert.eq(5, cursor.next()["a"]); -assert(!cursor.hasNext()); - -cursor = t.find().max({a: 4}); -assert.eq(3, cursor.next()["a"]); -assert(!cursor.hasNext()); - -// Test descending index. -t.dropIndexes(); -t.ensureIndex({a: -1}); - -cursor = t.find().min({a: 4}); -assert.eq(4, cursor.next()["a"]); -assert.eq(3, cursor.next()["a"]); -assert(!cursor.hasNext()); - -cursor = t.find().max({a: 4}); -assert.eq(5, cursor.next()["a"]); -assert(!cursor.hasNext()); +// Test min / max query parameters. +(function() { + "use strict"; + + load("jstests/aggregation/extras/utils.js"); // For resultsEq. + + const coll = db.jstests_minmax; + coll.drop(); + + function addData() { + assert.writeOK(coll.save({a: 1, b: 1})); + assert.writeOK(coll.save({a: 1, b: 2})); + assert.writeOK(coll.save({a: 2, b: 1})); + assert.writeOK(coll.save({a: 2, b: 2})); + } + + assert.commandWorked(coll.ensureIndex({a: 1, b: 1})); + addData(); + + assert.eq(1, coll.find().min({a: 1, b: 2}).max({a: 2, b: 1}).toArray().length); + assert.eq(2, coll.find().min({a: 1, b: 2}).max({a: 2, b: 1.5}).toArray().length); + assert.eq(2, coll.find().min({a: 1, b: 2}).max({a: 2, b: 2}).toArray().length); + + // Single bound. + assert.eq(3, coll.find().min({a: 1, b: 2}).toArray().length); + assert.eq(3, coll.find().max({a: 2, b: 1.5}).toArray().length); + assert.eq(3, coll.find().min({a: 1, b: 2}).hint({a: 1, b: 1}).toArray().length); + assert.eq(3, coll.find().max({a: 2, b: 1.5}).hint({a: 1, b: 1}).toArray().length); + + coll.drop(); + assert.commandWorked(coll.ensureIndex({a: 1, b: -1})); + addData(); + assert.eq(4, coll.find().min({a: 1, b: 2}).toArray().length); + assert.eq(4, coll.find().max({a: 2, b: 0.5}).toArray().length); + assert.eq(1, coll.find().min({a: 2, b: 1}).toArray().length); + assert.eq(1, coll.find().max({a: 1, b: 1.5}).toArray().length); + assert.eq(4, coll.find().min({a: 1, b: 2}).hint({a: 1, b: -1}).toArray().length); + assert.eq(4, coll.find().max({a: 2, b: 0.5}).hint({a: 1, b: -1}).toArray().length); + assert.eq(1, coll.find().min({a: 2, b: 1}).hint({a: 1, b: -1}).toArray().length); + assert.eq(1, coll.find().max({a: 1, b: 1.5}).hint({a: 1, b: -1}).toArray().length); + + // Hint doesn't match. + assert.throws(function() { + coll.find().min({a: 1}).hint({a: 1, b: -1}).toArray(); + }); + assert.throws(function() { + coll.find().min({a: 1, b: 1}).max({a: 1}).hint({a: 1, b: -1}).toArray(); + }); + assert.throws(function() { + coll.find().min({b: 1}).max({a: 1, b: 2}).hint({a: 1, b: -1}).toArray(); + }); + assert.throws(function() { + coll.find().min({a: 1}).hint({$natural: 1}).toArray(); + }); + assert.throws(function() { + coll.find().max({a: 1}).hint({$natural: 1}).toArray(); + }); + + coll.drop(); + assert.commandWorked(coll.ensureIndex({a: 1})); + for (let i = 0; i < 10; ++i) { + assert.writeOK(coll.save({_id: i, a: i})); + } + + // Reverse direction scan of the a:1 index between a:6 (inclusive) and a:3 (exclusive) is + // expected to fail, as max must be > min. + let error = assert.throws(function() { + coll.find().min({a: 6}).max({a: 3}).sort({a: -1}).toArray(); + }); + assert.eq(error.code, ErrorCodes.BadValue); + + error = assert.throws(function() { + coll.find().min({a: 6}).max({a: 3}).sort({a: -1}).hint({a: 1}).toArray(); + }); + assert.eq(error.code, ErrorCodes.BadValue); + + // A find with identical min and max values is expected to fail, as max is exclusive. + error = assert.throws(function() { + coll.find().min({a: 2}).max({a: 2}).toArray(); + }); + assert.eq(error.code, ErrorCodes.BadValue); + + error = assert.throws(function() { + coll.find().min({a: 2}).max({a: 2}).hint({a: 1}).toArray(); + }); + assert.eq(error.code, ErrorCodes.BadValue); + + error = assert.throws(function() { + coll.find().min({a: 2}).max({a: 2}).sort({a: -1}).toArray(); + }); + assert.eq(error.code, ErrorCodes.BadValue); + + error = assert.throws(function() { + coll.find().min({a: 2}).max({a: 2}).sort({a: -1}).hint({a: 1}).toArray(); + }); + assert.eq(error.code, ErrorCodes.BadValue); + + coll.drop(); + addData(); + assert.commandWorked(coll.ensureIndex({a: 1, b: 1})); + + error = assert.throws(function() { + coll.find().min({a: 1, b: 2}).max({a: 1, b: 2}).toArray(); + }); + assert.eq(error.code, ErrorCodes.BadValue); + + error = assert.throws(function() { + coll.find().min({a: 1, b: 2}).max({a: 1, b: 2}).hint({a: 1, b: 1}).toArray(); + }); + assert.eq(error.code, ErrorCodes.BadValue); + + // Test ascending index. + coll.drop(); + assert.commandWorked(coll.ensureIndex({a: 1})); + assert.writeOK(coll.insert({a: 3})); + assert.writeOK(coll.insert({a: 4})); + assert.writeOK(coll.insert({a: 5})); + + let cursor = coll.find().min({a: 4}); + assert.eq(4, cursor.next().a); + assert.eq(5, cursor.next().a); + + assert(!cursor.hasNext()); + + cursor = coll.find().max({a: 4}); + assert.eq(3, cursor.next()["a"]); + assert(!cursor.hasNext()); + + // Test descending index. + assert.commandWorked(coll.dropIndexes()); + assert.commandWorked(coll.ensureIndex({a: -1})); + + cursor = coll.find().min({a: 4}); + assert.eq(4, cursor.next().a); + assert.eq(3, cursor.next().a); + + assert(!cursor.hasNext()); + + cursor = coll.find().max({a: 4}); + assert.eq(5, cursor.next()["a"]); + assert(!cursor.hasNext()); +}()); diff --git a/jstests/core/minmax_edge.js b/jstests/core/minmax_edge.js index abd39724a80..759d87dc21e 100644 --- a/jstests/core/minmax_edge.js +++ b/jstests/core/minmax_edge.js @@ -79,8 +79,6 @@ verifyMax({a: {a: 1}}, [0, 1, 2, 3, 4, 5, 6, 7, 8]); verifyMin({a: 'a'}, []); verifyMax({a: 'a'}, [0, 1, 2, 3, 4, 5, 6, 7, 8]); -verifyResultIds(t.find().min({a: 4}).max({a: 4}).toArray(), []); - // Now with a compound index. reset(t); assert.commandWorked(t.ensureIndex({a: 1, b: -1})); @@ -101,8 +99,6 @@ verifyMax({a: {a: 1}, b: 1}, [0, 1, 2, 3, 4, 5, 6, 7, 8]); verifyMin({a: 'a', b: 1}, []); verifyMax({a: 'a', b: 1}, [0, 1, 2, 3, 4, 5, 6, 7, 8]); -verifyResultIds(t.find().min({a: 4, b: 1}).max({a: 4, b: 1}).toArray(), []); - // Edge cases on b values verifyMin({a: 1, b: Infinity}, [0, 1, 2, 3, 4, 5, 6, 7, 8]); verifyMin({a: 2, b: Infinity}, [3, 4, 5, 6, 7, 8]); @@ -146,8 +142,6 @@ verifyMax({a: {a: 1}}, []); verifyMin({a: 'a'}, [0, 1, 2, 3, 4, 5, 6, 7, 8]); verifyMax({a: 'a'}, []); -verifyResultIds(t.find().min({a: 4}).max({a: 4}).toArray(), []); - // Now with a compound index. reset(t); t.ensureIndex({a: -1, b: -1}); diff --git a/src/mongo/db/query/query_planner.cpp b/src/mongo/db/query/query_planner.cpp index 850276f35e7..2388154276c 100644 --- a/src/mongo/db/query/query_planner.cpp +++ b/src/mongo/db/query/query_planner.cpp @@ -659,7 +659,7 @@ Status QueryPlanner::plan(const CanonicalQuery& query, // Now we have the final min and max. This index is only relevant for // the min/max query if min < max. - if (0 >= + if (0 > finishedMinObj.woCompare(finishedMaxObj, indexEntry.keyPattern, false)) { // Found a relevant index. idxNo = i; diff --git a/src/mongo/db/query/query_planner_collation_test.cpp b/src/mongo/db/query/query_planner_collation_test.cpp index 54a549723c8..97860b36add 100644 --- a/src/mongo/db/query/query_planner_collation_test.cpp +++ b/src/mongo/db/query/query_planner_collation_test.cpp @@ -220,19 +220,6 @@ TEST_F(QueryPlannerTest, MinMaxWithStringBoundsCannotBeCoveredWithCollator) { "{locale: 'reverse'}, node: {ixscan: {pattern: {a: 1, b: 1}}}}}}}"); } -TEST_F(QueryPlannerTest, MinMaxWithoutStringBoundsBoundsCanBeCoveredWithCollator) { - CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString); - addIndex(fromjson("{a: 1, b: 1}"), &collator); - - runQueryAsCommand( - fromjson("{find: 'testns', min: {a: 1, b: 2}, max: {a: 1, b: 2}, " - "projection: {_id: 0, a: 1, b: 1}, collation: {locale: 'reverse'}}")); - - assertNumSolutions(1U); - assertSolutionExists( - "{proj: {spec: {_id: 0, a: 1, b: 1}, node: {ixscan: {pattern: {a: 1, b: 1}}}}}"); -} - TEST_F(QueryPlannerTest, SimpleRegexCanUseAnIndexWithACollatorWithLooseBounds) { CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString); addIndex(fromjson("{a: 1}"), &collator); diff --git a/src/mongo/db/query/query_planner_common.cpp b/src/mongo/db/query/query_planner_common.cpp index 33eb72100cc..337f3a045fc 100644 --- a/src/mongo/db/query/query_planner_common.cpp +++ b/src/mongo/db/query/query_planner_common.cpp @@ -71,8 +71,8 @@ void QueryPlannerCommon::reverseScans(QuerySolutionNode* node) { } if (!isn->bounds.isValidFor(isn->index.keyPattern, isn->direction)) { - LOG(5) << "Invalid bounds: " << redact(isn->bounds.toString()); - invariant(0); + severe() << "Invalid bounds: " << redact(isn->bounds.toString()); + MONGO_UNREACHABLE; } // TODO: we can just negate every value in the already computed properties. diff --git a/src/mongo/db/query/query_planner_test.cpp b/src/mongo/db/query/query_planner_test.cpp index 7fc29a4e42f..f4f608e0b58 100644 --- a/src/mongo/db/query/query_planner_test.cpp +++ b/src/mongo/db/query/query_planner_test.cpp @@ -1055,12 +1055,7 @@ TEST_F(QueryPlannerTest, MaxValid) { TEST_F(QueryPlannerTest, MinMaxSameValue) { addIndex(BSON("a" << 1)); - runQueryHintMinMax(BSONObj(), BSONObj(), fromjson("{a: 1}"), fromjson("{a: 1}")); - - assertNumSolutions(1U); - assertSolutionExists( - "{fetch: {filter: null, " - "node: {ixscan: {filter: null, pattern: {a: 1}}}}}"); + runInvalidQueryHintMinMax(BSONObj(), BSONObj(), fromjson("{a: 1}"), fromjson("{a: 1}")); } TEST_F(QueryPlannerTest, MaxWithoutIndex) { |