mongodb/mongo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

SERVER-120174 Ensure 'bits' index parameter is stored as integer type on-disk (#48531)

Browse Source 
GitOrigin-RevId: ac0f529e5d38f27be0995b9e88d8f9df772e9d65
This commit is contained in:
Silvia Surroca 2026-03-18 10:51:27 +01:00 committed by MongoDB Bot
parent 53a0fec37a
commit 0518a1b5aa
13 changed files with 431 additions and 134 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

236
jstests/core/ddl/create_indexes.js
View File

@ -1,122 +1,176 @@
/**
* Tests that createIndexes enforces index spec validation, and correctly updates the catalog on
* success while leaving it unchanged on failure.
*
* @tags: [
* assumes_superuser_permissions,
* # simulate_atlas_proxy.js can't simulate req on config.transaction as tested
* simulate_atlas_proxy_incompatible,
* ]
*/
import {afterEach, beforeEach, describe, it} from "jstests/libs/mochalite.js";
import {IndexUtils} from "jstests/libs/index_utils.js";
const dbTest = db.getSiblingDB("create_indexes_db");
dbTest.dropDatabase();
const dbName = jsTestName();
const testDb = db.getSiblingDB(dbName);
const collName = "collTest";
const coll = testDb.getCollection(collName);
const t = dbTest.create_indexes;
dbTest.createCollection(t.getName());
const isMultiversion =
Boolean(jsTest.options().useRandomBinVersionsWithinReplicaSet) || Boolean(TestData.multiversionBinVersion);
// Test that index creation fails with an empty list of specs.
let res = t.runCommand("createIndexes", {indexes: []});
assert.commandFailedWithCode(res, ErrorCodes.BadValue);
describe("createIndexes", function () {
beforeEach(function () {
testDb.dropDatabase();
assert.commandWorked(testDb.createCollection(collName));
});
// Test that index creation fails on specs that are missing required fields such as 'key'.
res = t.runCommand("createIndexes", {indexes: [{}]});
assert.commandFailedWithCode(res, ErrorCodes.FailedToParse);
describe("malformed index specs", function () {
afterEach(function () {
// Ensure that no indexes were created
IndexUtils.assertIndexes(coll, [{_id: 1}]);
});
// Test that any malformed specs in the list causes the entire index creation to fail and
// will not result in new indexes in the catalog.
res = t.runCommand("createIndexes", {indexes: [{}, {key: {m: 1}, name: "asd"}]});
assert.commandFailedWithCode(res, ErrorCodes.FailedToParse);
it("fails with an empty list of index specs", function () {
const res = coll.runCommand("createIndexes", {indexes: []});
assert.commandFailedWithCode(res, ErrorCodes.BadValue);
});
IndexUtils.assertIndexes(t, [{_id: 1}]);
it("fails when the 'key' field is missing from an index spec", function () {
const res = coll.runCommand("createIndexes", {indexes: [{}]});
assert.commandFailedWithCode(res, ErrorCodes.FailedToParse);
});
res = t.runCommand("createIndexes", {indexes: [{key: {"c": 1}, sparse: true, name: "c_1"}]});
IndexUtils.assertIndexes(t, [{_id: 1}, {c: 1}]);
assert.eq(
1,
t.getIndexes().filter(function (z) {
return z.sparse;
}).length,
);
it("does not create any indexes when any spec in the list is malformed", function () {
const res = coll.runCommand("createIndexes", {indexes: [{}, {key: {m: 1}, name: "asd"}]});
assert.commandFailedWithCode(res, ErrorCodes.FailedToParse);
});
// Test that index creation fails if we specify an unsupported index type.
res = t.runCommand("createIndexes", {indexes: [{key: {"x": "invalid_index_type"}, name: "x_1"}]});
assert.commandFailedWithCode(res, ErrorCodes.CannotCreateIndex);
it("fails with an unsupported index type", function () {
const res = coll.runCommand("createIndexes", {indexes: [{key: {x: "invalid_index_type"}, name: "x_1"}]});
assert.commandFailedWithCode(res, ErrorCodes.CannotCreateIndex);
});
IndexUtils.assertIndexes(t, [{_id: 1}, {c: 1}]);
it("fails when the index name is empty", function () {
const res = coll.runCommand("createIndexes", {indexes: [{key: {x: 1}, name: ""}]});
assert.commandFailedWithCode(res, ErrorCodes.CannotCreateIndex);
});
// Test that an index name, if provided by the user, cannot be empty.
res = t.runCommand("createIndexes", {indexes: [{key: {"x": 1}, name: ""}]});
assert.commandFailedWithCode(res, ErrorCodes.CannotCreateIndex);
it("fails with index version v0", function () {
const res = coll.runCommand("createIndexes", {indexes: [{key: {d: 1}, name: "d_1", v: 0}]});
assert.commandFailed(res, "v0 index creation should fail");
});
IndexUtils.assertIndexes(t, [{_id: 1}, {c: 1}]);
it("fails with an invalid top-level field", function () {
const res = coll.runCommand("createIndexes", {indexes: [{key: {e: 1}, name: "e_1"}], invalidField: 1});
assert.commandFailedWithCode(res, ErrorCodes.IDLUnknownField);
});
// Test that v0 indexes cannot be created.
res = t.runCommand("createIndexes", {indexes: [{key: {d: 1}, name: "d_1", v: 0}]});
assert.commandFailed(res, "v0 index creation should fail");
it("fails with an invalid field in an index spec (version V2)", function () {
const res = coll.runCommand("createIndexes", {
indexes: [{key: {e: 1}, name: "e_1", v: 2, invalidField: 1}],
});
assert.commandFailedWithCode(res, ErrorCodes.InvalidIndexSpecificationOption);
});
IndexUtils.assertIndexes(t, [{_id: 1}, {c: 1}]);
it("fails with an invalid field in an index spec (version V1)", function () {
const res = coll.runCommand("createIndexes", {
indexes: [{key: {e: 1}, name: "e_1", v: 1, invalidField: 1}],
});
assert.commandFailedWithCode(res, ErrorCodes.InvalidIndexSpecificationOption);
});
// Test that v1 indexes can be created explicitly.
res = t.runCommand("createIndexes", {indexes: [{key: {d: 1}, name: "d_1", v: 1}]});
assert.commandWorked(res, "v1 index creation should succeed");
it("fails with an index named '*'", function () {
const res = coll.runCommand("createIndexes", {indexes: [{key: {star: 1}, name: "*"}]});
assert.commandFailedWithCode(res, ErrorCodes.BadValue);
});
IndexUtils.assertIndexes(t, [{_id: 1}, {c: 1}, {d: 1}]);
it("fails when an index key value is an empty string", function () {
const res = coll.runCommand("createIndexes", {indexes: [{key: {f: ""}, name: "f_1"}]});
assert.commandFailedWithCode(res, ErrorCodes.CannotCreateIndex);
});
// Test that index creation fails with an invalid top-level field.
res = t.runCommand("createIndexes", {indexes: [{key: {e: 1}, name: "e_1"}], "invalidField": 1});
assert.commandFailedWithCode(res, ErrorCodes.IDLUnknownField);
it("fails with duplicate index names in the same request", function () {
const res = coll.runCommand("createIndexes", {
indexes: [
{key: {g: 1}, name: "myidx"},
{key: {h: 1}, name: "myidx"},
],
});
assert.commandFailedWithCode(res, ErrorCodes.IndexKeySpecsConflict);
});
});
// Test that index creation fails with an invalid field in the index spec for index version V2.
res = t.runCommand("createIndexes", {indexes: [{key: {e: 1}, name: "e_1", "v": 2, "invalidField": 1}]});
assert.commandFailedWithCode(res, ErrorCodes.InvalidIndexSpecificationOption);
it("successfully creates a sparse index and updates the catalog", function () {
assert.commandWorked(coll.runCommand("createIndexes", {indexes: [{key: {c: 1}, sparse: true, name: "c_1"}]}));
IndexUtils.assertIndexes(coll, [{_id: 1}, {c: 1}]);
assert.eq(1, coll.getIndexes().filter((z) => z.sparse).length);
});
// Test that index creation fails with an invalid field in the index spec for index version V1.
res = t.runCommand("createIndexes", {indexes: [{key: {e: 1}, name: "e_1", "v": 1, "invalidField": 1}]});
assert.commandFailedWithCode(res, ErrorCodes.InvalidIndexSpecificationOption);
it("successfully creates a v1 index explicitly", function () {
assert.commandWorked(
coll.runCommand("createIndexes", {indexes: [{key: {d: 1}, name: "d_1", v: 1}]}),
"v1 index creation should succeed",
);
IndexUtils.assertIndexes(coll, [{_id: 1}, {d: 1}]);
});
IndexUtils.assertIndexes(t, [{_id: 1}, {c: 1}, {d: 1}]);
it("createIndexes on a view fails with CollectionUUIDMismatch when collectionUUID is provided", function () {
assert.commandWorked(testDb.createView("toApple", "apple", []));
const res = testDb.runCommand({
createIndexes: "toApple",
collectionUUID: UUID(),
indexes: [{name: "_id_hashed", key: {_id: "hashed"}}],
});
assert.commandFailedWithCode(res, ErrorCodes.CollectionUUIDMismatch);
// Test that index creation fails with an index named '*'.
res = t.runCommand("createIndexes", {indexes: [{key: {star: 1}, name: "*"}]});
assert.commandFailedWithCode(res, ErrorCodes.BadValue);
testDb.getCollection("toApple").drop();
});
// Test that index creation fails with an index value of empty string.
res = t.runCommand("createIndexes", {indexes: [{key: {f: ""}, name: "f_1"}]});
assert.commandFailedWithCode(res, ErrorCodes.CannotCreateIndex);
it("createIndexes on a view fails with CommandNotSupportedOnView when no collectionUUID is provided", function () {
assert.commandWorked(testDb.createView("toApple", "apple", []));
const res = testDb.runCommand({
createIndexes: "toApple",
indexes: [{name: "_id_hashed", key: {_id: "hashed"}}],
});
assert.commandFailedWithCode(res, ErrorCodes.CommandNotSupportedOnView);
});
// Test that index creation fails with duplicate index names in the index specs.
res = t.runCommand("createIndexes", {
indexes: [
{key: {g: 1}, name: "myidx"},
{key: {h: 1}, name: "myidx"},
],
describe("User is not allowed to create indexes in config.transactions", function () {
it("createIndexes on config.transactions fails with IllegalOperation", function () {
const configDB = db.getSiblingDB("config");
const res = configDB.runCommand({
createIndexes: "transactions",
indexes: [{key: {star: 1}, name: "star"}],
});
assert.commandFailedWithCode(res, ErrorCodes.IllegalOperation);
});
it("createIndexes on config.transactions fails with IllegalOperation even with an empty index list", function () {
const configDB = db.getSiblingDB("config");
const res = configDB.runCommand({createIndexes: "transactions", indexes: []});
assert.commandFailedWithCode(res, ErrorCodes.IllegalOperation);
});
});
describe("Bits parameter must be stored as an integer", function () {
it("bits parameter is stored as an integer", function () {
// TODO SERVER-120350: Remove this once v9.0 becomes last LTS
if (isMultiversion) {
jsTestLog(
"Skipping test when running on mixed binary versions because the bits parameter may have been stored as a non-int",
);
return;
}
assert.commandWorked(
coll.runCommand("createIndexes", {indexes: [{key: {loc: "2d"}, name: "loc_2d", bits: 11.6}]}),
);
IndexUtils.assertIndexExists(coll, {loc: "2d"}, {bits: 11});
assert(
!IndexUtils.indexExists(coll, {loc: "2d"}, {bits: 11.6}),
"index with non-int bits should not exist",
);
});
});
});
assert.commandFailedWithCode(res, ErrorCodes.IndexKeySpecsConflict);
IndexUtils.assertIndexes(t, [{_id: 1}, {c: 1}, {d: 1}]);
// Test that creating an index on a view fails with CollectionUUIDMismatch if a collection UUID is
// provided. CollectionUUIDMismatch has to prevail over CommandNotSupportedOnView for mongosync.
assert.commandWorked(db.createView("toApple", "apple", []));
res = db.runCommand({
createIndexes: "toApple",
collectionUUID: UUID(),
indexes: [{name: "_id_hashed", key: {_id: "hashed"}}],
});
assert.commandFailedWithCode(res, ErrorCodes.CollectionUUIDMismatch);
// Test that creating an index on a view fails with CommandNotSupportedOnView if a collection UUID
// is not provided
assert.commandWorked(db.createView("toApple", "apple", []));
res = db.runCommand({createIndexes: "toApple", indexes: [{name: "_id_hashed", key: {_id: "hashed"}}]});
assert.commandFailedWithCode(res, ErrorCodes.CommandNotSupportedOnView);
// Test that user is not allowed to create indexes in config.transactions.
const configDB = db.getSiblingDB("config");
res = configDB.runCommand({createIndexes: "transactions", indexes: [{key: {star: 1}, name: "star"}]});
assert.commandFailedWithCode(res, ErrorCodes.IllegalOperation);
// Test that providing an empty list of index spec for config.transactions should also fail with
// IllegalOperation, rather than BadValue for a normal collection.
// This is consistent with server behavior prior to 6.0.
res = configDB.runCommand({createIndexes: "transactions", indexes: []});
assert.commandFailedWithCode(res, ErrorCodes.IllegalOperation);

42
jstests/libs/index_utils.js
View File

@ -56,6 +56,17 @@ export var IndexUtils = (function () {
assert.sameMembers(expectedIndexes, actualIndexes, msg);
}
function _indexExists(indexesList, indexKey, options = undefined) {
return indexesList.some(
(index) =>
bsonWoCompare(indexKey, index.key) === 0 &&
(!options ||
Object.keys(options).every(
(optionKey) => bsonWoCompare(options[optionKey], index[optionKey]) === 0,
)),
);
}
/**
* Checks whether the specified index exists.
*
@ -63,21 +74,32 @@ export var IndexUtils = (function () {
* To check that a field does not exist, you can use the syntax: { field: undefined }.
*/
function indexExists(coll, indexKey, options = undefined) {
return coll
.getIndexes()
.some(
(index) =>
bsonWoCompare(indexKey, index.key) === 0 &&
(!options ||
Object.keys(options).every(
(optionKey) => bsonWoCompare(options[optionKey], index[optionKey]) === 0,
)),
);
return _indexExists(coll.getIndexes(), indexKey, options);
}
/**
* Asserts that the specified index exists.
*
* If `options` are provided, only the specified fields will be verified.
* To check that a field does not exist, you can use the syntax: { field: undefined }.
*/
function assertIndexExists(coll, indexKey, options = undefined) {
const indexes = coll.getIndexes();
assert(
_indexExists(indexes, indexKey, options),
"Index " +
tojson(indexKey) +
", whith options " +
tojson(options) +
" does not exist. Indexes list: " +
tojson(indexes),
);
}
return {
assertIndexes: assertIndexes,
assertIndexesMatch: assertIndexesMatch,
assertIndexExists: assertIndexExists,
indexExists: indexExists,
};
})();

180
jstests/multiVersion/genericSetFCVUsage/shard_catalog/convert_all_bits_index_param_to_int_on_upgrade.js Normal file
View File

@ -0,0 +1,180 @@
/**
* Tests that a 2d index spec with a non-integer 'bits' value written in lastLTS FCV is converted to
* an integer 'bits' value on FCV upgrade.
*
* TODO SERVER-120350: Remove this test once v9.0 becomes last LTS
*/
import "jstests/multiVersion/libs/multi_cluster.js";
import {ShardingTest} from "jstests/libs/shardingtest.js";
import {ReplSetTest} from "jstests/libs/replsettest.js";
// Skip test if lastLTSFCV is not 8.0. In that case, the bits parameter would already be stored as
// an integer, so there is no upgrade path to test.
if (lastLTSFCV !== "8.0") {
jsTest.log.info(
"Skipping test: lastLTSFCV is not 8.0 anymore. This test can be removed once v9.0 becomes last LTS.",
);
quit();
}
const dbName = jsTestName();
const collName = "collTest";
// Retrieves the 'bits' value for the 2d index with the given index name.
function getIndexBits(node, indexName = "loc_2d") {
const indexes = node
.getDB(dbName)
[collName].aggregate([{$indexStats: {}}])
.toArray();
const indexEntry = indexes.find((index) => index.name === indexName);
assert(indexEntry, "Expected index called " + indexName + " to be found");
return indexEntry.spec.bits;
}
function testForReplicaSet() {
const rst = new ReplSetTest({nodes: 2, nodeOptions: {binVersion: "last-lts"}});
rst.startSet();
rst.initiate();
let db = rst.getPrimary().getDB(dbName);
// Ensure we are in last LTS FCV (8.0).
assert.commandWorked(db.adminCommand({setFeatureCompatibilityVersion: lastLTSFCV, confirm: true}));
assert.commandWorked(
db.runCommand({
createIndexes: collName,
indexes: [{key: {loc: "2d"}, name: "loc_2d", bits: 11.6}],
}),
);
// Ensure changes are replicated to all nodes before asserting.
rst.awaitReplication();
// Check bits is stored as a non-integer in lastLTS FCV.
rst.nodes.forEach((node) => {
const bitsValue = getIndexBits(node);
assert.eq(11.6, bitsValue, "Expected bits=11.6 (non-integer) to be stored as-is in lastLTS FCV");
assert(!Number.isInteger(bitsValue));
});
// Upgrade all nodes to latest binary version.
rst.upgradeSet({binVersion: "latest"});
db = rst.getPrimary().getDB(dbName);
// The upgrade process has not been called yet, so the bits parameter should still be stored as a non-integer.
rst.nodes.forEach((node) => {
const bitsValue = getIndexBits(node);
assert.eq(11.6, bitsValue, "Expected bits=11.6 (non-integer) to be stored as-is in lastLTS FCV");
assert(!Number.isInteger(bitsValue));
});
// Upgrade to latest FCV.
assert.commandWorked(db.adminCommand({setFeatureCompatibilityVersion: latestFCV, confirm: true}));
// Ensure changes are replicated to all nodes before asserting.
rst.awaitReplication();
// Check bits have been converted to an integer.
rst.nodes.forEach((node) => {
const bitsValue = getIndexBits(node, "loc_2d");
assert.eq(11, bitsValue, "Expected bits=11 (integer) to be converted to an integer");
assert(Number.isInteger(bitsValue));
});
// From now on, any new 2d index will have an integer 'bits' value.
assert.commandWorked(
db.runCommand({
createIndexes: collName,
indexes: [{key: {loc2: "2d"}, name: "loc2_2d", bits: 9.2}],
}),
);
rst.awaitReplication();
rst.nodes.forEach((node) => {
const bitsValue = getIndexBits(node, "loc2_2d");
assert.eq(9, bitsValue, "Expected bits=9 (integer) to be stored as an integer for index loc2_2d");
assert(Number.isInteger(bitsValue));
});
rst.stopSet();
}
function testForShardedCluster() {
const st = new ShardingTest({
shards: 2,
mongos: 1,
config: 1,
rs: {nodes: 2},
mongosOptions: {binVersion: "last-lts"},
rsOptions: {binVersion: "last-lts"},
});
// Ensure we are in last LTS FCV (8.0).
assert.commandWorked(st.s.adminCommand({setFeatureCompatibilityVersion: lastLTSFCV, confirm: true}));
let db = st.s.getDB(dbName);
const fullCollName = dbName + "." + collName;
assert.commandWorked(st.s.adminCommand({enableSharding: dbName, primaryShard: st.shard0.shardName}));
assert.commandWorked(st.s.adminCommand({shardCollection: fullCollName, key: {x: 1}}));
assert.commandWorked(
db.runCommand({
createIndexes: collName,
indexes: [{key: {loc: "2d"}, name: "loc_2d", bits: 11.6}],
}),
);
// Ensure changes are replicated to all nodes of shard0 before asserting.
st.rs0.awaitReplication();
// Check bits is stored as a non-integer in lastLTS FCV.
st.rs0.nodes.forEach((node) => {
assert.eq(11.6, getIndexBits(node), "Expected bits=11.6 (non-integer) to be stored as-is in lastLTS FCV");
});
// Upgrade all nodes to latest binary version.
st.upgradeCluster("latest", {waitUntilStable: true});
db = st.s.getDB(dbName);
// The upgrade process has not been called yet, so the bits parameter should still be stored as a non-integer.
st.rs0.nodes.forEach((node) => {
const bitsValue = getIndexBits(node);
assert.eq(11.6, bitsValue, "Expected bits=11.6 (non-integer) to be stored as-is in lastLTS FCV");
assert(!Number.isInteger(bitsValue));
});
// Upgrade to latest FCV.
assert.commandWorked(st.s.adminCommand({setFeatureCompatibilityVersion: latestFCV, confirm: true}));
// Ensure changes are replicated to all nodes before asserting.
st.rs0.awaitReplication();
// Ensure bits is stored as an integer.
st.rs0.nodes.forEach((node) => {
const bitsValue = getIndexBits(node, "loc_2d");
assert.eq(11, bitsValue, "Expected bits=11 (integer) to be stored as an integer");
assert(Number.isInteger(bitsValue));
});
// From now on, any new 2d index will have an integer 'bits' value.
assert.commandWorked(
db.runCommand({
createIndexes: collName,
indexes: [{key: {loc2: "2d"}, name: "loc2_2d", bits: 9.2}],
}),
);
st.rs0.awaitReplication();
st.rs0.nodes.forEach((node) => {
const bitsValue = getIndexBits(node, "loc2_2d");
assert.eq(9, bitsValue, "Expected bits=9 (integer) to be stored as an integer for index loc2_2d");
assert(Number.isInteger(bitsValue));
});
st.stop();
}
testForReplicaSet();
testForShardedCluster();

15
src/mongo/db/commands/set_feature_compatibility_version_command.cpp
View File

@ -1226,17 +1226,17 @@ private:
rangedeletionutil::setPreMigrationShardVersionOnRangeDeletionTasks(opCtx);
}
_cleanUpDeprecatedCatalogMetadata(opCtx);
_cleanUpIndexCatalogMetadataOnUpgrade(opCtx);
FCVStepRegistry::get(opCtx->getServiceContext())
.upgradeServerMetadata(opCtx, originalVersion, requestedVersion);
}
// TODO(SERVER-100328): remove after 9.0 is branched.
// WARNING: do not rely on this method to clean up metadata that can be created concurrently. It
// is fine to rely on this only when missing concurrently created collections is fine, when
// newly created collections no longer use the metadata format we wish to remove.
void _cleanUpDeprecatedCatalogMetadata(OperationContext* opCtx) {
// WARNING: do not rely on this method to clean up index metadata that can be created
// concurrently. It is fine to rely on this only when missing concurrently created collections
// is fine, when newly created collections no longer use the metadata format we wish to remove.
void _cleanUpIndexCatalogMetadataOnUpgrade(OperationContext* opCtx) {
// We bypass the UserWritesBlock mode here in order to not see errors arising from the
// block. The user already has permission to run FCV at this point and the writes performed
// here aren't modifying any user data with the exception of fixing up the collection
@ -1249,8 +1249,9 @@ private:
Lock::DBLock dbLock(opCtx, dbName, MODE_IX);
catalog::forEachCollectionFromDb(
opCtx, dbName, MODE_X, [&](const Collection* collection) {
// To remove deprecated catalog metadata, issue a collmod with no other options
// set.
// Issue a no-op collMod command to each collection to trigger removal of
// deprecated catalog metadata and to correct any invalid value types previously
// allowed in metadata.
BSONObjBuilder responseBuilder;
uassertStatusOK(processCollModCommand(opCtx,
collection->ns(),

25
src/mongo/db/index_key_validate.cpp
View File

@ -47,6 +47,7 @@
#include "mongo/db/query/compiler/parsers/matcher/expression_parser.h"
#include "mongo/db/shard_role/shard_catalog/clustered_collection_options_gen.h"
#include "mongo/db/storage/storage_options.h"
#include "mongo/db/storage/storage_parameters_gen.h"
#include "mongo/logv2/log.h"
#include "mongo/platform/compiler.h"
#include "mongo/util/assert_util.h"
@ -382,7 +383,8 @@ BSONObj repairIndexSpec(const NamespaceString& ns,
StatusWith<BSONObj> validateIndexSpec(
OperationContext* opCtx,
const BSONObj& indexSpec,
const std::map<StringData, std::set<IndexType>>& allowedFieldNames) {
const std::map<StringData, std::set<IndexType>>& allowedFieldNames,
bool isUpgradeRepair) {
bool hasKeyPatternField = false;
bool hasIndexNameField = false;
bool hasNamespaceField = false;
@ -396,6 +398,7 @@ StatusWith<BSONObj> validateIndexSpec(
bool prepareUnique = false;
auto clusteredField = indexSpec[IndexDescriptor::kClusteredFieldName];
bool apiStrict = opCtx && APIParameters::get(opCtx).getAPIStrict().value_or(false);
bool is2dIndexWithNonIntBits = false;
auto fieldNamesValidStatus = validateIndexSpecFieldNames(indexSpec, allowedFieldNames);
if (!fieldNamesValidStatus.isOK()) {
@ -649,6 +652,18 @@ StatusWith<BSONObj> validateIndexSpec(
str::stream() << "The field '" << indexSpecElemFieldName
<< "' must be a number, but got "
<< typeName(indexSpecElem.type())};
} else if (IndexDescriptor::k2dIndexBitsFieldName == indexSpecElemFieldName) {
is2dIndexWithNonIntBits = indexSpecElem.type() != BSONType::numberInt;
// Prior to SERVER-120174, non-integer values could be stored for this parameter. During
// FCV upgrade, signal the repair path by returning an error here so that
// repairIndexSpec() will convert any such on-disk values to integers.
if (isUpgradeRepair && is2dIndexWithNonIntBits) {
return {ErrorCodes::TypeMismatch,
str::stream()
<< "The field '" << indexSpecElemFieldName
<< "' must be an integer, but got " << typeName(indexSpecElem.type())};
}
} else if (IndexDescriptor::kExpireAfterSecondsFieldName == indexSpecElemFieldName) {
auto swType = validateExpireAfterSeconds(
indexSpecElem, ValidateExpireAfterSecondsMode::kSecondaryTTLIndex);
@ -773,6 +788,14 @@ StatusWith<BSONObj> validateIndexSpec(
modifiedSpec = modifiedSpec.addField(specToAdd.firstElement());
}
if (is2dIndexWithNonIntBits) {
// Store this field as an integer value.
BSONObj specToAdd =
BSON(IndexDescriptor::k2dIndexBitsFieldName
<< indexSpec[IndexDescriptor::k2dIndexBitsFieldName].safeNumberInt());
modifiedSpec = modifiedSpec.addField(specToAdd.firstElement());
}
return modifiedSpec;
}

12
src/mongo/db/index_key_validate.h
View File

@ -82,12 +82,22 @@ Status validateKeyPattern(const BSONObj& key, IndexDescriptor::IndexVersion inde
* Validates the index specification 'indexSpec' and returns an equivalent index specification that
* has any missing attributes filled in. If the index specification is malformed, then an error
* status is returned.
*
* The 'isUpgradeRepair' parameter should be set to true only when this function is called during a
* setFCV upgrade operation. When true, certain fields that have historically been stored with
* invalid types on disk (e.g. a non-integer value for '2d' index 'bits') will cause a non-OK
* status to be returned, signaling to callers that a repair is needed.
* When false (the default), those same conditions are tolerated without error.
*
* TODO (SERVER-120350) Update the previous comment accordingly and consider removing the
* 'isUpgradeRepair' flag once 9.0 branches out.
*/
StatusWith<BSONObj> validateIndexSpec(
OperationContext* opCtx,
const BSONObj& indexSpec,
const std::map<StringData, std::set<IndexType>>& allowedFieldNames =
index_key_validate::kAllowedFieldNames);
index_key_validate::kAllowedFieldNames,
bool isUpgradeRepair = false);
/**
* Returns a new index spec with any unknown field names removed from 'indexSpec'.

2
src/mongo/db/s/migration_chunk_cloner_source_test.cpp
View File

@ -394,7 +394,7 @@ public:
}
std::vector<std::string> repairInvalidIndexOptions(OperationContext* opCtx,
bool removeDeprecatedFields) override {
bool isUpgradeRepair) override {
MONGO_UNREACHABLE;
}

17
src/mongo/db/shard_role/shard_catalog/coll_mod.cpp
View File

@ -1055,23 +1055,20 @@ Status _collModInternal(OperationContext* opCtx,
writableColl->setTimeseriesBucketingParametersChanged(opCtx, boost::none);
}
// Fix any invalid index options for indexes belonging to this collection, only for empty
// collMod requests which are called during setFCV upgrade.
const auto removeDeprecatedFields = [&]() {
if (cmrNew.numModifications > 0) {
return false;
}
const auto isUpgrading = [&]() {
if (!ServerGlobalParams::FCVSnapshot::isUpgradingOrDowngrading(version)) {
return false;
}
const auto transitionInfo = getTransitionFCVInfo(version);
return transitionInfo.from < transitionInfo.to;
}();
};
// Indicates whether this is an empty collMod command invoked as part of a setFCV upgrade.
// Certain index repairs are performed exclusively during the upgrade process.
const bool isUpgradeRepair = (cmrNew.numModifications == 0) && isUpgrading();
std::vector<std::string> indexesWithInvalidOptions =
writableColl->repairInvalidIndexOptions(opCtx, removeDeprecatedFields);
writableColl->repairInvalidIndexOptions(opCtx, isUpgradeRepair);
for (const auto& indexWithInvalidOptions : indexesWithInvalidOptions) {
const auto entry =
writableColl->getIndexCatalog()->findIndexByName(opCtx, indexWithInvalidOptions);

17
src/mongo/db/shard_role/shard_catalog/collection.h
View File

@ -541,12 +541,19 @@ public:
/**
* Repairs invalid index options on all indexes in this collection. Returns a list of
* index names that were repaired. Specifying 'removeDeprecatedFields' as true, causes
* deprecated fields, which much be otherwise supported for backwards compatibility, to be
* removed when performing the repair.
* index names that were repaired.
*
* When 'isUpgradeRepair' is true, this function is being called as part of a setFCV upgrade
* operation. In that mode, deprecated index fields are also removed (fields that are otherwise
* kept for backwards compatibility), and stricter validation is applied so that index specs
* with historically-tolerated invalid values (e.g. non-integer '2d' index 'bits') are
* detected and corrected on disk.
*
* TODO (SERVER-120350) Update the previous comment accordingly and consider removing the
* 'isUpgradeRepair' flag once 9.0 branches out.
*/
virtual std::vector<std::string> repairInvalidIndexOptions(
OperationContext* opCtx, bool removeDeprecatedFields = false) = 0;
virtual std::vector<std::string> repairInvalidIndexOptions(OperationContext* opCtx,
bool isUpgradeRepair = false) = 0;
/**
* Updates the 'temp' setting for this collection.

13
src/mongo/db/shard_role/shard_catalog/collection_impl.cpp
View File

@ -1501,9 +1501,12 @@ void CollectionImpl::updatePrepareUniqueSetting(OperationContext* opCtx,
}
std::vector<std::string> CollectionImpl::repairInvalidIndexOptions(OperationContext* opCtx,
bool removeDeprecatedFields) {
bool isUpgradeRepair) {
std::vector<std::string> indexesWithInvalidOptions;
const auto& allowedFieldNames = removeDeprecatedFields
// Deprecated fields are stripped from index specs only when repairing during an upgrade
// process.
const auto& allowedFieldNames = isUpgradeRepair
? index_key_validate::kNonDeprecatedAllowedFieldNames
: index_key_validate::kAllowedFieldNames;
@ -1512,9 +1515,9 @@ std::vector<std::string> CollectionImpl::repairInvalidIndexOptions(OperationCont
if (index.isPresent()) {
BSONObj oldSpec = index.spec;
Status status =
index_key_validate::validateIndexSpec(opCtx, oldSpec, allowedFieldNames)
.getStatus();
Status status = index_key_validate::validateIndexSpec(
opCtx, oldSpec, allowedFieldNames, isUpgradeRepair)
.getStatus();
if (status.isOK()) {
continue;
}

2
src/mongo/db/shard_role/shard_catalog/collection_impl.h
View File

@ -351,7 +351,7 @@ public:
bool prepareUnique) final;
std::vector<std::string> repairInvalidIndexOptions(OperationContext* opCtx,
bool removeDeprecatedFields) final;
bool isUpgradeRepair) final;
void setIsTemp(OperationContext* opCtx, bool isTemp) final;

2
src/mongo/db/shard_role/shard_catalog/collection_mock.h
View File

@ -389,7 +389,7 @@ public:
}
std::vector<std::string> repairInvalidIndexOptions(OperationContext* opCtx,
bool removeDeprecatedFields) override {
bool isUpgradeRepair) override {
MONGO_UNREACHABLE;
}

2
src/mongo/db/shard_role/shard_catalog/virtual_collection_impl.h
View File

@ -369,7 +369,7 @@ public:
}
std::vector<std::string> repairInvalidIndexOptions(OperationContext* opCtx,
bool removeDeprecatedFields) final {
bool isUpgradeRepair) final {
unimplementedTasserted();
return {};
}