From b3d55fee239a6c5545957f5a718edb421d0eba4b Mon Sep 17 00:00:00 2001 From: rexan Date: Thu, 10 Oct 2024 23:31:36 -0400 Subject: [PATCH] Address comments --- velox/functions/sparksql/DateTimeFunctions.h | 7 +++-- .../sparksql/tests/DateTimeFunctionsTest.cpp | 27 ++++++++++++------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/velox/functions/sparksql/DateTimeFunctions.h b/velox/functions/sparksql/DateTimeFunctions.h index 529af1bf81c7..bd920ed4d930 100644 --- a/velox/functions/sparksql/DateTimeFunctions.h +++ b/velox/functions/sparksql/DateTimeFunctions.h @@ -235,16 +235,15 @@ struct UnixTimestampParseWithFormatFunction FOLLY_ALWAYS_INLINE void call( int64_t& result, const arg_type& input, - const arg_type& format) { + const arg_type& /*format*/) { result = input.getSeconds(); } FOLLY_ALWAYS_INLINE void call( int64_t& result, const arg_type& input, - const arg_type& format) { - auto seconds = input * kSecondsInDay; - Timestamp timestamp{seconds, 0}; + const arg_type& /*format*/) { + auto timestamp = Timestamp::fromDate(input); timestamp.toGMT(*this->sessionTimeZone_); result = timestamp.getSeconds(); } diff --git a/velox/functions/sparksql/tests/DateTimeFunctionsTest.cpp b/velox/functions/sparksql/tests/DateTimeFunctionsTest.cpp index ddd4c44ea5b0..49a2d659650e 100644 --- a/velox/functions/sparksql/tests/DateTimeFunctionsTest.cpp +++ b/velox/functions/sparksql/tests/DateTimeFunctionsTest.cpp @@ -301,25 +301,32 @@ TEST_F(DateTimeFunctionsTest, unixTimestampCustomFormat) { EXPECT_EQ( std::nullopt, unixTimestamp("2022-12-12 asd 07:45:31", "yyyy-MM-dd 'asd HH:mm:ss")); +} - const auto unixTimestamp1 = [&](std::optional timestamp, - std::optional formatStr) { +TEST_F(DateTimeFunctionsTest, unixTimestampTimestampFormat) { + const auto unixTimestamp = [&](std::optional timestamp, + std::optional formatStr) { return evaluateOnce( "unix_timestamp(c0, c1)", timestamp, formatStr); }; - EXPECT_EQ(0, unixTimestamp1(Timestamp(0, 0), "yyyy-MM-dd")); - EXPECT_EQ(1, unixTimestamp1(Timestamp(1, 990), "yyyy-MM-dd")); - EXPECT_EQ(61, unixTimestamp1(Timestamp(61, 0), "yyyy-MM-dd")); + EXPECT_EQ(0, unixTimestamp(Timestamp(0, 0), "yyyy-MM-dd")); + EXPECT_EQ(1, unixTimestamp(Timestamp(1, 990), "yyyy-MM-dd")); + EXPECT_EQ(61, unixTimestamp(Timestamp(61, 0), "yyyy-MM-dd")); +} - const auto unixTimestamp2 = [&](std::optional date, - std::optional formatStr) { +TEST_F(DateTimeFunctionsTest, unixTimestampDateFormat) { + const auto unixTimestamp = [&](std::optional date, + std::optional formatStr) { return evaluateOnce( "unix_timestamp(c0, c1)", {DATE(), VARCHAR()}, date, formatStr); }; - EXPECT_EQ(0, unixTimestamp2(parseDate("1970-01-01"), "yyyy-MM-dd")); - EXPECT_EQ(1727740800, unixTimestamp2(parseDate("2024-10-01"), "yyyy-MM-dd")); + EXPECT_EQ(0, unixTimestamp(parseDate("1970-01-01"), "yyyy-MM-dd")); + EXPECT_EQ(1727740800, unixTimestamp(parseDate("2024-10-01"), "yyyy-MM-dd")); + VELOX_ASSERT_THROW( + unixTimestamp(kMax, "yyyy-MM-dd"), + "Timepoint is outside of supported year range"); setQueryTimeZone("America/Los_Angeles"); - EXPECT_EQ(1727766000, unixTimestamp2(parseDate("2024-10-01"), "yyyy-MM-dd")); + EXPECT_EQ(1727766000, unixTimestamp(parseDate("2024-10-01"), "yyyy-MM-dd")); } // unix_timestamp and to_unix_timestamp are aliases.