From 19da845352be0522d068b31b722a031ccf385072 Mon Sep 17 00:00:00 2001 From: Daniel Hunte Date: Tue, 15 Oct 2024 15:57:06 -0700 Subject: [PATCH] Final clean up of DateTimeStampFunctionsTest.cpp (#11266) Summary: Pull Request resolved: https://github.com/facebookincubator/velox/pull/11266 Velox functions now support the logical type TimeStampWithTimeZone. The use of "evaluateWithTimestampWithTimezone" is therefore no longer needed. Reviewed By: kevinwilfong Differential Revision: D64413816 fbshipit-source-id: 2648f2052b2d6b96e5d151510378335dc7158e1b --- .../prestosql/tests/DateTimeFunctionsTest.cpp | 103 ++++++++---------- 1 file changed, 44 insertions(+), 59 deletions(-) diff --git a/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp b/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp index 4dbc6ff744a6..d7b4165c33c6 100644 --- a/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp +++ b/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp @@ -123,41 +123,6 @@ class DateTimeFunctionsTest : public functions::test::FunctionBaseTest { } }; - template - std::optional evaluateWithTimestampWithTimezone( - const std::string& expression, - std::optional timestamp, - const std::optional& timeZoneName) { - if (!timestamp.has_value() || !timeZoneName.has_value()) { - return evaluateOnce( - expression, - makeRowVector({BaseVector::createNullConstant( - TIMESTAMP_WITH_TIME_ZONE(), 1, pool())})); - } - - return evaluateOnce( - expression, - makeRowVector({makeTimestampWithTimeZoneVector( - timestamp.value(), timeZoneName.value().c_str())})); - } - - VectorPtr evaluateWithTimestampWithTimezone( - const std::string& expression, - std::optional timestamp, - const std::optional& timeZoneName) { - if (!timestamp.has_value() || !timeZoneName.has_value()) { - return evaluate( - expression, - makeRowVector({BaseVector::createNullConstant( - TIMESTAMP_WITH_TIME_ZONE(), 1, pool())})); - } - - return evaluate( - expression, - makeRowVector({makeTimestampWithTimeZoneVector( - timestamp.value(), timeZoneName.value().c_str())})); - } - VectorPtr makeTimestampWithTimeZoneVector(int64_t timestamp, const char* tz) { auto tzid = tz::getTimeZoneID(tz); @@ -506,12 +471,16 @@ TEST_F(DateTimeFunctionsTest, weekTimestampWithTimezone) { }); auto timestamp = ts.getSeconds() * 100'000'000; - auto week = evaluateWithTimestampWithTimezone( - "week(c0)", timestamp, timezone) - .value(); - auto weekOfYear = evaluateWithTimestampWithTimezone( - "week_of_year(c0)", timestamp, timezone) - .value(); + auto week = *evaluateOnce( + "week(c0)", + TIMESTAMP_WITH_TIME_ZONE(), + TimestampWithTimezone::pack( + TimestampWithTimezone(timestamp, timezone))); + auto weekOfYear = *evaluateOnce( + "week_of_year(c0)", + TIMESTAMP_WITH_TIME_ZONE(), + TimestampWithTimezone::pack( + TimestampWithTimezone(timestamp, timezone))); VELOX_CHECK_EQ( week, weekOfYear, "week and week_of_year must return the same value"); return week; @@ -1801,12 +1770,14 @@ TEST_F(DateTimeFunctionsTest, dateTruncTimeStampWithTimezoneForWeek) { int64_t inputTimestamp, const std::string& timeZone, int64_t expectedTimestamp) { - assertEqualVectors( - makeTimestampWithTimeZoneVector(expectedTimestamp, timeZone.c_str()), - evaluateWithTimestampWithTimezone( + EXPECT_EQ( + TimestampWithTimezone::pack( + TimestampWithTimezone(expectedTimestamp, timeZone.c_str())), + evaluateOnce( fmt::format("date_trunc('{}', c0)", truncUnit), - inputTimestamp, - timeZone)); + TIMESTAMP_WITH_TIME_ZONE(), + TimestampWithTimezone::pack( + TimestampWithTimezone(inputTimestamp, timeZone)))); }; // input 2023-08-07 00:00:00 (19576 days) with timeZone +01:00 // output 2023-08-06 23:00:00" in UTC.(1691362800000) @@ -1889,12 +1860,14 @@ TEST_F(DateTimeFunctionsTest, dateTruncTimestampWithTimezone) { int64_t inputTimestamp, const std::string& timeZone, int64_t expectedTimestamp) { - assertEqualVectors( - makeTimestampWithTimeZoneVector(expectedTimestamp, timeZone.c_str()), - evaluateWithTimestampWithTimezone( + EXPECT_EQ( + TimestampWithTimezone::pack( + TimestampWithTimezone(expectedTimestamp, timeZone.c_str())), + evaluateOnce( fmt::format("date_trunc('{}', c0)", truncUnit), - inputTimestamp, - timeZone)); + TIMESTAMP_WITH_TIME_ZONE(), + TimestampWithTimezone::pack( + TimestampWithTimezone(inputTimestamp, timeZone)))); }; evaluateDateTrunc("second", 123, "+01:00", 0); @@ -4015,10 +3988,16 @@ TEST_F(DateTimeFunctionsTest, dateFunctionTimestampWithTimezone) { const auto dateFunction = [&](std::optional timestamp, const std::optional& timeZoneName) { - auto r1 = evaluateWithTimestampWithTimezone( - "date(c0)", timestamp, timeZoneName); - auto r2 = evaluateWithTimestampWithTimezone( - "cast(c0 as date)", timestamp, timeZoneName); + auto r1 = evaluateOnce( + "date(c0)", + TIMESTAMP_WITH_TIME_ZONE(), + TimestampWithTimezone::pack( + TimestampWithTimezone(*timestamp, *timeZoneName))); + auto r2 = evaluateOnce( + "cast(c0 as date)", + TIMESTAMP_WITH_TIME_ZONE(), + TimestampWithTimezone::pack( + TimestampWithTimezone(*timestamp, *timeZoneName))); EXPECT_EQ(r1, r2); return r1; }; @@ -4213,8 +4192,11 @@ TEST_F(DateTimeFunctionsTest, timeZoneHour) { const auto timezone_hour = [&](const char* time, const char* timezone) { Timestamp ts = parseTimestamp(time); auto timestamp = ts.toMillis(); - auto hour = evaluateWithTimestampWithTimezone( - "timezone_hour(c0)", timestamp, timezone) + auto hour = evaluateOnce( + "timezone_hour(c0)", + TIMESTAMP_WITH_TIME_ZONE(), + TimestampWithTimezone::pack( + TimestampWithTimezone(timestamp, timezone))) .value(); return hour; }; @@ -4253,8 +4235,11 @@ TEST_F(DateTimeFunctionsTest, timeZoneMinute) { const auto timezone_minute = [&](const char* time, const char* timezone) { Timestamp ts = parseTimestamp(time); auto timestamp = ts.toMillis(); - auto minute = evaluateWithTimestampWithTimezone( - "timezone_minute(c0)", timestamp, timezone) + auto minute = evaluateOnce( + "timezone_minute(c0)", + TIMESTAMP_WITH_TIME_ZONE(), + TimestampWithTimezone::pack( + TimestampWithTimezone(timestamp, timezone))) .value(); return minute; };