SERVER-113685 SERVER-113900 SERVER-113897 Disallow wildcard index creation where projection is only _id exclusion (#44583) (#46113)

GitOrigin-RevId: 36692734f58ffbea8bc077f6e535572057ae2da2

evergreen/multiversion_selection.sh

@@ -46,7 +46,7 @@ evergreen_args=" -sb \
 local_args="--edition $edition \
   --debug \
   ${last_lts_arg} \
-  ${last_continuous_arg} 6.0"
+  ${last_continuous_arg} 6.0 8.0.16"
 
 remote_invocation="${base_command} ${evergreen_args} ${local_args}"
 eval "${remote_invocation}"

jstests/core/compound_wildcard_index_validation.js

@@ -28,11 +28,13 @@ assert.commandWorked(
 // Tests that _id can be excluded in an inclusion projection statement.
 assert.commandWorked(
     coll.createIndex({"$**": 1, "other": 1}, {"wildcardProjection": {"_id": 0, "a": 1}}));
-// Tests that _id can be inccluded in an exclusion projection statement.
+// Tests that _id can be included in an exclusion projection statement.
 assert.commandWorked(coll.createIndex({"$**": 1, "another": 1},
                                       {"wildcardProjection": {"_id": 1, "a": 0, "another": 0}}));
+assert.commandWorked(
+    coll.createIndex({"$**": 1, "yetAnother": 1}, {"wildcardProjection": {"_id": 1}}));
 
-// Tests we wildcard projections allow nested objects.
+// Tests wildcard projections allow nested objects.
 assert.commandWorked(
     coll.createIndex({"$**": 1, "d": 1}, {"wildcardProjection": {"a": {"b": 1, "c": 1}}}));
 
@@ -63,10 +65,55 @@ assert.commandFailedWithCode(coll.createIndex({"a.$**": 1, b: 1}, {expireAfterSe
 // 'wildcardProjection' is not specified.
 assert.commandFailedWithCode(coll.createIndex({a: 1, "$**": 1}), 67);
 
+// Tests that the wildcardProjection cannot be empty.
+assert.commandFailedWithCode(coll.createIndex({a: 1, "$**": 1}, {wildcardProjection: {}}),
+                             ErrorCodes.FailedToParse);
+
+// Tests that a wildcardProjection is not allowed for non-wildcard indexes.
+assert.commandFailedWithCode(coll.createIndex({a: 1, b: 1}, {wildcardProjection: {c: 1}}),
+                             ErrorCodes.BadValue);
+
+// Tests that wildcardProjection cannot include regular index fields.
+assert.commandFailedWithCode(coll.createIndex({a: 1, "$**": 1}, {wildcardProjection: {a: 1}}),
+                             7246208);
+assert.commandFailedWithCode(coll.createIndex({a: 1, "$**": 1}, {wildcardProjection: {a: 1, b: 1}}),
+                             7246208);
+assert.commandFailedWithCode(
+    coll.createIndex({a: 1, "$**": 1, b: 1}, {wildcardProjection: {a: 1, b: 1}}), 7246208);
+assert.commandFailedWithCode(coll.createIndex({_id: 1, "$**": 1}, {wildcardProjection: {_id: 1}}),
+                             7246208);
+
+// Tests that a wildcardProjection can only mix inclusion/exclusion projections with _id.
+assert.commandFailedWithCode(
+    coll.createIndex({"$**": 1, c: 1}, {wildcardProjection: {_id: 0, a: 0, b: 1}}), 7246211);
+assert.commandFailedWithCode(
+    coll.createIndex({"$**": 1, c: 1}, {wildcardProjection: {_id: 1, a: 0, b: 1}}), 7246211);
+assert.commandWorked(coll.createIndex({"$**": 1, "c": 1}, {wildcardProjection: {"_id": 0, b: 1}}));
+assert.commandWorked(coll.createIndex({"$**": 1, "e": 1}, {wildcardProjection: {"_id": 1, e: 0}}));
+assert.commandFailedWithCode(
+    coll.createIndex({"$**": 1, "_id": 1, "e": 1}, {wildcardProjection: {"_id": 1, e: 0}}),
+    7246209,
+);
+assert.commandWorked(
+    coll.createIndex({"$**": 1, "_id": 1, "e": 1}, {wildcardProjection: {"_id": 0, a: 1}}));
+
 // Tests that wildcard projections accept only numeric values.
 assert.commandFailedWithCode(
     coll.createIndex({"st": 1, "$**": 1}, {wildcardProjection: {"a": "something"}}), 51271);
 
+// Tests that just excluding _id is not valid in the wildcardProjection, unless the regular part is
+// _id.
+assert.commandFailedWithCode(coll.createIndex({"a": 1, "$**": 1}, {wildcardProjection: {"_id": 0}}),
+                             7246210);
+assert.commandFailedWithCode(coll.createIndex({"$**": 1, "a": 1}, {wildcardProjection: {"_id": 0}}),
+                             7246210);
+assert.commandFailedWithCode(
+    coll.createIndex({"b": 1, "$**": 1, "a": 1}, {wildcardProjection: {"_id": 0}}), 7246210);
+assert.commandWorked(coll.createIndex({"_id": 1, "$**": 1}, {wildcardProjection: {"_id": 0}}));
+assert.commandWorked(coll.createIndex({"$**": 1, "_id": 1}, {wildcardProjection: {"_id": 0}}));
+assert.commandFailedWithCode(
+    coll.createIndex({"_id": 1, "$**": 1, "a": 1}, {wildcardProjection: {"_id": 0}}), 7246210);
+
 // Tests that all compound wildcard indexes in the catalog can be validated by running validate()
 // command.
 
@@ -75,6 +122,54 @@ assert.commandWorked(coll.createIndex({a: 1, "b.$**": 1, str: 1}));
 assert.commandWorked(coll.createIndex({"b.$**": 1, str: 1}));
 assert.commandWorked(coll.createIndex({a: 1, "b.$**": 1}));
 assert.commandWorked(coll.createIndex({"$**": 1}));
+assert.commandWorked(
+    coll.createIndex({"b": 1, "$**": 1}, {wildcardProjection: {"_id": 0, "a": 1}}));
+assert.commandWorked(
+    coll.createIndex({"_id": 1, "a": 1, "$**": 1}, {wildcardProjection: {"_id": 0, "a": 0}}));
+assert.commandFailedWithCode(
+    coll.createIndex({"a": 1, "_id": 1, "$**": 1}, {wildcardProjection: {"a": 0}}), 7246209);
+assert.commandFailedWithCode(
+    coll.createIndex({"a": 1, "b": 1, "$**": 1}, {wildcardProjection: {"b": 0}}), 7246209);
+assert.commandWorked(
+    coll.createIndex({"a": 1, "$**": 1}, {wildcardProjection: {"_id": 0, "a": 0}}));
+assert.commandWorked(
+    coll.createIndex({"e": 1, "$**": 1}, {wildcardProjection: {"_id": 1, "f": 1}}));
+assert.commandFailedWithCode(
+    coll.createIndex({"b": 1, "$**": 1}, {wildcardProjection: {"_id": 0, "a": 0}}), 7246210);
+assert.commandFailedWithCode(
+    coll.createIndex({"a": 1, "$**": 1}, {wildcardProjection: {"_id": 0, "a": 1}}), 7246208);
+assert.commandFailedWithCode(
+    coll.createIndex({"a": 1, "$**": 1}, {wildcardProjection: {"_id": 1, "a": 1}}), 7246208);
+assert.commandFailedWithCode(
+    coll.createIndex({"_id": 1, "$**": 1}, {wildcardProjection: {"_id": 1, "a": 1}}), 7246208);
+assert.commandFailedWithCode(
+    coll.createIndex({"$**": 1, "d": 1}, {wildcardProjection: {"_id": 1, e: 0}}), 7246209);
+
+// Dotted paths
+assert.commandWorked(
+    coll.createIndex({"_id": 1, "a.b": 1, "$**": 1}, {wildcardProjection: {"_id": 0, "a": 0}}));
+assert.commandFailedWithCode(
+    coll.createIndex({"_id": 1, "a": 1, "$**": 1}, {wildcardProjection: {"_id": 0, "a.b": 0}}),
+    7246209,
+);
+assert.commandWorked(
+    coll.createIndex({"a.b": 1, "$**": 1}, {wildcardProjection: {"_id": 0, "a": 0}}));
+assert.commandFailedWithCode(
+    coll.createIndex({"a": 1, "$**": 1}, {wildcardProjection: {"_id": 0, "a.b": 0}}), 7246209);
+assert.commandWorked(
+    coll.createIndex({"$**": 1, "_id": 1, "e.b": 1}, {wildcardProjection: {"_id": 0, "a.b": 1}}));
+assert.commandWorked(
+    coll.createIndex({"b.c": 1, "$**": 1}, {wildcardProjection: {"_id": 0, "a": 1}}));
+assert.commandWorked(
+    coll.createIndex({"b.d": 1, "$**": 1}, {wildcardProjection: {"_id": 0, "a.b": 1}}));
+assert.commandWorked(
+    coll.createIndex({"$**": 1, "e.b": 1}, {wildcardProjection: {"_id": 1, e: 0}}));
+assert.commandFailedWithCode(
+    coll.createIndex({"$**": 1, "e": 1}, {wildcardProjection: {"_id": 1, "e.b": 0}}), 7246209);
+assert.commandFailedWithCode(
+    coll.createIndex({"$**": 1, "e.c": 1}, {wildcardProjection: {"_id": 1, "e.b": 0}}),
+    7246210,
+);
 
 // Insert documents to index.
 for (let i = 0; i < 10; i++) {

jstests/multiVersion/genericBinVersion/invalid_wildcard_index_log_at_startup.js

@@ -0,0 +1,94 @@
+/**
+ * Tests that a server containing an invalid wildcard index will log a warning on startup.
+ *
+ * @tags: [
+ *     requires_persistence,
+ *     requires_replication,
+ * ]
+ */
+
+// This is a version that allows the bad index to be created.
+const oldVersion = "8.0.16";
+
+// Standalone mongod
+{
+    const testName = "invalid_wildcard_index_log_at_startup";
+    const dbpath = MongoRunner.dataPath + testName;
+    const collName = "collectionWithInvalidWildcardIndex";
+
+    {
+        // Startup mongod version where we are allowed to create the invalid index.
+        const conn = MongoRunner.runMongod({dbpath: dbpath, binVersion: oldVersion});
+        assert.neq(null, conn, "mongod was unable to start up");
+
+        const testDB = conn.getDB("test");
+        assert.commandWorked(testDB[collName].insert({a: 1}));
+
+        // Invalid index
+        assert.commandWorked(testDB[collName].createIndex({"a": 1, "$**": 1}, {wildcardProjection: {"_id": 0}}));
+
+        MongoRunner.stopMongod(conn);
+    }
+    {
+        const conn = MongoRunner.runMongod({dbpath: dbpath, noCleanData: true});
+        assert.neq(null, conn, "mongod was unable to start up");
+        const testDB = conn.getDB("test");
+
+        const cmdRes = assert.commandWorked(testDB.adminCommand({getLog: "startupWarnings"}));
+        assert(
+            /Found a compound wildcard index with an invalid wildcardProjection. Such indexes can no longer be created./.test(
+                cmdRes.log,
+            ),
+        );
+
+        // Be sure that inserting to the collection with the invalid index succeeds.
+        assert.commandWorked(testDB[collName].insert({a: 2}));
+
+        // Inserting to another collection should succeed.
+        assert.commandWorked(testDB.someOtherCollection.insert({a: 1}));
+        assert.eq(testDB.someOtherCollection.find().itcount(), 1);
+
+        MongoRunner.stopMongod(conn);
+    }
+}
+
+// Replica set
+{
+    let nodes = {
+        n1: {binVersion: oldVersion},
+        n2: {binVersion: oldVersion},
+    };
+
+    const rst = new ReplSetTest({nodes: nodes});
+    rst.startSet();
+    rst.initiate();
+
+    let primary = rst.getPrimary();
+    const db = primary.getDB("test");
+    const coll = db.t;
+    assert.commandWorked(coll.insert({a: 1}));
+
+    assert.commandWorked(coll.createIndex({"a": 1, "$**": 1}, {wildcardProjection: {"_id": 0}}));
+
+    // Force checkpoint in storage engine to ensure index is part of the catalog in
+    // in finished state at startup.
+    rst.awaitReplication();
+    let secondary = rst.getSecondary();
+    assert.commandWorked(secondary.adminCommand({fsync: 1}));
+
+    // Check that initial sync works, this node would not allow the index to be created
+    // (since it is on a version with the new validation logic) but should not fail on startup.
+    const initialSyncNode = rst.add({rsConfig: {priority: 0}});
+    rst.reInitiate();
+    rst.awaitSecondaryNodes(null, [initialSyncNode]);
+
+    // Restart the new node and check for the startup warning in the logs.
+    rst.restart(initialSyncNode);
+    rst.awaitSecondaryNodes(null, [initialSyncNode]);
+
+    checkLog.containsJson(initialSyncNode, 11389700, {
+        ns: coll.getFullName(),
+    });
+
+    rst.stopSet();
+}

src/mongo/db/catalog/index_catalog_impl.cpp

@@ -69,6 +69,7 @@
 #include "mongo/db/index/index_descriptor.h"
 #include "mongo/db/index/s2_access_method.h"
 #include "mongo/db/index/s2_bucket_access_method.h"
+#include "mongo/db/index/wildcard_validation.h"
 #include "mongo/db/index_names.h"
 #include "mongo/db/matcher/expression.h"
 #include "mongo/db/matcher/expression_parser.h"
@@ -251,6 +252,23 @@ void IndexCatalogImpl::init(OperationContext* opCtx,
                           "spec"_attr = spec);
         }
 
+        // Look for an invalid compound wildcard index.
+        if (IndexNames::findPluginName(keyPattern) == IndexNames::WILDCARD &&
+            keyPattern.nFields() > 1 && spec.hasField("wildcardProjection")) {
+            auto validationStatus =
+                validateWildcardProjection(keyPattern, spec.getObjectField("wildcardProjection"));
+            if (!validationStatus.isOK()) {
+                LOGV2_OPTIONS(11389700,
+                              {logv2::LogTag::kStartupWarnings},
+                              "Found a compound wildcard index with an invalid wildcardProjection. "
+                              "Such indexes can no longer be created.",
+                              "ns"_attr = collection->ns(),
+                              "uuid"_attr = collection->uuid(),
+                              "index"_attr = indexName,
+                              "spec"_attr = spec);
+            }
+        }
+
         auto descriptor = IndexDescriptor(_getAccessMethodName(keyPattern), spec);
 
         if (spec.hasField(IndexDescriptor::kExpireAfterSecondsFieldName)) {

src/mongo/db/index/wildcard_validation.cpp

@@ -49,7 +49,7 @@ namespace mongo {
 namespace {
 static const StringData idFieldName = "_id";
 /*
- * Validate that wildcatdProject fields have no overlapping. It takes a sorted list of the
+ * Validate that wildcardProjection fields do not overlap. It takes a sorted list of the
  * projection fields.
  */
 Status validateOverlappingFieldsInWildcardProjectionOnly(
@@ -123,7 +123,7 @@ Status validateWildcardIndex(const BSONObj& keyPattern) {
 
 Status validateWildcardProjection(const BSONObj& keyPattern, const BSONObj& pathProjection) {
     if (pathProjection.isEmpty()) {
-        return {ErrorCodes::Error{7246205}, "WildcardProjection must be non-empty if specified."};
+        return {ErrorCodes::Error{7246205}, "WildcardProjection must be non-empty if specified"};
     }
 
     // Prepare data for validation.
@@ -165,8 +165,41 @@ Status validateWildcardProjection(const BSONObj& keyPattern, const BSONObj& path
         return status;
     }
 
-    // test overlappings between index keys and wildcard projection
-    {
+    // The wildcardProjection cannot combine inclusion and exclusion statements, with the exception
+    // that _id may be excluded for inclusion projections and included for exclusion projections.
+    if (!projectionIncludedFields.empty() && !projectionExcludedFields.empty()) {
+        const FieldRef idFieldRef{idFieldName};
+        const bool idOnlyExclusion =
+            projectionExcludedFields.size() == 1 && projectionExcludedFields.front() == idFieldRef;
+        const bool idOnlyInclusion =
+            projectionIncludedFields.size() == 1 && projectionIncludedFields.front() == idFieldRef;
+
+        // In order for the projection to be valid when there are both inclusions and exclusions,
+        // _id has to be the sole field whose inclusion/exclusion value does not match the others.
+        if (idOnlyExclusion && idOnlyInclusion) {
+            return {ErrorCodes::Error{11368500},
+                    "The wildcard projection both excludes and includes _id"};
+        } else if (!idOnlyExclusion && !idOnlyInclusion) {
+            return {ErrorCodes::Error{7246211},
+                    "The wildcardProjection cannot combine inclusion and exclusion statements, "
+                    "with the exception that _id may be excluded for inclusion projections and "
+                    "included for exclusion projections"};
+        }
+
+        // If _id is the only excluded field, ignore the exclusion in the checks below. For example,
+        // we can treat {_id: 0, a: 1} as just {a: 1}. In wildcard indexes (unlike regular
+        // projections) _id is excluded by default.
+        if (idOnlyExclusion) {
+            projectionExcludedFields.clear();
+        } else {
+            // Here idOnlyInclusion is implied from the checks above. Similarly, ignore an _id-only
+            // inclusion.
+            projectionIncludedFields.clear();
+        }
+    }
+
+    // There cannot be overlap between the index keys and the wildcard projection's inclusions.
+    if (!projectionIncludedFields.empty()) {
         auto indexPos = indexFields.begin();
         auto projectionPos = projectionIncludedFields.begin();
         while (indexPos != indexFields.end() && projectionPos != projectionIncludedFields.end()) {
@@ -175,8 +208,8 @@ Status validateWildcardProjection(const BSONObj& keyPattern, const BSONObj& path
                         str::stream()
                             << "Index Key and Wildcard Projection cannot contain "
                                "overlapping fields, however '"
-                            << indexPos->dottedField() << "' index field is ovverlapping with '"
-                            << projectionPos->dottedField() << "' wildcardProjection path."};
+                            << indexPos->dottedField() << "' index field is overlapping with '"
+                            << projectionPos->dottedField() << "' wildcardProjection path"};
             }
 
             int cmp = projectionPos->compare(*indexPos);
@@ -186,14 +219,12 @@ Status validateWildcardProjection(const BSONObj& keyPattern, const BSONObj& path
                 ++indexPos;
             }
         }
-    }
+    } else {
+        tassert(11368501,
+                "Expected projectionExcludedFields to be populated",
+                !projectionExcludedFields.empty());
 
-    const FieldRef idFieldRef{idFieldName};
-    const bool idOnlyExclusion =
-        projectionExcludedFields.size() == 1 && projectionExcludedFields.front() == idFieldRef;
-
-    // test test wildcard projects exclude all regular index fields
-    if (!projectionExcludedFields.empty() && !idOnlyExclusion) {
+        // If the wildcardProjection is an exclusion, it must exclude all regular index fields.
         auto indexPos = indexFields.begin();
         auto projectionPos = projectionExcludedFields.begin();
         while (indexPos != indexFields.end() && projectionPos != projectionExcludedFields.end()) {
@@ -208,7 +239,7 @@ Status validateWildcardProjection(const BSONObj& keyPattern, const BSONObj& path
                     return {ErrorCodes::Error{7246209},
                             str::stream() << "wildcardProjection paths must exclude all regular "
                                              "index fields, however '"
-                                          << indexPos->dottedField() << "'is not excluded."};
+                                          << indexPos->dottedField() << "'is not excluded"};
                 }
             }
         }
@@ -217,22 +248,7 @@ Status validateWildcardProjection(const BSONObj& keyPattern, const BSONObj& path
             return {ErrorCodes::Error{7246210},
                     str::stream() << "wildcardProjection paths must exclude all regular "
                                      "index fields, however '"
-                                  << indexPos->dottedField() << "'is not excluded."};
-        }
-    }
-
-    // With the exception of explicitly including _id field, you cannot combine inclusion and
-    // exclusion statements in the wildcardProjection document.
-    if (!projectionIncludedFields.empty() && !projectionExcludedFields.empty()) {
-        const bool idOnlyInclusion =
-            projectionIncludedFields.size() == 1 && projectionIncludedFields.front() == idFieldRef;
-        const bool idIsSingleField = idOnlyExclusion || idOnlyInclusion;
-        if (!idIsSingleField) {
-            return {
-                ErrorCodes::Error{7246211},
-                str::stream()
-                    << "Inclusion and exclusion statements cannot combine in the "
-                       "wildcardProjection with an exception of explicitly including _id field"};
+                                  << indexPos->dottedField() << "'is not excluded"};
         }
     }
 

src/mongo/db/index/wildcard_validation_test.cpp

@@ -115,4 +115,14 @@ TEST(WildcardProjectionValidation, IdField) {
     ASSERT_OK(validateWildcardProjection(BSON("$**" << 1 << "other" << 1),
                                          BSON("_id" << 1 << "a" << 0 << "other" << 0)));
 }
+
+TEST(WildcardProjectionValidation, IdFieldExcludedWithCompoundIndex) {
+    ASSERT_NOT_OK(validateWildcardProjection(BSON("a" << 1 << "$**" << 1), BSON("_id" << 0)));
+    ASSERT_NOT_OK(validateWildcardProjection(BSON("$**" << 1 << "a" << 1), BSON("_id" << 0)));
+    ASSERT_NOT_OK(
+        validateWildcardProjection(BSON("b" << 1 << "$**" << 1 << "a" << 1), BSON("_id" << 0)));
+
+    ASSERT_OK(validateWildcardProjection(BSON("$**" << 1 << "_id" << 1), BSON("_id" << 0)));
+    ASSERT_OK(validateWildcardProjection(BSON("_id" << 1 << "$**" << 1), BSON("_id" << 0)));
+}
 }  // namespace mongo

src/mongo/db/query/plan_enumerator/plan_enumerator.cpp

@@ -1678,6 +1678,10 @@ void PlanEnumerator::tagMemo(size_t id) {
                     for (size_t j = 0; j < assign.preds.size(); ++j) {
                         MatchExpression* pred = assign.preds[j];
                         if (pred->getTag()) {
+                            tassert(11390000,
+                                    "Expected the predicate's tag to be of type OrPushdownTag",
+                                    pred->getTag()->getType() ==
+                                        MatchExpression::TagData::Type::OrPushdownTag);
                             OrPushdownTag* orPushdownTag =
                                 static_cast<OrPushdownTag*>(pred->getTag());
                             orPushdownTag->setIndexTag(new IndexTag(

src/mongo/db/query/planner_wildcard_helpers.cpp

@@ -427,6 +427,11 @@ std::pair<BSONObj, size_t> expandWildcardIndexKeyPattern(const BSONObj& wildcard
             builder.appendAs(field, expandFieldName);
             wildcardFieldPos = fieldPos;
         } else {
+            tassert(11390001,
+                    str::stream() << "Expansion of wildcard index " << wildcardKeyPattern
+                                  << " would result in duplicate field: " << expandFieldName,
+                    fieldName != expandFieldName);
+
             builder.append(field);
         }
         ++fieldPos;

src/mongo/db/query/planner_wildcard_helpers_test.cpp

@@ -51,6 +51,7 @@
 #include "mongo/idl/server_parameter_test_util.h"
 #include "mongo/unittest/assert.h"
 #include "mongo/unittest/bson_test_util.h"
+#include "mongo/unittest/death_test.h"
 #include "mongo/unittest/framework.h"
 
 namespace mongo::wildcard_planning {
@@ -233,4 +234,19 @@ TEST(PlannerWildcardHelpersTest, Expand_CompoundWildcardIndex_NumericComponents)
     ASSERT_FALSE(expandedIndexes.front().multikey);
     ASSERT_EQ(expectedMks, expandedIndexes.front().multikeyPaths);
 }
+
+DEATH_TEST(PlannerWildcardHelpersTest, InvalidIndexExpansion, "11390001") {
+    WildcardIndexEntryMock wildcardIndex{BSON("a" << 1 << "$**" << 1), BSON("_id" << 0), {}};
+    std::set<std::string> fields{"a"};
+    std::vector<IndexEntry> expandedIndexes{};
+    expandWildcardIndexEntry(*wildcardIndex.indexEntry, fields, &expandedIndexes);
+}
+
+DEATH_TEST(PlannerWildcardHelpersTest, AnotherInvalidIndexExpansion, "11390001") {
+    WildcardIndexEntryMock wildcardIndex{BSON("$**" << 1 << "a" << 1), BSON("_id" << 0), {}};
+    std::set<std::string> fields{"a"};
+    std::vector<IndexEntry> expandedIndexes{};
+    expandWildcardIndexEntry(*wildcardIndex.indexEntry, fields, &expandedIndexes);
+}
+
 }  // namespace mongo::wildcard_planning