diff options
| -rw-r--r-- | jstests/noPassthrough/noFetchBonus.js | 29 | ||||
| -rw-r--r-- | src/mongo/db/query/plan_ranker.cpp | 7 | ||||
| -rw-r--r-- | src/mongo/db/query/plan_ranker_test.cpp | 79 |
3 files changed, 110 insertions, 5 deletions
diff --git a/jstests/noPassthrough/noFetchBonus.js b/jstests/noPassthrough/noFetchBonus.js new file mode 100644 index 00000000000..7035eac5aea --- /dev/null +++ b/jstests/noPassthrough/noFetchBonus.js @@ -0,0 +1,29 @@ +// @tags: [requires_sharding, requires_replication] +(function() { + "use strict"; + + load('jstests/libs/analyze_plan.js'); + + const st = new ShardingTest({shards: 1, rs: {nodes: 1}, config: 1}); + const db = st.s.getDB("test"); + const coll = db.getCollection('coll'); + + assert.commandWorked(coll.createIndex({a: 1})); + assert.commandWorked(coll.createIndex({a: 1, b: 1})); + st.shardColl(coll, {a: 1, b: 1}, false); + + // SERVER-39241 One plan uses the {a: 1} index, which isn't enough to cover the query, because + // the + // sharding filter needs check the value of b. The other plan uses the {a: 1, b: 1}, which does + // cover the query. Assert the covered plan wins. + let explain = coll.explain().count({a: 1}); + assert(planHasStage(db, explain.queryPlanner.winningPlan, 'SHARDING_FILTER'), explain); + assert(isIndexOnly(db, explain.queryPlanner.winningPlan), explain); + + let rejected = explain.queryPlanner.winningPlan.shards[0].rejectedPlans; + assert.eq(rejected.length, 1, rejected); + assert(planHasStage(db, rejected[0], 'SHARDING_FILTER'), explain); + assert(planHasStage(db, rejected[0], 'FETCH'), rejected); + + st.stop(); +}()); diff --git a/src/mongo/db/query/plan_ranker.cpp b/src/mongo/db/query/plan_ranker.cpp index a8260b22c7f..d8d5ff809f9 100644 --- a/src/mongo/db/query/plan_ranker.cpp +++ b/src/mongo/db/query/plan_ranker.cpp @@ -209,12 +209,9 @@ double PlanRanker::scoreTree(const PlanStageStats* stats) { // plan doesn't lose to a less productive plan due to tie breaking. const double epsilon = std::min(1.0 / static_cast<double>(10 * workUnits), 1e-4); - // We prefer covered projections. - // - // We only do this when we have a projection stage because we have so many jstests that - // check bounds even when a collscan plan is just as good as the ixscan'd plan :( + // We prefer queries that don't require a fetch stage. double noFetchBonus = epsilon; - if (hasStage(STAGE_PROJECTION, stats) && hasStage(STAGE_FETCH, stats)) { + if (hasStage(STAGE_FETCH, stats)) { noFetchBonus = 0; } diff --git a/src/mongo/db/query/plan_ranker_test.cpp b/src/mongo/db/query/plan_ranker_test.cpp new file mode 100644 index 00000000000..8f51a0edab1 --- /dev/null +++ b/src/mongo/db/query/plan_ranker_test.cpp @@ -0,0 +1,79 @@ +/** + * Copyright (C) 2019-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * <http://www.mongodb.com/licensing/server-side-public-license>. + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the Server Side Public License in all respects for + * all of the code used other than as permitted herein. If you modify file(s) + * with this exception, you may extend this exception to your version of the + * file(s), but you are not obligated to do so. If you do not wish to do so, + * delete this exception statement from your version. If you delete this + * exception statement from all source files in the program, then also delete + * it in the license file. + */ + +/** + * This file contains tests for mongo/db/query/plan_ranker.h + */ + +#include "mongo/db/query/plan_ranker.h" + +#include "mongo/unittest/unittest.h" +#include "mongo/util/assert_util.h" + +using namespace mongo; + +namespace { + +using std::make_unique; +using std::string; +using std::unique_ptr; +using std::vector; + +unique_ptr<PlanStageStats> makeStats(const char* name, + StageType type, + unique_ptr<SpecificStats> specific) { + auto stats = make_unique<PlanStageStats>(name, type); + stats->common.works = 1; + stats->specific = std::move(specific); + return stats; +} + +TEST(PlanRankerTest, NoFetchBonus) { + // Two plans: one does a fetch, one does not. Assert the plan without the fetch has a higher + // score. Note there is no projection involved: before SERVER-39241 was fixed we would give + // these two plans the same score. + + auto goodPlan = + makeStats("SHARDING_FILTER", STAGE_SHARDING_FILTER, make_unique<ShardingFilterStats>()); + goodPlan->children.emplace_back( + makeStats("IXSCAN", STAGE_IXSCAN, make_unique<IndexScanStats>())); + + auto badPlan = + makeStats("SHARDING_FILTER", STAGE_SHARDING_FILTER, make_unique<ShardingFilterStats>()); + badPlan->children.emplace_back(makeStats("FETCH", STAGE_FETCH, make_unique<IndexScanStats>())); + badPlan->children[0]->children.emplace_back( + makeStats("IXSCAN", STAGE_IXSCAN, make_unique<IndexScanStats>())); + + auto goodScore = PlanRanker::scoreTree(goodPlan.get()); + auto badScore = PlanRanker::scoreTree(badPlan.get()); + + ASSERT_GT(goodScore, badScore); +} + +}; // namespace |