SERVER-23924 Make _id index inherit the collection's default collation

7 files changed, 45 insertions, 20 deletions

diff --git a/src/mongo/db/catalog/index_catalog.cpp b/src/mongo/db/catalog/index_catalog.cpp
index 27c74cbdd37..a0653ba6661 100644
--- a/src/mongo/db/catalog/index_catalog.cpp
+++ b/src/mongo/db/catalog/index_catalog.cpp
@@ -605,6 +605,12 @@ Status IndexCatalog::_isSpecOk(OperationContext* txn, const BSONObj& spec) const
         if (isSparse) {
             return Status(ErrorCodes::CannotCreateIndex, "_id index cannot be sparse");
         }
+
+        if (collationElement &&
+            !CollatorInterface::collatorsMatch(collator.get(), _collection->getDefaultCollator())) {
+            return Status(ErrorCodes::CannotCreateIndex,
+                          "_id index must have the collection default collation");
+        }
     } else {
         // for non _id indexes, we check to see if replication has turned off all indexes
         // we _always_ created _id index
diff --git a/src/mongo/db/exec/idhack.cpp b/src/mongo/db/exec/idhack.cpp
index b1113be2e9e..fd392ac91ab 100644
--- a/src/mongo/db/exec/idhack.cpp
+++ b/src/mongo/db/exec/idhack.cpp
@@ -226,11 +226,12 @@ void IDHackStage::doInvalidate(OperationContext* txn, const RecordId& dl, Invali
 }
 
 // static
-bool IDHackStage::supportsQuery(const CanonicalQuery& query) {
+bool IDHackStage::supportsQuery(Collection* collection, const CanonicalQuery& query) {
     return !query.getQueryRequest().showRecordId() && query.getQueryRequest().getHint().isEmpty() &&
-        query.getQueryRequest().getCollation().isEmpty() && !query.getQueryRequest().getSkip() &&
+        !query.getQueryRequest().getSkip() &&
         CanonicalQuery::isSimpleIdQuery(query.getQueryRequest().getFilter()) &&
-        !query.getQueryRequest().isTailable();
+        !query.getQueryRequest().isTailable() &&
+        CollatorInterface::collatorsMatch(query.getCollator(), collection->getDefaultCollator());
 }
 
 unique_ptr<PlanStageStats> IDHackStage::getStats() {
diff --git a/src/mongo/db/exec/idhack.h b/src/mongo/db/exec/idhack.h
index c989bf3cff7..ac2466a956b 100644
--- a/src/mongo/db/exec/idhack.h
+++ b/src/mongo/db/exec/idhack.h
@@ -41,8 +41,9 @@ class IndexAccessMethod;
 class RecordCursor;
 
 /**
- * A standalone stage implementing the fast path for key-value retrievals
- * via the _id index.
+ * A standalone stage implementing the fast path for key-value retrievals via the _id index. Since
+ * the _id index always has the collection default collation, the IDHackStage can only be used when
+ * the query's collation is equal to the collection default.
  */
 class IDHackStage final : public PlanStage {
 public:
@@ -73,7 +74,7 @@ public:
     /**
      * ID Hack has a very strict criteria for the queries it supports.
      */
-    static bool supportsQuery(const CanonicalQuery& query);
+    static bool supportsQuery(Collection* collection, const CanonicalQuery& query);
 
     StageType stageType() const final {
         return STAGE_IDHACK;
diff --git a/src/mongo/db/index/index_access_method.cpp b/src/mongo/db/index/index_access_method.cpp
index d3ca3c0c808..93decde8b15 100644
--- a/src/mongo/db/index/index_access_method.cpp
+++ b/src/mongo/db/index/index_access_method.cpp
@@ -241,13 +241,20 @@ Status IndexAccessMethod::touch(OperationContext* txn) const {
 }
 
 RecordId IndexAccessMethod::findSingle(OperationContext* txn, const BSONObj& key) const {
+    // Generate the key for this index.
+    BSONObjSet keys;
+    MultikeyPaths* multikeyPaths = nullptr;
+    getKeys(key, &keys, multikeyPaths);
+    invariant(keys.size() == 1);
+
     std::unique_ptr<SortedDataInterface::Cursor> cursor(_newInterface->newCursor(txn));
     const auto requestedInfo = kDebugBuild ? SortedDataInterface::Cursor::kKeyAndLoc
                                            : SortedDataInterface::Cursor::kWantLoc;
-    if (auto kv = cursor->seekExact(key, requestedInfo)) {
+    if (auto kv = cursor->seekExact(*keys.begin(), requestedInfo)) {
         // StorageEngine should guarantee these.
         dassert(!kv->loc.isNull());
-        dassert(kv->key.woCompare(key, /*order*/ BSONObj(), /*considerFieldNames*/ false) == 0);
+        dassert(kv->key.woCompare(
+                    *keys.begin(), /*order*/ BSONObj(), /*considerFieldNames*/ false) == 0);
 
         return kv->loc;
     }
diff --git a/src/mongo/db/ops/parsed_delete.cpp b/src/mongo/db/ops/parsed_delete.cpp
index 77b070681a9..dbf6a0254f7 100644
--- a/src/mongo/db/ops/parsed_delete.cpp
+++ b/src/mongo/db/ops/parsed_delete.cpp
@@ -60,10 +60,7 @@ Status ParsedDelete::parseRequest() {
     // DeleteStage would not return the deleted document.
     invariant(_request->getProj().isEmpty() || _request->shouldReturnDeleted());
 
-    // TODO SERVER-23924: Create decision logic for idhack when the query has no collation, but
-    // there may be a collection default collation.
-    if (_request->getCollation().isEmpty() &&
-        CanonicalQuery::isSimpleIdQuery(_request->getQuery())) {
+    if (CanonicalQuery::isSimpleIdQuery(_request->getQuery())) {
         return Status::OK();
     }
 
diff --git a/src/mongo/db/ops/parsed_update.cpp b/src/mongo/db/ops/parsed_update.cpp
index e4b756e9efd..aea37881858 100644
--- a/src/mongo/db/ops/parsed_update.cpp
+++ b/src/mongo/db/ops/parsed_update.cpp
@@ -75,10 +75,7 @@ Status ParsedUpdate::parseRequest() {
 Status ParsedUpdate::parseQuery() {
     dassert(!_canonicalQuery.get());
 
-    // TODO SERVER-23924: Create decision logic for idhack when the query has no collation, but
-    // there may be a collection default collation.
-    if (!_driver.needMatchDetails() && _request->getCollation().isEmpty() &&
-        CanonicalQuery::isSimpleIdQuery(_request->getQuery())) {
+    if (!_driver.needMatchDetails() && CanonicalQuery::isSimpleIdQuery(_request->getQuery())) {
         return Status::OK();
     }
 
@@ -104,7 +101,6 @@ Status ParsedUpdate::parseQueryToCQ() {
     // deleted/modified under it, but a limit could inhibit that and give an EOF when the update
     // has not actually updated a document. This behavior is fine for findAndModify, but should
     // not apply to update in general.
-    // TODO SERVER-23473: Pass the collation to canonicalize().
     if (!_request->isMulti() && !_request->getSort().isEmpty()) {
         qr->setLimit(1);
     }
diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp
index 2e9189c87e3..8130ac57e4d 100644
--- a/src/mongo/db/query/get_executor.cpp
+++ b/src/mongo/db/query/get_executor.cpp
@@ -58,6 +58,7 @@
 #include "mongo/db/matcher/extensions_callback_real.h"
 #include "mongo/db/ops/update_lifecycle.h"
 #include "mongo/db/query/canonical_query.h"
+#include "mongo/db/query/collation/collator_factory_interface.h"
 #include "mongo/db/query/explain.h"
 #include "mongo/db/query/index_bounds_builder.h"
 #include "mongo/db/query/internal_plans.h"
@@ -272,7 +273,7 @@ StatusWith<PrepareExecutionResult> prepareExecution(OperationContext* opCtx,
     const IndexDescriptor* descriptor = collection->getIndexCatalog()->findIdIndex(opCtx);
 
     // If we have an _id index we can use an idhack plan.
-    if (descriptor && IDHackStage::supportsQuery(*canonicalQuery)) {
+    if (descriptor && IDHackStage::supportsQuery(collection, *canonicalQuery)) {
         LOG(2) << "Using idhack: " << canonicalQuery->toStringShort();
 
         root = make_unique<IDHackStage>(opCtx, collection, canonicalQuery.get(), ws, descriptor);
@@ -743,8 +744,21 @@ StatusWith<unique_ptr<PlanExecutor>> getExecutorDelete(OperationContext* txn,
 
         const IndexDescriptor* descriptor = collection->getIndexCatalog()->findIdIndex(txn);
 
+        // Construct delete request collator.
+        std::unique_ptr<CollatorInterface> collator;
+        if (!request->getCollation().isEmpty()) {
+            auto statusWithCollator = CollatorFactoryInterface::get(txn->getServiceContext())
+                                          ->makeFromBSON(request->getCollation());
+            if (!statusWithCollator.isOK()) {
+                return statusWithCollator.getStatus();
+            }
+            collator = std::move(statusWithCollator.getValue());
+        }
+        const bool hasCollectionDefaultCollation = request->getCollation().isEmpty() ||
+            CollatorInterface::collatorsMatch(collator.get(), collection->getDefaultCollator());
+
         if (descriptor && CanonicalQuery::isSimpleIdQuery(unparsedQuery) &&
-            request->getProj().isEmpty()) {
+            request->getProj().isEmpty() && hasCollectionDefaultCollation) {
             LOG(2) << "Using idhack: " << unparsedQuery.toString();
 
             PlanStage* idHackStage =
@@ -896,8 +910,11 @@ StatusWith<unique_ptr<PlanExecutor>> getExecutorUpdate(OperationContext* txn,
 
         const IndexDescriptor* descriptor = collection->getIndexCatalog()->findIdIndex(txn);
 
+        const bool hasCollectionDefaultCollation = CollatorInterface::collatorsMatch(
+            parsedUpdate->getCollator(), collection->getDefaultCollator());
+
         if (descriptor && CanonicalQuery::isSimpleIdQuery(unparsedQuery) &&
-            request->getProj().isEmpty()) {
+            request->getProj().isEmpty() && hasCollectionDefaultCollation) {
             LOG(2) << "Using idhack: " << unparsedQuery.toString();
 
             PlanStage* idHackStage =