diff options
author | Justin Seyster <justin.seyster@mongodb.com> | 2019-06-12 15:30:30 -0400 |
---|---|---|
committer | Justin Seyster <justin.seyster@mongodb.com> | 2019-07-01 16:30:19 -0400 |
commit | 40543b7be5307541f4cab8df5bee73ffa65b5a1c (patch) | |
tree | ac38fe0135cda43f4b6924938718ac7ea9e8327d | |
parent | 23e5403a2e4728acc1125b1919aca54553f897da (diff) | |
download | mongo-40543b7be5307541f4cab8df5bee73ffa65b5a1c.tar.gz |
SERVER-40383 Handle week, day edge cases in timelib_date_from_isodate
(cherry picked from commit d4843fc49931c7ce4332dc373623267c293eb518)
-rw-r--r-- | jstests/aggregation/expressions/date_from_parts.js | 125 | ||||
-rw-r--r-- | src/mongo/db/pipeline/expression.cpp | 61 | ||||
-rw-r--r-- | src/mongo/db/pipeline/expression.h | 20 | ||||
-rw-r--r-- | src/mongo/db/query/datetime/date_time_support_test.cpp | 56 | ||||
-rw-r--r-- | src/third_party/timelib-2018.01alpha1/dow.c | 55 |
5 files changed, 258 insertions, 59 deletions
diff --git a/jstests/aggregation/expressions/date_from_parts.js b/jstests/aggregation/expressions/date_from_parts.js index aae6bf2343f..cd16fe6af49 100644 --- a/jstests/aggregation/expressions/date_from_parts.js +++ b/jstests/aggregation/expressions/date_from_parts.js @@ -9,7 +9,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE /* Basic Sanity Checks */ coll.drop(); - assert.writeOK(coll.insert([ + assert.commandWorked(coll.insert([ {_id: 0, year: 2017, month: 6, day: 19, hour: 15, minute: 13, second: 25, millisecond: 713}, { _id: 1, @@ -105,7 +105,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE coll.drop(); - assert.writeOK(coll.insert([ + assert.commandWorked(coll.insert([ { _id: 0, year: 2017, @@ -389,7 +389,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE coll.drop(); - assert.writeOK(coll.insert([ + assert.commandWorked(coll.insert([ {_id: 0}, ])); @@ -421,7 +421,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE coll.drop(); - assert.writeOK(coll.insert([ + assert.commandWorked(coll.insert([ {_id: 0, falseValue: false}, ])); @@ -453,7 +453,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE coll.drop(); - assert.writeOK(coll.insert([ + assert.commandWorked(coll.insert([ {_id: 0, outOfRangeValue: 10002}, ])); @@ -472,7 +472,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE coll.drop(); - assert.writeOK(coll.insert([{ + assert.commandWorked(coll.insert([{ _id: 0, minusOne: -1, zero: 0, @@ -548,7 +548,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE */ coll.drop(); - assert.writeOK(coll.insert([{ + assert.commandWorked(coll.insert([{ _id: 0, veryBigDoubleA: 18014398509481984.0, veryBigDecimal128A: NumberDecimal("9223372036854775807"), // 2^63-1 @@ -581,11 +581,112 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE assertErrMsgContains(coll, pipeline, 40515, "'millisecond' must evaluate to an integer"); /* --------------------------------------------------------------------------------------- */ + /* Testing that year values are only allowed in the range [0, 9999] and that month, day, hour, + * and minute values are only allowed in the range [-32,768, 32,767]. */ + coll.drop(); + + assert.commandWorked(coll.insert([{ + _id: 0, + bigYear: 10000, + smallYear: -1, + prettyBigInt: 32768, + prettyBigNegativeInt: -32769 + }])); + + pipeline = [{$project: {date: {"$dateFromParts": {year: "$bigYear"}}}}]; + assertErrMsgContains( + coll, pipeline, 40523, "'year' must evaluate to an integer in the range 0 to 9999"); + + pipeline = [{$project: {date: {"$dateFromParts": {year: "$smallYear"}}}}]; + assertErrMsgContains( + coll, pipeline, 40523, "'year' must evaluate to an integer in the range 0 to 9999"); + + pipeline = [{$project: {date: {"$dateFromParts": {year: 1970, month: "$prettyBigInt"}}}}]; + assertErrMsgContains( + coll, pipeline, 31034, "'month' must evaluate to a value in the range [-32768, 32767]"); + + pipeline = + [{$project: {date: {"$dateFromParts": {year: 1970, month: "$prettyBigNegativeInt"}}}}]; + assertErrMsgContains( + coll, pipeline, 31034, "'month' must evaluate to a value in the range [-32768, 32767]"); + + pipeline = + [{$project: {date: {"$dateFromParts": {year: 1970, month: 1, day: "$prettyBigInt"}}}}]; + assertErrMsgContains( + coll, pipeline, 31034, "'day' must evaluate to a value in the range [-32768, 32767]"); + + pipeline = [{ + $project: + {date: {"$dateFromParts": {year: 1970, month: 1, day: "$prettyBigNegativeInt"}}} + }]; + assertErrMsgContains( + coll, pipeline, 31034, "'day' must evaluate to a value in the range [-32768, 32767]"); + + pipeline = [{$project: {date: {"$dateFromParts": {year: 1970, hour: "$prettyBigInt"}}}}]; + assertErrMsgContains( + coll, pipeline, 31034, "'hour' must evaluate to a value in the range [-32768, 32767]"); + + pipeline = + [{$project: {date: {"$dateFromParts": {year: 1970, hour: "$prettyBigNegativeInt"}}}}]; + assertErrMsgContains( + coll, pipeline, 31034, "'hour' must evaluate to a value in the range [-32768, 32767]"); + + pipeline = + [{$project: {date: {"$dateFromParts": {year: 1970, hour: 0, minute: "$prettyBigInt"}}}}]; + assertErrMsgContains( + coll, pipeline, 31034, "'minute' must evaluate to a value in the range [-32768, 32767]"); + + pipeline = [{ + $project: + {date: {"$dateFromParts": {year: 1970, hour: 0, minute: "$prettyBigNegativeInt"}}} + }]; + assertErrMsgContains( + coll, pipeline, 31034, "'minute' must evaluate to a value in the range [-32768, 32767]"); + + pipeline = [{$project: {date: {"$dateFromParts": {isoWeekYear: "$bigYear"}}}}]; + assertErrMsgContains( + coll, pipeline, 31095, "'isoWeekYear' must evaluate to an integer in the range 0 to 9999"); + + pipeline = [{$project: {date: {"$dateFromParts": {isoWeekYear: "$smallYear"}}}}]; + assertErrMsgContains( + coll, pipeline, 31095, "'isoWeekYear' must evaluate to an integer in the range 0 to 9999"); + + pipeline = + [{$project: {date: {"$dateFromParts": {isoWeekYear: 1970, isoWeek: "$prettyBigInt"}}}}]; + assertErrMsgContains( + coll, pipeline, 31034, "'isoWeek' must evaluate to a value in the range [-32768, 32767]"); + + pipeline = [{ + $project: + {date: {"$dateFromParts": {isoWeekYear: 1970, isoWeek: "$prettyBigNegativeInt"}}} + }]; + assertErrMsgContains( + coll, pipeline, 31034, "'isoWeek' must evaluate to a value in the range [-32768, 32767]"); + + pipeline = [ + {$project: {date: {"$dateFromParts": {isoWeekYear: 1970, isoDayOfWeek: "$prettyBigInt"}}}} + ]; + assertErrMsgContains(coll, + pipeline, + 31034, + "'isoDayOfWeek' must evaluate to a value in the range [-32768, 32767]"); + + pipeline = [{ + $project: { + date: {"$dateFromParts": {isoWeekYear: 1970, isoDayOfWeek: "$prettyBigNegativeInt"}} + } + }]; + assertErrMsgContains(coll, + pipeline, + 31034, + "'isoDayOfWeek' must evaluate to a value in the range [-32768, 32767]"); + + /* --------------------------------------------------------------------------------------- */ /* Testing wrong arguments */ coll.drop(); - assert.writeOK(coll.insert([ + assert.commandWorked(coll.insert([ {_id: 0}, ])); @@ -630,7 +731,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE coll.drop(); - assert.writeOK(coll.insert([ + assert.commandWorked(coll.insert([ {_id: 0, floatField: 2017.5, decimalField: NumberDecimal("2017.5")}, ])); @@ -653,7 +754,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE coll.drop(); - assert.writeOK(coll.insert([ + assert.commandWorked(coll.insert([ {_id: 0, year: NumberDecimal("2017"), month: 6.0, day: NumberInt(19), hour: NumberLong(15)}, { _id: 1, @@ -733,7 +834,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE coll.drop(); - assert.writeOK(coll.insert([ + assert.commandWorked(coll.insert([ { _id: 0, year: NumberDecimal("2017"), @@ -783,7 +884,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE coll.drop(); - assert.writeOK(coll.insert([ + assert.commandWorked(coll.insert([ { _id: 0, isoWeekYear: NumberDecimal("2017"), diff --git a/src/mongo/db/pipeline/expression.cpp b/src/mongo/db/pipeline/expression.cpp index afaac7c12f4..e1ef5d763fc 100644 --- a/src/mongo/db/pipeline/expression.cpp +++ b/src/mongo/db/pipeline/expression.cpp @@ -1017,6 +1017,8 @@ boost::optional<TimeZone> makeTimeZone(const TimeZoneDatabase* tzdb, } // namespace +constexpr long long ExpressionDateFromParts::kMaxValueForDatePart; +constexpr long long ExpressionDateFromParts::kMinValueForDatePart; REGISTER_EXPRESSION(dateFromParts, ExpressionDateFromParts::parse); intrusive_ptr<Expression> ExpressionDateFromParts::parse( @@ -1198,7 +1200,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, @@ -1226,14 +1228,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); } @@ -1248,9 +1275,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); } @@ -1270,14 +1298,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 4f2cfc7efc8..a997e87cfdf 100644 --- a/src/mongo/db/pipeline/expression.h +++ b/src/mongo/db/pipeline/expression.h @@ -954,12 +954,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; @@ -971,6 +983,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 7b17409825b..b8230180621 100644 --- a/src/mongo/db/query/datetime/date_time_support_test.cpp +++ b/src/mongo/db/query/datetime/date_time_support_test.cpp @@ -1100,5 +1100,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 diff --git a/src/third_party/timelib-2018.01alpha1/dow.c b/src/third_party/timelib-2018.01alpha1/dow.c index 33553e7806f..0ae02178b72 100644 --- a/src/third_party/timelib-2018.01alpha1/dow.c +++ b/src/third_party/timelib-2018.01alpha1/dow.c @@ -144,7 +144,7 @@ void timelib_isodate_from_date(timelib_sll y, timelib_sll m, timelib_sll d, time *id = timelib_day_of_week_ex(y, m, d, 1); } -static timelib_sll timelib_daynr_from_weeknr_ex(timelib_sll iy, timelib_sll iw, timelib_sll id, timelib_sll *y) +timelib_sll timelib_daynr_from_weeknr(timelib_sll iy, timelib_sll iw, timelib_sll id) { timelib_sll dow, day; @@ -152,54 +152,41 @@ static timelib_sll timelib_daynr_from_weeknr_ex(timelib_sll iy, timelib_sll iw, dow = timelib_day_of_week(iy, 1, 1); /* then use that to figure out the offset for day 1 of week 1 */ day = 0 - (dow > 4 ? dow - 7 : dow); - /* and adjust the year to the natural year if we need to */ - *y = (iw == 1 && day < 0 && id < dow) ? iy - 1 : iy; /* Add weeks and days */ return day + ((iw - 1) * 7) + id; } -timelib_sll timelib_daynr_from_weeknr(timelib_sll iy, timelib_sll iw, timelib_sll id) -{ - timelib_sll dummy_iso_year; - - return timelib_daynr_from_weeknr_ex(iy, iw, id, &dummy_iso_year); -} - void timelib_date_from_isodate(timelib_sll iy, timelib_sll iw, timelib_sll id, timelib_sll *y, timelib_sll *m, timelib_sll *d) { - timelib_sll daynr = timelib_daynr_from_weeknr_ex(iy, iw, id, y) + 1; + timelib_sll daynr = timelib_daynr_from_weeknr(iy, iw, id) + 1; int *table; + bool is_leap_year; - *m = 0; + // Invariant: is_leap_year == timelib_is_leap(*y) + *y = iy; + is_leap_year = timelib_is_leap(*y); - if (daynr <= 0) { - *y += 1; + // Establish invariant that daynr >= 0 + while (daynr <= 0) { + *y -= 1; + daynr += (is_leap_year = timelib_is_leap(*y)) ? 366 : 365; } - if (timelib_is_leap(*y)) { - table = ml_table_leap; - if (daynr > 366) { - *y += 1; - daynr -= 366; - } - } else { - table = ml_table_common; - if (daynr > 365) { - *y += 1; - daynr -= 365; - } + // Establish invariant that daynr <= number of days in *yr + while (daynr > (is_leap_year ? 366 : 365)) { + daynr -= is_leap_year ? 366 : 365; + *y += 1; + is_leap_year = timelib_is_leap(*y); } - do { - daynr -= table[*m]; - (*m)++; - } while (daynr > table[*m]); + table = is_leap_year ? ml_table_leap : ml_table_common; - if (daynr <= 0) { - daynr += 31; - *y -= 1; - *m = 12; + // Establish invariant that daynr <= number of days in *m + *m = 1; + while (daynr > table[*m]) { + daynr -= table[*m]; + *m += 1; } *d = daynr; |