From 901ab9acbfbfdb7e5e62ab27406b43efe3646ad9 Mon Sep 17 00:00:00 2001 From: "Ma, Rong" Date: Tue, 5 Mar 2024 18:01:51 +0800 Subject: [PATCH] unify API --- velox/functions/prestosql/DateTimeFunctions.h | 27 ++++++++- velox/functions/sparksql/DateTimeFunctions.h | 26 +++++++-- velox/type/TimestampConversion.cpp | 14 +---- velox/type/TimestampConversion.h | 9 +-- velox/type/tests/TimestampConversionTest.cpp | 58 ++++++++++++------- 5 files changed, 87 insertions(+), 47 deletions(-) diff --git a/velox/functions/prestosql/DateTimeFunctions.h b/velox/functions/prestosql/DateTimeFunctions.h index 160fb3ef52a37..6ba2768b44fc7 100644 --- a/velox/functions/prestosql/DateTimeFunctions.h +++ b/velox/functions/prestosql/DateTimeFunctions.h @@ -339,14 +339,28 @@ struct LastDayOfMonthFunction : public InitSessionTimezone, out_type& result, const arg_type& timestamp) { auto dt = getDateTime(timestamp, this->timeZone_); - result = util::lastDayOfMonthSinceEpochFromDate(dt); + int64_t daysSinceEpochFromDate; + auto status = + util::lastDayOfMonthSinceEpochFromDate(dt, daysSinceEpochFromDate); + if (!status.ok()) { + VELOX_DCHECK(status.isUserError()); + VELOX_USER_FAIL(status.message()); + } + result = daysSinceEpochFromDate; } FOLLY_ALWAYS_INLINE void call( out_type& result, const arg_type& date) { auto dt = getDateTime(date); - result = util::lastDayOfMonthSinceEpochFromDate(dt); + int64_t daysSinceEpochFromDate; + auto status = + util::lastDayOfMonthSinceEpochFromDate(dt, daysSinceEpochFromDate); + if (!status.ok()) { + VELOX_DCHECK(status.isUserError()); + VELOX_USER_FAIL(status.message()); + } + result = daysSinceEpochFromDate; } FOLLY_ALWAYS_INLINE void call( @@ -354,7 +368,14 @@ struct LastDayOfMonthFunction : public InitSessionTimezone, const arg_type& timestampWithTimezone) { auto timestamp = this->toTimestamp(timestampWithTimezone); auto dt = getDateTime(timestamp, nullptr); - result = util::lastDayOfMonthSinceEpochFromDate(dt); + int64_t daysSinceEpochFromDate; + auto status = + util::lastDayOfMonthSinceEpochFromDate(dt, daysSinceEpochFromDate); + if (!status.ok()) { + VELOX_DCHECK(status.isUserError()); + VELOX_USER_FAIL(status.message()); + } + result = daysSinceEpochFromDate; } }; diff --git a/velox/functions/sparksql/DateTimeFunctions.h b/velox/functions/sparksql/DateTimeFunctions.h index eb5c321a254f5..6db3371bf9170 100644 --- a/velox/functions/sparksql/DateTimeFunctions.h +++ b/velox/functions/sparksql/DateTimeFunctions.h @@ -312,7 +312,13 @@ struct MakeDateFunction { const int32_t year, const int32_t month, const int32_t day) { - auto daysSinceEpoch = util::daysSinceEpochFromDate(year, month, day); + int64_t daysSinceEpoch; + auto status = + util::daysSinceEpochFromDate(year, month, day, daysSinceEpoch); + if (!status.ok()) { + VELOX_DCHECK(status.isUserError()); + VELOX_USER_FAIL(status.message()); + } VELOX_USER_CHECK_EQ( daysSinceEpoch, (int32_t)daysSinceEpoch, @@ -348,7 +354,13 @@ struct LastDayFunction { int32_t month = getMonth(dateTime); int32_t day = getMonth(dateTime); auto lastDay = util::getMaxDayOfMonth(year, month); - auto daysSinceEpoch = util::daysSinceEpochFromDate(year, month, lastDay); + int64_t daysSinceEpoch; + auto status = + util::daysSinceEpochFromDate(year, month, lastDay, daysSinceEpoch); + if (!status.ok()) { + VELOX_DCHECK(status.isUserError()); + VELOX_USER_FAIL(status.message()); + } VELOX_USER_CHECK_EQ( daysSinceEpoch, (int32_t)daysSinceEpoch, @@ -451,8 +463,14 @@ struct AddMonthsFunction { auto lastDayOfMonth = util::getMaxDayOfMonth(yearResult, monthResult); // Adjusts day to valid one. auto dayResult = lastDayOfMonth < day ? lastDayOfMonth : day; - auto daysSinceEpoch = - util::daysSinceEpochFromDate(yearResult, monthResult, dayResult); + + int64_t daysSinceEpoch; + auto status = util::daysSinceEpochFromDate( + yearResult, monthResult, dayResult, daysSinceEpoch); + if (!status.ok()) { + VELOX_DCHECK(status.isUserError()); + VELOX_USER_FAIL(status.message()); + } VELOX_USER_CHECK_EQ( daysSinceEpoch, (int32_t)daysSinceEpoch, diff --git a/velox/type/TimestampConversion.cpp b/velox/type/TimestampConversion.cpp index 8a07ebaaa74a8..b414a9d8d63cf 100644 --- a/velox/type/TimestampConversion.cpp +++ b/velox/type/TimestampConversion.cpp @@ -523,27 +523,17 @@ bool isValidDayOfYear(int32_t year, int32_t dayOfYear) { return true; } -int64_t lastDayOfMonthSinceEpochFromDate(const std::tm& dateTime) { +Status lastDayOfMonthSinceEpochFromDate(const std::tm& dateTime, int64_t& out) { auto year = dateTime.tm_year + 1900; auto month = dateTime.tm_mon + 1; auto day = util::getMaxDayOfMonth(year, month); - return util::daysSinceEpochFromDate(year, month, day); + return util::daysSinceEpochFromDate(year, month, day, out); } int32_t getMaxDayOfMonth(int32_t year, int32_t month) { return isLeapYear(year) ? kLeapDays[month] : kNormalDays[month]; } -int64_t daysSinceEpochFromDate(int32_t year, int32_t month, int32_t day) { - int64_t daysSinceEpoch; - auto status = daysSinceEpochFromDate(year, month, day, daysSinceEpoch); - if (!status.ok()) { - VELOX_DCHECK(status.isUserError()); - VELOX_USER_FAIL(status.message()); - } - return daysSinceEpoch; -} - Status daysSinceEpochFromDate(int32_t year, int32_t month, int32_t day, int64_t& out) { int64_t daysSinceEpoch = 0; diff --git a/velox/type/TimestampConversion.h b/velox/type/TimestampConversion.h index 5e0ead6c417df..3213900ef42c2 100644 --- a/velox/type/TimestampConversion.h +++ b/velox/type/TimestampConversion.h @@ -56,15 +56,12 @@ bool isValidDayOfYear(int32_t year, int32_t dayOfYear); // Returns max day of month for inputted month of inputted year int32_t getMaxDayOfMonth(int32_t year, int32_t month); -// Returns last day of month since unix epoch (1970-01-01). -int64_t lastDayOfMonthSinceEpochFromDate(const std::tm& dateTime); +/// Computes the last day of month since unix epoch (1970-01-01). +/// Returns UserError status if the date is invalid. +Status lastDayOfMonthSinceEpochFromDate(const std::tm& dateTime, int64_t& out); /// Date conversions. -/// Returns the (signed) number of days since unix epoch (1970-01-01). -/// Throws VeloxUserError if the date is invalid. -int64_t daysSinceEpochFromDate(int32_t year, int32_t month, int32_t day); - /// Computes the (signed) number of days since unix epoch (1970-01-01). /// Returns UserError status if the date is invalid. Status diff --git a/velox/type/tests/TimestampConversionTest.cpp b/velox/type/tests/TimestampConversionTest.cpp index 304b625390b4c..619a57564fc34 100644 --- a/velox/type/tests/TimestampConversionTest.cpp +++ b/velox/type/tests/TimestampConversionTest.cpp @@ -26,35 +26,49 @@ namespace facebook::velox::util { namespace { TEST(DateTimeUtilTest, fromDate) { - EXPECT_EQ(0, daysSinceEpochFromDate(1970, 1, 1)); - EXPECT_EQ(1, daysSinceEpochFromDate(1970, 1, 2)); - EXPECT_EQ(365, daysSinceEpochFromDate(1971, 1, 1)); - EXPECT_EQ(730, daysSinceEpochFromDate(1972, 1, 1)); // leap year. - EXPECT_EQ(1096, daysSinceEpochFromDate(1973, 1, 1)); - - EXPECT_EQ(10957, daysSinceEpochFromDate(2000, 1, 1)); - EXPECT_EQ(18474, daysSinceEpochFromDate(2020, 7, 31)); + auto testDaysSinceEpochFromDate = + [](int32_t year, int32_t month, int32_t day) { + int64_t daysSinceEpoch; + auto status = + util::daysSinceEpochFromDate(year, month, day, daysSinceEpoch); + EXPECT_TRUE(status.ok()); + return daysSinceEpoch; + }; + EXPECT_EQ(0, testDaysSinceEpochFromDate(1970, 1, 1)); + EXPECT_EQ(1, testDaysSinceEpochFromDate(1970, 1, 2)); + EXPECT_EQ(365, testDaysSinceEpochFromDate(1971, 1, 1)); + EXPECT_EQ(730, testDaysSinceEpochFromDate(1972, 1, 1)); // leap year. + EXPECT_EQ(1096, testDaysSinceEpochFromDate(1973, 1, 1)); + + EXPECT_EQ(10957, testDaysSinceEpochFromDate(2000, 1, 1)); + EXPECT_EQ(18474, testDaysSinceEpochFromDate(2020, 7, 31)); // Before unix epoch. - EXPECT_EQ(-1, daysSinceEpochFromDate(1969, 12, 31)); - EXPECT_EQ(-365, daysSinceEpochFromDate(1969, 1, 1)); - EXPECT_EQ(-731, daysSinceEpochFromDate(1968, 1, 1)); // leap year. - EXPECT_EQ(-719528, daysSinceEpochFromDate(0, 1, 1)); + EXPECT_EQ(-1, testDaysSinceEpochFromDate(1969, 12, 31)); + EXPECT_EQ(-365, testDaysSinceEpochFromDate(1969, 1, 1)); + EXPECT_EQ(-731, testDaysSinceEpochFromDate(1968, 1, 1)); // leap year. + EXPECT_EQ(-719528, testDaysSinceEpochFromDate(0, 1, 1)); // Negative year - BC. - EXPECT_EQ(-719529, daysSinceEpochFromDate(-1, 12, 31)); - EXPECT_EQ(-719893, daysSinceEpochFromDate(-1, 1, 1)); + EXPECT_EQ(-719529, testDaysSinceEpochFromDate(-1, 12, 31)); + EXPECT_EQ(-719893, testDaysSinceEpochFromDate(-1, 1, 1)); } TEST(DateTimeUtilTest, fromDateInvalid) { - EXPECT_THROW(daysSinceEpochFromDate(1970, 1, -1), VeloxUserError); - EXPECT_THROW(daysSinceEpochFromDate(1970, -1, 1), VeloxUserError); - EXPECT_THROW(daysSinceEpochFromDate(1970, 0, 1), VeloxUserError); - EXPECT_THROW(daysSinceEpochFromDate(1970, 13, 1), VeloxUserError); - EXPECT_THROW(daysSinceEpochFromDate(1970, 1, 32), VeloxUserError); - EXPECT_THROW( - daysSinceEpochFromDate(1970, 2, 29), VeloxUserError); // non-leap. - EXPECT_THROW(daysSinceEpochFromDate(1970, 6, 31), VeloxUserError); + auto testDaysSinceEpochFromDateInvalid = + [](int32_t year, int32_t month, int32_t day) { + int64_t daysSinceEpoch; + auto status = + util::daysSinceEpochFromDate(year, month, day, daysSinceEpoch); + EXPECT_TRUE(status.isUserError()); + }; + EXPECT_NO_THROW(testDaysSinceEpochFromDateInvalid(1970, 1, -1)); + EXPECT_NO_THROW(testDaysSinceEpochFromDateInvalid(1970, -1, 1)); + EXPECT_NO_THROW(testDaysSinceEpochFromDateInvalid(1970, 0, 1)); + EXPECT_NO_THROW(testDaysSinceEpochFromDateInvalid(1970, 13, 1)); + EXPECT_NO_THROW(testDaysSinceEpochFromDateInvalid(1970, 1, 32)); + EXPECT_NO_THROW(testDaysSinceEpochFromDateInvalid(1970, 2, 29)); // non-leap. + EXPECT_NO_THROW(testDaysSinceEpochFromDateInvalid(1970, 6, 31)); } TEST(DateTimeUtilTest, fromDateString) {