SERVER-115160 Remove stub fallback parsers from listMqlEntities (#45142)

GitOrigin-RevId: b0466ba903392d3e85f8e6aad2c5a5c700e94dfd
This commit is contained in:
Daniel Segel 2025-12-16 11:47:20 -05:00 committed by MongoDB Bot
parent 514b7bbae0
commit b006f9bfed
7 changed files with 35 additions and 41 deletions

View File

@ -474,21 +474,7 @@ const allPipelines = [
]; ];
const testedStages = new Set(allPipelines.flatMap((pipeline) => pipeline.map((obj) => Object.keys(obj)[0]))); const testedStages = new Set(allPipelines.flatMap((pipeline) => pipeline.map((obj) => Object.keys(obj)[0])));
// Confirm that every aggregation stage is either tested or explicitly skipped.
for (const aggStage of aggStages) { for (const aggStage of aggStages) {
// Confirm that every aggregation stage is either tested or explicitly skipped. assert(testedStages.has(aggStage) || skips.has(aggStage), aggStage + " has not been tested for null bytes.");
if (testedStages.has(aggStage) || skips.has(aggStage)) {
continue;
}
// If we reach here, then the aggregation stage has not been tested or skipped. We confirm that
// this stage is a stub stage that is defined for tests in aggregation_stage_stub_parsers.json.
// If not, we will trigger the assertion and should test or skip the new stage.
try {
coll.aggregate([{[aggStage]: {}}]);
assert(false, aggStage + " has not been tested for null bytes.");
} catch (e) {
if (e.code !== 10918500) {
// Stub aggregation stage error code.
assert(false, aggStage + " has not been tested for null bytes.");
}
}
} }

View File

@ -243,10 +243,6 @@ const skippedAuthTestingAggStages = [
"$sessionWindow", "$sessionWindow",
"$validate", "$validate",
"$setStreamMeta", "$setStreamMeta",
// The following stages are stubs defined in aggregation_stage_stub_parsers.json.
"$stubStage",
"$testFoo",
]; ];
// The following commands are skipped in 'authCommandsLib' because they are unable to be // The following commands are skipped in 'authCommandsLib' because they are unable to be

View File

@ -52,10 +52,9 @@ for (let i = 0; i < 10; i++) {
* This is expected for all stages that return **user** inserted documents. * This is expected for all stages that return **user** inserted documents.
* 4. skippedStages: These stages **must** either be * 4. skippedStages: These stages **must** either be
* (a) run on the admin db * (a) run on the admin db
* (b) stub stages for testing * (b) tested elsewhere
* (c) tested elsewhere * (c) stages that can only run in stream processors
* (d) stages that can only run in stream processors * (d) internal only stages (cannot be made by user requests) that run on oplog data
* (e) internal only stages (cannot be made by user requests) that run on oplog data
* *
* Once you've determined which set your new aggregation stage belongs to add a test case to the appropriate set. * Once you've determined which set your new aggregation stage belongs to add a test case to the appropriate set.
* Each set has a slightly different test format. * Each set has a slightly different test format.
@ -291,10 +290,9 @@ const unpackTests = [
// The following pipeline stages do not need to be tested for timeseries collections. // The following pipeline stages do not need to be tested for timeseries collections.
// Stages that are skipped **must** be one of the following: // Stages that are skipped **must** be one of the following:
// 1. Stages that only run on the admin database. // 1. Stages that only run on the admin database.
// 2. Stub stages that are defined for tests in aggregation_stage_stub_parsers.json. // 2. Stages that are tested elsewhere.
// 3. Stages that are tested elsewhere. // 3. Stages that can only run in stream processors.
// 4. Stages that can only run in stream processors. // 4. Stages that cannot be made by user requests and run on oplog data.
// 5. Stages that cannot be made by user requests and run on oplog data.
const skippedStages = [ const skippedStages = [
// All change stream stages are temporarily here. TODO SERVER-113494 enable tests here. // All change stream stages are temporarily here. TODO SERVER-113494 enable tests here.
"$changeStream", "$changeStream",
@ -331,10 +329,6 @@ const skippedStages = [
"$_internalAllCollectionStats", "$_internalAllCollectionStats",
"$queue", "$queue",
// Stub stages defined in 'aggregation_stage_stub_parsers.json'.
"$stubStage",
"$testFoo",
// Stages tested in 'search_disallowed_on_timeseries.js', since they require extra setup. // Stages tested in 'search_disallowed_on_timeseries.js', since they require extra setup.
"$search", "$search",
"$searchMeta", "$searchMeta",

View File

@ -79,8 +79,11 @@ ListMqlEntitiesStage::ListMqlEntitiesStage(
MqlEntityTypeEnum type, MqlEntityTypeEnum type,
const LiteParsedDocumentSource::ParserMap& docSourceParserMap) const LiteParsedDocumentSource::ParserMap& docSourceParserMap)
: Stage(stageName, pExpCtx), _type(type) { : Stage(stageName, pExpCtx), _type(type) {
for (auto&& [stageName, _] : docSourceParserMap) { for (auto&& [stageName, registration] : docSourceParserMap) {
_results.push_back(stageName); // Exclude this stage if it is a stub that has no primary parser.
if (registration.isExecutable()) {
_results.push_back(stageName);
}
} }
// Canonicalize output order of results. Sort in descending order so that we can use a cheap // Canonicalize output order of results. Sort in descending order so that we can use a cheap

View File

@ -92,7 +92,8 @@ void registerStubParser(std::string stageName, std::string message, FeatureFlag*
std::move(stubParser), std::move(stubParser),
featureFlag, featureFlag,
AllowedWithApiStrict::kAlways, AllowedWithApiStrict::kAlways,
AllowedWithClientType::kAny); AllowedWithClientType::kAny,
true /*isStub*/);
} }
void registerUnloadedExtensionStubParsers() { void registerUnloadedExtensionStubParsers() {

View File

@ -82,10 +82,11 @@ void LiteParsedDocumentSource::LiteParserRegistration::setPrimaryParser(LitePars
} }
void LiteParsedDocumentSource::LiteParserRegistration::setFallbackParser( void LiteParsedDocumentSource::LiteParserRegistration::setFallbackParser(
LiteParserInfo&& lpi, IncrementalRolloutFeatureFlag* ff) { LiteParserInfo&& lpi, IncrementalRolloutFeatureFlag* ff, bool isStub) {
_fallbackParser = std::move(lpi); _fallbackParser = std::move(lpi);
_primaryParserFeatureFlag = ff; _primaryParserFeatureFlag = ff;
_fallbackIsSet = true; _fallbackIsSet = true;
_isStub = isStub;
} }
bool LiteParsedDocumentSource::LiteParserRegistration::isPrimarySet() const { bool LiteParsedDocumentSource::LiteParserRegistration::isPrimarySet() const {
@ -117,7 +118,8 @@ void LiteParsedDocumentSource::registerFallbackParser(const std::string& name,
Parser parser, Parser parser,
FeatureFlag* parserFeatureFlag, FeatureFlag* parserFeatureFlag,
AllowedWithApiStrict allowedWithApiStrict, AllowedWithApiStrict allowedWithApiStrict,
AllowedWithClientType allowedWithClientType) { AllowedWithClientType allowedWithClientType,
bool isStub) {
if (parserMap.contains(name)) { if (parserMap.contains(name)) {
const auto& registration = parserMap.at(name); const auto& registration = parserMap.at(name);
@ -151,8 +153,8 @@ void LiteParsedDocumentSource::registerFallbackParser(const std::string& name,
ifrFeatureFlag != nullptr); ifrFeatureFlag != nullptr);
} }
registration.setFallbackParser({parser, allowedWithApiStrict, allowedWithClientType}, registration.setFallbackParser(
ifrFeatureFlag); {parser, allowedWithApiStrict, allowedWithClientType}, ifrFeatureFlag, isStub);
} }
void LiteParsedDocumentSource::unregisterParser_forTest(const std::string& name) { void LiteParsedDocumentSource::unregisterParser_forTest(const std::string& name) {

View File

@ -112,12 +112,20 @@ public:
void setPrimaryParser(LiteParserInfo&& lpi); void setPrimaryParser(LiteParserInfo&& lpi);
// TODO SERVER-114028 Update when fallback parsing supports all feature flags. // TODO SERVER-114028 Update when fallback parsing supports all feature flags.
void setFallbackParser(LiteParserInfo&& lpi, IncrementalRolloutFeatureFlag* ff); void setFallbackParser(LiteParserInfo&& lpi,
IncrementalRolloutFeatureFlag* ff,
bool isStub = false);
bool isPrimarySet() const; bool isPrimarySet() const;
bool isFallbackSet() const; bool isFallbackSet() const;
// Returns true if the parser is executable, meaning it has either a primary or a non-stub
// fallback.
bool isExecutable() const {
return _primaryIsSet || !_isStub;
}
private: private:
// The preferred method of parsing this LiteParsedDocumentSource. If the feature flag is // The preferred method of parsing this LiteParsedDocumentSource. If the feature flag is
// enabled, the primary parser will be used to parse the stage. // enabled, the primary parser will be used to parse the stage.
@ -137,6 +145,9 @@ public:
// Whether or not the fallback parser has been registered or not. // Whether or not the fallback parser has been registered or not.
bool _fallbackIsSet = false; bool _fallbackIsSet = false;
// Whether the fallback parser is a stub parser that just throws an error.
bool _isStub = false;
}; };
using ParserMap = StringMap<LiteParsedDocumentSource::LiteParserRegistration>; using ParserMap = StringMap<LiteParsedDocumentSource::LiteParserRegistration>;
@ -170,7 +181,8 @@ public:
Parser parser, Parser parser,
FeatureFlag* parserFeatureFlag, FeatureFlag* parserFeatureFlag,
AllowedWithApiStrict allowedWithApiStrict, AllowedWithApiStrict allowedWithApiStrict,
AllowedWithClientType allowedWithClientType); AllowedWithClientType allowedWithClientType,
bool isStub = false);
/** /**
* Function that will be used as an alternate parser for a document source that has been * Function that will be used as an alternate parser for a document source that has been