SERVER-40383 Handle week, day edge cases in timelib_date_from_isodate

3 files changed, 122 insertions, 13 deletions

diff --git a/src/mongo/db/pipeline/expression.cpp b/src/mongo/db/pipeline/expression.cpp
index 8b641f35fda..5958b5d931e 100644
--- a/src/mongo/db/pipeline/expression.cpp
+++ b/src/mongo/db/pipeline/expression.cpp
@@ -1204,7 +1204,7 @@ Value ExpressionDateFromParts::serialize(bool explain) const {
 }
 
 bool ExpressionDateFromParts::evaluateNumberWithDefault(const Document& root,
-                                                        intrusive_ptr<Expression> field,
+                                                        const Expression* field,
                                                         StringData fieldName,
                                                         long long defaultValue,
                                                         long long* returnValue,
@@ -1232,14 +1232,39 @@ bool ExpressionDateFromParts::evaluateNumberWithDefault(const Document& root,
     return true;
 }
 
+bool ExpressionDateFromParts::evaluateNumberWithDefaultAndBounds(const Document& root,
+                                                                 const Expression* field,
+                                                                 StringData fieldName,
+                                                                 long long defaultValue,
+                                                                 long long* returnValue,
+                                                                 Variables* variables) const {
+    bool result =
+        evaluateNumberWithDefault(root, field, fieldName, defaultValue, returnValue, variables);
+
+    uassert(31034,
+            str::stream() << "'" << fieldName << "'"
+                          << " must evaluate to a value in the range ["
+                          << kMinValueForDatePart
+                          << ", "
+                          << kMaxValueForDatePart
+                          << "]; value "
+                          << *returnValue
+                          << " is not in range",
+            !result ||
+                (*returnValue >= kMinValueForDatePart && *returnValue <= kMaxValueForDatePart));
+
+    return result;
+}
+
 Value ExpressionDateFromParts::evaluate(const Document& root, Variables* variables) const {
     long long hour, minute, second, millisecond;
 
-    if (!evaluateNumberWithDefault(root, _hour, "hour"_sd, 0, &hour, variables) ||
-        !evaluateNumberWithDefault(root, _minute, "minute"_sd, 0, &minute, variables) ||
-        !evaluateNumberWithDefault(root, _second, "second"_sd, 0, &second, variables) ||
+    if (!evaluateNumberWithDefaultAndBounds(root, _hour.get(), "hour"_sd, 0, &hour, variables) ||
+        !evaluateNumberWithDefaultAndBounds(
+            root, _minute.get(), "minute"_sd, 0, &minute, variables) ||
+        !evaluateNumberWithDefault(root, _second.get(), "second"_sd, 0, &second, variables) ||
         !evaluateNumberWithDefault(
-            root, _millisecond, "millisecond"_sd, 0, &millisecond, variables)) {
+            root, _millisecond.get(), "millisecond"_sd, 0, &millisecond, variables)) {
         // One of the evaluated inputs in nullish.
         return Value(BSONNULL);
     }
@@ -1254,9 +1279,10 @@ Value ExpressionDateFromParts::evaluate(const Document& root, Variables* variabl
     if (_year) {
         long long year, month, day;
 
-        if (!evaluateNumberWithDefault(root, _year, "year"_sd, 1970, &year, variables) ||
-            !evaluateNumberWithDefault(root, _month, "month"_sd, 1, &month, variables) ||
-            !evaluateNumberWithDefault(root, _day, "day"_sd, 1, &day, variables)) {
+        if (!evaluateNumberWithDefault(root, _year.get(), "year"_sd, 1970, &year, variables) ||
+            !evaluateNumberWithDefaultAndBounds(
+                root, _month.get(), "month"_sd, 1, &month, variables) ||
+            !evaluateNumberWithDefaultAndBounds(root, _day.get(), "day"_sd, 1, &day, variables)) {
             // One of the evaluated inputs in nullish.
             return Value(BSONNULL);
         }
@@ -1276,14 +1302,23 @@ Value ExpressionDateFromParts::evaluate(const Document& root, Variables* variabl
         long long isoWeekYear, isoWeek, isoDayOfWeek;
 
         if (!evaluateNumberWithDefault(
-                root, _isoWeekYear, "isoWeekYear"_sd, 1970, &isoWeekYear, variables) ||
-            !evaluateNumberWithDefault(root, _isoWeek, "isoWeek"_sd, 1, &isoWeek, variables) ||
-            !evaluateNumberWithDefault(
-                root, _isoDayOfWeek, "isoDayOfWeek"_sd, 1, &isoDayOfWeek, variables)) {
+                root, _isoWeekYear.get(), "isoWeekYear"_sd, 1970, &isoWeekYear, variables) ||
+            !evaluateNumberWithDefaultAndBounds(
+                root, _isoWeek.get(), "isoWeek"_sd, 1, &isoWeek, variables) ||
+            !evaluateNumberWithDefaultAndBounds(
+                root, _isoDayOfWeek.get(), "isoDayOfWeek"_sd, 1, &isoDayOfWeek, variables)) {
             // One of the evaluated inputs in nullish.
             return Value(BSONNULL);
         }
 
+        uassert(31095,
+                str::stream() << "'isoWeekYear' must evaluate to an integer in the range " << 0
+                              << " to "
+                              << 9999
+                              << ", found "
+                              << isoWeekYear,
+                isoWeekYear >= 0 && isoWeekYear <= 9999);
+
         return Value(timeZone->createFromIso8601DateParts(
             isoWeekYear, isoWeek, isoDayOfWeek, hour, minute, second, millisecond));
     }
diff --git a/src/mongo/db/pipeline/expression.h b/src/mongo/db/pipeline/expression.h
index d8843cf3c1e..9b54e5e7995 100644
--- a/src/mongo/db/pipeline/expression.h
+++ b/src/mongo/db/pipeline/expression.h
@@ -1100,12 +1100,24 @@ private:
      *   out parameter, and the function returns true.
      */
     bool evaluateNumberWithDefault(const Document& root,
-                                   boost::intrusive_ptr<Expression> field,
+                                   const Expression* field,
                                    StringData fieldName,
                                    long long defaultValue,
                                    long long* returnValue,
                                    Variables* variables) const;
 
+    /**
+     * This function has the same behavior as evaluteNumberWithDefault(), except that it uasserts if
+     * the resulting value is not in the range defined by kMaxValueForDatePart and
+     * kMinValueForDatePart.
+     */
+    bool evaluateNumberWithDefaultAndBounds(const Document& root,
+                                            const Expression* field,
+                                            StringData fieldName,
+                                            long long defaultValue,
+                                            long long* returnValue,
+                                            Variables* variables) const;
+
     boost::intrusive_ptr<Expression>& _year;
     boost::intrusive_ptr<Expression>& _month;
     boost::intrusive_ptr<Expression>& _day;
@@ -1117,6 +1129,12 @@ private:
     boost::intrusive_ptr<Expression>& _isoWeek;
     boost::intrusive_ptr<Expression>& _isoDayOfWeek;
     boost::intrusive_ptr<Expression>& _timeZone;
+
+    // Some date conversions spend a long time iterating through date tables when dealing with large
+    // input numbers, so we place a reasonable limit on the magnitude of any argument to
+    // $dateFromParts: inputs that fit within a 16-bit int are permitted.
+    static constexpr long long kMaxValueForDatePart = std::numeric_limits<int16_t>::max();
+    static constexpr long long kMinValueForDatePart = std::numeric_limits<int16_t>::lowest();
 };
 
 class ExpressionDateToParts final : public Expression {
diff --git a/src/mongo/db/query/datetime/date_time_support_test.cpp b/src/mongo/db/query/datetime/date_time_support_test.cpp
index a56bf6e01a2..f11973aa145 100644
--- a/src/mongo/db/query/datetime/date_time_support_test.cpp
+++ b/src/mongo/db/query/datetime/date_time_support_test.cpp
@@ -1099,5 +1099,61 @@ TEST(DateFromString, EmptyFormatStringThrowsForAllInputs) {
                        ErrorCodes::ConversionFailure);
 }
 
+TEST(DayOfWeek, WeekNumber) {
+    long long year = 2019;
+    long long week = 1;
+    long long day = 1;
+    long long hour = 0;
+    long long minute = 0;
+    long long second = 0;
+    long long millisecond = 0;
+
+    Date_t baseline = kDefaultTimeZone.createFromIso8601DateParts(
+        year, week, day, hour, minute, second, millisecond);
+
+    long long weekDurationInMillis = 7 * 24 * 60 * 60 * 1000;
+
+    for (int weekIt = -10000; weekIt < 10000; weekIt++) {
+        // Calculate a date using the ISO 8601 week-numbered year method.
+        Date_t dateFromIso8601 = kDefaultTimeZone.createFromIso8601DateParts(
+            year, weekIt, day, hour, minute, second, millisecond);
+
+        // Calculate the same date by adding 'weekDurationInMillis' to 'baseline' for each week past
+        // the baseline date.
+        Date_t dateFromArithmetic = baseline + Milliseconds(weekDurationInMillis * (weekIt - 1));
+
+        // The two methods should produce the same time.
+        ASSERT_EQ(dateFromIso8601, dateFromArithmetic);
+    }
+}
+
+TEST(DayOfWeek, DayNumber) {
+    long long year = 2019;
+    long long week = 34;
+    long long day = 1;
+    long long hour = 0;
+    long long minute = 0;
+    long long second = 0;
+    long long millisecond = 0;
+
+    Date_t baseline = kDefaultTimeZone.createFromIso8601DateParts(
+        year, week, day, hour, minute, second, millisecond);
+
+    long long dayDurationInMillis = 24 * 60 * 60 * 1000;
+
+    for (int dayIt = -10000; dayIt < 10000; dayIt++) {
+        // Calculate a date using the ISO 8601 week-numbered year method.
+        Date_t dateFromIso8601 = kDefaultTimeZone.createFromIso8601DateParts(
+            year, week, dayIt, hour, minute, second, millisecond);
+
+        // Calculate the same date by adding 'dayDurationInMillis' to 'baseline' for each day past
+        // the baseline date.
+        Date_t dateFromArithmetic = baseline + Milliseconds(dayDurationInMillis * (dayIt - 1));
+
+        // The two methods should produce the same time.
+        ASSERT_EQ(dateFromIso8601, dateFromArithmetic);
+    }
+}
+
 }  // namespace
 }  // namespace mongo