SERVER-59159 Update $min/max window functions to ignore null values to match their accumulator counterparts

(cherry picked from commit ba83473ade3ee39cd9053ed3b2b3bf3e5f64ba28)

2 files changed, 40 insertions, 16 deletions

diff --git a/src/mongo/db/pipeline/window_function/window_function_min_max.h b/src/mongo/db/pipeline/window_function/window_function_min_max.h
index 8c5d2822696..565cc07f4a6 100644
--- a/src/mongo/db/pipeline/window_function/window_function_min_max.h
+++ b/src/mongo/db/pipeline/window_function/window_function_min_max.h
@@ -37,12 +37,18 @@ namespace mongo {
 template <AccumulatorMinMax::Sense sense>
 class WindowFunctionMinMaxCommon : public WindowFunctionState {
 public:
-    void add(Value value) override {
+    void add(Value value) final {
+        // Ignore nullish values.
+        if (value.nullish())
+            return;
         _memUsageBytes += value.getApproximateSize();
         _values.insert(std::move(value));
     }
 
-    void remove(Value value) override {
+    void remove(Value value) final {
+        // Ignore nullish values.
+        if (value.nullish())
+            return;
         // std::multiset::insert is guaranteed to put the element after any equal elements
         // already in the container. So find() / erase() will remove the oldest equal element,
         // which is what we want, to satisfy "remove() undoes add() when called in FIFO order".
@@ -111,20 +117,6 @@ public:
         _memUsageBytes = sizeof(*this);
     }
 
-    void add(Value value) final {
-        // Ignore nullish values.
-        if (value.nullish())
-            return;
-        WindowFunctionMinMaxCommon<sense>::add(std::move(value));
-    }
-
-    void remove(Value value) final {
-        // Ignore nullish values.
-        if (value.nullish())
-            return;
-        WindowFunctionMinMaxCommon<sense>::remove(std::move(value));
-    }
-
     Value getValue() const final {
         if (_values.empty()) {
             return Value(std::vector<Value>());
diff --git a/src/mongo/db/pipeline/window_function/window_function_min_max_test.cpp b/src/mongo/db/pipeline/window_function/window_function_min_max_test.cpp
index 68a4cabe0de..aaf913e5786 100644
--- a/src/mongo/db/pipeline/window_function/window_function_min_max_test.cpp
+++ b/src/mongo/db/pipeline/window_function/window_function_min_max_test.cpp
@@ -80,6 +80,38 @@ TEST_F(WindowFunctionMinMaxTest, SmallWindow) {
     ASSERT_VALUE_EQ(max.getValue(), Value{8});
 }
 
+TEST_F(WindowFunctionMinMaxTest, NullsAndMissing) {
+    min.add(Value{2});
+
+    // Nulls should be ignored.
+    min.add(Value{BSONNULL});
+    ASSERT_VALUE_EQ(min.getValue(), Value{2});
+
+    // Missing values should be ignored.
+    min.add(Value());
+    ASSERT_VALUE_EQ(min.getValue(), Value{2});
+
+    // Removal of null/missing values is a no-op.
+    min.remove(Value{BSONNULL});
+    min.remove(Value());
+    ASSERT_VALUE_EQ(min.getValue(), Value{2});
+
+    max.add(Value{BSONNULL});
+    max.add(Value{8});
+
+    // Nulls should be ignored.
+    ASSERT_VALUE_EQ(max.getValue(), Value{8});
+
+    // Missing values should be ignored.
+    max.add(Value());
+    ASSERT_VALUE_EQ(max.getValue(), Value{8});
+
+    // Removal of null/missing values is a no-op.
+    max.remove(Value{BSONNULL});
+    max.remove(Value());
+    ASSERT_VALUE_EQ(max.getValue(), Value{8});
+}
+
 TEST_F(WindowFunctionMinMaxTest, Removal) {
     min.add(Value{5});
     min.add(Value{2});