SERVER-24320 PlanCacheIndexTree should have same sort order as MatchExpression used to generate plan cache key

4 files changed, 24 insertions, 6 deletions

diff --git a/src/mongo/db/query/plan_cache_test.cpp b/src/mongo/db/query/plan_cache_test.cpp
index 510cfd5064b..46a73563f62 100644
--- a/src/mongo/db/query/plan_cache_test.cpp
+++ b/src/mongo/db/query/plan_cache_test.cpp
@@ -786,6 +786,22 @@ TEST_F(CachePlanSelectionTest, Or2DSphereNonNear) {
         "{fetch: {node: {ixscan: {pattern: {b: '2dsphere'}}}}}]}}");
 }
 
+// Regression test for SERVER-24320. Tests that the PlanCacheIndexTree has the same sort order as
+// the MatchExpression used to generate the plan cache key.
+TEST_F(CachePlanSelectionTest, AndWithinPolygonWithinCenterSphere) {
+    addIndex(BSON("a"
+                  << "2dsphere"
+                  << "b" << 1));
+
+    BSONObj query = fromjson(
+        "{$and: [{b: 1}, {a: {$within: {$polygon: [[0, 0], [0, 0], [0, 0], [0, 0]]}}}, {a: "
+        "{$within: {$centerSphere: [[0, 0], 0]}}}]}");
+
+    runQuery(query);
+    assertPlanCacheRecoversSolution(query,
+                                    "{fetch: {node: {ixscan: {pattern: {a: '2dsphere', b: 1}}}}}");
+}
+
 //
 // tree operations
 //
diff --git a/src/mongo/db/query/plan_enumerator.cpp b/src/mongo/db/query/plan_enumerator.cpp
index c84615a7eec..8ea8d8a82e8 100644
--- a/src/mongo/db/query/plan_enumerator.cpp
+++ b/src/mongo/db/query/plan_enumerator.cpp
@@ -177,7 +177,6 @@ bool PlanEnumerator::getNext(MatchExpression** tree) {
 
     *tree = _root->shallowClone().release();
     tagForSort(*tree);
-    sortUsingTags(*tree);
 
     _root->resetTag();
     LOG(5) << "Enumerator: memo just before moving:" << endl
diff --git a/src/mongo/db/query/plan_enumerator.h b/src/mongo/db/query/plan_enumerator.h
index f12dde08897..2cc8813feee 100644
--- a/src/mongo/db/query/plan_enumerator.h
+++ b/src/mongo/db/query/plan_enumerator.h
@@ -101,6 +101,9 @@ public:
      *
      * Nodes in 'tree' are tagged with indices that should be used to answer the tagged nodes.
      * Only nodes that have a field name (isLogical() == false) will be tagged.
+     *
+     * The output tree is a clone identical to that used to initialize the enumerator, with tags
+     * added in order to indicate index usage.
      */
     bool getNext(MatchExpression** tree);
 
diff --git a/src/mongo/db/query/query_planner.cpp b/src/mongo/db/query/query_planner.cpp
index 85604514c5b..cb44bc12908 100644
--- a/src/mongo/db/query/query_planner.cpp
+++ b/src/mongo/db/query/query_planner.cpp
@@ -745,12 +745,10 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
             LOG(5) << "About to build solntree from tagged tree:" << endl
                    << rawTree->toString();
 
-            // The tagged tree produced by the plan enumerator is not guaranteed
-            // to be canonically sorted. In order to be compatible with the cached
-            // data, sort the tagged tree according to CanonicalQuery ordering.
+            // Store the plan cache index tree before sorting using index tags, so that the
+            // PlanCacheIndexTree has the same sort as the MatchExpression used to generate the plan
+            // cache key.
             std::unique_ptr<MatchExpression> clone(rawTree->shallowClone());
-            CanonicalQuery::sortTree(clone.get());
-
             PlanCacheIndexTree* cacheData;
             Status indexTreeStatus =
                 cacheDataFromTaggedTree(clone.get(), relevantIndices, &cacheData);
@@ -759,6 +757,8 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
             }
             unique_ptr<PlanCacheIndexTree> autoData(cacheData);
 
+            sortUsingTags(rawTree);
+
             // This can fail if enumeration makes a mistake.
             QuerySolutionNode* solnRoot = QueryPlannerAccess::buildIndexedDataAccess(
                 query, rawTree, false, relevantIndices, params);