SERVER-52739: Numerical stability issue in StdDev functions

2 files changed, 27 insertions, 4 deletions

diff --git a/src/mongo/db/pipeline/accumulator_std_dev.cpp b/src/mongo/db/pipeline/accumulator_std_dev.cpp
index 84588fef6f3..41e31a7f6b6 100644
--- a/src/mongo/db/pipeline/accumulator_std_dev.cpp
+++ b/src/mongo/db/pipeline/accumulator_std_dev.cpp
@@ -61,8 +61,10 @@ void AccumulatorStdDev::processInternal(const Value& input, bool merging) {
         // http://en.wikipedia.org/wiki/Algorithms_for_calculating_variance#Online_algorithm
         _count += 1;
         const double delta = val - _mean;
-        _mean += delta / _count;
-        _m2 += delta * (val - _mean);
+        if (delta != 0.0) {
+            _mean += delta / _count;
+            _m2 += delta * (val - _mean);
+        }
     } else {
         // This is what getValue(true) produced below.
         verify(input.getType() == Object);
@@ -78,8 +80,13 @@ void AccumulatorStdDev::processInternal(const Value& input, bool merging) {
         const double delta = mean - _mean;
         const long long newCount = count + _count;
 
-        _mean = ((_count * _mean) + (count * mean)) / newCount;
-        _m2 += m2 + (delta * delta * (double(_count) * count / newCount));
+        if (delta != 0.0) {
+            // Avoid potential numerical stability issues.
+            _mean = ((_count * _mean) + (count * mean)) / newCount;
+            _m2 += delta * delta * (double(_count) * count / newCount);
+        }
+        _m2 += m2;
+
         _count = newCount;
     }
 }
diff --git a/src/mongo/db/pipeline/expression_test.cpp b/src/mongo/db/pipeline/expression_test.cpp
index 19fbd6e68bf..41bb7c2df80 100644
--- a/src/mongo/db/pipeline/expression_test.cpp
+++ b/src/mongo/db/pipeline/expression_test.cpp
@@ -775,6 +775,22 @@ TEST(ExpressionFromAccumulators, StdDevSamp) {
          {{}, Value(BSONNULL)}});
 }
 
+TEST(ExpressionFromAccumulators, StdDevSampRepeated) {
+    ExpressionContextForTest expCtx;
+    AccumulatorStdDevPop mergedAcc(&expCtx);
+
+    for (int i = 0; i < 100; i++) {
+        AccumulatorStdDevPop acc(&expCtx);
+        acc.process(Value(std::exp(14.0)), false /*merging*/);
+        Value mergedValue = acc.getValue(true /*toBeMerged*/);
+        mergedAcc.process(mergedValue, true /*merging*/);
+    }
+
+    Value result = mergedAcc.getValue(false /*toBeMerged*/);
+    const double doubleVal = result.coerceToDouble();
+    ASSERT_EQ(0.0, doubleVal);
+}
+
 TEST(ExpressionPowTest, LargeExponentValuesWithBaseOfZero) {
     assertExpectedResults(
         "$pow",