diff options
Diffstat (limited to 'src')
22 files changed, 488 insertions, 38 deletions
diff --git a/src/mongo/db/matcher/SConscript b/src/mongo/db/matcher/SConscript index 495aa5ac1bd..0b9b64c99b4 100644 --- a/src/mongo/db/matcher/SConscript +++ b/src/mongo/db/matcher/SConscript @@ -31,6 +31,7 @@ env.Library( source=[ 'expression.cpp', 'expression_array.cpp', + 'expression_expr.cpp', 'expression_geo.cpp', 'expression_leaf.cpp', 'expression_parser.cpp', @@ -80,6 +81,7 @@ env.CppUnitTest( source=[ 'expression_always_boolean_test.cpp', 'expression_array_test.cpp', + 'expression_expr_test.cpp', 'expression_geo_test.cpp', 'expression_leaf_test.cpp', 'expression_parser_geo_test.cpp', diff --git a/src/mongo/db/matcher/expression.cpp b/src/mongo/db/matcher/expression.cpp index 1e3f284f664..5039b805745 100644 --- a/src/mongo/db/matcher/expression.cpp +++ b/src/mongo/db/matcher/expression.cpp @@ -67,4 +67,23 @@ void MatchExpression::setCollator(const CollatorInterface* collator) { _doSetCollator(collator); } + +void MatchExpression::addDependencies(DepsTracker* deps) const { + for (size_t i = 0; i < numChildren(); ++i) { + + // Don't recurse through MatchExpression nodes which require an entire array or entire + // subobject for matching. + const auto type = matchType(); + switch (type) { + case MatchExpression::ELEM_MATCH_VALUE: + case MatchExpression::ELEM_MATCH_OBJECT: + case MatchExpression::INTERNAL_SCHEMA_OBJECT_MATCH: + continue; + default: + getChild(i)->addDependencies(deps); + } + } + + _doAddDependencies(deps); +} } diff --git a/src/mongo/db/matcher/expression.h b/src/mongo/db/matcher/expression.h index c9eeb38463b..49c032232ef 100644 --- a/src/mongo/db/matcher/expression.h +++ b/src/mongo/db/matcher/expression.h @@ -37,6 +37,7 @@ #include "mongo/bson/bsonobjbuilder.h" #include "mongo/db/matcher/match_details.h" #include "mongo/db/matcher/matchable.h" +#include "mongo/db/pipeline/dependencies.h" #include "mongo/stdx/memory.h" namespace mongo { @@ -84,6 +85,7 @@ public: TYPE_OPERATOR, GEO, WHERE, + EXPRESSION, // Boolean expressions. ALWAYS_FALSE, @@ -244,6 +246,11 @@ public: void setCollator(const CollatorInterface* collator); /** + * Add the fields required for matching to 'deps'. + */ + void addDependencies(DepsTracker* deps) const; + + /** * Serialize the MatchExpression to BSON, appending to 'out'. Output of this method is expected * to be a valid query object, that, when parsed, produces a logically equivalent * MatchExpression. @@ -263,6 +270,8 @@ protected: */ virtual void _doSetCollator(const CollatorInterface* collator){}; + virtual void _doAddDependencies(DepsTracker* deps) const {} + void _debugAddSpace(StringBuilder& debug, int level) const; private: diff --git a/src/mongo/db/matcher/expression_expr.cpp b/src/mongo/db/matcher/expression_expr.cpp new file mode 100644 index 00000000000..8344ffbca7c --- /dev/null +++ b/src/mongo/db/matcher/expression_expr.cpp @@ -0,0 +1,77 @@ +/** + * Copyright (C) 2017 MongoDB Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the GNU Affero General Public License in all respects for + * all of the code used other than as permitted herein. If you modify file(s) + * with this exception, you may extend this exception to your version of the + * file(s), but you are not obligated to do so. If you do not wish to do so, + * delete this exception statement from your version. If you delete this + * exception statement from all source files in the program, then also delete + * it in the license file. + */ + +#include "mongo/platform/basic.h" + +#include "mongo/db/matcher/expression_expr.h" + +namespace mongo { +ExprMatchExpression::ExprMatchExpression(BSONElement elem, + const boost::intrusive_ptr<ExpressionContext>& expCtx) + : MatchExpression(MatchType::EXPRESSION), + _expCtx(expCtx), + _expression(Expression::parseOperand(expCtx, elem, expCtx->variablesParseState)) {} + +bool ExprMatchExpression::matches(const MatchableDocument* doc, MatchDetails* details) const { + Document document(doc->toBSON()); + auto value = _expression->evaluate(document); + return value.coerceToBool(); +} + +void ExprMatchExpression::serialize(BSONObjBuilder* out) const { + *out << "$expr" << _expression->serialize(false); +} + +bool ExprMatchExpression::equivalent(const MatchExpression* other) const { + if (other->matchType() != matchType()) { + return false; + } + + const ExprMatchExpression* realOther = static_cast<const ExprMatchExpression*>(other); + + if (!CollatorInterface::collatorsMatch(_expCtx->getCollator(), + realOther->_expCtx->getCollator())) { + return false; + } + + // TODO SERVER-30982: Add mechanism to allow for checking Expression equivalency. + return ValueComparator().evaluate(_expression->serialize(false) == + realOther->_expression->serialize(false)); +} + + +std::unique_ptr<MatchExpression> ExprMatchExpression::shallowClone() const { + // TODO SERVER-31003: Replace Expression clone via serialization with Expression::clone(). + BSONObjBuilder bob; + bob << "" << _expression->serialize(false); + boost::intrusive_ptr<Expression> clonedExpr = + Expression::parseOperand(_expCtx, bob.obj().firstElement(), _expCtx->variablesParseState); + + return stdx::make_unique<ExprMatchExpression>(std::move(clonedExpr), _expCtx); +} +} // namespace mongo diff --git a/src/mongo/db/matcher/expression_expr.h b/src/mongo/db/matcher/expression_expr.h new file mode 100644 index 00000000000..1177b7ef360 --- /dev/null +++ b/src/mongo/db/matcher/expression_expr.h @@ -0,0 +1,87 @@ +/** + * Copyright (C) 2017 MongoDB Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the GNU Affero General Public License in all respects for + * all of the code used other than as permitted herein. If you modify file(s) + * with this exception, you may extend this exception to your version of the + * file(s), but you are not obligated to do so. If you do not wish to do so, + * delete this exception statement from your version. If you delete this + * exception statement from all source files in the program, then also delete + * it in the license file. + */ + +#pragma once + +#include <vector> + +#include "mongo/db/matcher/expression.h" +#include "mongo/db/matcher/expression_tree.h" +#include "mongo/db/pipeline/expression.h" +#include "mongo/db/pipeline/expression_context.h" + +namespace mongo { + +/** + * MatchExpression for the top-level $expr keyword. Take an expression as an argument, evaluates and + * coerces to boolean form which determines whether a document is a match. + */ +class ExprMatchExpression final : public MatchExpression { +public: + ExprMatchExpression(BSONElement elem, const boost::intrusive_ptr<ExpressionContext>& expCtx); + + ExprMatchExpression(boost::intrusive_ptr<Expression> expr, + const boost::intrusive_ptr<ExpressionContext>& expCtx) + : MatchExpression(MatchType::EXPRESSION), _expCtx(expCtx), _expression(expr) {} + + bool matchesSingleElement(const BSONElement& e, MatchDetails* details = nullptr) const final { + MONGO_UNREACHABLE; + } + + bool matches(const MatchableDocument* doc, MatchDetails* details = nullptr) const final; + + std::unique_ptr<MatchExpression> shallowClone() const final; + + void debugString(StringBuilder& debug, int level = 0) const final { + _debugAddSpace(debug, level); + debug << "$expr " << _expression->serialize(false).toString(); + } + + void serialize(BSONObjBuilder* out) const final; + + bool equivalent(const MatchExpression* other) const final; + + MatchCategory getCategory() const final { + return MatchCategory::kOther; + } + +private: + void _doAddDependencies(DepsTracker* deps) const final { + if (_expression) { + _expression->addDependencies(deps); + } + } + + boost::intrusive_ptr<ExpressionContext> _expCtx; + + // TODO SERVER-30991: '_expression' should be optimized as part of MatchExpression::optimize(). + boost::intrusive_ptr<Expression> _expression; +}; + + +} // namespace mongo diff --git a/src/mongo/db/matcher/expression_expr_test.cpp b/src/mongo/db/matcher/expression_expr_test.cpp new file mode 100644 index 00000000000..e0dfae1bb38 --- /dev/null +++ b/src/mongo/db/matcher/expression_expr_test.cpp @@ -0,0 +1,129 @@ +/** + * Copyright (C) 2017 MongoDB Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the GNU Affero General Public License in all respects for + * all of the code used other than as permitted herein. If you modify file(s) + * with this exception, you may extend this exception to your version of the + * file(s), but you are not obligated to do so. If you do not wish to do so, + * delete this exception statement from your version. If you delete this + * exception statement from all source files in the program, then also delete + * it in the license file. + */ + +#include "mongo/platform/basic.h" + +#include "mongo/db/matcher/expression.h" +#include "mongo/db/matcher/expression_expr.h" +#include "mongo/db/matcher/matcher.h" +#include "mongo/db/pipeline/aggregation_context_fixture.h" +#include "mongo/unittest/unittest.h" + +namespace mongo { + +namespace { + +TEST(ExprMatchExpression, ComparisonToConstantMatchesCorrectly) { + const boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + const CollatorInterface* kSimpleCollator = nullptr; + + auto match = BSON("a" << 5); + auto notMatch = BSON("a" << 6); + + auto expression1 = BSON("$expr" << BSON("$eq" << BSON_ARRAY("$a" << 5))); + Matcher matcher1(expression1, + kSimpleCollator, + expCtx, + ExtensionsCallbackNoop(), + MatchExpressionParser::kAllowAllSpecialFeatures); + ASSERT_TRUE(matcher1.matches(match)); + ASSERT_FALSE(matcher1.matches(notMatch)); + + auto varId = expCtx->variablesParseState.defineVariable("var"); + expCtx->variables.setValue(varId, Value(5)); + auto expression2 = BSON("$expr" << BSON("$eq" << BSON_ARRAY("$a" + << "$$var"))); + Matcher matcher2(expression2, + kSimpleCollator, + expCtx, + ExtensionsCallbackNoop(), + MatchExpressionParser::kAllowAllSpecialFeatures); + ASSERT_TRUE(matcher2.matches(match)); + ASSERT_FALSE(matcher2.matches(notMatch)); +} + +TEST(ExprMatchExpression, ComparisonBetweenTwoFieldPathsMatchesCorrectly) { + const boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + + auto expression = BSON("$expr" << BSON("$gt" << BSON_ARRAY("$a" + << "$b"))); + auto match = BSON("a" << 10 << "b" << 2); + auto notMatch = BSON("a" << 2 << "b" << 10); + + const CollatorInterface* kSimpleCollator = nullptr; + Matcher matcher(expression, + kSimpleCollator, + expCtx, + ExtensionsCallbackNoop(), + MatchExpressionParser::kAllowAllSpecialFeatures); + + ASSERT_TRUE(matcher.matches(match)); + ASSERT_FALSE(matcher.matches(notMatch)); +} + +TEST(ExprMatchExpression, ComparisonThrowsWithUnboundVariable) { + const boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + auto expression = BSON("$expr" << BSON("$eq" << BSON_ARRAY("$a" + << "$$var"))); + ASSERT_THROWS(ExprMatchExpression pipelineExpr(expression.firstElement(), expCtx), DBException); +} + +// TODO SERVER-30991: Uncomment once MatchExpression::optimize() is in place and handles +// optimization of the Expression held by ExprMatchExpression. Also add a second expression, +// BSON("$expr" << "$$var"), with $$var bound to 4 to confirm it optimizes to {$const: 4} as +// well. +/* +TEST(ExprMatchExpression, IdenticalPostOptimizedExpressionsAreEquivalent) { + BSONObj expression = BSON("$expr" << BSON("$multiply" << BSON_ARRAY(2 << 2))); + BSONObj expressionEquiv = BSON("$expr" << BSON("$const" << 4)); + BSONObj expressionNotEquiv = BSON("$expr" << BSON("$const" << 10)); + + const boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + ExprMatchExpression pipelineExpr(expression.firstElement(), expCtx); + pipelineExpr::optimize(); + ASSERT_TRUE(pipelineExpr.equivalent(&pipelineExpr)); + + ExprMatchExpression pipelineExprEquiv(expressionEquiv.firstElement(), expCtx); + ASSERT_TRUE(pipelineExpr.equivalent(&pipelineExprEquiv)); + + ExprMatchExpression pipelineExprNotEquiv(expressionNotEquiv.firstElement(), expCtx); + ASSERT_FALSE(pipelineExpr.equivalent(&pipelineExprNotEquiv)); +} +*/ + +TEST(ExprMatchExpression, ShallowClonedExpressionIsEquivalentToOriginal) { + BSONObj expression = BSON("$expr" << BSON("$eq" << BSON_ARRAY("$a" << 5))); + + const boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + ExprMatchExpression pipelineExpr(expression.firstElement(), expCtx); + auto shallowClone = pipelineExpr.shallowClone(); + ASSERT_TRUE(pipelineExpr.equivalent(shallowClone.get())); +} + +} // namespace +} // namespace mongo diff --git a/src/mongo/db/matcher/expression_parser.cpp b/src/mongo/db/matcher/expression_parser.cpp index 08d8712ad83..ba80393ccc8 100644 --- a/src/mongo/db/matcher/expression_parser.cpp +++ b/src/mongo/db/matcher/expression_parser.cpp @@ -42,6 +42,7 @@ #include "mongo/db/commands/feature_compatibility_version_command_parser.h" #include "mongo/db/matcher/expression_always_boolean.h" #include "mongo/db/matcher/expression_array.h" +#include "mongo/db/matcher/expression_expr.h" #include "mongo/db/matcher/expression_geo.h" #include "mongo/db/matcher/expression_leaf.h" #include "mongo/db/matcher/expression_tree.h" @@ -516,6 +517,15 @@ StatusWithMatchExpression MatchExpressionParser::_parse( if (!s.isOK()) return s; root->add(s.getValue().release()); + } else if (mongoutils::str::equals("expr", rest)) { + if (!topLevel) { + return {Status(ErrorCodes::BadValue, "$expr has to be at the top level")}; + } + + auto status = _parseExpr(e, allowedFeatures, expCtx); + if (!status.isOK()) + return status; + root->add(status.getValue().release()); } else if (mongoutils::str::equals("text", rest)) { if ((allowedFeatures & AllowedFeatures::kText) == 0u) { return {Status(ErrorCodes::BadValue, "$text is not allowed in this context")}; @@ -1654,6 +1664,19 @@ StatusWithMatchExpression MatchExpressionParser::_parseGeo(const char* name, } } +StatusWithMatchExpression MatchExpressionParser::_parseExpr( + BSONElement elem, + AllowedFeatureSet allowedFeatures, + const boost::intrusive_ptr<ExpressionContext>& expCtx) { + if ((allowedFeatures & AllowedFeatures::kExpr) == 0u) { + return {Status(ErrorCodes::BadValue, "$expr is not allowed in this context")}; + } + + invariant(expCtx); + + return {stdx::make_unique<ExprMatchExpression>(std::move(elem), expCtx)}; +} + namespace { // Maps from query operator string name to operator PathAcceptingKeyword. std::unique_ptr<StringMap<PathAcceptingKeyword>> queryOperatorMap; diff --git a/src/mongo/db/matcher/expression_parser.h b/src/mongo/db/matcher/expression_parser.h index dc7b5041517..f69eafbff12 100644 --- a/src/mongo/db/matcher/expression_parser.h +++ b/src/mongo/db/matcher/expression_parser.h @@ -250,6 +250,10 @@ private: const BSONObj& section, AllowedFeatureSet allowedFeatures); + StatusWithMatchExpression _parseExpr(BSONElement elem, + AllowedFeatureSet allowedFeatures, + const boost::intrusive_ptr<ExpressionContext>& expCtx); + // arrays StatusWithMatchExpression _parseElemMatch(const char* name, diff --git a/src/mongo/db/matcher/expression_parser_test.cpp b/src/mongo/db/matcher/expression_parser_test.cpp index e4d0e67b388..0caafd75f92 100644 --- a/src/mongo/db/matcher/expression_parser_test.cpp +++ b/src/mongo/db/matcher/expression_parser_test.cpp @@ -372,16 +372,14 @@ TEST(MatchExpressionParserTest, NearParsesSuccessfullyWhenAllowed) { .getStatus()); } -// TODO SERVER-30951: Convert these tests to use top-level $expr and enable them. -/* TEST(MatchExpressionParserTest, ExprFailsToParseWhenDisallowed) { - auto query = fromjson("{a: {$expr: 5}}"); + auto query = fromjson("{$expr: {$eq: ['$a', 5]}}"); const CollatorInterface* collator = nullptr; ASSERT_NOT_OK(MatchExpressionParser::parse(query, collator).getStatus()); } TEST(MatchExpressionParserTest, ExprParsesSuccessfullyWhenAllowed) { - auto query = fromjson("{a: {$expr: 5}}"); + auto query = fromjson("{$expr: {$eq: ['$a', 5]}}"); const CollatorInterface* collator = nullptr; const boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); ASSERT_OK(MatchExpressionParser::parse(query, @@ -391,5 +389,28 @@ TEST(MatchExpressionParserTest, ExprParsesSuccessfullyWhenAllowed) { MatchExpressionParser::AllowedFeatures::kExpr) .getStatus()); } -*/ + +TEST(MatchExpressionParserTest, ExprParsesSuccessfullyWithAdditionalTopLevelPredicates) { + auto query = fromjson("{x: 1, $expr: {$eq: ['$a', 5]}, y: 1}"); + const CollatorInterface* collator = nullptr; + const boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + ASSERT_OK(MatchExpressionParser::parse(query, + collator, + expCtx, + ExtensionsCallbackNoop(), + MatchExpressionParser::AllowedFeatures::kExpr) + .getStatus()); +} + +TEST(MatchExpressionParserTest, ExprFailsToParseWithinTopLevelOr) { + auto query = fromjson("{$or: [{x: 1}, {$expr: {$eq: ['$a', 5]}}]}"); + const CollatorInterface* collator = nullptr; + const boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + ASSERT_NOT_OK(MatchExpressionParser::parse(query, + collator, + expCtx, + ExtensionsCallbackNoop(), + MatchExpressionParser::AllowedFeatures::kExpr) + .getStatus()); +} } diff --git a/src/mongo/db/matcher/expression_path.h b/src/mongo/db/matcher/expression_path.h index 41c8a0addf7..6c45a9ea04d 100644 --- a/src/mongo/db/matcher/expression_path.h +++ b/src/mongo/db/matcher/expression_path.h @@ -85,6 +85,13 @@ public: return Status::OK(); } +protected: + void _doAddDependencies(DepsTracker* deps) const final { + if (!_path.empty()) { + deps->fields.insert(_path.toString()); + } + } + private: StringData _path; ElementPath _elementPath; diff --git a/src/mongo/db/matcher/expression_serialization_test.cpp b/src/mongo/db/matcher/expression_serialization_test.cpp index 2fb9fd6dc22..2d2bd3127fc 100644 --- a/src/mongo/db/matcher/expression_serialization_test.cpp +++ b/src/mongo/db/matcher/expression_serialization_test.cpp @@ -1255,6 +1255,29 @@ TEST(SerializeBasic, ExpressionWhereWithScopeSerializesCorrectly) { ASSERT_BSONOBJ_EQ(*reserialized.getQuery(), serialize(reserialized.getMatchExpression())); } +TEST(SerializeBasic, ExpressionExprSerializesCorrectly) { + boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + Matcher original(fromjson("{$expr: {$eq: ['$a', 2]}}"), + kSimpleCollator, + expCtx, + ExtensionsCallbackNoop(), + MatchExpressionParser::kAllowAllSpecialFeatures); + Matcher reserialized(serialize(original.getMatchExpression()), + kSimpleCollator, + expCtx, + ExtensionsCallbackNoop(), + MatchExpressionParser::kAllowAllSpecialFeatures); + + ASSERT_BSONOBJ_EQ(*reserialized.getQuery(), fromjson("{$expr: {$eq: ['$a', {$const: 2}]}}")); + ASSERT_BSONOBJ_EQ(*reserialized.getQuery(), serialize(reserialized.getMatchExpression())); + + BSONObj obj = fromjson("{a: 2}"); + ASSERT_EQ(original.matches(obj), reserialized.matches(obj)); + + obj = fromjson("{a: 3}"); + ASSERT_EQ(original.matches(obj), reserialized.matches(obj)); +} + TEST(SerializeBasic, ExpressionCommentSerializesCorrectly) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); Matcher original(fromjson("{$comment: 'Hello'}"), diff --git a/src/mongo/db/matcher/expression_with_placeholder_test.cpp b/src/mongo/db/matcher/expression_with_placeholder_test.cpp index 5609755d89b..3c7702e2703 100644 --- a/src/mongo/db/matcher/expression_with_placeholder_test.cpp +++ b/src/mongo/db/matcher/expression_with_placeholder_test.cpp @@ -236,15 +236,12 @@ TEST(ExpressionWithPlaceholderTest, GeoNearExpressionFailsToParse) { ASSERT_NOT_OK(status.getStatus()); } -// TODO SERVER-30951: Convert this test to use top-level $expr and enable it. -/* TEST(ExpressionWithPlaceholderTest, ExprExpressionFailsToParse) { const CollatorInterface* collator = nullptr; - auto rawFilter = fromjson("{i: {$expr: 5}}"); + auto rawFilter = fromjson("{$expr: {$eq: ['$i', 5]}}"); auto status = ExpressionWithPlaceholder::parse(rawFilter, collator); ASSERT_NOT_OK(status.getStatus()); } -*/ TEST(ExpressionWithPlaceholderTest, EquivalentIfPlaceholderAndExpressionMatch) { constexpr auto collator = nullptr; diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_allowed_properties.h b/src/mongo/db/matcher/schema/expression_internal_schema_allowed_properties.h index 11026eaee44..48503c46b0e 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_allowed_properties.h +++ b/src/mongo/db/matcher/schema/expression_internal_schema_allowed_properties.h @@ -146,6 +146,10 @@ private: */ bool _matchesBSONObj(const BSONObj& obj) const; + void _doAddDependencies(DepsTracker* deps) const final { + deps->needWholeDocument = true; + } + // The names of the properties are owned by the BSONObj used to create this match expression. // Since that BSONObj must outlive this object, we can safely store StringData. boost::container::flat_set<StringData> _properties; diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_num_properties.h b/src/mongo/db/matcher/schema/expression_internal_schema_num_properties.h index 6c26a650993..c31b0aaf5ac 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_num_properties.h +++ b/src/mongo/db/matcher/schema/expression_internal_schema_num_properties.h @@ -64,6 +64,10 @@ protected: return _numProperties; } + void _doAddDependencies(DepsTracker* deps) const final { + deps->needWholeDocument = true; + } + private: long long _numProperties; std::string _name; diff --git a/src/mongo/db/ops/modifier_pull_test.cpp b/src/mongo/db/ops/modifier_pull_test.cpp index 2870bf0ade1..471ed3c18b3 100644 --- a/src/mongo/db/ops/modifier_pull_test.cpp +++ b/src/mongo/db/ops/modifier_pull_test.cpp @@ -121,10 +121,8 @@ TEST(SimpleMod, InitWithGeoNearObjectFails) { ASSERT_EQUALS(ErrorCodes::BadValue, status); } -// TODO SERVER-30951: Convert these tests to use top-level $expr and enable them. -/* TEST(SimpleMod, InitWithExprElemFails) { - auto update = fromjson("{$pull: {a: {$expr: 5}}}"); + auto update = fromjson("{$pull: {a: {$expr: {$eq: ['$a', 5]}}}}"); const CollatorInterface* collator = nullptr; ModifierPull node; auto status = node.init(update["$pull"]["a"], ModifierInterface::Options::normal(collator)); @@ -133,14 +131,13 @@ TEST(SimpleMod, InitWithExprElemFails) { } TEST(SimpleMod, InitWithExprObjectFails) { - auto update = fromjson("{$pull: {a: {b: {$expr: 5}}}}"); + auto update = fromjson("{$pull: {a: {$expr: {$eq: ['$a', {$literal: {b: 5}}]}}}}"); const CollatorInterface* collator = nullptr; ModifierPull node; auto status = node.init(update["$pull"]["a"], ModifierInterface::Options::normal(collator)); ASSERT_NOT_OK(status); ASSERT_EQUALS(ErrorCodes::BadValue, status); } -*/ TEST(SimpleMod, PrepareOKTargetNotFound) { Document doc(fromjson("{}")); diff --git a/src/mongo/db/pipeline/document_source_match.cpp b/src/mongo/db/pipeline/document_source_match.cpp index 17f8cc34e83..7bc49ef0ac3 100644 --- a/src/mongo/db/pipeline/document_source_match.cpp +++ b/src/mongo/db/pipeline/document_source_match.cpp @@ -478,20 +478,10 @@ DocumentSource::GetDepsReturn DocumentSourceMatch::getDependencies(DepsTracker* return EXHAUSTIVE_FIELDS; } - addDependencies(deps); + _expression->addDependencies(deps); return SEE_NEXT; } -void DocumentSourceMatch::addDependencies(DepsTracker* deps) const { - expression::mapOver(_expression.get(), [deps](MatchExpression* node, std::string path) -> void { - if (!path.empty() && - (node->numChildren() == 0 || node->matchType() == MatchExpression::ELEM_MATCH_VALUE || - node->matchType() == MatchExpression::ELEM_MATCH_OBJECT)) { - deps->fields.insert(path); - } - }); -} - DocumentSourceMatch::DocumentSourceMatch(const BSONObj& query, const intrusive_ptr<ExpressionContext>& pExpCtx) : DocumentSource(pExpCtx), diff --git a/src/mongo/db/pipeline/document_source_match.h b/src/mongo/db/pipeline/document_source_match.h index 06219b22596..6b2fc653d43 100644 --- a/src/mongo/db/pipeline/document_source_match.h +++ b/src/mongo/db/pipeline/document_source_match.h @@ -156,8 +156,6 @@ protected: const boost::intrusive_ptr<ExpressionContext>& pExpCtx); private: - void addDependencies(DepsTracker* deps) const; - std::unique_ptr<MatchExpression> _expression; BSONObj _predicate; diff --git a/src/mongo/db/pipeline/document_source_match_test.cpp b/src/mongo/db/pipeline/document_source_match_test.cpp index cbe9c522aeb..77f31252caa 100644 --- a/src/mongo/db/pipeline/document_source_match_test.cpp +++ b/src/mongo/db/pipeline/document_source_match_test.cpp @@ -235,18 +235,78 @@ TEST_F(DocumentSourceMatchTest, ShouldOnlyAddOuterFieldAsDependencyOfImplicitEqu ASSERT_EQUALS(false, dependencies.getNeedTextScore()); } -TEST_F(DocumentSourceMatchTest, ShouldAddDependenciesOfClausesWithinElemMatchAsDottedPaths) { +TEST_F(DocumentSourceMatchTest, ShouldOnlyAddOuterFieldAsDependencyOfClausesWithinElemMatch) { auto match = DocumentSourceMatch::create(fromjson("{a: {$elemMatch: {c: {$gte: 4}}}}"), getExpCtx()); DepsTracker dependencies; ASSERT_EQUALS(DocumentSource::SEE_NEXT, match->getDependencies(&dependencies)); - ASSERT_EQUALS(1U, dependencies.fields.count("a.c")); ASSERT_EQUALS(1U, dependencies.fields.count("a")); - ASSERT_EQUALS(2U, dependencies.fields.size()); + ASSERT_EQUALS(1U, dependencies.fields.size()); ASSERT_EQUALS(false, dependencies.needWholeDocument); ASSERT_EQUALS(false, dependencies.getNeedTextScore()); } +TEST_F(DocumentSourceMatchTest, + ShouldOnlyAddOuterFieldAsDependencyOfClausesWithinInternalSchemaObjectMatch) { + auto query = fromjson( + " {a: {$_internalSchemaObjectMatch: {" + " b: {$_internalSchemaObjectMatch: {" + " $or: [{c: {$type: 'string'}}, {c: {$gt: 0}}]" + " }}}" + " }}}"); + auto match = DocumentSourceMatch::create(query, getExpCtx()); + DepsTracker dependencies; + ASSERT_EQUALS(DocumentSource::SEE_NEXT, match->getDependencies(&dependencies)); + ASSERT_EQUALS(1U, dependencies.fields.count("a")); + ASSERT_EQUALS(1U, dependencies.fields.size()); + ASSERT_EQUALS(false, dependencies.needWholeDocument); + ASSERT_EQUALS(false, dependencies.getNeedTextScore()); +} + +TEST_F(DocumentSourceMatchTest, + ShouldAddWholeDocumentAsDependencyOfClausesWithinInternalSchemaMinProperties) { + auto query = fromjson("{$_internalSchemaMinProperties: 1}"); + auto match = DocumentSourceMatch::create(query, getExpCtx()); + DepsTracker dependencies; + ASSERT_EQUALS(DocumentSource::SEE_NEXT, match->getDependencies(&dependencies)); + ASSERT_EQUALS(0U, dependencies.fields.size()); + ASSERT_EQUALS(true, dependencies.needWholeDocument); + ASSERT_EQUALS(false, dependencies.getNeedTextScore()); +} + +TEST_F(DocumentSourceMatchTest, + ShouldAddWholeDocumentAsDependencyOfClausesWithinInternalSchemaMaxProperties) { + auto query = fromjson("{$_internalSchemaMaxProperties: 1}"); + auto match = DocumentSourceMatch::create(query, getExpCtx()); + DepsTracker dependencies1; + ASSERT_EQUALS(DocumentSource::SEE_NEXT, match->getDependencies(&dependencies1)); + ASSERT_EQUALS(0U, dependencies1.fields.size()); + ASSERT_EQUALS(true, dependencies1.needWholeDocument); + ASSERT_EQUALS(false, dependencies1.getNeedTextScore()); + + query = fromjson("{a: {$_internalSchemaObjectMatch: {$_internalSchemaMaxProperties: 1}}}"); + match = DocumentSourceMatch::create(query, getExpCtx()); + DepsTracker dependencies2; + ASSERT_EQUALS(DocumentSource::SEE_NEXT, match->getDependencies(&dependencies2)); + ASSERT_EQUALS(1U, dependencies2.fields.size()); + ASSERT_EQUALS(1U, dependencies2.fields.count("a")); + ASSERT_EQUALS(false, dependencies2.needWholeDocument); + ASSERT_EQUALS(false, dependencies2.getNeedTextScore()); +} + +TEST_F(DocumentSourceMatchTest, + ShouldAddWholeDocumentAsDependencyOfClausesWithinInternalSchemaAllowedProperties) { + auto query = fromjson( + "{$_internalSchemaAllowedProperties: {properties: ['a', 'b']," + "namePlaceholder: 'i', patternProperties: [], otherwise: {i: 0}}}"); + auto match = DocumentSourceMatch::create(query, getExpCtx()); + DepsTracker dependencies; + ASSERT_EQUALS(DocumentSource::SEE_NEXT, match->getDependencies(&dependencies)); + ASSERT_EQUALS(0U, dependencies.fields.size()); + ASSERT_EQUALS(true, dependencies.needWholeDocument); + ASSERT_EQUALS(false, dependencies.getNeedTextScore()); +} + TEST_F(DocumentSourceMatchTest, ShouldAddOuterFieldToDependenciesIfElemMatchContainsNoFieldNames) { auto match = DocumentSourceMatch::create(fromjson("{a: {$elemMatch: {$gt: 1, $lt: 5}}}"), getExpCtx()); diff --git a/src/mongo/db/query/parsed_projection_test.cpp b/src/mongo/db/query/parsed_projection_test.cpp index 5d433bb4416..22046f8b531 100644 --- a/src/mongo/db/query/parsed_projection_test.cpp +++ b/src/mongo/db/query/parsed_projection_test.cpp @@ -172,12 +172,9 @@ TEST(ParsedProjectionTest, InvalidElemMatchGeoNearProjection) { "{a: {$elemMatch: {$nearSphere: {$geometry: {type: 'Point', coordinates: [0, 0]}}}}}"); } -// TODO SERVER-30951: Convert this test to use top-level $expr and enable it. -/* TEST(ParsedProjectionTest, InvalidElemMatchExprProjection) { assertInvalidProjection("{}", "{a: {$elemMatch: {$expr: 5}}}"); } -*/ TEST(ParsedProjectionTest, ValidPositionalOperatorProjections) { createParsedProjection("{a: 1}", "{'a.$': 1}"); diff --git a/src/mongo/db/query/plan_cache.cpp b/src/mongo/db/query/plan_cache.cpp index 14625fc903e..092a3bf6512 100644 --- a/src/mongo/db/query/plan_cache.cpp +++ b/src/mongo/db/query/plan_cache.cpp @@ -174,6 +174,9 @@ const char* encodeMatchType(MatchExpression::MatchType mt) { case MatchExpression::BITS_ANY_CLEAR: return "yc"; + case MatchExpression::EXPRESSION: + return "xp"; + case MatchExpression::INTERNAL_SCHEMA_ALL_ELEM_MATCH_FROM_INDEX: return "internalSchemaAllElemMatchFromIndex"; diff --git a/src/mongo/db/update/pull_node_test.cpp b/src/mongo/db/update/pull_node_test.cpp index a00a80e394e..2fc996d05f1 100644 --- a/src/mongo/db/update/pull_node_test.cpp +++ b/src/mongo/db/update/pull_node_test.cpp @@ -101,10 +101,8 @@ TEST(PullNodeTest, InitWithGeoNearObjectFails) { ASSERT_EQUALS(ErrorCodes::BadValue, status); } -// TODO SERVER-30951: Convert these tests to use top-level $expr and enable them. -/* TEST(PullNodeTest, InitWithExprElemFails) { - auto update = fromjson("{$pull: {a: {$expr: 5}}}"); + auto update = fromjson("{$pull: {a: {$expr: {$eq: [5, 5]}}}}"); const CollatorInterface* collator = nullptr; PullNode node; auto status = node.init(update["$pull"]["a"], collator); @@ -113,14 +111,13 @@ TEST(PullNodeTest, InitWithExprElemFails) { } TEST(PullNodeTest, InitWithExprObjectFails) { - auto update = fromjson("{$pull: {a: {b: {$expr: 5}}}}"); + auto update = fromjson("{$pull: {a: {$expr: {$eq: ['$a', {$literal: {b: 5}}]}}}}"); const CollatorInterface* collator = nullptr; PullNode node; auto status = node.init(update["$pull"]["a"], collator); ASSERT_NOT_OK(status); ASSERT_EQUALS(ErrorCodes::BadValue, status); } -*/ TEST_F(PullNodeTest, TargetNotFound) { auto update = fromjson("{$pull : {a: {$lt: 1}}}"); diff --git a/src/mongo/s/chunk_manager.cpp b/src/mongo/s/chunk_manager.cpp index 3ec757bb218..9b64c68bdfd 100644 --- a/src/mongo/s/chunk_manager.cpp +++ b/src/mongo/s/chunk_manager.cpp @@ -113,6 +113,8 @@ void ChunkManager::getShardIdsForQuery(OperationContext* opCtx, qr->setCollation(_defaultCollator->getSpec().toBSON()); } + // TODO SERVER-30731: Allow AllowedFeatures::kExpr here so that $expr can be used in queries + // against sharded collections. const boost::intrusive_ptr<ExpressionContext> expCtx; auto cq = uassertStatusOK( CanonicalQuery::canonicalize(opCtx, |