SERVER-103028 Avoid validating feature flags during parsing when secondaries apply oplog entries (#39307)

GitOrigin-RevId: 3b81dd5d65a586a092a9ba50e1694f3bf4ead6e4

jstests/multiVersion/targetedTestsLastLtsFeatures/create_coll_validator_new_feature_no_applyops_check.js

@@ -0,0 +1,80 @@
+/**
+ * Creating a collection with a validator with expressions that are enabled on 8.0+ can interleave
+ * with FCV downgrade, such that the primary checks the query feature flag on FCV 8.0+ and
+ * successfully creates the collection. Then an oplog entry is committed with the new query feature
+ * in the validator. The secondary will try to apply the oplog entry on the downgraded FCV and fail
+ * to create the collection since the validator fails to parse.
+ *
+ * The secondary must not check feature flags when applying oplog entries and parsing expressions.
+ */
+
+import "jstests/multiVersion/libs/multi_rs.js";
+import {assertDropCollection} from "jstests/libs/collection_drop_recreate.js";
+import {configureFailPoint} from "jstests/libs/fail_point_util.js";
+import {assertCommandWorkedInParallelShell} from "jstests/libs/parallel_shell_helpers.js";
+
+// This test depends on a specific lastLTS version and should only run when 7.0 is lastLTS.
+if (lastLTSFCV !== '7.0') {
+    jsTest.log.info("Skipping test since lastLTS is greater than 7.0", lastLTSFCV);
+    quit();
+}
+
+const rst = new ReplSetTest({nodes: 2});
+rst.startSet();
+rst.initiate();
+const primary = rst.getPrimary();
+const collName = jsTestName();
+
+function runTest(validatorSpec) {
+    // Start creating a collection that has a validator with new query features.
+    // Hang it after the validator has been parsed, but before the create entry has been put in the
+    // oplog.
+    const fpCreate = configureFailPoint(primary, 'hangAfterParsingValidator');
+
+    const awaitCreate = assertCommandWorkedInParallelShell(
+        primary, primary.getDB("test"), {create: collName, validator: validatorSpec});
+    fpCreate.wait();
+
+    // Start downgrading to last LTS. Wait until the transition has started, at which point the
+    // feature flags controlling the query features will have been disabled. We need to do this in
+    // background because the downgrade will hang on the global lock barrier, which happens *after*
+    // transitioning.
+    const awaitSetFCV = assertCommandWorkedInParallelShell(
+        primary,
+        primary.getDB("admin"),
+        {setFeatureCompatibilityVersion: lastLTSFCV, confirm: true});
+    assert.soon(() => {
+        const fcvDoc = assert
+                           .commandWorked(primary.adminCommand(
+                               {getParameter: 1, featureCompatibilityVersion: 1}))
+                           .featureCompatibilityVersion;
+        return fcvDoc.version == lastLTSFCV;
+    });
+
+    // Now, let the collection creation continue and produce the create oplog entry.
+    // The secondaries are already on the last LTS FCV, so they should not re-check feature flags
+    // when applying the oplog entry, as they would be using a different FCV than the primary.
+    fpCreate.off();
+    awaitCreate();
+
+    awaitSetFCV();
+
+    // Reset for the next test.
+    assertDropCollection(primary.getDB("test"), collName);
+    assert.commandWorked(
+        primary.adminCommand({setFeatureCompatibilityVersion: latestFCV, confirm: true}));
+}
+
+// '$toUUID' checks if the feature flag is enabled with the REGISTER_X_WITH_FEATURE_FLAG macro.
+runTest({$expr: {$eq: ["$stuff", {$toUUID: "$x"}]}} /* validatorSpec */);
+
+// The 'format' argument to $convert is checked during parsing and is not allowed on FCVs below 8.0.
+runTest({
+    $expr: {$convert: {input: "$stuff", to: {type: "binData", subtype: 0}, format: "base64"}}
+} /* validatorSpec */);
+
+// TODO BACKPORT-25868 enable test
+// $meta checks if a feature flag is enabled during parsing.
+// runTest({$expr: {$meta: "score"}} /* validatorSpec */);
+
+rst.stopSet();

src/mongo/db/catalog/collection_impl.cpp

@@ -722,10 +722,6 @@ Collection::Validator CollectionImpl::parseValidator(
 
     expCtx->variables.setDefaultRuntimeConstants(opCtx);
 
-    // The MatchExpression and contained ExpressionContext created as part of the validator are
-    // owned by the Collection and will outlive the OperationContext they were created under.
-    expCtx->opCtx = nullptr;
-
     // Enforce a maximum feature version if requested.
     expCtx->maxFeatureCompatibilityVersion = maxFeatureCompatibilityVersion;
 
@@ -784,6 +780,10 @@ Collection::Validator CollectionImpl::parseValidator(
         combinedMatchExpr = std::move(explicitMatchExpr);
     }
 
+    // The MatchExpression and contained ExpressionContext created as part of the validator are
+    // owned by the Collection and will outlive the OperationContext they were created under.
+    expCtx->opCtx = nullptr;
+
     LOGV2_DEBUG(6364301,
                 5,
                 "Combined match expression",

src/mongo/db/catalog/database_impl.cpp

@@ -118,6 +118,7 @@ namespace {
 
 MONGO_FAIL_POINT_DEFINE(throwWCEDuringTxnCollCreate);
 MONGO_FAIL_POINT_DEFINE(hangBeforeLoggingCreateCollection);
+MONGO_FAIL_POINT_DEFINE(hangAfterParsingValidator);
 MONGO_FAIL_POINT_DEFINE(hangAndFailAfterCreateCollectionReservesOpTime);
 MONGO_FAIL_POINT_DEFINE(openCreateCollectionWindowFp);
 // Allows creating a buckets NS without timeseries options, as could ocurr on FCV 7.x and earlier,
@@ -1001,6 +1002,8 @@ Status DatabaseImpl::userCreateNS(OperationContext* opCtx,
         if (!statusWithMatcher.isOK()) {
             return statusWithMatcher.getStatus();
         }
+
+        hangAfterParsingValidator.pauseWhileSet();
     }
 
     Status status = validateStorageOptions(

src/mongo/db/pipeline/accumulation_statement.cpp

@@ -108,7 +108,7 @@ AccumulationStatement AccumulationStatement::parseAccumulationStatement(
     auto&& [parser, allowedWithApiStrict, allowedWithClientType, featureFlag] =
         AccumulationStatement::getParser(accName);
 
-    expCtx->throwIfFeatureFlagIsNotEnabledOnFCV(accName, featureFlag);
+    expCtx->ignoreFeatureInParserOrRejectAndThrow(accName, featureFlag);
 
     tassert(5837900, "Accumulators should only appear in a user operation", expCtx->opCtx);
     assertLanguageFeatureIsAllowed(

src/mongo/db/pipeline/document_source.cpp

@@ -146,7 +146,7 @@ list<intrusive_ptr<DocumentSource>> DocumentSource::parse(
             it != parserMap.end());
 
     auto& entry = it->second;
-    expCtx->throwIfFeatureFlagIsNotEnabledOnFCV(stageName, entry.featureFlag);
+    expCtx->ignoreFeatureInParserOrRejectAndThrow(stageName, entry.featureFlag);
 
     return it->second.parser(stageSpec, expCtx);
 }

src/mongo/db/pipeline/document_source_set_window_fields.cpp

@@ -294,7 +294,7 @@ list<intrusive_ptr<DocumentSource>> document_source_set_window_fields::create(
             expCtx,
             SortPattern{std::move(combined)},
             // We will rely on this to efficiently compute ranks.
-            {.outputSortKeyMetadata = expCtx->isBasicRankFusionEnabled()}));
+            {.outputSortKeyMetadata = expCtx->shouldParserAllowBasicRankFusion()}));
     }
 
     // $_internalSetWindowFields

src/mongo/db/pipeline/expression.cpp

@@ -290,7 +290,7 @@ intrusive_ptr<Expression> Expression::parseExpression(ExpressionContext* const e
             it != parserMap.end());
 
     auto& entry = it->second;
-    expCtx->throwIfFeatureFlagIsNotEnabledOnFCV(opName, entry.featureFlag);
+    expCtx->ignoreFeatureInParserOrRejectAndThrow(opName, entry.featureFlag);
 
     if (expCtx->opCtx) {
         assertLanguageFeatureIsAllowed(
@@ -3251,7 +3251,7 @@ void ExpressionMeta::_assertMetaFieldCompatibleWithHybridScoringFF(ExpressionCon
     static const std::set<MetaType> kHybridScoringProtectedFields = {MetaType::kScore,
                                                                      MetaType::kScoreDetails};
     const bool usesHybridScoringProtectedField = kHybridScoringProtectedFields.contains(type);
-    const bool hybridScoringFFEnabled =
+    const bool hybridScoringFFEnabled = expCtx->shouldParserIgnoreFeatureFlagCheck() ||
         feature_flags::gFeatureFlagRankFusionFull.isEnabledUseLastLTSFCVWhenUninitialized(
             serverGlobalParams.featureCompatibility.acquireFCVSnapshot());
     uassert(ErrorCodes::FailedToParse,
@@ -7152,7 +7152,7 @@ Expression::Parser makeConversionAlias(const StringData shortcutName,
             toType,
             // The 'format' argument to $convert is not allowed on FCVs below 8.0. On a newer
             // binary, $toString will still specify it.
-            expCtx->isFeatureFlagBinDataConvertEnabled() ? format : boost::none,
+            expCtx->shouldParserAllowBinDataConvert() ? format : boost::none,
             toSubtype);
     };
 }
@@ -7229,7 +7229,7 @@ boost::intrusive_ptr<Expression> ExpressionConvert::create(ExpressionContext* co
         format ? ExpressionConstant::create(expCtx, Value(toStringData(*format))) : nullptr,
         nullptr,
         nullptr,
-        expCtx->isFeatureFlagBinDataConvertEnabled());
+        expCtx->shouldParserAllowBinDataConvert());
 }
 
 ExpressionConvert::ExpressionConvert(ExpressionContext* const expCtx,
@@ -7262,7 +7262,7 @@ intrusive_ptr<Expression> ExpressionConvert::parse(ExpressionContext* const expC
         hangBeforeSecondFeatureFlagBinDataConvertCheck.pauseWhileSet();
     }
 
-    const bool allowBinDataConvert = expCtx->isFeatureFlagBinDataConvertEnabled();
+    const bool allowBinDataConvert = expCtx->shouldParserAllowBinDataConvert();
 
     boost::intrusive_ptr<Expression> input;
     boost::intrusive_ptr<Expression> to;

src/mongo/db/pipeline/expression_context.cpp

@@ -483,4 +483,19 @@ bool ExpressionContext::isFeatureFlagBinDataConvertEnabled() {
     return _featureFlagBinDataConvertValue.get();
 }
 
+bool ExpressionContext::shouldParserAllowBinDataConvert() const {
+    return shouldParserIgnoreFeatureFlagCheck() || _featureFlagBinDataConvertValue.get();
+}
+
+bool ExpressionContext::shouldParserAllowBasicRankFusion() const {
+    return shouldParserIgnoreFeatureFlagCheck() || _featureFlagRankFusionBasic.get();
+}
+
+void ExpressionContext::ignoreFeatureInParserOrRejectAndThrow(
+    StringData name, const boost::optional<FeatureFlag>& flag) {
+    if (!shouldParserIgnoreFeatureFlagCheck()) {
+        throwIfFeatureFlagIsNotEnabledOnFCV(name, flag);
+    }
+}
+
 }  // namespace mongo

src/mongo/db/pipeline/expression_context.h

@@ -547,11 +547,29 @@ public:
     /**
      * Throws if the provided feature flag is not enabled in the current FCV or
      * 'maxFeatureCompatibilityVersion' if set. Will do nothing if the feature flag is enabled
-     * or boost::none.
+     * or boost::none.  This function assumes the caller has verified that the feature flag should
+     * be checked.
      */
     void throwIfFeatureFlagIsNotEnabledOnFCV(StringData name,
                                              const boost::optional<FeatureFlag>& flag);
 
+    /**
+     * Returns true if parsers should not check if feature flags are enabled in the current FCV or
+     * 'maxFeatureCompatibilityVersion' if set.
+     *
+     */
+    bool shouldParserIgnoreFeatureFlagCheck() const {
+        return (isParsingCollectionValidator || isParsingViewDefinition) && opCtx &&
+            !opCtx->isEnforcingConstraints();
+    }
+
+    /**
+     * Throws only if the parser should check the feature flag and the feature flag provided is not
+     * enabled in the current FCV or 'maxFeatureCompatibilityVersion' if set.
+     */
+    void ignoreFeatureInParserOrRejectAndThrow(StringData name,
+                                               const boost::optional<FeatureFlag>& flag);
+
     // The explain verbosity requested by the user, or boost::none if no explain was requested.
     boost::optional<ExplainOptions::Verbosity> explain;
 
@@ -778,10 +796,9 @@ public:
      * '_featureFlagBinDataConvertValue' for more information about this pattern.
      */
     bool isFeatureFlagBinDataConvertEnabled();
+    bool shouldParserAllowBinDataConvert() const;
 
-    bool isBasicRankFusionEnabled() const {
-        return _featureFlagRankFusionBasic.get();
-    }
+    bool shouldParserAllowBasicRankFusion() const;
 
     void setIsRankFusion() {
         _isRankFusion = true;

src/mongo/db/pipeline/window_function/window_function_expression.cpp

@@ -100,7 +100,7 @@ intrusive_ptr<Expression> Expression::parse(BSONObj obj,
                 const auto& parser = parserRegistration.parser;
                 const auto& featureFlag = parserRegistration.featureFlag;
 
-                expCtx->throwIfFeatureFlagIsNotEnabledOnFCV(exprName, featureFlag);
+                expCtx->ignoreFeatureInParserOrRejectAndThrow(exprName, featureFlag);
 
                 auto allowedWithApi = parserRegistration.allowedWithApi;
 

src/mongo/db/pipeline/window_function/window_function_expression.h

@@ -489,7 +489,7 @@ public:
                                  "exactly one element",
                 sortBy && sortBy->isSingleElementKey());
 
-        if (expCtx->isBasicRankFusionEnabled()) {
+        if (expCtx->shouldParserAllowBasicRankFusion()) {
             // The 'modern' way to do $rank is to just use the sort key. But we only support this on
             // newer versions, since we need to make sure that the $sort stage is giving us the sort
             // key.

src/mongo/db/query/sort_pattern.cpp

@@ -61,7 +61,7 @@ static const StringDataSet kValidMetaSorts{"textScore"_sd,
 
 bool isSupportedMetaSort(const boost::intrusive_ptr<ExpressionContext>& expCtx, StringData name) {
     if (name == "searchScore"_sd || name == "vectorSearchScore"_sd || name == "score"_sd) {
-        expCtx->throwIfFeatureFlagIsNotEnabledOnFCV(
+        expCtx->ignoreFeatureInParserOrRejectAndThrow(
             "sorting by searchScore, vectorSearchScore, or score",
             feature_flags::gFeatureFlagRankFusionFull);
     }