SERVER-43267 Fail MapReduce when source collection is the same as destination collection

Author: mtandrei

src/mongo/db/commands/map_reduce_agg_test.cpp

@@ -46,6 +46,15 @@
 #include "mongo/db/pipeline/expression_context_for_test.h"
 #include "mongo/unittest/unittest.h"
 
+#define ASSERT_DOES_NOT_THROW(EXPRESSION)                                          \
+    try {                                                                          \
+        EXPRESSION;                                                                \
+    } catch (const AssertionException& e) {                                        \
+        str::stream err;                                                           \
+        err << "Threw an exception incorrectly: " << e.toString();                 \
+        ::mongo::unittest::TestAssertionFailure(__FILE__, __LINE__, err).stream(); \
+    }
+
 namespace mongo {
 namespace {
 
@@ -212,5 +221,28 @@ TEST(MapReduceAggTest, testOutSameCollection) {
     ASSERT(typeid(DocumentSourceOut) == typeid(**iter));
 }
 
+TEST(MapReduceAggTest, testSourceDestinationCollectionsEqualMergeFail) {
+    auto nss = NamespaceString{"db", "coll"};
+    auto mr = MapReduce{
+        nss,
+        MapReduceJavascriptCode{mapJavascript.toString()},
+        MapReduceJavascriptCode{reduceJavascript.toString()},
+        MapReduceOutOptions{boost::make_optional("db"s), "coll", OutputType::Merge, false}};
+    boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest(nss));
+    ASSERT_THROWS_CODE(
+        map_reduce_common::translateFromMR(mr, expCtx), DBException, ErrorCodes::InvalidOptions);
+}
+
+TEST(MapReduceAggTest, testSourceDestinationCollectionsNotEqualMergeDoesNotFail) {
+    auto nss = NamespaceString{"db", "coll"};
+    auto mr = MapReduce{
+        nss,
+        MapReduceJavascriptCode{mapJavascript.toString()},
+        MapReduceJavascriptCode{reduceJavascript.toString()},
+        MapReduceOutOptions{boost::make_optional("db2"s), "coll", OutputType::Merge, false}};
+    boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest(nss));
+    ASSERT_DOES_NOT_THROW(map_reduce_common::translateFromMR(mr, expCtx));
+}
+
 }  // namespace
 }  // namespace mongo

src/mongo/db/commands/mr_common.cpp

@@ -302,6 +302,23 @@ bool mrSupportsWriteConcern(const BSONObj& cmd) {
 
 std::unique_ptr<Pipeline, PipelineDeleter> translateFromMR(
     MapReduce parsedMr, boost::intrusive_ptr<ExpressionContext> expCtx) {
+    // Verify that source and output collections are different.
+    // Note that $out allows for the source and the destination to match, so only reject
+    // in the case that the out option is being converted to a $merge.
+    auto& inNss = parsedMr.getNamespace();
+    auto outNss = NamespaceString{parsedMr.getOutOptions().getDatabaseName()
+                                      ? *parsedMr.getOutOptions().getDatabaseName()
+                                      : parsedMr.getNamespace().db(),
+                                  parsedMr.getOutOptions().getCollectionName()};
+
+    auto outType = parsedMr.getOutOptions().getOutputType();
+    if (outType == OutputType::Merge || outType == OutputType::Reduce) {
+        uassert(ErrorCodes::InvalidOptions,
+                "Source collection cannot be the same as destination collection in MapReduce when "
+                "using merge or "
+                "reduce actions",
+                inNss != outNss);
+    }
 
     // TODO: It would be good to figure out what kind of errors this would produce in the Status.
     // It would be better not to produce something incomprehensible out of an internal translation.
@@ -318,12 +335,9 @@ std::unique_ptr<Pipeline, PipelineDeleter> translateFromMR(
                 return translateFinalize(expCtx, parsedMr.getFinalize()->getCode());
             }),
             translateOut(expCtx,
-                         parsedMr.getOutOptions().getOutputType(),
+                         outType,
                          parsedMr.getNamespace().db(),
-                         NamespaceString{parsedMr.getOutOptions().getDatabaseName()
-                                             ? *parsedMr.getOutOptions().getDatabaseName()
-                                             : parsedMr.getNamespace().db(),
-                                         parsedMr.getOutOptions().getCollectionName()},
+                         std::move(outNss),
                          parsedMr.getReduce().getCode())),
         expCtx));
 }