From a91436f2dff19f1ea7cc69bb00531f9bcaaaa681 Mon Sep 17 00:00:00 2001 From: Yu Jin Kang Park Date: Tue, 5 Apr 2022 12:32:30 +0000 Subject: SERVER-64928 Disallow createIndexes with clustered:false --- jstests/core/clustered_collection_creation.js | 7 +++++++ ...red_collection_create_index_clustered_common.js | 22 ++++++++++++++++++++++ src/mongo/db/catalog/index_key_validate.cpp | 22 ++++++++++++++-------- 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/jstests/core/clustered_collection_creation.js b/jstests/core/clustered_collection_creation.js index ee721115b4e..7481f1876e9 100644 --- a/jstests/core/clustered_collection_creation.js +++ b/jstests/core/clustered_collection_creation.js @@ -336,6 +336,13 @@ assert.commandFailedWithCode( {clusteredIndex: {key: {_id: 1}, unique: true}, expireAfterSeconds: -10}), ErrorCodes.InvalidOptions); +// Check using clustered : false, which is disallowed, fails for implicit collection creation +assert.commandFailedWithCode(db.runCommand({ + createIndexes: "some_collection", + indexes: [{"key": {"somedata": 1}, "name": "s2", "clustered": false, "unique": true}] +}), + 6492800); + // Validate that it's possible to create secondary indexes, regardless of whether they // include the cluster key as one of the fields. validateCompoundSecondaryIndexes(replicatedDB, replicatedColl, {_id: 1}); diff --git a/jstests/libs/clustered_collections/clustered_collection_create_index_clustered_common.js b/jstests/libs/clustered_collections/clustered_collection_create_index_clustered_common.js index 83b95160714..3292b2cbd1f 100644 --- a/jstests/libs/clustered_collections/clustered_collection_create_index_clustered_common.js +++ b/jstests/libs/clustered_collections/clustered_collection_create_index_clustered_common.js @@ -29,6 +29,13 @@ const CreateIndexesClusteredTest = (function() { assert.commandFailedWithCode(testColl.createIndex({a: 1}, {clustered: true, unique: true}), 6243700); + // Check using clustered : false, which is disallowed, fails in an empty collection + assert.commandFailedWithCode(testDB.runCommand({ + createIndexes: collName, + "indexes": [{key: {"_id": 1}, name: "anyName", clustered: false, unique: true}], + }), + 6492800); + // Insert some docs. Sometimes empty collections are treated as special when it comes to // index builds. const batchSize = 100; @@ -50,6 +57,10 @@ const CreateIndexesClusteredTest = (function() { assert.commandFailedWithCode(testColl.createIndex({a: 1}, {clustered: true, unique: true}), 6243700); + + // Check using clustered : false, which is disallowed, fails in non empty collection + assert.commandFailedWithCode( + testColl.createIndex({_id: 1}, {clustered: false, unique: true}), 6492800); }; /** @@ -97,6 +108,13 @@ const CreateIndexesClusteredTest = (function() { assert.commandFailedWithCode( testColl.createIndex({notMyIndex: 1}, {clustered: true, unique: true}), 6243700); + // Check using clustered : false, which is disallowed, fails in empty collection + assert.commandFailedWithCode(testDB.runCommand({ + createIndexes: collName, + "indexes": [{key: {"newKey": 1}, name: "anyName", clustered: false, unique: true}], + }), + 6492800); + // Insert some docs. Empty collections are treated as special (single phase) when // it comes to index builds. const batchSize = 100; @@ -137,6 +155,10 @@ const CreateIndexesClusteredTest = (function() { // a clusteredIndex with a custom name. assert.commandWorked(testColl.createIndex({_id: 1}, {name: "notTheClusterKeyName"})); + // Check using clustered : false, which is disallowed, fails non empty collection + assert.commandFailedWithCode( + testColl.createIndex({_id: 1}, {clustered: false, unique: true}), 6492800); + ClusteredCollectionUtil.validateListIndexes(testDB, collName, fullCreateOptions); }; diff --git a/src/mongo/db/catalog/index_key_validate.cpp b/src/mongo/db/catalog/index_key_validate.cpp index f6081f0ff73..3ab24d48ff0 100644 --- a/src/mongo/db/catalog/index_key_validate.cpp +++ b/src/mongo/db/catalog/index_key_validate.cpp @@ -288,7 +288,7 @@ StatusWith validateIndexSpec(OperationContext* opCtx, const BSONObj& in bool hasCollationField = false; bool hasWeightsField = false; bool hasOriginalSpecField = false; - bool hasClusteredField = indexSpec.hasField("clustered"); + auto clusteredField = indexSpec["clustered"]; bool apiStrict = opCtx && APIParameters::get(opCtx).getAPIStrict().value_or(false); auto fieldNamesValidStatus = validateIndexSpecFieldNames(indexSpec); @@ -545,13 +545,19 @@ StatusWith validateIndexSpec(OperationContext* opCtx, const BSONObj& in << "' field is a required property of an index specification"}; } - if (hasClusteredField && - (!indexSpec.hasField(IndexDescriptor::kUniqueFieldName) || - indexSpec.getBoolField(IndexDescriptor::kUniqueFieldName) == false)) { - // Only require 'unique' if clustered is specified. - return {ErrorCodes::CannotCreateIndex, - str::stream() << "The '" << IndexDescriptor::kUniqueFieldName - << "' field is required when 'clustered' is specified"}; + if (clusteredField) { + if (!clusteredField.trueValue()) { + // Disallow 'clustered' from taking value 'false' + return {ErrorCodes::Error(6492800), "Value 'false' for field 'clustered' is invalid"}; + } + + if (!indexSpec.hasField(IndexDescriptor::kUniqueFieldName) || + indexSpec.getBoolField(IndexDescriptor::kUniqueFieldName) == false) { + // Only require 'unique' if clustered is specified. + return {ErrorCodes::CannotCreateIndex, + str::stream() << "The '" << IndexDescriptor::kUniqueFieldName + << "' field is required when 'clustered' is specified"}; + } } if (hasCollationField && *resolvedIndexVersion < IndexVersion::kV2) { -- cgit v1.2.1