Skip to content

Commit

Permalink
Refactor timestamp with timezone unit tests (#10187)
Browse files Browse the repository at this point in the history 
Summary:
Pull Request resolved: #10187

Refactoring some of the unit tests using the timestamp with time zone data
type to make them more concise and consistent with other tests by using
"evaluateOnce()", in addition to allowing it to tests null input and output.
Also adding some more convenience functions to the test helper class
TimestampWithTimezone.

Looking into this since it was pretty hard to understand and compare output of
date_add() unit tests, since they were done based on the packed values.

There is still *a lot* of clean up to be done in this unit test suite. Fixing a
few callsites for now, but leaving additional callsites to a future refactor.

Reviewed By: mbasmanova

Differential Revision: D58471534

fbshipit-source-id: 680879e5f374889ddddc92a7caa42a0ca3559f05
  • Loading branch information
@pedroerp @facebook-github-bot
pedroerp authored and facebook-github-bot committed Jun 14, 2024
1 parent 8c9d2d5 commit 39743cd
Showing 1 changed file with 86 additions and 83 deletions.
169 changes: 86 additions & 83 deletions velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,37 @@ class DateTimeFunctionsTest : public functions::test::FunctionBaseTest {
}

public:
// Helper class to manipulate timestamp with timezone types in tests. Provided
// only for convenience.
struct TimestampWithTimezone {
TimestampWithTimezone(int64_t milliSeconds, int16_t timezoneId)
: milliSeconds_(milliSeconds), timezoneId_(timezoneId) {}

TimestampWithTimezone(int64_t milliSeconds, std::string_view timezoneName)
: TimestampWithTimezone{
milliSeconds,
util::getTimeZoneID(timezoneName)} {}

int64_t milliSeconds_{0};
int16_t timezoneId_{0};

static std::optional<int64_t> pack(
std::optional<TimestampWithTimezone> zone) {
if (zone.has_value()) {
return facebook::velox::pack(zone->milliSeconds_, zone->timezoneId_);
}
return std::nullopt;
}

static std::optional<TimestampWithTimezone> unpack(
std::optional<int64_t> input) {
if (input.has_value()) {
return TimestampWithTimezone(
unpackMillisUtc(input.value()), unpackZoneKeyId(input.value()));
}
return std::nullopt;
}

// Provides a nicer printer for gtest.
friend std::ostream& operator<<(
std::ostream& os,
Expand All @@ -98,27 +122,6 @@ class DateTimeFunctionsTest : public functions::test::FunctionBaseTest {
}
};

std::optional<TimestampWithTimezone> parseDatetime(
const std::optional<std::string>& input,
const std::optional<std::string>& format) {
auto resultVector = evaluate(
"parse_datetime(c0, c1)",
makeRowVector(
{makeNullableFlatVector<std::string>({input}),
makeNullableFlatVector<std::string>({format})}));
EXPECT_EQ(1, resultVector->size());

if (resultVector->isNullAt(0)) {
return std::nullopt;
}

auto timestampWithTimezone =
resultVector->as<FlatVector<int64_t>>()->valueAt(0);
return TimestampWithTimezone{
unpackMillisUtc(timestampWithTimezone),
unpackZoneKeyId(timestampWithTimezone)};
}

std::optional<Timestamp> dateParse(
const std::optional<std::string>& input,
const std::optional<std::string>& format) {
Expand Down Expand Up @@ -157,19 +160,6 @@ class DateTimeFunctionsTest : public functions::test::FunctionBaseTest {
return resultVector->as<SimpleVector<StringView>>()->valueAt(0);
}

std::optional<std::string> formatDatetimeWithTimezone(
std::optional<Timestamp> timestamp,
std::optional<std::string> timeZoneName,
const std::string& format) {
auto resultVector = evaluate(
"format_datetime(c0, c1)",
makeRowVector(
{makeTimestampWithTimeZoneVector(
timestamp.value().toMillis(), timeZoneName.value().c_str()),
makeNullableFlatVector<std::string>({format})}));
return resultVector->as<SimpleVector<StringView>>()->valueAt(0);
}

template <typename T>
std::optional<T> evaluateWithTimestampWithTimezone(
const std::string& expression,
Expand Down Expand Up @@ -2335,35 +2325,38 @@ TEST_F(DateTimeFunctionsTest, dateAddTimestamp) {
}

TEST_F(DateTimeFunctionsTest, dateAddTimestampWithTimeZone) {
const auto dateAdd = [&](const std::string& unit,
std::optional<int32_t> value,
std::optional<int64_t> timestamp,
const std::optional<std::string>& timeZoneName) {
return evaluateWithTimestampWithTimezone(
fmt::format("date_add('{}', {}, c0)", unit, *value),
timestamp,
timeZoneName);
};
auto dateAdd =
[&](std::optional<std::string> unit,
std::optional<int32_t> value,
std::optional<TimestampWithTimezone> timestampWithTimezone) {
auto result = evaluateOnce<int64_t>(
"date_add(c0, c1, c2)",
{VARCHAR(), INTEGER(), TIMESTAMP_WITH_TIME_ZONE()},
unit,
value,
TimestampWithTimezone::pack(timestampWithTimezone));
return TimestampWithTimezone::unpack(result);
};

// 1970-01-01 00:00:00.000 UTC-8
auto result = dateAdd("day", 5, 0, "-08:00");
auto expected = makeTimestampWithTimeZoneVector(432000000, "-08:00");
assertEqualVectors(expected, result);
EXPECT_EQ(
TimestampWithTimezone(432000000, "-08:00"),
dateAdd("day", 5, TimestampWithTimezone(0, "-08:00")));

// 2023-01-08 00:00:00.000 UTC-8
result = dateAdd("day", -7, 1673136000, "-08:00");
expected = makeTimestampWithTimeZoneVector(1068336000, "-08:00");
assertEqualVectors(expected, result);
EXPECT_EQ(
TimestampWithTimezone(1068336000, "-08:00"),
dateAdd("day", -7, TimestampWithTimezone(1673136000, "-08:00")));

// 2023-01-08 00:00:00.000 UTC-8
result = dateAdd("millisecond", -7, 1673136000, "-08:00");
expected = makeTimestampWithTimeZoneVector(1673135993, "-08:00");
assertEqualVectors(expected, result);
EXPECT_EQ(
TimestampWithTimezone(1673135993, "-08:00"),
dateAdd("millisecond", -7, TimestampWithTimezone(1673136000, "-08:00")));

// 2023-01-08 00:00:00.000 UTC-8
result = dateAdd("millisecond", +7, 1673136000, "-08:00");
expected = makeTimestampWithTimeZoneVector(1673136007, "-08:00");
assertEqualVectors(expected, result);
EXPECT_EQ(
TimestampWithTimezone(1673136007, "-08:00"),
dateAdd("millisecond", +7, TimestampWithTimezone(1673136000, "-08:00")));

const auto evaluateDateAddFromStrings = [&](const std::string& unit,
int32_t value,
Expand Down Expand Up @@ -2821,6 +2814,13 @@ TEST_F(DateTimeFunctionsTest, dateDiffTimestampWithTimezone) {
}

TEST_F(DateTimeFunctionsTest, parseDatetime) {
const auto parseDatetime = [&](const std::optional<std::string>& input,
const std::optional<std::string>& format) {
auto result =
evaluateOnce<int64_t>("parse_datetime(c0, c1)", input, format);
return TimestampWithTimezone::unpack(result);
};

// Check null behavior.
EXPECT_EQ(std::nullopt, parseDatetime("1970-01-01", std::nullopt));
EXPECT_EQ(std::nullopt, parseDatetime(std::nullopt, "YYYY-MM-dd"));
Expand Down Expand Up @@ -2853,47 +2853,46 @@ TEST_F(DateTimeFunctionsTest, parseDatetime) {
// 118860000 is the number of milliseconds since epoch at 1970-01-02
// 09:01:00.000 UTC.
EXPECT_EQ(
TimestampWithTimezone(118860000, util::getTimeZoneID("+00:00")),
TimestampWithTimezone(118860000, "+00:00"),
parseDatetime("1970-01-02+09:01+00:00", "YYYY-MM-dd+HH:mmZZ"));
EXPECT_EQ(
TimestampWithTimezone(118860000, util::getTimeZoneID("-09:00")),
TimestampWithTimezone(118860000, "-09:00"),
parseDatetime("1970-01-02+00:01-09:00", "YYYY-MM-dd+HH:mmZZ"));
EXPECT_EQ(
TimestampWithTimezone(118860000, util::getTimeZoneID("-02:00")),
TimestampWithTimezone(118860000, "-02:00"),
parseDatetime("1970-01-02+07:01-02:00", "YYYY-MM-dd+HH:mmZZ"));
EXPECT_EQ(
TimestampWithTimezone(118860000, util::getTimeZoneID("+14:00")),
TimestampWithTimezone(118860000, "+14:00"),
parseDatetime("1970-01-02+23:01+14:00", "YYYY-MM-dd+HH:mmZZ"));
EXPECT_EQ(
TimestampWithTimezone(
198060000, util::getTimeZoneID("America/Los_Angeles")),
TimestampWithTimezone(198060000, "America/Los_Angeles"),
parseDatetime("1970-01-02+23:01 PST", "YYYY-MM-dd+HH:mm z"));
EXPECT_EQ(
TimestampWithTimezone(169260000, util::getTimeZoneID("+00:00")),
TimestampWithTimezone(169260000, "+00:00"),
parseDatetime("1970-01-02+23:01 GMT", "YYYY-MM-dd+HH:mm z"));

setQueryTimeZone("Asia/Kolkata");

// 66600000 is the number of millisecond since epoch at 1970-01-01
// 18:30:00.000 UTC.
EXPECT_EQ(
TimestampWithTimezone(66600000, util::getTimeZoneID("Asia/Kolkata")),
TimestampWithTimezone(66600000, "Asia/Kolkata"),
parseDatetime("1970-01-02+00:00", "YYYY-MM-dd+HH:mm"));
EXPECT_EQ(
TimestampWithTimezone(66600000, util::getTimeZoneID("-03:00")),
TimestampWithTimezone(66600000, "-03:00"),
parseDatetime("1970-01-01+15:30-03:00", "YYYY-MM-dd+HH:mmZZ"));

// -66600000 is the number of millisecond since epoch at 1969-12-31
// 05:30:00.000 UTC.
EXPECT_EQ(
TimestampWithTimezone(-66600000, util::getTimeZoneID("Asia/Kolkata")),
TimestampWithTimezone(-66600000, "Asia/Kolkata"),
parseDatetime("1969-12-31+11:00", "YYYY-MM-dd+HH:mm"));
EXPECT_EQ(
TimestampWithTimezone(-66600000, util::getTimeZoneID("+02:00")),
TimestampWithTimezone(-66600000, "+02:00"),
parseDatetime("1969-12-31+07:30+02:00", "YYYY-MM-dd+HH:mmZZ"));

// Joda also lets 'Z' to be UTC|UCT|GMT|GMT0.
auto ts = TimestampWithTimezone(1708840800000, util::getTimeZoneID("GMT"));
auto ts = TimestampWithTimezone(1708840800000, "GMT");
EXPECT_EQ(
ts, parseDatetime("2024-02-25+06:00:99 GMT", "yyyy-MM-dd+HH:mm:99 ZZZ"));
EXPECT_EQ(
Expand Down Expand Up @@ -3181,6 +3180,16 @@ TEST_F(DateTimeFunctionsTest, formatDateTime) {
}

TEST_F(DateTimeFunctionsTest, formatDateTimeTimezone) {
const auto formatDatetimeWithTimezone =
[&](std::optional<TimestampWithTimezone> timestampWithTimezone,
std::optional<std::string> format) {
return evaluateOnce<std::string>(
"format_datetime(c0, c1)",
{TIMESTAMP_WITH_TIME_ZONE(), VARCHAR()},
TimestampWithTimezone::pack(timestampWithTimezone),
format);
};

const auto zeroTs = parseTimestamp("1970-01-01");

// No timezone set; default to GMT.
Expand All @@ -3191,12 +3200,14 @@ TEST_F(DateTimeFunctionsTest, formatDateTimeTimezone) {
EXPECT_EQ(
"1970-01-01 05:30:00",
formatDatetimeWithTimezone(
zeroTs, "Asia/Kolkata", "YYYY-MM-dd HH:mm:ss"));
TimestampWithTimezone(zeroTs.toMillis(), "Asia/Kolkata"),
"YYYY-MM-dd HH:mm:ss"));

EXPECT_EQ(
"1969-12-31 16:00:00",
formatDatetimeWithTimezone(
zeroTs, "America/Los_Angeles", "YYYY-MM-dd HH:mm:ss"));
TimestampWithTimezone(zeroTs.toMillis(), "America/Los_Angeles"),
"YYYY-MM-dd HH:mm:ss"));
}

TEST_F(DateTimeFunctionsTest, dateFormat) {
Expand Down Expand Up @@ -3519,10 +3530,7 @@ TEST_F(DateTimeFunctionsTest, fromIso8601Timestamp) {
const auto fromIso = [&](const std::string& input) {
auto result =
evaluateOnce<int64_t, std::string>("from_iso8601_timestamp(c0)", input);
VELOX_CHECK(result.has_value());

return TimestampWithTimezone(
unpackMillisUtc(result.value()), unpackZoneKeyId(result.value()));
return TimestampWithTimezone::unpack(result);
};

// Full strings with different time zones.
Expand All @@ -3531,21 +3539,16 @@ TEST_F(DateTimeFunctionsTest, fromIso8601Timestamp) {
const std::string ts = "1970-01-02T11:38:56.123";

EXPECT_EQ(
TimestampWithTimezone(
millis + 5 * kMillisInHour, util::getTimeZoneID("-05:00")),
TimestampWithTimezone(millis + 5 * kMillisInHour, "-05:00"),
fromIso(ts + "-05:00"));

EXPECT_EQ(
TimestampWithTimezone(
millis - 8 * kMillisInHour, util::getTimeZoneID("+08:00")),
TimestampWithTimezone(millis - 8 * kMillisInHour, "+08:00"),
fromIso(ts + "+08:00"));

EXPECT_EQ(
TimestampWithTimezone(millis, util::getTimeZoneID("UTC")),
fromIso(ts + "Z"));
EXPECT_EQ(TimestampWithTimezone(millis, "UTC"), fromIso(ts + "Z"));

EXPECT_EQ(
TimestampWithTimezone(millis, util::getTimeZoneID("UTC")), fromIso(ts));
EXPECT_EQ(TimestampWithTimezone(millis, "UTC"), fromIso(ts));

// Partial strings with different session time zones.
struct {
Expand All @@ -3556,7 +3559,7 @@ TEST_F(DateTimeFunctionsTest, fromIso8601Timestamp) {
{"Asia/Kolkata", 5 * kMillisInHour + 30 * kMillisInMinute},
};

for (auto timezone : timezones) {
for (const auto& timezone : timezones) {
setQueryTimeZone(timezone.name);

const auto timezoneId = util::getTimeZoneID(timezone.name);
Expand Down

0 comments on commit 39743cd

Please sign in to comment.