From 54f15bec7e431d831ef23ac7eb52a4c27b069592 Mon Sep 17 00:00:00 2001 From: Pawel Terlecki Date: Fri, 28 Feb 2020 01:12:25 -0500 Subject: SERVER-44367 Allow hinting the delete command Support hinting in delete and findAndModify with remove=true based on hinting in update. --- src/mongo/db/commands/find_and_modify.cpp | 1 + .../db/commands/write_commands/write_commands.cpp | 1 + src/mongo/db/ops/delete_request.h | 7 +++ src/mongo/db/ops/parsed_delete.cpp | 1 + src/mongo/db/ops/write_ops.idl | 4 ++ src/mongo/db/ops/write_ops_exec.cpp | 2 +- src/mongo/db/query/find_and_modify_request.cpp | 6 -- .../db/query/find_and_modify_request_test.cpp | 18 ------ src/mongo/db/query/get_executor.cpp | 71 ++++++++++++---------- 9 files changed, 53 insertions(+), 58 deletions(-) (limited to 'src') diff --git a/src/mongo/db/commands/find_and_modify.cpp b/src/mongo/db/commands/find_and_modify.cpp index 923d44eb10c..c08d7664160 100644 --- a/src/mongo/db/commands/find_and_modify.cpp +++ b/src/mongo/db/commands/find_and_modify.cpp @@ -146,6 +146,7 @@ void makeDeleteRequest(OperationContext* opCtx, requestOut->setRuntimeConstants( args.getRuntimeConstants().value_or(Variables::generateRuntimeConstants(opCtx))); requestOut->setSort(args.getSort()); + requestOut->setHint(args.getHint()); requestOut->setCollation(args.getCollation()); requestOut->setMulti(false); requestOut->setReturnDeleted(true); // Always return the old value. diff --git a/src/mongo/db/commands/write_commands/write_commands.cpp b/src/mongo/db/commands/write_commands/write_commands.cpp index 3435008fc6c..7c29d9055ec 100644 --- a/src/mongo/db/commands/write_commands/write_commands.cpp +++ b/src/mongo/db/commands/write_commands/write_commands.cpp @@ -474,6 +474,7 @@ private: deleteRequest.setCollation(write_ops::collationOf(_batch.getDeletes()[0])); deleteRequest.setMulti(_batch.getDeletes()[0].getMulti()); deleteRequest.setYieldPolicy(PlanExecutor::YIELD_AUTO); + deleteRequest.setHint(_batch.getDeletes()[0].getHint()); deleteRequest.setExplain(); ParsedDelete parsedDelete(opCtx, &deleteRequest); diff --git a/src/mongo/db/ops/delete_request.h b/src/mongo/db/ops/delete_request.h index a49e6eb37b9..4758b3486bc 100644 --- a/src/mongo/db/ops/delete_request.h +++ b/src/mongo/db/ops/delete_request.h @@ -116,6 +116,12 @@ public: bool isExplain() const { return _isExplain; } + void setHint(const BSONObj& hint) { + _hint = hint; + } + BSONObj getHint() const { + return _hint; + } bool shouldReturnDeleted() const { return _returnDeleted; } @@ -133,6 +139,7 @@ public: private: const NamespaceString& _nsString; + BSONObj _hint; BSONObj _query; BSONObj _proj; BSONObj _sort; diff --git a/src/mongo/db/ops/parsed_delete.cpp b/src/mongo/db/ops/parsed_delete.cpp index be01c8d2863..4f71d9b9562 100644 --- a/src/mongo/db/ops/parsed_delete.cpp +++ b/src/mongo/db/ops/parsed_delete.cpp @@ -94,6 +94,7 @@ Status ParsedDelete::parseQueryToCQ() { qr->setSort(_request->getSort()); qr->setCollation(_request->getCollation()); qr->setExplain(_request->isExplain()); + qr->setHint(_request->getHint()); // Limit should only used for the findAndModify command when a sort is specified. If a sort // is requested, we want to use a top-k sort for efficiency reasons, so should pass the diff --git a/src/mongo/db/ops/write_ops.idl b/src/mongo/db/ops/write_ops.idl index 42f86b1f481..464b25ae9d4 100644 --- a/src/mongo/db/ops/write_ops.idl +++ b/src/mongo/db/ops/write_ops.idl @@ -150,6 +150,10 @@ structs: matching documents and 1 deletes a single document." type: multi_delete_bool cpp_name: multi + hint: + description: "Specifies the hint to use for the operation." + type: indexHint + default: mongo::BSONObj() collation: description: "Specifies the collation to use for the operation." type: object diff --git a/src/mongo/db/ops/write_ops_exec.cpp b/src/mongo/db/ops/write_ops_exec.cpp index 149525ce366..8ebd6c54a8c 100644 --- a/src/mongo/db/ops/write_ops_exec.cpp +++ b/src/mongo/db/ops/write_ops_exec.cpp @@ -27,7 +27,6 @@ * it in the license file. */ -#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kWrite #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kWrite #include "mongo/platform/basic.h" @@ -879,6 +878,7 @@ static SingleWriteResult performSingleDeleteOp(OperationContext* opCtx, request.setYieldPolicy(opCtx->inMultiDocumentTransaction() ? PlanExecutor::INTERRUPT_ONLY : PlanExecutor::YIELD_AUTO); request.setStmtId(stmtId); + request.setHint(op.getHint()); ParsedDelete parsedDelete(opCtx, &request); uassertStatusOK(parsedDelete.parseRequest()); diff --git a/src/mongo/db/query/find_and_modify_request.cpp b/src/mongo/db/query/find_and_modify_request.cpp index c86edcbde5c..703a3621a43 100644 --- a/src/mongo/db/query/find_and_modify_request.cpp +++ b/src/mongo/db/query/find_and_modify_request.cpp @@ -168,7 +168,6 @@ StatusWith FindAndModifyRequest::parseFromBSON(NamespaceSt bool isRemove = false; bool bypassDocumentValidation = false; bool arrayFiltersSet = false; - bool hintSet = false; std::vector arrayFilters; boost::optional runtimeConstants; bool writeConcernOptionsSet = false; @@ -195,7 +194,6 @@ StatusWith FindAndModifyRequest::parseFromBSON(NamespaceSt sort = sortElement.embeddedObject(); } else if (field == kHintField) { hint = parseHint(cmdObj[kHintField]); - hintSet = true; } else if (field == kRemoveField) { isRemove = cmdObj[kRemoveField].trueValue(); } else if (field == kUpdateField) { @@ -287,10 +285,6 @@ StatusWith FindAndModifyRequest::parseFromBSON(NamespaceSt " 'remove' always returns the deleted document"}; } - if (hintSet) { - return {ErrorCodes::FailedToParse, "Cannot specify a hint with remove=true"}; - } - if (arrayFiltersSet) { return {ErrorCodes::FailedToParse, "Cannot specify arrayFilters and remove=true"}; } diff --git a/src/mongo/db/query/find_and_modify_request_test.cpp b/src/mongo/db/query/find_and_modify_request_test.cpp index afd8991cec8..a5d55fcd503 100644 --- a/src/mongo/db/query/find_and_modify_request_test.cpp +++ b/src/mongo/db/query/find_and_modify_request_test.cpp @@ -617,24 +617,6 @@ TEST(FindAndModifyRequest, ParseWithRemoveAndArrayFilters) { ASSERT_NOT_OK(parseStatus.getStatus()); } -TEST(FindAndModifyRequest, ParseWithRemoveAndHint) { - const BSONObj query(BSON("x" << 1)); - const BSONObj hint(BSON("z" << -1)); - - auto request = FindAndModifyRequest::makeRemove(NamespaceString("test.user"), query); - request.setHint(hint); - - BSONObj cmdObj(fromjson(R"json({ - findAndModify: 'user', - query: { x: 1 }, - remove: true, - hint: { z: -1 } - })json")); - - auto parseStatus = FindAndModifyRequest::parseFromBSON(NamespaceString("a.b"), cmdObj); - ASSERT_NOT_OK(parseStatus.getStatus()); -} - TEST(FindAndModifyRequest, ParseWithCollationTypeMismatch) { BSONObj cmdObj(fromjson(R"json({ query: { x: 1 }, diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp index 6388f5986b0..c9b0db6c5b3 100644 --- a/src/mongo/db/query/get_executor.cpp +++ b/src/mongo/db/query/get_executor.cpp @@ -783,41 +783,46 @@ StatusWith> getExecutorDelete( } if (!parsedDelete->hasParsedQuery()) { - // This is the idhack fast-path for getting a PlanExecutor without doing the work to create - // a CanonicalQuery. - const BSONObj& unparsedQuery = request->getQuery(); - - const IndexDescriptor* descriptor = collection->getIndexCatalog()->findIdIndex(opCtx); - - // Construct delete request collator. - std::unique_ptr collator; - if (!request->getCollation().isEmpty()) { - auto statusWithCollator = CollatorFactoryInterface::get(opCtx->getServiceContext()) - ->makeFromBSON(request->getCollation()); - if (!statusWithCollator.isOK()) { - return statusWithCollator.getStatus(); + + // Only consider using the idhack if no hint was provided. + if (request->getHint().isEmpty()) { + // This is the idhack fast-path for getting a PlanExecutor without doing the work to + // create a CanonicalQuery. + const BSONObj& unparsedQuery = request->getQuery(); + + const IndexDescriptor* descriptor = collection->getIndexCatalog()->findIdIndex(opCtx); + + // Construct delete request collator. + std::unique_ptr collator; + if (!request->getCollation().isEmpty()) { + auto statusWithCollator = CollatorFactoryInterface::get(opCtx->getServiceContext()) + ->makeFromBSON(request->getCollation()); + if (!statusWithCollator.isOK()) { + return statusWithCollator.getStatus(); + } + collator = std::move(statusWithCollator.getValue()); } - collator = std::move(statusWithCollator.getValue()); - } - const bool hasCollectionDefaultCollation = request->getCollation().isEmpty() || - CollatorInterface::collatorsMatch(collator.get(), collection->getDefaultCollator()); + const bool hasCollectionDefaultCollation = request->getCollation().isEmpty() || + CollatorInterface::collatorsMatch(collator.get(), collection->getDefaultCollator()); - if (descriptor && CanonicalQuery::isSimpleIdQuery(unparsedQuery) && - request->getProj().isEmpty() && hasCollectionDefaultCollation) { - LOGV2_DEBUG(20928, - 2, - "Using idhack: {unparsedQuery}", - "unparsedQuery"_attr = redact(unparsedQuery)); - - auto idHackStage = std::make_unique( - expCtx.get(), unparsedQuery["_id"].wrap(), ws.get(), descriptor); - unique_ptr root = - std::make_unique(expCtx.get(), - std::move(deleteStageParams), - ws.get(), - collection, - idHackStage.release()); - return PlanExecutor::make(opCtx, std::move(ws), std::move(root), collection, policy); + if (descriptor && CanonicalQuery::isSimpleIdQuery(unparsedQuery) && + request->getProj().isEmpty() && hasCollectionDefaultCollation) { + LOGV2_DEBUG(20928, + 2, + "Using idhack: {unparsedQuery}", + "unparsedQuery"_attr = redact(unparsedQuery)); + + auto idHackStage = std::make_unique( + expCtx.get(), unparsedQuery["_id"].wrap(), ws.get(), descriptor); + unique_ptr root = + std::make_unique(expCtx.get(), + std::move(deleteStageParams), + ws.get(), + collection, + idHackStage.release()); + return PlanExecutor::make( + opCtx, std::move(ws), std::move(root), collection, policy); + } } // If we're here then we don't have a parsed query, but we're also not eligible for -- cgit v1.2.1