SERVER-42649 Use Value instead of BSONObj in the in-memory sort key

6 files changed, 84 insertions, 99 deletions

diff --git a/src/mongo/dbtests/cursor_manager_test.cpp b/src/mongo/dbtests/cursor_manager_test.cpp
index 72934d8348a..90def084bba 100644
--- a/src/mongo/dbtests/cursor_manager_test.cpp
+++ b/src/mongo/dbtests/cursor_manager_test.cpp
@@ -76,8 +76,9 @@ public:
         return unittest::assertGet(PlanExecutor::make(opCtx,
                                                       std::move(workingSet),
                                                       std::move(queuedDataStage),
-                                                      kTestNss,
-                                                      PlanExecutor::YieldPolicy::NO_YIELD));
+                                                      nullptr,
+                                                      PlanExecutor::YieldPolicy::NO_YIELD,
+                                                      kTestNss));
     }
 
     ClientCursorParams makeParams(OperationContext* opCtx) {
diff --git a/src/mongo/dbtests/documentsourcetests.cpp b/src/mongo/dbtests/documentsourcetests.cpp
index 8963c988bfd..2165ed6695a 100644
--- a/src/mongo/dbtests/documentsourcetests.cpp
+++ b/src/mongo/dbtests/documentsourcetests.cpp
@@ -343,10 +343,9 @@ TEST_F(DocumentSourceCursorTest, TailableAwaitDataCursorShouldErrorAfterTimeout)
     auto canonicalQuery = unittest::assertGet(
         CanonicalQuery::canonicalize(opCtx(), std::move(queryRequest), nullptr));
     auto planExecutor =
-        uassertStatusOK(PlanExecutor::make(opCtx(),
+        uassertStatusOK(PlanExecutor::make(std::move(canonicalQuery),
                                            std::move(workingSet),
                                            std::move(collectionScan),
-                                           std::move(canonicalQuery),
                                            readLock.getCollection(),
                                            PlanExecutor::YieldPolicy::ALWAYS_TIME_OUT));
 
@@ -380,10 +379,9 @@ TEST_F(DocumentSourceCursorTest, NonAwaitDataCursorShouldErrorAfterTimeout) {
     auto canonicalQuery = unittest::assertGet(
         CanonicalQuery::canonicalize(opCtx(), std::move(queryRequest), nullptr));
     auto planExecutor =
-        uassertStatusOK(PlanExecutor::make(opCtx(),
+        uassertStatusOK(PlanExecutor::make(std::move(canonicalQuery),
                                            std::move(workingSet),
                                            std::move(collectionScan),
-                                           std::move(canonicalQuery),
                                            readLock.getCollection(),
                                            PlanExecutor::YieldPolicy::ALWAYS_TIME_OUT));
 
@@ -426,10 +424,9 @@ TEST_F(DocumentSourceCursorTest, TailableAwaitDataCursorShouldErrorAfterBeingKil
     auto canonicalQuery = unittest::assertGet(
         CanonicalQuery::canonicalize(opCtx(), std::move(queryRequest), nullptr));
     auto planExecutor =
-        uassertStatusOK(PlanExecutor::make(opCtx(),
+        uassertStatusOK(PlanExecutor::make(std::move(canonicalQuery),
                                            std::move(workingSet),
                                            std::move(collectionScan),
-                                           std::move(canonicalQuery),
                                            readLock.getCollection(),
                                            PlanExecutor::YieldPolicy::ALWAYS_MARK_KILLED));
 
@@ -462,10 +459,9 @@ TEST_F(DocumentSourceCursorTest, NormalCursorShouldErrorAfterBeingKilled) {
     auto canonicalQuery = unittest::assertGet(
         CanonicalQuery::canonicalize(opCtx(), std::move(queryRequest), nullptr));
     auto planExecutor =
-        uassertStatusOK(PlanExecutor::make(opCtx(),
+        uassertStatusOK(PlanExecutor::make(std::move(canonicalQuery),
                                            std::move(workingSet),
                                            std::move(collectionScan),
-                                           std::move(canonicalQuery),
                                            readLock.getCollection(),
                                            PlanExecutor::YieldPolicy::ALWAYS_MARK_KILLED));
 
diff --git a/src/mongo/dbtests/plan_executor_invalidation_test.cpp b/src/mongo/dbtests/plan_executor_invalidation_test.cpp
index 61eedeab72f..590d518feae 100644
--- a/src/mongo/dbtests/plan_executor_invalidation_test.cpp
+++ b/src/mongo/dbtests/plan_executor_invalidation_test.cpp
@@ -85,10 +85,9 @@ public:
         std::unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue());
 
         // Takes ownership of 'ws', 'scan', and 'cq'.
-        auto statusWithPlanExecutor = PlanExecutor::make(&_opCtx,
+        auto statusWithPlanExecutor = PlanExecutor::make(std::move(cq),
                                                          std::move(ws),
                                                          std::move(scan),
-                                                         std::move(cq),
                                                          _ctx->db()->getCollection(&_opCtx, nss),
                                                          PlanExecutor::YIELD_MANUAL);
         ASSERT_OK(statusWithPlanExecutor.getStatus());
diff --git a/src/mongo/dbtests/query_plan_executor.cpp b/src/mongo/dbtests/query_plan_executor.cpp
index 82f6251867a..be1b3448555 100644
--- a/src/mongo/dbtests/query_plan_executor.cpp
+++ b/src/mongo/dbtests/query_plan_executor.cpp
@@ -119,8 +119,8 @@ public:
             new CollectionScan(&_opCtx, coll, csparams, ws.get(), cq.get()->root()));
 
         // Hand the plan off to the executor.
-        auto statusWithPlanExecutor = PlanExecutor::make(
-            &_opCtx, std::move(ws), std::move(root), std::move(cq), coll, yieldPolicy);
+        auto statusWithPlanExecutor =
+            PlanExecutor::make(std::move(cq), std::move(ws), std::move(root), coll, yieldPolicy);
         ASSERT_OK(statusWithPlanExecutor.getStatus());
         return std::move(statusWithPlanExecutor.getValue());
     }
@@ -162,12 +162,8 @@ public:
         verify(nullptr != cq.get());
 
         // Hand the plan off to the executor.
-        auto statusWithPlanExecutor = PlanExecutor::make(&_opCtx,
-                                                         std::move(ws),
-                                                         std::move(root),
-                                                         std::move(cq),
-                                                         coll,
-                                                         PlanExecutor::YIELD_MANUAL);
+        auto statusWithPlanExecutor = PlanExecutor::make(
+            std::move(cq), std::move(ws), std::move(root), coll, PlanExecutor::YIELD_MANUAL);
         ASSERT_OK(statusWithPlanExecutor.getStatus());
         return std::move(statusWithPlanExecutor.getValue());
     }
diff --git a/src/mongo/dbtests/query_stage_multiplan.cpp b/src/mongo/dbtests/query_stage_multiplan.cpp
index c08ca0abad0..877b4c2213c 100644
--- a/src/mongo/dbtests/query_stage_multiplan.cpp
+++ b/src/mongo/dbtests/query_stage_multiplan.cpp
@@ -253,12 +253,8 @@ TEST_F(QueryStageMultiPlanTest, MPSCollectionScanVsHighlySelectiveIXScan) {
     ASSERT_EQUALS(0, mps->bestPlanIdx());
 
     // Takes ownership of arguments other than 'collection'.
-    auto statusWithPlanExecutor = PlanExecutor::make(_opCtx.get(),
-                                                     std::move(sharedWs),
-                                                     std::move(mps),
-                                                     std::move(cq),
-                                                     coll,
-                                                     PlanExecutor::NO_YIELD);
+    auto statusWithPlanExecutor = PlanExecutor::make(
+        std::move(cq), std::move(sharedWs), std::move(mps), coll, PlanExecutor::NO_YIELD);
     ASSERT_OK(statusWithPlanExecutor.getStatus());
     auto exec = std::move(statusWithPlanExecutor.getValue());
 
diff --git a/src/mongo/dbtests/query_stage_sort_key_generator.cpp b/src/mongo/dbtests/query_stage_sort_key_generator.cpp
index 74aab27fc88..6ee9b1ba86c 100644
--- a/src/mongo/dbtests/query_stage_sort_key_generator.cpp
+++ b/src/mongo/dbtests/query_stage_sort_key_generator.cpp
@@ -34,6 +34,7 @@
 #include "mongo/db/exec/queued_data_stage.h"
 #include "mongo/db/exec/sort_key_generator.h"
 #include "mongo/db/json.h"
+#include "mongo/db/pipeline/document_value_test_util.h"
 #include "mongo/db/query/collation/collator_interface_mock.h"
 #include "mongo/db/query/query_test_service_context.h"
 #include "mongo/unittest/unittest.h"
@@ -42,7 +43,7 @@ namespace mongo {
 
 namespace {
 
-BSONObj extractKeyFromKeyGenStage(SortKeyGeneratorStage* sortKeyGen, WorkingSet* workingSet) {
+Value extractKeyFromKeyGenStage(SortKeyGeneratorStage* sortKeyGen, WorkingSet* workingSet) {
     WorkingSetID wsid;
     PlanStage::StageState state = PlanStage::NEED_TIME;
     while (state == PlanStage::NEED_TIME) {
@@ -62,7 +63,7 @@ BSONObj extractKeyFromKeyGenStage(SortKeyGeneratorStage* sortKeyGen, WorkingSet*
  * The 'collator' is used to specify the string comparison semantics that should be used when
  * generating the sort key.
  */
-BSONObj extractSortKey(const char* sortSpec, const char* doc, const CollatorInterface* collator) {
+Value extractSortKey(const char* sortSpec, const char* doc, const CollatorInterface* collator) {
     QueryTestServiceContext serviceContext;
     auto opCtx = serviceContext.makeOperationContext();
     boost::intrusive_ptr<ExpressionContext> pExpCtx(new ExpressionContext(opCtx.get(), collator));
@@ -89,9 +90,9 @@ BSONObj extractSortKey(const char* sortSpec, const char* doc, const CollatorInte
  * The 'collator' is used to specify the string comparison semantics that should be used when
  * generating the sort key.
  */
-BSONObj extractSortKeyCovered(const char* sortSpec,
-                              const IndexKeyDatum& ikd,
-                              const CollatorInterface* collator) {
+Value extractSortKeyCovered(const char* sortSpec,
+                            const IndexKeyDatum& ikd,
+                            const CollatorInterface* collator) {
     QueryTestServiceContext serviceContext;
     auto opCtx = serviceContext.makeOperationContext();
     boost::intrusive_ptr<ExpressionContext> pExpCtx(new ExpressionContext(opCtx.get(), collator));
@@ -112,150 +113,146 @@ BSONObj extractSortKeyCovered(const char* sortSpec,
 }
 
 TEST(SortKeyGeneratorStageTest, SortKeyNormal) {
-    BSONObj actualOut = extractSortKey("{a: 1}", "{_id: 0, a: 5}", nullptr);
-    BSONObj expectedOut = BSON("" << 5);
-    ASSERT_BSONOBJ_EQ(actualOut, expectedOut);
+    Value actualOut = extractSortKey("{a: 1}", "{_id: 0, a: 5}", nullptr);
+    Value expectedOut(5);
+    ASSERT_VALUE_EQ(actualOut, expectedOut);
 }
 
 TEST(SortKeyGeneratorStageTest, SortKeyNormal2) {
-    BSONObj actualOut = extractSortKey("{a: 1}", "{_id: 0, z: 10, a: 6, b: 16}", nullptr);
-    BSONObj expectedOut = BSON("" << 6);
-    ASSERT_BSONOBJ_EQ(actualOut, expectedOut);
+    Value actualOut = extractSortKey("{a: 1}", "{_id: 0, z: 10, a: 6, b: 16}", nullptr);
+    Value expectedOut(6);
+    ASSERT_VALUE_EQ(actualOut, expectedOut);
 }
 
 TEST(SortKeyGeneratorStageTest, SortKeyString) {
-    BSONObj actualOut =
+    Value actualOut =
         extractSortKey("{a: 1}", "{_id: 0, z: 'thing1', a: 'thing2', b: 16}", nullptr);
-    BSONObj expectedOut = BSON(""
-                               << "thing2");
-    ASSERT_BSONOBJ_EQ(actualOut, expectedOut);
+    Value expectedOut("thing2"_sd);
+    ASSERT_VALUE_EQ(actualOut, expectedOut);
 }
 
 TEST(SortKeyGeneratorStageTest, SortKeyCompound) {
-    BSONObj actualOut =
+    Value actualOut =
         extractSortKey("{a: 1, b: 1}", "{_id: 0, z: 'thing1', a: 99, c: {a: 4}, b: 16}", nullptr);
-    BSONObj expectedOut = BSON("" << 99 << "" << 16);
-    ASSERT_BSONOBJ_EQ(actualOut, expectedOut);
+    Value expectedOut({Value(99), Value(16)});
+    ASSERT_VALUE_EQ(actualOut, expectedOut);
 }
 
 TEST(SortKeyGeneratorStageTest, SortKeyEmbedded) {
-    BSONObj actualOut = extractSortKey(
+    Value actualOut = extractSortKey(
         "{'c.a': 1, b: 1}", "{_id: 0, z: 'thing1', a: 99, c: {a: 4}, b: 16}", nullptr);
-    BSONObj expectedOut = BSON("" << 4 << "" << 16);
-    ASSERT_BSONOBJ_EQ(actualOut, expectedOut);
+    Value expectedOut = Value({Value(4), Value(16)});
+    ASSERT_VALUE_EQ(actualOut, expectedOut);
 }
 
 TEST(SortKeyGeneratorStageTest, SortKeyArray) {
-    BSONObj actualOut = extractSortKey(
+    Value actualOut = extractSortKey(
         "{'c': 1, b: 1}", "{_id: 0, z: 'thing1', a: 99, c: [2, 4, 1], b: 16}", nullptr);
-    BSONObj expectedOut = BSON("" << 1 << "" << 16);
-    ASSERT_BSONOBJ_EQ(actualOut, expectedOut);
+    Value expectedOut({Value(1), Value(16)});
+    ASSERT_VALUE_EQ(actualOut, expectedOut);
 }
 
 TEST(SortKeyGeneratorStageTest, SortKeyCoveredNormal) {
     CollatorInterface* collator = nullptr;
-    BSONObj actualOut = extractSortKeyCovered(
+    Value actualOut = extractSortKeyCovered(
         "{a: 1}", IndexKeyDatum(BSON("a" << 1), BSON("" << 5), 0, SnapshotId{}), collator);
-    BSONObj expectedOut = BSON("" << 5);
-    ASSERT_BSONOBJ_EQ(actualOut, expectedOut);
+    Value expectedOut({Value(5)});
+    ASSERT_VALUE_EQ(actualOut, expectedOut);
 }
 
 TEST(SortKeyGeneratorStageTest, SortKeyCoveredEmbedded) {
     CollatorInterface* collator = nullptr;
-    BSONObj actualOut = extractSortKeyCovered(
+    Value actualOut = extractSortKeyCovered(
         "{'a.c': 1}",
         IndexKeyDatum(BSON("a.c" << 1 << "c" << 1), BSON("" << 5 << "" << 6), 0, SnapshotId{}),
         collator);
-    BSONObj expectedOut = BSON("" << 5);
-    ASSERT_BSONOBJ_EQ(actualOut, expectedOut);
+    Value expectedOut(5);
+    ASSERT_VALUE_EQ(actualOut, expectedOut);
 }
 
 TEST(SortKeyGeneratorStageTest, SortKeyCoveredCompound) {
     CollatorInterface* collator = nullptr;
-    BSONObj actualOut = extractSortKeyCovered(
+    Value actualOut = extractSortKeyCovered(
         "{a: 1, c: 1}",
         IndexKeyDatum(BSON("a" << 1 << "c" << 1), BSON("" << 5 << "" << 6), 0, SnapshotId{}),
         collator);
-    BSONObj expectedOut = BSON("" << 5 << "" << 6);
-    ASSERT_BSONOBJ_EQ(actualOut, expectedOut);
+    Value expectedOut({Value(5), Value(6)});
+    ASSERT_VALUE_EQ(actualOut, expectedOut);
 }
 
 TEST(SortKeyGeneratorStageTest, SortKeyCoveredCompound2) {
     CollatorInterface* collator = nullptr;
-    BSONObj actualOut = extractSortKeyCovered("{a: 1, b: 1}",
-                                              IndexKeyDatum(BSON("a" << 1 << "b" << 1 << "c" << 1),
-                                                            BSON("" << 5 << "" << 6 << "" << 4),
-                                                            0,
-                                                            SnapshotId{}),
-                                              collator);
-    BSONObj expectedOut = BSON("" << 5 << "" << 6);
-    ASSERT_BSONOBJ_EQ(actualOut, expectedOut);
+    Value actualOut = extractSortKeyCovered("{a: 1, b: 1}",
+                                            IndexKeyDatum(BSON("a" << 1 << "b" << 1 << "c" << 1),
+                                                          BSON("" << 5 << "" << 6 << "" << 4),
+                                                          0,
+                                                          SnapshotId{}),
+                                            collator);
+    Value expectedOut({Value(5), Value(6)});
+    ASSERT_VALUE_EQ(actualOut, expectedOut);
 }
 
 TEST(SortKeyGeneratorStageTest, SortKeyCoveredCompound3) {
     CollatorInterface* collator = nullptr;
-    BSONObj actualOut =
+    Value actualOut =
         extractSortKeyCovered("{b: 1, c: 1}",
                               IndexKeyDatum(BSON("a" << 1 << "b" << 1 << "c" << 1 << "d" << 1),
                                             BSON("" << 5 << "" << 6 << "" << 4 << "" << 9000),
                                             0,
                                             SnapshotId{}),
                               collator);
-    BSONObj expectedOut = BSON("" << 6 << "" << 4);
-    ASSERT_BSONOBJ_EQ(actualOut, expectedOut);
+    Value expectedOut({Value(6), Value(4)});
+    ASSERT_VALUE_EQ(actualOut, expectedOut);
 }
 
 TEST(SortKeyGeneratorStageTest, ExtractStringSortKeyWithCollatorUsesComparisonKey) {
     CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
-    BSONObj actualOut =
+    Value actualOut =
         extractSortKey("{a: 1}", "{_id: 0, z: 'thing1', a: 'thing2', b: 16}", &collator);
-    BSONObj expectedOut = BSON(""
-                               << "2gniht");
-    ASSERT_BSONOBJ_EQ(actualOut, expectedOut);
+    Value expectedOut = Value("2gniht"_sd);
+    ASSERT_VALUE_EQ(actualOut, expectedOut);
 }
 
 TEST(SortKeyGeneratorStageTest, CollatorHasNoEffectWhenExtractingNonStringSortKey) {
     CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
-    BSONObj actualOut = extractSortKey("{a: 1}", "{_id: 0, z: 10, a: 6, b: 16}", &collator);
-    BSONObj expectedOut = BSON("" << 6);
-    ASSERT_BSONOBJ_EQ(actualOut, expectedOut);
+    Value actualOut = extractSortKey("{a: 1}", "{_id: 0, z: 10, a: 6, b: 16}", &collator);
+    Value expectedOut = Value(6);
+    ASSERT_VALUE_EQ(actualOut, expectedOut);
 }
 
 TEST(SortKeyGeneratorStageTest, CollatorAppliesWhenExtractingCoveredSortKeyString) {
     CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
-    BSONObj actualOut = extractSortKeyCovered("{b: 1}",
-                                              IndexKeyDatum(BSON("a" << 1 << "b" << 1),
-                                                            BSON("" << 4 << ""
-                                                                    << "foo"),
-                                                            0,
-                                                            SnapshotId{}),
-                                              &collator);
-    BSONObj expectedOut = BSON(""
-                               << "oof");
-    ASSERT_BSONOBJ_EQ(actualOut, expectedOut);
+    Value actualOut = extractSortKeyCovered("{b: 1}",
+                                            IndexKeyDatum(BSON("a" << 1 << "b" << 1),
+                                                          BSON("" << 4 << ""
+                                                                  << "foo"),
+                                                          0,
+                                                          SnapshotId{}),
+                                            &collator);
+    Value expectedOut = Value("oof"_sd);
+    ASSERT_VALUE_EQ(actualOut, expectedOut);
 }
 
 TEST(SortKeyGeneratorStageTest, SortKeyGenerationForArraysChoosesCorrectKey) {
-    BSONObj actualOut = extractSortKey("{a: -1}", "{_id: 0, a: [1, 2, 3, 4]}", nullptr);
-    BSONObj expectedOut = BSON("" << 4);
-    ASSERT_BSONOBJ_EQ(actualOut, expectedOut);
+    Value actualOut = extractSortKey("{a: -1}", "{_id: 0, a: [1, 2, 3, 4]}", nullptr);
+    Value expectedOut(4);
+    ASSERT_VALUE_EQ(actualOut, expectedOut);
 }
 
 TEST(SortKeyGeneratorStageTest, EnsureSortKeyGenerationForArraysRespectsCollation) {
     CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString);
-    BSONObj actualOut =
+    Value actualOut =
         extractSortKey("{a: 1}", "{_id: 0, a: ['aaz', 'zza', 'yya', 'zzb']}", &collator);
-    BSONObj expectedOut = BSON(""
-                               << "ayy");
-    ASSERT_BSONOBJ_EQ(actualOut, expectedOut);
+    Value expectedOut("ayy"_sd);
+    ASSERT_VALUE_EQ(actualOut, expectedOut);
 }
 
 TEST(SortKeyGeneratorStageTest, SortKeyGenerationForArraysRespectsCompoundOrdering) {
-    BSONObj actualOut = extractSortKey("{'a.b': 1, 'a.c': -1}",
-                                       "{_id: 0, a: [{b: 1, c: 0}, {b: 0, c: 3}, {b: 0, c: 1}]}",
-                                       nullptr);
-    BSONObj expectedOut = BSON("" << 0 << "" << 3);
-    ASSERT_BSONOBJ_EQ(actualOut, expectedOut);
+    Value actualOut = extractSortKey("{'a.b': 1, 'a.c': -1}",
+                                     "{_id: 0, a: [{b: 1, c: 0}, {b: 0, c: 3}, {b: 0, c: 1}]}",
+                                     nullptr);
+    Value expectedOut({Value(0), Value(3)});
+    ASSERT_VALUE_EQ(actualOut, expectedOut);
 }
 
 }  // namespace