SERVER-68544 Assert on overflow of $add[Date, Decimal128] in Classic

4 files changed, 37 insertions, 49 deletions

diff --git a/jstests/aggregation/add_with_date.js b/jstests/aggregation/add_with_date.js
index 03f61d2edd5..b80c304e42c 100644
--- a/jstests/aggregation/add_with_date.js
+++ b/jstests/aggregation/add_with_date.js
@@ -1,6 +1,4 @@
 // Test $add with date
-// TODO SERVER-68544: to remove this tag after fix.
-// @tags: [do_not_wrap_aggregations_in_facets]
 (function() {
 "use strict";
 
@@ -12,7 +10,7 @@ coll.drop();
 
 function getResultOfExpression(expr) {
     const resultArray = coll.aggregate({$project: {computed: expr}}).toArray();
-    assert.eq(1, resultArray.length);
+    assert.eq(1, resultArray.length, "ERROR from " + tojson(expr));
     return resultArray[0].computed;
 }
 
@@ -92,22 +90,12 @@ assert.throwsWithCode(
     () => getResultOfExpression({$add: ["$int64Val", "$dateVal", "$overflowDouble"]}),
     ErrorCodes.Overflow);
 
-// TODO SERVER-68544: classic and sbe have different behavior now, should update after fix.
-if (isSBEEnabled) {
-    // An overflow into the domain of Decimal128 results in an overflow exception.
-    assert.throwsWithCode(() => getResultOfExpression({$add: ["$dateVal", "$overflowDecimal"]}),
-                          ErrorCodes.Overflow);
-    assert.throwsWithCode(
-        () => getResultOfExpression({$add: ["$int64Val", "$dateVal", "$overflowDecimal"]}),
-        ErrorCodes.Overflow);
-} else {
-    // One quirk of date addition semantics is that an overflow into the domain of Decimal128 is not
-    // fatal and instead results in an invalid "NaN" Date value.
-    const nanDate = new Date("");
-    assert.eq(nanDate, getResultOfExpression({$add: ["$dateVal", "$overflowDecimal"]}));
-    assert.eq(nanDate,
-              getResultOfExpression({$add: ["$int64Val", "$dateVal", "$overflowDecimal"]}));
-}
+// An overflow into the domain of Decimal128 results in an overflow exception.
+assert.throwsWithCode(() => getResultOfExpression({$add: ["$dateVal", "$overflowDecimal"]}),
+                      ErrorCodes.Overflow);
+assert.throwsWithCode(
+    () => getResultOfExpression({$add: ["$int64Val", "$dateVal", "$overflowDecimal"]}),
+    ErrorCodes.Overflow);
 assert.throwsWithCode(
     () => getResultOfExpression({$add: ["$dateVal", "$overflowDouble", "$overflowDecimal"]}),
     ErrorCodes.Overflow);
@@ -119,19 +107,11 @@ assert.throwsWithCode(() => getResultOfExpression({$add: ["$dateVal", "$nanDoubl
 assert.throwsWithCode(() => getResultOfExpression({$add: ["$nanDouble", "$dateVal"]}),
                       ErrorCodes.Overflow);
 
-// Adding a Decimal128-typed NaN to a date value.
-// TODO SERVER-68544: classic and sbe have different behavior now, should update after fix.
-if (isSBEEnabled) {
-    // An NaN Decimal128 added to date results in an overflow exception.
-    assert.throwsWithCode(() => getResultOfExpression({$add: ["$dateVal", "$nanDecimal"]}),
-                          ErrorCodes.Overflow);
-    assert.throwsWithCode(() => getResultOfExpression({$add: ["$nanDecimal", "$dateVal"]}),
-                          ErrorCodes.Overflow);
-} else {
-    const nanDate = new Date("");
-    assert.eq(nanDate, getResultOfExpression({$add: ["$dateVal", "$nanDecimal"]}));
-    assert.eq(nanDate, getResultOfExpression({$add: ["$nanDecimal", "$dateVal"]}));
-}
+// An NaN Decimal128 added to date results in an overflow exception.
+assert.throwsWithCode(() => getResultOfExpression({$add: ["$dateVal", "$nanDecimal"]}),
+                      ErrorCodes.Overflow);
+assert.throwsWithCode(() => getResultOfExpression({$add: ["$nanDecimal", "$dateVal"]}),
+                      ErrorCodes.Overflow);
 
 // Addition with a date, a double-typed NaN, and a third value.
 assert.throwsWithCode(() => getResultOfExpression({$add: ["$dateVal", "$doubleVal", "$nanDouble"]}),
diff --git a/src/mongo/db/exec/sbe/values/arith_common.cpp b/src/mongo/db/exec/sbe/values/arith_common.cpp
index 795dec4ea7d..db920191534 100644
--- a/src/mongo/db/exec/sbe/values/arith_common.cpp
+++ b/src/mongo/db/exec/sbe/values/arith_common.cpp
@@ -215,12 +215,12 @@ std::tuple<bool, value::TypeTags, value::Value> genericArithmeticOp(value::TypeT
                     break;
                 }
                 case TypeTags::NumberDecimal: {
-                    using limits = std::numeric_limits<int64_t>;
                     auto decimalRhs = numericCast<Decimal128>(rhsTag, rhsValue);
-                    if (decimalRhs.isGreaterEqual(Decimal128{limits::min()}) &&
-                        decimalRhs.isLess(Decimal128{limits::max()}) &&
-                        !Op::doOperation(
-                            bitcastTo<int64_t>(lhsValue), decimalRhs.toLong(), result)) {
+
+                    std::uint32_t signalingFlags = Decimal128::SignalingFlag::kNoFlag;
+                    std::int64_t longRhs = decimalRhs.toLong(&signalingFlags);
+                    if (signalingFlags == Decimal128::SignalingFlag::kNoFlag &&
+                        !Op::doOperation(bitcastTo<int64_t>(lhsValue), longRhs, result)) {
                         return {false, value::TypeTags::Date, value::bitcastFrom<int64_t>(result)};
                     }
                     break;
diff --git a/src/mongo/db/pipeline/expression.cpp b/src/mongo/db/pipeline/expression.cpp
index fed4d3fbf35..1f8bff1b9d9 100644
--- a/src/mongo/db/pipeline/expression.cpp
+++ b/src/mongo/db/pipeline/expression.cpp
@@ -34,6 +34,7 @@
 
 #include <algorithm>
 #include <boost/algorithm/string.hpp>
+#include <cstdint>
 #include <cstdio>
 #include <utility>
 #include <vector>
@@ -417,7 +418,7 @@ public:
     }
 
 private:
-    // Convert current value into date.
+    // Convert 'valToAdd' into the data type used for dates (long long) and add it to 'longTotal'.
     void addToDateValue(Value valToAdd) {
         switch (valToAdd.getType()) {
             case NumberInt:
@@ -441,10 +442,17 @@ private:
                 }
                 break;
             }
-            case NumberDecimal:
-                // Decimal dates are not checked for overflow.
-                longTotal += valToAdd.coerceToDecimal().toLong();
+            case NumberDecimal: {
+                Decimal128 decimalToAdd = valToAdd.coerceToDecimal();
+
+                std::uint32_t signalingFlags = Decimal128::SignalingFlag::kNoFlag;
+                std::int64_t longToAdd = decimalToAdd.toLong(&signalingFlags);
+                if (signalingFlags != Decimal128::SignalingFlag::kNoFlag ||
+                    overflow::add(longTotal, longToAdd, &longTotal)) {
+                    uasserted(ErrorCodes::Overflow, "date overflow in $add");
+                }
                 break;
+            }
             default:
                 MONGO_UNREACHABLE;
         }
diff --git a/src/mongo/platform/decimal128.h b/src/mongo/platform/decimal128.h
index f14e6b6d73b..34002337786 100644
--- a/src/mongo/platform/decimal128.h
+++ b/src/mongo/platform/decimal128.h
@@ -355,8 +355,8 @@ public:
      * This set of functions converts a Decimal128 to a certain integer type with a
      * given rounding mode.
      *
-     * Each function is overloaded to provide an optional signalingFlags output parameter
-     * that can be set to one of the Decimal128::SignalingFlag enumerators:
+     * Each function is overloaded to provide an optional signalingFlags input-output parameter that
+     * will be bitwise ORed with one of the following Decimal128::SignalingFlag enumerators:
      * kNoFlag, kInvalid
      *
      * Note: The signaling flags for these functions only signal
@@ -376,8 +376,8 @@ public:
      * given rounding mode. The signaling flags for these functions will also signal
      * inexact computation.
      *
-     * Each function is overloaded to provide an optional signalingFlags output parameter
-     * that can be set to one of the Decimal128::SignalingFlag enumerators:
+     * Each function is overloaded to provide an optional signalingFlags input-output parameter that
+     * will be bitwise ORed with one of the following Decimal128::SignalingFlag enumerators:
      * kNoFlag, kInexact, kInvalid
      */
     std::int32_t toIntExact(RoundingMode roundMode = kRoundTiesToEven) const;
@@ -394,8 +394,8 @@ public:
      * These functions convert decimals to doubles and have the ability to signal
      * inexact, underflow, overflow, and invalid operation.
      *
-     * This function is overloaded to provide an optional signalingFlags output parameter
-     * that can be set to one of the Decimal128::SignalingFlag enumerators:
+     * Each function is overloaded to provide an optional signalingFlags input-output parameter that
+     * will be bitwise ORed with one of the following Decimal128::SignalingFlag enumerators:
      * kNoFlag, kInexact, kUnderflow, kOverflow, kInvalid
      */
     double toDouble(RoundingMode roundMode = kRoundTiesToEven) const;
@@ -443,8 +443,8 @@ public:
      * is performed using the supplied rounding mode (defaulting to kRoundTiesToEven).
      * NaNs and infinities are handled according to the IEEE 754-2008 specification.
      *
-     * Each function is overloaded to provide an optional signalingFlags output parameter
-     * that can be set to one of the Decimal128::SignalingFlag enumerators:
+     * Each function is overloaded to provide an optional signalingFlags input-output parameter that
+     * will be bitwise ORed with one of the following Decimal128::SignalingFlag enumerators:
      * kNoFlag, kInexact, kUnderflow, kOverflow, kInvalid
      *
      * The divide operation may also set signalingFlags to kDivideByZero