diff options
author | Arun Banala <arun.banala@10gen.com> | 2019-11-20 13:47:55 +0000 |
---|---|---|
committer | evergreen <evergreen@mongodb.com> | 2019-11-20 13:47:55 +0000 |
commit | 7a3bdd35b859ac8462e756b909c0a4773195b99a (patch) | |
tree | 24817c974522f06f958cc9f1055a62870d3e3c6f | |
parent | 623e743d049f3330a273abfcee31b2f9d07866da (diff) | |
download | mongo-7a3bdd35b859ac8462e756b909c0a4773195b99a.tar.gz |
SERVER-44571 Documents involved in SERVER-44050 corruption scenario cannot be updated or deleted after upgrade
(cherry picked from commit 35c6778143fc55eb9617ab4a54e616ba1e537ad5)
(cherry picked from commit 6dd33f3f725d8df801603b8f1dcbd7b13a85f1ce)
(cherry picked from commit 30701d77ca133ecd1b184587f61c832b1d028a4b)
22 files changed, 272 insertions, 35 deletions
diff --git a/etc/evergreen.yml b/etc/evergreen.yml index 909dbf903f2..f9f74ee23db 100644 --- a/etc/evergreen.yml +++ b/etc/evergreen.yml @@ -1181,7 +1181,7 @@ functions: --edition ${multiversion_edition|base} \ --platform ${multiversion_platform|linux} \ --architecture ${multiversion_architecture|x86_64} \ - --useLatest 3.0 3.2 3.4 + --useLatest 3.0 3.2 3.4 3.6.14 "do snmp setup" : command: shell.exec diff --git a/jstests/multiVersion/hashed_index_bad_keys_cleanup.js b/jstests/multiVersion/hashed_index_bad_keys_cleanup.js new file mode 100644 index 00000000000..ed1a201e79f --- /dev/null +++ b/jstests/multiVersion/hashed_index_bad_keys_cleanup.js @@ -0,0 +1,120 @@ +/** + * Prior to SERVER-44050, for hashed indexes, if there is an array along index field path, we did + * not fail insertion. We incorrectly stored empty index key value for those cases. This lead to + * corruption of index keys. + * + * In this test we verify that we are able to successfully update and delete documents that were + * involved in creating corrupt indexes. + * + */ +(function() { + "use strict"; + + load("jstests/multiVersion/libs/multi_rs.js"); // For upgradeSet. + load("jstests/multiVersion/libs/verify_versions.js"); // For binVersion. + + const preBackportVersion = "3.6.14"; + const preBackportNodeOptions = {binVersion: preBackportVersion}; + const nodeOptionsOfLatestVersion = {binVersion: "latest"}; + + // Set up a new replSet consisting of 2 nodes, initially running on bad binaries. + const rst = new ReplSetTest({nodes: 2, nodeOptions: preBackportNodeOptions}); + rst.startSet(); + rst.initiate(); + + let testDB = rst.getPrimary().getDB(jsTestName()); + let coll = testDB.coll; + coll.drop(); + + // Verify that the replset is on binary version specified in 'preBackportVersion'. + assert.binVersion(testDB.getMongo(), preBackportVersion); + + // Insert bad documents using older version. + assert.commandWorked(coll.createIndex({"p.q.r": "hashed"})); + assert.writeOK(coll.insert({_id: 1, p: []})); + assert.writeOK(coll.insert({_id: 2, p: {q: [1]}})); + assert.writeOK(coll.insert({_id: 3, p: [{q: 1}]})); + assert.writeOK(coll.insert({_id: 4, a: 1, p: [{q: 1}]})); + assert.writeOK(coll.insert({_id: 5, a: 1, p: [{q: 1}]})); + + // Helper function which runs validate() on primary and secondary nodes, then verifies that the + // command returned the expected result. + function assertValidateCmdReturned(expectedResult) { + const resFromPrimary = assert.commandWorked(coll.validate({full: true})); + assert.eq(resFromPrimary.valid, expectedResult, resFromPrimary); + + rst.awaitReplication(); + const testDBOnSecondary = rst.getSecondary().getDB(jsTestName()); + const resFromSecondary = + assert.commandWorked(testDBOnSecondary.coll.validate({full: true})); + assert.eq(resFromSecondary.valid, expectedResult, resFromSecondary); + } + + // Confirm that validate() does not perceive a problem with the malformed documents. + assertValidateCmdReturned(true); + + // Upgrade the set to the new binary version. + rst.upgradeSet(nodeOptionsOfLatestVersion); + testDB = rst.getPrimary().getDB(jsTestName()); + coll = testDB.coll; + + // Verify that the five documents inserted earlier have their index keys. + let res = coll.find().hint({"p.q.r": "hashed"}).returnKey().itcount(); + assert.eq(res, 5); + + // Verify that after upgrade, inserting bad documents is not allowed. + const arrayAlongPathFailCode = 16766; + assert.writeErrorWithCode(coll.insert({p: []}), arrayAlongPathFailCode); + assert.writeErrorWithCode(coll.insert({p: [{q: 1}]}), arrayAlongPathFailCode); + assert.writeErrorWithCode(coll.insert({p: {q: {r: [3]}}}), arrayAlongPathFailCode); + + // After upgrade, validate() should now fail since there are existing bad documents. + assertValidateCmdReturned(false); + + // Deleting bad documents succeeds. + assert.eq(coll.deleteOne({_id: 1}).deletedCount, 1); + assert.eq(coll.deleteMany({a: 1}).deletedCount, 2); + + // Updating documents to contain array along field path should fail. + assert.writeErrorWithCode(coll.update({_id: 2}, {p: {q: [{r: 1}]}}), arrayAlongPathFailCode); + assert.commandFailedWithCode( + testDB.runCommand( + {findAndModify: coll.getName(), query: {_id: 2}, update: {p: {q: [{r: 1}]}}}), + arrayAlongPathFailCode); + + assert.writeErrorWithCode(coll.update({_id: 2}, {p: {q: {r: [3]}}}), arrayAlongPathFailCode); + const updateRes = testDB.runCommand({ + update: coll.getName(), + updates: [ + {q: {_id: 3}, u: {$set: {p: {q: [{r: 1}]}}}}, + {q: {_id: 2}, u: {$set: {p: {q: [{r: 1}]}}}} + ], + ordered: false + }); + assert.eq(updateRes.writeErrors.length, 2); + assert.eq(updateRes.writeErrors[0].code, arrayAlongPathFailCode); + assert.eq(updateRes.writeErrors[1].code, arrayAlongPathFailCode); + + // Verify that updating to a valid index field works. + assert.writeOK(coll.update({_id: 2}, {p: {q: {r: 4}}})); + + // Verify that the index key is updated correctly by quering with hashed index. + res = coll.find({"p.q.r": 4}).hint({"p.q.r": "hashed"}).toArray(); + assert.eq(res, [{_id: 2, p: {q: {r: 4}}}]); + + // Validate should still fail since a bad document {_id: 3} exists. + assertValidateCmdReturned(false); + + // Delete the last remaining bad document. + assert.eq(coll.deleteOne({_id: 3}).deletedCount, 1); + + // Now that all the bad documents are deleted or updated, verify that validate succeeds. + assertValidateCmdReturned(true); + + // Verify that there is only one index key left (for {_id: 2}). + res = coll.find().hint({"p.q.r": "hashed"}).returnKey().itcount(); + assert.eq(res, 1); + + rst.awaitReplication(); + rst.stopSet(); +}()); diff --git a/src/mongo/db/catalog/private/record_store_validate_adaptor.cpp b/src/mongo/db/catalog/private/record_store_validate_adaptor.cpp index 1ba4fd1a6aa..ccbc0c96670 100644 --- a/src/mongo/db/catalog/private/record_store_validate_adaptor.cpp +++ b/src/mongo/db/catalog/private/record_store_validate_adaptor.cpp @@ -89,6 +89,7 @@ Status RecordStoreValidateAdaptor::validate(const RecordId& recordId, MultikeyPaths* multikeyPaths = nullptr; iam->getKeys(recordBson, IndexAccessMethod::GetKeysMode::kEnforceConstraints, + IndexAccessMethod::GetKeysContext::kReadOrAddKeys, &documentKeySet, multikeyPaths); diff --git a/src/mongo/db/exec/working_set_common.cpp b/src/mongo/db/exec/working_set_common.cpp index ce2763fe00d..16dba8a76d1 100644 --- a/src/mongo/db/exec/working_set_common.cpp +++ b/src/mongo/db/exec/working_set_common.cpp @@ -122,6 +122,7 @@ bool WorkingSetCommon::fetch(OperationContext* opCtx, MultikeyPaths* multikeyPaths = nullptr; member->keyData[i].index->getKeys(member->obj.value(), IndexAccessMethod::GetKeysMode::kEnforceConstraints, + IndexAccessMethod::GetKeysContext::kReadOrAddKeys, &keys, multikeyPaths); if (!keys.count(member->keyData[i].keyData)) { diff --git a/src/mongo/db/index/2d_access_method.cpp b/src/mongo/db/index/2d_access_method.cpp index c6df95e5fba..55b7a2d7159 100644 --- a/src/mongo/db/index/2d_access_method.cpp +++ b/src/mongo/db/index/2d_access_method.cpp @@ -51,6 +51,7 @@ TwoDAccessMethod::TwoDAccessMethod(IndexCatalogEntry* btreeState, SortedDataInte /** Finds the key objects to put in an index */ void TwoDAccessMethod::doGetKeys(const BSONObj& obj, + GetKeysContext context, BSONObjSet* keys, MultikeyPaths* multikeyPaths) const { ExpressionKeysPrivate::get2DKeys(obj, _params, keys); diff --git a/src/mongo/db/index/2d_access_method.h b/src/mongo/db/index/2d_access_method.h index 4a889fc676c..dc23a36167a 100644 --- a/src/mongo/db/index/2d_access_method.h +++ b/src/mongo/db/index/2d_access_method.h @@ -59,7 +59,10 @@ private: * This function ignores the 'multikeyPaths' pointer because 2d indexes don't support tracking * path-level multikey information. */ - void doGetKeys(const BSONObj& obj, BSONObjSet* keys, MultikeyPaths* multikeyPaths) const final; + void doGetKeys(const BSONObj& obj, + GetKeysContext context, + BSONObjSet* keys, + MultikeyPaths* multikeyPaths) const final; TwoDIndexingParams _params; }; diff --git a/src/mongo/db/index/btree_access_method.cpp b/src/mongo/db/index/btree_access_method.cpp index 0a5811be033..bddb464bf06 100644 --- a/src/mongo/db/index/btree_access_method.cpp +++ b/src/mongo/db/index/btree_access_method.cpp @@ -65,6 +65,7 @@ BtreeAccessMethod::BtreeAccessMethod(IndexCatalogEntry* btreeState, SortedDataIn } void BtreeAccessMethod::doGetKeys(const BSONObj& obj, + GetKeysContext context, BSONObjSet* keys, MultikeyPaths* multikeyPaths) const { _keyGenerator->getKeys(obj, keys, multikeyPaths); diff --git a/src/mongo/db/index/btree_access_method.h b/src/mongo/db/index/btree_access_method.h index e84f48e4faa..d596c318601 100644 --- a/src/mongo/db/index/btree_access_method.h +++ b/src/mongo/db/index/btree_access_method.h @@ -50,7 +50,10 @@ public: BtreeAccessMethod(IndexCatalogEntry* btreeState, SortedDataInterface* btree); private: - void doGetKeys(const BSONObj& obj, BSONObjSet* keys, MultikeyPaths* multikeyPaths) const final; + void doGetKeys(const BSONObj& obj, + GetKeysContext context, + BSONObjSet* keys, + MultikeyPaths* multikeyPaths) const final; // Our keys differ for V0 and V1. std::unique_ptr<BtreeKeyGenerator> _keyGenerator; diff --git a/src/mongo/db/index/expression_keys_private.cpp b/src/mongo/db/index/expression_keys_private.cpp index 424db842960..6b595c4c790 100644 --- a/src/mongo/db/index/expression_keys_private.cpp +++ b/src/mongo/db/index/expression_keys_private.cpp @@ -345,20 +345,35 @@ void ExpressionKeysPrivate::getHashKeys(const BSONObj& obj, int hashVersion, bool isSparse, const CollatorInterface* collator, - BSONObjSet* keys) { + BSONObjSet* keys, + bool ignoreArraysAlongPath) { + static const BSONObj nullObj = BSON("" << BSONNULL); const char* cstr = hashedField.c_str(); BSONElement fieldVal = dps::extractElementAtPathOrArrayAlongPath(obj, cstr); // If we hit an array while traversing the path, 'cstr' will point to the path component - // immediately following the array, or the null termination byte if the final field in the path - // was an array. - uassert( - 16766, - str::stream() - << "Error: hashed indexes do not currently support array values. Found array at path: " - << hashedField.substr( - 0, hashedField.size() - StringData(cstr).size() - !StringData(cstr).empty()), - fieldVal.type() != BSONType::Array); + // immediately following the array, or the null termination byte if the terminal path + // component was an array. In the latter case, 'remainingPath' will be empty. + auto remainingPath = StringData(cstr); + + // If 'ignoreArraysAlongPath' is set, we want to use the behaviour prior to SERVER-44050, + // which is to allow arrays along the field path (except the terminal path). This is done so + // that the document keys inserted prior to SERVER-44050 can be deleted or updated after the + // upgrade, allowing users to recover from the possible index corruption. The old behaviour + // before SERVER-44050 was to store 'null' index key if we encountered an array along the + // index field path. We will use the same logic in the context of removing index keys. + if (ignoreArraysAlongPath && fieldVal.type() == BSONType::Array && !remainingPath.empty()) { + fieldVal = nullObj.firstElement(); + } + + // Otherwise, throw if an array was encountered at any point along the path. + uassert(16766, + str::stream() << "Error: hashed indexes do not currently support array values. " + "Found array at path: " + << hashedField.substr(0, + hashedField.size() - remainingPath.size() - + !remainingPath.empty()), + fieldVal.type() != BSONType::Array); // Convert strings to comparison keys. BSONObj fieldValObj; @@ -370,7 +385,6 @@ void ExpressionKeysPrivate::getHashKeys(const BSONObj& obj, BSONObj key = BSON("" << makeSingleHashKey(fieldVal, seed, hashVersion)); keys->insert(key); } else if (!isSparse) { - BSONObj nullObj = BSON("" << BSONNULL); keys->insert(BSON("" << makeSingleHashKey(nullObj.firstElement(), seed, hashVersion))); } } diff --git a/src/mongo/db/index/expression_keys_private.h b/src/mongo/db/index/expression_keys_private.h index 87d48aaaded..8fc8e612cba 100644 --- a/src/mongo/db/index/expression_keys_private.h +++ b/src/mongo/db/index/expression_keys_private.h @@ -81,7 +81,8 @@ public: int hashVersion, bool isSparse, const CollatorInterface* collator, - BSONObjSet* keys); + BSONObjSet* keys, + bool ignoreArraysAlongPath); /** * Hashing function used by both getHashKeys and the cursors we create. diff --git a/src/mongo/db/index/fts_access_method.cpp b/src/mongo/db/index/fts_access_method.cpp index 08ab9c11933..27164bfdc97 100644 --- a/src/mongo/db/index/fts_access_method.cpp +++ b/src/mongo/db/index/fts_access_method.cpp @@ -39,6 +39,7 @@ FTSAccessMethod::FTSAccessMethod(IndexCatalogEntry* btreeState, SortedDataInterf : IndexAccessMethod(btreeState, btree), _ftsSpec(btreeState->descriptor()->infoObj()) {} void FTSAccessMethod::doGetKeys(const BSONObj& obj, + GetKeysContext context, BSONObjSet* keys, MultikeyPaths* multikeyPaths) const { ExpressionKeysPrivate::getFTSKeys(obj, _ftsSpec, keys); diff --git a/src/mongo/db/index/fts_access_method.h b/src/mongo/db/index/fts_access_method.h index cbf254207ba..a200c4451d4 100644 --- a/src/mongo/db/index/fts_access_method.h +++ b/src/mongo/db/index/fts_access_method.h @@ -53,7 +53,10 @@ private: * This function ignores the 'multikeyPaths' pointer because text indexes don't support tracking * path-level multikey information. */ - void doGetKeys(const BSONObj& obj, BSONObjSet* keys, MultikeyPaths* multikeyPaths) const final; + void doGetKeys(const BSONObj& obj, + GetKeysContext context, + BSONObjSet* keys, + MultikeyPaths* multikeyPaths) const final; fts::FTSSpec _ftsSpec; }; diff --git a/src/mongo/db/index/hash_access_method.cpp b/src/mongo/db/index/hash_access_method.cpp index af3382a60c1..d7b79f1c0c9 100644 --- a/src/mongo/db/index/hash_access_method.cpp +++ b/src/mongo/db/index/hash_access_method.cpp @@ -56,10 +56,17 @@ HashAccessMethod::HashAccessMethod(IndexCatalogEntry* btreeState, SortedDataInte } void HashAccessMethod::doGetKeys(const BSONObj& obj, + GetKeysContext context, BSONObjSet* keys, MultikeyPaths* multikeyPaths) const { - ExpressionKeysPrivate::getHashKeys( - obj, _hashedField, _seed, _hashVersion, _descriptor->isSparse(), _collator, keys); + ExpressionKeysPrivate::getHashKeys(obj, + _hashedField, + _seed, + _hashVersion, + _descriptor->isSparse(), + _collator, + keys, + (context == GetKeysContext::kRemovingKeys)); } } // namespace mongo diff --git a/src/mongo/db/index/hash_access_method.h b/src/mongo/db/index/hash_access_method.h index 80d0eec4500..83dd52b8a29 100644 --- a/src/mongo/db/index/hash_access_method.h +++ b/src/mongo/db/index/hash_access_method.h @@ -56,7 +56,10 @@ private: * This function ignores the 'multikeyPaths' pointer because hashed indexes don't support * tracking path-level multikey information. */ - void doGetKeys(const BSONObj& obj, BSONObjSet* keys, MultikeyPaths* multikeyPaths) const final; + void doGetKeys(const BSONObj& obj, + GetKeysContext context, + BSONObjSet* keys, + MultikeyPaths* multikeyPaths) const final; // Only one of our fields is hashed. This is the field name for it. std::string _hashedField; diff --git a/src/mongo/db/index/hash_key_generator_test.cpp b/src/mongo/db/index/hash_key_generator_test.cpp index cd1ac3acace..9354a9a9507 100644 --- a/src/mongo/db/index/hash_key_generator_test.cpp +++ b/src/mongo/db/index/hash_key_generator_test.cpp @@ -90,7 +90,7 @@ TEST(HashKeyGeneratorTest, CollationAppliedBeforeHashing) { BSONObjSet actualKeys = SimpleBSONObjComparator::kInstance.makeBSONObjSet(); CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString); ExpressionKeysPrivate::getHashKeys( - obj, "a", kHashSeed, kHashVersion, false, &collator, &actualKeys); + obj, "a", kHashSeed, kHashVersion, false, &collator, &actualKeys, false); BSONObj backwardsObj = fromjson("{a: 'gnirts'}"); BSONObjSet expectedKeys = SimpleBSONObjComparator::kInstance.makeBSONObjSet(); @@ -104,7 +104,7 @@ TEST(HashKeyGeneratorTest, CollationDoesNotAffectNonStringFields) { BSONObjSet actualKeys = SimpleBSONObjComparator::kInstance.makeBSONObjSet(); CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString); ExpressionKeysPrivate::getHashKeys( - obj, "a", kHashSeed, kHashVersion, false, &collator, &actualKeys); + obj, "a", kHashSeed, kHashVersion, false, &collator, &actualKeys, false); BSONObjSet expectedKeys = SimpleBSONObjComparator::kInstance.makeBSONObjSet(); expectedKeys.insert(makeHashKey(obj["a"])); @@ -118,7 +118,7 @@ TEST(HashKeyGeneratorTest, CollatorAppliedBeforeHashingNestedObject) { BSONObjSet actualKeys = SimpleBSONObjComparator::kInstance.makeBSONObjSet(); CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString); ExpressionKeysPrivate::getHashKeys( - obj, "a", kHashSeed, kHashVersion, false, &collator, &actualKeys); + obj, "a", kHashSeed, kHashVersion, false, &collator, &actualKeys, false); BSONObjSet expectedKeys = SimpleBSONObjComparator::kInstance.makeBSONObjSet(); expectedKeys.insert(makeHashKey(backwardsObj["a"])); @@ -130,7 +130,7 @@ TEST(HashKeyGeneratorTest, NoCollation) { BSONObj obj = fromjson("{a: 'string'}"); BSONObjSet actualKeys = SimpleBSONObjComparator::kInstance.makeBSONObjSet(); ExpressionKeysPrivate::getHashKeys( - obj, "a", kHashSeed, kHashVersion, false, nullptr, &actualKeys); + obj, "a", kHashSeed, kHashVersion, false, nullptr, &actualKeys, false); BSONObjSet expectedKeys = SimpleBSONObjComparator::kInstance.makeBSONObjSet(); expectedKeys.insert(makeHashKey(obj["a"])); @@ -138,4 +138,48 @@ TEST(HashKeyGeneratorTest, NoCollation) { ASSERT(assertKeysetsEqual(expectedKeys, actualKeys)); } +TEST(HashKeyGeneratorTest, ArrayAlongIndexFieldPathFails) { + BSONObj obj = fromjson("{a: []}"); + BSONObjSet actualKeys = SimpleBSONObjComparator::kInstance.makeBSONObjSet(); + ASSERT_THROWS_CODE( + ExpressionKeysPrivate::getHashKeys( + obj, "a.b.c", kHashSeed, kHashVersion, false, nullptr, &actualKeys, false), + DBException, + 16766); +} + +TEST(HashKeyGeneratorTest, ArrayAlongIndexFieldPathDoesNotFailWhenIgnoreFlagIsSet) { + BSONObj obj = fromjson("{a: []}"); + BSONObjSet actualKeys = SimpleBSONObjComparator::kInstance.makeBSONObjSet(); + ExpressionKeysPrivate::getHashKeys(obj, + "a.b.c", + kHashSeed, + kHashVersion, + false, + nullptr, + &actualKeys, + true // ignoreArraysAlongPath + ); + + BSONObjSet expectedKeys = SimpleBSONObjComparator::kInstance.makeBSONObjSet(); + expectedKeys.insert(makeHashKey(BSON("" << BSONNULL).firstElement())); + ASSERT(assertKeysetsEqual(expectedKeys, actualKeys)); +} + +TEST(HashKeyGeneratorTest, ArrayAtTerminalPathAlwaysFails) { + BSONObj obj = fromjson("{a : {b: {c: [1]}}}"); + BSONObjSet actualKeys = SimpleBSONObjComparator::kInstance.makeBSONObjSet(); + ASSERT_THROWS_CODE(ExpressionKeysPrivate::getHashKeys(obj, + "a.b.c", + kHashSeed, + kHashVersion, + true, // isSparse + nullptr, + &actualKeys, + true // ignoreArraysAlongPath + ), + DBException, + 16766); +} + } // namespace diff --git a/src/mongo/db/index/haystack_access_method.cpp b/src/mongo/db/index/haystack_access_method.cpp index 050b375a3a7..06a07c27533 100644 --- a/src/mongo/db/index/haystack_access_method.cpp +++ b/src/mongo/db/index/haystack_access_method.cpp @@ -66,6 +66,7 @@ HaystackAccessMethod::HaystackAccessMethod(IndexCatalogEntry* btreeState, } void HaystackAccessMethod::doGetKeys(const BSONObj& obj, + GetKeysContext context, BSONObjSet* keys, MultikeyPaths* multikeyPaths) const { ExpressionKeysPrivate::getHaystackKeys(obj, _geoField, _otherFields, _bucketSize, keys); diff --git a/src/mongo/db/index/haystack_access_method.h b/src/mongo/db/index/haystack_access_method.h index f4d743c1021..a15933c98f6 100644 --- a/src/mongo/db/index/haystack_access_method.h +++ b/src/mongo/db/index/haystack_access_method.h @@ -77,7 +77,10 @@ private: * This function ignores the 'multikeyPaths' pointer because geoHaystack indexes don't support * tracking path-level multikey information. */ - void doGetKeys(const BSONObj& obj, BSONObjSet* keys, MultikeyPaths* multikeyPaths) const final; + void doGetKeys(const BSONObj& obj, + GetKeysContext context, + BSONObjSet* keys, + MultikeyPaths* multikeyPaths) const final; std::string _geoField; std::vector<std::string> _otherFields; diff --git a/src/mongo/db/index/index_access_method.cpp b/src/mongo/db/index/index_access_method.cpp index 38e727bdf71..ed6e0a9d206 100644 --- a/src/mongo/db/index/index_access_method.cpp +++ b/src/mongo/db/index/index_access_method.cpp @@ -136,7 +136,7 @@ Status IndexAccessMethod::insert(OperationContext* opCtx, BSONObjSet keys = SimpleBSONObjComparator::kInstance.makeBSONObjSet(); MultikeyPaths multikeyPaths; // Delegate to the subclass. - getKeys(obj, options.getKeysMode, &keys, &multikeyPaths); + getKeys(obj, options.getKeysMode, GetKeysContext::kReadOrAddKeys, &keys, &multikeyPaths); Status ret = Status::OK(); for (BSONObjSet::const_iterator i = keys.begin(); i != keys.end(); ++i) { @@ -220,7 +220,7 @@ Status IndexAccessMethod::remove(OperationContext* opCtx, // multikey when removing a document since the index metadata isn't updated when keys are // deleted. MultikeyPaths* multikeyPaths = nullptr; - getKeys(obj, options.getKeysMode, &keys, multikeyPaths); + getKeys(obj, options.getKeysMode, GetKeysContext::kRemovingKeys, &keys, multikeyPaths); for (BSONObjSet::const_iterator i = keys.begin(); i != keys.end(); ++i) { removeOneKey(opCtx, *i, loc, options.dupsAllowed); @@ -239,7 +239,11 @@ Status IndexAccessMethod::touch(OperationContext* opCtx, const BSONObj& obj) { // There's no need to compute the prefixes of the indexed fields that cause the index to be // multikey when paging a document's index entries into memory. MultikeyPaths* multikeyPaths = nullptr; - getKeys(obj, GetKeysMode::kEnforceConstraints, &keys, multikeyPaths); + getKeys(obj, + GetKeysMode::kEnforceConstraints, + GetKeysContext::kReadOrAddKeys, + &keys, + multikeyPaths); std::unique_ptr<SortedDataInterface::Cursor> cursor(_newInterface->newCursor(opCtx)); for (BSONObjSet::const_iterator i = keys.begin(); i != keys.end(); ++i) { @@ -261,7 +265,11 @@ RecordId IndexAccessMethod::findSingle(OperationContext* opCtx, const BSONObj& r // For performance, call get keys only if there is a non-simple collation. BSONObjSet keys = SimpleBSONObjComparator::kInstance.makeBSONObjSet(); MultikeyPaths* multikeyPaths = nullptr; - getKeys(requestedKey, GetKeysMode::kEnforceConstraints, &keys, multikeyPaths); + getKeys(requestedKey, + GetKeysMode::kEnforceConstraints, + GetKeysContext::kReadOrAddKeys, + &keys, + multikeyPaths); invariant(keys.size() == 1); actualKey = *keys.begin(); } else { @@ -349,11 +357,19 @@ Status IndexAccessMethod::validateUpdate(OperationContext* opCtx, // index to be multikey when the old version of the document was written since the index // metadata isn't updated when keys are deleted. MultikeyPaths* multikeyPaths = nullptr; - getKeys(from, options.getKeysMode, &ticket->oldKeys, multikeyPaths); + getKeys(from, + options.getKeysMode, + GetKeysContext::kRemovingKeys, + &ticket->oldKeys, + multikeyPaths); } if (!indexFilter || indexFilter->matchesBSON(to)) { - getKeys(to, options.getKeysMode, &ticket->newKeys, &ticket->newMultikeyPaths); + getKeys(to, + options.getKeysMode, + GetKeysContext::kReadOrAddKeys, + &ticket->newKeys, + &ticket->newMultikeyPaths); } ticket->loc = record; @@ -440,7 +456,7 @@ Status IndexAccessMethod::BulkBuilder::insert(OperationContext* opCtx, BSONObjSet keys = SimpleBSONObjComparator::kInstance.makeBSONObjSet(); MultikeyPaths multikeyPaths; - _real->getKeys(obj, options.getKeysMode, &keys, &multikeyPaths); + _real->getKeys(obj, options.getKeysMode, GetKeysContext::kReadOrAddKeys, &keys, &multikeyPaths); _everGeneratedMultipleKeys = _everGeneratedMultipleKeys || (keys.size() > 1); @@ -555,6 +571,7 @@ Status IndexAccessMethod::commitBulk(OperationContext* opCtx, void IndexAccessMethod::getKeys(const BSONObj& obj, GetKeysMode mode, + GetKeysContext context, BSONObjSet* keys, MultikeyPaths* multikeyPaths) const { static stdx::unordered_set<int> whiteList{ErrorCodes::CannotBuildIndexKeys, @@ -582,7 +599,7 @@ void IndexAccessMethod::getKeys(const BSONObj& obj, 13026, 13027}; try { - doGetKeys(obj, keys, multikeyPaths); + doGetKeys(obj, context, keys, multikeyPaths); } catch (const AssertionException& ex) { if (mode == GetKeysMode::kEnforceConstraints) { throw; diff --git a/src/mongo/db/index/index_access_method.h b/src/mongo/db/index/index_access_method.h index d552e9a91f1..64fac5faf2d 100644 --- a/src/mongo/db/index/index_access_method.h +++ b/src/mongo/db/index/index_access_method.h @@ -262,6 +262,12 @@ public: enum class GetKeysMode { kRelaxConstraints, kEnforceConstraints }; /** + * Specifies whether getKeys is being used in the context of creating new keys or deleting + * existing keys. + */ + enum class GetKeysContext { kRemovingKeys, kReadOrAddKeys }; + + /** * Fills 'keys' with the keys that should be generated for 'obj' on this index. * Based on 'mode', it will honor or ignore index constraints, e.g. duplicated key, key too * long, and geo index parsing errors. The ignoring of constraints is for replication due to @@ -275,6 +281,7 @@ public: */ void getKeys(const BSONObj& obj, GetKeysMode mode, + GetKeysContext context, BSONObjSet* keys, MultikeyPaths* multikeyPaths) const; @@ -300,6 +307,7 @@ protected: * a result of inserting 'keys'. */ virtual void doGetKeys(const BSONObj& obj, + GetKeysContext context, BSONObjSet* keys, MultikeyPaths* multikeyPaths) const = 0; diff --git a/src/mongo/db/index/s2_access_method.cpp b/src/mongo/db/index/s2_access_method.cpp index b86b08fd97e..0f414aaa242 100644 --- a/src/mongo/db/index/s2_access_method.cpp +++ b/src/mongo/db/index/s2_access_method.cpp @@ -143,6 +143,7 @@ StatusWith<BSONObj> S2AccessMethod::fixSpec(const BSONObj& specObj) { } void S2AccessMethod::doGetKeys(const BSONObj& obj, + GetKeysContext context, BSONObjSet* keys, MultikeyPaths* multikeyPaths) const { ExpressionKeysPrivate::getS2Keys(obj, _descriptor->keyPattern(), _params, keys, multikeyPaths); diff --git a/src/mongo/db/index/s2_access_method.h b/src/mongo/db/index/s2_access_method.h index 52153b73f65..dc45f33f6e3 100644 --- a/src/mongo/db/index/s2_access_method.h +++ b/src/mongo/db/index/s2_access_method.h @@ -65,7 +65,10 @@ private: * and fills each element with the prefixes of the indexed field that would cause this index to * be multikey as a result of inserting 'keys'. */ - void doGetKeys(const BSONObj& obj, BSONObjSet* keys, MultikeyPaths* multikeyPaths) const final; + void doGetKeys(const BSONObj& obj, + GetKeysContext context, + BSONObjSet* keys, + MultikeyPaths* multikeyPaths) const final; S2IndexingParams _params; diff --git a/src/mongo/dbtests/namespacetests.cpp b/src/mongo/dbtests/namespacetests.cpp index 3eaae091f78..e21750630f7 100644 --- a/src/mongo/dbtests/namespacetests.cpp +++ b/src/mongo/dbtests/namespacetests.cpp @@ -105,7 +105,8 @@ public: // Call getKeys on the nullObj. BSONObjSet nullFieldKeySet = SimpleBSONObjComparator::kInstance.makeBSONObjSet(); const CollatorInterface* collator = nullptr; - ExpressionKeysPrivate::getHashKeys(nullObj, "a", 0, 0, false, collator, &nullFieldKeySet); + ExpressionKeysPrivate::getHashKeys( + nullObj, "a", 0, 0, false, collator, &nullFieldKeySet, false); BSONElement nullFieldFromKey = nullFieldKeySet.begin()->firstElement(); ASSERT_EQUALS(ExpressionKeysPrivate::makeSingleHashKey(nullObj.firstElement(), 0, 0), @@ -135,7 +136,7 @@ public: BSONObjSet nullFieldKeySet = SimpleBSONObjComparator::kInstance.makeBSONObjSet(); const CollatorInterface* collator = nullptr; ExpressionKeysPrivate::getHashKeys( - nullObj, "a", 0x5eed, 0, false, collator, &nullFieldKeySet); + nullObj, "a", 0x5eed, 0, false, collator, &nullFieldKeySet, false); BSONElement nullFieldFromKey = nullFieldKeySet.begin()->firstElement(); ASSERT_EQUALS(ExpressionKeysPrivate::makeSingleHashKey(nullObj.firstElement(), 0x5eed, 0), |