diff options
author | Nick Zolnierz <nicholas.zolnierz@mongodb.com> | 2017-06-30 11:48:35 -0400 |
---|---|---|
committer | Nick Zolnierz <nicholas.zolnierz@mongodb.com> | 2017-07-07 13:19:15 -0400 |
commit | 23f44557852e61349fd28505ec6b953e22024d8b (patch) | |
tree | 5898f879e8a5db71ee300cd2ded70e388e4ac21e /src | |
parent | be4c4f1fffe6ca69fb67ee872b52b3bd4e630659 (diff) | |
download | mongo-23f44557852e61349fd28505ec6b953e22024d8b.tar.gz |
SERVER-29587: Partition MatchExpression types into categories, fixes an
issue with previous commit where {min/max}Items was categorized as a
leaf.
Diffstat (limited to 'src')
-rw-r--r-- | src/mongo/db/commands/list_databases.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression.h | 66 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_algo.cpp | 38 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_algo_test.cpp | 42 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_array.h | 4 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_leaf.h | 4 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_tree.h | 8 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_where_base.h | 4 | ||||
-rw-r--r-- | src/mongo/db/pipeline/document_source_match.cpp | 12 | ||||
-rw-r--r-- | src/mongo/db/pipeline/pipeline_test.cpp | 14 | ||||
-rw-r--r-- | src/mongo/db/query/index_bounds_builder.h | 4 | ||||
-rw-r--r-- | src/mongo/db/query/indexability.h | 4 | ||||
-rw-r--r-- | src/mongo/db/query/parsed_projection.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/query/plan_cache_indexability.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/query/plan_enumerator.h | 2 | ||||
-rw-r--r-- | src/mongo/db/query/planner_access.cpp | 5 | ||||
-rw-r--r-- | src/mongo/db/query/planner_ixselect.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/update/array_filter.cpp | 70 |
18 files changed, 171 insertions, 118 deletions
diff --git a/src/mongo/db/commands/list_databases.cpp b/src/mongo/db/commands/list_databases.cpp index 4b96d386577..37c0f0c0590 100644 --- a/src/mongo/db/commands/list_databases.cpp +++ b/src/mongo/db/commands/list_databases.cpp @@ -119,7 +119,9 @@ public: vector<BSONObj> dbInfos; - bool filterNameOnly = filter && filter->isLeaf() && filter->path() == kNameField; + bool filterNameOnly = filter && + filter->getCategory() == MatchExpression::MatchCategory::kLeaf && + filter->path() == kNameField; intmax_t totalSize = 0; for (vector<string>::iterator i = dbNames.begin(); i != dbNames.end(); ++i) { const string& dbname = *i; diff --git a/src/mongo/db/matcher/expression.h b/src/mongo/db/matcher/expression.h index b6a2701702f..2c99218bdf7 100644 --- a/src/mongo/db/matcher/expression.h +++ b/src/mongo/db/matcher/expression.h @@ -150,57 +150,19 @@ public: return StringData(); } - /** - * Notes on structure: - * isLogical, isArray, and isLeaf define three partitions of all possible operators. - * - * isLogical can have children and its children can be arbitrary operators. - * - * isArray can have children and its children are predicates over one field. - * - * isLeaf is a predicate over one field. - */ - - /** - * Returns true if this match expression node operates logically on its children. - * For example, the AND node is logical in that it returns true only if all of its - * children return true. - */ - bool isLogical() const { - switch (_matchType) { - case AND: - case OR: - case NOT: - case NOR: - case INTERNAL_SCHEMA_XOR: - return true; - default: - return false; - } - } - - /** - * Is this node an array operator? Array operators have multiple clauses but operate on one - * field. - * - * ELEM_MATCH_VALUE, ELEM_MATCH_OBJECT, SIZE (ArrayMatchingMatchExpression) - */ - bool isArray() const { - return SIZE == _matchType || ELEM_MATCH_VALUE == _matchType || - ELEM_MATCH_OBJECT == _matchType; - } + enum class MatchCategory { + // Expressions that are leaves on the AST, these do not have any children. + kLeaf, + // Logical Expressions such as $and, $or, etc. that do not have a path and may have + // one or more children. + kLogical, + // Expressions that operate on arrays only. + kArrayMatching, + // Expressions that don't fall into any particular bucket. + kOther, + }; - /** - * Not-internal nodes, predicates over one field. Almost all of these inherit from - * LeafMatchExpression. - * - * Exceptions: WHERE, which doesn't have a field. - * TYPE_OPERATOR, which inherits from MatchExpression due to unique array - * semantics. - */ - bool isLeaf() const { - return !isArray() && !isLogical(); - } + virtual MatchCategory getCategory() const = 0; // XXX: document virtual std::unique_ptr<MatchExpression> shallowClone() const = 0; @@ -320,6 +282,10 @@ public: return other->matchType() == ALWAYS_FALSE; } + MatchCategory getCategory() const final { + return MatchCategory::kOther; + } + private: StringData _path; }; diff --git a/src/mongo/db/matcher/expression_algo.cpp b/src/mongo/db/matcher/expression_algo.cpp index 0979eb7b475..609eef099e6 100644 --- a/src/mongo/db/matcher/expression_algo.cpp +++ b/src/mongo/db/matcher/expression_algo.cpp @@ -276,11 +276,12 @@ unique_ptr<MatchExpression> createNorOfNodes(std::vector<unique_ptr<MatchExpress } void applyRenamesToExpression(MatchExpression* expr, const StringMap<std::string>& renames) { - if (expr->isArray()) { + if (expr->getCategory() == MatchExpression::MatchCategory::kArrayMatching || + expr->getCategory() == MatchExpression::MatchCategory::kOther) { return; } - if (expr->isLeaf()) { + if (expr->getCategory() == MatchExpression::MatchCategory::kLeaf) { auto it = renames.find(expr->path()); if (it != renames.end()) { LeafMatchExpression* leafExpr = checked_cast<LeafMatchExpression*>(expr); @@ -300,7 +301,7 @@ splitMatchExpressionByWithoutRenames(unique_ptr<MatchExpression> expr, // 'expr' does not depend upon 'fields', so it can be completely moved. return {std::move(expr), nullptr}; } - if (!expr->isLogical()) { + if (expr->getCategory() != MatchExpression::MatchCategory::kLogical) { // 'expr' is a leaf, and was not independent of 'fields'. return {nullptr, std::move(expr)}; } @@ -410,23 +411,28 @@ bool isSubsetOf(const MatchExpression* lhs, const MatchExpression* rhs) { } bool isIndependentOf(const MatchExpression& expr, const std::set<std::string>& pathSet) { - if (expr.isLogical()) { - // Any logical expression is independent of 'pathSet' if all its children are independent of - // 'pathSet'. - for (size_t i = 0; i < expr.numChildren(); i++) { - if (!isIndependentOf(*expr.getChild(i), pathSet)) { - return false; + switch (expr.getCategory()) { + case MatchExpression::MatchCategory::kLogical: { + // Any logical expression is independent of 'pathSet' if all its children are + // independent of 'pathSet'. + for (size_t i = 0; i < expr.numChildren(); i++) { + if (!isIndependentOf(*expr.getChild(i), pathSet)) { + return false; + } } + return true; + } + case MatchExpression::MatchCategory::kLeaf: { + return isLeafIndependentOf(expr.path(), pathSet); + } + // All other match expressions are never considered independent. + case MatchExpression::MatchCategory::kArrayMatching: + case MatchExpression::MatchCategory::kOther: { + return false; } - return true; - } - - // Array match expressions are never considered independent. - if (expr.isArray()) { - return false; } - return isLeafIndependentOf(expr.path(), pathSet); + MONGO_UNREACHABLE; } std::pair<unique_ptr<MatchExpression>, unique_ptr<MatchExpression>> splitMatchExpressionBy( diff --git a/src/mongo/db/matcher/expression_algo_test.cpp b/src/mongo/db/matcher/expression_algo_test.cpp index e02e41c27cc..36a4ea84506 100644 --- a/src/mongo/db/matcher/expression_algo_test.cpp +++ b/src/mongo/db/matcher/expression_algo_test.cpp @@ -1100,6 +1100,42 @@ TEST(SplitMatchExpression, ShouldNotMoveSizeAcrossRename) { ASSERT_BSONOBJ_EQ(secondBob.obj(), fromjson("{a: {$size: 3}}")); } +TEST(SplitMatchExpression, ShouldNotMoveMinItemsAcrossRename) { + BSONObj matchPredicate = fromjson("{a: {$_internalSchemaMinItems: 3}}"); + const CollatorInterface* collator = nullptr; + auto matcher = MatchExpressionParser::parse(matchPredicate, ExtensionsCallbackNoop(), collator); + ASSERT_OK(matcher.getStatus()); + + StringMap<std::string> renames{{"a", "c"}}; + std::pair<unique_ptr<MatchExpression>, unique_ptr<MatchExpression>> splitExpr = + expression::splitMatchExpressionBy(std::move(matcher.getValue()), {}, renames); + + ASSERT_FALSE(splitExpr.first.get()); + + ASSERT_TRUE(splitExpr.second.get()); + BSONObjBuilder secondBob; + splitExpr.second->serialize(&secondBob); + ASSERT_BSONOBJ_EQ(secondBob.obj(), fromjson("{a: {$_internalSchemaMinItems: 3}}")); +} + +TEST(SplitMatchExpression, ShouldNotMoveMaxItemsAcrossRename) { + BSONObj matchPredicate = fromjson("{a: {$_internalSchemaMaxItems: 3}}"); + const CollatorInterface* collator = nullptr; + auto matcher = MatchExpressionParser::parse(matchPredicate, ExtensionsCallbackNoop(), collator); + ASSERT_OK(matcher.getStatus()); + + StringMap<std::string> renames{{"a", "c"}}; + std::pair<unique_ptr<MatchExpression>, unique_ptr<MatchExpression>> splitExpr = + expression::splitMatchExpressionBy(std::move(matcher.getValue()), {}, renames); + + ASSERT_FALSE(splitExpr.first.get()); + + ASSERT_TRUE(splitExpr.second.get()); + BSONObjBuilder secondBob; + splitExpr.second->serialize(&secondBob); + ASSERT_BSONOBJ_EQ(secondBob.obj(), fromjson("{a: {$_internalSchemaMaxItems: 3}}")); +} + TEST(MapOverMatchExpression, DoesMapOverLogicalNodes) { BSONObj matchPredicate = fromjson("{a: {$not: {$eq: 1}}}"); const CollatorInterface* collator = nullptr; @@ -1110,7 +1146,8 @@ TEST(MapOverMatchExpression, DoesMapOverLogicalNodes) { bool hasLogicalNode = false; expression::mapOver(swMatchExpression.getValue().get(), [&hasLogicalNode](MatchExpression* expression, std::string path) -> void { - if (expression->isLogical()) { + if (expression->getCategory() == + MatchExpression::MatchCategory::kLogical) { hasLogicalNode = true; } }); @@ -1128,7 +1165,8 @@ TEST(MapOverMatchExpression, DoesMapOverLeafNodes) { bool hasLeafNode = false; expression::mapOver(swMatchExpression.getValue().get(), [&hasLeafNode](MatchExpression* expression, std::string path) -> void { - if (!expression->isLogical()) { + if (expression->getCategory() != + MatchExpression::MatchCategory::kLogical) { hasLeafNode = true; } }); diff --git a/src/mongo/db/matcher/expression_array.h b/src/mongo/db/matcher/expression_array.h index e121788a091..931e6403e1e 100644 --- a/src/mongo/db/matcher/expression_array.h +++ b/src/mongo/db/matcher/expression_array.h @@ -62,6 +62,10 @@ public: return _path; } + MatchCategory getCategory() const final { + return MatchCategory::kArrayMatching; + } + private: StringData _path; ElementPath _elementPath; diff --git a/src/mongo/db/matcher/expression_leaf.h b/src/mongo/db/matcher/expression_leaf.h index 346ba831492..e88e09f4ce2 100644 --- a/src/mongo/db/matcher/expression_leaf.h +++ b/src/mongo/db/matcher/expression_leaf.h @@ -73,6 +73,10 @@ public: Status setPath(StringData path); + MatchCategory getCategory() const final { + return MatchCategory::kLeaf; + } + private: StringData _path; ElementPath _elementPath; diff --git a/src/mongo/db/matcher/expression_tree.h b/src/mongo/db/matcher/expression_tree.h index 7432e5a15f8..0d759eed65e 100644 --- a/src/mongo/db/matcher/expression_tree.h +++ b/src/mongo/db/matcher/expression_tree.h @@ -89,6 +89,10 @@ public: bool equivalent(const MatchExpression* other) const; + MatchCategory getCategory() const final { + return MatchCategory::kLogical; + } + protected: void _debugList(StringBuilder& debug, int level) const; @@ -221,6 +225,10 @@ public: _exp.reset(newChild); } + MatchCategory getCategory() const final { + return MatchCategory::kLogical; + } + private: std::unique_ptr<MatchExpression> _exp; }; diff --git a/src/mongo/db/matcher/expression_where_base.h b/src/mongo/db/matcher/expression_where_base.h index ac845559bb2..62e8f1ac8f1 100644 --- a/src/mongo/db/matcher/expression_where_base.h +++ b/src/mongo/db/matcher/expression_where_base.h @@ -58,6 +58,10 @@ public: bool equivalent(const MatchExpression* other) const final; + MatchCategory getCategory() const final { + return MatchCategory::kOther; + } + protected: const std::string& getCode() const { return _code; diff --git a/src/mongo/db/pipeline/document_source_match.cpp b/src/mongo/db/pipeline/document_source_match.cpp index 7606bdcf1b0..1ce429bb6be 100644 --- a/src/mongo/db/pipeline/document_source_match.cpp +++ b/src/mongo/db/pipeline/document_source_match.cpp @@ -426,9 +426,9 @@ boost::intrusive_ptr<DocumentSourceMatch> DocumentSourceMatch::descendMatchOnPat // Cannot call this method on a $match including a $elemMatch. invariant(node->matchType() != MatchExpression::ELEM_MATCH_OBJECT && node->matchType() != MatchExpression::ELEM_MATCH_VALUE); - // Logical nodes do not have a path, but both 'leaf' and 'array' nodes - // do. - if (node->isLogical()) { + // Only leaf and array match expressions have a path. + if (node->getCategory() != MatchExpression::MatchCategory::kLeaf && + node->getCategory() != MatchExpression::MatchCategory::kArrayMatching) { return; } @@ -436,11 +436,11 @@ boost::intrusive_ptr<DocumentSourceMatch> DocumentSourceMatch::descendMatchOnPat invariant(expression::isPathPrefixOf(descendOn, leafPath)); auto newPath = leafPath.substr(descendOn.size() + 1); - if (node->isLeaf() && node->matchType() != MatchExpression::TYPE_OPERATOR && - node->matchType() != MatchExpression::WHERE) { + if (node->getCategory() == MatchExpression::MatchCategory::kLeaf && + node->matchType() != MatchExpression::TYPE_OPERATOR) { auto leafNode = static_cast<LeafMatchExpression*>(node); leafNode->setPath(newPath).transitional_ignore(); - } else if (node->isArray()) { + } else if (node->getCategory() == MatchExpression::MatchCategory::kArrayMatching) { auto arrayNode = static_cast<ArrayMatchingMatchExpression*>(node); arrayNode->setPath(newPath).transitional_ignore(); } diff --git a/src/mongo/db/pipeline/pipeline_test.cpp b/src/mongo/db/pipeline/pipeline_test.cpp index 6408f191fa4..dc440ecb62b 100644 --- a/src/mongo/db/pipeline/pipeline_test.cpp +++ b/src/mongo/db/pipeline/pipeline_test.cpp @@ -928,6 +928,20 @@ TEST(PipelineOptimizationTest, MatchCannotSwapWithSortLimit) { assertPipelineOptimizesAndSerializesTo(inputPipe, outputPipe, inputPipe); } +TEST(PipelineOptimizationTest, MatchOnMinItemsShouldNotMoveAcrossRename) { + string pipeline = + "[{$project: {_id: true, a: '$b'}}, " + "{$match: {a: {$_internalSchemaMinItems: 1}}}]"; + assertPipelineOptimizesTo(pipeline, pipeline); +} + +TEST(PipelineOptimizationTest, MatchOnMaxItemsShouldNotMoveAcrossRename) { + string pipeline = + "[{$project: {_id: true, a: '$b'}}, " + "{$match: {a: {$_internalSchemaMaxItems: 1}}}]"; + assertPipelineOptimizesTo(pipeline, pipeline); +} + } // namespace Local namespace Sharded { diff --git a/src/mongo/db/query/index_bounds_builder.h b/src/mongo/db/query/index_bounds_builder.h index b397f81b295..6a4a74a94b0 100644 --- a/src/mongo/db/query/index_bounds_builder.h +++ b/src/mongo/db/query/index_bounds_builder.h @@ -75,8 +75,8 @@ public: * * If 'expr' is elemMatch, the index tag is affixed to a child. * - * The expression must be a predicate over one field. That is, expr->isLeaf() or - * expr->isArray() must be true, and expr->isLogical() must be false. + * The expression must be a predicate over one field. That is, expression category must be + * kLeaf or kArrayMatching. */ static void translate(const MatchExpression* expr, const BSONElement& elt, diff --git a/src/mongo/db/query/indexability.h b/src/mongo/db/query/indexability.h index 1e3bf0b17b0..6861be067f6 100644 --- a/src/mongo/db/query/indexability.h +++ b/src/mongo/db/query/indexability.h @@ -59,7 +59,7 @@ public: * Example: a: {$elemMatch: {$gte: 1, $lte: 1}}. */ static bool arrayUsesIndexOnOwnField(const MatchExpression* me) { - if (!me->isArray()) { + if (me->getCategory() != MatchExpression::MatchCategory::kArrayMatching) { return false; } @@ -111,7 +111,7 @@ public: * Example: a: {$elemMatch: {b:1, c:1}}. */ static bool arrayUsesIndexOnChildren(const MatchExpression* me) { - return me->isArray() && MatchExpression::ELEM_MATCH_OBJECT == me->matchType(); + return MatchExpression::ELEM_MATCH_OBJECT == me->matchType(); } /** diff --git a/src/mongo/db/query/parsed_projection.cpp b/src/mongo/db/query/parsed_projection.cpp index cc92f8a9691..d83b9e786ee 100644 --- a/src/mongo/db/query/parsed_projection.cpp +++ b/src/mongo/db/query/parsed_projection.cpp @@ -380,7 +380,7 @@ bool ParsedProjection::_isPositionalOperator(const char* fieldName) { // static bool ParsedProjection::_hasPositionalOperatorMatch(const MatchExpression* const query, const std::string& matchfield) { - if (query->isLogical()) { + if (query->getCategory() == MatchExpression::MatchCategory::kLogical) { for (unsigned int i = 0; i < query->numChildren(); ++i) { if (_hasPositionalOperatorMatch(query->getChild(i), matchfield)) { return true; diff --git a/src/mongo/db/query/plan_cache_indexability.cpp b/src/mongo/db/query/plan_cache_indexability.cpp index 96840021d33..a4646714abd 100644 --- a/src/mongo/db/query/plan_cache_indexability.cpp +++ b/src/mongo/db/query/plan_cache_indexability.cpp @@ -68,7 +68,7 @@ void PlanCacheIndexabilityState::processPartialIndex(const std::string& indexNam for (size_t i = 0; i < filterExpr->numChildren(); ++i) { processPartialIndex(indexName, filterExpr->getChild(i)); } - if (!filterExpr->isLogical()) { + if (filterExpr->getCategory() != MatchExpression::MatchCategory::kLogical) { _pathDiscriminatorsMap[filterExpr->path()][indexName].addDiscriminator( [filterExpr](const MatchExpression* queryExpr) { return expression::isSubsetOf(queryExpr, filterExpr); diff --git a/src/mongo/db/query/plan_enumerator.h b/src/mongo/db/query/plan_enumerator.h index 97a7549e6ad..f2a49865bf8 100644 --- a/src/mongo/db/query/plan_enumerator.h +++ b/src/mongo/db/query/plan_enumerator.h @@ -101,7 +101,7 @@ public: * provided CanonicalQuery. * * Nodes in 'tree' are tagged with indices that should be used to answer the tagged nodes. - * Only nodes that have a field name (isLogical() == false) will be tagged. + * Only nodes that have a field name (getCategory() != kLogical) will be tagged. * * The output tree is a clone identical to that used to initialize the enumerator, with tags * added in order to indicate index usage. diff --git a/src/mongo/db/query/planner_access.cpp b/src/mongo/db/query/planner_access.cpp index b83f4bbe9b0..cd35a45d4aa 100644 --- a/src/mongo/db/query/planner_access.cpp +++ b/src/mongo/db/query/planner_access.cpp @@ -1154,7 +1154,8 @@ QuerySolutionNode* QueryPlannerAccess::buildIndexedDataAccess(const CanonicalQue bool inArrayOperator, const vector<IndexEntry>& indices, const QueryPlannerParams& params) { - if (root->isLogical() && !Indexability::isBoundsGeneratingNot(root)) { + if (root->getCategory() == MatchExpression::MatchCategory::kLogical && + !Indexability::isBoundsGeneratingNot(root)) { if (MatchExpression::AND == root->matchType()) { // Takes ownership of root. return buildIndexedAnd(query, root, inArrayOperator, indices, params); @@ -1174,8 +1175,6 @@ QuerySolutionNode* QueryPlannerAccess::buildIndexedDataAccess(const CanonicalQue autoRoot.reset(root); } - // isArray or isLeaf is true. Either way, it's over one field, and the bounds builder - // deals with it. if (NULL == root->getTag()) { // No index to use here, not in the context of logical operator, so we're SOL. return NULL; diff --git a/src/mongo/db/query/planner_ixselect.cpp b/src/mongo/db/query/planner_ixselect.cpp index 321e856d1b7..b32dc7c8b1a 100644 --- a/src/mongo/db/query/planner_ixselect.cpp +++ b/src/mongo/db/query/planner_ixselect.cpp @@ -143,7 +143,7 @@ void QueryPlannerIXSelect::getFields(const MatchExpression* node, for (size_t i = 0; i < node->numChildren(); ++i) { getFields(node->getChild(i), prefix, out); } - } else if (node->isLogical()) { + } else if (node->getCategory() == MatchExpression::MatchCategory::kLogical) { for (size_t i = 0; i < node->numChildren(); ++i) { getFields(node->getChild(i), prefix, out); } @@ -411,7 +411,7 @@ void QueryPlannerIXSelect::rateIndices(MatchExpression* node, for (size_t i = 0; i < node->numChildren(); ++i) { rateIndices(node->getChild(i), prefix, indices, collator); } - } else if (node->isLogical()) { + } else if (node->getCategory() == MatchExpression::MatchCategory::kLogical) { for (size_t i = 0; i < node->numChildren(); ++i) { rateIndices(node->getChild(i), prefix, indices, collator); } diff --git a/src/mongo/db/update/array_filter.cpp b/src/mongo/db/update/array_filter.cpp index f1c8c80b323..9725964ef20 100644 --- a/src/mongo/db/update/array_filter.cpp +++ b/src/mongo/db/update/array_filter.cpp @@ -45,42 +45,50 @@ const std::regex idRegex("^[a-z][a-zA-Z0-9]*$"); * Finds the top-level field that 'expr' is over. The must be unique and not the empty string. */ StatusWith<StringData> parseId(MatchExpression* expr) { - if (expr->isArray() || expr->isLeaf()) { - auto firstDotPos = expr->path().find('.'); - if (firstDotPos == std::string::npos) { - return expr->path(); - } - return expr->path().substr(0, firstDotPos); - } else if (expr->isLogical()) { - if (expr->numChildren() == 0) { - return Status(ErrorCodes::FailedToParse, - "No top-level field name found in array filter."); - } - - StringData id; - for (size_t i = 0; i < expr->numChildren(); ++i) { - auto statusWithId = parseId(expr->getChild(i)); - if (!statusWithId.isOK()) { - return statusWithId.getStatus(); + switch (expr->getCategory()) { + case MatchExpression::MatchCategory::kLeaf: + case MatchExpression::MatchCategory::kArrayMatching: { + auto firstDotPos = expr->path().find('.'); + if (firstDotPos == std::string::npos) { + return expr->path(); } - - if (id == StringData()) { - id = statusWithId.getValue(); - continue; + return expr->path().substr(0, firstDotPos); + } + case MatchExpression::MatchCategory::kLogical: { + if (expr->numChildren() == 0) { + return Status(ErrorCodes::FailedToParse, + "No top-level field name found in array filter."); } - if (id != statusWithId.getValue()) { - return Status( - ErrorCodes::FailedToParse, - str::stream() - << "Each array filter must use a single top-level field name, found '" - << id - << "' and '" - << statusWithId.getValue() - << "'"); + StringData id; + for (size_t i = 0; i < expr->numChildren(); ++i) { + auto statusWithId = parseId(expr->getChild(i)); + if (!statusWithId.isOK()) { + return statusWithId.getStatus(); + } + + if (id == StringData()) { + id = statusWithId.getValue(); + continue; + } + + if (id != statusWithId.getValue()) { + return Status( + ErrorCodes::FailedToParse, + str::stream() + << "Each array filter must use a single top-level field name, found '" + << id + << "' and '" + << statusWithId.getValue() + << "'"); + } } + return id; + } + case MatchExpression::MatchCategory::kOther: { + return Status(ErrorCodes::FailedToParse, + str::stream() << "Unsupported match expression in array filter"); } - return id; } MONGO_UNREACHABLE; |