SERVER-37182 Correctly handle duplicate fields in $arrayToObject

3 files changed, 106 insertions, 102 deletions

diff --git a/jstests/aggregation/expressions/arrayToObject.js b/jstests/aggregation/expressions/arrayToObject.js
index 5ec450a79eb..1251df20348 100644
--- a/jstests/aggregation/expressions/arrayToObject.js
+++ b/jstests/aggregation/expressions/arrayToObject.js
@@ -8,106 +8,70 @@
     let coll = db.array_to_object_expr;
     coll.drop();
 
-    let array_to_object_expr = {$project: {collapsed: {$arrayToObject: "$expanded"}}};
+    // Write one document so that the aggregations which use $const produce a result.
+    assert.writeOK(coll.insert({_id: "sentinel", a: 1}));
+
+    /*
+     * Check that the collapsed, object form of 'expanded' (which is computed using $arrayToObject)
+     * matches our expectation.
+     */
+    function assertCollapsed(expanded, expectedCollapsed) {
+        const result =
+            coll.aggregate(
+                    [{$project: {collapsed: {$arrayToObject: {$const: expanded}}}}, {$limit: 1}])
+                .toArray();
+        assert.eq(result, [{_id: "sentinel", collapsed: expectedCollapsed}]);
+    }
+
+    /*
+     * Check that $arrayToObject on the given value produces the expected error.
+     */
+    function assertPipelineErrors(expanded, errorCode) {
+        assertErrorCode(
+            coll,
+            [{$project: {collapsed: {$arrayToObject: {$const: expanded}}}}, {$limit: 1}],
+            errorCode);
+    }
 
     // $arrayToObject correctly converts a key-value pairs to an object.
-    assert.writeOK(coll.insert({_id: 0, expanded: [["price", 24], ["item", "apple"]]}));
-    let result = coll.aggregate([{$match: {_id: 0}}, array_to_object_expr]).toArray();
-    assert.eq(result, [{_id: 0, collapsed: {"price": 24, "item": "apple"}}]);
-
-    assert.writeOK(
-        coll.insert({_id: 1, expanded: [{"k": "price", "v": 24}, {"k": "item", "v": "apple"}]}));
-    result = coll.aggregate([{$match: {_id: 1}}, array_to_object_expr]).toArray();
-    assert.eq(result, [{_id: 1, collapsed: {"price": 24, "item": "apple"}}]);
-
-    assert.writeOK(
-        coll.insert({_id: 2, expanded: [{"k": "price", "v": 24}, {"k": "item", "v": "apple"}]}));
-    result =
-        coll.aggregate([
-                {$match: {_id: 2}},
-                {
-                  $project: {
-                      collapsed:
-                          {$arrayToObject: {$zip: {inputs: ["$expanded.k", "$expanded.v"]}}}
-                  }
-                }
-            ])
-            .toArray();
-    assert.eq(result, [{_id: 2, collapsed: {"price": 24, "item": "apple"}}]);
-
-    assert.writeOK(coll.insert({_id: 3, expanded: []}));
-    result = coll.aggregate([{$match: {_id: 3}}, array_to_object_expr]).toArray();
-    assert.eq(result, [{_id: 3, collapsed: {}}]);
-
-    // $arrayToObject outputs null on null-ish types.
-    assert.writeOK(coll.insert({_id: 4}));
-    result = coll.aggregate([{$match: {_id: 4}}, array_to_object_expr]).toArray();
-    assert.eq(result, [{_id: 4, collapsed: null}]);
-
-    assert.writeOK(coll.insert({_id: 5, expanded: null}));
-    result = coll.aggregate([{$match: {_id: 5}}, array_to_object_expr]).toArray();
-    assert.eq(result, [{_id: 5, collapsed: null}]);
-
-    assert.writeOK(coll.insert({_id: 6, expanded: undefined}));
-    result = coll.aggregate([{$match: {_id: 6}}, array_to_object_expr]).toArray();
-    assert.eq(result, [{_id: 6, collapsed: null}]);
-
-    assert.writeOK(coll.insert({_id: 7, expanded: [{"k": "price", "v": null}]}));
-    result = coll.aggregate([{$match: {_id: 7}}, array_to_object_expr]).toArray();
-    assert.eq(result, [{_id: 7, collapsed: {"price": null}}]);
-
-    assert.writeOK(coll.insert({_id: 8, expanded: [{"k": "price", "v": undefined}]}));
-    result = coll.aggregate([{$match: {_id: 8}}, array_to_object_expr]).toArray();
-    assert.eq(result, [{_id: 8, collapsed: {"price": undefined}}]);
-
-    assert.writeOK(coll.insert({_id: 9, expanded: [{"k": "price", "v": 24}, ["item", "apple"]]}));
-    assertErrorCode(coll, [{$match: {_id: 9}}, array_to_object_expr], 40391);
-
-    assert.writeOK(coll.insert({_id: 10, expanded: [["item", "apple"], {"k": "price", "v": 24}]}));
-    assertErrorCode(coll, [{$match: {_id: 10}}, array_to_object_expr], 40396);
-
-    assert.writeOK(coll.insert({_id: 11, expanded: "string"}));
-    assertErrorCode(coll, [{$match: {_id: 11}}, array_to_object_expr], 40386);
-
-    assert.writeOK(coll.insert({_id: 12, expanded: ObjectId()}));
-    assertErrorCode(coll, [{$match: {_id: 12}}, array_to_object_expr], 40386);
-
-    assert.writeOK(coll.insert({_id: 13, expanded: NumberLong(0)}));
-    assertErrorCode(coll, [{$match: {_id: 13}}, array_to_object_expr], 40386);
-
-    assert.writeOK(coll.insert({_id: 14, expanded: [0]}));
-    assertErrorCode(coll, [{$match: {_id: 14}}, array_to_object_expr], 40398);
-
-    assert.writeOK(coll.insert({_id: 15, expanded: [["missing_value"]]}));
-    assertErrorCode(coll, [{$match: {_id: 15}}, array_to_object_expr], 40397);
-
-    assert.writeOK(coll.insert({_id: 16, expanded: [[321, 12]]}));
-    assertErrorCode(coll, [{$match: {_id: 16}}, array_to_object_expr], 40395);
-
-    assert.writeOK(coll.insert({_id: 17, expanded: [["key", "value", "offset"]]}));
-    assertErrorCode(coll, [{$match: {_id: 17}}, array_to_object_expr], 40397);
-
-    assert.writeOK(coll.insert({_id: 18, expanded: {y: []}}));
-    assertErrorCode(coll, [{$match: {_id: 18}}, array_to_object_expr], 40386);
-
-    assert.writeOK(coll.insert({_id: 19, expanded: [{y: "x", x: "y"}]}));
-    assertErrorCode(coll, [{$match: {_id: 19}}, array_to_object_expr], 40393);
-
-    assert.writeOK(coll.insert({_id: 20, expanded: [{k: "missing"}]}));
-    assertErrorCode(coll, [{$match: {_id: 20}}, array_to_object_expr], 40392);
-
-    assert.writeOK(coll.insert({_id: 21, expanded: [{k: 24, v: "string"}]}));
-    assertErrorCode(coll, [{$match: {_id: 21}}, array_to_object_expr], 40394);
-
-    assert.writeOK(coll.insert({_id: 22, expanded: [{k: null, v: "nullKey"}]}));
-    assertErrorCode(coll, [{$match: {_id: 22}}, array_to_object_expr], 40394);
-
-    assert.writeOK(coll.insert({_id: 23, expanded: [{k: undefined, v: "undefinedKey"}]}));
-    assertErrorCode(coll, [{$match: {_id: 23}}, array_to_object_expr], 40394);
-
-    assert.writeOK(coll.insert({_id: 24, expanded: [{y: "ignored", k: "item", v: "pear"}]}));
-    assertErrorCode(coll, [{$match: {_id: 24}}, array_to_object_expr], 40392);
-
-    assert.writeOK(coll.insert({_id: 25, expanded: NaN}));
-    assertErrorCode(coll, [{$match: {_id: 25}}, array_to_object_expr], 40386);
+    assertCollapsed([["price", 24], ["item", "apple"]], {"price": 24, "item": "apple"});
+    assertCollapsed([{"k": "price", "v": 24}, {"k": "item", "v": "apple"}],
+                    {"price": 24, "item": "apple"});
+    // If duplicate field names are in the array, $arrayToObject should use value from the last one.
+    assertCollapsed([{"k": "price", "v": 24}, {"k": "price", "v": 100}], {"price": 100});
+    assertCollapsed([["price", 24], ["price", 100]], {"price": 100});
+
+    assertCollapsed([["price", 24], ["item", "apple"]], {"price": 24, "item": "apple"});
+    assertCollapsed([], {});
+
+    assertCollapsed(null, null);
+    assertCollapsed(undefined, null);
+    assertCollapsed([{"k": "price", "v": null}], {"price": null});
+    assertCollapsed([{"k": "price", "v": undefined}], {"price": undefined});
+    // Need to manually check the case where 'expanded' is not in the document.
+    assert.writeOK(coll.insert({_id: "missing-expanded-field"}));
+    const result = coll.aggregate([
+                           {$match: {_id: "missing-expanded-field"}},
+                           {$project: {collapsed: {$arrayToObject: "$expanded"}}}
+                       ])
+                       .toArray();
+    assert.eq(result, [{_id: "missing-expanded-field", collapsed: null}]);
+
+    assertPipelineErrors([{"k": "price", "v": 24}, ["item", "apple"]], 40391);
+    assertPipelineErrors([["item", "apple"], {"k": "price", "v": 24}], 40396);
+    assertPipelineErrors("string", 40386);
+    assertPipelineErrors(ObjectId(), 40386);
+    assertPipelineErrors(NumberLong(0), 40386);
+    assertPipelineErrors([0], 40398);
+    assertPipelineErrors([["missing_value"]], 40397);
+    assertPipelineErrors([[321, 12]], 40395);
+    assertPipelineErrors([["key", "value", "offset"]], 40397);
+    assertPipelineErrors({y: []}, 40386);
+    assertPipelineErrors([{y: "x", x: "y"}], 40393);
+    assertPipelineErrors([{k: "missing"}], 40392);
+    assertPipelineErrors([{k: 24, v: "string"}], 40394);
+    assertPipelineErrors([{k: null, v: "nullKey"}], 40394);
+    assertPipelineErrors([{k: undefined, v: "undefinedKey"}], 40394);
+    assertPipelineErrors([{y: "ignored", k: "item", v: "pear"}], 40392);
+    assertPipelineErrors(NaN, 40386);
 }());
diff --git a/src/mongo/db/pipeline/expression.cpp b/src/mongo/db/pipeline/expression.cpp
index 7406e2ccaea..a86eb18f2f8 100644
--- a/src/mongo/db/pipeline/expression.cpp
+++ b/src/mongo/db/pipeline/expression.cpp
@@ -688,7 +688,7 @@ Value ExpressionArrayToObject::evaluateInternal(Variables* vars) const {
                                   << typeName(valArray[0].getType()),
                     (valArray[0].getType() == BSONType::String));
 
-            output.addField(valArray[0].getString(), valArray[1]);
+            output[valArray[0].getString()] = valArray[1];
 
         } else {
             uassert(
@@ -720,7 +720,7 @@ Value ExpressionArrayToObject::evaluateInternal(Variables* vars) const {
                               << typeName(key.getType()),
                 (key.getType() == BSONType::String));
 
-            output.addField(key.getString(), value);
+            output[key.getString()] = value;
         }
     }
 
diff --git a/src/mongo/db/pipeline/expression_test.cpp b/src/mongo/db/pipeline/expression_test.cpp
index ac427e61b81..8acf746a696 100644
--- a/src/mongo/db/pipeline/expression_test.cpp
+++ b/src/mongo/db/pipeline/expression_test.cpp
@@ -681,6 +681,46 @@ TEST_F(ExpressionNaryTest, FlattenInnerOperandsOptimizationOnCommutativeAndAssoc
     assertContents(_associativeAndCommutative, expectedContent);
 }
 
+/* ------------------------- ExpressionArrayToObject -------------------------- */
+
+TEST(ExpressionArrayToObjectTest, KVFormatSimple) {
+    assertExpectedResults("$arrayToObject",
+                          {{{Value(BSON_ARRAY(BSON("k"
+                                                   << "key1"
+                                                   << "v"
+                                                   << 2)
+                                              << BSON("k"
+                                                      << "key2"
+                                                      << "v"
+                                                      << 3)))},
+                            {Value(BSON("key1" << 2 << "key2" << 3))}}});
+}
+
+TEST(ExpressionArrayToObjectTest, KVFormatWithDuplicates) {
+    assertExpectedResults("$arrayToObject",
+                          {{{Value(BSON_ARRAY(BSON("k"
+                                                   << "hi"
+                                                   << "v"
+                                                   << 2)
+                                              << BSON("k"
+                                                      << "hi"
+                                                      << "v"
+                                                      << 3)))},
+                            {Value(BSON("hi" << 3))}}});
+}
+
+TEST(ExpressionArrayToObjectTest, ListFormatSimple) {
+    assertExpectedResults("$arrayToObject",
+                          {{{Value(BSON_ARRAY(BSON_ARRAY("key1" << 2) << BSON_ARRAY("key2" << 3)))},
+                            {Value(BSON("key1" << 2 << "key2" << 3))}}});
+}
+
+TEST(ExpressionArrayToObjectTest, ListFormWithDuplicates) {
+    assertExpectedResults("$arrayToObject",
+                          {{{Value(BSON_ARRAY(BSON_ARRAY("key1" << 2) << BSON_ARRAY("key1" << 3)))},
+                            {Value(BSON("key1" << 3))}}});
+}
+
 /* ------------------------- ExpressionCeil -------------------------- */
 
 class ExpressionCeilTest : public ExpressionNaryTestOneArg {