diff options
| -rw-r--r-- | jstests/aggregation/illegal_reference_in_match.js | 36 | ||||
| -rw-r--r-- | src/mongo/db/pipeline/pipeline_d.cpp | 7 |
2 files changed, 40 insertions, 3 deletions
diff --git a/jstests/aggregation/illegal_reference_in_match.js b/jstests/aggregation/illegal_reference_in_match.js new file mode 100644 index 00000000000..7ef6c904406 --- /dev/null +++ b/jstests/aggregation/illegal_reference_in_match.js @@ -0,0 +1,36 @@ +// This is intended to reproduce SERVER-39109. The test ensures that when a field path which is +// illegal inside the aggregation system is used in a $match that is not pushed down to the query +// system, the correct error is raised. +(function() { + "use strict"; + + const coll = db.illegal_reference_in_match; + assert.commandWorked(coll.insert({a: 1})); + + const pipeline = [ + // The limit stage prevents the planner from pushing the match into the query layer. + {$limit: 10}, + + // 'a.$c' is an illegal path in the aggregation system (though it is legal in the query + // system). The $limit above forces this $match to run as an aggregation stage, so the path + // will be interpreted as illegal. + {$match: {"a.$c": 4}}, + + // This inclusion-projection allows the planner to determine that the only necessary fields + // we need to fetch from the document are "_id" (by default), "a.$c" (since we do a match + // on it) and "dummy" since we include/rename it as part of this $project. + + // The reason we need to explicitly include a "dummy" field, rather than just including + // "a.$c" is that, as mentioned before, a.$c is an illegal path in the aggregation system, + // so if we use it as part of the project, the $project will fail to parse (and the + // relevant code will not be exercised). + { + $project: { + "newAndUnrelatedField": "$dummy", + } + } + ]; + + const err = assert.throws(() => coll.aggregate(pipeline)); + assert.eq(err.code, 16410); +})(); diff --git a/src/mongo/db/pipeline/pipeline_d.cpp b/src/mongo/db/pipeline/pipeline_d.cpp index c99b3acadd2..f56359d761f 100644 --- a/src/mongo/db/pipeline/pipeline_d.cpp +++ b/src/mongo/db/pipeline/pipeline_d.cpp @@ -517,10 +517,12 @@ void PipelineD::addCursorSource(Collection* collection, intrusive_ptr<DocumentSourceCursor> pSource = DocumentSourceCursor::create(collection, std::move(exec), expCtx); - // Note the query, sort, and projection for explain. + // Add the cursor to the pipeline first so that it's correctly disposed of as part of the + // pipeline if an exception is thrown during this method. + pipeline->addInitialSource(pSource); + pSource->setQuery(queryObj); pSource->setSort(sortObj); - if (deps.hasNoRequirements()) { pSource->shouldProduceEmptyDocs(); } @@ -537,7 +539,6 @@ void PipelineD::addCursorSource(Collection* collection, pSource->setProjection(deps.toProjection(), deps.toParsedDeps()); } - pipeline->addInitialSource(pSource); } Timestamp PipelineD::getLatestOplogTimestamp(const Pipeline* pipeline) { |