summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--jstests/noPassthrough/noFetchBonus.js29
-rw-r--r--src/mongo/db/query/plan_ranker.cpp7
-rw-r--r--src/mongo/db/query/plan_ranker_test.cpp79
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