SERVER-62364 Fix $fill arbitrary value filling bug

3 files changed, 99 insertions, 12 deletions

diff --git a/jstests/aggregation/sources/fill/fill.js b/jstests/aggregation/sources/fill/fill.js
index 8ebe1165fbd..39f49563f27 100644
--- a/jstests/aggregation/sources/fill/fill.js
+++ b/jstests/aggregation/sources/fill/fill.js
@@ -1,7 +1,8 @@
 /**
  * Test the syntax of $fill.
  * @tags: [
- *   requires_fcv_52,
+ *   requires_fcv_53,
+ *   do_not_wrap_aggregations_in_facets,
  * ]
  */
 
@@ -189,6 +190,7 @@ const testCases = [
         ]
 
     ],  // 7
+    // Test with first element in partition having a null fill field.
     [
         [
             {$set: {linear: {$cond: [{$eq: ["$linear", 1]}, null, "$linear"]}}},
@@ -212,7 +214,77 @@ const testCases = [
             {_id: 8, linear: 3, other: 5, part: 2},
             {_id: 10, linear: null, other: 5, part: 2}
         ]
-    ],  // 8 Test with first element in partition having a null fill field.
+    ],  // 8
+    // Test $fill with arbitrary values.
+    [
+        [
+            {$match: {part: 1}},
+            {$project: {linear: 0, part: 0}},
+            {$fill: {sortBy: {_id: 1}, output: {other: {value: "$_id"}}}}
+        ],
+        [
+            {_id: 1, other: 1},
+            {_id: 3, other: 3},
+            {_id: 5, other: 10},
+            {_id: 7, other: 7},
+            {_id: 9, other: 15},
+        ]
+    ],  // 9
+    [
+        [
+            {$match: {part: 1}},
+            {$project: {linear: 0, part: 0}},
+            {$fill: {sortBy: {_id: 1}, output: {other: {value: -1}}}}
+        ],
+        [
+            {_id: 1, other: 1},
+            {_id: 3, other: -1},
+            {_id: 5, other: 10},
+            {_id: 7, other: -1},
+            {_id: 9, other: 15},
+        ]
+    ],  // 10
+    [
+        [
+            {$match: {part: 1}},
+            {
+                $fill: {
+                    sortBy: {_id: 1},
+                    output: {other: {value: -1}, linear: {value: {$add: ["$part", 1]}}}
+                }
+            },
+            {$project: {part: 0}},
+        ],
+        [
+            {_id: 1, other: 1, linear: 1},
+            {_id: 3, other: -1, linear: 2},
+            {_id: 5, other: 10, linear: 5},
+            {_id: 7, other: -1, linear: 2},
+            {_id: 9, other: 15, linear: 7},
+        ]
+    ],  // 11
+    // Verify behavior if the filling expression can evaluate to missing or null.
+    [
+        [
+            {$match: {part: 1}},
+            {$project: {part: 0}},
+            {$unionWith: {pipeline: [{$documents: [{_id: 2}, {_id: 4}]}]}},
+            {$fill: {sortBy: {_id: 1}, output: {other: {value: "$linear"}}}},
+            {$project: {linear: 0}},
+        ],
+        [
+            {_id: 1, other: 1},
+            {_id: 2},
+            {_id: 3, other: null},
+            {_id: 4},
+            {_id: 5, other: 10},
+            {_id: 7, other: null},
+            {_id: 9, other: 15},
+
+        ]
+
+    ],  // 12
+
 ];
 
 for (let i = 0; i < testCases.length; i++) {
diff --git a/jstests/aggregation/sources/fill/fill_parse.js b/jstests/aggregation/sources/fill/fill_parse.js
index b034359fb56..f25c1530307 100644
--- a/jstests/aggregation/sources/fill/fill_parse.js
+++ b/jstests/aggregation/sources/fill/fill_parse.js
@@ -79,14 +79,22 @@ let testCases = [
             }
         ],
         []
-    ],                                                                                     // 1
-    [{$fill: {output: {val: {value: 5}}}}, [{"$addFields": {"val": {"$const": 5}}}], []],  // 2
-    [{$fill: {output: {val: {value: "$test"}}}}, [{"$addFields": {"val": "$test"}}], []],  // 3
+    ],  // 1
+    [
+        {$fill: {output: {val: {value: 5}}}},
+        [{"$addFields": {"val": {$ifNull: ["$val", {"$const": 5}]}}}],
+        []
+    ],  // 2
+    [
+        {$fill: {output: {val: {value: "$test"}}}},
+        [{"$addFields": {"val": {$ifNull: ["$val", "$test"]}}}],
+        []
+    ],  // 3
     [
         {$fill: {output: {val: {value: "$test"}, second: {method: "locf"}}}},
         [
             {"$_internalSetWindowFields": {"output": {"second": {"$locf": "$second"}}}},
-            {"$addFields": {"val": "$test"}}
+            {"$addFields": {"val": {$ifNull: ["$val", "$test"]}}}
         ],
         []
     ],  // 4
@@ -106,7 +114,12 @@ let testCases = [
                 "$_internalSetWindowFields":
                     {"output": {"second": {"$locf": "$second"}, "fourth": {"$locf": "$fourth"}}}
             },
-            {"$addFields": {"val": "$test", "third": {"$add": ["$val", "$second"]}}}
+            {
+                "$addFields": {
+                    "val": {$ifNull: ["$val", "$test"]},
+                    "third": {$ifNull: ["$third", {"$add": ["$val", "$second"]}]}
+                }
+            }
         ],
         []
     ],  // 5
@@ -195,7 +208,7 @@ let testCases = [
                 }
             },
             {"$project": {"UUIDPLACEHOLDER": false, "_id": true}},
-            {"$addFields": {"second": {"$const": 7}}}
+            {"$addFields": {"second": {$ifNull: ["$second", {"$const": 7}]}}}
         ],
         [
             [0, "$addFields", true],
@@ -235,8 +248,8 @@ function modifyObjectAtPath(orig, path) {
 
 for (let i = 0; i < testCases.length; i++) {
     let result = desugarSingleStageAggregation(db, coll, testCases[i][0]);
-    // $setWindowFields generates random fieldnames. Use the paths in the test case to replace the
-    // UUID with "UUIDPLACEHOLDER".
+    // $setWindowFields generates random fieldnames. Use the paths in the test case to
+    // replace the UUID with "UUIDPLACEHOLDER".
     if (testCases[i][2].length != 0) {
         for (let pathNum = 0; pathNum < testCases[i][2].length; pathNum++) {
             result = modifyObjectAtPath(result, testCases[i][2][pathNum]);
diff --git a/src/mongo/db/pipeline/document_source_fill.cpp b/src/mongo/db/pipeline/document_source_fill.cpp
index beb335bf31f..ea69f0cdf0a 100644
--- a/src/mongo/db/pipeline/document_source_fill.cpp
+++ b/src/mongo/db/pipeline/document_source_fill.cpp
@@ -96,8 +96,10 @@ std::list<boost::intrusive_ptr<DocumentSource>> createFromBson(
         }
         if (auto&& unparsedValueExpr = parsedSpec.getValue()) {
             // Value fields are BSONAnyType.
-            auto valueObj = unparsedValueExpr.value().getElement().wrap(fieldName);
-            addFieldsSpec.appendElements(valueObj);
+            auto valueElem = unparsedValueExpr.value().getElement();
+            BSONObj fullFieldSpec =
+                BSON(fieldName << BSON("$ifNull" << BSON_ARRAY("$" + fieldName << valueElem)));
+            addFieldsSpec.appendElements(fullFieldSpec);
         }
     }
     setWindowFieldsOutputSpec.done();