diff options
-rw-r--r-- | jstests/aggregation/add_with_date.js | 44 | ||||
-rw-r--r-- | src/mongo/db/exec/sbe/values/arith_common.cpp | 10 | ||||
-rw-r--r-- | src/mongo/db/pipeline/expression.cpp | 16 | ||||
-rw-r--r-- | src/mongo/platform/decimal128.h | 16 |
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 |