Revert "SERVER-67282 $add not to use DoubleDoubleSum in SBE"

This reverts commit 1284566dfda0a21a6da9451742967a8f67f586d6.

2 files changed, 56 insertions, 85 deletions

diff --git a/src/mongo/db/exec/sbe/values/arith_common.cpp b/src/mongo/db/exec/sbe/values/arith_common.cpp
index 795dec4ea7d..04c640d086f 100644
--- a/src/mongo/db/exec/sbe/values/arith_common.cpp
+++ b/src/mongo/db/exec/sbe/values/arith_common.cpp
@@ -165,73 +165,15 @@ std::tuple<bool, value::TypeTags, value::Value> genericArithmeticOp(value::TypeT
     } else if (lhsTag == TypeTags::Date || rhsTag == TypeTags::Date) {
         if (isNumber(lhsTag)) {
             int64_t result;
-            switch (lhsTag) {
-                case TypeTags::NumberDouble: {
-                    using limits = std::numeric_limits<int64_t>;
-                    double doubleLhs = numericCast<double>(lhsTag, lhsValue);
-                    // The upper bound is exclusive because it rounds up when it is cast to a
-                    // double.
-                    if (doubleLhs >= static_cast<double>(limits::min()) &&
-                        doubleLhs < static_cast<double>(limits::max()) &&
-                        !Op::template doOperation<int64_t>(
-                            llround(doubleLhs), bitcastTo<int64_t>(rhsValue), result)) {
-                        return {false, value::TypeTags::Date, value::bitcastFrom<int64_t>(result)};
-                    }
-                    break;
-                }
-                case TypeTags::NumberDecimal: {
-                    using limits = std::numeric_limits<int64_t>;
-                    auto decimalLhs = numericCast<Decimal128>(lhsTag, lhsValue);
-                    if (decimalLhs.isGreaterEqual(Decimal128{limits::min()}) &&
-                        decimalLhs.isLess(Decimal128{limits::max()}) &&
-                        !Op::doOperation(
-                            decimalLhs.toLong(), bitcastTo<int64_t>(rhsValue), result)) {
-                        return {false, value::TypeTags::Date, value::bitcastFrom<int64_t>(result)};
-                    }
-                    break;
-                }
-                default: {
-                    if (!Op::doOperation(numericCast<int64_t>(lhsTag, lhsValue),
-                                         bitcastTo<int64_t>(rhsValue),
-                                         result)) {
-                        return {false, value::TypeTags::Date, value::bitcastFrom<int64_t>(result)};
-                    }
-                }
+            if (!Op::doOperation(
+                    numericCast<int64_t>(lhsTag, lhsValue), bitcastTo<int64_t>(rhsValue), result)) {
+                return {false, value::TypeTags::Date, value::bitcastFrom<int64_t>(result)};
             }
         } else if (isNumber(rhsTag)) {
             int64_t result;
-            switch (rhsTag) {
-                case TypeTags::NumberDouble: {
-                    using limits = std::numeric_limits<int64_t>;
-                    double doubleRhs = numericCast<double>(rhsTag, rhsValue);
-                    // The upper bound is exclusive because it rounds up when it is cast to a
-                    // double.
-                    if (doubleRhs >= static_cast<double>(limits::min()) &&
-                        doubleRhs < static_cast<double>(limits::max()) &&
-                        !Op::template doOperation<int64_t>(
-                            bitcastTo<int64_t>(lhsValue), llround(doubleRhs), result)) {
-                        return {false, value::TypeTags::Date, value::bitcastFrom<int64_t>(result)};
-                    }
-                    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)) {
-                        return {false, value::TypeTags::Date, value::bitcastFrom<int64_t>(result)};
-                    }
-                    break;
-                }
-                default: {
-                    if (!Op::doOperation(bitcastTo<int64_t>(lhsValue),
-                                         numericCast<int64_t>(rhsTag, rhsValue),
-                                         result)) {
-                        return {false, value::TypeTags::Date, value::bitcastFrom<int64_t>(result)};
-                    }
-                }
+            if (!Op::doOperation(
+                    bitcastTo<int64_t>(lhsValue), numericCast<int64_t>(rhsTag, rhsValue), result)) {
+                return {false, value::TypeTags::Date, value::bitcastFrom<int64_t>(result)};
             }
         } else {
             int64_t result;
diff --git a/src/mongo/db/query/sbe_stage_builder_expression.cpp b/src/mongo/db/query/sbe_stage_builder_expression.cpp
index 8ec8a79e1b1..cbeac015678 100644
--- a/src/mongo/db/query/sbe_stage_builder_expression.cpp
+++ b/src/mongo/db/query/sbe_stage_builder_expression.cpp
@@ -796,6 +796,48 @@ public:
         if (arity == 0) {
             // Return a zero constant if the expression has no operand children.
             _context->pushExpr(makeConstant(sbe::value::TypeTags::NumberInt32, 0));
+        } else if (arity == 2) {
+            auto rhs = _context->popExpr();
+            auto lhs = _context->popExpr();
+            auto binds = sbe::makeEs(std::move(lhs), std::move(rhs));
+            sbe::EVariable lhsVar{frameId, 0};
+            sbe::EVariable rhsVar{frameId, 1};
+
+            auto addExpr = makeLocalBind(
+                _context->state.frameIdGenerator,
+                [&](sbe::EVariable lhsIsDate, sbe::EVariable rhsIsDate) {
+                    return buildMultiBranchConditional(
+                        CaseValuePair{makeBinaryOp(sbe::EPrimBinary::logicOr,
+                                                   generateNullOrMissing(frameId, 0),
+                                                   generateNullOrMissing(frameId, 1)),
+                                      sbe::makeE<sbe::EConstant>(sbe::value::TypeTags::Null, 0)},
+                        CaseValuePair{
+                            makeBinaryOp(sbe::EPrimBinary::logicOr,
+                                         generateNotNumberOrDate(0),
+                                         generateNotNumberOrDate(1)),
+                            sbe::makeE<sbe::EFail>(
+                                ErrorCodes::Error{4974201},
+                                "only numbers and dates are allowed in an $add expression")},
+                        CaseValuePair{
+                            makeBinaryOp(
+                                sbe::EPrimBinary::logicAnd, lhsIsDate.clone(), rhsIsDate.clone()),
+                            sbe::makeE<sbe::EFail>(ErrorCodes::Error{4974202},
+                                                   "only one date allowed in an $add expression")},
+                        // An EPrimBinary::add expression, which compiles directly into an "add"
+                        // instruction, efficiently handles the general case for for $add with
+                        // exactly two operands, but when one of the operands is a date, we need to
+                        // use the "doubleDoubleSum" function to perform the required conversions.
+                        CaseValuePair{
+                            makeBinaryOp(
+                                sbe::EPrimBinary::logicOr, lhsIsDate.clone(), rhsIsDate.clone()),
+                            makeFunction("doubleDoubleSum", lhsVar.clone(), rhsVar.clone())},
+                        makeBinaryOp(sbe::EPrimBinary::add, lhsVar.clone(), rhsVar.clone()));
+                },
+                makeFunction("isDate", lhsVar.clone()),
+                makeFunction("isDate", rhsVar.clone()));
+
+            _context->pushExpr(
+                sbe::makeE<sbe::ELocalBind>(frameId, std::move(binds), std::move(addExpr)));
         } else {
             sbe::EExpression::Vector binds;
             sbe::EExpression::Vector argVars;
@@ -834,28 +876,15 @@ public:
                 [](auto&& acc, auto&& ex) {
                     return makeBinaryOp(sbe::EPrimBinary::logicOr, std::move(acc), std::move(ex));
                 });
-            auto addOp = std::move(argVars[0]);
-            for (size_t idx = 1; idx < arity; ++idx) {
-                addOp = makeLocalBind(
-                    _context->state.frameIdGenerator,
-                    [&](sbe::EVariable var1, sbe::EVariable var2) {
-                        return sbe::makeE<sbe::EIf>(
-                            makeBinaryOp(sbe::EPrimBinary::logicAnd,
-                                         makeFunction("isDate", var1.clone()),
-                                         makeFunction("isDate", var2.clone())),
-                            makeFail(4974202, "only one date allowed in an $add expression"),
-                            makeBinaryOp(sbe::EPrimBinary::add, var1.clone(), var2.clone()));
-                    },
-                    std::move(addOp),
-                    std::move(argVars[idx]));
-            }
-            auto addExpr = buildMultiBranchConditional(
-                CaseValuePair{std::move(checkNullAllArguments),
-                              makeConstant(sbe::value::TypeTags::Null, 0)},
-                CaseValuePair{
+            auto addExpr = sbe::makeE<sbe::EIf>(
+                std::move(checkNullAllArguments),
+                sbe::makeE<sbe::EConstant>(sbe::value::TypeTags::Null, 0),
+                sbe::makeE<sbe::EIf>(
                     std::move(checkNotNumberOrDateAllArguments),
-                    makeFail(4974201, "only numbers and dates are allowed in an $add expression")},
-                std::move(addOp));
+                    sbe::makeE<sbe::EFail>(
+                        ErrorCodes::Error{4974203},
+                        "only numbers and dates are allowed in an $add expression"),
+                    sbe::makeE<sbe::EFunction>("doubleDoubleSum", std::move(argVars))));
             _context->pushExpr(
                 sbe::makeE<sbe::ELocalBind>(frameId, std::move(binds), std::move(addExpr)));
         }