diff options
author | James Wahlin <james@mongodb.com> | 2022-03-02 15:15:05 -0500 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2022-04-06 14:21:24 +0000 |
commit | ba218f4a7fd63bf898cef934be794a4cdb8b0c51 (patch) | |
tree | 13dac5f48b51c917a2336b907a001792041cc373 | |
parent | 67a97eebee3470f6c04a628e6c5b1b8fa9684424 (diff) | |
download | mongo-ba218f4a7fd63bf898cef934be794a4cdb8b0c51.tar.gz |
SERVER-63010 Ensure that unpacking measurements doesn't overwrite pushed down addFields that are computed on meta data
(cherry picked from commit 3a8cf3d2d1c6f668607756ab0b972f9ca2148f18)
-rw-r--r-- | jstests/core/timeseries/timeseries_project.js | 34 | ||||
-rw-r--r-- | src/mongo/db/exec/inclusion_projection_executor.cpp | 21 | ||||
-rw-r--r-- | src/mongo/db/exec/inclusion_projection_executor_test.cpp | 19 |
3 files changed, 73 insertions, 1 deletions
diff --git a/jstests/core/timeseries/timeseries_project.js b/jstests/core/timeseries/timeseries_project.js index 22769863bd4..4958ded193a 100644 --- a/jstests/core/timeseries/timeseries_project.js +++ b/jstests/core/timeseries/timeseries_project.js @@ -87,4 +87,36 @@ assert.docEq(result, [{_id: 0, time: docDate, a: {b: 1}, b: 4, c: [{}, {}]}]); // Test that an exclude does not overwrite meta field pushdown. result = coll.aggregate([{$unset: "b"}, {$set: {b: "$meta"}}]).toArray(); assert.docEq(result, [{_id: 0, time: docDate, meta: 4, a: {b: 1}, b: 4, c: [{}, {}]}]); -})();
\ No newline at end of file + +// Test that a field reference in a projection refers to the stage's input document +// rather than another field with the same name in the projection. +(function() { +const regColl = db.timeseries_project_reg; +regColl.drop(); + +const tsColl = db.timeseries_project_ts; +tsColl.drop(); +assert.commandWorked( + db.createCollection(tsColl.getName(), {timeseries: {timeField: 'time', metaField: 'x'}})); + +const doc = { + time: new Date("2019-10-11T14:39:18.670Z"), + x: 5, + a: 3, +}; +assert.commandWorked(tsColl.insert(doc)); +assert.commandWorked(regColl.insert(doc)); + +// Test $project. +let pipeline = [{$project: {_id: 0, a: "$x", b: "$a"}}]; +let tsDoc = tsColl.aggregate(pipeline).toArray(); +let regDoc = regColl.aggregate(pipeline).toArray(); +assert.docEq(tsDoc, regDoc); + +// Test $addFields. +pipeline = [{$addFields: {a: "$x", b: "$a"}}, {$project: {_id: 0}}]; +tsDoc = tsColl.aggregate(pipeline).toArray(); +regDoc = regColl.aggregate(pipeline).toArray(); +assert.docEq(tsDoc, regDoc); +})(); +})(); diff --git a/src/mongo/db/exec/inclusion_projection_executor.cpp b/src/mongo/db/exec/inclusion_projection_executor.cpp index e330f868ca6..8826703a440 100644 --- a/src/mongo/db/exec/inclusion_projection_executor.cpp +++ b/src/mongo/db/exec/inclusion_projection_executor.cpp @@ -108,6 +108,10 @@ std::pair<BSONObj, bool> InclusionNode::extractComputedProjectionsInProject( if (_policies.computedFieldsPolicy != ComputedFieldsPolicy::kAllowComputedFields) { return {BSONObj{}, false}; } + + DepsTracker allDeps; + reportDependencies(&allDeps); + // Auxiliary vector with extracted computed projections: <name, expression, replacement // strategy>. If the replacement strategy flag is true, the expression is replaced with a // projected field. If it is false - the expression is replaced with an identity projection. @@ -120,6 +124,13 @@ std::pair<BSONObj, bool> InclusionNode::extractComputedProjectionsInProject( replaceWithProjField = false; continue; } + if (allDeps.fields.count(field) > 0) { + // Do not extract a computed projection if its name is the same as a dependent field. If + // the extracted $addFields were to be placed before this projection, the dependency + // with the common name would be shadowed by the computed projection. + replaceWithProjField = false; + continue; + } auto expressionIt = _expressions.find(field); if (expressionIt == _expressions.end()) { // After seeing the first dotted path expression we need to replace computed @@ -185,6 +196,10 @@ std::pair<BSONObj, bool> InclusionNode::extractComputedProjectionsInAddFields( if (_policies.computedFieldsPolicy != ComputedFieldsPolicy::kAllowComputedFields) { return {BSONObj{}, false}; } + + DepsTracker allDeps; + reportDependencies(&allDeps); + // Auxiliary vector with extracted computed projections: <name, expression>. // To preserve the original fields order, only projections at the beginning of the // _orderToProcessAdditionsAndChildren list can be extracted for pushdown. @@ -194,6 +209,12 @@ std::pair<BSONObj, bool> InclusionNode::extractComputedProjectionsInAddFields( if (reservedNames.count(field) > 0) { break; } + if (allDeps.fields.count(field) > 0) { + // Do not extract a computed projection if its name is the same as a dependent field. If + // the extracted $addFields were to be placed before this $addFields, the dependency + // with the common name would be shadowed by the computed projection. + break; + } auto expressionIt = _expressions.find(field); if (expressionIt == _expressions.end()) { break; diff --git a/src/mongo/db/exec/inclusion_projection_executor_test.cpp b/src/mongo/db/exec/inclusion_projection_executor_test.cpp index 029b3b091c1..63892baa546 100644 --- a/src/mongo/db/exec/inclusion_projection_executor_test.cpp +++ b/src/mongo/db/exec/inclusion_projection_executor_test.cpp @@ -1071,6 +1071,25 @@ TEST_F(InclusionProjectionExecutionTestWithFallBackToDefault, ExtractComputedPro ASSERT_DOCUMENT_EQ(expectedProjection, inclusion->serializeTransformation(boost::none)); } +TEST_F(InclusionProjectionExecutionTestWithFallBackToDefault, + ExtractComputedProjectionInProjectShouldNotHideDependentFields) { + auto inclusion = makeInclusionProjectionWithDefaultPolicies(BSON("a" + << "$myMeta" + << "b" + << "$a")); + + auto r = static_cast<InclusionProjectionExecutor*>(inclusion.get())->getRoot(); + const std::set<StringData> reservedNames{}; + auto [addFields, deleteFlag] = + r->extractComputedProjectionsInProject("myMeta", "meta", reservedNames); + + ASSERT_EQ(addFields.nFields(), 0); + ASSERT_EQ(deleteFlag, false); + + auto expectedProjection = Document(fromjson("{_id: true, a: '$myMeta', b: '$a'}")); + ASSERT_DOCUMENT_EQ(expectedProjection, inclusion->serializeTransformation(boost::none)); +} + TEST_F(InclusionProjectionExecutionTestWithFallBackToDefault, ApplyProjectionAfterSplit) { auto inclusion = makeInclusionProjectionWithDefaultPolicies( BSON("a" << true << "computedMeta1" |