diff options
author | David Percy <david.percy@mongodb.com> | 2019-11-19 18:52:43 +0000 |
---|---|---|
committer | evergreen <evergreen@mongodb.com> | 2019-11-19 18:52:43 +0000 |
commit | 24821c1d1266ada109dd62ec71d4d8a900fbdec0 (patch) | |
tree | f903b6bcc57823695550441ee503034fc31f3b77 /src/mongo/db/query | |
parent | 5976013e3655bd769833f32463048ffd4f45b065 (diff) | |
download | mongo-24821c1d1266ada109dd62ec71d4d8a900fbdec0.tar.gz |
SERVER-39241 Apply noFetchBonus iff no FETCH stage; disregard PROJECTION
Diffstat (limited to 'src/mongo/db/query')
-rw-r--r-- | src/mongo/db/query/SConscript | 1 | ||||
-rw-r--r-- | src/mongo/db/query/plan_ranker.cpp | 9 | ||||
-rw-r--r-- | src/mongo/db/query/plan_ranker_test.cpp | 79 |
3 files changed, 82 insertions, 7 deletions
diff --git a/src/mongo/db/query/SConscript b/src/mongo/db/query/SConscript index 95463e76357..f286bca9ccc 100644 --- a/src/mongo/db/query/SConscript +++ b/src/mongo/db/query/SConscript @@ -273,6 +273,7 @@ env.CppUnitTest( "parsed_distinct_test.cpp", "plan_cache_indexability_test.cpp", "plan_cache_test.cpp", + "plan_ranker_test.cpp", "planner_access_test.cpp", "planner_analysis_test.cpp", "planner_ixselect_test.cpp", diff --git a/src/mongo/db/query/plan_ranker.cpp b/src/mongo/db/query/plan_ranker.cpp index 2970428be3b..1cac7f7424c 100644 --- a/src/mongo/db/query/plan_ranker.cpp +++ b/src/mongo/db/query/plan_ranker.cpp @@ -225,14 +225,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_DEFAULT, stats) || hasStage(STAGE_PROJECTION_COVERED, stats) || - hasStage(STAGE_PROJECTION_SIMPLE, 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 |