From f64776b399ee0934724da0065541886c31da90f4 Mon Sep 17 00:00:00 2001 From: Benety Goh Date: Wed, 3 Nov 2021 18:33:46 -0400 Subject: SERVER-61158 add unique to collMod index request This also adds unique_new to the collMod response when the command completes successfully. --- jstests/core/collmod_convert_to_unique.js | 79 +++++++++++++++++++++++++++++++ src/mongo/db/catalog/coll_mod.cpp | 27 +++++++++-- src/mongo/db/catalog/coll_mod_index.cpp | 35 +++++++++++++- src/mongo/db/catalog/coll_mod_index.h | 1 + src/mongo/db/coll_mod.idl | 6 +++ src/mongo/db/op_observer.h | 1 + src/mongo/db/op_observer_util.cpp | 3 ++ 7 files changed, 147 insertions(+), 5 deletions(-) create mode 100644 jstests/core/collmod_convert_to_unique.js diff --git a/jstests/core/collmod_convert_to_unique.js b/jstests/core/collmod_convert_to_unique.js new file mode 100644 index 00000000000..280f571c005 --- /dev/null +++ b/jstests/core/collmod_convert_to_unique.js @@ -0,0 +1,79 @@ +/** + * Basic js tests for the collMod command converting regular indexes to unique indexes. + * + * @tags: [ + * # Cannot implicitly shard accessed collections because of collection existing when none + * # expected. + * assumes_no_implicit_collection_creation_after_drop, # common tag in collMod tests. + * requires_fcv_52, + * requires_non_retryable_commands, # common tag in collMod tests. + * ] + */ + +(function() { +'use strict'; + +const collModIndexUniqueEnabled = assert + .commandWorked(db.getMongo().adminCommand( + {getParameter: 1, featureFlagCollModIndexUnique: 1})) + .featureFlagCollModIndexUnique.value; + +if (!collModIndexUniqueEnabled) { + jsTestLog('Skipping test because the collMod unique index feature flag is disabled.'); + return; +} + +const collName = 'collmod_convert_to_unique'; +const coll = db.getCollection(collName); +coll.drop(); +assert.commandWorked(db.createCollection(collName)); + +function countUnique(key) { + const all = coll.getIndexes().filter(function(z) { + return z.unique && friendlyEqual(z.key, key); + }); + return all.length; +} + +// Creates a regular index and use collMod to convert it to a unique index. +assert.commandWorked(coll.createIndex({a: 1})); + +// Tries to modify with a string 'unique' value. +assert.commandFailedWithCode( + db.runCommand({collMod: collName, index: {keyPattern: {a: 1}, unique: '100'}}), + ErrorCodes.TypeMismatch); + +// Tries to modify with a false 'unique' value. +assert.commandFailedWithCode(db.runCommand({ + collMod: collName, + index: {keyPattern: {a: 1}, unique: false}, +}), + ErrorCodes.BadValue); + +assert.commandWorked(coll.insert({_id: 1, a: 100})); +assert.commandWorked(coll.insert({_id: 2, a: 100})); +assert.commandWorked(coll.remove({_id: 2})); + +// Successfully converts to a unique index. +const result = assert.commandWorked( + db.runCommand({collMod: collName, index: {keyPattern: {a: 1}, unique: true}})); + +// New index state should be reflected in 'unique_new' field in collMod response. +const assertUniqueNew = function(result) { + assert(result.hasOwnProperty('unique_new'), tojson(result)); + assert(result.unique_new, tojson(result)); +}; +if (db.getMongo().isMongos()) { + // Check the first shard's result from mongos. + assert(result.hasOwnProperty('raw'), tojson(result)); + assertUniqueNew(Object.values(result.raw)[0]); +} else { + assertUniqueNew(result); +} + +// Look up index details in listIndexes output. +assert.eq(countUnique({a: 1}), 0, 'index should not be unique yet: ' + tojson(coll.getIndexes())); + +// Test uniqueness constraint. +assert.commandWorked(coll.insert({_id: 100, a: 100})); +})(); diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp index 3e24be31e7e..4c8bbdbf40c 100644 --- a/src/mongo/db/catalog/coll_mod.cpp +++ b/src/mongo/db/catalog/coll_mod.cpp @@ -52,8 +52,10 @@ #include "mongo/db/repl/replication_coordinator.h" #include "mongo/db/s/collection_sharding_state.h" #include "mongo/db/s/database_sharding_state.h" +#include "mongo/db/server_options.h" #include "mongo/db/service_context.h" #include "mongo/db/storage/recovery_unit.h" +#include "mongo/db/storage/storage_parameters_gen.h" #include "mongo/db/timeseries/timeseries_options.h" #include "mongo/db/ttl_collection_cache.h" #include "mongo/db/views/view_catalog.h" @@ -136,7 +138,7 @@ StatusWith parseCollModRequest(OperationContext* opCtx, for (auto&& elem : indexObj) { const auto field = elem.fieldNameStringData(); if (field != "name" && field != "keyPattern" && field != "expireAfterSeconds" && - field != "hidden") { + field != "hidden" && field != "unique") { return {ErrorCodes::InvalidOptions, str::stream() << "Unrecognized field '" << field << "' in 'index' option"}; @@ -172,9 +174,19 @@ StatusWith parseCollModRequest(OperationContext* opCtx, auto cmrIndex = &cmr.indexRequest; cmrIndex->indexExpireAfterSeconds = indexObj["expireAfterSeconds"]; cmrIndex->indexHidden = indexObj["hidden"]; + cmrIndex->indexUnique = indexObj["unique"]; - if (cmrIndex->indexExpireAfterSeconds.eoo() && cmrIndex->indexHidden.eoo()) { - return Status(ErrorCodes::InvalidOptions, "no expireAfterSeconds or hidden field"); + if (cmrIndex->indexUnique) { + uassert(ErrorCodes::InvalidOptions, + "collMod does not support converting an index to unique", + feature_flags::gCollModIndexUnique.isEnabled( + serverGlobalParams.featureCompatibility)); + } + + if (cmrIndex->indexExpireAfterSeconds.eoo() && cmrIndex->indexHidden.eoo() && + cmrIndex->indexUnique.eoo()) { + return Status(ErrorCodes::InvalidOptions, + "no expireAfterSeconds, hidden, or unique field"); } if (!cmrIndex->indexExpireAfterSeconds.eoo()) { if (isTimeseries) { @@ -192,6 +204,9 @@ StatusWith parseCollModRequest(OperationContext* opCtx, if (!cmrIndex->indexHidden.eoo() && !cmrIndex->indexHidden.isBoolean()) { return Status(ErrorCodes::InvalidOptions, "hidden field must be a boolean"); } + if (!cmrIndex->indexUnique.eoo() && !cmrIndex->indexUnique.isBoolean()) { + return Status(ErrorCodes::InvalidOptions, "unique field must be a boolean"); + } if (!cmrIndex->indexHidden.eoo() && coll->isClustered() && !nss.isTimeseriesBucketsCollection()) { auto clusteredInfo = coll->getClusteredInfo(); @@ -286,6 +301,12 @@ StatusWith parseCollModRequest(OperationContext* opCtx, return Status(ErrorCodes::BadValue, "can't hide _id index"); } } + + if (cmrIndex->indexUnique) { + if (!cmrIndex->indexUnique.trueValue()) { + return Status(ErrorCodes::BadValue, "Cannot make index non-unique"); + } + } } else if (fieldName == "validator" && !isView && !isTimeseries) { // If the feature compatibility version is not kLatest, and we are validating features // as primary, ban the use of new agg features introduced in kLatest to prevent them diff --git a/src/mongo/db/catalog/coll_mod_index.cpp b/src/mongo/db/catalog/coll_mod_index.cpp index 11d92d1641b..a9cf651155d 100644 --- a/src/mongo/db/catalog/coll_mod_index.cpp +++ b/src/mongo/db/catalog/coll_mod_index.cpp @@ -50,11 +50,13 @@ public: const BSONElement& newExpireSecs, const BSONElement& oldHidden, const BSONElement& newHidden, + const BSONElement& newUnique, BSONObjBuilder* result) : _oldExpireSecs(oldExpireSecs), _newExpireSecs(newExpireSecs), _oldHidden(oldHidden), _newHidden(newHidden), + _newUnique(newUnique), _result(result) {} void commit(boost::optional) override { @@ -70,6 +72,10 @@ public: _result->append("hidden_old", oldValue); _result->appendAs(_newHidden, "hidden_new"); } + if (!_newUnique.eoo()) { + invariant(_newUnique.trueValue()); + _result->appendBool("unique_new", true); + } } void rollback() override {} @@ -79,6 +85,7 @@ private: const BSONElement _newExpireSecs; const BSONElement _oldHidden; const BSONElement _newHidden; + const BSONElement _newUnique; BSONObjBuilder* _result; }; @@ -129,6 +136,22 @@ void _processCollModIndexRequestHidden(OperationContext* opCtx, } } +/** + * Adjusts unique setting on an index. + * An index can be converted to unique but removing the uniqueness property is not allowed. + */ +void _processCollModIndexRequestUnique(OperationContext* opCtx, + AutoGetCollection* autoColl, + const IndexDescriptor* idx, + BSONElement indexUnique, + BSONElement* newUnique) { + // Do not update catalog if index is already unique. + if (idx->infoObj().getField("unique").trueValue()) { + return; + } + *newUnique = indexUnique; +} + } // namespace void processCollModIndexRequest(OperationContext* opCtx, @@ -139,9 +162,10 @@ void processCollModIndexRequest(OperationContext* opCtx, auto idx = collModIndexRequest.idx; auto indexExpireAfterSeconds = collModIndexRequest.indexExpireAfterSeconds; auto indexHidden = collModIndexRequest.indexHidden; + auto indexUnique = collModIndexRequest.indexUnique; // Return early if there are no index modifications requested. - if (!indexExpireAfterSeconds && !indexHidden) { + if (!indexExpireAfterSeconds && !indexHidden && !indexUnique) { return; } @@ -149,6 +173,7 @@ void processCollModIndexRequest(OperationContext* opCtx, BSONElement oldExpireSecs = {}; BSONElement newHidden = {}; BSONElement oldHidden = {}; + BSONElement newUnique = {}; // TTL Index if (indexExpireAfterSeconds) { @@ -163,6 +188,11 @@ void processCollModIndexRequest(OperationContext* opCtx, opCtx, autoColl, idx, indexHidden, &newHidden, &oldHidden); } + // User wants to convert an index to be unique. + if (indexUnique) { + _processCollModIndexRequestUnique(opCtx, autoColl, idx, indexUnique, &newUnique); + } + *indexCollModInfo = IndexCollModInfo{ !indexExpireAfterSeconds ? boost::optional() : Seconds(newExpireSecs.safeNumberLong()), @@ -170,6 +200,7 @@ void processCollModIndexRequest(OperationContext* opCtx, : Seconds(oldExpireSecs.safeNumberLong()), !indexHidden ? boost::optional() : newHidden.booleanSafe(), !indexHidden ? boost::optional() : oldHidden.booleanSafe(), + !indexUnique ? boost::optional() : newUnique.booleanSafe(), idx->indexName()}; // Notify the index catalog that the definition of this index changed. This will invalidate the @@ -178,7 +209,7 @@ void processCollModIndexRequest(OperationContext* opCtx, opCtx, autoColl->getWritableCollection(), idx); opCtx->recoveryUnit()->registerChange(std::make_unique( - oldExpireSecs, newExpireSecs, oldHidden, newHidden, result)); + oldExpireSecs, newExpireSecs, oldHidden, newHidden, newUnique, result)); if (MONGO_unlikely(assertAfterIndexUpdate.shouldFail())) { LOGV2(20307, "collMod - assertAfterIndexUpdate fail point enabled"); diff --git a/src/mongo/db/catalog/coll_mod_index.h b/src/mongo/db/catalog/coll_mod_index.h index 13998bfdc67..83b30138dac 100644 --- a/src/mongo/db/catalog/coll_mod_index.h +++ b/src/mongo/db/catalog/coll_mod_index.h @@ -45,6 +45,7 @@ struct CollModIndexRequest { const IndexDescriptor* idx = nullptr; BSONElement indexExpireAfterSeconds = {}; BSONElement indexHidden = {}; + BSONElement indexUnique = {}; }; /** diff --git a/src/mongo/db/coll_mod.idl b/src/mongo/db/coll_mod.idl index 5cc5fdeb8bd..9817f06d0aa 100644 --- a/src/mongo/db/coll_mod.idl +++ b/src/mongo/db/coll_mod.idl @@ -55,6 +55,9 @@ structs: hidden: optional: true type: safeBool + unique: + optional: true + type: safeBool CollModReply: description: "The collMod command's reply." @@ -72,6 +75,9 @@ structs: hidden_new: optional: true type: safeBool + unique_new: + optional: true + type: safeBool CollModTimeseries: description: "A type representing the adjustable options on timeseries collections" diff --git a/src/mongo/db/op_observer.h b/src/mongo/db/op_observer.h index 5d0da247203..54f32d09792 100644 --- a/src/mongo/db/op_observer.h +++ b/src/mongo/db/op_observer.h @@ -98,6 +98,7 @@ struct IndexCollModInfo { boost::optional oldExpireAfterSeconds; boost::optional hidden; boost::optional oldHidden; + boost::optional unique; std::string indexName; }; diff --git a/src/mongo/db/op_observer_util.cpp b/src/mongo/db/op_observer_util.cpp index 7d69555618f..8aaddf9e7e3 100644 --- a/src/mongo/db/op_observer_util.cpp +++ b/src/mongo/db/op_observer_util.cpp @@ -59,6 +59,9 @@ BSONObj makeCollModCmdObj(const BSONObj& collModCmd, if (indexInfo->hidden) indexObjBuilder.append("hidden", indexInfo->hidden.get()); + if (indexInfo->unique) + indexObjBuilder.append("unique", indexInfo->unique.get()); + cmdObjBuilder.append(indexFieldName, indexObjBuilder.obj()); } else { cmdObjBuilder.append(elem); -- cgit v1.2.1