SERVER-64227 Remove $sum/$avg merging logic to handle the old over-the-wire format

2 files changed, 24 insertions, 96 deletions

diff --git a/src/mongo/db/pipeline/accumulator_avg.cpp b/src/mongo/db/pipeline/accumulator_avg.cpp
index 48d955806ad..1d1f22ad46a 100644
--- a/src/mongo/db/pipeline/accumulator_avg.cpp
+++ b/src/mongo/db/pipeline/accumulator_avg.cpp
@@ -49,15 +49,6 @@ REGISTER_ACCUMULATOR(avg, genericParseSingleExpressionAccumulator<AccumulatorAvg
 REGISTER_STABLE_EXPRESSION(avg, ExpressionFromAccumulator<AccumulatorAvg>::parse);
 REGISTER_STABLE_REMOVABLE_WINDOW_FUNCTION(avg, AccumulatorAvg, WindowFunctionAvg);
 
-namespace {
-// TODO SERVER-64227 Remove 'subTotal' and 'subTotalError' fields when we branch for 6.1 because all
-// nodes in a sharded cluster would use the new data format.
-const char subTotalName[] = "subTotal";
-const char subTotalErrorName[] = "subTotalError";  // Used for extra precision
-const char partialSumName[] = "ps";                // Used for the full state of partial sum
-const char countName[] = "count";
-}  // namespace
-
 void applyPartialSum(const std::vector<Value>& arr,
                      BSONType& nonDecimalTotalType,
                      BSONType& totalType,
@@ -71,33 +62,21 @@ Value serializePartialSum(BSONType nonDecimalTotalType,
 
 void AccumulatorAvg::processInternal(const Value& input, bool merging) {
     if (merging) {
-        // We expect an object that contains both a subtotal and a count. Additionally there may
-        // be an error value, that allows for additional precision.
-        // 'input' is what getValue(true) produced below.
+        // We expect an object that contains both a partial sum and a count.
         verify(input.getType() == Object);
 
-        // TODO SERVER-64227 Remove 'if' block when we branch for 6.1 because all nodes in a sharded
-        // cluster would use the new data format.
-        if (auto partialSumVal = input[partialSumName]; partialSumVal.missing()) {
-            // We're recursively adding the subtotal to get the proper type treatment, but this only
-            // increments the count by one, so adjust the count afterwards. Similarly for 'error'.
-            processInternal(input[subTotalName], false);
-            _count += input[countName].getLong() - 1;
-            Value error = input[subTotalErrorName];
-            if (!error.missing()) {
-                processInternal(error, false);
-                _count--;  // The error correction only adjusts the total, not the number of items.
-            }
-        } else {
-            // The merge-side must be ready to process the full state of a partial sum from a
-            // shard-side if a shard chooses to do so. See Accumulator::getValue() for details.
-            applyPartialSum(partialSumVal.getArray(),
-                            _nonDecimalTotalType,
-                            _totalType,
-                            _nonDecimalTotal,
-                            _decimalTotal);
-            _count += input[countName].getLong();
-        }
+        auto partialSumVal = input[stage_builder::partialSumName];
+        tassert(6422700, "'ps' field must be present", !partialSumVal.missing());
+        tassert(6422701, "'ps' field must be an array", partialSumVal.isArray());
+
+        // The merge-side must be ready to process the full state of a partial sum from a
+        // shard-side if a shard chooses to do so. See Accumulator::getValue() for details.
+        applyPartialSum(partialSumVal.getArray(),
+                        _nonDecimalTotalType,
+                        _totalType,
+                        _nonDecimalTotal,
+                        _decimalTotal);
+        _count += input[stage_builder::countName].getLong();
 
         return;
     }
@@ -146,17 +125,8 @@ Value AccumulatorAvg::getValue(bool toBeMerged) {
     if (toBeMerged) {
         auto partialSumVal =
             serializePartialSum(_nonDecimalTotalType, _totalType, _nonDecimalTotal, _decimalTotal);
-        if (_totalType == NumberDecimal) {
-            return Value(Document{{subTotalName, _getDecimalTotal()},
-                                  {countName, _count},
-                                  {partialSumName, partialSumVal}});
-        }
-
-        auto [total, error] = _nonDecimalTotal.getDoubleDouble();
-        return Value(Document{{subTotalName, total},
-                              {countName, _count},
-                              {subTotalErrorName, error},
-                              {partialSumName, partialSumVal}});
+        return Value(Document{{stage_builder::countName, _count},
+                              {stage_builder::partialSumName, partialSumVal}});
     }
 
     if (_count == 0)
diff --git a/src/mongo/db/pipeline/accumulator_sum.cpp b/src/mongo/db/pipeline/accumulator_sum.cpp
index 12fe26305ec..1f5015200ba 100644
--- a/src/mongo/db/pipeline/accumulator_sum.cpp
+++ b/src/mongo/db/pipeline/accumulator_sum.cpp
@@ -56,11 +56,6 @@ REGISTER_ACCUMULATOR_WITH_MIN_VERSION(count,
 
 REGISTER_STABLE_WINDOW_FUNCTION(count, window_function::parseCountWindowFunction);
 
-namespace {
-const char subTotalName[] = "subTotal";
-const char subTotalErrorName[] = "subTotalError";  // Used for extra precision.
-}  // namespace
-
 void applyPartialSum(const std::vector<Value>& arr,
                      BSONType& nonDecimalTotalType,
                      BSONType& totalType,
@@ -109,27 +104,13 @@ void AccumulatorSum::processInternal(const Value& input, bool merging) {
             return;
         }
 
-        switch (input.getType()) {
-            // TODO SERVER-64227: Remove 'Object' case which is no longer necessary when we
-            // branch for 6.1.
-            case Object:
-                // Process merge document, see getValue() below.
-                nonDecimalTotal.addDouble(
-                    input[subTotalName].getDouble());  // Sum without adjusting type.
-                processInternal(input[subTotalErrorName],
-                                false);  // Sum adjusting for type of error.
-                break;
+        if (input.getType() == BSONType::Array) {
             // The merge-side must be ready to process the full state of a partial sum from a
-            // shard-side if a shard chooses to do so. See Accumulator::getValue() for details.
-            case Array:
-                applyPartialSum(input.getArray(),
-                                nonDecimalTotalType,
-                                totalType,
-                                nonDecimalTotal,
-                                decimalTotal);
-                break;
-            default:
-                MONGO_UNREACHABLE;
+            // shard-side.
+            applyPartialSum(
+                input.getArray(), nonDecimalTotalType, totalType, nonDecimalTotal, decimalTotal);
+        } else {
+            MONGO_UNREACHABLE_TASSERT(64227002);
         }
         return;
     }
@@ -192,21 +173,13 @@ Value AccumulatorSum::getValue(bool toBeMerged) {
     // of some of 'NumberDecimal' values and other numeric type values happen to lose precision
     // because 'NumberDecimal' can't represent the partial sum precisely, or the other way around.
     //
-    // For example, [{n: 1e+34}, {n: NumberDecimal("0,1")}, {n: NumberDecimal("0.11")}, {n:
+    // For example, [{n: 1e+34}, {n: NumberDecimal("0.1")}, {n: NumberDecimal("0.11")}, {n:
     // -1e+34}].
     //
     // More fundamentally, addition is neither commutative nor associative on computer. So, it's
     // desirable to keep the full state of the partial sum along the way to maintain the result as
     // close to the real truth as possible until all additions are done.
-    //
-    // This requires changing over-the-wire data format from a shard-side to a merge-side and is
-    // incompatible change and is gated with FCV until 6.0.
-    //
-    // TODO SERVER-64227: Remove FCV gating which is unnecessary when we branch for 6.1.
-    auto&& fcv = serverGlobalParams.featureCompatibility;
-    auto canUseNewPartialResultFormat = fcv.isVersionInitialized() &&
-        fcv.isGreaterThanOrEqualTo(multiversion::FeatureCompatibilityVersion::kVersion_6_0);
-    if (canUseNewPartialResultFormat && toBeMerged) {
+    if (toBeMerged) {
         return serializePartialSum(nonDecimalTotalType, totalType, nonDecimalTotal, decimalTotal);
     }
 
@@ -218,22 +191,7 @@ Value AccumulatorSum::getValue(bool toBeMerged) {
         case NumberLong:
             if (nonDecimalTotal.fitsLong())
                 return Value(nonDecimalTotal.getLong());
-            // TODO SERVER-64227: Remove the following 'if' block which is buggy.
-            if (toBeMerged) {
-                // The value was too large for a NumberLong, so output a document with two values
-                // adding up to the desired total. Older MongoDB versions used to ignore signed
-                // integer overflow and cause undefined behavior, that in practice resulted in
-                // values that would wrap around modulo 2**64. Now an older mongos with a newer
-                // mongod will yield an error that $sum resulted in a non-numeric type, which is
-                // OK for this case. Output the error using the totalType, so in the future we can
-                // determine the correct totalType for the sum. For the error to exceed 2**63,
-                //  more than 2**53 integers would have to be summed, which is impossible.
-                double total;
-                double error;
-                std::tie(total, error) = nonDecimalTotal.getDoubleDouble();
-                long long llerror = static_cast<long long>(error);
-                return Value(DOC(subTotalName << total << subTotalErrorName << llerror));
-            }
+
             // Sum doesn't fit a NumberLong, so return a NumberDouble instead.
             [[fallthrough]];
         case NumberDouble: