diff options
author | Yoonsoo Kim <yoonsoo.kim@mongodb.com> | 2022-05-09 21:31:28 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2022-05-24 23:41:09 +0000 |
commit | 8bcf420fbfaeb218a0bee3e5265da04c2b0f8109 (patch) | |
tree | eb682c6e9d3b997c2b1c61f39314464e59b81bcf | |
parent | 2bd6810b6afe06530969d77ffd3931978feef8c3 (diff) | |
download | mongo-8bcf420fbfaeb218a0bee3e5265da04c2b0f8109.tar.gz |
SERVER-65863 Have optimized _id expression(s) pushed down properly
(cherry picked from commit 7f55c457738556cf96236ef9c7aa4b30bbe07390)
-rw-r--r-- | jstests/noPassthroughWithMongod/group_pushdown.js | 8 | ||||
-rw-r--r-- | src/mongo/db/pipeline/document_source_group.cpp | 25 | ||||
-rw-r--r-- | src/mongo/db/pipeline/document_source_group.h | 8 |
3 files changed, 32 insertions, 9 deletions
diff --git a/jstests/noPassthroughWithMongod/group_pushdown.js b/jstests/noPassthroughWithMongod/group_pushdown.js index 1c795f76f84..5dd8ba660d7 100644 --- a/jstests/noPassthroughWithMongod/group_pushdown.js +++ b/jstests/noPassthroughWithMongod/group_pushdown.js @@ -247,6 +247,14 @@ assertResultsMatchWithAndWithoutPushdown( [{"_id": 1, "s": 2}, {"_id": 2, "s": 2}, {"_id": 4, "s": 1}], 2); +// Verifies that an optimized expression can be pushed down. +assertResultsMatchWithAndWithoutPushdown( + coll, + // {"$ifNull": [1, 2]} will be optimized into just the constant 1. + [{$group: {_id: {"$ifNull": [1, 2]}, o: {$min: "$quantity"}}}], + [{"_id": 1, o: 1}], + 1); + // Run a group with a supported $stdDevSamp accumultor and check that it gets pushed down. assertGroupPushdown(coll, [{$group: {_id: "$item", s: {$stdDevSamp: "$quantity"}}}], diff --git a/src/mongo/db/pipeline/document_source_group.cpp b/src/mongo/db/pipeline/document_source_group.cpp index 4b79d54f7c2..c561c1102b8 100644 --- a/src/mongo/db/pipeline/document_source_group.cpp +++ b/src/mongo/db/pipeline/document_source_group.cpp @@ -459,9 +459,6 @@ intrusive_ptr<Expression> parseIdExpression(const intrusive_ptr<ExpressionContex } // namespace void DocumentSourceGroup::setIdExpression(const boost::intrusive_ptr<Expression> idExpression) { - // Remember the whole _id expression so that we can translate it to a SBE stage tree as a whole. - _idExpression = idExpression; - if (auto object = dynamic_cast<ExpressionObject*>(idExpression.get())) { auto& childExpressions = object->getChildExpressions(); invariant(!childExpressions.empty()); // We expect to have converted an empty object into a @@ -479,6 +476,28 @@ void DocumentSourceGroup::setIdExpression(const boost::intrusive_ptr<Expression> } } +boost::intrusive_ptr<Expression> DocumentSourceGroup::getIdExpression() const { + // _idFieldNames is empty and _idExpressions has one element when the _id expression is not an + // object expression. + if (_idFieldNames.empty() && _idExpressions.size() == 1) { + return _idExpressions[0]; + } + + tassert(6586300, + "Field and its expression must be always paired in ExpressionObject", + _idFieldNames.size() > 0 && _idFieldNames.size() == _idExpressions.size()); + + // Each expression in '_idExpressions' may have been optimized and so, compose the object _id + // expression out of the optimized expressions. + std::vector<std::pair<std::string, boost::intrusive_ptr<Expression>>> fieldsAndExprs; + for (size_t i = 0; i < _idExpressions.size(); ++i) { + fieldsAndExprs.emplace_back(_idFieldNames[i], _idExpressions[i]); + } + + return ExpressionObject::create(_idExpressions[0]->getExpressionContext(), + std::move(fieldsAndExprs)); +} + intrusive_ptr<DocumentSource> DocumentSourceGroup::createFromBson( BSONElement elem, const intrusive_ptr<ExpressionContext>& expCtx) { uassert(15947, "a group's fields must be specified in an object", elem.type() == Object); diff --git a/src/mongo/db/pipeline/document_source_group.h b/src/mongo/db/pipeline/document_source_group.h index 72fd2832bf1..edffed6758b 100644 --- a/src/mongo/db/pipeline/document_source_group.h +++ b/src/mongo/db/pipeline/document_source_group.h @@ -145,9 +145,7 @@ public: /** * Returns the expression to use to determine the group id of each document. */ - const boost::intrusive_ptr<Expression> getIdExpression() const { - return _idExpression; - } + boost::intrusive_ptr<Expression> getIdExpression() const; /** * Returns true if this $group stage represents a 'global' $group which is merging together @@ -284,13 +282,11 @@ private: std::shared_ptr<Sorter<Value, Value>::File> _file; - // The expression for the '_id' field. - boost::intrusive_ptr<Expression> _idExpression; // If the expression for the '_id' field represents a non-empty object, we track its fields' // names in '_idFieldNames'. std::vector<std::string> _idFieldNames; // Expressions for the individual fields when '_id' produces a document in the order of - // '_idFieldNames' or the whole expression if '_id' produces a scalar value. + // '_idFieldNames' or the whole expression otherwise. std::vector<boost::intrusive_ptr<Expression>> _idExpressions; bool _initialized; |