diff options
author | David Percy <david.percy@mongodb.com> | 2022-04-11 21:20:57 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2022-04-18 17:49:56 +0000 |
commit | 3197971a3b481538c485c40f77199f7780747613 (patch) | |
tree | 9def93f22517e1617ae14e83fae0d83327fc9e1a | |
parent | ef19a6cd0bff73be9314a46ca92d4ac82e30b9fd (diff) | |
download | mongo-3197971a3b481538c485c40f77199f7780747613.tar.gz |
SERVER-35512 Use unique_ptr in QuerySolutionNode::children and clone()
20 files changed, 332 insertions, 405 deletions
diff --git a/src/mongo/db/exec/plan_cache_util.cpp b/src/mongo/db/exec/plan_cache_util.cpp index 14745c4d128..80b9ece6f42 100644 --- a/src/mongo/db/exec/plan_cache_util.cpp +++ b/src/mongo/db/exec/plan_cache_util.cpp @@ -189,7 +189,7 @@ plan_cache_debug_info::DebugInfoSBE buildDebugInfo(const QuerySolution* solution } for (auto&& child : node->children) { - queue.push(child); + queue.push(child.get()); } } diff --git a/src/mongo/db/query/classic_stage_builder.cpp b/src/mongo/db/query/classic_stage_builder.cpp index e3fe78eb3b7..19aee75c1ca 100644 --- a/src/mongo/db/query/classic_stage_builder.cpp +++ b/src/mongo/db/query/classic_stage_builder.cpp @@ -117,13 +117,13 @@ std::unique_ptr<PlanStage> ClassicStageBuilder::build(const QuerySolutionNode* r } case STAGE_FETCH: { const FetchNode* fn = static_cast<const FetchNode*>(root); - auto childStage = build(fn->children[0]); + auto childStage = build(fn->children[0].get()); return std::make_unique<FetchStage>( expCtx, _ws, std::move(childStage), fn->filter.get(), _collection); } case STAGE_SORT_DEFAULT: { auto snDefault = static_cast<const SortNodeDefault*>(root); - auto childStage = build(snDefault->children[0]); + auto childStage = build(snDefault->children[0].get()); return std::make_unique<SortStageDefault>( _cq.getExpCtx(), _ws, @@ -135,7 +135,7 @@ std::unique_ptr<PlanStage> ClassicStageBuilder::build(const QuerySolutionNode* r } case STAGE_SORT_SIMPLE: { auto snSimple = static_cast<const SortNodeSimple*>(root); - auto childStage = build(snSimple->children[0]); + auto childStage = build(snSimple->children[0].get()); return std::make_unique<SortStageSimple>( _cq.getExpCtx(), _ws, @@ -147,19 +147,19 @@ std::unique_ptr<PlanStage> ClassicStageBuilder::build(const QuerySolutionNode* r } case STAGE_SORT_KEY_GENERATOR: { const SortKeyGeneratorNode* keyGenNode = static_cast<const SortKeyGeneratorNode*>(root); - auto childStage = build(keyGenNode->children[0]); + auto childStage = build(keyGenNode->children[0].get()); return std::make_unique<SortKeyGeneratorStage>( _cq.getExpCtx(), std::move(childStage), _ws, keyGenNode->sortSpec); } case STAGE_RETURN_KEY: { auto returnKeyNode = static_cast<const ReturnKeyNode*>(root); - auto childStage = build(returnKeyNode->children[0]); + auto childStage = build(returnKeyNode->children[0].get()); return std::make_unique<ReturnKeyStage>( expCtx, std::move(returnKeyNode->sortKeyMetaFields), _ws, std::move(childStage)); } case STAGE_PROJECTION_DEFAULT: { auto pn = static_cast<const ProjectionNodeDefault*>(root); - auto childStage = build(pn->children[0]); + auto childStage = build(pn->children[0].get()); return std::make_unique<ProjectionStageDefault>( _cq.getExpCtx(), _cq.getFindCommandRequest().getProjection(), @@ -169,7 +169,7 @@ std::unique_ptr<PlanStage> ClassicStageBuilder::build(const QuerySolutionNode* r } case STAGE_PROJECTION_COVERED: { auto pn = static_cast<const ProjectionNodeCovered*>(root); - auto childStage = build(pn->children[0]); + auto childStage = build(pn->children[0].get()); return std::make_unique<ProjectionStageCovered>( _cq.getExpCtxRaw(), _cq.getFindCommandRequest().getProjection(), @@ -180,7 +180,7 @@ std::unique_ptr<PlanStage> ClassicStageBuilder::build(const QuerySolutionNode* r } case STAGE_PROJECTION_SIMPLE: { auto pn = static_cast<const ProjectionNodeSimple*>(root); - auto childStage = build(pn->children[0]); + auto childStage = build(pn->children[0].get()); return std::make_unique<ProjectionStageSimple>( _cq.getExpCtxRaw(), _cq.getFindCommandRequest().getProjection(), @@ -190,19 +190,19 @@ std::unique_ptr<PlanStage> ClassicStageBuilder::build(const QuerySolutionNode* r } case STAGE_LIMIT: { const LimitNode* ln = static_cast<const LimitNode*>(root); - auto childStage = build(ln->children[0]); + auto childStage = build(ln->children[0].get()); return std::make_unique<LimitStage>(expCtx, ln->limit, _ws, std::move(childStage)); } case STAGE_SKIP: { const SkipNode* sn = static_cast<const SkipNode*>(root); - auto childStage = build(sn->children[0]); + auto childStage = build(sn->children[0].get()); return std::make_unique<SkipStage>(expCtx, sn->skip, _ws, std::move(childStage)); } case STAGE_AND_HASH: { const AndHashNode* ahn = static_cast<const AndHashNode*>(root); auto ret = std::make_unique<AndHashStage>(expCtx, _ws); for (size_t i = 0; i < ahn->children.size(); ++i) { - auto childStage = build(ahn->children[i]); + auto childStage = build(ahn->children[i].get()); ret->addChild(std::move(childStage)); } return ret; @@ -211,7 +211,7 @@ std::unique_ptr<PlanStage> ClassicStageBuilder::build(const QuerySolutionNode* r const OrNode* orn = static_cast<const OrNode*>(root); auto ret = std::make_unique<OrStage>(expCtx, _ws, orn->dedup, orn->filter.get()); for (size_t i = 0; i < orn->children.size(); ++i) { - auto childStage = build(orn->children[i]); + auto childStage = build(orn->children[i].get()); ret->addChild(std::move(childStage)); } return ret; @@ -220,7 +220,7 @@ std::unique_ptr<PlanStage> ClassicStageBuilder::build(const QuerySolutionNode* r const AndSortedNode* asn = static_cast<const AndSortedNode*>(root); auto ret = std::make_unique<AndSortedStage>(expCtx, _ws); for (size_t i = 0; i < asn->children.size(); ++i) { - auto childStage = build(asn->children[i]); + auto childStage = build(asn->children[i].get()); ret->addChild(std::move(childStage)); } return ret; @@ -233,7 +233,7 @@ std::unique_ptr<PlanStage> ClassicStageBuilder::build(const QuerySolutionNode* r params.collator = _cq.getCollator(); auto ret = std::make_unique<MergeSortStage>(expCtx, params, _ws); for (size_t i = 0; i < msn->children.size(); ++i) { - auto childStage = build(msn->children[i]); + auto childStage = build(msn->children[i].get()); ret->addChild(std::move(childStage)); } return ret; @@ -281,8 +281,8 @@ std::unique_ptr<PlanStage> ClassicStageBuilder::build(const QuerySolutionNode* r auto node = static_cast<const TextOrNode*>(root); auto ret = std::make_unique<TextOrStage>( expCtx, *_ftsKeyPrefixSize, _ws, node->filter.get(), _collection); - for (auto childNode : root->children) { - ret->addChild(build(childNode)); + for (auto&& childNode : root->children) { + ret->addChild(build(childNode.get())); } return ret; } @@ -313,11 +313,12 @@ std::unique_ptr<PlanStage> ClassicStageBuilder::build(const QuerySolutionNode* r _ftsKeyPrefixSize.emplace(params.spec.numExtraBefore()); ON_BLOCK_EXIT([&] { _ftsKeyPrefixSize = {}; }); - return std::make_unique<TextMatchStage>(expCtx, build(root->children[0]), params, _ws); + return std::make_unique<TextMatchStage>( + expCtx, build(root->children[0].get()), params, _ws); } case STAGE_SHARDING_FILTER: { const ShardingFilterNode* fn = static_cast<const ShardingFilterNode*>(root); - auto childStage = build(fn->children[0]); + auto childStage = build(fn->children[0].get()); auto css = CollectionShardingState::get(_opCtx, _collection->ns()); return std::make_unique<ShardFilterStage>( diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp index 2a5fcad2cc8..8094a658374 100644 --- a/src/mongo/db/query/get_executor.cpp +++ b/src/mongo/db/query/get_executor.cpp @@ -1049,7 +1049,7 @@ protected: initializePlannerParamsIfNeeded(); if (_plannerParams.options & QueryPlannerParams::INCLUDE_SHARD_FILTER) { auto shardFilter = std::make_unique<ShardingFilterNode>(); - shardFilter->children.push_back(root.release()); + shardFilter->children.push_back(std::move(root)); root = std::move(shardFilter); } @@ -1927,7 +1927,7 @@ bool turnIxscanIntoCount(QuerySolution* soln) { } IndexScanNode* isn = (STAGE_FETCH == root->getType()) - ? static_cast<IndexScanNode*>(root->children[0]) + ? static_cast<IndexScanNode*>(root->children[0].get()) : static_cast<IndexScanNode*>(root); // No filters allowed and side-stepping isSimpleRange for now. TODO: do we ever see @@ -2240,13 +2240,13 @@ bool turnIxscanIntoDistinctIxscan(QuerySolution* soln, } if (!fetchNode && (STAGE_FETCH == root->children[0]->getType())) { - fetchNode = static_cast<FetchNode*>(root->children[0]); + fetchNode = static_cast<FetchNode*>(root->children[0].get()); } if (fetchNode && (STAGE_IXSCAN == fetchNode->children[0]->getType())) { - indexScanNode = static_cast<IndexScanNode*>(fetchNode->children[0]); + indexScanNode = static_cast<IndexScanNode*>(fetchNode->children[0].get()); } else if (projectNode && (STAGE_IXSCAN == projectNode->children[0]->getType())) { - indexScanNode = static_cast<IndexScanNode*>(projectNode->children[0]); + indexScanNode = static_cast<IndexScanNode*>(projectNode->children[0].get()); } if (!indexScanNode) { @@ -2345,36 +2345,22 @@ bool turnIxscanIntoDistinctIxscan(QuerySolution* soln, // transforming the plan from PROJECT=>FETCH=>IXSCAN to FETCH=>DISTINCT_SCAN. if (projectNode) { invariant(projectNode == root); - projectNode = nullptr; - + invariant(fetchNode == root->children[0].get()); invariant(STAGE_FETCH == root->children[0]->getType()); invariant(STAGE_IXSCAN == root->children[0]->children[0]->getType()); - - // Detach the fetch from its parent projection. - root->children.clear(); - // Make the fetch the new root. This destroys the project stage. - soln->setRoot(std::unique_ptr<QuerySolutionNode>(fetchNode)); + soln->setRoot(std::move(root->children[0])); } - // Whenver we have a FETCH node, the IXSCAN is its child. We detach the IXSCAN from the - // solution tree and take ownership of it, so that it gets destroyed when we leave this - // scope. - std::unique_ptr<IndexScanNode> ownedIsn(indexScanNode); - indexScanNode = nullptr; - // Attach the distinct node in the index scan's place. - fetchNode->children[0] = distinctNode.release(); + fetchNode->children[0] = std::move(distinctNode); } else { // There is no fetch node. The PROJECT=>IXSCAN tree should become PROJECT=>DISTINCT_SCAN. invariant(projectNode == root); invariant(STAGE_IXSCAN == root->children[0]->getType()); - // Take ownership of the index scan node, detaching it from the solution tree. - std::unique_ptr<IndexScanNode> ownedIsn(indexScanNode); - // Attach the distinct node in the index scan's place. - root->children[0] = distinctNode.release(); + root->children[0] = std::move(distinctNode); } return true; @@ -2500,7 +2486,7 @@ StatusWith<std::unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorForS if (plannerParams.indices[distinctNodeIndex].collator) { if (!solnRoot->fetched()) { auto fetch = std::make_unique<FetchNode>(); - fetch->children.push_back(solnRoot.release()); + fetch->children.push_back(std::move(solnRoot)); solnRoot = std::move(fetch); } } diff --git a/src/mongo/db/query/plan_explainer_sbe.cpp b/src/mongo/db/query/plan_explainer_sbe.cpp index 2f8f5b43b08..a7482873eb8 100644 --- a/src/mongo/db/query/plan_explainer_sbe.cpp +++ b/src/mongo/db/query/plan_explainer_sbe.cpp @@ -197,7 +197,7 @@ void statsToBSON(const QuerySolutionNode* node, // rather than 'inputStages'. if (node->children.size() == 1) { BSONObjBuilder childBob(bob->subobjStart("inputStage")); - statsToBSON(node->children[0], &childBob, topLevelBob); + statsToBSON(node->children[0].get(), &childBob, topLevelBob); return; } @@ -206,7 +206,7 @@ void statsToBSON(const QuerySolutionNode* node, BSONArrayBuilder childrenBob(bob->subarrayStart("inputStages")); for (auto&& child : node->children) { BSONObjBuilder childBob(childrenBob.subobjStart()); - statsToBSON(child, &childBob, topLevelBob); + statsToBSON(child.get(), &childBob, topLevelBob); } childrenBob.doneFast(); } diff --git a/src/mongo/db/query/planner_access.cpp b/src/mongo/db/query/planner_access.cpp index 0415155cce9..506493a3437 100644 --- a/src/mongo/db/query/planner_access.cpp +++ b/src/mongo/db/query/planner_access.cpp @@ -65,13 +65,6 @@ namespace wcp = ::mongo::wildcard_planning; namespace dps = ::mongo::dotted_path_support; /** - * Text node functors. - */ -bool isTextNode(const QuerySolutionNode* node) { - return STAGE_TEXT_MATCH == node->getType(); -} - -/** * Casts 'node' to a FetchNode* if it is a FetchNode, otherwise returns null. */ FetchNode* getFetchNode(QuerySolutionNode* node) { @@ -92,7 +85,7 @@ const IndexScanNode* getIndexScanNode(const QuerySolutionNode* node) { return static_cast<const IndexScanNode*>(node); } else if (STAGE_FETCH == node->getType()) { invariant(1U == node->children.size()); - const QuerySolutionNode* child = node->children[0]; + const QuerySolutionNode* child = node->children[0].get(); if (STAGE_IXSCAN == child->getType()) { return static_cast<const IndexScanNode*>(child); } @@ -790,11 +783,8 @@ void buildTextSubPlan(TextMatchNode* tn) { // compute their text scores. This is a blocking operation. auto textScorer = std::make_unique<TextOrNode>(); textScorer->filter = std::move(tn->filter); - for (auto&& ixscan : indexScanList) { - textScorer->children.push_back(ixscan.release()); - } - - tn->children.push_back(textScorer.release()); + textScorer->addChildren(std::move(indexScanList)); + tn->children.push_back(std::move(textScorer)); } else { // Because we don't need the text score, we can use a non-blocking OR stage to get the union // of the index scans or use the index scan directly if there is only one. @@ -808,10 +798,8 @@ void buildTextSubPlan(TextMatchNode* tn) { } else { auto orTextSearcher = std::make_unique<OrNode>(); orTextSearcher->filter = std::move(tn->filter); - for (auto&& ixscan : indexScanList) { - orTextSearcher->children.push_back(ixscan.release()); - } - return std::move(orTextSearcher); + orTextSearcher->addChildren(std::move(indexScanList)); + return orTextSearcher; } }(); @@ -819,9 +807,9 @@ void buildTextSubPlan(TextMatchNode* tn) { // add our own FETCH stage to satisfy the requirement of the TEXT_MATCH stage that its // WorkingSetMember inputs have fetched data. auto fetchNode = std::make_unique<FetchNode>(); - fetchNode->children.push_back(textSearcher.release()); + fetchNode->children.push_back(std::move(textSearcher)); - tn->children.push_back(fetchNode.release()); + tn->children.push_back(std::move(fetchNode)); } } @@ -932,7 +920,7 @@ void QueryPlannerAccess::finishAndOutputLeaf(ScanBuildingState* scanState, // Takes ownership. fetch->filter = std::move(scanState->curOr); // Takes ownership. - fetch->children.push_back(scanState->currentScan.release()); + fetch->children.push_back(std::move(scanState->currentScan)); scanState->currentScan = std::move(fetch); } else if (scanState->loosestBounds == IndexBoundsBuilder::INEXACT_COVERED) { @@ -1527,8 +1515,7 @@ std::unique_ptr<QuerySolutionNode> QueryPlannerAccess::buildIndexedAnd( invariant(clonedRoot); auto fetch = std::make_unique<FetchNode>(); fetch->filter = std::move(clonedRoot); - // Takes ownership of 'andResult'. - fetch->children.push_back(andResult.release()); + fetch->children.push_back(std::move(andResult)); return fetch; } @@ -1547,7 +1534,7 @@ std::unique_ptr<QuerySolutionNode> QueryPlannerAccess::buildIndexedAnd( fetch->filter = std::move(ownedRoot); } // takes ownership - fetch->children.push_back(andResult.release()); + fetch->children.push_back(std::move(andResult)); andResult = std::move(fetch); } else { // root has no children, let autoRoot get rid of it when it goes out of scope. @@ -1622,7 +1609,9 @@ std::unique_ptr<QuerySolutionNode> QueryPlannerAccess::buildIndexedOr( // Evaluate text nodes first to ensure that text scores are available. // Move text nodes to front of vector. - std::stable_partition(orResult->children.begin(), orResult->children.end(), isTextNode); + std::stable_partition(orResult->children.begin(), orResult->children.end(), [](auto&& child) { + return STAGE_TEXT_MATCH == child->getType(); + }); // OR must have an index for each child, so we should have detached all children from // 'root', and there's nothing useful to do with an empty or MatchExpression. We let it die @@ -1711,7 +1700,7 @@ std::unique_ptr<QuerySolutionNode> QueryPlannerAccess::_buildIndexedDataAccess( } else { auto fetch = std::make_unique<FetchNode>(); fetch->filter = std::move(ownedRoot); - fetch->children.push_back(soln.release()); + fetch->children.push_back(std::move(soln)); return fetch; } } else if (Indexability::arrayUsesIndexOnChildren(root)) { @@ -1735,7 +1724,7 @@ std::unique_ptr<QuerySolutionNode> QueryPlannerAccess::_buildIndexedDataAccess( auto fetch = std::make_unique<FetchNode>(); fetch->filter = std::move(ownedRoot); - fetch->children.push_back(solution.release()); + fetch->children.push_back(std::move(solution)); return fetch; } } @@ -1772,7 +1761,7 @@ std::unique_ptr<QuerySolutionNode> QueryPlannerAccess::scanWholeIndex( // for now it's safe (though *maybe* slower). unique_ptr<FetchNode> fetch = std::make_unique<FetchNode>(); fetch->filter = std::move(filter); - fetch->children.push_back(isn.release()); + fetch->children.push_back(std::move(isn)); solnRoot = std::move(fetch); } @@ -1903,7 +1892,7 @@ std::unique_ptr<QuerySolutionNode> QueryPlannerAccess::makeIndexScan( // for now it's safe (though *maybe* slower). unique_ptr<FetchNode> fetch = std::make_unique<FetchNode>(); fetch->filter = std::move(filter); - fetch->children.push_back(isn.release()); + fetch->children.push_back(std::move(isn)); solnRoot = std::move(fetch); } diff --git a/src/mongo/db/query/planner_analysis.cpp b/src/mongo/db/query/planner_analysis.cpp index 654fa0f56ca..c294ce7de17 100644 --- a/src/mongo/db/query/planner_analysis.cpp +++ b/src/mongo/db/query/planner_analysis.cpp @@ -68,7 +68,7 @@ void getLeafNodes(QuerySolutionNode* root, vector<QuerySolutionNode*>* leafNodes leafNodes->push_back(root); } else { for (size_t i = 0; i < root->children.size(); ++i) { - getLeafNodes(root->children[i], leafNodes); + getLeafNodes(root->children[i].get(), leafNodes); } } } @@ -91,7 +91,7 @@ void getExplodableNodes(QuerySolutionNode* root, vector<QuerySolutionNode*>* exp explodableNodes->push_back(root); } else { for (auto&& childNode : root->children) { - getExplodableNodes(childNode, explodableNodes); + getExplodableNodes(childNode.get(), explodableNodes); } } } @@ -104,7 +104,7 @@ const IndexScanNode* getIndexScanNode(const QuerySolutionNode* node) { if (STAGE_IXSCAN == node->getType()) { return static_cast<const IndexScanNode*>(node); } else if (isFetchNodeWithIndexScanChild(node)) { - return static_cast<const IndexScanNode*>(node->children[0]); + return static_cast<const IndexScanNode*>(node->children[0].get()); } MONGO_UNREACHABLE; return nullptr; @@ -132,10 +132,12 @@ bool isUnionOfPoints(const OrderedIntervalList& oil) { /** * Should we try to expand the index scan(s) in 'solnRoot' to pull out an indexed sort? * - * Returns the node which should be replaced by the merge sort of exploded scans - * in the out-parameter 'toReplace'. + * Returns the node which should be replaced by the merge sort of exploded scans, or nullptr + * if there is no such node. The result is a pointer to a unique_ptr so that the caller + * can replace the unique_ptr. */ -bool structureOKForExplode(QuerySolutionNode* solnRoot, QuerySolutionNode** toReplace) { +std::unique_ptr<QuerySolutionNode>* structureOKForExplode( + std::unique_ptr<QuerySolutionNode>* solnRoot) { // For now we only explode if we *know* we will pull the sort out. We can look at // more structure (or just explode and recalculate properties and see what happens) // but for now we just explode if it's a sure bet. @@ -144,33 +146,30 @@ bool structureOKForExplode(QuerySolutionNode* solnRoot, QuerySolutionNode** toRe // or other less obvious cases... // Skip over a sharding filter stage. - if (STAGE_SHARDING_FILTER == solnRoot->getType()) { - solnRoot = solnRoot->children[0]; + if (STAGE_SHARDING_FILTER == (*solnRoot)->getType()) { + solnRoot = &(*solnRoot)->children[0]; } - if (STAGE_IXSCAN == solnRoot->getType()) { - *toReplace = solnRoot; - return true; + if (STAGE_IXSCAN == (*solnRoot)->getType()) { + return solnRoot; } - if (isFetchNodeWithIndexScanChild(solnRoot)) { - *toReplace = solnRoot->children[0]; - return true; + if (isFetchNodeWithIndexScanChild(solnRoot->get())) { + return &(*solnRoot)->children[0]; } // If we have a STAGE_OR, we can explode only when all children are either IXSCANs or FETCHes // that have an IXSCAN as a child. - if (STAGE_OR == solnRoot->getType()) { - for (auto&& child : solnRoot->children) { - if (STAGE_IXSCAN != child->getType() && !isFetchNodeWithIndexScanChild(child)) { - return false; + if (STAGE_OR == (*solnRoot)->getType()) { + for (auto&& child : (*solnRoot)->children) { + if (STAGE_IXSCAN != child->getType() && !isFetchNodeWithIndexScanChild(child.get())) { + return nullptr; } } - *toReplace = solnRoot; - return true; + return solnRoot; } - return false; + return nullptr; } // vectors of vectors can be > > annoying. @@ -223,10 +222,10 @@ void makeCartesianProduct(const IndexBounds& bounds, /** * Takes the provided 'node', either an IndexScanNode or FetchNode with a direct child that is an - * IndexScanNode. Returns a list of nodes which are logically equivalent to 'node' if joined by a - * MergeSort through the out-parameter 'explosionResult'. These nodes are owned by the caller. + * IndexScanNode. Produces a list of new nodes, which are logically equivalent to 'node' if joined + * by a MergeSort. Inserts these new nodes at the end of 'explosionResult'. * - * fieldsToExplode is a count of how many fields in the scan's bounds are the union of point + * 'fieldsToExplode' is a count of how many fields in the scan's bounds are the union of point * intervals. This is computed beforehand and provided as a small optimization. * * Example: @@ -243,7 +242,7 @@ void makeCartesianProduct(const IndexBounds& bounds, void explodeNode(const QuerySolutionNode* node, const BSONObj& sort, size_t fieldsToExplode, - vector<QuerySolutionNode*>* explosionResult) { + vector<std::unique_ptr<QuerySolutionNode>>* explosionResult) { // Get the 'isn' from either the FetchNode or IndexScanNode. const IndexScanNode* isn = getIndexScanNode(node); @@ -256,7 +255,7 @@ void explodeNode(const QuerySolutionNode* node, verify(prefix.size() == fieldsToExplode); // Copy boring fields into new child. - IndexScanNode* child = new IndexScanNode(isn->index); + auto child = std::make_unique<IndexScanNode>(isn->index); child->direction = isn->direction; child->addKeyMetadata = isn->addKeyMetadata; child->queryCollator = isn->queryCollator; @@ -287,25 +286,10 @@ void explodeNode(const QuerySolutionNode* node, } // Add the 'child' IXSCAN under the FETCH stage, and the FETCH stage to the result set. - newFetchNode->children.push_back(child); - explosionResult->push_back(newFetchNode.release()); + newFetchNode->children.push_back(std::move(child)); + explosionResult->push_back(std::move(newFetchNode)); } else { - explosionResult->push_back(child); - } - } -} - -/** - * In the tree '*root', replace 'oldNode' with 'newNode'. - */ -void replaceNodeInTree(QuerySolutionNode** root, - QuerySolutionNode* oldNode, - QuerySolutionNode* newNode) { - if (*root == oldNode) { - *root = newNode; - } else { - for (size_t i = 0; i < (*root)->children.size(); ++i) { - replaceNodeInTree(&(*root)->children[i], oldNode, newNode); + explosionResult->push_back(std::move(child)); } } } @@ -328,8 +312,8 @@ void geoSkipValidationOn(const std::set<StringData>& twoDSphereFields, } } - for (QuerySolutionNode* child : solnRoot->children) { - geoSkipValidationOn(twoDSphereFields, child); + for (auto&& child : solnRoot->children) { + geoSkipValidationOn(twoDSphereFields, child.get()); } } @@ -374,7 +358,7 @@ std::unique_ptr<QuerySolutionNode> addSortKeyGeneratorStageIfNeeded( if (!hasSortStage && query.metadataDeps()[DocumentMetadataFields::kSortKey]) { auto keyGenNode = std::make_unique<SortKeyGeneratorNode>(); keyGenNode->sortSpec = query.getFindCommandRequest().getSort(); - keyGenNode->children.push_back(solnRoot.release()); + keyGenNode->children.push_back(std::move(solnRoot)); return keyGenNode; } return solnRoot; @@ -395,7 +379,7 @@ std::unique_ptr<ProjectionNode> analyzeProjection(const CanonicalQuery& query, (query.getProj()->requiresDocument() || (!providesAllFields(query.getProj()->getRequiredFields(), *solnRoot)))) { auto fetch = std::make_unique<FetchNode>(); - fetch->children.push_back(solnRoot.release()); + fetch->children.push_back(std::move(solnRoot)); solnRoot = std::move(fetch); } @@ -461,10 +445,10 @@ std::unique_ptr<QuerySolutionNode> tryPushdownProjectBeneathSort( // PROJECT => SKIP => SORT // In this case we still want to push PROJECT beneath SORT. bool hasSkipBetween = false; - auto sortNodeCandidate = projectNode->children[0]; + QuerySolutionNode* sortNodeCandidate = projectNode->children[0].get(); if (sortNodeCandidate->getType() == STAGE_SKIP) { hasSkipBetween = true; - sortNodeCandidate = sortNodeCandidate->children[0]; + sortNodeCandidate = sortNodeCandidate->children[0].get(); } if (!isSortStageType(sortNodeCandidate->getType())) { @@ -498,7 +482,7 @@ std::unique_ptr<QuerySolutionNode> tryPushdownProjectBeneathSort( // SKIP => SORT => PROJECT => CHILD // // First, detach the bottom of the tree. This part is CHILD in the comment above. - std::unique_ptr<QuerySolutionNode> restOfTree{sortNode->children[0]}; + std::unique_ptr<QuerySolutionNode> restOfTree = std::move(sortNode->children[0]); invariant(sortNode->children.size() == 1u); sortNode->children.clear(); @@ -507,7 +491,7 @@ std::unique_ptr<QuerySolutionNode> tryPushdownProjectBeneathSort( // SORT // Or this if we have SKIP: // SKIP => SORT - std::unique_ptr<QuerySolutionNode> ownedProjectionInput{projectNode->children[0]}; + std::unique_ptr<QuerySolutionNode> ownedProjectionInput = std::move(projectNode->children[0]); sortNode = nullptr; invariant(projectNode->children.size() == 1u); projectNode->children.clear(); @@ -516,19 +500,19 @@ std::unique_ptr<QuerySolutionNode> tryPushdownProjectBeneathSort( // We want to get the following structure: // PROJECT => CHILD std::unique_ptr<QuerySolutionNode> ownedProjectionNode = std::move(root); - ownedProjectionNode->children.push_back(restOfTree.release()); + ownedProjectionNode->children.push_back(std::move(restOfTree)); // Attach the projection as the child of the sort stage. if (hasSkipBetween) { // In this case 'ownedProjectionInput' points to the structure: // SKIP => SORT // And to attach PROJECT => CHILD to it, we need to access children of SORT stage. - ownedProjectionInput->children[0]->children.push_back(ownedProjectionNode.release()); + ownedProjectionInput->children[0]->children.push_back(std::move(ownedProjectionNode)); } else { // In this case 'ownedProjectionInput' points to the structure: // SORT // And we can just add PROJECT => CHILD to its children. - ownedProjectionInput->children.push_back(ownedProjectionNode.release()); + ownedProjectionInput->children.push_back(std::move(ownedProjectionNode)); } // Re-compute properties so that they reflect the new structure of the tree. @@ -565,7 +549,7 @@ void removeProjectSimpleBelowGroupRecursive(QuerySolutionNode* solnRoot) { if (solnRoot->getType() == StageType::STAGE_GROUP) { auto groupNode = static_cast<GroupNode*>(solnRoot); - auto projectNodeCandidate = groupNode->children[0]; + QuerySolutionNode* projectNodeCandidate = groupNode->children[0].get(); if (projectNodeCandidate->getType() == StageType::STAGE_GROUP) { // Multiple $group stages may be pushed down. So, if the child is a GROUP, then recurse. removeProjectSimpleBelowGroupRecursive(projectNodeCandidate); @@ -588,15 +572,11 @@ void removeProjectSimpleBelowGroupRecursive(QuerySolutionNode* solnRoot) { }; // Attach the projectNode's child to the groupNode's child. - groupNode->children[0] = projectNode->children[0]; - projectNode->children[0] = nullptr; - - // Get rid of the project simple node. - delete projectNode; + groupNode->children[0] = std::move(projectNode->children[0]); } else { // Keep traversing the tree in search of a GROUP stage. for (size_t i = 0; i < solnRoot->children.size(); ++i) { - removeProjectSimpleBelowGroupRecursive(solnRoot->children[i]); + removeProjectSimpleBelowGroupRecursive(solnRoot->children[i].get()); } } } @@ -730,16 +710,15 @@ BSONObj QueryPlannerAnalysis::getSortPattern(const BSONObj& indexKeyPattern) { // static bool QueryPlannerAnalysis::explodeForSort(const CanonicalQuery& query, const QueryPlannerParams& params, - QuerySolutionNode** solnRoot) { + std::unique_ptr<QuerySolutionNode>* solnRoot) { vector<QuerySolutionNode*> explodableNodes; - QuerySolutionNode* toReplace; - if (!structureOKForExplode(*solnRoot, &toReplace)) { + std::unique_ptr<QuerySolutionNode>* toReplace = structureOKForExplode(solnRoot); + if (!toReplace) return false; - } // Find explodable nodes in the subtree rooted at 'toReplace'. - getExplodableNodes(toReplace, &explodableNodes); + getExplodableNodes(toReplace->get(), &explodableNodes); const BSONObj& desiredSort = query.getFindCommandRequest().getSort(); @@ -860,7 +839,7 @@ bool QueryPlannerAnalysis::explodeForSort(const CanonicalQuery& query, // If we're here, we can (probably? depends on how restrictive the structure check is) // get our sort order via ixscan blow-up. - MergeSortNode* merge = new MergeSortNode(); + auto merge = std::make_unique<MergeSortNode>(); merge->sort = desiredSort; for (size_t i = 0; i < explodableNodes.size(); ++i) { explodeNode(explodableNodes[i], desiredSort, fieldsToExplode[i], &merge->children); @@ -868,19 +847,17 @@ bool QueryPlannerAnalysis::explodeForSort(const CanonicalQuery& query, merge->computeProperties(); - // Replace 'toReplace' with the new merge sort node. - replaceNodeInTree(solnRoot, toReplace, merge); - // And get rid of the node that got replaced. - delete toReplace; + *toReplace = std::move(merge); return true; } // static -QuerySolutionNode* QueryPlannerAnalysis::analyzeSort(const CanonicalQuery& query, - const QueryPlannerParams& params, - QuerySolutionNode* solnRoot, - bool* blockingSortOut) { +std::unique_ptr<QuerySolutionNode> QueryPlannerAnalysis::analyzeSort( + const CanonicalQuery& query, + const QueryPlannerParams& params, + std::unique_ptr<QuerySolutionNode> solnRoot, + bool* blockingSortOut) { *blockingSortOut = false; const FindCommandRequest& findCommand = query.getFindCommandRequest(); @@ -913,7 +890,7 @@ QuerySolutionNode* QueryPlannerAnalysis::analyzeSort(const CanonicalQuery& query // If so, we can reverse the scan direction(s). BSONObj reverseSort = QueryPlannerCommon::reverseSortObj(sortObj); if (providedSorts.contains(reverseSort)) { - QueryPlannerCommon::reverseScans(solnRoot); + QueryPlannerCommon::reverseScans(solnRoot.get()); LOGV2_DEBUG(20951, 5, "Reversing ixscan to provide sort", @@ -931,19 +908,18 @@ QuerySolutionNode* QueryPlannerAnalysis::analyzeSort(const CanonicalQuery& query // If we're here, we need to add a sort stage. if (!solnRoot->fetched()) { - const bool sortIsCovered = - std::all_of(sortObj.begin(), sortObj.end(), [solnRoot](BSONElement e) { - // Note that hasField() will return 'false' in the case that this field is a string - // and there is a non-simple collation on the index. This will lead to encoding of - // the field from the document on fetch, despite having read the encoded value from - // the index. - return solnRoot->hasField(e.fieldName()); - }); + const bool sortIsCovered = std::all_of(sortObj.begin(), sortObj.end(), [&](BSONElement e) { + // Note that hasField() will return 'false' in the case that this field is a string + // and there is a non-simple collation on the index. This will lead to encoding of + // the field from the document on fetch, despite having read the encoded value from + // the index. + return solnRoot->hasField(e.fieldName()); + }); if (!sortIsCovered) { - FetchNode* fetch = new FetchNode(); - fetch->children.push_back(solnRoot); - solnRoot = fetch; + auto fetch = std::make_unique<FetchNode>(); + fetch->children.push_back(std::move(solnRoot)); + solnRoot = std::move(fetch); } } @@ -954,20 +930,19 @@ QuerySolutionNode* QueryPlannerAnalysis::analyzeSort(const CanonicalQuery& query sortNode = std::make_unique<SortNodeDefault>(); } sortNode->pattern = sortObj; - sortNode->children.push_back(solnRoot); + sortNode->children.push_back(std::move(solnRoot)); sortNode->addSortKeyMetadata = query.metadataDeps()[DocumentMetadataFields::kSortKey]; - solnRoot = sortNode.release(); - auto sortNodeRaw = static_cast<SortNode*>(solnRoot); // When setting the limit on the sort, we need to consider both // the limit N and skip count M. The sort should return an ordered list // N + M items so that the skip stage can discard the first M results. if (findCommand.getLimit()) { // The limit can be combined with the SORT stage. - sortNodeRaw->limit = static_cast<size_t>(*findCommand.getLimit()) + + sortNode->limit = static_cast<size_t>(*findCommand.getLimit()) + static_cast<size_t>(findCommand.getSkip().value_or(0)); } else { - sortNodeRaw->limit = 0; + sortNode->limit = 0; } + solnRoot = std::move(sortNode); *blockingSortOut = true; @@ -1015,19 +990,19 @@ std::unique_ptr<QuerySolution> QueryPlannerAnalysis::analyzeDataAccess( } if (fetch) { - FetchNode* fetchNode = new FetchNode(); - fetchNode->children.push_back(solnRoot.release()); - solnRoot.reset(fetchNode); + auto fetchNode = std::make_unique<FetchNode>(); + fetchNode->children.push_back(std::move(solnRoot)); + solnRoot = std::move(fetchNode); } } - ShardingFilterNode* sfn = new ShardingFilterNode(); - sfn->children.push_back(solnRoot.release()); - solnRoot.reset(sfn); + auto sfn = std::make_unique<ShardingFilterNode>(); + sfn->children.push_back(std::move(solnRoot)); + solnRoot = std::move(sfn); } bool hasSortStage = false; - solnRoot.reset(analyzeSort(query, params, solnRoot.release(), &hasSortStage)); + solnRoot = analyzeSort(query, params, std::move(solnRoot), &hasSortStage); // This can happen if we need to create a blocking sort stage and we're not allowed to. if (!solnRoot) { @@ -1044,7 +1019,7 @@ std::unique_ptr<QuerySolution> QueryPlannerAnalysis::analyzeDataAccess( if (findCommand.getSkip()) { auto skip = std::make_unique<SkipNode>(); skip->skip = *findCommand.getSkip(); - skip->children.push_back(solnRoot.release()); + skip->children.push_back(std::move(solnRoot)); solnRoot = std::move(skip); } @@ -1065,9 +1040,9 @@ std::unique_ptr<QuerySolution> QueryPlannerAnalysis::analyzeDataAccess( // If there's no projection, we must fetch, as the user wants the entire doc. if (!solnRoot->fetched() && !(params.options & QueryPlannerParams::IS_COUNT)) { - FetchNode* fetch = new FetchNode(); - fetch->children.push_back(solnRoot.release()); - solnRoot.reset(fetch); + auto fetch = std::make_unique<FetchNode>(); + fetch->children.push_back(std::move(solnRoot)); + solnRoot = std::move(fetch); } } @@ -1075,10 +1050,10 @@ std::unique_ptr<QuerySolution> QueryPlannerAnalysis::analyzeDataAccess( // sort. Otherwise, we will have to enforce the limit ourselves since it's not handled inside // SORT. if (!hasSortStage && findCommand.getLimit()) { - LimitNode* limit = new LimitNode(); + auto limit = std::make_unique<LimitNode>(); limit->limit = *findCommand.getLimit(); - limit->children.push_back(solnRoot.release()); - solnRoot.reset(limit); + limit->children.push_back(std::move(solnRoot)); + solnRoot = std::move(limit); } solnRoot = tryPushdownProjectBeneathSort(std::move(solnRoot)); diff --git a/src/mongo/db/query/planner_analysis.h b/src/mongo/db/query/planner_analysis.h index 95fa0607d47..9f4a9a598a8 100644 --- a/src/mongo/db/query/planner_analysis.h +++ b/src/mongo/db/query/planner_analysis.h @@ -81,10 +81,11 @@ public: /** * Sort the results, if there is a sort required. */ - static QuerySolutionNode* analyzeSort(const CanonicalQuery& query, - const QueryPlannerParams& params, - QuerySolutionNode* solnRoot, - bool* blockingSortOut); + static std::unique_ptr<QuerySolutionNode> analyzeSort( + const CanonicalQuery& query, + const QueryPlannerParams& params, + std::unique_ptr<QuerySolutionNode> solnRoot, + bool* blockingSortOut); /** * Internal helper function used by analyzeSort. @@ -111,7 +112,7 @@ public: */ static bool explodeForSort(const CanonicalQuery& query, const QueryPlannerParams& params, - QuerySolutionNode** solnRoot); + std::unique_ptr<QuerySolutionNode>* solnRoot); /** * Walks the QuerySolutionNode tree rooted in 'soln', and looks for a ProjectionNodeSimple that diff --git a/src/mongo/db/query/planner_analysis_test.cpp b/src/mongo/db/query/planner_analysis_test.cpp index d980dcb9444..171e91e6cfe 100644 --- a/src/mongo/db/query/planner_analysis_test.cpp +++ b/src/mongo/db/query/planner_analysis_test.cpp @@ -214,7 +214,7 @@ TEST(QueryPlannerAnalysis, GeoSkipValidation) { OrNode orNode; // Takes ownership. - orNode.children.push_back(fetchNodePtr.release()); + orNode.children.push_back(std::move(fetchNodePtr)); // We should not skip validation if there are no indices. QueryPlannerAnalysis::analyzeGeo(params, fetchNode); diff --git a/src/mongo/db/query/query_planner_common.cpp b/src/mongo/db/query/query_planner_common.cpp index 85534e6128c..f5b23b68dad 100644 --- a/src/mongo/db/query/query_planner_common.cpp +++ b/src/mongo/db/query/query_planner_common.cpp @@ -77,7 +77,7 @@ void QueryPlannerCommon::reverseScans(QuerySolutionNode* node) { } for (size_t i = 0; i < node->children.size(); ++i) { - reverseScans(node->children[i]); + reverseScans(node->children[i].get()); } } diff --git a/src/mongo/db/query/query_planner_test_lib.cpp b/src/mongo/db/query/query_planner_test_lib.cpp index bec219226b5..87d34b9b196 100644 --- a/src/mongo/db/query/query_planner_test_lib.cpp +++ b/src/mongo/db/query/query_planner_test_lib.cpp @@ -295,7 +295,7 @@ static Status childrenMatch(const BSONObj& testSoln, continue; } auto matchStatus = QueryPlannerTestLib::solutionMatches( - child.Obj(), trueSoln->children[j], relaxBoundsCheck); + child.Obj(), trueSoln->children[j].get(), relaxBoundsCheck); if (matchStatus.isOK()) { LOGV2_DEBUG(5619202, 2, "Found a matching child"); found = true; @@ -830,7 +830,7 @@ Status QueryPlannerTestLib::solutionMatches(const BSONObj& testSoln, "found a fetch stage in the solution but no 'node' sub-object in the provided " "JSON"}; } - return solutionMatches(child.Obj(), fn->children[0], relaxBoundsCheck) + return solutionMatches(child.Obj(), fn->children[0].get(), relaxBoundsCheck) .withContext("mismatch beneath fetch node"); } else if (STAGE_OR == trueSoln->getType()) { const OrNode* orn = static_cast<const OrNode*>(trueSoln); @@ -1008,7 +1008,7 @@ Status QueryPlannerTestLib::solutionMatches(const BSONObj& testSoln, "mismatching 'spec'. Expected: " << specProjObj << " Found: " << solnProjObj}; } - return solutionMatches(child.Obj(), pn->children[0], relaxBoundsCheck) + return solutionMatches(child.Obj(), pn->children[0].get(), relaxBoundsCheck) .withContext("mismatch below projection stage"); } else if (isSortStageType(trueSoln->getType())) { const SortNode* sn = static_cast<const SortNode*>(trueSoln); @@ -1093,7 +1093,7 @@ Status QueryPlannerTestLib::solutionMatches(const BSONObj& testSoln, "mismatching 'limit'. Expected: " << expectedLimit << " Found: " << sn->limit}; } - return solutionMatches(child.Obj(), sn->children[0], relaxBoundsCheck) + return solutionMatches(child.Obj(), sn->children[0].get(), relaxBoundsCheck) .withContext("mismatch below sort stage"); } else if (STAGE_SORT_KEY_GENERATOR == trueSoln->getType()) { const SortKeyGeneratorNode* keyGenNode = static_cast<const SortKeyGeneratorNode*>(trueSoln); @@ -1113,7 +1113,7 @@ Status QueryPlannerTestLib::solutionMatches(const BSONObj& testSoln, "the provided JSON"}; } - return solutionMatches(child.Obj(), keyGenNode->children[0], relaxBoundsCheck) + return solutionMatches(child.Obj(), keyGenNode->children[0].get(), relaxBoundsCheck) .withContext("mismatch below sortKeyGen"); } else if (STAGE_SORT_MERGE == trueSoln->getType()) { const MergeSortNode* msn = static_cast<const MergeSortNode*>(trueSoln); @@ -1157,7 +1157,7 @@ Status QueryPlannerTestLib::solutionMatches(const BSONObj& testSoln, "mismatching 'n'. Expected: " << skipEl.numberInt() << " Found: " << sn->skip}; } - return solutionMatches(child.Obj(), sn->children[0], relaxBoundsCheck) + return solutionMatches(child.Obj(), sn->children[0].get(), relaxBoundsCheck) .withContext("mismatch below skip stage"); } else if (STAGE_LIMIT == trueSoln->getType()) { const LimitNode* ln = static_cast<const LimitNode*>(trueSoln); @@ -1189,7 +1189,7 @@ Status QueryPlannerTestLib::solutionMatches(const BSONObj& testSoln, "mismatching 'n'. Expected: " << limitEl.numberInt() << " Found: " << ln->limit}; } - return solutionMatches(child.Obj(), ln->children[0], relaxBoundsCheck) + return solutionMatches(child.Obj(), ln->children[0].get(), relaxBoundsCheck) .withContext("mismatch below limit stage"); } else if (STAGE_SHARDING_FILTER == trueSoln->getType()) { const ShardingFilterNode* fn = static_cast<const ShardingFilterNode*>(trueSoln); @@ -1210,7 +1210,7 @@ Status QueryPlannerTestLib::solutionMatches(const BSONObj& testSoln, "the provided JSON"}; } - return solutionMatches(child.Obj(), fn->children[0], relaxBoundsCheck) + return solutionMatches(child.Obj(), fn->children[0].get(), relaxBoundsCheck) .withContext("mismatch below shard filter stage"); } else if (STAGE_GROUP == trueSoln->getType()) { const auto* actualGroupNode = static_cast<const GroupNode*>(trueSoln); @@ -1265,7 +1265,7 @@ Status QueryPlannerTestLib::solutionMatches(const BSONObj& testSoln, "found a group stage in the solution but no 'node' sub-object in " "the provided JSON"}; } - return solutionMatches(child.Obj(), actualGroupNode->children[0], relaxBoundsCheck) + return solutionMatches(child.Obj(), actualGroupNode->children[0].get(), relaxBoundsCheck) .withContext("mismatch below group stage"); } else if (STAGE_SENTINEL == trueSoln->getType()) { const auto* actualSentinelNode = static_cast<const SentinelNode*>(trueSoln); @@ -1484,7 +1484,7 @@ Status QueryPlannerTestLib::solutionMatches(const BSONObj& testSoln, "found a eq_lookup stage in the solution but no 'node' sub-object in " "the provided JSON"}; } - return solutionMatches(child.Obj(), actualEqLookupNode->children[0], relaxBoundsCheck) + return solutionMatches(child.Obj(), actualEqLookupNode->children[0].get(), relaxBoundsCheck) .withContext("mismatch below eq_lookup stage"); } return {ErrorCodes::Error{5698301}, diff --git a/src/mongo/db/query/query_solution.cpp b/src/mongo/db/query/query_solution.cpp index ffc9bf18cff..3f06db2c830 100644 --- a/src/mongo/db/query/query_solution.cpp +++ b/src/mongo/db/query/query_solution.cpp @@ -113,8 +113,8 @@ void getAllSecondaryNamespacesHelper(const QuerySolutionNode* qsn, } } - for (auto child : qsn->children) { - getAllSecondaryNamespacesHelper(child, mainNss, secondaryNssSet); + for (auto&& child : qsn->children) { + getAllSecondaryNamespacesHelper(child.get(), mainNss, secondaryNssSet); } } } // namespace @@ -238,7 +238,7 @@ std::string QuerySolution::summaryString() const { } for (auto&& child : node->children) { - queue.push(child); + queue.push(child.get()); } } @@ -272,11 +272,9 @@ void QuerySolution::extendWith(std::unique_ptr<QuerySolutionNode> extensionRoot) tassert(5842800, "Only chain extension trees are supported", parentOfSentinel->children.size() == 1); - current = parentOfSentinel->children[0]; + current = parentOfSentinel->children[0].get(); } - parentOfSentinel->children[0] = _root.release(); - delete current; // The sentinel node itself isn't used anymore. - + parentOfSentinel->children[0] = std::move(_root); setRoot(std::move(extensionRoot)); } @@ -326,9 +324,9 @@ void CollectionScanNode::appendToString(str::stream* ss, int indent) const { addCommon(ss, indent); } -QuerySolutionNode* CollectionScanNode::clone() const { - CollectionScanNode* copy = new CollectionScanNode(); - cloneBaseData(copy); +std::unique_ptr<QuerySolutionNode> CollectionScanNode::clone() const { + auto copy = std::make_unique<CollectionScanNode>(); + cloneBaseData(copy.get()); copy->name = this->name; copy->tailable = this->tailable; @@ -368,9 +366,9 @@ void VirtualScanNode::appendToString(str::stream* ss, int indent) const { addCommon(ss, indent); } -QuerySolutionNode* VirtualScanNode::clone() const { - auto copy = new VirtualScanNode(docs, scanType, hasRecordId, indexKeyPattern); - cloneBaseData(copy); +std::unique_ptr<QuerySolutionNode> VirtualScanNode::clone() const { + auto copy = std::make_unique<VirtualScanNode>(docs, scanType, hasRecordId, indexKeyPattern); + cloneBaseData(copy.get()); return copy; } @@ -417,9 +415,9 @@ FieldAvailability AndHashNode::getFieldAvailability(const string& field) const { return result; } -QuerySolutionNode* AndHashNode::clone() const { - AndHashNode* copy = new AndHashNode(); - cloneBaseData(copy); +std::unique_ptr<QuerySolutionNode> AndHashNode::clone() const { + auto copy = std::make_unique<AndHashNode>(); + cloneBaseData(copy.get()); return copy; } @@ -462,9 +460,9 @@ FieldAvailability AndSortedNode::getFieldAvailability(const string& field) const return result; } -QuerySolutionNode* AndSortedNode::clone() const { - AndSortedNode* copy = new AndSortedNode(); - cloneBaseData(copy); +std::unique_ptr<QuerySolutionNode> AndSortedNode::clone() const { + auto copy = std::make_unique<AndSortedNode>(); + cloneBaseData(copy.get()); return copy; } @@ -517,9 +515,9 @@ FieldAvailability OrNode::getFieldAvailability(const string& field) const { return result; } -QuerySolutionNode* OrNode::clone() const { - OrNode* copy = new OrNode(); - cloneBaseData(copy); +std::unique_ptr<QuerySolutionNode> OrNode::clone() const { + auto copy = std::make_unique<OrNode>(); + cloneBaseData(copy.get()); copy->dedup = this->dedup; @@ -575,9 +573,9 @@ FieldAvailability MergeSortNode::getFieldAvailability(const string& field) const return result; } -QuerySolutionNode* MergeSortNode::clone() const { - MergeSortNode* copy = new MergeSortNode(); - cloneBaseData(copy); +std::unique_ptr<QuerySolutionNode> MergeSortNode::clone() const { + auto copy = std::make_unique<MergeSortNode>(); + cloneBaseData(copy.get()); copy->dedup = this->dedup; copy->sort = this->sort; @@ -605,9 +603,9 @@ void FetchNode::appendToString(str::stream* ss, int indent) const { children[0]->appendToString(ss, indent + 2); } -QuerySolutionNode* FetchNode::clone() const { - FetchNode* copy = new FetchNode(); - cloneBaseData(copy); +std::unique_ptr<QuerySolutionNode> FetchNode::clone() const { + auto copy = std::make_unique<FetchNode>(); + cloneBaseData(copy.get()); return copy; } @@ -1047,9 +1045,9 @@ void IndexScanNode::computeProperties() { computeSortsAndMultikeyPathsForScan(index, direction, bounds, queryCollator); } -QuerySolutionNode* IndexScanNode::clone() const { - IndexScanNode* copy = new IndexScanNode(this->index); - cloneBaseData(copy); +std::unique_ptr<QuerySolutionNode> IndexScanNode::clone() const { + auto copy = std::make_unique<IndexScanNode>(this->index); + cloneBaseData(copy.get()); copy->direction = this->direction; copy->addKeyMetadata = this->addKeyMetadata; @@ -1132,10 +1130,8 @@ void ReturnKeyNode::appendToString(str::stream* ss, int indent) const { children[0]->appendToString(ss, indent + 2); } -QuerySolutionNode* ReturnKeyNode::clone() const { - auto copy = std::make_unique<ReturnKeyNode>( - std::unique_ptr<QuerySolutionNode>(children[0]->clone()), std::vector(sortKeyMetaFields)); - return copy.release(); +std::unique_ptr<QuerySolutionNode> ReturnKeyNode::clone() const { + return std::make_unique<ReturnKeyNode>(children[0]->clone(), std::vector(sortKeyMetaFields)); } // @@ -1180,28 +1176,23 @@ void ProjectionNode::cloneProjectionData(ProjectionNode* copy) const { copy->sortSet = this->sortSet; } -ProjectionNode* ProjectionNodeDefault::clone() const { - auto copy = std::make_unique<ProjectionNodeDefault>( - std::unique_ptr<QuerySolutionNode>(children[0]->clone()), fullExpression, proj); +std::unique_ptr<QuerySolutionNode> ProjectionNodeDefault::clone() const { + auto copy = std::make_unique<ProjectionNodeDefault>(children[0]->clone(), fullExpression, proj); ProjectionNode::cloneProjectionData(copy.get()); - return copy.release(); + return copy; } -ProjectionNode* ProjectionNodeCovered::clone() const { +std::unique_ptr<QuerySolutionNode> ProjectionNodeCovered::clone() const { auto copy = std::make_unique<ProjectionNodeCovered>( - std::unique_ptr<QuerySolutionNode>(children[0]->clone()), - fullExpression, - proj, - coveredKeyObj); + children[0]->clone(), fullExpression, proj, coveredKeyObj); ProjectionNode::cloneProjectionData(copy.get()); - return copy.release(); + return copy; } -ProjectionNode* ProjectionNodeSimple::clone() const { - auto copy = std::make_unique<ProjectionNodeSimple>( - std::unique_ptr<QuerySolutionNode>(children[0]->clone()), fullExpression, proj); +std::unique_ptr<QuerySolutionNode> ProjectionNodeSimple::clone() const { + auto copy = std::make_unique<ProjectionNodeSimple>(children[0]->clone(), fullExpression, proj); ProjectionNode::cloneProjectionData(copy.get()); - return copy.release(); + return copy; } @@ -1220,9 +1211,9 @@ void SortKeyGeneratorNode::appendToString(str::stream* ss, int indent) const { children[0]->appendToString(ss, indent + 2); } -QuerySolutionNode* SortKeyGeneratorNode::clone() const { - SortKeyGeneratorNode* copy = new SortKeyGeneratorNode(); - cloneBaseData(copy); +std::unique_ptr<QuerySolutionNode> SortKeyGeneratorNode::clone() const { + auto copy = std::make_unique<SortKeyGeneratorNode>(); + cloneBaseData(copy.get()); copy->sortSpec = this->sortSpec; return copy; } @@ -1253,16 +1244,16 @@ void SortNode::cloneSortData(SortNode* copy) const { copy->addSortKeyMetadata = this->addSortKeyMetadata; } -QuerySolutionNode* SortNodeDefault::clone() const { +std::unique_ptr<QuerySolutionNode> SortNodeDefault::clone() const { auto copy = std::make_unique<SortNodeDefault>(); cloneSortData(copy.get()); - return copy.release(); + return copy; } -QuerySolutionNode* SortNodeSimple::clone() const { +std::unique_ptr<QuerySolutionNode> SortNodeSimple::clone() const { auto copy = std::make_unique<SortNodeSimple>(); cloneSortData(copy.get()); - return copy.release(); + return copy; } // @@ -1282,9 +1273,9 @@ void LimitNode::appendToString(str::stream* ss, int indent) const { children[0]->appendToString(ss, indent + 2); } -QuerySolutionNode* LimitNode::clone() const { - LimitNode* copy = new LimitNode(); - cloneBaseData(copy); +std::unique_ptr<QuerySolutionNode> LimitNode::clone() const { + auto copy = std::make_unique<LimitNode>(); + cloneBaseData(copy.get()); copy->limit = this->limit; @@ -1306,9 +1297,9 @@ void SkipNode::appendToString(str::stream* ss, int indent) const { children[0]->appendToString(ss, indent + 2); } -QuerySolutionNode* SkipNode::clone() const { - SkipNode* copy = new SkipNode(); - cloneBaseData(copy); +std::unique_ptr<QuerySolutionNode> SkipNode::clone() const { + auto copy = std::make_unique<SkipNode>(); + cloneBaseData(copy.get()); copy->skip = this->skip; @@ -1334,9 +1325,9 @@ void GeoNear2DNode::appendToString(str::stream* ss, int indent) const { } } -QuerySolutionNode* GeoNear2DNode::clone() const { - GeoNear2DNode* copy = new GeoNear2DNode(this->index); - cloneBaseData(copy); +std::unique_ptr<QuerySolutionNode> GeoNear2DNode::clone() const { + auto copy = std::make_unique<GeoNear2DNode>(this->index); + cloneBaseData(copy.get()); copy->nq = this->nq; copy->baseBounds = this->baseBounds; @@ -1367,9 +1358,9 @@ void GeoNear2DSphereNode::appendToString(str::stream* ss, int indent) const { } } -QuerySolutionNode* GeoNear2DSphereNode::clone() const { - GeoNear2DSphereNode* copy = new GeoNear2DSphereNode(this->index); - cloneBaseData(copy); +std::unique_ptr<QuerySolutionNode> GeoNear2DSphereNode::clone() const { + auto copy = std::make_unique<GeoNear2DSphereNode>(this->index); + cloneBaseData(copy.get()); copy->nq = this->nq; copy->baseBounds = this->baseBounds; @@ -1399,9 +1390,9 @@ void ShardingFilterNode::appendToString(str::stream* ss, int indent) const { children[0]->appendToString(ss, indent + 2); } -QuerySolutionNode* ShardingFilterNode::clone() const { - ShardingFilterNode* copy = new ShardingFilterNode(); - cloneBaseData(copy); +std::unique_ptr<QuerySolutionNode> ShardingFilterNode::clone() const { + auto copy = std::make_unique<ShardingFilterNode>(); + cloneBaseData(copy.get()); return copy; } @@ -1422,9 +1413,9 @@ void DistinctNode::appendToString(str::stream* ss, int indent) const { *ss << "bounds = " << bounds.toString(index.collator != nullptr) << '\n'; } -QuerySolutionNode* DistinctNode::clone() const { - DistinctNode* copy = new DistinctNode(this->index); - cloneBaseData(copy); +std::unique_ptr<QuerySolutionNode> DistinctNode::clone() const { + auto copy = std::make_unique<DistinctNode>(this->index); + cloneBaseData(copy.get()); copy->direction = this->direction; copy->bounds = this->bounds; @@ -1457,9 +1448,9 @@ void CountScanNode::appendToString(str::stream* ss, int indent) const { *ss << "endKey = " << endKey << '\n'; } -QuerySolutionNode* CountScanNode::clone() const { - CountScanNode* copy = new CountScanNode(this->index); - cloneBaseData(copy); +std::unique_ptr<QuerySolutionNode> CountScanNode::clone() const { + auto copy = std::make_unique<CountScanNode>(this->index); + cloneBaseData(copy.get()); copy->startKey = this->startKey; copy->startKeyInclusive = this->startKeyInclusive; @@ -1478,9 +1469,9 @@ void EofNode::appendToString(str::stream* ss, int indent) const { *ss << "EOF\n"; } -QuerySolutionNode* EofNode::clone() const { - auto copy = new EofNode(); - cloneBaseData(copy); +std::unique_ptr<QuerySolutionNode> EofNode::clone() const { + auto copy = std::make_unique<EofNode>(); + cloneBaseData(copy.get()); return copy; } @@ -1503,11 +1494,11 @@ void TextOrNode::appendToString(str::stream* ss, int indent) const { } } -QuerySolutionNode* TextOrNode::clone() const { +std::unique_ptr<QuerySolutionNode> TextOrNode::clone() const { auto copy = std::make_unique<TextOrNode>(); cloneBaseData(copy.get()); copy->dedup = this->dedup; - return copy.release(); + return copy; } // @@ -1539,11 +1530,11 @@ void TextMatchNode::appendToString(str::stream* ss, int indent) const { addCommon(ss, indent); } -QuerySolutionNode* TextMatchNode::clone() const { +std::unique_ptr<QuerySolutionNode> TextMatchNode::clone() const { auto copy = std::make_unique<TextMatchNode>(index, ftsQuery->clone(), wantTextScore); cloneBaseData(copy.get()); copy->indexPrefix = indexPrefix; - return copy.release(); + return copy; } /** @@ -1584,14 +1575,10 @@ void GroupNode::appendToString(str::stream* ss, int indent) const { children[0]->appendToString(ss, indent + 2); } -QuerySolutionNode* GroupNode::clone() const { - auto copy = - std::make_unique<GroupNode>(std::unique_ptr<QuerySolutionNode>(children[0]->clone()), - groupByExpression, - accumulators, - doingMerge, - shouldProduceBson); - return copy.release(); +std::unique_ptr<QuerySolutionNode> GroupNode::clone() const { + auto copy = std::make_unique<GroupNode>( + children[0]->clone(), groupByExpression, accumulators, doingMerge, shouldProduceBson); + return copy; } /** @@ -1616,21 +1603,20 @@ void EqLookupNode::appendToString(str::stream* ss, int indent) const { children[0]->appendToString(ss, indent + 2); } -QuerySolutionNode* EqLookupNode::clone() const { - auto copy = - std::make_unique<EqLookupNode>(std::unique_ptr<QuerySolutionNode>(children[0]->clone()), - foreignCollection, - joinFieldLocal, - joinFieldForeign, - joinField, - shouldProduceBson); - return copy.release(); +std::unique_ptr<QuerySolutionNode> EqLookupNode::clone() const { + auto copy = std::make_unique<EqLookupNode>(children[0]->clone(), + foreignCollection, + joinFieldLocal, + joinFieldForeign, + joinField, + shouldProduceBson); + return copy; } /** * SentinelNode. */ -QuerySolutionNode* SentinelNode::clone() const { - return std::make_unique<SentinelNode>().release(); +std::unique_ptr<QuerySolutionNode> SentinelNode::clone() const { + return std::make_unique<SentinelNode>(); } void SentinelNode::appendToString(str::stream* ss, int indent) const { diff --git a/src/mongo/db/query/query_solution.h b/src/mongo/db/query/query_solution.h index 694f5d7c681..82ee27df96b 100644 --- a/src/mongo/db/query/query_solution.h +++ b/src/mongo/db/query/query_solution.h @@ -135,14 +135,12 @@ struct QuerySolutionNode { /** * Constructs a QuerySolutionNode with a single child. */ - QuerySolutionNode(std::unique_ptr<QuerySolutionNode> child) : children{child.release()} {} - - virtual ~QuerySolutionNode() { - for (size_t i = 0; i < children.size(); ++i) { - delete children[i]; - } + QuerySolutionNode(std::unique_ptr<QuerySolutionNode> child) { + children.push_back(std::move(child)); } + virtual ~QuerySolutionNode() = default; + /** * Return a std::string representation of this node and any children. */ @@ -225,32 +223,16 @@ struct QuerySolutionNode { /** * Make a deep copy. */ - virtual QuerySolutionNode* clone() const = 0; - - /** - * Copy base query solution data from 'this' to 'other'. - */ - void cloneBaseData(QuerySolutionNode* other) const { - for (size_t i = 0; i < this->children.size(); i++) { - other->children.push_back(this->children[i]->clone()); - } - if (nullptr != this->filter) { - other->filter = this->filter->shallowClone(); - } - } + virtual std::unique_ptr<QuerySolutionNode> clone() const = 0; /** * Adds a vector of query solution nodes to the list of children of this node. - * - * TODO SERVER-35512: Once 'children' are held by unique_ptr, this method should no longer be - * necessary. */ void addChildren(std::vector<std::unique_ptr<QuerySolutionNode>> newChildren) { children.reserve(children.size() + newChildren.size()); - std::transform(newChildren.begin(), - newChildren.end(), - std::back_inserter(children), - [](auto& child) { return child.release(); }); + children.insert(children.end(), + std::make_move_iterator(newChildren.begin()), + std::make_move_iterator(newChildren.end())); } bool getScanLimit() { @@ -282,10 +264,7 @@ struct QuerySolutionNode { return _nodeId; } - // These are owned here. - // - // TODO SERVER-35512: Make this a vector of unique_ptr. - std::vector<QuerySolutionNode*> children; + std::vector<std::unique_ptr<QuerySolutionNode>> children; // If a stage has a non-NULL filter all values outputted from that stage must pass that // filter. @@ -305,6 +284,18 @@ protected: */ void addCommon(str::stream* ss, int indent) const; + /** + * Copy base query solution data from 'this' to 'other'. + */ + void cloneBaseData(QuerySolutionNode* other) const { + for (size_t i = 0; i < this->children.size(); i++) { + other->children.push_back(this->children[i]->clone()); + } + if (nullptr != this->filter) { + other->filter = this->filter->shallowClone(); + } + } + private: // Allows the QuerySolution constructor to set '_nodeId'. friend class QuerySolution; @@ -461,7 +452,7 @@ struct CollectionScanNode : public QuerySolutionNodeWithSortSet { return false; } - QuerySolutionNode* clone() const; + std::unique_ptr<QuerySolutionNode> clone() const final; // Name of the namespace. std::string name; @@ -541,16 +532,16 @@ struct ColumnIndexScanNode : public QuerySolutionNode { return kEmptySet; } - QuerySolutionNode* clone() const { + std::unique_ptr<QuerySolutionNode> clone() const final { StringMap<std::unique_ptr<MatchExpression>> clonedFiltersByPath; for (auto&& [path, filter] : filtersByPath) { clonedFiltersByPath[path] = filter->shallowClone(); } - return new ColumnIndexScanNode(indexEntry, - outputFields, - matchFields, - std::move(clonedFiltersByPath), - postAssemblyFilter->shallowClone()); + return std::make_unique<ColumnIndexScanNode>(indexEntry, + outputFields, + matchFields, + std::move(clonedFiltersByPath), + postAssemblyFilter->shallowClone()); } ColumnIndexEntry indexEntry; @@ -612,7 +603,7 @@ struct VirtualScanNode : public QuerySolutionNodeWithSortSet { return false; } - QuerySolutionNode* clone() const; + std::unique_ptr<QuerySolutionNode> clone() const final; // A representation of a collection's documents. Here we use a BSONArray so metadata like a // RecordId can be stored alongside of the main document payload. The format of the data in @@ -657,7 +648,7 @@ struct AndHashNode : public QuerySolutionNode { return children.back()->providedSorts(); } - QuerySolutionNode* clone() const; + std::unique_ptr<QuerySolutionNode> clone() const final; }; struct AndSortedNode : public QuerySolutionNodeWithSortSet { @@ -676,7 +667,7 @@ struct AndSortedNode : public QuerySolutionNodeWithSortSet { return true; } - QuerySolutionNode* clone() const; + std::unique_ptr<QuerySolutionNode> clone() const final; }; struct OrNode : public QuerySolutionNodeWithSortSet { @@ -697,7 +688,7 @@ struct OrNode : public QuerySolutionNodeWithSortSet { return false; } - QuerySolutionNode* clone() const; + std::unique_ptr<QuerySolutionNode> clone() const override; bool dedup; }; @@ -718,7 +709,7 @@ struct MergeSortNode : public QuerySolutionNodeWithSortSet { return false; } - QuerySolutionNode* clone() const; + std::unique_ptr<QuerySolutionNode> clone() const final; virtual void computeProperties() { for (size_t i = 0; i < children.size(); ++i) { @@ -755,7 +746,7 @@ struct FetchNode : public QuerySolutionNode { return children[0]->providedSorts(); } - QuerySolutionNode* clone() const; + std::unique_ptr<QuerySolutionNode> clone() const final; }; struct IndexScanNode : public QuerySolutionNodeWithSortSet { @@ -776,7 +767,7 @@ struct IndexScanNode : public QuerySolutionNodeWithSortSet { FieldAvailability getFieldAvailability(const std::string& field) const; bool sortedByDiskLoc() const; - QuerySolutionNode* clone() const; + std::unique_ptr<QuerySolutionNode> clone() const final; bool operator==(const IndexScanNode& other) const; @@ -837,7 +828,7 @@ struct ReturnKeyNode : public QuerySolutionNode { return children[0]->providedSorts(); } - QuerySolutionNode* clone() const final; + std::unique_ptr<QuerySolutionNode> clone() const final; std::vector<FieldPath> sortKeyMetaFields; }; @@ -912,7 +903,7 @@ struct ProjectionNodeDefault final : ProjectionNode { return STAGE_PROJECTION_DEFAULT; } - ProjectionNode* clone() const final; + std::unique_ptr<QuerySolutionNode> clone() const final; StringData projectionImplementationTypeToString() const final { return "DEFAULT"_sd; @@ -934,7 +925,7 @@ struct ProjectionNodeCovered final : ProjectionNode { return STAGE_PROJECTION_COVERED; } - ProjectionNode* clone() const final; + std::unique_ptr<QuerySolutionNode> clone() const final; StringData projectionImplementationTypeToString() const final { return "COVERED_ONE_INDEX"_sd; @@ -955,7 +946,7 @@ struct ProjectionNodeSimple final : ProjectionNode { return STAGE_PROJECTION_SIMPLE; } - ProjectionNode* clone() const final; + std::unique_ptr<QuerySolutionNode> clone() const final; StringData projectionImplementationTypeToString() const final { return "SIMPLE_DOC"_sd; @@ -983,7 +974,7 @@ struct SortKeyGeneratorNode : public QuerySolutionNode { return children[0]->providedSorts(); } - QuerySolutionNode* clone() const final; + std::unique_ptr<QuerySolutionNode> clone() const final; void appendToString(str::stream* ss, int indent) const final; @@ -1042,7 +1033,7 @@ struct SortNodeDefault final : public SortNode { return STAGE_SORT_DEFAULT; } - QuerySolutionNode* clone() const override; + std::unique_ptr<QuerySolutionNode> clone() const final; StringData sortImplementationTypeToString() const override { return "DEFAULT"_sd; @@ -1060,7 +1051,7 @@ struct SortNodeSimple final : public SortNode { return STAGE_SORT_SIMPLE; } - QuerySolutionNode* clone() const override; + std::unique_ptr<QuerySolutionNode> clone() const final; StringData sortImplementationTypeToString() const override { return "SIMPLE"_sd; @@ -1090,7 +1081,7 @@ struct LimitNode : public QuerySolutionNode { return children[0]->providedSorts(); } - QuerySolutionNode* clone() const; + std::unique_ptr<QuerySolutionNode> clone() const final; long long limit; }; @@ -1117,7 +1108,7 @@ struct SkipNode : public QuerySolutionNode { return children[0]->providedSorts(); } - QuerySolutionNode* clone() const; + std::unique_ptr<QuerySolutionNode> clone() const final; long long skip; }; @@ -1143,7 +1134,7 @@ struct GeoNear2DNode : public QuerySolutionNodeWithSortSet { return false; } - QuerySolutionNode* clone() const; + std::unique_ptr<QuerySolutionNode> clone() const final; // Not owned here const GeoNearExpression* nq; @@ -1175,7 +1166,7 @@ struct GeoNear2DSphereNode : public QuerySolutionNodeWithSortSet { return false; } - QuerySolutionNode* clone() const; + std::unique_ptr<QuerySolutionNode> clone() const final; // Not owned here const GeoNearExpression* nq; @@ -1218,7 +1209,7 @@ struct ShardingFilterNode : public QuerySolutionNode { return children[0]->providedSorts(); } - QuerySolutionNode* clone() const; + std::unique_ptr<QuerySolutionNode> clone() const final; }; /** @@ -1252,7 +1243,7 @@ struct DistinctNode : public QuerySolutionNodeWithSortSet { return false; } - QuerySolutionNode* clone() const; + std::unique_ptr<QuerySolutionNode> clone() const final; virtual void computeProperties(); @@ -1290,7 +1281,7 @@ struct CountScanNode : public QuerySolutionNodeWithSortSet { return false; } - QuerySolutionNode* clone() const; + std::unique_ptr<QuerySolutionNode> clone() const final; IndexEntry index; @@ -1322,7 +1313,7 @@ struct EofNode : public QuerySolutionNodeWithSortSet { return false; } - QuerySolutionNode* clone() const; + std::unique_ptr<QuerySolutionNode> clone() const final; }; struct TextOrNode : public OrNode { @@ -1333,7 +1324,7 @@ struct TextOrNode : public OrNode { } void appendToString(str::stream* ss, int indent) const override; - QuerySolutionNode* clone() const override; + std::unique_ptr<QuerySolutionNode> clone() const final; }; struct TextMatchNode : public QuerySolutionNodeWithSortSet { @@ -1357,7 +1348,7 @@ struct TextMatchNode : public QuerySolutionNodeWithSortSet { return false; } - QuerySolutionNode* clone() const override; + std::unique_ptr<QuerySolutionNode> clone() const final; IndexEntry index; std::unique_ptr<fts::FTSQuery> ftsQuery; @@ -1424,7 +1415,7 @@ struct GroupNode : public QuerySolutionNode { return kEmptySet; } - QuerySolutionNode* clone() const override; + std::unique_ptr<QuerySolutionNode> clone() const final; boost::intrusive_ptr<Expression> groupByExpression; std::vector<AccumulationStatement> accumulators; @@ -1525,7 +1516,7 @@ struct EqLookupNode : public QuerySolutionNode { return kEmptySet; } - QuerySolutionNode* clone() const override; + std::unique_ptr<QuerySolutionNode> clone() const final; /** * The foreign (inner) collection namespace name. @@ -1593,6 +1584,6 @@ struct SentinelNode : public QuerySolutionNode { return kEmptySet; } - QuerySolutionNode* clone() const override; + std::unique_ptr<QuerySolutionNode> clone() const final; }; } // namespace mongo diff --git a/src/mongo/db/query/query_solution_test.cpp b/src/mongo/db/query/query_solution_test.cpp index cfe45eae714..bdba8b50f6b 100644 --- a/src/mongo/db/query/query_solution_test.cpp +++ b/src/mongo/db/query/query_solution_test.cpp @@ -1137,7 +1137,7 @@ TEST(QuerySolutionTest, EqLookupNodeWithIndexScan) { node.computeProperties(); - auto child = node.children[0]; + auto child = node.children[0].get(); ASSERT_EQ(node.fetched(), child->fetched()); ASSERT_EQ(node.sortedByDiskLoc(), child->sortedByDiskLoc()); @@ -1165,7 +1165,7 @@ TEST(QuerySolutionTest, EqLookupNodeWithIndexScanFieldOverwrite) { node.computeProperties(); - auto child = node.children[0]; + auto child = node.children[0].get(); // Expected empty sort order, as the EqLookupNode order inferrence is not supported yet. ASSERT_EQ(node.providedSorts(), kEmptySet); diff --git a/src/mongo/db/query/sbe_and_hash_test.cpp b/src/mongo/db/query/sbe_and_hash_test.cpp index 7942a3d89ce..95f0d083cbc 100644 --- a/src/mongo/db/query/sbe_and_hash_test.cpp +++ b/src/mongo/db/query/sbe_and_hash_test.cpp @@ -51,7 +51,7 @@ protected: for (auto docs : docsVec) { auto virtScan = std::make_unique<VirtualScanNode>(docs, VirtualScanNode::ScanType::kCollScan, true); - andHashNode->children.push_back(virtScan.release()); + andHashNode->children.push_back(std::move(virtScan)); } return std::move(andHashNode); } diff --git a/src/mongo/db/query/sbe_and_sorted_test.cpp b/src/mongo/db/query/sbe_and_sorted_test.cpp index a05ee696cd7..45e0d38f3ac 100644 --- a/src/mongo/db/query/sbe_and_sorted_test.cpp +++ b/src/mongo/db/query/sbe_and_sorted_test.cpp @@ -49,7 +49,7 @@ protected: for (auto docs : docsVec) { auto virtScan = std::make_unique<VirtualScanNode>(docs, VirtualScanNode::ScanType::kCollScan, true); - andSortedNode->children.push_back(virtScan.release()); + andSortedNode->children.push_back(std::move(virtScan)); } return std::move(andSortedNode); } diff --git a/src/mongo/db/query/sbe_shard_filter_test.cpp b/src/mongo/db/query/sbe_shard_filter_test.cpp index 00c9895c2fa..82585905f6e 100644 --- a/src/mongo/db/query/sbe_shard_filter_test.cpp +++ b/src/mongo/db/query/sbe_shard_filter_test.cpp @@ -79,7 +79,7 @@ protected: auto virtScan = std::make_unique<VirtualScanNode>(docs, VirtualScanNode::ScanType::kCollScan, false); auto shardFilter = std::make_unique<ShardingFilterNode>(); - shardFilter->children.push_back(virtScan.release()); + shardFilter->children.push_back(std::move(virtScan)); return std::move(shardFilter); } @@ -251,7 +251,7 @@ TEST_F(SbeShardFilterTest, CoveredShardFilterPlan) { auto virtScan = std::make_unique<VirtualScanNode>( mockedIndexKeys, VirtualScanNode::ScanType::kIxscan, false, indexKeyPattern); auto shardFilter = std::make_unique<ShardingFilterNode>(); - shardFilter->children.push_back(virtScan.release()); + shardFilter->children.push_back(std::move(virtScan)); auto projectNode = std::make_unique<ProjectionNodeCovered>( std::move(shardFilter), *emptyMatchExpression, projectionAst, indexKeyPattern); diff --git a/src/mongo/db/query/sbe_stage_builder.cpp b/src/mongo/db/query/sbe_stage_builder.cpp index 573fa246ea3..969ed8991f5 100644 --- a/src/mongo/db/query/sbe_stage_builder.cpp +++ b/src/mongo/db/query/sbe_stage_builder.cpp @@ -582,7 +582,7 @@ void getAllNodesByTypeHelper(const QuerySolutionNode* root, } for (auto&& child : root->children) { - getAllNodesByTypeHelper(child, type, results); + getAllNodesByTypeHelper(child.get(), type, results); } } @@ -609,7 +609,7 @@ std::pair<const QuerySolutionNode*, size_t> getFirstNodeByType(const QuerySoluti } for (auto&& child : root->children) { - auto [subTreeResult, subTreeCount] = getFirstNodeByType(child, type); + auto [subTreeResult, subTreeCount] = getFirstNodeByType(child.get(), type); if (!result) { result = subTreeResult; } @@ -1016,7 +1016,7 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder .set(kIndexKey) .set(kIndexKeyPattern); - auto [stage, outputs] = build(fn->children[0], childReqs); + auto [stage, outputs] = build(fn->children[0].get(), childReqs); auto iamMap = _data.iamMap; uassert(4822880, "RecordId slot is not defined", outputs.has(kRecordId)); @@ -1085,11 +1085,11 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder if (ln->children[0]->getType() == StageType::STAGE_SKIP) { // If we have both limit and skip stages and the skip stage is beneath the limit, then // we can combine these two stages into one. - const auto sn = static_cast<const SkipNode*>(ln->children[0]); + const auto sn = static_cast<const SkipNode*>(ln->children[0].get()); skip = sn->skip; - return build(sn->children[0], reqs); + return build(sn->children[0].get(), reqs); } else { - return build(ln->children[0], reqs); + return build(ln->children[0].get(), reqs); } }(); @@ -1104,7 +1104,7 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder::buildSkip( const QuerySolutionNode* root, const PlanStageReqs& reqs) { const auto sn = static_cast<const SkipNode*>(root); - auto [stage, outputs] = build(sn->children[0], reqs); + auto [stage, outputs] = build(sn->children[0].get(), reqs); if (!reqs.getIsTailableCollScanResumeBranch()) { stage = std::make_unique<sbe::LimitSkipStage>( @@ -1288,7 +1288,7 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder // The child must produce all of the slots required by the parent of this SortNode. auto childReqs = reqs.copy(); - auto child = sn->children[0]; + auto child = sn->children[0].get(); const auto isCoveredQuery = reqs.getIndexKeyBitset().has_value(); BSONObj indexKeyPattern; @@ -1546,11 +1546,11 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder // if the bounds are fixed, the fields will be marked as 'ignored'). Otherwise, we attempt // to retrieve it from 'providedSorts'. auto childSortPattern = [&]() { - if (auto [msn, _] = getFirstNodeByType(child, STAGE_SORT_MERGE); msn) { + if (auto [msn, _] = getFirstNodeByType(child.get(), STAGE_SORT_MERGE); msn) { auto node = static_cast<const MergeSortNode*>(msn); return node->sort; } else { - auto [ixn, ct] = getFirstNodeByType(child, STAGE_IXSCAN); + auto [ixn, ct] = getFirstNodeByType(child.get(), STAGE_IXSCAN); if (ixn && ct == 1) { auto node = static_cast<const IndexScanNode*>(ixn); return node->index.keyPattern; @@ -1587,7 +1587,7 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder childReqs.getIndexKeyBitset() = indexKeyBitset; // Children must produce a 'resultSlot' if they produce fetched results. - auto [stage, outputs] = build(child, childReqs); + auto [stage, outputs] = build(child.get(), childReqs); tassert(5184301, "SORT_MERGE node must receive a RecordID slot as input from child stage" @@ -1682,7 +1682,7 @@ SlotBasedStageBuilder::buildProjectionSimple(const QuerySolutionNode* root, // In addition to that, the child must always produce a 'resultSlot' because it's needed by the // projection logic below. auto childReqs = reqs.copy().set(kResult); - auto [inputStage, outputs] = build(pn->children[0], childReqs); + auto [inputStage, outputs] = build(pn->children[0].get(), childReqs); const auto childResult = outputs.get(kResult); @@ -1735,7 +1735,7 @@ SlotBasedStageBuilder::buildProjectionCovered(const QuerySolutionNode* root, makeIndexKeyInclusionSet(pn->coveredKeyObj, requiredFields); childReqs.getIndexKeyBitset() = std::move(indexKeyBitset); - auto [inputStage, outputs] = build(pn->children[0], childReqs); + auto [inputStage, outputs] = build(pn->children[0].get(), childReqs); // Assert that the index scan produced index key slots for this covered projection. auto indexKeySlots = *outputs.extractIndexKeySlots(); @@ -1810,7 +1810,7 @@ SlotBasedStageBuilder::buildProjectionDefault(const QuerySolutionNode* root, childReqs.set(kResult); } - auto [inputStage, outputs] = build(pn->children[0], childReqs); + auto [inputStage, outputs] = build(pn->children[0].get(), childReqs); sbe::value::SlotId resultSlot; std::unique_ptr<sbe::PlanStage> resultStage; @@ -1862,7 +1862,7 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder auto childReqs = reqs.copy().setIf(kResult, orn->filter.get()).setIf(kRecordId, orn->dedup); for (auto&& child : orn->children) { - auto [stage, outputs] = build(child, childReqs); + auto [stage, outputs] = build(child.get(), childReqs); auto sv = sbe::makeSV(); outputs.forEachSlot(childReqs, [&](auto&& slot) { sv.push_back(slot); }); @@ -1918,7 +1918,7 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder tassert(5432216, "text match input must be fetched", root->children[0]->fetched()); auto childReqs = reqs.copy().set(kResult); - auto [stage, outputs] = build(textNode->children[0], childReqs); + auto [stage, outputs] = build(textNode->children[0].get(), childReqs); tassert(5432217, "result slot is not produced by text match sub-plan", outputs.has(kResult)); // Create an FTS 'matcher' to apply 'ftsQuery' to matching documents. @@ -1969,7 +1969,7 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder // After build() returns, we take the 'returnKeySlot' produced by the child and store it into // 'resultSlot' for the parent of this ReturnKeyNode to consume. auto childReqs = reqs.copy().clear(kResult).set(kReturnKey); - auto [stage, outputs] = build(returnKeyNode->children[0], childReqs); + auto [stage, outputs] = build(returnKeyNode->children[0].get(), childReqs); outputs.set(kResult, outputs.get(kReturnKey)); outputs.clear(kReturnKey); @@ -1990,8 +1990,8 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder auto childReqs = reqs.copy().set(kResult).set(kRecordId); - auto outerChild = andHashNode->children[0]; - auto innerChild = andHashNode->children[1]; + auto outerChild = andHashNode->children[0].get(); + auto innerChild = andHashNode->children[1].get(); auto [outerStage, outerOutputs] = build(outerChild, childReqs); auto outerIdSlot = outerOutputs.get(kRecordId); @@ -2056,7 +2056,7 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder // If there are more than 2 children, iterate all remaining children and hash // join together. for (size_t i = 2; i < andHashNode->children.size(); i++) { - auto [stage, outputs] = build(andHashNode->children[i], childReqs); + auto [stage, outputs] = build(andHashNode->children[i].get(), childReqs); tassert(5073714, "outputs must contain kRecordId slot", outputs.has(kRecordId)); tassert(5073715, "outputs must contain kResult slot", outputs.has(kResult)); auto idSlot = outputs.get(kRecordId); @@ -2089,8 +2089,8 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder auto childReqs = reqs.copy().set(kResult).set(kRecordId); - auto outerChild = andSortedNode->children[0]; - auto innerChild = andSortedNode->children[1]; + auto outerChild = andSortedNode->children[0].get(); + auto innerChild = andSortedNode->children[1].get(); auto [outerStage, outerOutputs] = build(outerChild, childReqs); auto outerIdSlot = outerOutputs.get(kRecordId); @@ -2167,7 +2167,7 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder // If there are more than 2 children, iterate all remaining children and merge // join together. for (size_t i = 2; i < andSortedNode->children.size(); i++) { - auto [stage, outputs] = build(andSortedNode->children[i], childReqs); + auto [stage, outputs] = build(andSortedNode->children[i].get(), childReqs); tassert(5073709, "outputs must contain kRecordId slot", outputs.has(kRecordId)); tassert(5073710, "outputs must contain kResult slot", outputs.has(kResult)); auto idSlot = outputs.get(kRecordId); @@ -2564,7 +2564,7 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder 5851600, "should have one and only one child for GROUP", groupNode->children.size() == 1); tassert(5851601, "GROUP should have had group-by key expression", idExpr); - const auto& childNode = groupNode->children[0]; + const auto& childNode = groupNode->children[0].get(); const auto& accStmts = groupNode->accumulators; auto childStageType = childNode->getType(); @@ -2932,7 +2932,7 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder // Determine if our child is an index scan and extract it's key pattern, or empty BSONObj if our // child is not an IXSCAN node. BSONObj indexKeyPattern = [&]() { - auto childNode = filterNode->children[0]; + auto childNode = filterNode->children[0].get(); switch (childNode->getType()) { case StageType::STAGE_IXSCAN: return static_cast<const IndexScanNode*>(childNode)->index.keyPattern; @@ -2957,11 +2957,11 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder shardFiltererSlot, shardKeyPattern, std::move(indexKeyPattern), - filterNode->children[0], + filterNode->children[0].get(), std::move(childReqs)); } - auto [stage, outputs] = build(filterNode->children[0], childReqs); + auto [stage, outputs] = build(filterNode->children[0].get(), childReqs); // Build an expression to extract the shard key from the document based on the shard key // pattern. To do this, we iterate over the shard key pattern parts and build nested 'getField' diff --git a/src/mongo/db/query/sbe_stage_builder_lookup.cpp b/src/mongo/db/query/sbe_stage_builder_lookup.cpp index 58819f7c32e..98a0b139b17 100644 --- a/src/mongo/db/query/sbe_stage_builder_lookup.cpp +++ b/src/mongo/db/query/sbe_stage_builder_lookup.cpp @@ -1010,7 +1010,7 @@ std::pair<std::unique_ptr<sbe::PlanStage>, PlanStageSlots> SlotBasedStageBuilder _shouldProduceRecordIdSlot = false; auto localReqs = reqs.copy().set(kResult); - auto [localStage, localOutputs] = build(eqLookupNode->children[0], localReqs); + auto [localStage, localOutputs] = build(eqLookupNode->children[0].get(), localReqs); SlotId localDocumentSlot = localOutputs.get(PlanStageSlots::kResult); auto [matchedDocumentsSlot, foreignStage] = [&, localStage = std::move(localStage)]() mutable diff --git a/src/mongo/db/query/sbe_stage_builder_test.cpp b/src/mongo/db/query/sbe_stage_builder_test.cpp index f94bc45fc20..e712217a870 100644 --- a/src/mongo/db/query/sbe_stage_builder_test.cpp +++ b/src/mongo/db/query/sbe_stage_builder_test.cpp @@ -91,7 +91,7 @@ TEST_F(SbeStageBuilderTest, TestLimitOneVirtualScan) { auto virtScan = std::make_unique<VirtualScanNode>(docs, VirtualScanNode::ScanType::kCollScan, true); auto limitNode = std::make_unique<LimitNode>(); - limitNode->children.push_back(virtScan.release()); + limitNode->children.push_back(std::move(virtScan)); limitNode->limit = 1; // Make a QuerySolution from the root limitNode. diff --git a/src/mongo/s/chunk_manager.cpp b/src/mongo/s/chunk_manager.cpp index 888ba644b52..c269d700f5e 100644 --- a/src/mongo/s/chunk_manager.cpp +++ b/src/mongo/s/chunk_manager.cpp @@ -654,7 +654,7 @@ IndexBounds ChunkManager::collapseQuerySolution(const QuerySolutionNode* node) { if (node->children.size() == 1) { // e.g. FETCH -> IXSCAN - return collapseQuerySolution(node->children.front()); + return collapseQuerySolution(node->children.front().get()); } // children.size() > 1, assert it's OR / SORT_MERGE. @@ -671,20 +671,18 @@ IndexBounds ChunkManager::collapseQuerySolution(const QuerySolutionNode* node) { IndexBounds bounds; - for (std::vector<QuerySolutionNode*>::const_iterator it = node->children.begin(); - it != node->children.end(); - it++) { + for (auto it = node->children.begin(); it != node->children.end(); it++) { // The first branch under OR if (it == node->children.begin()) { invariant(bounds.size() == 0); - bounds = collapseQuerySolution(*it); + bounds = collapseQuerySolution(it->get()); if (bounds.size() == 0) { // Got unexpected node in query solution tree return IndexBounds(); } continue; } - IndexBounds childBounds = collapseQuerySolution(*it); + IndexBounds childBounds = collapseQuerySolution(it->get()); if (childBounds.size() == 0) { // Got unexpected node in query solution tree return IndexBounds(); |