diff options
-rw-r--r-- | jstests/core/distinct_compound_index.js | 28 | ||||
-rw-r--r-- | jstests/core/distinct_index1.js | 6 | ||||
-rw-r--r-- | jstests/core/explain_distinct.js | 7 | ||||
-rw-r--r-- | src/mongo/db/query/get_executor.cpp | 12 | ||||
-rw-r--r-- | src/mongo/dbtests/query_stage_distinct.cpp | 71 |
5 files changed, 111 insertions, 13 deletions
diff --git a/jstests/core/distinct_compound_index.js b/jstests/core/distinct_compound_index.js new file mode 100644 index 00000000000..9b7c0226265 --- /dev/null +++ b/jstests/core/distinct_compound_index.js @@ -0,0 +1,28 @@ +(function() { + "use strict"; + + load("jstests/libs/analyze_plan.js"); + + var coll = db.distinct_multikey_index; + + coll.drop(); + for (var i = 0; i < 10; i++) { + 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: 3})); + } + coll.createIndex({a: 1, b: 1}); + + var explain_distinct_with_query = coll.explain("executionStats").distinct('b', {a: 1}); + assert.commandWorked(explain_distinct_with_query); + assert(planHasStage(explain_distinct_with_query.queryPlanner.winningPlan, "DISTINCT_SCAN")); + assert(planHasStage(explain_distinct_with_query.queryPlanner.winningPlan, "PROJECTION")); + assert.eq(2, explain_distinct_with_query.executionStats.nReturned); + + var explain_distinct_without_query = coll.explain("executionStats").distinct('b'); + assert.commandWorked(explain_distinct_without_query); + assert(planHasStage(explain_distinct_without_query.queryPlanner.winningPlan, "COLLSCAN")); + assert(!planHasStage(explain_distinct_without_query.queryPlanner.winningPlan, "DISTINCT_SCAN")); + assert.eq(40, explain_distinct_without_query.executionStats.nReturned); +})(); diff --git a/jstests/core/distinct_index1.js b/jstests/core/distinct_index1.js index b9dd3553a55..63f61b8292c 100644 --- a/jstests/core/distinct_index1.js +++ b/jstests/core/distinct_index1.js @@ -57,15 +57,13 @@ assert.eq(398, x.executionStats.nReturned, "BC1"); assert.eq(398, x.executionStats.totalKeysExamined, "BC2"); assert.eq(398, x.executionStats.totalDocsExamined, "BC3"); -// Check proper nscannedObjects count when using a query optimizer cursor. +// Test that a distinct over a trailing field of the index can be covered. t.dropIndexes(); t.ensureIndex({a: 1, b: 1}); x = d("b", {a: {$gt: 5}, b: {$gt: 5}}); printjson(x); -// 171 is the # of results we happen to scan when we don't use a distinct -// hack. When we use the distinct hack we scan 16, currently. assert.lte(x.executionStats.nReturned, 171); -assert.eq(171, x.executionStats.totalDocsExamined, "BD3"); +assert.eq(0, x.executionStats.totalDocsExamined, "BD3"); // Should use an index scan over the hashed index. t.dropIndexes(); diff --git a/jstests/core/explain_distinct.js b/jstests/core/explain_distinct.js index 57e5f55e065..f2908caa5e7 100644 --- a/jstests/core/explain_distinct.js +++ b/jstests/core/explain_distinct.js @@ -80,7 +80,8 @@ assert.eq([1], coll.distinct('b', {a: 1})); var explain = runDistinctExplain(coll, 'b', {a: 1}); assert.commandWorked(explain); - assert.eq(10, explain.executionStats.nReturned); - assert(planHasStage(explain.queryPlanner.winningPlan, "FETCH")); - assert(isIxscan(explain.queryPlanner.winningPlan)); + assert.eq(1, explain.executionStats.nReturned); + assert(!planHasStage(explain.queryPlanner.winningPlan, "FETCH")); + assert(planHasStage(explain.queryPlanner.winningPlan, "PROJECTION")); + assert(planHasStage(explain.queryPlanner.winningPlan, "DISTINCT_SCAN")); })(); diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp index 0963df541a9..4742f915d0d 100644 --- a/src/mongo/db/query/get_executor.cpp +++ b/src/mongo/db/query/get_executor.cpp @@ -1137,6 +1137,10 @@ bool getDistinctNodeIndex(const std::vector<IndexEntry>& indices, if (indices[i].multikey && isDottedField) { continue; } + // Skip indices where the first key is not field. + if (indices[i].keyPattern.firstElement().fieldNameStringData() != StringData(field)) { + continue; + } int nFields = indices[i].keyPattern.nFields(); // Pick the index with the lowest number of fields. if (nFields < minFields) { @@ -1352,9 +1356,7 @@ bool turnIxscanIntoDistinctIxscan(QuerySolution* soln, const string& field) { distinctNode->direction = indexScanNode->direction; distinctNode->bounds = indexScanNode->bounds; - // Figure out which field we're skipping to the next value of. TODO: We currently only - // try to distinct-hack when there is an index prefixed by the field we're distinct-ing - // over. Consider removing this code if we stick with that policy. + // Figure out which field we're skipping to the next value of. distinctNode->fieldNo = 0; BSONObjIterator it(indexScanNode->index.keyPattern); while (it.more()) { @@ -1431,9 +1433,7 @@ StatusWith<unique_ptr<PlanExecutor>> getExecutorDistinct(OperationContext* txn, while (ii.more()) { const IndexDescriptor* desc = ii.next(); IndexCatalogEntry* ice = ii.catalogEntry(desc); - // The distinct hack can work if any field is in the index but it's not always clear - // if it's a win unless it's the first field. - if (desc->keyPattern().firstElement().fieldName() == parsedDistinct->getKey()) { + if (desc->keyPattern().hasField(parsedDistinct->getKey())) { plannerParams.indices.push_back(IndexEntry(desc->keyPattern(), desc->getAccessMethodName(), desc->isMultikey(txn), diff --git a/src/mongo/dbtests/query_stage_distinct.cpp b/src/mongo/dbtests/query_stage_distinct.cpp index 1cbc7b440c4..1558f62574a 100644 --- a/src/mongo/dbtests/query_stage_distinct.cpp +++ b/src/mongo/dbtests/query_stage_distinct.cpp @@ -236,6 +236,76 @@ public: } }; +class QueryStageDistinctCompoundIndex : public DistinctBase { +public: + void run() { + // insert documents with a: 1 and b: 1 + for (size_t i = 0; i < 1000; ++i) { + insert(BSON("a" << 1 << "b" << 1)); + } + // insert documents with a: 1 and b: 2 + for (size_t i = 0; i < 1000; ++i) { + insert(BSON("a" << 1 << "b" << 2)); + } + // insert documents with a: 2 and b: 1 + for (size_t i = 0; i < 1000; ++i) { + insert(BSON("a" << 2 << "b" << 1)); + } + // insert documents with a: 2 and b: 3 + for (size_t i = 0; i < 1000; ++i) { + insert(BSON("a" << 2 << "b" << 3)); + } + + addIndex(BSON("a" << 1 << "b" << 1)); + + AutoGetCollectionForRead ctx(&_txn, ns()); + Collection* coll = ctx.getCollection(); + + std::vector<IndexDescriptor*> indices; + coll->getIndexCatalog()->findIndexesByKeyPattern( + &_txn, BSON("a" << 1 << "b" << 1), false, &indices); + ASSERT_EQ(1U, indices.size()); + + DistinctParams params; + params.descriptor = indices[0]; + ASSERT_TRUE(params.descriptor); + + params.direction = 1; + params.fieldNo = 1; + params.bounds.isSimpleRange = false; + + OrderedIntervalList aOil{"a"}; + aOil.intervals.push_back(IndexBoundsBuilder::allValues()); + params.bounds.fields.push_back(aOil); + + OrderedIntervalList bOil{"b"}; + bOil.intervals.push_back(IndexBoundsBuilder::allValues()); + params.bounds.fields.push_back(bOil); + + WorkingSet ws; + DistinctScan distinct(&_txn, params, &ws); + + WorkingSetID wsid; + PlanStage::StageState state; + + std::vector<int> seen; + + while (PlanStage::IS_EOF != (state = distinct.work(&wsid))) { + ASSERT_NE(PlanStage::FAILURE, state); + ASSERT_NE(PlanStage::DEAD, state); + if (PlanStage::ADVANCED == state) { + seen.push_back(getIntFieldDotted(ws, wsid, "b")); + } + } + + ASSERT_EQUALS(4U, seen.size()); + ASSERT_EQUALS(1, seen[0]); + ASSERT_EQUALS(2, seen[1]); + ASSERT_EQUALS(1, seen[2]); + ASSERT_EQUALS(3, seen[3]); + } +}; + // XXX: add a test case with bounds where skipping to the next key gets us a result that's not // valid w.r.t. our query. @@ -246,6 +316,7 @@ public: void setupTests() { add<QueryStageDistinctBasic>(); add<QueryStageDistinctMultiKey>(); + add<QueryStageDistinctCompoundIndex>(); } }; |