diff options
author | Tess Avitabile <tess.avitabile@mongodb.com> | 2017-10-02 17:24:03 -0400 |
---|---|---|
committer | Tess Avitabile <tess.avitabile@mongodb.com> | 2017-10-05 16:47:26 -0400 |
commit | cb9f7cdcb7eb6ad9077ac8af3a4c0d7275c7e34f (patch) | |
tree | 24fea4c596a4652a457bc496d34a5590b7882190 /src/mongo/db | |
parent | e02285559b5f15be6afbf876f169874bd9008b0b (diff) | |
download | mongo-cb9f7cdcb7eb6ad9077ac8af3a4c0d7275c7e34f.tar.gz |
SERVER-30731 Add expr support in MatchExpression outside of aggregation
Diffstat (limited to 'src/mongo/db')
32 files changed, 166 insertions, 108 deletions
diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp index 1a24d9a8caa..2d7e6e60538 100644 --- a/src/mongo/db/catalog/coll_mod.cpp +++ b/src/mongo/db/catalog/coll_mod.cpp @@ -175,19 +175,20 @@ StatusWith<CollModRequest> parseCollModRequest(OperationContext* opCtx, if (!serverGlobalParams.featureCompatibility.validateFeaturesAsMaster.load() || serverGlobalParams.featureCompatibility.version.load() != ServerGlobalParams::FeatureCompatibility::Version::k34) { - // Allow $jsonSchema only if the feature compatibility version is newer than 3.4. - // Note that we don't enforce this restriction on the secondary or on backup - // instances, as indicated by !validateFeaturesAsMaster. + // Allow $jsonSchema and $expr only if the feature compatibility version is newer + // than 3.4. Note that we don't enforce this restriction on secondary instances, as + // indicated by !validateFeaturesAsMaster. allowedFeatures |= MatchExpressionParser::kJSONSchema; + allowedFeatures |= MatchExpressionParser::kExpr; } auto statusW = coll->parseValidator(opCtx, e.Obj(), allowedFeatures); if (!statusW.isOK()) { - if (statusW.getStatus().code() == ErrorCodes::JSONSchemaNotAllowed) { - // The default error message for disallowed $jsonSchema is not descriptive - // enough, so we rewrite it here. - return {ErrorCodes::JSONSchemaNotAllowed, + if (statusW.getStatus().code() == ErrorCodes::QueryFeatureNotAllowed) { + // The default error message for disallowed $jsonSchema and $expr is not + // descriptive enough, so we rewrite it here. + return {ErrorCodes::QueryFeatureNotAllowed, str::stream() << "The featureCompatibilityVersion must be 3.6 to add a " - "$jsonSchema validator to a collection. See " + "collection validator using 3.6 query features. See " << feature_compatibility_version::kDochubLink << "."}; } else { diff --git a/src/mongo/db/catalog/collection_impl.cpp b/src/mongo/db/catalog/collection_impl.cpp index f41d9d2a467..2262613bee8 100644 --- a/src/mongo/db/catalog/collection_impl.cpp +++ b/src/mongo/db/catalog/collection_impl.cpp @@ -159,11 +159,8 @@ CollectionImpl::CollectionImpl(Collection* _this_init, _indexCatalog(_this_init, this->getCatalogEntry()->getMaxAllowedIndexes()), _collator(parseCollation(opCtx, _ns, _details->getCollectionOptions(opCtx).collation)), _validatorDoc(_details->getCollectionOptions(opCtx).validator.getOwned()), - _validator( - uassertStatusOK(parseValidator(opCtx, - _validatorDoc, - MatchExpressionParser::kAllowAllSpecialFeatures & - ~MatchExpressionParser::AllowedFeatures::kExpr))), + _validator(uassertStatusOK( + parseValidator(opCtx, _validatorDoc, MatchExpressionParser::kAllowAllSpecialFeatures))), _validationAction(uassertStatusOK( parseValidationAction(_details->getCollectionOptions(opCtx).validationAction))), _validationLevel(uassertStatusOK( @@ -898,10 +895,8 @@ Status CollectionImpl::setValidator(OperationContext* opCtx, BSONObj validatorDo // Note that, by the time we reach this, we should have already done a pre-parse that checks for // banned features, so we don't need to include that check again. - auto statusWithMatcher = parseValidator(opCtx, - validatorDoc, - MatchExpressionParser::kAllowAllSpecialFeatures & - ~MatchExpressionParser::AllowedFeatures::kExpr); + auto statusWithMatcher = + parseValidator(opCtx, validatorDoc, MatchExpressionParser::kAllowAllSpecialFeatures); if (!statusWithMatcher.isOK()) return statusWithMatcher.getStatus(); diff --git a/src/mongo/db/catalog/database_impl.cpp b/src/mongo/db/catalog/database_impl.cpp index f4fe399674c..096b7287e2d 100644 --- a/src/mongo/db/catalog/database_impl.cpp +++ b/src/mongo/db/catalog/database_impl.cpp @@ -1021,10 +1021,11 @@ auto mongo::userCreateNSImpl(OperationContext* opCtx, if (!serverGlobalParams.featureCompatibility.validateFeaturesAsMaster.load() || serverGlobalParams.featureCompatibility.version.load() != ServerGlobalParams::FeatureCompatibility::Version::k34) { - // $jsonSchema is only permitted when the feature compatibility version is newer - // than 3.4. Note that we don't enforce this feature compatibility check when we are on - // the secondary or on a backup instance, as indicated by !validateFeaturesAsMaster. + // $jsonSchema and $expr are only permitted when the feature compatibility version is + // newer than 3.4. Note that we don't enforce this feature compatibility check when we + // are on the secondary, as indicated by !validateFeaturesAsMaster. allowedFeatures |= MatchExpressionParser::kJSONSchema; + allowedFeatures |= MatchExpressionParser::kExpr; } boost::intrusive_ptr<ExpressionContext> expCtx( new ExpressionContext(opCtx, collator.get())); @@ -1036,12 +1037,12 @@ auto mongo::userCreateNSImpl(OperationContext* opCtx, // We check the status of the parse to see if there are any banned features, but we don't // actually need the result for now. if (!statusWithMatcher.isOK()) { - if (statusWithMatcher.getStatus().code() == ErrorCodes::JSONSchemaNotAllowed) { - // The default error message for disallowed $jsonSchema is not descriptive enough, - // so we rewrite it here. - return {ErrorCodes::JSONSchemaNotAllowed, + if (statusWithMatcher.getStatus().code() == ErrorCodes::QueryFeatureNotAllowed) { + // The default error message for disallowed $jsonSchema and $expr is not descriptive + // enough, so we rewrite it here. + return {ErrorCodes::QueryFeatureNotAllowed, str::stream() << "The featureCompatibilityVersion must be 3.6 to create a " - "collection with a $jsonSchema validator. See " + "collection validator using 3.6 query featuress. See " << feature_compatibility_version::kDochubLink << "."}; } else { diff --git a/src/mongo/db/catalog/index_catalog_impl.cpp b/src/mongo/db/catalog/index_catalog_impl.cpp index 7636cd6af6a..578291a06e9 100644 --- a/src/mongo/db/catalog/index_catalog_impl.cpp +++ b/src/mongo/db/catalog/index_catalog_impl.cpp @@ -668,7 +668,10 @@ Status IndexCatalogImpl::_isSpecOk(OperationContext* opCtx, const BSONObj& spec) boost::intrusive_ptr<ExpressionContext> expCtx( new ExpressionContext(opCtx, collator.get())); StatusWithMatchExpression statusWithMatcher = - MatchExpressionParser::parse(filterElement.Obj(), std::move(expCtx)); + MatchExpressionParser::parse(filterElement.Obj(), + std::move(expCtx), + ExtensionsCallbackNoop(), + MatchExpressionParser::kBanAllSpecialFeatures); if (!statusWithMatcher.isOK()) { return statusWithMatcher.getStatus(); } diff --git a/src/mongo/db/commands/find_cmd.cpp b/src/mongo/db/commands/find_cmd.cpp index d3d9aafc096..3ef407c1d3f 100644 --- a/src/mongo/db/commands/find_cmd.cpp +++ b/src/mongo/db/commands/find_cmd.cpp @@ -163,8 +163,7 @@ public: std::move(qrStatus.getValue()), expCtx, extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures & - ~MatchExpressionParser::AllowedFeatures::kExpr); + MatchExpressionParser::kAllowAllSpecialFeatures); if (!statusWithCQ.isOK()) { return statusWithCQ.getStatus(); } @@ -281,8 +280,7 @@ public: std::move(qr), expCtx, extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures & - ~MatchExpressionParser::AllowedFeatures::kExpr); + MatchExpressionParser::kAllowAllSpecialFeatures); if (!statusWithCQ.isOK()) { return appendCommandStatus(result, statusWithCQ.getStatus()); } diff --git a/src/mongo/db/commands/geo_near_cmd.cpp b/src/mongo/db/commands/geo_near_cmd.cpp index 02a57e7d0e8..0df75717f42 100644 --- a/src/mongo/db/commands/geo_near_cmd.cpp +++ b/src/mongo/db/commands/geo_near_cmd.cpp @@ -221,8 +221,7 @@ public: std::move(qr), expCtx, extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures & - ~MatchExpressionParser::AllowedFeatures::kExpr); + MatchExpressionParser::kAllowAllSpecialFeatures); if (!statusWithCQ.isOK()) { errmsg = "Can't parse filter / create query"; return false; diff --git a/src/mongo/db/commands/index_filter_commands.cpp b/src/mongo/db/commands/index_filter_commands.cpp index 1992f122edc..9945beb82f5 100644 --- a/src/mongo/db/commands/index_filter_commands.cpp +++ b/src/mongo/db/commands/index_filter_commands.cpp @@ -316,8 +316,7 @@ Status ClearFilters::clear(OperationContext* opCtx, std::move(qr), expCtx, extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures & - ~MatchExpressionParser::AllowedFeatures::kExpr); + MatchExpressionParser::kAllowAllSpecialFeatures); invariantOK(statusWithCQ.getStatus()); std::unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); diff --git a/src/mongo/db/commands/mr.cpp b/src/mongo/db/commands/mr.cpp index b9ecf5b4006..27f907f4ab8 100644 --- a/src/mongo/db/commands/mr.cpp +++ b/src/mongo/db/commands/mr.cpp @@ -1122,8 +1122,7 @@ void State::finalReduce(OperationContext* opCtx, CurOp* curOp, ProgressMeterHold std::move(qr), expCtx, extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures & - ~MatchExpressionParser::AllowedFeatures::kExpr); + MatchExpressionParser::kAllowAllSpecialFeatures); verify(statusWithCQ.isOK()); std::unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); @@ -1489,13 +1488,12 @@ public: const ExtensionsCallbackReal extensionsCallback(opCtx, &config.nss); const boost::intrusive_ptr<ExpressionContext> expCtx; - auto statusWithCQ = CanonicalQuery::canonicalize( - opCtx, - std::move(qr), - expCtx, - extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures & - ~MatchExpressionParser::AllowedFeatures::kExpr); + auto statusWithCQ = + CanonicalQuery::canonicalize(opCtx, + std::move(qr), + expCtx, + extensionsCallback, + MatchExpressionParser::kAllowAllSpecialFeatures); if (!statusWithCQ.isOK()) { uasserted(17238, "Can't canonicalize query " + config.filter.toString()); return 0; diff --git a/src/mongo/db/commands/plan_cache_commands.cpp b/src/mongo/db/commands/plan_cache_commands.cpp index 031d59e408d..3704105d17f 100644 --- a/src/mongo/db/commands/plan_cache_commands.cpp +++ b/src/mongo/db/commands/plan_cache_commands.cpp @@ -213,8 +213,7 @@ StatusWith<unique_ptr<CanonicalQuery>> PlanCacheCommand::canonicalize(OperationC std::move(qr), expCtx, extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures & - ~MatchExpressionParser::AllowedFeatures::kExpr); + MatchExpressionParser::kAllowAllSpecialFeatures); if (!statusWithCQ.isOK()) { return statusWithCQ.getStatus(); } diff --git a/src/mongo/db/dbhelpers.cpp b/src/mongo/db/dbhelpers.cpp index bb27f53b049..f3ed7df4f97 100644 --- a/src/mongo/db/dbhelpers.cpp +++ b/src/mongo/db/dbhelpers.cpp @@ -119,8 +119,7 @@ RecordId Helpers::findOne(OperationContext* opCtx, std::move(qr), expCtx, extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures & - ~MatchExpressionParser::AllowedFeatures::kExpr); + MatchExpressionParser::kAllowAllSpecialFeatures); massert(17244, "Could not canonicalize " + query.toString(), statusWithCQ.isOK()); unique_ptr<CanonicalQuery> cq = std::move(statusWithCQ.getValue()); diff --git a/src/mongo/db/exec/stagedebug_cmd.cpp b/src/mongo/db/exec/stagedebug_cmd.cpp index f3d9c92cafd..6e65094764c 100644 --- a/src/mongo/db/exec/stagedebug_cmd.cpp +++ b/src/mongo/db/exec/stagedebug_cmd.cpp @@ -250,12 +250,11 @@ public: const CollatorInterface* collator = nullptr; const boost::intrusive_ptr<ExpressionContext> expCtx( new ExpressionContext(opCtx, collator)); - StatusWithMatchExpression statusWithMatcher = MatchExpressionParser::parse( - argObj, - expCtx, - ExtensionsCallbackReal(opCtx, &collection->ns()), - MatchExpressionParser::kAllowAllSpecialFeatures & - ~MatchExpressionParser::AllowedFeatures::kExpr); + auto statusWithMatcher = + MatchExpressionParser::parse(argObj, + expCtx, + ExtensionsCallbackReal(opCtx, &collection->ns()), + MatchExpressionParser::kAllowAllSpecialFeatures); if (!statusWithMatcher.isOK()) { return NULL; } diff --git a/src/mongo/db/matcher/expression_expr.cpp b/src/mongo/db/matcher/expression_expr.cpp index 8963dd003d8..77349430e46 100644 --- a/src/mongo/db/matcher/expression_expr.cpp +++ b/src/mongo/db/matcher/expression_expr.cpp @@ -64,6 +64,10 @@ bool ExprMatchExpression::equivalent(const MatchExpression* other) const { realOther->_expression->serialize(false)); } +void ExprMatchExpression::_doSetCollator(const CollatorInterface* collator) { + _expCtx->setCollator(collator); +} + std::unique_ptr<MatchExpression> ExprMatchExpression::shallowClone() const { // TODO SERVER-31003: Replace Expression clone via serialization with Expression::clone(). diff --git a/src/mongo/db/matcher/expression_expr.h b/src/mongo/db/matcher/expression_expr.h index e97cce982dc..1a09e2a9334 100644 --- a/src/mongo/db/matcher/expression_expr.h +++ b/src/mongo/db/matcher/expression_expr.h @@ -85,6 +85,8 @@ public: private: ExpressionOptimizerFunc getOptimizer() const final; + void _doSetCollator(const CollatorInterface* collator) final; + void _doAddDependencies(DepsTracker* deps) const final { if (_expression) { _expression->addDependencies(deps); diff --git a/src/mongo/db/matcher/expression_expr_test.cpp b/src/mongo/db/matcher/expression_expr_test.cpp index 3ad0ac4340e..e89ef9a5c39 100644 --- a/src/mongo/db/matcher/expression_expr_test.cpp +++ b/src/mongo/db/matcher/expression_expr_test.cpp @@ -32,12 +32,15 @@ #include "mongo/db/matcher/expression_expr.h" #include "mongo/db/matcher/matcher.h" #include "mongo/db/pipeline/expression_context_for_test.h" +#include "mongo/db/query/collation/collator_interface_mock.h" #include "mongo/unittest/unittest.h" namespace mongo { namespace { +using unittest::assertGet; + TEST(ExprMatchExpression, ComparisonToConstantMatchesCorrectly) { boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); auto match = BSON("a" << 5); @@ -148,5 +151,25 @@ TEST(ExprMatchExpression, ShallowClonedExpressionIsEquivalentToOriginal) { ASSERT_TRUE(pipelineExpr.equivalent(shallowClone.get())); } +TEST(ExprMatchExpression, SetCollatorChangesCollationUsedForComparisons) { + boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + auto match = BSON("a" + << "abc"); + auto notMatch = BSON("a" + << "ABC"); + + auto expression = BSON("$expr" << BSON("$eq" << BSON_ARRAY("$a" + << "abc"))); + auto matchExpression = assertGet(MatchExpressionParser::parse(expression, expCtx)); + ASSERT_TRUE(matchExpression->matchesBSON(match)); + ASSERT_FALSE(matchExpression->matchesBSON(notMatch)); + + CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual); + matchExpression->setCollator(&collator); + + ASSERT_TRUE(matchExpression->matchesBSON(match)); + ASSERT_TRUE(matchExpression->matchesBSON(notMatch)); +} + } // namespace } // namespace mongo diff --git a/src/mongo/db/matcher/expression_parser.cpp b/src/mongo/db/matcher/expression_parser.cpp index de8d2717a96..ddb6491739b 100644 --- a/src/mongo/db/matcher/expression_parser.cpp +++ b/src/mongo/db/matcher/expression_parser.cpp @@ -598,7 +598,7 @@ StatusWithMatchExpression MatchExpressionParser::_parse( root->add(maxPropsExpr.getValue().release()); } else if (mongoutils::str::equals("jsonSchema", rest)) { if ((allowedFeatures & AllowedFeatures::kJSONSchema) == 0u) { - return Status(ErrorCodes::JSONSchemaNotAllowed, + return Status(ErrorCodes::QueryFeatureNotAllowed, "$jsonSchema is not allowed in this context"); } @@ -1692,7 +1692,7 @@ StatusWithMatchExpression MatchExpressionParser::_parseExpr( AllowedFeatureSet allowedFeatures, const boost::intrusive_ptr<ExpressionContext>& expCtx) { if ((allowedFeatures & AllowedFeatures::kExpr) == 0u) { - return {Status(ErrorCodes::BadValue, "$expr is not allowed in this context")}; + return {Status(ErrorCodes::QueryFeatureNotAllowed, "$expr is not allowed in this context")}; } invariant(expCtx); diff --git a/src/mongo/db/matcher/expression_parser.h b/src/mongo/db/matcher/expression_parser.h index 44ffd4ae9a0..1adb7640ed1 100644 --- a/src/mongo/db/matcher/expression_parser.h +++ b/src/mongo/db/matcher/expression_parser.h @@ -99,7 +99,8 @@ public: static constexpr AllowedFeatureSet kBanAllSpecialFeatures = 0; static constexpr AllowedFeatureSet kAllowAllSpecialFeatures = std::numeric_limits<unsigned long long>::max(); - static constexpr AllowedFeatureSet kDefaultSpecialFeatures = AllowedFeatures::kJSONSchema; + static constexpr AllowedFeatureSet kDefaultSpecialFeatures = + AllowedFeatures::kExpr | AllowedFeatures::kJSONSchema; /** * Constant double representation of 2^63. diff --git a/src/mongo/db/matcher/expression_parser_test.cpp b/src/mongo/db/matcher/expression_parser_test.cpp index ce520064261..3c8796833b6 100644 --- a/src/mongo/db/matcher/expression_parser_test.cpp +++ b/src/mongo/db/matcher/expression_parser_test.cpp @@ -367,33 +367,27 @@ TEST(MatchExpressionParserTest, NearParsesSuccessfullyWhenAllowed) { TEST(MatchExpressionParserTest, ExprFailsToParseWhenDisallowed) { auto query = fromjson("{$expr: {$eq: ['$a', 5]}}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); - ASSERT_NOT_OK(MatchExpressionParser::parse(query, expCtx).getStatus()); + ASSERT_NOT_OK( + MatchExpressionParser::parse( + query, expCtx, ExtensionsCallbackNoop(), MatchExpressionParser::kBanAllSpecialFeatures) + .getStatus()); } TEST(MatchExpressionParserTest, ExprParsesSuccessfullyWhenAllowed) { auto query = fromjson("{$expr: {$eq: ['$a', 5]}}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); - ASSERT_OK( - MatchExpressionParser::parse( - query, expCtx, ExtensionsCallbackNoop(), MatchExpressionParser::AllowedFeatures::kExpr) - .getStatus()); + ASSERT_OK(MatchExpressionParser::parse(query, expCtx).getStatus()); } TEST(MatchExpressionParserTest, ExprParsesSuccessfullyWithAdditionalTopLevelPredicates) { auto query = fromjson("{x: 1, $expr: {$eq: ['$a', 5]}, y: 1}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); - ASSERT_OK( - MatchExpressionParser::parse( - query, expCtx, ExtensionsCallbackNoop(), MatchExpressionParser::AllowedFeatures::kExpr) - .getStatus()); + ASSERT_OK(MatchExpressionParser::parse(query, expCtx).getStatus()); } TEST(MatchExpressionParserTest, ExprFailsToParseWithinTopLevelOr) { auto query = fromjson("{$or: [{x: 1}, {$expr: {$eq: ['$a', 5]}}]}"); boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); - ASSERT_NOT_OK( - MatchExpressionParser::parse( - query, expCtx, ExtensionsCallbackNoop(), MatchExpressionParser::AllowedFeatures::kExpr) - .getStatus()); + ASSERT_NOT_OK(MatchExpressionParser::parse(query, expCtx).getStatus()); } } diff --git a/src/mongo/db/matcher/expression_with_placeholder.cpp b/src/mongo/db/matcher/expression_with_placeholder.cpp index cd68587b057..bb878e05cfa 100644 --- a/src/mongo/db/matcher/expression_with_placeholder.cpp +++ b/src/mongo/db/matcher/expression_with_placeholder.cpp @@ -90,7 +90,8 @@ const std::regex ExpressionWithPlaceholder::placeholderRegex("^[a-z][a-zA-Z0-9]* // static StatusWith<std::unique_ptr<ExpressionWithPlaceholder>> ExpressionWithPlaceholder::parse( BSONObj rawFilter, const boost::intrusive_ptr<ExpressionContext>& expCtx) { - StatusWithMatchExpression statusWithFilter = MatchExpressionParser::parse(rawFilter, expCtx); + StatusWithMatchExpression statusWithFilter = MatchExpressionParser::parse( + rawFilter, expCtx, ExtensionsCallbackNoop(), MatchExpressionParser::kBanAllSpecialFeatures); if (!statusWithFilter.isOK()) { return statusWithFilter.getStatus(); diff --git a/src/mongo/db/matcher/expression_with_placeholder_test.cpp b/src/mongo/db/matcher/expression_with_placeholder_test.cpp index 7bd19b12d00..27a2a1d549c 100644 --- a/src/mongo/db/matcher/expression_with_placeholder_test.cpp +++ b/src/mongo/db/matcher/expression_with_placeholder_test.cpp @@ -245,6 +245,15 @@ TEST(ExpressionWithPlaceholderTest, ExprExpressionFailsToParse) { auto rawFilter = fromjson("{$expr: {$eq: ['$i', 5]}}"); auto status = ExpressionWithPlaceholder::parse(rawFilter, expCtx); ASSERT_NOT_OK(status.getStatus()); + ASSERT_EQ(status.getStatus().code(), ErrorCodes::QueryFeatureNotAllowed); +} + +TEST(ExpressionWithPlaceholderTest, JSONSchemaExpressionFailsToParse) { + boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + auto rawFilter = fromjson("{$jsonSchema: {}}"); + auto status = ExpressionWithPlaceholder::parse(rawFilter, expCtx); + ASSERT_NOT_OK(status.getStatus()); + ASSERT_EQ(status.getStatus().code(), ErrorCodes::QueryFeatureNotAllowed); } TEST(ExpressionWithPlaceholderTest, EquivalentIfPlaceholderAndExpressionMatch) { diff --git a/src/mongo/db/ops/modifier_pull.cpp b/src/mongo/db/ops/modifier_pull.cpp index a030e4e5598..2d67fdf9505 100644 --- a/src/mongo/db/ops/modifier_pull.cpp +++ b/src/mongo/db/ops/modifier_pull.cpp @@ -122,8 +122,12 @@ Status ModifierPull::init(const BSONElement& modExpr, const Options& opts, bool* } // Build the matcher around the object we built above. Currently, we do not allow $pull - // operations to contain $text/$where/$geoNear/$near/$nearSphere/$expr clauses. - StatusWithMatchExpression parseResult = MatchExpressionParser::parse(_exprObj, opts.expCtx); + // operations to contain $text/$where/$geoNear/$near/$nearSphere/$expr/$jsonSchema clauses. + StatusWithMatchExpression parseResult = + MatchExpressionParser::parse(_exprObj, + opts.expCtx, + ExtensionsCallbackNoop(), + MatchExpressionParser::kBanAllSpecialFeatures); if (!parseResult.isOK()) { return parseResult.getStatus(); } diff --git a/src/mongo/db/ops/modifier_pull_test.cpp b/src/mongo/db/ops/modifier_pull_test.cpp index ee5a2025310..e21e1648e0c 100644 --- a/src/mongo/db/ops/modifier_pull_test.cpp +++ b/src/mongo/db/ops/modifier_pull_test.cpp @@ -130,7 +130,7 @@ TEST(SimpleMod, InitWithExprElemFails) { ModifierPull node; auto status = node.init(update["$pull"]["a"], ModifierInterface::Options::normal(expCtx)); ASSERT_NOT_OK(status); - ASSERT_EQUALS(ErrorCodes::BadValue, status); + ASSERT_EQUALS(ErrorCodes::QueryFeatureNotAllowed, status); } TEST(SimpleMod, InitWithExprObjectFails) { @@ -139,7 +139,16 @@ TEST(SimpleMod, InitWithExprObjectFails) { ModifierPull node; auto status = node.init(update["$pull"]["a"], ModifierInterface::Options::normal(expCtx)); ASSERT_NOT_OK(status); - ASSERT_EQUALS(ErrorCodes::BadValue, status); + ASSERT_EQUALS(ErrorCodes::QueryFeatureNotAllowed, status); +} + +TEST(SimpleMod, InitWithJSONSchemaFails) { + auto update = fromjson("{$pull: {a: {$jsonSchema: {}}}}"); + boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + ModifierPull node; + auto status = node.init(update["$pull"]["a"], ModifierInterface::Options::normal(expCtx)); + ASSERT_NOT_OK(status); + ASSERT_EQUALS(ErrorCodes::QueryFeatureNotAllowed, status); } TEST(SimpleMod, PrepareOKTargetNotFound) { diff --git a/src/mongo/db/ops/parsed_delete.cpp b/src/mongo/db/ops/parsed_delete.cpp index 8e5d146562e..839381e358f 100644 --- a/src/mongo/db/ops/parsed_delete.cpp +++ b/src/mongo/db/ops/parsed_delete.cpp @@ -96,8 +96,7 @@ Status ParsedDelete::parseQueryToCQ() { std::move(qr), expCtx, extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures & - ~MatchExpressionParser::AllowedFeatures::kExpr); + MatchExpressionParser::kAllowAllSpecialFeatures); if (statusWithCQ.isOK()) { _canonicalQuery = std::move(statusWithCQ.getValue()); diff --git a/src/mongo/db/ops/parsed_update.cpp b/src/mongo/db/ops/parsed_update.cpp index c3da51e352e..108475f84be 100644 --- a/src/mongo/db/ops/parsed_update.cpp +++ b/src/mongo/db/ops/parsed_update.cpp @@ -115,18 +115,28 @@ Status ParsedUpdate::parseQueryToCQ() { qr->setLimit(1); } + // $expr is not allowed in the query for an upsert, since it is not clear what the equality + // extraction behavior for $expr should be. + MatchExpressionParser::AllowedFeatureSet allowedMatcherFeatures = + MatchExpressionParser::kAllowAllSpecialFeatures; + if (_request->isUpsert()) { + allowedMatcherFeatures &= ~MatchExpressionParser::AllowedFeatures::kExpr; + } + boost::intrusive_ptr<ExpressionContext> expCtx; - auto statusWithCQ = - CanonicalQuery::canonicalize(_opCtx, - std::move(qr), - std::move(expCtx), - extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures & - ~MatchExpressionParser::AllowedFeatures::kExpr); + auto statusWithCQ = CanonicalQuery::canonicalize( + _opCtx, std::move(qr), std::move(expCtx), extensionsCallback, allowedMatcherFeatures); if (statusWithCQ.isOK()) { _canonicalQuery = std::move(statusWithCQ.getValue()); } + if (statusWithCQ.getStatus().code() == ErrorCodes::QueryFeatureNotAllowed) { + // The default error message for disallowed $expr is not descriptive enough, so we rewrite + // it here. + return {ErrorCodes::QueryFeatureNotAllowed, + "$expr is not allowed in the query predicate for an upsert"}; + } + return statusWithCQ.getStatus(); } diff --git a/src/mongo/db/pipeline/document_source_match.cpp b/src/mongo/db/pipeline/document_source_match.cpp index bd212cf1925..f65aa8db073 100644 --- a/src/mongo/db/pipeline/document_source_match.cpp +++ b/src/mongo/db/pipeline/document_source_match.cpp @@ -368,12 +368,8 @@ bool DocumentSourceMatch::isTextQuery(const BSONObj& query) { void DocumentSourceMatch::joinMatchWith(intrusive_ptr<DocumentSourceMatch> other) { _predicate = BSON("$and" << BSON_ARRAY(_predicate << other->getQuery())); - StatusWithMatchExpression status = uassertStatusOK( - MatchExpressionParser::parse(_predicate, - pExpCtx, - ExtensionsCallbackNoop(), - MatchExpressionParser::AllowedFeatures::kText | - MatchExpressionParser::AllowedFeatures::kExpr)); + StatusWithMatchExpression status = uassertStatusOK(MatchExpressionParser::parse( + _predicate, pExpCtx, ExtensionsCallbackNoop(), Pipeline::kAllowedMatcherFeatures)); _expression = std::move(status.getValue()); _dependencies = DepsTracker(_dependencies.getMetadataAvailable()); getDependencies(&_dependencies); diff --git a/src/mongo/db/query/find.cpp b/src/mongo/db/query/find.cpp index e11706e99b0..26a249f7928 100644 --- a/src/mongo/db/query/find.cpp +++ b/src/mongo/db/query/find.cpp @@ -533,8 +533,7 @@ std::string runQuery(OperationContext* opCtx, q, expCtx, ExtensionsCallbackReal(opCtx, &nss), - MatchExpressionParser::kAllowAllSpecialFeatures & - ~MatchExpressionParser::AllowedFeatures::kExpr); + MatchExpressionParser::kAllowAllSpecialFeatures); if (!statusWithCQ.isOK()) { uasserted(17287, str::stream() << "Can't canonicalize query: " diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp index cf241557d19..e818b7429c4 100644 --- a/src/mongo/db/query/get_executor.cpp +++ b/src/mongo/db/query/get_executor.cpp @@ -1010,8 +1010,7 @@ StatusWith<unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorGroup( std::move(qr), expCtx, extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures & - ~MatchExpressionParser::AllowedFeatures::kExpr); + MatchExpressionParser::kAllowAllSpecialFeatures); if (!statusWithCQ.isOK()) { return statusWithCQ.getStatus(); } @@ -1249,8 +1248,7 @@ StatusWith<unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorCount( collection ? static_cast<const ExtensionsCallback&>( ExtensionsCallbackReal(opCtx, &collection->ns())) : static_cast<const ExtensionsCallback&>(ExtensionsCallbackNoop()), - MatchExpressionParser::kAllowAllSpecialFeatures & - ~MatchExpressionParser::AllowedFeatures::kExpr); + MatchExpressionParser::kAllowAllSpecialFeatures); if (!statusWithCQ.isOK()) { return statusWithCQ.getStatus(); @@ -1507,8 +1505,7 @@ StatusWith<unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorDistinct( std::move(qr), expCtx, extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures & - ~MatchExpressionParser::AllowedFeatures::kExpr); + MatchExpressionParser::kAllowAllSpecialFeatures); if (!statusWithCQ.isOK()) { return statusWithCQ.getStatus(); } diff --git a/src/mongo/db/query/parsed_distinct.cpp b/src/mongo/db/query/parsed_distinct.cpp index da071756726..915636dd98c 100644 --- a/src/mongo/db/query/parsed_distinct.cpp +++ b/src/mongo/db/query/parsed_distinct.cpp @@ -206,8 +206,7 @@ StatusWith<ParsedDistinct> ParsedDistinct::parse(OperationContext* opCtx, std::move(qr), expCtx, extensionsCallback, - MatchExpressionParser::kAllowAllSpecialFeatures & - ~MatchExpressionParser::AllowedFeatures::kExpr); + MatchExpressionParser::kAllowAllSpecialFeatures); if (!cq.isOK()) { return cq.getStatus(); } diff --git a/src/mongo/db/query/parsed_projection.cpp b/src/mongo/db/query/parsed_projection.cpp index 069ff5a235e..53e5f5a523e 100644 --- a/src/mongo/db/query/parsed_projection.cpp +++ b/src/mongo/db/query/parsed_projection.cpp @@ -127,13 +127,17 @@ Status ParsedProjection::make(OperationContext* opCtx, // is ok because the parsed MatchExpression is not used after being created. We are // only parsing here in order to ensure that the elemMatch projection is valid. // - // Match expression extensions such as $text, $where, $geoNear, $near, $nearSphere, - // and $expr are not allowed in $elemMatch projections. + // Match expression extensions such as $text, $where, $geoNear, $near, and + // $nearSphere are not allowed in $elemMatch projections. $expr and $jsonSchema are + // not allowed because the matcher is not applied to the root of the document. const CollatorInterface* collator = nullptr; boost::intrusive_ptr<ExpressionContext> expCtx( new ExpressionContext(opCtx, collator)); StatusWithMatchExpression statusWithMatcher = - MatchExpressionParser::parse(elemMatchObj, std::move(expCtx)); + MatchExpressionParser::parse(elemMatchObj, + std::move(expCtx), + ExtensionsCallbackNoop(), + MatchExpressionParser::kBanAllSpecialFeatures); if (!statusWithMatcher.isOK()) { return statusWithMatcher.getStatus(); } diff --git a/src/mongo/db/sessions_collection_sharded.cpp b/src/mongo/db/sessions_collection_sharded.cpp index 38738207196..bbcf195d8dd 100644 --- a/src/mongo/db/sessions_collection_sharded.cpp +++ b/src/mongo/db/sessions_collection_sharded.cpp @@ -113,8 +113,7 @@ StatusWith<LogicalSessionIdSet> SessionsCollectionSharded::findRemovedSessions( std::move(qr.getValue()), expCtx, ExtensionsCallbackNoop(), - MatchExpressionParser::kAllowAllSpecialFeatures & - ~MatchExpressionParser::AllowedFeatures::kExpr); + MatchExpressionParser::kBanAllSpecialFeatures); if (!cq.isOK()) { return cq.getStatus(); } diff --git a/src/mongo/db/update/pull_node.cpp b/src/mongo/db/update/pull_node.cpp index b712e2ece4e..2e29b3a9527 100644 --- a/src/mongo/db/update/pull_node.cpp +++ b/src/mongo/db/update/pull_node.cpp @@ -42,7 +42,10 @@ namespace mongo { class PullNode::ObjectMatcher final : public PullNode::ElementMatcher { public: ObjectMatcher(BSONObj matchCondition, const boost::intrusive_ptr<ExpressionContext>& expCtx) - : _matchExpr(matchCondition, expCtx) {} + : _matchExpr(matchCondition, + expCtx, + stdx::make_unique<ExtensionsCallbackNoop>(), + MatchExpressionParser::kBanAllSpecialFeatures) {} std::unique_ptr<ElementMatcher> clone() const final { return stdx::make_unique<ObjectMatcher>(*this); @@ -75,7 +78,10 @@ class PullNode::WrappedObjectMatcher final : public PullNode::ElementMatcher { public: WrappedObjectMatcher(BSONElement matchCondition, const boost::intrusive_ptr<ExpressionContext>& expCtx) - : _matchExpr(matchCondition.wrap(""), expCtx) {} + : _matchExpr(matchCondition.wrap(""), + expCtx, + stdx::make_unique<ExtensionsCallbackNoop>(), + MatchExpressionParser::kBanAllSpecialFeatures) {} std::unique_ptr<ElementMatcher> clone() const final { return stdx::make_unique<WrappedObjectMatcher>(*this); diff --git a/src/mongo/db/update/pull_node_test.cpp b/src/mongo/db/update/pull_node_test.cpp index 744abd739f4..7cfb15542cb 100644 --- a/src/mongo/db/update/pull_node_test.cpp +++ b/src/mongo/db/update/pull_node_test.cpp @@ -108,7 +108,7 @@ TEST(PullNodeTest, InitWithExprElemFails) { PullNode node; auto status = node.init(update["$pull"]["a"], expCtx); ASSERT_NOT_OK(status); - ASSERT_EQUALS(ErrorCodes::BadValue, status); + ASSERT_EQUALS(ErrorCodes::QueryFeatureNotAllowed, status); } TEST(PullNodeTest, InitWithExprObjectFails) { @@ -117,7 +117,16 @@ TEST(PullNodeTest, InitWithExprObjectFails) { PullNode node; auto status = node.init(update["$pull"]["a"], expCtx); ASSERT_NOT_OK(status); - ASSERT_EQUALS(ErrorCodes::BadValue, status); + ASSERT_EQUALS(ErrorCodes::QueryFeatureNotAllowed, status); +} + +TEST(PullNodeTest, InitWithJSONSchemaFails) { + auto update = fromjson("{$pull: {a: {$jsonSchema: {}}}}"); + boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + PullNode node; + auto status = node.init(update["$pull"]["a"], expCtx); + ASSERT_NOT_OK(status); + ASSERT_EQUALS(ErrorCodes::QueryFeatureNotAllowed, status); } TEST_F(PullNodeTest, TargetNotFound) { diff --git a/src/mongo/db/update/update_driver.cpp b/src/mongo/db/update/update_driver.cpp index 7eb4f4ffd7c..5380de021ae 100644 --- a/src/mongo/db/update/update_driver.cpp +++ b/src/mongo/db/update/update_driver.cpp @@ -288,6 +288,8 @@ Status UpdateDriver::populateDocumentWithQueryFields(OperationContext* opCtx, auto qr = stdx::make_unique<QueryRequest>(NamespaceString("")); qr->setFilter(query); const boost::intrusive_ptr<ExpressionContext> expCtx; + // $expr is not allowed in the query for an upsert, since it is not clear what the equality + // extraction behavior for $expr should be. auto statusWithCQ = CanonicalQuery::canonicalize(opCtx, std::move(qr), |