From 8ae61492abeacda59fe10d2eafbd5599cc54a313 Mon Sep 17 00:00:00 2001 From: Mihai Andrei Date: Mon, 14 Mar 2022 13:54:27 +0000 Subject: SERVER-63452 Clean up use of 'getMainCollection()' --- src/mongo/db/pipeline/pipeline_d.cpp | 15 ++++++++------- src/mongo/db/query/get_executor.cpp | 10 +++------- src/mongo/db/query/sbe_cached_solution_planner.cpp | 2 +- src/mongo/db/query/sbe_multi_planner.cpp | 10 +++------- src/mongo/db/query/sbe_runtime_planner.cpp | 2 +- src/mongo/db/query/sbe_stage_builder.cpp | 3 ++- src/mongo/db/query/sbe_stage_builder.h | 2 +- 7 files changed, 19 insertions(+), 25 deletions(-) diff --git a/src/mongo/db/pipeline/pipeline_d.cpp b/src/mongo/db/pipeline/pipeline_d.cpp index 96e9c1675ac..1d6e05d358d 100644 --- a/src/mongo/db/pipeline/pipeline_d.cpp +++ b/src/mongo/db/pipeline/pipeline_d.cpp @@ -127,7 +127,7 @@ std::vector> extractSbeCompatibleSt std::vector> stagesForPushdown; // This handles the case of unionWith against an unknown collection. - if (collections.getMainCollection() == nullptr) { + if (!collections.getMainCollection()) { return {}; } @@ -207,7 +207,6 @@ StatusWith> attemptToGetExe const size_t plannerOpts, const MatchExpressionParser::AllowedFeatureSet& matcherFeatures, Pipeline* pipeline) { - const auto& mainColl = collections.getMainCollection(); auto findCommand = std::make_unique(nss); query_request_helper::setTailableMode(expCtx->tailableMode, findCommand.get()); findCommand->setFilter(queryObj.getOwned()); @@ -288,8 +287,10 @@ StatusWith> attemptToGetExe // 2) We not want a plan that will return separate values for each array element. For // example, if we have a document {a: [1,2]} and group by "a" a DISTINCT_SCAN on an "a" // index would produce one result for '1' and another for '2', which would be incorrect. - auto distinctExecutor = getExecutorDistinct( - &mainColl, plannerOpts | QueryPlannerParams::STRICT_DISTINCT_ONLY, &parsedDistinct); + auto distinctExecutor = + getExecutorDistinct(&collections.getMainCollection(), + plannerOpts | QueryPlannerParams::STRICT_DISTINCT_ONLY, + &parsedDistinct); if (!distinctExecutor.isOK()) { return distinctExecutor.getStatus().withContext( "Unable to use distinct scan to optimize $group stage"); @@ -673,7 +674,6 @@ PipelineD::buildInnerQueryExecutor(const MultipleCollectionAccessor& collections const NamespaceString& nss, const AggregateCommandRequest* aggRequest, Pipeline* pipeline) { - const auto& collection = collections.getMainCollection(); auto expCtx = pipeline->getContext(); // We will be modifying the source vector as we go. @@ -687,6 +687,7 @@ PipelineD::buildInnerQueryExecutor(const MultipleCollectionAccessor& collections // Try to inspect if the DocumentSourceSample or a DocumentSourceInternalUnpackBucket stage // can be optimized for sampling backed by a storage engine supplied random cursor. auto&& [sampleStage, unpackBucketStage] = extractSampleUnpackBucket(sources); + const auto& collection = collections.getMainCollection(); // Optimize an initial $sample stage if possible. if (collection && sampleStage) { @@ -714,12 +715,11 @@ void PipelineD::attachInnerQueryExecutorToPipeline( PipelineD::AttachExecutorCallback attachExecutorCallback, std::unique_ptr exec, Pipeline* pipeline) { - auto& collection = collections.getMainCollection(); // If the pipeline doesn't need a $cursor stage, there will be no callback function and // PlanExecutor provided in the 'attachExecutorCallback' object, so we don't need to do // anything. if (attachExecutorCallback && exec) { - attachExecutorCallback(collection, std::move(exec), pipeline); + attachExecutorCallback(collections.getMainCollection(), std::move(exec), pipeline); } } @@ -955,6 +955,7 @@ PipelineD::buildInnerQueryExecutorGeoNear(const MultipleCollectionAccessor& coll const NamespaceString& nss, const AggregateCommandRequest* aggRequest, Pipeline* pipeline) { + // $geoNear can only run over the main collection. const auto& collection = collections.getMainCollection(); uassert(ErrorCodes::NamespaceNotFound, str::stream() << "$geoNear requires a geo index to run, but " << nss.ns() diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp index 20f24c8b8e0..797a61f064e 100644 --- a/src/mongo/db/query/get_executor.cpp +++ b/src/mongo/db/query/get_executor.cpp @@ -974,9 +974,6 @@ public: std::pair, stage_builder::PlanStageData> buildExecutableTree( const QuerySolution& solution) const final { - // TODO SERVER-62677 We don't pass '_collections' to the function below because at the - // moment, no pushdown is actually happening. This should be changed once the logic for - // pushdown is implemented. return stage_builder::buildSlotBasedExecutableTree( _opCtx, _collections, *_cq, solution, _yieldPolicy); } @@ -997,10 +994,9 @@ protected: } invariant(descriptor->getEntry()); - const auto& mainColl = _collections.getMainCollection(); std::unique_ptr root = [&]() { - auto ixScan = std::make_unique( - indexEntryFromIndexCatalogEntry(_opCtx, mainColl, *descriptor->getEntry(), _cq)); + auto ixScan = std::make_unique(indexEntryFromIndexCatalogEntry( + _opCtx, _collections.getMainCollection(), *descriptor->getEntry(), _cq)); const auto bsonKey = IndexBoundsBuilder::objFromElement(_cq->getQueryObj()["_id"], _cq->getCollator()); @@ -1325,7 +1321,7 @@ StatusWith> getSlotBasedExe // Prepare the SBE tree for execution. stage_builder::prepareSlotBasedExecutableTree( - opCtx, root.get(), &data, *cq, *mainColl, yieldPolicy.get()); + opCtx, root.get(), &data, *cq, collections, yieldPolicy.get()); return plan_executor_factory::make(opCtx, std::move(cq), diff --git a/src/mongo/db/query/sbe_cached_solution_planner.cpp b/src/mongo/db/query/sbe_cached_solution_planner.cpp index aea87407908..032033c1454 100644 --- a/src/mongo/db/query/sbe_cached_solution_planner.cpp +++ b/src/mongo/db/query/sbe_cached_solution_planner.cpp @@ -172,10 +172,10 @@ CandidatePlans CachedSolutionPlanner::replan(bool shouldCache, std::string reaso // Therefore, if any of the collection's indexes have been dropped, the query should fail with // a 'QueryPlanKilled' error. _indexExistenceChecker.check(); - const auto& mainColl = _collections.getMainCollection(); if (shouldCache) { // Deactivate the current cache entry. + const auto& mainColl = _collections.getMainCollection(); auto cache = CollectionQueryInfo::get(mainColl).getPlanCache(); cache->deactivate(plan_cache_key_factory::make(_cq, mainColl)); } diff --git a/src/mongo/db/query/sbe_multi_planner.cpp b/src/mongo/db/query/sbe_multi_planner.cpp index 8a53a3fb952..5f2104a0668 100644 --- a/src/mongo/db/query/sbe_multi_planner.cpp +++ b/src/mongo/db/query/sbe_multi_planner.cpp @@ -121,12 +121,8 @@ CandidatePlans MultiPlanner::finalizeExecutionPlans( } winner.root = winner.clonedPlan->first->clone(); - stage_builder::prepareSlotBasedExecutableTree(_opCtx, - winner.root.get(), - &winner.data, - _cq, - _collections.getMainCollection(), - _yieldPolicy); + stage_builder::prepareSlotBasedExecutableTree( + _opCtx, winner.root.get(), &winner.data, _cq, _collections, _yieldPolicy); // Clear the results queue. winner.results = {}; winner.root->open(false); @@ -152,7 +148,7 @@ CandidatePlans MultiPlanner::finalizeExecutionPlans( auto [rootStage, data] = stage_builder::buildSlotBasedExecutableTree( _opCtx, _collections, _cq, *solution, _yieldPolicy); stage_builder::prepareSlotBasedExecutableTree( - _opCtx, rootStage.get(), &data, _cq, _collections.getMainCollection(), _yieldPolicy); + _opCtx, rootStage.get(), &data, _cq, _collections, _yieldPolicy); candidates[winnerIdx] = sbe::plan_ranker::CandidatePlan{ std::move(solution), std::move(rootStage), std::move(data)}; candidates[winnerIdx].root->open(false); diff --git a/src/mongo/db/query/sbe_runtime_planner.cpp b/src/mongo/db/query/sbe_runtime_planner.cpp index f92a7a0dcda..1ed0c450a95 100644 --- a/src/mongo/db/query/sbe_runtime_planner.cpp +++ b/src/mongo/db/query/sbe_runtime_planner.cpp @@ -92,7 +92,7 @@ BaseRuntimePlanner::prepareExecutionPlan(PlanStage* root, invariant(data); stage_builder::prepareSlotBasedExecutableTree( - _opCtx, root, data, _cq, _collections.getMainCollection(), _yieldPolicy); + _opCtx, root, data, _cq, _collections, _yieldPolicy); value::SlotAccessor* resultSlot{nullptr}; if (auto slot = data->outputs.getIfExists(stage_builder::PlanStageSlots::kResult); slot) { diff --git a/src/mongo/db/query/sbe_stage_builder.cpp b/src/mongo/db/query/sbe_stage_builder.cpp index 1bf6d589465..4555264191f 100644 --- a/src/mongo/db/query/sbe_stage_builder.cpp +++ b/src/mongo/db/query/sbe_stage_builder.cpp @@ -397,7 +397,7 @@ void prepareSlotBasedExecutableTree(OperationContext* opCtx, sbe::PlanStage* root, PlanStageData* data, const CanonicalQuery& cq, - const CollectionPtr& collection, + const MultipleCollectionAccessor& collections, PlanYieldPolicySBE* yieldPolicy) { tassert(6183502, "PlanStage cannot be null", root); tassert(6142205, "PlanStageData cannot be null", data); @@ -421,6 +421,7 @@ void prepareSlotBasedExecutableTree(OperationContext* opCtx, // Populate/renew "shardFilterer" if there exists a "shardFilterer" slot. The slot value should // be set to Nothing in the plan cache to avoid extending the lifetime of the ownership filter. if (auto shardFiltererSlot = data->env->getSlotIfExists("shardFilterer"_sd)) { + const auto& collection = collections.getMainCollection(); tassert(6108307, "Setting shard filterer slot on un-sharded collection", collection.isSharded()); diff --git a/src/mongo/db/query/sbe_stage_builder.h b/src/mongo/db/query/sbe_stage_builder.h index b97d7406be4..f5a0904a773 100644 --- a/src/mongo/db/query/sbe_stage_builder.h +++ b/src/mongo/db/query/sbe_stage_builder.h @@ -59,7 +59,7 @@ void prepareSlotBasedExecutableTree(OperationContext* opCtx, sbe::PlanStage* root, PlanStageData* data, const CanonicalQuery& cq, - const CollectionPtr& collection, + const MultipleCollectionAccessor& collections, PlanYieldPolicySBE* yieldPolicy); class PlanStageReqs; -- cgit v1.2.1