SERVER-35455 Eliminate owned raw pointers from QueryPlannerAccess.

This fixes a previous version of this commit by adding
std::move() where copy elision is not guaranteed.

(cherry picked from commit 1d89d2c88bcb39045701b87612b866ae2eb49378)

7 files changed, 334 insertions, 352 deletions

diff --git a/src/mongo/db/exec/subplan.cpp b/src/mongo/db/exec/subplan.cpp
index 314a5652d1c..207e32b1ea8 100644
--- a/src/mongo/db/exec/subplan.cpp
+++ b/src/mongo/db/exec/subplan.cpp
@@ -397,7 +397,7 @@ Status SubplanStage::choosePlanForSubqueries(PlanYieldPolicy* yieldPolicy) {
 
     // Use the cached index assignments to build solnRoot. Takes ownership of '_orExpression'.
     std::unique_ptr<QuerySolutionNode> solnRoot(QueryPlannerAccess::buildIndexedDataAccess(
-        *_query, _orExpression.release(), false, _plannerParams.indices, _plannerParams));
+        *_query, std::move(_orExpression), _plannerParams.indices, _plannerParams));
 
     if (!solnRoot) {
         mongoutils::str::stream ss;
diff --git a/src/mongo/db/query/planner_access.cpp b/src/mongo/db/query/planner_access.cpp
index b40418e8439..00e8235ac84 100644
--- a/src/mongo/db/query/planner_access.cpp
+++ b/src/mongo/db/query/planner_access.cpp
@@ -117,8 +117,8 @@ bool scansAreEquivalent(const QuerySolutionNode* lhs, const QuerySolutionNode* r
  * their index scans reversed to provide the sort. Otherwise, returns an empty vector.
  * 'nodes' must not be empty.
  */
-std::vector<bool> canProvideSortWithMergeSort(const std::vector<QuerySolutionNode*>& nodes,
-                                              const BSONObj& requestedSort) {
+std::vector<bool> canProvideSortWithMergeSort(
+    const std::vector<std::unique_ptr<QuerySolutionNode>>& nodes, const BSONObj& requestedSort) {
     invariant(!nodes.empty());
     std::vector<bool> shouldReverseScan;
     const auto reverseSort = QueryPlannerCommon::reverseSortObj(requestedSort);
@@ -144,7 +144,6 @@ using std::unique_ptr;
 using std::vector;
 using stdx::make_unique;
 
-// static
 std::unique_ptr<QuerySolutionNode> QueryPlannerAccess::makeCollectionScan(
     const CanonicalQuery& query, bool tailable, const QueryPlannerParams& params) {
     // Make the (only) node, a collection scan.
@@ -178,12 +177,11 @@ std::unique_ptr<QuerySolutionNode> QueryPlannerAccess::makeCollectionScan(
     return std::move(csn);
 }
 
-// static
-QuerySolutionNode* QueryPlannerAccess::makeLeafNode(
+std::unique_ptr<QuerySolutionNode> QueryPlannerAccess::makeLeafNode(
     const CanonicalQuery& query,
     const IndexEntry& index,
     size_t pos,
-    MatchExpression* expr,
+    const MatchExpression* expr,
     IndexBoundsBuilder::BoundsTightness* tightnessOut) {
     // We're guaranteed that all GEO_NEARs are first.  This slightly violates the "sort index
     // predicates by their position in the compound index" rule but GEO_NEAR isn't an ixscan.
@@ -198,13 +196,13 @@ QuerySolutionNode* QueryPlannerAccess::makeLeafNode(
     if (MatchExpression::GEO_NEAR == expr->matchType()) {
         // We must not keep the expression node around.
         *tightnessOut = IndexBoundsBuilder::EXACT;
-        GeoNearMatchExpression* nearExpr = static_cast<GeoNearMatchExpression*>(expr);
+        auto nearExpr = static_cast<const GeoNearMatchExpression*>(expr);
 
         BSONElement elt = index.keyPattern.firstElement();
         bool indexIs2D = (String == elt.type() && "2d" == elt.String());
 
         if (indexIs2D) {
-            GeoNear2DNode* ret = new GeoNear2DNode(index);
+            auto ret = stdx::make_unique<GeoNear2DNode>(index);
             ret->nq = &nearExpr->getData();
             ret->baseBounds.fields.resize(index.keyPattern.nFields());
             if (NULL != query.getProj()) {
@@ -212,22 +210,22 @@ QuerySolutionNode* QueryPlannerAccess::makeLeafNode(
                 ret->addDistMeta = query.getProj()->wantGeoNearDistance();
             }
 
-            return ret;
+            return std::move(ret);
         } else {
-            GeoNear2DSphereNode* ret = new GeoNear2DSphereNode(index);
+            auto ret = stdx::make_unique<GeoNear2DSphereNode>(index);
             ret->nq = &nearExpr->getData();
             ret->baseBounds.fields.resize(index.keyPattern.nFields());
             if (NULL != query.getProj()) {
                 ret->addPointMeta = query.getProj()->wantGeoNearPoint();
                 ret->addDistMeta = query.getProj()->wantGeoNearDistance();
             }
-            return ret;
+            return std::move(ret);
         }
     } else if (MatchExpression::TEXT == expr->matchType()) {
         // We must not keep the expression node around.
         *tightnessOut = IndexBoundsBuilder::EXACT;
-        TextMatchExpressionBase* textExpr = static_cast<TextMatchExpressionBase*>(expr);
-        TextNode* ret = new TextNode(index);
+        auto textExpr = static_cast<const TextMatchExpressionBase*>(expr);
+        auto ret = stdx::make_unique<TextNode>(index);
         ret->ftsQuery = textExpr->getFTSQuery().clone();
 
         // Count the number of prefix fields before the "text" field.
@@ -240,11 +238,11 @@ QuerySolutionNode* QueryPlannerAccess::makeLeafNode(
             ++(ret->numPrefixFields);
         }
 
-        return ret;
+        return std::move(ret);
     } else {
         // Note that indexKeyPattern.firstElement().fieldName() may not equal expr->path()
         // because expr might be inside an array operator that provides a path prefix.
-        IndexScanNode* isn = new IndexScanNode(index);
+        auto isn = stdx::make_unique<IndexScanNode>(index);
         isn->bounds.fields.resize(index.keyPattern.nFields());
         isn->maxScan = query.getQueryRequest().getMaxScan();
         isn->addKeyMetadata = query.getQueryRequest().returnKey();
@@ -262,7 +260,7 @@ QuerySolutionNode* QueryPlannerAccess::makeLeafNode(
 
         IndexBoundsBuilder::translate(expr, keyElt, index, &isn->bounds.fields[pos], tightnessOut);
 
-        return isn;
+        return std::move(isn);
     }
 }
 
@@ -450,7 +448,6 @@ void QueryPlannerAccess::mergeWithLeafNode(MatchExpression* expr, ScanBuildingSt
     }
 }
 
-// static
 void QueryPlannerAccess::finishTextNode(QuerySolutionNode* node, const IndexEntry& index) {
     TextNode* tn = static_cast<TextNode*>(node);
 
@@ -533,7 +530,6 @@ void QueryPlannerAccess::finishTextNode(QuerySolutionNode* node, const IndexEntr
     tn->indexPrefix = prefixBob.obj();
 }
 
-// static
 bool QueryPlannerAccess::orNeedsFetch(const ScanBuildingState* scanState) {
     if (scanState->loosestBounds == IndexBoundsBuilder::EXACT) {
         return false;
@@ -546,9 +542,8 @@ bool QueryPlannerAccess::orNeedsFetch(const ScanBuildingState* scanState) {
     }
 }
 
-// static
 void QueryPlannerAccess::finishAndOutputLeaf(ScanBuildingState* scanState,
-                                             vector<QuerySolutionNode*>* out) {
+                                             vector<std::unique_ptr<QuerySolutionNode>>* out) {
     finishLeafNode(scanState->currentScan.get(), scanState->indices[scanState->currentIndexNumber]);
 
     if (MatchExpression::OR == scanState->root->matchType()) {
@@ -556,13 +551,13 @@ void QueryPlannerAccess::finishAndOutputLeaf(ScanBuildingState* scanState,
             // In order to correctly evaluate the predicates for this index, we have to
             // fetch the full documents. Add a fetch node above the index scan whose filter
             // includes *all* of the predicates used to generate the ixscan.
-            FetchNode* fetch = new FetchNode();
+            auto fetch = stdx::make_unique<FetchNode>();
             // Takes ownership.
-            fetch->filter.reset(scanState->curOr.release());
+            fetch->filter = std::move(scanState->curOr);
             // Takes ownership.
             fetch->children.push_back(scanState->currentScan.release());
 
-            scanState->currentScan.reset(fetch);
+            scanState->currentScan = std::move(fetch);
         } else if (scanState->loosestBounds == IndexBoundsBuilder::INEXACT_COVERED) {
             // This an OR, at least one of the predicates used to generate 'currentScan'
             // is inexact covered, but none is inexact fetch. This means that we can put
@@ -573,14 +568,13 @@ void QueryPlannerAccess::finishAndOutputLeaf(ScanBuildingState* scanState,
             //   Say we have index {a: 1} and query {$or: [{a: /foo/}, {a: /bar/}]}.
             //   The entire query, {$or: [{a: /foo/}, {a: /bar/}]}, should be a filter
             //   in the index scan stage itself.
-            scanState->currentScan->filter.reset(scanState->curOr.release());
+            scanState->currentScan->filter = std::move(scanState->curOr);
         }
     }
 
-    out->push_back(scanState->currentScan.release());
+    out->push_back(std::move(scanState->currentScan));
 }
 
-// static
 void QueryPlannerAccess::finishLeafNode(QuerySolutionNode* node, const IndexEntry& index) {
     const StageType type = node->getType();
 
@@ -649,7 +643,6 @@ void QueryPlannerAccess::finishLeafNode(QuerySolutionNode* node, const IndexEntr
     IndexBoundsBuilder::alignBounds(bounds, index.keyPattern);
 }
 
-// static
 void QueryPlannerAccess::findElemMatchChildren(const MatchExpression* node,
                                                vector<MatchExpression*>* out,
                                                vector<MatchExpression*>* subnodesOut) {
@@ -666,12 +659,9 @@ void QueryPlannerAccess::findElemMatchChildren(const MatchExpression* node,
     }
 }
 
-// static
-std::vector<QuerySolutionNode*> QueryPlannerAccess::collapseEquivalentScans(
-    const std::vector<QuerySolutionNode*> scans) {
-    std::vector<std::unique_ptr<QuerySolutionNode>> ownedScans =
-        transitional_tools_do_not_use::spool_vector(scans);
-    invariant(ownedScans.size() > 0);
+std::vector<std::unique_ptr<QuerySolutionNode>> QueryPlannerAccess::collapseEquivalentScans(
+    std::vector<std::unique_ptr<QuerySolutionNode>> scans) {
+    invariant(scans.size() > 0);
 
     // Scans that need to be collapsed will be adjacent to each other in the list due to how we
     // sort the query predicate. We step through the list, either merging the current scan into
@@ -679,11 +669,11 @@ std::vector<QuerySolutionNode*> QueryPlannerAccess::collapseEquivalentScans(
     // be merged.
     std::vector<std::unique_ptr<QuerySolutionNode>> collapsedScans;
 
-    collapsedScans.push_back(std::move(ownedScans[0]));
-    for (size_t i = 1; i < ownedScans.size(); ++i) {
-        if (scansAreEquivalent(collapsedScans.back().get(), ownedScans[i].get())) {
+    collapsedScans.push_back(std::move(scans[0]));
+    for (size_t i = 1; i < scans.size(); ++i) {
+        if (scansAreEquivalent(collapsedScans.back().get(), scans[i].get())) {
             // We collapse the entry from 'ownedScans' into the back of 'collapsedScans'.
-            std::unique_ptr<QuerySolutionNode> collapseFrom(std::move(ownedScans[i]));
+            std::unique_ptr<QuerySolutionNode> collapseFrom = std::move(scans[i]);
             FetchNode* collapseFromFetch = getFetchNode(collapseFrom.get());
             FetchNode* collapseIntoFetch = getFetchNode(collapsedScans.back().get());
 
@@ -713,21 +703,20 @@ std::vector<QuerySolutionNode*> QueryPlannerAccess::collapseEquivalentScans(
             collapseIntoFetch->filter = MatchExpression::optimize(std::move(collapsedFilter));
         } else {
             // Scans are not equivalent and can't be collapsed.
-            collapsedScans.push_back(std::move(ownedScans[i]));
+            collapsedScans.push_back(std::move(scans[i]));
         }
     }
 
     invariant(collapsedScans.size() > 0);
-    return transitional_tools_do_not_use::leak_vector(collapsedScans);
+    return collapsedScans;
 }
 
-// static
 bool QueryPlannerAccess::processIndexScans(const CanonicalQuery& query,
                                            MatchExpression* root,
                                            bool inArrayOperator,
                                            const std::vector<IndexEntry>& indices,
                                            const QueryPlannerParams& params,
-                                           std::vector<QuerySolutionNode*>* out) {
+                                           std::vector<std::unique_ptr<QuerySolutionNode>>* out) {
     // Initialize the ScanBuildingState.
     ScanBuildingState scanState(root, inArrayOperator, indices);
 
@@ -807,11 +796,11 @@ bool QueryPlannerAccess::processIndexScans(const CanonicalQuery& query,
             // Reset state before producing a new leaf.
             scanState.resetForNextScan(scanState.ixtag);
 
-            scanState.currentScan.reset(makeLeafNode(query,
-                                                     indices[scanState.currentIndexNumber],
-                                                     scanState.ixtag->pos,
-                                                     child,
-                                                     &scanState.tightness));
+            scanState.currentScan = makeLeafNode(query,
+                                                 indices[scanState.currentIndexNumber],
+                                                 scanState.ixtag->pos,
+                                                 child,
+                                                 &scanState.tightness);
 
             handleFilter(&scanState);
         }
@@ -825,11 +814,11 @@ bool QueryPlannerAccess::processIndexScans(const CanonicalQuery& query,
     return true;
 }
 
-// static
-bool QueryPlannerAccess::processIndexScansElemMatch(const CanonicalQuery& query,
-                                                    ScanBuildingState* scanState,
-                                                    const QueryPlannerParams& params,
-                                                    std::vector<QuerySolutionNode*>* out) {
+bool QueryPlannerAccess::processIndexScansElemMatch(
+    const CanonicalQuery& query,
+    ScanBuildingState* scanState,
+    const QueryPlannerParams& params,
+    std::vector<std::unique_ptr<QuerySolutionNode>>* out) {
     MatchExpression* root = scanState->root;
     MatchExpression* child = root->getChild(scanState->curChild);
     const vector<IndexEntry>& indices = scanState->indices;
@@ -858,23 +847,22 @@ bool QueryPlannerAccess::processIndexScansElemMatch(const CanonicalQuery& query,
         MatchExpression* subnode = emSubnodes[i];
 
         if (!Indexability::isBoundsGenerating(subnode)) {
-            // Must pass true for 'inArrayOperator' because the subnode is
-            // beneath an ELEM_MATCH_OBJECT.
-            QuerySolutionNode* childSolution =
-                buildIndexedDataAccess(query, subnode, true, indices, params);
-
-            // buildIndexedDataAccess(...) returns NULL in error conditions, when
-            // it is unable to construct a query solution from a tagged match
-            // expression tree. If we are unable to construct a solution according
-            // to the instructions from the enumerator, then we bail out early
-            // (by returning false) rather than continuing on and potentially
-            // constructing an invalid solution tree.
-            if (NULL == childSolution) {
+            // 'subnode' is beneath an $elemMatch. When planning the children of array operators, we
+            // keep ownership of the match expression node. Therefore, we pass nullptr for the
+            // 'ownedRoot' argument.
+            auto childSolution = _buildIndexedDataAccess(query, subnode, nullptr, indices, params);
+
+            // _buildIndexedDataAccess(...) returns NULL in error conditions, when it is unable to
+            // construct a query solution from a tagged match expression tree. If we are unable to
+            // construct a solution according to the instructions from the enumerator, then we bail
+            // out early (by returning false) rather than continuing on and potentially constructing
+            // an invalid solution tree.
+            if (!childSolution) {
                 return false;
             }
 
             // Output the resulting solution tree.
-            out->push_back(childSolution);
+            out->push_back(std::move(childSolution));
         }
     }
 
@@ -913,11 +901,11 @@ bool QueryPlannerAccess::processIndexScansElemMatch(const CanonicalQuery& query,
             scanState->currentIndexNumber = scanState->ixtag->index;
 
             scanState->tightness = IndexBoundsBuilder::INEXACT_FETCH;
-            scanState->currentScan.reset(makeLeafNode(query,
-                                                      indices[scanState->currentIndexNumber],
-                                                      scanState->ixtag->pos,
-                                                      emChild,
-                                                      &scanState->tightness));
+            scanState->currentScan = makeLeafNode(query,
+                                                  indices[scanState->currentIndexNumber],
+                                                  scanState->ixtag->pos,
+                                                  emChild,
+                                                  &scanState->tightness);
         }
     }
 
@@ -928,52 +916,49 @@ bool QueryPlannerAccess::processIndexScansElemMatch(const CanonicalQuery& query,
     return true;
 }
 
-// static
-bool QueryPlannerAccess::processIndexScansSubnode(const CanonicalQuery& query,
-                                                  ScanBuildingState* scanState,
-                                                  const QueryPlannerParams& params,
-                                                  std::vector<QuerySolutionNode*>* out) {
+bool QueryPlannerAccess::processIndexScansSubnode(
+    const CanonicalQuery& query,
+    ScanBuildingState* scanState,
+    const QueryPlannerParams& params,
+    std::vector<std::unique_ptr<QuerySolutionNode>>* out) {
     MatchExpression* root = scanState->root;
     MatchExpression* child = root->getChild(scanState->curChild);
     const vector<IndexEntry>& indices = scanState->indices;
     bool inArrayOperator = scanState->inArrayOperator;
 
+    // We may detach the current child from the tree and assume ownership.
+    std::unique_ptr<MatchExpression> ownedChild;
+
     if (MatchExpression::AND == root->matchType() &&
         MatchExpression::ELEM_MATCH_OBJECT == child->matchType()) {
         return processIndexScansElemMatch(query, scanState, params, out);
     } else if (!inArrayOperator) {
-        // The logical sub-tree is responsible for fully evaluating itself.  Any
-        // required filters or fetches are already hung on it.  As such, we remove the
-        // filter branch from our tree.  buildIndexedDataAccess takes ownership of the
-        // child.
+        // The logical sub-tree is responsible for fully evaluating itself. Any required filters or
+        // fetches are already hung on it. As such, we remove the filter branch from our tree and
+        // assume ownership of it.
         root->getChildVector()->erase(root->getChildVector()->begin() + scanState->curChild);
-        // The curChild of today is the curChild+1 of yesterday.
+        ownedChild.reset(child);
     } else {
         ++scanState->curChild;
     }
 
     // If inArrayOperator: takes ownership of child, which is OK, since we detached
     // child from root.
-    QuerySolutionNode* childSolution =
-        buildIndexedDataAccess(query, child, inArrayOperator, indices, params);
-    if (NULL == childSolution) {
+    auto childSolution =
+        _buildIndexedDataAccess(query, child, std::move(ownedChild), indices, params);
+    if (!childSolution) {
         return false;
     }
-    out->push_back(childSolution);
+    out->push_back(std::move(childSolution));
     return true;
 }
 
-// static
-QuerySolutionNode* QueryPlannerAccess::buildIndexedAnd(const CanonicalQuery& query,
-                                                       MatchExpression* root,
-                                                       bool inArrayOperator,
-                                                       const vector<IndexEntry>& indices,
-                                                       const QueryPlannerParams& params) {
-    unique_ptr<MatchExpression> autoRoot;
-    if (!inArrayOperator) {
-        autoRoot.reset(root);
-    }
-
+std::unique_ptr<QuerySolutionNode> QueryPlannerAccess::buildIndexedAnd(
+    const CanonicalQuery& query,
+    MatchExpression* root,
+    std::unique_ptr<MatchExpression> ownedRoot,
+    const vector<IndexEntry>& indices,
+    const QueryPlannerParams& params) {
     // If we are not allowed to trim for ixisect, then clone the match expression before
     // passing it to processIndexScans(), which may do the trimming. If we end up with
     // an index intersection solution, then we use our copy of the match expression to be
@@ -985,7 +970,8 @@ QuerySolutionNode* QueryPlannerAccess::buildIndexedAnd(const CanonicalQuery& que
         clonedRoot = root->shallowClone();
     }
 
-    vector<QuerySolutionNode*> ixscanNodes;
+    std::vector<std::unique_ptr<QuerySolutionNode>> ixscanNodes;
+    const bool inArrayOperator = !ownedRoot;
     if (!processIndexScans(query, root, inArrayOperator, indices, params, &ixscanNodes)) {
         return NULL;
     }
@@ -996,7 +982,7 @@ QuerySolutionNode* QueryPlannerAccess::buildIndexedAnd(const CanonicalQuery& que
     //
 
     // This is the node we're about to return.
-    QuerySolutionNode* andResult;
+    std::unique_ptr<QuerySolutionNode> andResult;
 
     // We must use an index for at least one child of the AND.  We shouldn't be here if this
     // isn't the case.
@@ -1004,7 +990,7 @@ QuerySolutionNode* QueryPlannerAccess::buildIndexedAnd(const CanonicalQuery& que
 
     // Short-circuit: an AND of one child is just the child.
     if (ixscanNodes.size() == 1) {
-        andResult = ixscanNodes[0];
+        andResult = std::move(ixscanNodes[0]);
     } else {
         // Figure out if we want AndHashNode or AndSortedNode.
         bool allSortedByDiskLoc = true;
@@ -1015,21 +1001,24 @@ QuerySolutionNode* QueryPlannerAccess::buildIndexedAnd(const CanonicalQuery& que
             }
         }
         if (allSortedByDiskLoc) {
-            AndSortedNode* asn = new AndSortedNode();
-            asn->children.swap(ixscanNodes);
-            andResult = asn;
+            auto asn = stdx::make_unique<AndSortedNode>();
+            asn->addChildren(std::move(ixscanNodes));
+            andResult = std::move(asn);
         } else if (internalQueryPlannerEnableHashIntersection.load()) {
-            AndHashNode* ahn = new AndHashNode();
-            ahn->children.swap(ixscanNodes);
-            andResult = ahn;
+            {
+                auto ahn = stdx::make_unique<AndHashNode>();
+                ahn->addChildren(std::move(ixscanNodes));
+                andResult = std::move(ahn);
+            }
+
             // The AndHashNode provides the sort order of its last child.  If any of the
             // possible subnodes of AndHashNode provides the sort order we care about, we put
             // that one last.
-            for (size_t i = 0; i < ahn->children.size(); ++i) {
-                ahn->children[i]->computeProperties();
-                const BSONObjSet& sorts = ahn->children[i]->getSort();
+            for (size_t i = 0; i < andResult->children.size(); ++i) {
+                andResult->children[i]->computeProperties();
+                const BSONObjSet& sorts = andResult->children[i]->getSort();
                 if (sorts.end() != sorts.find(query.getQueryRequest().getSort())) {
-                    std::swap(ahn->children[i], ahn->children.back());
+                    std::swap(andResult->children[i], andResult->children.back());
                     break;
                 }
             }
@@ -1038,11 +1027,7 @@ QuerySolutionNode* QueryPlannerAccess::buildIndexedAnd(const CanonicalQuery& que
             // Clean up the index scans and bail out by returning NULL.
             LOG(5) << "Can't build index intersection solution: "
                    << "AND_SORTED is not possible and AND_HASH is disabled.";
-
-            for (size_t i = 0; i < ixscanNodes.size(); i++) {
-                delete ixscanNodes[i];
-            }
-            return NULL;
+            return nullptr;
         }
     }
 
@@ -1057,32 +1042,32 @@ QuerySolutionNode* QueryPlannerAccess::buildIndexedAnd(const CanonicalQuery& que
         // We got an index intersection solution, and we aren't allowed to answer predicates
         // using the index. We add a fetch with the entire filter.
         invariant(clonedRoot.get());
-        FetchNode* fetch = new FetchNode();
-        fetch->filter.reset(clonedRoot.release());
+        auto fetch = stdx::make_unique<FetchNode>();
+        fetch->filter = std::move(clonedRoot);
         // Takes ownership of 'andResult'.
-        fetch->children.push_back(andResult);
-        return fetch;
+        fetch->children.push_back(andResult.release());
+        return std::move(fetch);
     }
 
     // If there are any nodes still attached to the AND, we can't answer them using the
     // index, so we put a fetch with filter.
     if (root->numChildren() > 0) {
-        FetchNode* fetch = new FetchNode();
-        verify(NULL != autoRoot.get());
-        if (autoRoot->numChildren() == 1) {
+        auto fetch = stdx::make_unique<FetchNode>();
+        verify(ownedRoot);
+        if (ownedRoot->numChildren() == 1) {
             // An $and of one thing is that thing.
-            MatchExpression* child = autoRoot->getChild(0);
-            autoRoot->getChildVector()->clear();
+            MatchExpression* child = ownedRoot->getChild(0);
+            ownedRoot->getChildVector()->clear();
             // Takes ownership.
             fetch->filter.reset(child);
             // 'autoRoot' will delete the empty $and.
         } else {  // root->numChildren() > 1
             // Takes ownership.
-            fetch->filter.reset(autoRoot.release());
+            fetch->filter = std::move(ownedRoot);
         }
         // takes ownership
-        fetch->children.push_back(andResult);
-        andResult = fetch;
+        fetch->children.push_back(andResult.release());
+        andResult = std::move(fetch);
     } else {
         // root has no children, let autoRoot get rid of it when it goes out of scope.
     }
@@ -1090,18 +1075,15 @@ QuerySolutionNode* QueryPlannerAccess::buildIndexedAnd(const CanonicalQuery& que
     return andResult;
 }
 
-// static
-QuerySolutionNode* QueryPlannerAccess::buildIndexedOr(const CanonicalQuery& query,
-                                                      MatchExpression* root,
-                                                      bool inArrayOperator,
-                                                      const vector<IndexEntry>& indices,
-                                                      const QueryPlannerParams& params) {
-    unique_ptr<MatchExpression> autoRoot;
-    if (!inArrayOperator) {
-        autoRoot.reset(root);
-    }
+std::unique_ptr<QuerySolutionNode> QueryPlannerAccess::buildIndexedOr(
+    const CanonicalQuery& query,
+    MatchExpression* root,
+    std::unique_ptr<MatchExpression> ownedRoot,
+    const vector<IndexEntry>& indices,
+    const QueryPlannerParams& params) {
 
-    vector<QuerySolutionNode*> ixscanNodes;
+    const bool inArrayOperator = !ownedRoot;
+    std::vector<std::unique_ptr<QuerySolutionNode>> ixscanNodes;
     if (!processIndexScans(query, root, inArrayOperator, indices, params, &ixscanNodes)) {
         return NULL;
     }
@@ -1119,13 +1101,13 @@ QuerySolutionNode* QueryPlannerAccess::buildIndexedOr(const CanonicalQuery& quer
 
     // If all index scans are identical, then we collapse them into a single scan. This prevents
     // us from creating OR plans where the branches of the OR perform duplicate work.
-    ixscanNodes = collapseEquivalentScans(ixscanNodes);
+    ixscanNodes = collapseEquivalentScans(std::move(ixscanNodes));
 
-    QuerySolutionNode* orResult = NULL;
+    std::unique_ptr<QuerySolutionNode> orResult;
 
     // An OR of one node is just that node.
     if (1 == ixscanNodes.size()) {
-        orResult = ixscanNodes[0];
+        orResult = std::move(ixscanNodes[0]);
     } else {
         std::vector<bool> shouldReverseScan;
 
@@ -1142,18 +1124,18 @@ QuerySolutionNode* QueryPlannerAccess::buildIndexedOr(const CanonicalQuery& quer
             invariant(ixscanNodes.size() == shouldReverseScan.size());
             for (size_t i = 0; i < ixscanNodes.size(); ++i) {
                 if (shouldReverseScan[i]) {
-                    QueryPlannerCommon::reverseScans(ixscanNodes[i]);
+                    QueryPlannerCommon::reverseScans(ixscanNodes[i].get());
                 }
             }
 
-            MergeSortNode* msn = new MergeSortNode();
+            auto msn = stdx::make_unique<MergeSortNode>();
             msn->sort = query.getQueryRequest().getSort();
-            msn->children.swap(ixscanNodes);
-            orResult = msn;
+            msn->addChildren(std::move(ixscanNodes));
+            orResult = std::move(msn);
         } else {
-            OrNode* orn = new OrNode();
-            orn->children.swap(ixscanNodes);
-            orResult = orn;
+            auto orn = stdx::make_unique<OrNode>();
+            orn->addChildren(std::move(ixscanNodes));
+            orResult = std::move(orn);
         }
     }
 
@@ -1168,47 +1150,48 @@ QuerySolutionNode* QueryPlannerAccess::buildIndexedOr(const CanonicalQuery& quer
     return orResult;
 }
 
-// static
-QuerySolutionNode* QueryPlannerAccess::buildIndexedDataAccess(const CanonicalQuery& query,
-                                                              MatchExpression* root,
-                                                              bool inArrayOperator,
-                                                              const vector<IndexEntry>& indices,
-                                                              const QueryPlannerParams& params) {
+std::unique_ptr<QuerySolutionNode> QueryPlannerAccess::buildIndexedDataAccess(
+    const CanonicalQuery& query,
+    std::unique_ptr<MatchExpression> root,
+    const vector<IndexEntry>& indices,
+    const QueryPlannerParams& params) {
+    MatchExpression* unownedRoot = root.get();
+    return _buildIndexedDataAccess(query, unownedRoot, std::move(root), indices, params);
+}
+
+std::unique_ptr<QuerySolutionNode> QueryPlannerAccess::_buildIndexedDataAccess(
+    const CanonicalQuery& query,
+    MatchExpression* root,
+    std::unique_ptr<MatchExpression> ownedRoot,
+    const vector<IndexEntry>& indices,
+    const QueryPlannerParams& params) {
     if (root->getCategory() == MatchExpression::MatchCategory::kLogical &&
         !Indexability::isBoundsGeneratingNot(root)) {
         if (MatchExpression::AND == root->matchType()) {
             // Takes ownership of root.
-            return buildIndexedAnd(query, root, inArrayOperator, indices, params);
+            return buildIndexedAnd(query, root, std::move(ownedRoot), indices, params);
         } else if (MatchExpression::OR == root->matchType()) {
             // Takes ownership of root.
-            return buildIndexedOr(query, root, inArrayOperator, indices, params);
+            return buildIndexedOr(query, root, std::move(ownedRoot), indices, params);
         } else {
-            // Can't do anything with negated logical nodes index-wise.
-            if (!inArrayOperator) {
-                delete root;
-            }
-            return NULL;
+            return nullptr;
         }
     } else {
-        unique_ptr<MatchExpression> autoRoot;
-        if (!inArrayOperator) {
-            autoRoot.reset(root);
-        }
-
-        if (NULL == root->getTag()) {
+        if (!root->getTag()) {
             // No index to use here, not in the context of logical operator, so we're SOL.
-            return NULL;
+            return nullptr;
         } else if (Indexability::isBoundsGenerating(root)) {
             // Make an index scan over the tagged index #.
             IndexTag* tag = static_cast<IndexTag*>(root->getTag());
 
             IndexBoundsBuilder::BoundsTightness tightness = IndexBoundsBuilder::EXACT;
-            QuerySolutionNode* soln =
-                makeLeafNode(query, indices[tag->index], tag->pos, root, &tightness);
+            auto soln = makeLeafNode(query, indices[tag->index], tag->pos, root, &tightness);
             verify(NULL != soln);
-            finishLeafNode(soln, indices[tag->index]);
+            finishLeafNode(soln.get(), indices[tag->index]);
 
-            if (inArrayOperator) {
+            if (!ownedRoot) {
+                // We're performing access planning for the child of an array operator such as
+                // $elemMatch value.
                 return soln;
             }
 
@@ -1224,52 +1207,49 @@ QuerySolutionNode* QueryPlannerAccess::buildIndexedDataAccess(const CanonicalQue
             } else if (tightness == IndexBoundsBuilder::INEXACT_COVERED &&
                        !indices[tag->index].multikey) {
                 verify(NULL == soln->filter.get());
-                soln->filter.reset(autoRoot.release());
+                soln->filter = std::move(ownedRoot);
                 return soln;
             } else {
-                FetchNode* fetch = new FetchNode();
-                verify(NULL != autoRoot.get());
-                fetch->filter.reset(autoRoot.release());
-                fetch->children.push_back(soln);
-                return fetch;
+                auto fetch = stdx::make_unique<FetchNode>();
+                fetch->filter = std::move(ownedRoot);
+                fetch->children.push_back(soln.release());
+                return std::move(fetch);
             }
         } else if (Indexability::arrayUsesIndexOnChildren(root)) {
-            QuerySolutionNode* solution = NULL;
+            std::unique_ptr<QuerySolutionNode> solution;
 
             invariant(root->matchType() == MatchExpression::ELEM_MATCH_OBJECT);
             // The child is an AND.
             invariant(1 == root->numChildren());
-            solution = buildIndexedDataAccess(query, root->getChild(0), true, indices, params);
-            if (NULL == solution) {
-                return NULL;
+
+            // Recursively build a data access plan for the child of the $elemMatch object. We
+            // maintain ownership of 'ownedRoot'.
+            solution = _buildIndexedDataAccess(query, root->getChild(0), nullptr, indices, params);
+            if (!solution) {
+                return nullptr;
             }
 
             // There may be an array operator above us.
-            if (inArrayOperator) {
+            if (!ownedRoot) {
                 return solution;
             }
 
-            FetchNode* fetch = new FetchNode();
-            // Takes ownership of 'root'.
-            verify(NULL != autoRoot.get());
-            fetch->filter.reset(autoRoot.release());
-            fetch->children.push_back(solution);
-            return fetch;
+            auto fetch = stdx::make_unique<FetchNode>();
+            fetch->filter = std::move(ownedRoot);
+            fetch->children.push_back(solution.release());
+            return std::move(fetch);
         }
     }
 
-    if (!inArrayOperator) {
-        delete root;
-    }
-
-    return NULL;
+    return nullptr;
 }
 
-QuerySolutionNode* QueryPlannerAccess::scanWholeIndex(const IndexEntry& index,
-                                                      const CanonicalQuery& query,
-                                                      const QueryPlannerParams& params,
-                                                      int direction) {
-    QuerySolutionNode* solnRoot = NULL;
+std::unique_ptr<QuerySolutionNode> QueryPlannerAccess::scanWholeIndex(
+    const IndexEntry& index,
+    const CanonicalQuery& query,
+    const QueryPlannerParams& params,
+    int direction) {
+    std::unique_ptr<QuerySolutionNode> solnRoot;
 
     // Build an ixscan over the id index, use it, and return it.
     unique_ptr<IndexScanNode> isn = make_unique<IndexScanNode>(index);
@@ -1288,20 +1268,19 @@ QuerySolutionNode* QueryPlannerAccess::scanWholeIndex(const IndexEntry& index,
 
     // If it's find({}) remove the no-op root.
     if (MatchExpression::AND == filter->matchType() && (0 == filter->numChildren())) {
-        solnRoot = isn.release();
+        solnRoot = std::move(isn);
     } else {
         // TODO: We may not need to do the fetch if the predicates in root are covered.  But
         // for now it's safe (though *maybe* slower).
         unique_ptr<FetchNode> fetch = make_unique<FetchNode>();
         fetch->filter = std::move(filter);
         fetch->children.push_back(isn.release());
-        solnRoot = fetch.release();
+        solnRoot = std::move(fetch);
     }
 
     return solnRoot;
 }
 
-// static
 void QueryPlannerAccess::addFilterToSolutionNode(QuerySolutionNode* node,
                                                  MatchExpression* match,
                                                  MatchExpression::MatchType type) {
@@ -1330,7 +1309,6 @@ void QueryPlannerAccess::addFilterToSolutionNode(QuerySolutionNode* node,
     }
 }
 
-// static
 void QueryPlannerAccess::handleFilter(ScanBuildingState* scanState) {
     if (MatchExpression::OR == scanState->root->matchType()) {
         handleFilterOr(scanState);
@@ -1342,7 +1320,6 @@ void QueryPlannerAccess::handleFilter(ScanBuildingState* scanState) {
     }
 }
 
-// static
 void QueryPlannerAccess::handleFilterOr(ScanBuildingState* scanState) {
     MatchExpression* root = scanState->root;
     MatchExpression* child = root->getChild(scanState->curChild);
@@ -1363,7 +1340,6 @@ void QueryPlannerAccess::handleFilterOr(ScanBuildingState* scanState) {
     }
 }
 
-// static
 void QueryPlannerAccess::handleFilterAnd(ScanBuildingState* scanState) {
     MatchExpression* root = scanState->root;
     MatchExpression* child = root->getChild(scanState->curChild);
@@ -1399,15 +1375,16 @@ void QueryPlannerAccess::handleFilterAnd(ScanBuildingState* scanState) {
     }
 }
 
-QuerySolutionNode* QueryPlannerAccess::makeIndexScan(const IndexEntry& index,
-                                                     const CanonicalQuery& query,
-                                                     const QueryPlannerParams& params,
-                                                     const BSONObj& startKey,
-                                                     const BSONObj& endKey) {
-    QuerySolutionNode* solnRoot = NULL;
+std::unique_ptr<QuerySolutionNode> QueryPlannerAccess::makeIndexScan(
+    const IndexEntry& index,
+    const CanonicalQuery& query,
+    const QueryPlannerParams& params,
+    const BSONObj& startKey,
+    const BSONObj& endKey) {
+    std::unique_ptr<QuerySolutionNode> solnRoot;
 
     // Build an ixscan over the id index, use it, and return it.
-    IndexScanNode* isn = new IndexScanNode(index);
+    auto isn = stdx::make_unique<IndexScanNode>(index);
     isn->direction = 1;
     isn->maxScan = query.getQueryRequest().getMaxScan();
     isn->addKeyMetadata = query.getQueryRequest().returnKey();
@@ -1421,14 +1398,14 @@ QuerySolutionNode* QueryPlannerAccess::makeIndexScan(const IndexEntry& index,
 
     // If it's find({}) remove the no-op root.
     if (MatchExpression::AND == filter->matchType() && (0 == filter->numChildren())) {
-        solnRoot = isn;
+        solnRoot = std::move(isn);
     } else {
         // TODO: We may not need to do the fetch if the predicates in root are covered.  But
         // for now it's safe (though *maybe* slower).
         unique_ptr<FetchNode> fetch = make_unique<FetchNode>();
         fetch->filter = std::move(filter);
-        fetch->children.push_back(isn);
-        solnRoot = fetch.release();
+        fetch->children.push_back(isn.release());
+        solnRoot = std::move(fetch);
     }
 
     return solnRoot;
diff --git a/src/mongo/db/query/planner_access.h b/src/mongo/db/query/planner_access.h
index 2dafa1841d2..6a9294b1282 100644
--- a/src/mongo/db/query/planner_access.h
+++ b/src/mongo/db/query/planner_access.h
@@ -97,6 +97,42 @@ namespace mongo {
 class QueryPlannerAccess {
 public:
     /**
+     * Return a CollectionScanNode that scans as requested in 'query'.
+     */
+    static std::unique_ptr<QuerySolutionNode> makeCollectionScan(const CanonicalQuery& query,
+                                                                 bool tailable,
+                                                                 const QueryPlannerParams& params);
+
+    /**
+     * Return a plan that uses the provided index as a proxy for a collection scan.
+     */
+    static std::unique_ptr<QuerySolutionNode> scanWholeIndex(const IndexEntry& index,
+                                                             const CanonicalQuery& query,
+                                                             const QueryPlannerParams& params,
+                                                             int direction = 1);
+
+    /**
+     * Return a plan that scans the provided index from [startKey to endKey).
+     */
+    static std::unique_ptr<QuerySolutionNode> makeIndexScan(const IndexEntry& index,
+                                                            const CanonicalQuery& query,
+                                                            const QueryPlannerParams& params,
+                                                            const BSONObj& startKey,
+                                                            const BSONObj& endKey);
+
+    /**
+     * Consructs a data access plan for 'query' which answers the predicate contained in 'root'.
+     * Assumes the presence of the passed in indices. Planning behavior is controlled by the
+     * settings in 'params'.
+     */
+    static std::unique_ptr<QuerySolutionNode> buildIndexedDataAccess(
+        const CanonicalQuery& query,
+        std::unique_ptr<MatchExpression> root,
+        const std::vector<IndexEntry>& indices,
+        const QueryPlannerParams& params);
+
+private:
+    /**
      * Building the leaves (i.e. the index scans) is done by looping through
      * predicates one at a time. During the process, there is a fair amount of state
      * information to keep track of, which we consolidate into this data structure.
@@ -128,7 +164,7 @@ public:
             loosestBounds = IndexBoundsBuilder::EXACT;
 
             if (MatchExpression::OR == root->matchType()) {
-                curOr.reset(new OrMatchExpression());
+                curOr = stdx::make_unique<OrMatchExpression>();
             }
         }
 
@@ -184,72 +220,42 @@ public:
         ScanBuildingState();
     };
 
-    /**
-     * Return a CollectionScanNode that scans as requested in 'query'.
-     */
-    static std::unique_ptr<QuerySolutionNode> makeCollectionScan(const CanonicalQuery& query,
-                                                                 bool tailable,
-                                                                 const QueryPlannerParams& params);
-
-    /**
-     * Return a plan that uses the provided index as a proxy for a collection scan.
-     */
-    static QuerySolutionNode* scanWholeIndex(const IndexEntry& index,
-                                             const CanonicalQuery& query,
-                                             const QueryPlannerParams& params,
-                                             int direction = 1);
-
-    /**
-     * Return a plan that scans the provided index from [startKey to endKey).
-     */
-    static QuerySolutionNode* makeIndexScan(const IndexEntry& index,
-                                            const CanonicalQuery& query,
-                                            const QueryPlannerParams& params,
-                                            const BSONObj& startKey,
-                                            const BSONObj& endKey);
-
-    //
-    // Indexed Data Access methods.
+    // When recursively building data access, the caller may either continue to hold ownership of
+    // 'root', or may transfer ownership. When the caller holds ownership, it passes an owned
+    // pointer in 'root' and nullptr in 'ownedRoot'. When the caller transfers ownership, it passes
+    // owned and unowned pointers to the same match expression node in 'root' and 'ownedRoot'.
     //
-    // The inArrayOperator flag deserves some attention.  It is set when we're processing a
-    // child of an MatchExpression::ELEM_MATCH_OBJECT.
+    // The caller only holds ownership when planning inside an "array operator", e.g. when
+    // recursively performing access planning for an $elemMatch object. Therefore, whether or not
+    // 'ownedRoot' is null is also used as a check for whether we need to apply special logic for
+    // the "in array operator" case.
     //
-    // When true, the following behavior changes for all methods below that take it as an argument:
-    // 0. No deletion of MatchExpression(s).  In fact,
-    // 1. No mutation of the MatchExpression at all.  We need the tree as-is in order to perform
-    //    a filter on the entire tree.
-    // 2. No fetches performed.  There will be a final fetch by the caller of buildIndexedDataAccess
-    //    who set the value of inArrayOperator to true.
-    // 3. No compound indices are used and no bounds are combined.  These are
-    //    incorrect in the context of these operators.
-    //
-
-    /**
-     * If 'inArrayOperator' is false, takes ownership of 'root'.
-     */
-    static QuerySolutionNode* buildIndexedDataAccess(const CanonicalQuery& query,
-                                                     MatchExpression* root,
-                                                     bool inArrayOperator,
-                                                     const std::vector<IndexEntry>& indices,
-                                                     const QueryPlannerParams& params);
-
-    /**
-     * Takes ownership of 'root'.
-     */
-    static QuerySolutionNode* buildIndexedAnd(const CanonicalQuery& query,
-                                              MatchExpression* root,
-                                              bool inArrayOperator,
-                                              const std::vector<IndexEntry>& indices,
-                                              const QueryPlannerParams& params);
-
-    /**
-     * Takes ownership of 'root'.
-     */
-    static QuerySolutionNode* buildIndexedOr(const CanonicalQuery& query,
-                                             MatchExpression* root,
-                                             bool inArrayOperator,
-                                             const std::vector<IndexEntry>& indices,
-                                             const QueryPlannerParams& params);
+    // Specifically, for $elemMatch nodes, the entire $elemMatch filter must be attached to a FETCH
+    // node. This is why the caller retains ownership of the $elemMatch filter. However, the
+    // recursive call for children of the $elemMatch will also refrain from adding any FETCH stages.
+    // There will be a final FETCH node added to which the entire $elemMatch filter will be affixed.
+    static std::unique_ptr<QuerySolutionNode> _buildIndexedDataAccess(
+        const CanonicalQuery& query,
+        MatchExpression* root,
+        std::unique_ptr<MatchExpression> ownedRoot,
+        const std::vector<IndexEntry>& indices,
+        const QueryPlannerParams& params);
+
+    // See _buildIndexedDataAccess() for description of 'root' and 'ownedRoot'.
+    static std::unique_ptr<QuerySolutionNode> buildIndexedAnd(
+        const CanonicalQuery& query,
+        MatchExpression* root,
+        std::unique_ptr<MatchExpression> ownedRoot,
+        const std::vector<IndexEntry>& indices,
+        const QueryPlannerParams& params);
+
+    // See _buildIndexedDataAccess() for description of 'root' and 'ownedRoot'.
+    static std::unique_ptr<QuerySolutionNode> buildIndexedOr(
+        const CanonicalQuery& query,
+        MatchExpression* root,
+        std::unique_ptr<MatchExpression> ownedRoot,
+        const std::vector<IndexEntry>& indices,
+        const QueryPlannerParams& params);
 
     /**
      * Traverses the tree rooted at the $elemMatch expression 'node',
@@ -268,8 +274,8 @@ public:
 
     /**
      * Given a list of OR-related subtrees returned by processIndexScans(), looks for logically
-     * equivalent IndexScanNodes and combines them. This is an optimization to avoid creating
-     * plans that repeat index access work.
+     * equivalent IndexScanNodes and combines them. This is an optimization to avoid creating plans
+     * that repeat index access work.
      *
      * Example:
      *  Suppose processIndexScans() returns a list of the following three query solutions:
@@ -281,20 +287,16 @@ public:
      *    2) FETCH (filter: {$or:[{d:1}, {e:1}]}) -> IXSCAN (bounds: {c: [[3,3]]})
      *
      * Used as a helper for buildIndexedOr().
-     *
-     * Takes ownership of 'scans'. The caller assumes ownership of the pointers in the returned
-     * list of QuerySolutionNode*.
      */
-    static std::vector<QuerySolutionNode*> collapseEquivalentScans(
-        const std::vector<QuerySolutionNode*> scans);
+    static std::vector<std::unique_ptr<QuerySolutionNode>> collapseEquivalentScans(
+        std::vector<std::unique_ptr<QuerySolutionNode>> scans);
 
     /**
      * Helper used by buildIndexedAnd and buildIndexedOr.
      *
-     * The children of AND and OR nodes are sorted by the index that the subtree rooted at
-     * that node uses.  Child nodes that use the same index are adjacent to one another to
-     * facilitate grouping of index scans.  As such, the processing for AND and OR is
-     * almost identical.
+     * The children of AND and OR nodes are sorted by the index that the subtree rooted at that node
+     * uses.  Child nodes that use the same index are adjacent to one another to facilitate grouping
+     * of index scans.  As such, the processing for AND and OR is almost identical.
      *
      * Does not take ownership of 'root' but may remove children from it.
      */
@@ -303,19 +305,18 @@ public:
                                   bool inArrayOperator,
                                   const std::vector<IndexEntry>& indices,
                                   const QueryPlannerParams& params,
-                                  std::vector<QuerySolutionNode*>* out);
+                                  std::vector<std::unique_ptr<QuerySolutionNode>>* out);
 
     /**
-     * Used by processIndexScans(...) in order to recursively build a data access
-     * plan for a "subnode", a node in the MatchExpression tree which is indexed by
-     * virtue of its children.
+     * Used by processIndexScans(...) in order to recursively build a data access plan for a
+     * "subnode", a node in the MatchExpression tree which is indexed by virtue of its children.
      *
      * The resulting scans are outputted in the out-parameter 'out'.
      */
     static bool processIndexScansSubnode(const CanonicalQuery& query,
                                          ScanBuildingState* scanState,
                                          const QueryPlannerParams& params,
-                                         std::vector<QuerySolutionNode*>* out);
+                                         std::vector<std::unique_ptr<QuerySolutionNode>>* out);
 
     /**
      * Used by processIndexScansSubnode(...) to build the leaves of the solution tree for an
@@ -326,7 +327,7 @@ public:
     static bool processIndexScansElemMatch(const CanonicalQuery& query,
                                            ScanBuildingState* scanState,
                                            const QueryPlannerParams& params,
-                                           std::vector<QuerySolutionNode*>* out);
+                                           std::vector<std::unique_ptr<QuerySolutionNode>>* out);
 
     //
     // Helpers for creating an index scan.
@@ -335,17 +336,18 @@ public:
     /**
      * Create a new data access node.
      *
-     * If the node is an index scan, the bounds for 'expr' are computed and placed into the
-     * first field's OIL position.  The rest of the OILs are allocated but uninitialized.
+     * If the node is an index scan, the bounds for 'expr' are computed and placed into the first
+     * field's OIL position.  The rest of the OILs are allocated but uninitialized.
      *
-     * If the node is a geo node, grab the geo data from 'expr' and stuff it into the
-     * geo solution node of the appropriate type.
+     * If the node is a geo node, grab the geo data from 'expr' and stuff it into the geo solution
+     * node of the appropriate type.
      */
-    static QuerySolutionNode* makeLeafNode(const CanonicalQuery& query,
-                                           const IndexEntry& index,
-                                           size_t pos,
-                                           MatchExpression* expr,
-                                           IndexBoundsBuilder::BoundsTightness* tightnessOut);
+    static std::unique_ptr<QuerySolutionNode> makeLeafNode(
+        const CanonicalQuery& query,
+        const IndexEntry& index,
+        size_t pos,
+        const MatchExpression* expr,
+        IndexBoundsBuilder::BoundsTightness* tightnessOut);
 
     /**
      * Merge the predicate 'expr' with the leaf node 'node'.
@@ -375,11 +377,11 @@ public:
      * 'scanState'. The resulting scan is outputted in the out-parameter 'out', transferring
      * ownership in the process.
      *
-     * If 'scanState' is building an index scan for OR-related predicates, filters
-     * may be affixed to the scan as necessary.
+     * If 'scanState' is building an index scan for OR-related predicates, filters may be affixed to
+     * the scan as necessary.
      */
     static void finishAndOutputLeaf(ScanBuildingState* scanState,
-                                    std::vector<QuerySolutionNode*>* out);
+                                    std::vector<std::unique_ptr<QuerySolutionNode>>* out);
 
     /**
      * Returns true if the current scan in 'scanState' requires a FetchNode.
diff --git a/src/mongo/db/query/query_planner.cpp b/src/mongo/db/query/query_planner.cpp
index f49584b6b54..9de0aec1c73 100644
--- a/src/mongo/db/query/query_planner.cpp
+++ b/src/mongo/db/query/query_planner.cpp
@@ -294,13 +294,9 @@ bool providesSort(const CanonicalQuery& query, const BSONObj& kp) {
 // static
 const int QueryPlanner::kPlannerVersion = 1;
 
-Status QueryPlanner::cacheDataFromTaggedTree(const MatchExpression* const taggedTree,
-                                             const vector<IndexEntry>& relevantIndices,
-                                             PlanCacheIndexTree** out) {
-    // On any early return, the out-parameter must contain NULL.
-    *out = NULL;
-
-    if (NULL == taggedTree) {
+StatusWith<unique_ptr<PlanCacheIndexTree>> QueryPlanner::cacheDataFromTaggedTree(
+    const MatchExpression* const taggedTree, const vector<IndexEntry>& relevantIndices) {
+    if (!taggedTree) {
         return Status(ErrorCodes::BadValue, "Cannot produce cache data: tree is NULL.");
     }
 
@@ -361,16 +357,14 @@ Status QueryPlanner::cacheDataFromTaggedTree(const MatchExpression* const tagged
 
     for (size_t i = 0; i < taggedTree->numChildren(); ++i) {
         MatchExpression* taggedChild = taggedTree->getChild(i);
-        PlanCacheIndexTree* indexTreeChild;
-        Status s = cacheDataFromTaggedTree(taggedChild, relevantIndices, &indexTreeChild);
-        if (!s.isOK()) {
-            return s;
+        auto swIndexTreeChild = cacheDataFromTaggedTree(taggedChild, relevantIndices);
+        if (!swIndexTreeChild.isOK()) {
+            return swIndexTreeChild;
         }
-        indexTree->children.push_back(indexTreeChild);
+        indexTree->children.push_back(swIndexTreeChild.getValue().release());
     }
 
-    *out = indexTree.release();
-    return Status::OK();
+    return {std::move(indexTree)};
 }
 
 // static
@@ -513,7 +507,7 @@ Status QueryPlanner::planFromCache(const CanonicalQuery& query,
 
     // Use the cached index assignments to build solnRoot.
     std::unique_ptr<QuerySolutionNode> solnRoot(QueryPlannerAccess::buildIndexedDataAccess(
-        query, clone.release(), false, params.indices, params));
+        query, std::move(clone), params.indices, params));
 
     if (!solnRoot) {
         return Status(ErrorCodes::BadValue,
@@ -873,30 +867,31 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
         PlanEnumerator isp(enumParams);
         isp.init().transitional_ignore();
 
-        unique_ptr<MatchExpression> rawTree;
-        while ((rawTree = isp.getNext()) && (out->size() < params.maxIndexedSolutions)) {
+        unique_ptr<MatchExpression> nextTaggedTree;
+        while ((nextTaggedTree = isp.getNext()) && (out->size() < params.maxIndexedSolutions)) {
             LOG(5) << "About to build solntree from tagged tree:" << endl
-                   << redact(rawTree.get()->toString());
+                   << redact(nextTaggedTree->toString());
 
             // Store the plan cache index tree before calling prepareForAccessingPlanning(), so that
             // the PlanCacheIndexTree has the same sort as the MatchExpression used to generate the
             // plan cache key.
-            std::unique_ptr<MatchExpression> clone(rawTree.get()->shallowClone());
-            PlanCacheIndexTree* cacheData;
-            Status indexTreeStatus =
-                cacheDataFromTaggedTree(clone.get(), relevantIndices, &cacheData);
-            if (!indexTreeStatus.isOK()) {
-                LOG(5) << "Query is not cachable: " << redact(indexTreeStatus.reason());
+            std::unique_ptr<MatchExpression> clone(nextTaggedTree->shallowClone());
+            std::unique_ptr<PlanCacheIndexTree> cacheData;
+            auto statusWithCacheData = cacheDataFromTaggedTree(clone.get(), relevantIndices);
+            if (!statusWithCacheData.isOK()) {
+                LOG(5) << "Query is not cachable: "
+                       << redact(statusWithCacheData.getStatus().reason());
+            } else {
+                cacheData = std::move(statusWithCacheData.getValue());
             }
-            unique_ptr<PlanCacheIndexTree> autoData(cacheData);
 
             // We have already cached the tree in canonical order, so now we can order the nodes for
             // access planning.
-            prepareForAccessPlanning(rawTree.get());
+            prepareForAccessPlanning(nextTaggedTree.get());
 
             // This can fail if enumeration makes a mistake.
             std::unique_ptr<QuerySolutionNode> solnRoot(QueryPlannerAccess::buildIndexedDataAccess(
-                query, rawTree.release(), false, relevantIndices, params));
+                query, std::move(nextTaggedTree), relevantIndices, params));
 
             if (!solnRoot) {
                 continue;
@@ -904,11 +899,11 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
 
             QuerySolution* soln =
                 QueryPlannerAnalysis::analyzeDataAccess(query, params, std::move(solnRoot));
-            if (NULL != soln) {
+            if (soln) {
                 LOG(5) << "Planner: adding solution:" << endl << redact(soln->toString());
-                if (indexTreeStatus.isOK()) {
+                if (statusWithCacheData.isOK()) {
                     SolutionCacheData* scd = new SolutionCacheData();
-                    scd->tree.reset(autoData.release());
+                    scd->tree.reset(cacheData.release());
                     soln->cacheData.reset(scd);
                 }
                 out->push_back(soln);
diff --git a/src/mongo/db/query/query_planner.h b/src/mongo/db/query/query_planner.h
index 33c799242d5..5cefd99a206 100644
--- a/src/mongo/db/query/query_planner.h
+++ b/src/mongo/db/query/query_planner.h
@@ -86,14 +86,10 @@ public:
      * @param relevantIndices -- a list of the index entries used to tag
      *   the tree (i.e. index numbers in the tags refer to entries in this vector)
      *
-     * On success, a new tagged tree is returned through the out-parameter 'out'.
-     * The caller has ownership of both taggedTree and *out.
-     *
-     * On failure, 'out' is set to NULL.
+     * On success, a new tagged tree is returned.
      */
-    static Status cacheDataFromTaggedTree(const MatchExpression* const taggedTree,
-                                          const std::vector<IndexEntry>& relevantIndices,
-                                          PlanCacheIndexTree** out);
+    static StatusWith<std::unique_ptr<PlanCacheIndexTree>> cacheDataFromTaggedTree(
+        const MatchExpression* const taggedTree, const std::vector<IndexEntry>& relevantIndices);
 
     /**
      * @param filter -- an untagged MatchExpression
diff --git a/src/mongo/db/query/query_planner_test.cpp b/src/mongo/db/query/query_planner_test.cpp
index 2ef835eaf8f..b9a79204a81 100644
--- a/src/mongo/db/query/query_planner_test.cpp
+++ b/src/mongo/db/query/query_planner_test.cpp
@@ -4364,13 +4364,10 @@ TEST_F(QueryPlannerTest, KeyPatternOverflowsInt) {
 //
 
 TEST_F(QueryPlannerTest, CacheDataFromTaggedTreeFailsOnBadInput) {
-    PlanCacheIndexTree* indexTree;
-
     // Null match expression.
     std::vector<IndexEntry> relevantIndices;
-    Status s = QueryPlanner::cacheDataFromTaggedTree(NULL, relevantIndices, &indexTree);
-    ASSERT_NOT_OK(s);
-    ASSERT(NULL == indexTree);
+    auto swIndexTree = QueryPlanner::cacheDataFromTaggedTree(NULL, relevantIndices);
+    ASSERT_NOT_OK(swIndexTree.getStatus());
 
     // No relevant index matching the index tag.
     relevantIndices.push_back(IndexEntry(BSON("a" << 1)));
@@ -4382,9 +4379,8 @@ TEST_F(QueryPlannerTest, CacheDataFromTaggedTreeFailsOnBadInput) {
     std::unique_ptr<CanonicalQuery> scopedCq = std::move(statusWithCQ.getValue());
     scopedCq->root()->setTag(new IndexTag(1));
 
-    s = QueryPlanner::cacheDataFromTaggedTree(scopedCq->root(), relevantIndices, &indexTree);
-    ASSERT_NOT_OK(s);
-    ASSERT(NULL == indexTree);
+    swIndexTree = QueryPlanner::cacheDataFromTaggedTree(scopedCq->root(), relevantIndices);
+    ASSERT_NOT_OK(swIndexTree.getStatus());
 }
 
 TEST_F(QueryPlannerTest, TagAccordingToCacheFailsOnBadInput) {
diff --git a/src/mongo/db/query/query_solution.h b/src/mongo/db/query/query_solution.h
index 7319cc6502e..79cb260cccc 100644
--- a/src/mongo/db/query/query_solution.h
+++ b/src/mongo/db/query/query_solution.h
@@ -146,7 +146,23 @@ struct QuerySolutionNode {
         }
     }
 
+    /**
+     * 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(); });
+    }
+
     // These are owned here.
+    //
+    // TODO SERVER-35512: Make this a vector of unique_ptr.
     std::vector<QuerySolutionNode*> children;
 
     // If a stage has a non-NULL filter all values outputted from that stage must pass that