SERVER-62636 setFeatureCompatibilityVersion 4.4 succeeds on 5.0 binVersion that has indexes with conflicting options

3 files changed, 80 insertions, 79 deletions

diff --git a/jstests/multiVersion/downgrade_conflicting_indexes.js b/jstests/multiVersion/downgrade_conflicting_indexes.js
new file mode 100644
index 00000000000..00d444eb011
--- /dev/null
+++ b/jstests/multiVersion/downgrade_conflicting_indexes.js
@@ -0,0 +1,28 @@
+/**
+ * Tests that user can not downgrade to lastLTS FCV if there exists a collection with conflicting
+ * indexes.
+ */
+(function() {
+"use strict";
+
+const rst = new ReplSetTest({nodes: [{binVersion: "latest"}, {binVersion: "latest"}]});
+rst.startSet();
+rst.initiate();
+
+const db = rst.getPrimary().getDB("test");
+const coll = db.getCollection("coll");
+
+// Create multiple indexes on a collection with conflicting options.
+assert.commandWorked(coll.runCommand("createIndexes", {
+    indexes: [
+        {key: {x: 1}, name: "x1", sparse: true},
+        {key: {x: 1}, name: "x2", unique: true},
+    ]
+}));
+
+// Ensure that downgrade to the lastLTS is failing.
+assert.commandFailedWithCode(db.adminCommand({setFeatureCompatibilityVersion: lastLTSFCV}),
+                             ErrorCodes.CannotDowngrade);
+
+rst.stopSet();
+})();
diff --git a/jstests/multiVersion/index_signature_fcv.js b/jstests/multiVersion/index_signature_fcv.js
index 5de098bb705..d5a9a527a2c 100644
--- a/jstests/multiVersion/index_signature_fcv.js
+++ b/jstests/multiVersion/index_signature_fcv.js
@@ -4,12 +4,10 @@
  *   - Multiple indexes which differ only by partialFilterExpression can be built in FCV 4.7+.
  *   - Multiple indexes which differ only by unique or sparse can be built in FCV 4.9+.
  *   - Multiple indexes which differ only by wildcardProjection can be built in FCV 5.0+.
- *   - The planner can continue to use these indexes after downgrading to FCV 4.4.
- *   - These indexes can be dropped in FCV 4.4.
+ *   - Indexes with conflicting options must be dropped for successful FCV 4.4 downgrade.
  *   - Indexes which differ only by partialFilterExpression cannot be created in FCV 4.4.
  *   - Indexes which differ only by either unique, sparse, or wildcardProjection cannot be created
  *     in FCV 4.4.
- *   - We do not fassert if the set is downgraded to binary 4.4 with "duplicate" indexes present.
  *
  * TODO SERVER-47766: this test is specific to the 4.4 - 4.7+ upgrade process, and can be removed
  * when 5.0 becomes last-lts.
@@ -132,66 +130,14 @@ assertIndexedQuery({a: -1}, 5);
 
 jsTestLog("Downgrade to the last LTS");
 
-// Downgrades to the last LTS FCV
-testDB.adminCommand({setFeatureCompatibilityVersion: lastLTSFCV});
-
-// Verifies that attempting to build an index with the same name and identical value for
-// 'partialFilterExpression' option as an existing index results in a no-op in FCV 4.4, and the
-// command reports successful execution.
-var cmdRes = assert.commandWorked(
-    coll.createIndex({a: 1}, {name: "index1", partialFilterExpression: {a: {$gte: 0}}}));
-assert.eq(cmdRes.numIndexesBefore, cmdRes.numIndexesAfter);
-
-// Verifies that attempting to build an index with the same name and identical value for 'unique'
-// option as an existing index results in a no-op in FCV 4.4, and the command reports successful
-// execution.
-cmdRes = assert.commandWorked(coll.createIndex({a: 1}, {name: "unique_index", unique: true}));
-assert.eq(cmdRes.numIndexesBefore, cmdRes.numIndexesAfter);
-
-// Verifies that attempting to build an index with the same name and identical value for 'sparse'
-// option as an existing index results in a no-op in FCV 4.4, and the command reports successful
-// execution.
-cmdRes = assert.commandWorked(coll.createIndex({a: 1}, {name: "sparse_index", sparse: true}));
-assert.eq(cmdRes.numIndexesBefore, cmdRes.numIndexesAfter);
-
-// Verifies that attempting to build an index with the same name and identical value for
-// 'wildcardProjection' option as an existing index results in a no-op in FCV 4.4, and the command
-// reports successful execution.
-cmdRes =
-    assert.commandWorked(coll.createIndex({"$**": 1}, {name: "wc_a", wildcardProjection: {a: 1}}));
-assert.eq(cmdRes.numIndexesBefore, cmdRes.numIndexesAfter);
-
-// Verifies that these indexes are retained and can be used by the planner when we downgrade to
-// FCV 4.4.
-assertIndexedQuery({a: 1}, 5);
-assertIndexedQuery({a: 11}, 6);
-assertIndexedQuery({a: 101}, 7);
+// Drop all the indexes, as conflicting indexes are not supported in the downgarded version.
+assert.commandWorked(coll.dropIndexes("*"));
 
-// Verifies that indexes distinguished only by 'partialFilterExpression' can be dropped by name in
-// FCV 4.4.
-assert.commandWorked(coll.dropIndex("index2"));
-assertIndexedQuery({a: 1}, 5);
-assertIndexedQuery({a: 11}, 5);
-assertIndexedQuery({a: 101}, 6);
-
-// Verifies that indexes distinguished only by 'unique' option can be dropped by name in FCV 4.4.
-assert.commandWorked(coll.dropIndex("unique_index"));
-assertIndexedQuery({a: 1}, 4);
-assertIndexedQuery({a: 11}, 4);
-assertIndexedQuery({a: 101}, 5);
-
-// Verifies that indexes distinguished only by 'sparse' option can be dropped by name in FCV 4.4.
-assert.commandWorked(coll.dropIndex("sparse_index"));
-assertIndexedQuery({a: 1}, 3);
-assertIndexedQuery({a: 11}, 3);
-assertIndexedQuery({a: 101}, 4);
+// Downgrades to the last LTS FCV
+assert.commandWorked(testDB.adminCommand({setFeatureCompatibilityVersion: lastLTSFCV}));
 
-// Verifies that indexes distinguished only by 'wildcardProjection' option can be dropped by name in
-// FCV 4.4.
-assert.commandWorked(coll.dropIndex("wc_a"));
-assertIndexedQuery({a: 1}, 2);
-assertIndexedQuery({a: 11}, 2);
-assertIndexedQuery({a: 101}, 3);
+// Create an index on 'a'.
+assert.commandWorked(coll.createIndex({a: 1}));
 
 // Verifies that an index distinguished only by 'partialFilterExpression' option cannot be created
 // in FCV 4.4.
@@ -209,6 +155,7 @@ assert.commandFailedWithCode(coll.createIndex({a: 1}, {name: "sparse_index", spa
 
 // Verifies that an index distinguished only by 'wildcardProjection' option cannot be created in
 // FCV 4.4.
+assert.commandWorked(coll.createIndex({"$**": 1}));
 assert.commandFailedWithCode(
     coll.createIndex({"$**": 1}, {name: "wc_a", wildcardProjection: {a: 1}}),
     ErrorCodes.IndexOptionsConflict);
@@ -216,32 +163,23 @@ assert.commandFailedWithCode(
 // We need to recreate the unique & sparse & wildcardProjection indexes that we just dropped before
 // downgrading to the LTS binary. To do so, we need to temporarily upgrade the FCV to 'latest'
 // again.
-testDB.adminCommand({setFeatureCompatibilityVersion: latestFCV});
+assert.commandWorked(testDB.adminCommand({setFeatureCompatibilityVersion: latestFCV}));
 
 assert.commandWorked(coll.createIndex({a: 1}, {name: "unique_index", unique: true}));
 assert.commandWorked(coll.createIndex({a: 1}, {name: "sparse_index", sparse: true}));
 assert.commandWorked(coll.createIndex({"$**": 1}, {name: "wc_a", wildcardProjection: {a: 1}}));
 
-// Need to downgrade to the LTS FCV before downgrade to the LTS binary.
-testDB.adminCommand({setFeatureCompatibilityVersion: lastLTSFCV});
+// Drop all the indexes, as conflicting indexes are not supported in the downgarded version.
+assert.commandWorked(coll.dropIndexes("*"));
 
-// Verifies that downgrading to binary 4.4 with overlapping partialFilterExpression, unique, sparse,
-// and wildcardProjection indexes present does not fassert.
+// Need to downgrade to the LTS FCV before downgrade to the LTS binary.
+assert.commandWorked(testDB.adminCommand({setFeatureCompatibilityVersion: lastLTSFCV}));
 rst.upgradeSet({binVersion: "last-lts"});
 testDB = rst.getPrimary().getDB(jsTestName());
 coll = testDB.test;
 
-// Verifies that the indexes still exist and can be used to answer queries on the binary 4.4
-// replset.
-assertIndexedQuery({a: 1}, 5);
-assertIndexedQuery({a: 11}, 5);
-assertIndexedQuery({a: 101}, 6);
-
-// Verifies that indexes which are distinguished only by 4.7+ signature fields can be dropped by
-// name on binary 4.4.
-assert.commandWorked(coll.dropIndex("unique_index"));
-assert.commandWorked(coll.dropIndex("sparse_index"));
-assert.commandWorked(coll.dropIndex("wc_a"));
+// Create an index on 'a'.
+assert.commandWorked(coll.createIndex({a: 1}));
 
 // Verifies that an index which differs only by 'partialFilterExpression' option cannot be created
 // on binary 4.4.
@@ -259,6 +197,7 @@ assert.commandFailedWithCode(coll.createIndex({a: 1}, {name: "sparse_index", spa
 
 // Verifies that an index distinguished only by 'wildcardProjection' option cannot be created in
 // binary 4.4.
+assert.commandWorked(coll.createIndex({"$**": 1}, {name: "wc_all"}));
 assert.commandFailedWithCode(
     coll.createIndex({"$**": 1}, {name: "wc_a", wildcardProjection: {a: 1}}),
     ErrorCodes.IndexOptionsConflict);
@@ -308,7 +247,7 @@ assert.commandFailedWithCode(
     ErrorCodes.IndexOptionsConflict);
 
 // Upgrades to the lastest FCV
-testDB.adminCommand({setFeatureCompatibilityVersion: latestFCV});
+assert.commandWorked(testDB.adminCommand({setFeatureCompatibilityVersion: latestFCV}));
 
 // Verifies that indexes with path projections which is identical after normalization can not be
 // created.
diff --git a/src/mongo/db/commands/set_feature_compatibility_version_command.cpp b/src/mongo/db/commands/set_feature_compatibility_version_command.cpp
index aedd869061c..a2acb9acea3 100644
--- a/src/mongo/db/commands/set_feature_compatibility_version_command.cpp
+++ b/src/mongo/db/commands/set_feature_compatibility_version_command.cpp
@@ -720,6 +720,38 @@ private:
         }
     }
 
+    /**
+     * Partial index filters are only supported in 4.7.0 and above. If the user tries to
+     * downgrade the cluster to an earlier version, they must first ensure that there are no
+     * indexes with conflicting options.
+     */
+    void _disallowIndexesWithConflictingOptionsOnDowngrade(OperationContext* opCtx) {
+        auto collCatalog = CollectionCatalog::get(opCtx);
+        for (const auto& db : collCatalog->getAllDbNames()) {
+            for (auto collIt = collCatalog->begin(opCtx, db); collIt != collCatalog->end(opCtx);
+                 ++collIt) {
+                NamespaceStringOrUUID collName(
+                    collCatalog->lookupNSSByUUID(opCtx, collIt.uuid().get()).get());
+                AutoGetCollectionForRead coll(opCtx, collName);
+                if (!coll) {
+                    continue;
+                }
+
+                SimpleBSONObjUnorderedSet indexKeyPatterns;
+                auto indexIterator = coll->getIndexCatalog()->getIndexIterator(
+                    opCtx, /* includeUnfinishedIndexes  */ true);
+                while (indexIterator->more()) {
+                    auto indexKeyPattern = indexIterator->next()->descriptor()->keyPattern();
+                    uassert(ErrorCodes::CannotDowngrade,
+                            str::stream() << "Cannot downgrade the cluster as collection "
+                                          << coll->ns() << " has indexes with conflicting options",
+                            indexKeyPatterns.find(indexKeyPattern) == indexKeyPatterns.end());
+                    indexKeyPatterns.emplace(indexKeyPattern.getOwned());
+                }
+            }
+        }
+    }
+
     void _runDowngrade(OperationContext* opCtx,
                        const SetFeatureCompatibilityVersion& request,
                        boost::optional<Timestamp> changeTimestamp) {
@@ -762,7 +794,6 @@ private:
             });
         }
 
-        _disallowTTLIndexesWithNaNExpireAfterSecondsOnDowngrade(opCtx);
         // TODO (SERVER-56171): Remove once 5.0 is last-lts.
         removeTimeseriesEntriesFromConfigTransactions(opCtx);
 
@@ -808,6 +839,9 @@ private:
                 "Failing upgrade due to 'failDowngrading' failpoint set",
                 !failDowngrading.shouldFail());
 
+        _disallowTTLIndexesWithNaNExpireAfterSecondsOnDowngrade(opCtx);
+        _disallowIndexesWithConflictingOptionsOnDowngrade(opCtx);
+
         if (serverGlobalParams.clusterRole == ClusterRole::ConfigServer) {
             // Always abort the reshardCollection regardless of version to ensure that it will run
             // on a consistent version from start to finish. This will ensure that it will be able