SERVER-47840 Do not write "hidden: false" to the index catalog when createIndexes specifies it explicitly

(cherry picked from commit bae19fea072ed8a8bc46db8f787b5474fbbc5f7b)

5 files changed, 83 insertions, 8 deletions

diff --git a/jstests/core/hidden_index.js b/jstests/core/hidden_index.js
index a5ea2d09f17..736460c9b3b 100644
--- a/jstests/core/hidden_index.js
+++ b/jstests/core/hidden_index.js
@@ -5,7 +5,7 @@
  * @tags: [
  *  multiversion_incompatible,
  *  requires_fcv_44,
- *  requires_non_retryable_commands,    // CollMod is not retryable.
+ *  requires_non_retryable_commands,    # CollMod is not retryable.
  * ]
  */
 
@@ -123,4 +123,21 @@ db.runCommand({
 idxSpec = GetIndexHelpers.findByName(coll.getIndexes(), "tm_1");
 assert(idxSpec.hidden);
 assert.eq(idxSpec.expireAfterSeconds, 1);
+
+//
+// Ensure that "hidden: false" won't be added to index specification.
+//
+assert.commandWorked(
+    db.runCommand({createIndexes: collName, indexes: [{key: {y: 1}, name: "y", hidden: false}]}));
+idxSpec = GetIndexHelpers.findByName(coll.getIndexes(), "y");
+assert.eq(idxSpec.hidden, undefined);
+
+assert.commandWorked(coll.hideIndex("y"));
+idxSpec = GetIndexHelpers.findByName(coll.getIndexes(), "y");
+assert(idxSpec.hidden);
+
+// Ensure that unhiding the hidden index won't add 'hidden: false' to the index spec as well.
+assert.commandWorked(coll.unhideIndex("y"));
+idxSpec = GetIndexHelpers.findByName(coll.getIndexes(), "y");
+assert.eq(idxSpec.hidden, undefined);
 })();
diff --git a/jstests/multiVersion/hiddenIndexes.js b/jstests/multiVersion/hiddenIndexes.js
index 4f99908fb42..c31e08e35ad 100644
--- a/jstests/multiVersion/hiddenIndexes.js
+++ b/jstests/multiVersion/hiddenIndexes.js
@@ -198,7 +198,7 @@ try {
 // Verify that a clean restart on 4.2 binary fails when the initial sync encounters a hidden index.
 try {
     restartReplSetNode(
-        st.rs0, secondaryNodeOfShard, Object.assign(nodeOptionsLatest, {startClean: true}));
+        st.rs0, secondaryNodeOfShard, Object.assign(nodeOptionsLastStable, {startClean: true}));
     assert(false, "Expected 'restartReplSetNode' to throw");
 } catch (err) {
     assert.eq(err.message, "MongoDB process stopped with exit code: 14");
@@ -207,11 +207,47 @@ try {
 }
 
 // Start that node and mongos with the latest binary for a clean shutdown.
-st.rs0.start(secondaryNodeOfShard, Object.assign(nodeOptionsLatest, {startClean: true}));
+st.rs0.start(secondaryNodeOfShard,
+             Object.assign(nodeOptionsLatest, {startClean: true, shardsvr: ""}));
 st.rs0.awaitReplication();
 st.upgradeCluster(nodeOptionsLatest.binVersion,
                   {upgradeMongos: true, upgradeShards: false, upgradeConfigs: false});
 })();
 
+//
+// Can successfully downgrade mongod from 4.4 to 4.2 with an index explicitly set option 'hidden' to
+// 'false' on 'createIndex()'.
+(function() {
+mongosDB = st.s.getDB(kDbName);
+coll = mongosDB.coll;
+coll.dropIndexes();
+
+assert.commandWorked(coll.insert({s: 1, t: 1}));
+assert.commandWorked(mongosDB.adminCommand({setFeatureCompatibilityVersion: latestFCV}));
+assertVersionAndFCV(["4.4", "4.3"], latestFCV);
+
+const createAsUnhiddenIndexName = "createAsUnhidden";
+assert.commandWorked(coll.createIndex({s: 1}, {hidden: false, name: createAsUnhiddenIndexName}));
+assert.eq(getIndex(createAsUnhiddenIndexName).hidden, undefined);
+assert.commandWorked(coll.createIndex({t: 1}));
+assert.commandWorked(mongosDB.runCommand({
+    "collMod": coll.getName(),
+    "index": {"name": "t_1", "hidden": false},
+}));
+
+assert.commandWorked(st.s.adminCommand({setFeatureCompatibilityVersion: lastStableFCV}));
+
+// Test that we can downgrade the cluster to 4.2.
+st.upgradeCluster(nodeOptionsLastStable.binVersion,
+                  {upgradeMongos: true, upgradeShards: true, upgradeConfigs: true});
+
+mongosDB = st.s.getDB(kDbName);
+coll = mongosDB.coll;
+
+// The unhidden index created with option 'hidden: false' can still be used by the query planner.
+assertStagesForExplainOfCommand(
+    {coll: coll, cmdObj: {find: coll.getName(), filter: {s: 1}}, expectedStages: ["IXSCAN"]});
+})();
+
 st.stop();
 }());
diff --git a/jstests/noPassthrough/durable_hidden_index.js b/jstests/noPassthrough/durable_hidden_index.js
index ce40d58106c..b0bb754d77f 100644
--- a/jstests/noPassthrough/durable_hidden_index.js
+++ b/jstests/noPassthrough/durable_hidden_index.js
@@ -28,9 +28,13 @@ const primaryDB = rst.getPrimary().getDB(dbName);
 primaryDB.coll.drop();
 
 // Create a hidden index.
-primaryDB.coll.createIndex({a: 1}, {hidden: true});
+assert.commandWorked(primaryDB.coll.createIndex({a: 1}, {hidden: true}));
 assert(isIndexHidden(primaryDB.coll.getIndexes(), "a_1"));
 
+// Explicitly create an unhidden index.
+assert.commandWorked(primaryDB.coll.createIndex({b: 1}, {hidden: false}));
+assert(!isIndexHidden(primaryDB.coll.getIndexes(), "b_1"));
+
 // Wait for the replication finishes before stopping the replica set.
 rst.awaitReplication();
 
@@ -39,12 +43,19 @@ rst.stopSet(/* signal */ undefined, /* forRestart */ true);
 rst.startSet(/* signal */ undefined, /* forRestart */ true);
 const secondaryDB = rst.getSecondary().getDB(dbName);
 
+// Test that after restart the index is still hidden.
 assert(isIndexHidden(secondaryDB.coll.getIndexes(), "a_1"));
 
+// Test that 'hidden: false' shouldn't be written to the index catalog.
+let idxSpec = GetIndexHelpers.findByName(secondaryDB.coll.getIndexes(), "b_1");
+assert.eq(idxSpec.hidden, undefined);
+
 rst.stopSet();
 
 //
-// Test that hidden index status will be persisted into the index catalog in a standalone mongod.
+// Test that hidden index status will be persisted into the index catalog in a standalone mongod,
+// whereas, an unhidden index will not write 'hidden: false' to the index catalog even when
+// createIndexes specifies 'hidden: false' explicitly.
 //
 // Start a mongod.
 let conn = MongoRunner.runMongod();
@@ -53,9 +64,13 @@ let db = conn.getDB(dbName);
 db.coll.drop();
 
 // Create a hidden index.
-db.coll.createIndex({a: 1}, {hidden: true});
+assert.commandWorked(db.coll.createIndex({a: 1}, {hidden: true}));
 assert(isIndexHidden(db.coll.getIndexes(), "a_1"));
 
+// Explicitly create an unhidden index.
+assert.commandWorked(db.coll.createIndex({b: 1}, {hidden: false}));
+assert(!isIndexHidden(db.coll.getIndexes(), "b_1"));
+
 // Restart the mongod.
 MongoRunner.stopMongod(conn);
 conn = MongoRunner.runMongod({restart: true, cleanData: false, dbpath: conn.dbpath});
@@ -64,5 +79,9 @@ db = conn.getDB(dbName);
 // Test that after restart the index is still hidden.
 assert(isIndexHidden(db.coll.getIndexes(), "a_1"));
 
+// Test that 'hidden: false' shouldn't be written to the index catalog.
+idxSpec = GetIndexHelpers.findByName(db.coll.getIndexes(), "b_1");
+assert.eq(idxSpec.hidden, undefined);
+
 MongoRunner.stopMongod(conn);
 })();
diff --git a/src/mongo/db/catalog/index_catalog_impl.cpp b/src/mongo/db/catalog/index_catalog_impl.cpp
index d38b3f010d8..dc315638f5a 100644
--- a/src/mongo/db/catalog/index_catalog_impl.cpp
+++ b/src/mongo/db/catalog/index_catalog_impl.cpp
@@ -1774,6 +1774,9 @@ StatusWith<BSONObj> IndexCatalogImpl::_fixIndexSpec(OperationContext* opCtx,
     if (o["unique"].trueValue())
         b.appendBool("unique", true);  // normalize to bool true in case was int 1 or something...
 
+    if (o["hidden"].trueValue())
+        b.appendBool("hidden", true);  // normalize to bool true in case was int 1 or something...
+
     BSONObj key = fixIndexKey(o["key"].Obj());
     b.append("key", key);
 
@@ -1802,7 +1805,7 @@ StatusWith<BSONObj> IndexCatalogImpl::_fixIndexSpec(OperationContext* opCtx,
             } else if (s == "dropDups" || s == "ns") {
                 // dropDups is silently ignored and removed from the spec as of SERVER-14710.
                 // ns is removed from the spec as of 4.4.
-            } else if (s == "v" || s == "unique" || s == "key" || s == "name") {
+            } else if (s == "v" || s == "unique" || s == "key" || s == "name" || s == "hidden") {
                 // covered above
             } else {
                 b.append(e);
diff --git a/src/mongo/db/index/index_descriptor.h b/src/mongo/db/index/index_descriptor.h
index 4eab168a65a..d8a5d2e3a23 100644
--- a/src/mongo/db/index/index_descriptor.h
+++ b/src/mongo/db/index/index_descriptor.h
@@ -71,6 +71,7 @@ public:
     static constexpr StringData kDropDuplicatesFieldName = "dropDups"_sd;
     static constexpr StringData kExpireAfterSecondsFieldName = "expireAfterSeconds"_sd;
     static constexpr StringData kGeoHaystackBucketSize = "bucketSize"_sd;
+    static constexpr StringData kHiddenFieldName = "hidden"_sd;
     static constexpr StringData kIndexNameFieldName = "name"_sd;
     static constexpr StringData kIndexVersionFieldName = "v"_sd;
     static constexpr StringData kKeyPatternFieldName = "key"_sd;
@@ -82,7 +83,6 @@ public:
     static constexpr StringData kStorageEngineFieldName = "storageEngine"_sd;
     static constexpr StringData kTextVersionFieldName = "textIndexVersion"_sd;
     static constexpr StringData kUniqueFieldName = "unique"_sd;
-    static constexpr StringData kHiddenFieldName = "hidden"_sd;
     static constexpr StringData kWeightsFieldName = "weights"_sd;
 
     /**