SERVER-26724 createIndexes command should reject invalid options for _id index

8 files changed, 127 insertions, 141 deletions

diff --git a/jstests/core/create_collection.js b/jstests/core/create_collection.js
index c7858f5f902..2af8bc721a2 100644
--- a/jstests/core/create_collection.js
+++ b/jstests/core/create_collection.js
@@ -52,11 +52,13 @@
         db.createCollection("create_collection", {idIndex: {key: {a: 1}, name: "_id_"}}),
         ErrorCodes.BadValue);
 
-    // "idIndex" field must have name equal to "_id_".
+    // The name of an _id index gets corrected to "_id_".
     db.create_collection.drop();
-    assert.commandFailedWithCode(
-        db.createCollection("create_collection", {idIndex: {key: {_id: 1}, name: "a_1"}}),
-        ErrorCodes.BadValue);
+    assert.commandWorked(
+        db.createCollection("create_collection", {idIndex: {key: {_id: 1}, name: "a_1"}}));
+    var indexSpec = GetIndexHelpers.findByKeyPattern(db.create_collection.getIndexes(), {_id: 1});
+    assert.neq(indexSpec, null);
+    assert.eq(indexSpec.name, "_id_", tojson(indexSpec));
 
     // "idIndex" field must only contain fields that are allowed for an _id index.
     db.create_collection.drop();
@@ -69,7 +71,7 @@
     db.create_collection.drop();
     assert.commandWorked(
         db.createCollection("create_collection", {idIndex: {key: {_id: 1}, name: "_id_"}}));
-    var indexSpec = GetIndexHelpers.findByName(db.create_collection.getIndexes(), "_id_");
+    indexSpec = GetIndexHelpers.findByName(db.create_collection.getIndexes(), "_id_");
     assert.neq(indexSpec, null);
     assert.eq(indexSpec.v, 2, tojson(indexSpec));
 
diff --git a/jstests/core/index_id_desc.js b/jstests/core/index_id_desc.js
deleted file mode 100644
index 515a422163e..00000000000
--- a/jstests/core/index_id_desc.js
+++ /dev/null
@@ -1,34 +0,0 @@
-// Test creation of an index with key pattern {_id: -1}.  It is expected that a request for creation
-// of a {_id: -1} index is treated as if it were a request for creation of a {_id: 1} index.
-// SERVER-14833.
-
-var coll = db.index_id_desc;
-var indexes;
-
-// Test ensureIndex({_id: -1}) on a nonexistent collection.
-coll.drop();
-assert.commandWorked(coll.ensureIndex({_id: -1}));
-indexes = coll.getIndexes();
-assert.eq(1, indexes.length);
-assert.eq("_id_", indexes[0].name);
-assert.eq({_id: 1}, indexes[0].key);
-
-// Test ensureIndex({_id: -1}) on a normal empty collection.
-coll.drop();
-assert.commandWorked(coll.runCommand("create"));
-assert.eq(1, coll.getIndexes().length);
-assert.commandWorked(coll.ensureIndex({_id: -1}));
-indexes = coll.getIndexes();
-assert.eq(1, indexes.length);
-assert.eq("_id_", indexes[0].name);
-assert.eq({_id: 1}, indexes[0].key);
-
-// Test ensureIndex({_id: -1}) on an empty collection with no _id index.
-coll.drop();
-assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
-assert.eq(0, coll.getIndexes().length);
-assert.commandWorked(coll.ensureIndex({_id: -1}));
-indexes = coll.getIndexes();
-assert.eq(1, indexes.length);
-assert.eq("_id_", indexes[0].name);
-assert.eq({_id: 1}, indexes[0].key);
diff --git a/jstests/core/index_id_options.js b/jstests/core/index_id_options.js
index 5333c12daa8..a387c409b6d 100644
--- a/jstests/core/index_id_options.js
+++ b/jstests/core/index_id_options.js
@@ -1,79 +1,72 @@
 // Test creation of the _id index with various options:
-// - _id indexes must be unique.
-// - _id indexes can't be sparse.
-// - _id indexes can't be partial indexes.
+// - _id indexes must have key pattern {_id: 1}.
+// - The name of an _id index gets corrected to "_id_".
+// - _id indexes cannot have any options other than "key", "name", "ns", "v", and "collation".
+// - _id indexes must have the collection default collation.
+// - Non-_id indexes cannot have the name "_id_".
 
-var coll = db.index_id_options;
+(function() {
+    "use strict";
 
-//
-// Unique index.
-//
+    load("jstests/libs/get_index_helpers.js");
 
-// Creation of _id index with "non-zero" value for "unique" should succeed.
-coll.drop();
-assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
-assert.commandWorked(coll.ensureIndex({_id: 1}, {unique: true}));
-coll.drop();
-assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
-assert.commandWorked(coll.ensureIndex({_id: 1}, {unique: 1}));
-coll.drop();
-assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
-assert.commandWorked(coll.ensureIndex({_id: 1}, {unique: NumberLong(1)}));
+    var coll = db.index_id_options;
 
-// Creation of _id index with "zero" value for "unique" should fail.
-coll.drop();
-assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
-assert.commandFailed(coll.ensureIndex({_id: 1}, {unique: false}));
-coll.drop();
-assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
-assert.commandFailed(coll.ensureIndex({_id: 1}, {unique: 0}));
-coll.drop();
-assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
-assert.commandFailed(coll.ensureIndex({_id: 1}, {unique: NumberLong(0)}));
+    // _id indexes must have key pattern {_id: 1}.
+    coll.drop();
+    assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
+    assert.commandFailed(coll.ensureIndex({_id: -1}, {name: "_id_"}));
 
-//
-// Sparse index.
-//
+    // The name of an _id index gets corrected to "_id_".
+    coll.drop();
+    assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
+    assert.commandWorked(coll.ensureIndex({_id: 1}, {name: "bad"}));
+    var spec = GetIndexHelpers.findByKeyPattern(coll.getIndexes(), {_id: 1});
+    assert.neq(null, spec, "_id index spec not found");
+    assert.eq("_id_", spec.name, tojson(spec));
 
-// Creation of _id index with "non-zero" value for "sparse" should fail.
-coll.drop();
-assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
-assert.commandFailed(coll.ensureIndex({_id: 1}, {sparse: true}));
-coll.drop();
-assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
-assert.commandFailed(coll.ensureIndex({_id: 1}, {sparse: 1}));
-coll.drop();
-assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
-assert.commandFailed(coll.ensureIndex({_id: 1}, {sparse: NumberLong(1)}));
+    // _id indexes cannot have any options other than "key", "name", "ns", "v", and "collation."
+    coll.drop();
+    assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
+    assert.commandFailed(coll.ensureIndex({_id: 1}, {name: "_id_", unique: true}));
+    assert.commandFailed(coll.ensureIndex({_id: 1}, {name: "_id_", sparse: false}));
+    assert.commandFailed(
+        coll.ensureIndex({_id: 1}, {name: "_id_", partialFilterExpression: {a: 1}}));
+    assert.commandFailed(coll.ensureIndex({_id: 1}, {name: "_id_", expireAfterSeconds: 3600}));
+    assert.commandFailed(coll.ensureIndex({_id: 1}, {name: "_id_", background: false}));
+    assert.commandFailed(coll.ensureIndex({_id: 1}, {name: "_id_", unknown: true}));
+    assert.commandWorked(coll.ensureIndex(
+        {_id: 1}, {name: "_id_", ns: coll.getFullName(), v: 2, collation: {locale: "simple"}}));
 
-// Creation of _id index with "zero" value for "unique" should succeed.
-coll.drop();
-assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
-assert.commandWorked(coll.ensureIndex({_id: 1}, {sparse: false}));
-coll.drop();
-assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
-assert.commandWorked(coll.ensureIndex({_id: 1}, {sparse: 0}));
-coll.drop();
-assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
-assert.commandWorked(coll.ensureIndex({_id: 1}, {sparse: NumberLong(0)}));
+    // _id indexes must have the collection default collation.
+    coll.drop();
+    assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
+    assert.commandFailed(coll.ensureIndex({_id: 1}, {name: "_id_", collation: {locale: "en_US"}}));
+    assert.commandWorked(coll.ensureIndex({_id: 1}, {name: "_id_", collation: {locale: "simple"}}));
 
-//
-// Partial index.
-//
+    coll.drop();
+    assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
+    assert.commandWorked(coll.ensureIndex({_id: 1}, {name: "_id_"}));
 
-// Creation of _id index with any value for "partialFilterExpression" should fail.
-coll.drop();
-assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
-assert.commandFailed(coll.ensureIndex({_id: 1}, {partialFilterExpression: false}));
-coll.drop();
-assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
-assert.commandFailed(coll.ensureIndex({_id: 1}, {partialFilterExpression: null}));
-coll.drop();
-assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
-assert.commandFailed(coll.ensureIndex({_id: 1}, {partialFilterExpression: {}}));
-coll.drop();
-assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
-assert.commandFailed(coll.ensureIndex({_id: 1}, {partialFilterExpression: {a: 1}}));
-coll.drop();
-assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
-assert.commandFailed(coll.ensureIndex({_id: 1}, {partialFilterExpression: []}));
+    coll.drop();
+    assert.commandWorked(
+        coll.runCommand("create", {autoIndexId: false, collation: {locale: "en_US"}}));
+    assert.commandFailed(coll.ensureIndex({_id: 1}, {name: "_id_", collation: {locale: "simple"}}));
+    assert.commandFailed(coll.ensureIndex({_id: 1}, {name: "_id_", collation: {locale: "fr_CA"}}));
+    assert.commandWorked(
+        coll.ensureIndex({_id: 1}, {name: "_id_", collation: {locale: "en_US", strength: 3}}));
+
+    coll.drop();
+    assert.commandWorked(
+        coll.runCommand("create", {autoIndexId: false, collation: {locale: "en_US"}}));
+    assert.commandWorked(coll.ensureIndex({_id: 1}, {name: "_id_"}));
+    spec = GetIndexHelpers.findByName(coll.getIndexes(), "_id_");
+    assert.neq(null, spec, "_id index spec not found");
+    assert.eq("en_US", spec.collation.locale, tojson(spec));
+
+    // Non-_id indexes cannot have the name "_id_".
+    coll.drop();
+    assert.commandWorked(coll.runCommand("create", {autoIndexId: false}));
+    assert.commandFailed(coll.ensureIndex({_id: "hashed"}, {name: "_id_"}));
+    assert.commandFailed(coll.ensureIndex({a: 1}, {name: "_id_"}));
+})();
diff --git a/jstests/sharding/key_many.js b/jstests/sharding/key_many.js
index 96e351c25e8..285647bf3cd 100644
--- a/jstests/sharding/key_many.js
+++ b/jstests/sharding/key_many.js
@@ -229,7 +229,7 @@
         assert.writeOK(
             c.update(makeObjectDotted(curT.values[3]), {$set: {xx: 17}}, {upsert: true}));
 
-        assert.commandWorked(c.ensureIndex({_id: 1}, {unique: true}));
+        assert.commandWorked(c.ensureIndex({_id: 1}));
 
         // multi update
         var mysum = 0;
diff --git a/src/mongo/db/catalog/index_key_validate.cpp b/src/mongo/db/catalog/index_key_validate.cpp
index eb7f794136e..e1616a34c86 100644
--- a/src/mongo/db/catalog/index_key_validate.cpp
+++ b/src/mongo/db/catalog/index_key_validate.cpp
@@ -94,7 +94,9 @@ static const std::set<StringData> allowedIdIndexFieldNames = {
     IndexDescriptor::kIndexNameFieldName,
     IndexDescriptor::kIndexVersionFieldName,
     IndexDescriptor::kKeyPatternFieldName,
-    IndexDescriptor::kNamespaceFieldName};
+    IndexDescriptor::kNamespaceFieldName,
+    // Index creation under legacy writeMode can result in an index spec with an _id field.
+    "_id"};
 }
 
 Status validateKeyPattern(const BSONObj& key, IndexDescriptor::IndexVersion indexVersion) {
@@ -411,16 +413,6 @@ Status validateIdIndexSpec(const BSONObj& indexSpec) {
                               << keyPatternElem.Obj()};
     }
 
-    auto nameElem = indexSpec[IndexDescriptor::kIndexNameFieldName];
-    // validateIndexSpec() should have already verified that 'nameElem' is a string.
-    invariant(nameElem.type() == BSONType::String);
-    if (nameElem.String() != "_id_"_sd) {
-        return {ErrorCodes::BadValue,
-                str::stream() << "The field '" << IndexDescriptor::kIndexNameFieldName
-                              << "' for an _id index must be '_id_', but got "
-                              << nameElem.String()};
-    }
-
     return Status::OK();
 }
 
diff --git a/src/mongo/db/catalog/index_spec_validate_test.cpp b/src/mongo/db/catalog/index_spec_validate_test.cpp
index e2e77d3cfe2..6d0bc3eb918 100644
--- a/src/mongo/db/catalog/index_spec_validate_test.cpp
+++ b/src/mongo/db/catalog/index_spec_validate_test.cpp
@@ -710,19 +710,9 @@ TEST(IdIndexSpecValidateTest, ReturnsAnErrorIfKeyPatternIsIncorrectForIdIndex) {
                                              << 2)));
 }
 
-TEST(IdIndexSpecValidateTest, ReturnsAnErrorIfNameIsIncorrectForIdIndex) {
-    ASSERT_EQ(ErrorCodes::BadValue,
-              validateIdIndexSpec(BSON("key" << BSON("_id" << 1) << "name"
-                                             << "indexName"
-                                             << "ns"
-                                             << kTestNamespace.ns()
-                                             << "v"
-                                             << 2)));
-}
-
-TEST(IdIndexSpecValidateTest, ReturnsOKStatusIfKeyAndNameCorrectForIdIndex) {
+TEST(IdIndexSpecValidateTest, ReturnsOKStatusIfKeyPatternCorrectForIdIndex) {
     ASSERT_OK(validateIdIndexSpec(BSON("key" << BSON("_id" << 1) << "name"
-                                             << "_id_"
+                                             << "anyname"
                                              << "ns"
                                              << kTestNamespace.ns()
                                              << "v"
diff --git a/src/mongo/db/commands/create_indexes.cpp b/src/mongo/db/commands/create_indexes.cpp
index fbd131211b4..116a6248cc9 100644
--- a/src/mongo/db/commands/create_indexes.cpp
+++ b/src/mongo/db/commands/create_indexes.cpp
@@ -45,6 +45,7 @@
 #include "mongo/db/index/index_descriptor.h"
 #include "mongo/db/op_observer.h"
 #include "mongo/db/ops/insert.h"
+#include "mongo/db/query/collation/collator_factory_interface.h"
 #include "mongo/db/repl/repl_client_info.h"
 #include "mongo/db/repl/replication_coordinator_global.h"
 #include "mongo/db/s/collection_metadata.h"
@@ -97,12 +98,27 @@ StatusWith<std::vector<BSONObj>> parseAndValidateIndexSpecs(
                                           << typeName(indexesElem.type())};
                 }
 
-                auto indexSpec = index_key_validate::validateIndexSpec(
+                auto indexSpecStatus = index_key_validate::validateIndexSpec(
                     indexesElem.Obj(), ns, featureCompatibility);
-                if (!indexSpec.isOK()) {
-                    return indexSpec.getStatus();
+                if (!indexSpecStatus.isOK()) {
+                    return indexSpecStatus.getStatus();
                 }
-                indexSpecs.push_back(std::move(indexSpec.getValue()));
+                auto indexSpec = indexSpecStatus.getValue();
+
+                if (IndexDescriptor::isIdIndexPattern(
+                        indexSpec[IndexDescriptor::kKeyPatternFieldName].Obj())) {
+                    auto status = index_key_validate::validateIdIndexSpec(indexSpec);
+                    if (!status.isOK()) {
+                        return status;
+                    }
+                } else if (indexSpec[IndexDescriptor::kIndexNameFieldName].String() == "_id_"_sd) {
+                    return {ErrorCodes::BadValue,
+                            str::stream() << "The index name '_id_' is reserved for the _id index, "
+                                             "which must have key pattern {_id: 1}, found "
+                                          << indexSpec[IndexDescriptor::kKeyPatternFieldName]};
+                }
+
+                indexSpecs.push_back(std::move(indexSpec));
             }
 
             hasIndexesField = true;
@@ -144,12 +160,39 @@ StatusWith<std::vector<BSONObj>> resolveCollectionDefaultProperties(
     std::vector<BSONObj> indexSpecsWithDefaults = std::move(indexSpecs);
 
     for (size_t i = 0, numIndexSpecs = indexSpecsWithDefaults.size(); i < numIndexSpecs; ++i) {
-        auto validatedSpec = index_key_validate::validateIndexSpecCollation(
+        auto indexSpecStatus = index_key_validate::validateIndexSpecCollation(
             txn, indexSpecsWithDefaults[i], collection->getDefaultCollator());
-        if (!validatedSpec.isOK()) {
-            return validatedSpec.getStatus();
+        if (!indexSpecStatus.isOK()) {
+            return indexSpecStatus.getStatus();
         }
-        indexSpecsWithDefaults[i] = validatedSpec.getValue();
+        auto indexSpec = indexSpecStatus.getValue();
+
+        if (IndexDescriptor::isIdIndexPattern(
+                indexSpec[IndexDescriptor::kKeyPatternFieldName].Obj())) {
+            std::unique_ptr<CollatorInterface> indexCollator;
+            if (auto collationElem = indexSpec[IndexDescriptor::kCollationFieldName]) {
+                auto collatorStatus = CollatorFactoryInterface::get(txn->getServiceContext())
+                                          ->makeFromBSON(collationElem.Obj());
+                // validateIndexSpecCollation() should have checked that the index collation spec is
+                // valid.
+                invariantOK(collatorStatus.getStatus());
+                indexCollator = std::move(collatorStatus.getValue());
+            }
+            if (!CollatorInterface::collatorsMatch(collection->getDefaultCollator(),
+                                                   indexCollator.get())) {
+                return {ErrorCodes::BadValue,
+                        str::stream() << "The _id index must have the same collation as the "
+                                         "collection. Index collation: "
+                                      << (indexCollator.get() ? indexCollator->getSpec().toBSON()
+                                                              : CollationSpec::kSimpleSpec)
+                                      << ", collection collation: "
+                                      << (collection->getDefaultCollator()
+                                              ? collection->getDefaultCollator()->getSpec().toBSON()
+                                              : CollationSpec::kSimpleSpec)};
+            }
+        }
+
+        indexSpecsWithDefaults[i] = indexSpec;
     }
 
     return indexSpecsWithDefaults;
diff --git a/src/mongo/s/cluster_write.cpp b/src/mongo/s/cluster_write.cpp
index f35d78fec3e..d4c81dcbcef 100644
--- a/src/mongo/s/cluster_write.cpp
+++ b/src/mongo/s/cluster_write.cpp
@@ -104,7 +104,7 @@ BSONObj createIndexDoc(const string& ns,
         indexDoc.append("collation", collation);
     }
 
-    if (unique) {
+    if (unique && !IndexDescriptor::isIdIndexPattern(keys)) {
         indexDoc.appendBool("unique", unique);
     }