diff --git a/velox/functions/lib/DateTimeFormatter.cpp b/velox/functions/lib/DateTimeFormatter.cpp index 5534ae62b6c6a..a9df69d71f050 100644 --- a/velox/functions/lib/DateTimeFormatter.cpp +++ b/velox/functions/lib/DateTimeFormatter.cpp @@ -1167,10 +1167,17 @@ uint32_t DateTimeFormatter::maxResultSize(const tz::TimeZone* timezone) const { size += std::max((int)token.pattern.minRepresentDigits, 9); break; case DateTimeFormatSpecifier::TIMEZONE: - VELOX_NYI( - "Date format specifier is not yet implemented: {} ({})", - getSpecifierName(token.pattern.specifier), - token.pattern.minRepresentDigits); + if (token.pattern.minRepresentDigits <= 3) { + // The longest abbreviation according to here is 5, e.g. some time + // zones use the offset as the abbreviation, like +0530. + // https://en.wikipedia.org/wiki/List_of_tz_database_time_zones + size += 5; + } else { + VELOX_NYI( + "Date format specifier is not yet implemented: {} ({})", + getSpecifierName(token.pattern.specifier), + token.pattern.minRepresentDigits); + } break; case DateTimeFormatSpecifier::TIMEZONE_OFFSET_ID: @@ -1412,9 +1419,20 @@ int32_t DateTimeFormatter::format( } break; case DateTimeFormatSpecifier::TIMEZONE: { - // TODO: implement short name time zone, need a map from full name - // to short name - VELOX_UNSUPPORTED("time zone name is not yet supported"); + VELOX_USER_CHECK_NOT_NULL(timezone); + if (token.pattern.minRepresentDigits <= 3) { + const std::string& abbrev = + tz::locateZone(timezone->id()) + ->tz() + ->get_info(date::local_seconds( + std::chrono::seconds(timestamp.getSeconds()))) + .first.abbrev; + std::memcpy(result, abbrev.data(), abbrev.length()); + result += abbrev.length(); + } else { + // TODO: implement full name time zone + VELOX_NYI("full time zone name is not yet supported"); + } } break; case DateTimeFormatSpecifier::TIMEZONE_OFFSET_ID: { diff --git a/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp b/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp index 97807df92a0e6..bb5a717740e37 100644 --- a/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp +++ b/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp @@ -3269,6 +3269,13 @@ TEST_F(DateTimeFunctionsTest, formatDateTime) { EXPECT_EQ("+05:30", formatDatetime(parseTimestamp("1970-01-01"), "ZZ")); EXPECT_EQ("+0530", formatDatetime(parseTimestamp("1970-01-01"), "Z")); + EXPECT_EQ("IST", formatDatetime(parseTimestamp("1970-01-01"), "zzz")); + EXPECT_EQ("IST", formatDatetime(parseTimestamp("1970-01-01"), "zz")); + EXPECT_EQ("IST", formatDatetime(parseTimestamp("1970-01-01"), "z")); + // We don't support more than 3 'z's yet. + EXPECT_THROW( + formatDatetime(parseTimestamp("1970-01-01"), "zzzz"), VeloxRuntimeError); + // Literal test cases. EXPECT_EQ("hello", formatDatetime(parseTimestamp("1970-01-01"), "'hello'")); EXPECT_EQ("'", formatDatetime(parseTimestamp("1970-01-01"), "''")); @@ -3313,15 +3320,14 @@ TEST_F(DateTimeFunctionsTest, formatDateTime) { EXPECT_THROW( formatDatetime(parseTimestamp("1970-01-01"), "'abcd"), VeloxUserError); - // System errors for patterns we haven't implemented yet. + // Time zone name patterns aren't supported when there isn't a time zone + // available. EXPECT_THROW( - formatDatetime(parseTimestamp("1970-01-01"), "z"), VeloxRuntimeError); + formatDatetime(parseTimestamp("1970-01-01"), "z"), VeloxUserError); EXPECT_THROW( - formatDatetime(parseTimestamp("1970-01-01"), "zz"), VeloxRuntimeError); + formatDatetime(parseTimestamp("1970-01-01"), "zz"), VeloxUserError); EXPECT_THROW( - formatDatetime(parseTimestamp("1970-01-01"), "zzz"), VeloxRuntimeError); - EXPECT_THROW( - formatDatetime(parseTimestamp("1970-01-01"), "zzzz"), VeloxRuntimeError); + formatDatetime(parseTimestamp("1970-01-01"), "zzz"), VeloxUserError); } TEST_F(DateTimeFunctionsTest, formatDateTimeTimezone) {