From 64906095f4a590a34e1f22042aea2836721f7bbe Mon Sep 17 00:00:00 2001 From: Ian Boros Date: Fri, 8 May 2020 14:05:12 -0400 Subject: SERVER-48052 Make updates create ExpressionContext with 'explain' field correctly initialized --- src/mongo/db/commands/find_and_modify.cpp | 9 ++++----- src/mongo/db/commands/write_commands/write_commands.cpp | 2 +- src/mongo/db/exec/update_stage.cpp | 8 ++++---- src/mongo/db/exec/upsert_stage.cpp | 2 +- src/mongo/db/ops/parsed_update.cpp | 16 ++++++++++------ src/mongo/db/ops/update.cpp | 2 +- src/mongo/db/ops/update_request.h | 15 ++++++++------- src/mongo/db/pipeline/expression_context.cpp | 6 ++++-- src/mongo/db/pipeline/expression_context.h | 3 ++- src/mongo/db/query/get_executor.cpp | 5 +---- 10 files changed, 36 insertions(+), 32 deletions(-) (limited to 'src/mongo/db') diff --git a/src/mongo/db/commands/find_and_modify.cpp b/src/mongo/db/commands/find_and_modify.cpp index 2c86f93243d..40ab8129ec2 100644 --- a/src/mongo/db/commands/find_and_modify.cpp +++ b/src/mongo/db/commands/find_and_modify.cpp @@ -121,7 +121,7 @@ boost::optional advanceExecutor(OperationContext* opCtx, void makeUpdateRequest(OperationContext* opCtx, const FindAndModifyRequest& args, - bool explain, + boost::optional explain, UpdateRequest* requestOut) { requestOut->setQuery(args.getQuery()); requestOut->setProj(args.getFields()); @@ -291,8 +291,7 @@ public: } else { auto request = UpdateRequest(); request.setNamespaceString(nsString); - const bool isExplain = true; - makeUpdateRequest(opCtx, args, isExplain, &request); + makeUpdateRequest(opCtx, args, verbosity, &request); const ExtensionsCallbackReal extensionsCallback(opCtx, &request.getNamespaceString()); ParsedUpdate parsedUpdate(opCtx, &request, extensionsCallback); @@ -385,8 +384,8 @@ public: for (;;) { auto request = UpdateRequest(); request.setNamespaceString(nsString); - const bool isExplain = false; - makeUpdateRequest(opCtx, args, isExplain, &request); + const auto verbosity = boost::none; + makeUpdateRequest(opCtx, args, verbosity, &request); if (opCtx->getTxnNumber()) { request.setStmtId(stmtId); diff --git a/src/mongo/db/commands/write_commands/write_commands.cpp b/src/mongo/db/commands/write_commands/write_commands.cpp index 9228fa50fb5..6d1ea157c21 100644 --- a/src/mongo/db/commands/write_commands/write_commands.cpp +++ b/src/mongo/db/commands/write_commands/write_commands.cpp @@ -419,7 +419,7 @@ private: _batch.getRuntimeConstants().value_or(Variables::generateRuntimeConstants(opCtx))); updateRequest.setLetParameters(_batch.getLet()); updateRequest.setYieldPolicy(PlanExecutor::YIELD_AUTO); - updateRequest.setExplain(); + updateRequest.setExplain(verbosity); const ExtensionsCallbackReal extensionsCallback(opCtx, &updateRequest.getNamespaceString()); diff --git a/src/mongo/db/exec/update_stage.cpp b/src/mongo/db/exec/update_stage.cpp index dd7f405115e..8e8c6694865 100644 --- a/src/mongo/db/exec/update_stage.cpp +++ b/src/mongo/db/exec/update_stage.cpp @@ -250,7 +250,7 @@ BSONObj UpdateStage::transformAndUpdate(const Snapshotted& oldObj, Reco RecordId newRecordId; CollectionUpdateArgs args; - if (!request->isExplain()) { + if (!request->explain()) { args.stmtId = request->getStmtId(); args.update = logObj; if (isUserInitiatedWrite) { @@ -272,7 +272,7 @@ BSONObj UpdateStage::transformAndUpdate(const Snapshotted& oldObj, Reco } if (inPlace) { - if (!request->isExplain()) { + if (!request->explain()) { newObj = oldObj.value(); const RecordData oldRec(oldObj.value().objdata(), oldObj.value().objsize()); @@ -302,7 +302,7 @@ BSONObj UpdateStage::transformAndUpdate(const Snapshotted& oldObj, Reco << BSONObjMaxUserSize, newObj.objsize() <= BSONObjMaxUserSize); - if (!request->isExplain()) { + if (!request->explain()) { if (_shouldCheckForShardKeyUpdate && checkUpdateChangesShardKeyFields(oldObj) && !args.preImageDoc) { args.preImageDoc = oldObj.value().getOwned(); @@ -336,7 +336,7 @@ BSONObj UpdateStage::transformAndUpdate(const Snapshotted& oldObj, Reco // Only record doc modifications if they wrote (exclude no-ops). Explains get // recorded as if they wrote. - if (docWasModified || request->isExplain()) { + if (docWasModified || request->explain()) { _specificStats.nModified++; } diff --git a/src/mongo/db/exec/upsert_stage.cpp b/src/mongo/db/exec/upsert_stage.cpp index dc81610bb96..5860ccfb1cf 100644 --- a/src/mongo/db/exec/upsert_stage.cpp +++ b/src/mongo/db/exec/upsert_stage.cpp @@ -91,7 +91,7 @@ PlanStage::StageState UpsertStage::doWork(WorkingSetID* out) { _specificStats.objInserted = _produceNewDocumentForInsert(); // If this is an explain, skip performing the actual insert. - if (!_params.request->isExplain()) { + if (!_params.request->explain()) { _performInsert(_specificStats.objInserted); } diff --git a/src/mongo/db/ops/parsed_update.cpp b/src/mongo/db/ops/parsed_update.cpp index 18285ed37d5..34458e1f8dd 100644 --- a/src/mongo/db/ops/parsed_update.cpp +++ b/src/mongo/db/ops/parsed_update.cpp @@ -45,11 +45,15 @@ ParsedUpdate::ParsedUpdate(OperationContext* opCtx, const ExtensionsCallback& extensionsCallback) : _opCtx(opCtx), _request(request), - _expCtx(make_intrusive(opCtx, - nullptr, - _request->getNamespaceString(), - _request->getRuntimeConstants(), - _request->getLetParameters())), + _expCtx(make_intrusive( + opCtx, + nullptr, + _request->getNamespaceString(), + _request->getRuntimeConstants(), + _request->getLetParameters(), + true, // mayDbProfile. We pass 'true' here conservatively. In the future we may + // change this. + request->explain())), _driver(_expCtx), _canonicalQuery(), _extensionsCallback(extensionsCallback) {} @@ -124,7 +128,7 @@ Status ParsedUpdate::parseQueryToCQ() { auto qr = std::make_unique(_request->getNamespaceString()); qr->setFilter(_request->getQuery()); qr->setSort(_request->getSort()); - qr->setExplain(_request->isExplain()); + qr->setExplain(static_cast(_request->explain())); qr->setHint(_request->getHint()); // We get the collation off the ExpressionContext because it may contain a collection-default diff --git a/src/mongo/db/ops/update.cpp b/src/mongo/db/ops/update.cpp index 62e88d71e2e..c065f670167 100644 --- a/src/mongo/db/ops/update.cpp +++ b/src/mongo/db/ops/update.cpp @@ -57,7 +57,7 @@ UpdateResult update(OperationContext* opCtx, Database* db, const UpdateRequest& invariant(db); // Explain should never use this helper. - invariant(!request.isExplain()); + invariant(!request.explain()); const NamespaceString& nsString = request.getNamespaceString(); invariant(opCtx->lockState()->isCollectionLockedForMode(nsString, MODE_IX)); diff --git a/src/mongo/db/ops/update_request.h b/src/mongo/db/ops/update_request.h index 6404b5dcd70..d5bbbd4bdcc 100644 --- a/src/mongo/db/ops/update_request.h +++ b/src/mongo/db/ops/update_request.h @@ -196,12 +196,12 @@ public: return _fromOplogApplication; } - void setExplain(bool value = true) { - _isExplain = value; + void setExplain(boost::optional verbosity) { + _explain = verbosity; } - bool isExplain() const { - return _isExplain; + boost::optional explain() const { + return _explain; } void setReturnDocs(ReturnDocOption value) { @@ -277,7 +277,7 @@ public: builder << " multi: " << isMulti(); builder << " fromMigration: " << _fromMigration; builder << " fromOplogApplication: " << _fromOplogApplication; - builder << " isExplain: " << _isExplain; + builder << " isExplain: " << static_cast(_explain); return builder.str(); } @@ -314,8 +314,9 @@ private: // True if this update was triggered by the application of an oplog entry. bool _fromOplogApplication = false; - // Whether or not we are requesting an explained update. Explained updates are read-only. - bool _isExplain = false; + // Whether or not we are requesting an explained update, and if so, which type. Explained + // updates may involve executing stages, but they will not perform writes. + boost::optional _explain; // Specifies which version of the documents to return, if any. // diff --git a/src/mongo/db/pipeline/expression_context.cpp b/src/mongo/db/pipeline/expression_context.cpp index 8625cf80903..f33d6a16e06 100644 --- a/src/mongo/db/pipeline/expression_context.cpp +++ b/src/mongo/db/pipeline/expression_context.cpp @@ -134,8 +134,10 @@ ExpressionContext::ExpressionContext(OperationContext* opCtx, const NamespaceString& nss, const boost::optional& runtimeConstants, const boost::optional& letParameters, - bool mayDbProfile) - : ns(nss), + bool mayDbProfile, + boost::optional explain) + : explain(explain), + ns(nss), opCtx(opCtx), mongoProcessInterface(std::make_shared()), timeZoneDatabase(opCtx && opCtx->getServiceContext() diff --git a/src/mongo/db/pipeline/expression_context.h b/src/mongo/db/pipeline/expression_context.h index d3a491119d3..6cd1bba4f3b 100644 --- a/src/mongo/db/pipeline/expression_context.h +++ b/src/mongo/db/pipeline/expression_context.h @@ -142,7 +142,8 @@ public: const NamespaceString& ns, const boost::optional& runtimeConstants = boost::none, const boost::optional& letParameters = boost::none, - bool mayDbProfile = true); + bool mayDbProfile = true, + boost::optional explain = boost::none); /** * Used by a pipeline to check for interrupts so that killOp() works. Throws a UserAssertion if diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp index b1bae642936..c27145180ac 100644 --- a/src/mongo/db/query/get_executor.cpp +++ b/src/mongo/db/query/get_executor.cpp @@ -915,7 +915,7 @@ StatusWith> getExecutorUpdate( // the collection prior to calling this method. Explain, however, will never do // collection or database creation. if (!collection && request->isUpsert()) { - invariant(request->isExplain()); + invariant(request->explain()); } // If the parsed update does not have a user-specified collation, set it from the collection @@ -1002,9 +1002,6 @@ StatusWith> getExecutorUpdate( // This is the regular path for when we have a CanonicalQuery. unique_ptr cq(parsedUpdate->releaseParsedQuery()); - // Transfer the explain verbosity level into the expression context. - cq->getExpCtx()->explain = verbosity; - std::unique_ptr projection; if (!request->getProj().isEmpty()) { invariant(request->shouldReturnAnyDocs()); -- cgit v1.2.1